patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v3 1/6] net/virtio-user: fix backend selection if stat fails
       [not found] <20201026163930.94032-1-amorenoz@redhat.com>
@ 2020-10-26 16:39 ` Adrian Moreno
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 2/6] net/virtio-user: don't set/get_status until FEATURES_OK Adrian Moreno
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Adrian Moreno @ 2020-10-26 16:39 UTC (permalink / raw)
  To: dev
  Cc: yinan.wang, patrick.fu, chenbo.xia, zhihong.wang,
	maxime.coquelin, Adrian Moreno, stable, Kevin Traynor

If stat fails because the file does not exist, it means that
the backend must be vhost-user in server mode.

Also, log the detected backend type.

Bugzilla ID: 559
Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev")
Cc: stable@dpdk.org

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +++++++
 drivers/net/virtio/virtio_user/virtio_user_dev.h | 1 +
 drivers/net/virtio/virtio_user_ethdev.c          | 6 +++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 3681758ef..27814eadb 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -22,6 +22,13 @@
 
 #define VIRTIO_USER_MEM_EVENT_CLB_NAME "virtio_user_mem_event_clb"
 
+const char * const virtio_user_backend_strings[] = {
+	[VIRTIO_USER_BACKEND_UNKNOWN] = "VIRTIO_USER_BACKEND_UNKNOWN",
+	[VIRTIO_USER_BACKEND_VHOST_USER] = "VHOST_USER",
+	[VIRTIO_USER_BACKEND_VHOST_KERNEL] = "VHOST_NET",
+	[VIRTIO_USER_BACKEND_VHOST_VDPA] = "VHOST_VDPA",
+};
+
 static int
 virtio_user_create_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
 {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 3e9d1a1eb..998986875 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -83,4 +83,5 @@ void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
 uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
 int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status);
 int virtio_user_update_status(struct virtio_user_dev *dev);
+extern const char * const virtio_user_backend_strings[];
 #endif
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 042665bc0..e870fb2ff 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -560,6 +560,9 @@ virtio_user_backend_type(const char *path)
 	struct stat sb;
 
 	if (stat(path, &sb) == -1) {
+		if (errno == ENOENT)
+			return VIRTIO_USER_BACKEND_VHOST_USER;
+
 		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
 			     strerror(errno));
 		return VIRTIO_USER_BACKEND_UNKNOWN;
@@ -697,7 +700,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 			path);
 		goto end;
 	}
-
+	PMD_INIT_LOG(INFO, "Backend type detected: %s",
+		     virtio_user_backend_strings[backend_type]);
 
 	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
 		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-stable] [PATCH v3 2/6] net/virtio-user: don't set/get_status until FEATURES_OK
       [not found] <20201026163930.94032-1-amorenoz@redhat.com>
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 1/6] net/virtio-user: fix backend selection if stat fails Adrian Moreno
@ 2020-10-26 16:39 ` Adrian Moreno
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 5/6] net/virtio-user: don't assume vhost status feature Adrian Moreno
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 6/6] net/virtio-user: set status on socket reconnect Adrian Moreno
  3 siblings, 0 replies; 6+ messages in thread
From: Adrian Moreno @ 2020-10-26 16:39 UTC (permalink / raw)
  To: dev
  Cc: yinan.wang, patrick.fu, chenbo.xia, zhihong.wang,
	maxime.coquelin, Adrian Moreno, stable

According to the virtio spec, ACK and DRIVER status bits should be set
before feature negotiation.

However, until the protocol features are negotiated, the driver does not
know if the device actually supports those vhost-user messages.
Therefore, until FEATURES_OK is set, the GET/SET_STATUS messages should
not be sent.

Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
Cc: maxime.coquelin@redhat.com
Cc: stable@dpdk.org

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost_user.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index ef290c357..450d77e92 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -278,8 +278,9 @@ vhost_user_sock(struct virtio_user_dev *dev,
 
 	switch (req) {
 	case VHOST_USER_GET_STATUS:
-		if (!(dev->protocol_features &
-				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+		if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) ||
+		    (!(dev->protocol_features &
+				(1ULL << VHOST_USER_PROTOCOL_F_STATUS))))
 			return 0;
 		/* Fallthrough */
 	case VHOST_USER_GET_FEATURES:
@@ -288,8 +289,9 @@ vhost_user_sock(struct virtio_user_dev *dev,
 		break;
 
 	case VHOST_USER_SET_STATUS:
-		if (!(dev->protocol_features &
-				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+		if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) ||
+		    (!(dev->protocol_features &
+				(1ULL << VHOST_USER_PROTOCOL_F_STATUS))))
 			return 0;
 
 		if (has_reply_ack)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-stable] [PATCH v3 5/6] net/virtio-user: don't assume vhost status feature
       [not found] <20201026163930.94032-1-amorenoz@redhat.com>
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 1/6] net/virtio-user: fix backend selection if stat fails Adrian Moreno
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 2/6] net/virtio-user: don't set/get_status until FEATURES_OK Adrian Moreno
@ 2020-10-26 16:39 ` Adrian Moreno
  2020-10-28 10:37   ` Maxime Coquelin
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 6/6] net/virtio-user: set status on socket reconnect Adrian Moreno
  3 siblings, 1 reply; 6+ messages in thread
From: Adrian Moreno @ 2020-10-26 16:39 UTC (permalink / raw)
  To: dev
  Cc: yinan.wang, patrick.fu, chenbo.xia, zhihong.wang,
	maxime.coquelin, Adrian Moreno, stable

There are some status reads and updates that need to happen before the
protocol features are negotiated. Therefore, assuming the backend does
support this feature can lead to failures.

On server mode, do not assume the backend supports
VHOST_USER_PROTOCOL_F_STATUS. Activate it back on reconnection and
cleare it on disconnection.

Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
Cc: maxime.coquelin@redhat.com
Cc: stable@dpdk.org

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ++++++
 drivers/net/virtio/virtio_user_ethdev.c          | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 36e5619df..053f0267c 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -523,6 +523,12 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		 * later.
 		 */
 		dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES;
+
+		/* We cannot assume VHOST_USER_PROTOCOL_F_STATUS is supported
+		 * until it's negotiated
+		 */
+		dev->protocol_features &=
+			~(1ULL << VHOST_USER_PROTOCOL_F_STATUS);
 	}
 
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 97ddc5651..142bdc0bd 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -92,6 +92,9 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev)
 					&protocol_features))
 			return -1;
 
+		/* Offer VHOST_USER_PROTOCOL_F_STATUS */
+		dev->protocol_features |=
+			(1ULL << VHOST_USER_PROTOCOL_F_STATUS);
 		dev->protocol_features &= protocol_features;
 
 		if (dev->ops->send_request(dev,
@@ -168,6 +171,11 @@ virtio_user_delayed_handler(void *param)
 		if (dev->vhostfd >= 0) {
 			close(dev->vhostfd);
 			dev->vhostfd = -1;
+			/* Until the featuers are negotiated again, don't assume
+			 * the backend supports VHOST_USER_PROTOCOL_F_STATUS
+			 */
+			dev->protocol_features &=
+				~(1ULL << VHOST_USER_PROTOCOL_F_STATUS);
 		}
 		eth_dev->intr_handle->fd = dev->listenfd;
 		rte_intr_callback_register(eth_dev->intr_handle,
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-stable] [PATCH v3 6/6] net/virtio-user: set status on socket reconnect
       [not found] <20201026163930.94032-1-amorenoz@redhat.com>
                   ` (2 preceding siblings ...)
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 5/6] net/virtio-user: don't assume vhost status feature Adrian Moreno
@ 2020-10-26 16:39 ` Adrian Moreno
  2020-10-28 11:01   ` Maxime Coquelin
  3 siblings, 1 reply; 6+ messages in thread
From: Adrian Moreno @ 2020-10-26 16:39 UTC (permalink / raw)
  To: dev
  Cc: yinan.wang, patrick.fu, chenbo.xia, zhihong.wang,
	maxime.coquelin, Adrian Moreno, stable

Newer vhost-user backends will rely on SET_STATUS to start the device
so this required to support them.

Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
Cc: maxime.coquelin@redhat.com
Cc: stable@dpdk.org

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 drivers/net/virtio/virtio_user_ethdev.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 142bdc0bd..40345193e 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -67,8 +67,7 @@ virtio_user_reset_queues_packed(struct rte_eth_dev *dev)
 static int
 virtio_user_server_reconnect(struct virtio_user_dev *dev)
 {
-	int ret;
-	int connectfd;
+	int ret, connectfd, old_status;
 	struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id];
 	struct virtio_hw *hw = eth_dev->data->dev_private;
 	uint64_t protocol_features;
@@ -78,6 +77,14 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev)
 		return -1;
 
 	dev->vhostfd = connectfd;
+	old_status = vtpci_get_status(hw);
+
+	vtpci_reset(hw);
+
+	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK);
+
+	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER);
+
 	if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES,
 				   &dev->device_features) < 0) {
 		PMD_INIT_LOG(ERR, "get_features failed: %s",
@@ -116,14 +123,17 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev)
 
 	/* For packed ring, resetting queues is required in reconnection. */
 	if (vtpci_packed_queue(hw) &&
-	   (vtpci_get_status(hw) & VIRTIO_CONFIG_STATUS_DRIVER_OK)) {
+	   (old_status & VIRTIO_CONFIG_STATUS_DRIVER_OK)) {
 		PMD_INIT_LOG(NOTICE, "Packets on the fly will be dropped"
 				" when packed ring reconnecting.");
 		virtio_user_reset_queues_packed(eth_dev);
 	}
 
-	ret = virtio_user_start_device(dev);
-	if (ret < 0)
+	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_FEATURES_OK);
+
+	/* Start the device */
+	vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER_OK);
+	if (!dev->started)
 		return -1;
 
 	if (dev->queue_pairs > 1) {
-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-stable] [PATCH v3 5/6] net/virtio-user: don't assume vhost status feature
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 5/6] net/virtio-user: don't assume vhost status feature Adrian Moreno
@ 2020-10-28 10:37   ` Maxime Coquelin
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2020-10-28 10:37 UTC (permalink / raw)
  To: Adrian Moreno, dev
  Cc: yinan.wang, patrick.fu, chenbo.xia, zhihong.wang, stable



On 10/26/20 5:39 PM, Adrian Moreno wrote:
> There are some status reads and updates that need to happen before the
> protocol features are negotiated. Therefore, assuming the backend does
> support this feature can lead to failures.
> 
> On server mode, do not assume the backend supports
> VHOST_USER_PROTOCOL_F_STATUS. Activate it back on reconnection and
> cleare it on disconnection.
> 
> Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ++++++
>  drivers/net/virtio/virtio_user_ethdev.c          | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-stable] [PATCH v3 6/6] net/virtio-user: set status on socket reconnect
  2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 6/6] net/virtio-user: set status on socket reconnect Adrian Moreno
@ 2020-10-28 11:01   ` Maxime Coquelin
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Coquelin @ 2020-10-28 11:01 UTC (permalink / raw)
  To: Adrian Moreno, dev
  Cc: yinan.wang, patrick.fu, chenbo.xia, zhihong.wang, stable



On 10/26/20 5:39 PM, Adrian Moreno wrote:
> Newer vhost-user backends will rely on SET_STATUS to start the device
> so this required to support them.
> 
> Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  drivers/net/virtio/virtio_user_ethdev.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-28 11:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20201026163930.94032-1-amorenoz@redhat.com>
2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 1/6] net/virtio-user: fix backend selection if stat fails Adrian Moreno
2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 2/6] net/virtio-user: don't set/get_status until FEATURES_OK Adrian Moreno
2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 5/6] net/virtio-user: don't assume vhost status feature Adrian Moreno
2020-10-28 10:37   ` Maxime Coquelin
2020-10-26 16:39 ` [dpdk-stable] [PATCH v3 6/6] net/virtio-user: set status on socket reconnect Adrian Moreno
2020-10-28 11:01   ` Maxime Coquelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).