DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: dev@dpdk.org, chenbo.xia@intel.com, olivier.matz@6wind.com,
	amorenoz@redhat.com, david.marchand@redhat.com
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Subject: [dpdk-dev] [PATCH 35/40] net/virtio: make server mode blocking
Date: Sun, 20 Dec 2020 22:14:00 +0100	[thread overview]
Message-ID: <20201220211405.313012-36-maxime.coquelin@redhat.com> (raw)
In-Reply-To: <20201220211405.313012-1-maxime.coquelin@redhat.com>

This patch makes the Vhost-user backend server mode
blocking at init, waiting for the client connection.

The goal is to make the driver more reliable, as without
waiting for client connection, the Virtio driver has to
assume the Vhost-user backend will support all the
features it has advertized, which could lead to undefined
behaviour.

For example, without this patch, if the user enables packed
ring Virtio feature but the backend does not support it,
the ring initialized by the driver will not be compatible
with the backend.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost_user.c   |   9 +-
 .../net/virtio/virtio_user/virtio_user_dev.c  | 118 +++++++-----------
 drivers/net/virtio/virtio_user_ethdev.c       |   5 -
 3 files changed, 54 insertions(+), 78 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index a57106a468..94a33326ae 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -677,6 +677,14 @@ virtio_user_start_server(struct virtio_user_dev *dev, struct sockaddr_un *un)
 	if (ret < 0)
 		return -1;
 
+	PMD_DRV_LOG(NOTICE, "(%s) waiting for client connection...", dev->path);
+	dev->vhostfd = accept(fd, NULL, NULL);
+	if (dev->vhostfd < 0) {
+		PMD_DRV_LOG(ERR, "Failed to accept initial client connection (%s)",
+				strerror(errno));
+		return -1;
+	}
+
 	flag = fcntl(fd, F_GETFL);
 	if (fcntl(fd, F_SETFL, flag | O_NONBLOCK) < 0) {
 		PMD_DRV_LOG(ERR, "fcntl failed, %s", strerror(errno));
@@ -721,7 +729,6 @@ vhost_user_setup(struct virtio_user_dev *dev)
 			close(fd);
 			return -1;
 		}
-		dev->vhostfd = -1;
 	} else {
 		if (connect(fd, (struct sockaddr *)&un, sizeof(un)) < 0) {
 			PMD_DRV_LOG(ERR, "connect error, %s", strerror(errno));
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index b92b7f7aae..19d59d401e 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -144,10 +144,6 @@ virtio_user_dev_set_features(struct virtio_user_dev *dev)
 
 	pthread_mutex_lock(&dev->mutex);
 
-	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
-			dev->vhostfd < 0)
-		goto error;
-
 	/* Step 0: tell vhost to create queues */
 	if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0)
 		goto error;
@@ -190,11 +186,6 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	rte_mcfg_mem_read_lock();
 	pthread_mutex_lock(&dev->mutex);
 
-	/* Vhost-user client not connected yet, will start later */
-	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
-			dev->vhostfd < 0)
-		goto out;
-
 	/* Step 2: share memory regions */
 	ret = dev->ops->set_memory_table(dev);
 	if (ret < 0)
@@ -213,7 +204,7 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 		goto error;
 
 	dev->started = true;
-out:
+
 	pthread_mutex_unlock(&dev->mutex);
 	rte_mcfg_mem_read_unlock();
 
@@ -421,36 +412,36 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 			PMD_DRV_LOG(ERR, "Server mode only supports vhost-user!");
 			return -1;
 		}
+	}
+
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
 		dev->ops = &virtio_ops_user;
-	} else {
-		if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
-			dev->ops = &virtio_ops_user;
-		} else if (dev->backend_type ==
-					VIRTIO_USER_BACKEND_VHOST_KERNEL) {
-			dev->ops = &virtio_ops_kernel;
-
-			dev->vhostfds = malloc(dev->max_queue_pairs *
-					       sizeof(int));
-			dev->tapfds = malloc(dev->max_queue_pairs *
-					     sizeof(int));
-			if (!dev->vhostfds || !dev->tapfds) {
-				PMD_INIT_LOG(ERR, "(%s) Failed to allocate FDs", dev->path);
-				return -1;
-			}
-
-			for (q = 0; q < dev->max_queue_pairs; ++q) {
-				dev->vhostfds[q] = -1;
-				dev->tapfds[q] = -1;
-			}
-		} else if (dev->backend_type ==
-				VIRTIO_USER_BACKEND_VHOST_VDPA) {
-			dev->ops = &virtio_ops_vdpa;
-		} else {
-			PMD_DRV_LOG(ERR, "(%s) Unknown backend type", dev->path);
+	} else if (dev->backend_type ==
+			VIRTIO_USER_BACKEND_VHOST_KERNEL) {
+		dev->ops = &virtio_ops_kernel;
+
+		dev->vhostfds = malloc(dev->max_queue_pairs *
+				sizeof(int));
+		dev->tapfds = malloc(dev->max_queue_pairs *
+				sizeof(int));
+		if (!dev->vhostfds || !dev->tapfds) {
+			PMD_INIT_LOG(ERR, "(%s) Failed to allocate FDs", dev->path);
 			return -1;
 		}
+
+		for (q = 0; q < dev->max_queue_pairs; ++q) {
+			dev->vhostfds[q] = -1;
+			dev->tapfds[q] = -1;
+		}
+	} else if (dev->backend_type ==
+			VIRTIO_USER_BACKEND_VHOST_VDPA) {
+		dev->ops = &virtio_ops_vdpa;
+	} else {
+		PMD_DRV_LOG(ERR, "(%s) Unknown backend type", dev->path);
+		return -1;
 	}
 
+
 	if (dev->ops->setup(dev) < 0) {
 		PMD_INIT_LOG(ERR, "(%s) Failed to setup backend\n", dev->path);
 		return -1;
@@ -533,52 +524,35 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		dev->unsupported_features |=
 			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
 
-	if (!dev->is_server) {
-		if (dev->ops->set_owner(dev) < 0) {
-			PMD_INIT_LOG(ERR, "(%s) Failed to set backend owner", dev->path);
-			return -1;
-		}
+	if (dev->ops->set_owner(dev) < 0) {
+		PMD_INIT_LOG(ERR, "(%s) Failed to set backend owner", dev->path);
+		return -1;
+	}
+
+	if (dev->ops->get_features(dev, &dev->device_features) < 0) {
+		PMD_INIT_LOG(ERR, "(%s) Failed to get backend features", dev->path);
+		return -1;
+	}
 
-		if (dev->ops->get_features(dev, &dev->device_features) < 0) {
-			PMD_INIT_LOG(ERR, "(%s) Failed to get backend features", dev->path);
+	if (dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) {
+		if (dev->ops->get_protocol_features(dev, &protocol_features)) {
+			PMD_INIT_LOG(ERR, "(%s) Failed to get backend protocol features",
+					dev->path);
 			return -1;
 		}
 
-		if (dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) {
-			if (dev->ops->get_protocol_features(dev, &protocol_features)) {
-				PMD_INIT_LOG(ERR, "(%s) Failed to get backend protocol features",
-						dev->path);
-				return -1;
-			}
-
-			dev->protocol_features &= protocol_features;
-
-			if (dev->ops->set_protocol_features(dev, dev->protocol_features)) {
-				PMD_INIT_LOG(ERR, "(%s) Failed to set backend protocol features",
-						dev->path);
-				return -1;
-			}
+		dev->protocol_features &= protocol_features;
 
-			if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
-				dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
+		if (dev->ops->set_protocol_features(dev, dev->protocol_features)) {
+			PMD_INIT_LOG(ERR, "(%s) Failed to set backend protocol features",
+					dev->path);
+			return -1;
 		}
-	} else {
-		/* We just pretend vhost-user can support all these features.
-		 * Note that this could be problematic that if some feature is
-		 * negotiated but not supported by the vhost-user which comes
-		 * 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);
+		if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
+			dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
 	}
 
-
-
 	if (!mrg_rxbuf)
 		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MRG_RXBUF);
 
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index c63d010e16..c4cfd3daed 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -174,11 +174,6 @@ 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.29.2


  parent reply	other threads:[~2020-12-20 21:25 UTC|newest]

Thread overview: 149+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-20 21:13 [dpdk-dev] [PATCH 00/40] net/virtio: Virtio PMD rework Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 01/40] bus/vdev: add helper to get vdev from eth dev Maxime Coquelin
2020-12-30  3:02   ` Xia, Chenbo
2021-01-05 21:15   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 02/40] net/virtio: Introduce Virtio bus type Maxime Coquelin
2020-12-30  3:02   ` Xia, Chenbo
2021-01-05 21:15   ` David Marchand
2021-01-14  9:24     ` Maxime Coquelin
2021-01-14 10:54       ` Maxime Coquelin
2021-01-14 11:55         ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 03/40] net/virtio: refactor virtio-user device Maxime Coquelin
2020-12-30  3:02   ` Xia, Chenbo
2021-01-05 21:16   ` David Marchand
2021-01-14  9:26     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 04/40] net/virtio: introduce PCI device metadata Maxime Coquelin
2020-12-30  3:02   ` Xia, Chenbo
2021-01-05 21:16   ` David Marchand
2021-01-14 11:05     ` Maxime Coquelin
2021-01-14 14:40       ` David Marchand
2021-01-14 14:44         ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 05/40] net/virtio: move PCI device init in dedicated file Maxime Coquelin
2020-12-30  3:02   ` Xia, Chenbo
2021-01-05 21:19   ` David Marchand
2021-01-14 16:04     ` Maxime Coquelin
2021-01-14 16:14       ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 06/40] net/virtio: move PCI specific dev init to PCI ethdev init Maxime Coquelin
2020-12-30  3:05   ` Xia, Chenbo
2021-01-06  8:58   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 07/40] net/virtio: move MSIX detection to PCI ethdev Maxime Coquelin
2020-12-30  3:05   ` Xia, Chenbo
2021-01-06  8:22   ` David Marchand
2021-01-14 17:19     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 08/40] net/virtio: force IOVA as VA mode for Virtio-user Maxime Coquelin
2020-12-30  3:06   ` Xia, Chenbo
2021-01-06  9:06   ` David Marchand
2021-01-06  9:11     ` Thomas Monjalon
2021-01-06  9:22       ` Maxime Coquelin
2021-01-06 16:37       ` Kinsella, Ray
2021-01-06  9:14     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 09/40] net/virtio: store PCI type in Virtio device metadata Maxime Coquelin
2020-12-30  3:07   ` Xia, Chenbo
2021-01-06  9:14   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 10/40] net/virtio: add callback for device closing Maxime Coquelin
2020-12-30  3:07   ` Xia, Chenbo
2021-01-06  9:33   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 11/40] net/virtio: validate features at bus level Maxime Coquelin
2020-12-30  3:08   ` Xia, Chenbo
2021-01-06  9:33   ` David Marchand
2021-01-15  9:20     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 12/40] net/virtio: remove bus type enum Maxime Coquelin
2020-12-30  3:08   ` Xia, Chenbo
2021-01-06  9:33   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 13/40] net/virtio: move PCI-specific fields to PCI device Maxime Coquelin
2020-12-30  3:08   ` Xia, Chenbo
2021-01-06  9:58   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 14/40] net/virtio: pack virtio HW struct Maxime Coquelin
2020-12-30  3:09   ` Xia, Chenbo
2021-01-06  9:58   ` David Marchand
2021-01-15  9:35     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 15/40] net/virtio: move legacy IO to Virtio PCI Maxime Coquelin
2020-12-30  3:09   ` Xia, Chenbo
2021-01-06 10:09   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 16/40] net/virtio: introduce generic virtio header Maxime Coquelin
2020-12-30  3:09   ` Xia, Chenbo
2021-01-06 10:08   ` David Marchand
2021-01-15  9:39     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 17/40] net/virtio: move features definition to generic header Maxime Coquelin
2020-12-30  3:14   ` Xia, Chenbo
2021-01-14  8:40     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 18/40] net/virtio: move virtqueue defines in " Maxime Coquelin
2020-12-30  3:14   ` Xia, Chenbo
2021-01-06 15:53   ` David Marchand
2021-01-15 10:55     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 19/40] net/virtio: move config definitions to " Maxime Coquelin
2020-12-30  3:15   ` Xia, Chenbo
2021-01-06 16:01   ` David Marchand
2021-01-15 11:01     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 20/40] net/virtio: make interrupt handling more generic Maxime Coquelin
2020-12-30  3:17   ` Xia, Chenbo
2021-01-14  8:43     ` Maxime Coquelin
2021-01-06 16:07   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 21/40] net/virtio: move vring alignment to generic header Maxime Coquelin
2020-12-30  3:18   ` Xia, Chenbo
2021-01-06 16:13   ` David Marchand
2020-12-20 21:13 ` [dpdk-dev] [PATCH 22/40] net/virtio: remove last PCI refs in non-PCI code Maxime Coquelin
2020-12-30  3:25   ` Xia, Chenbo
2021-01-14  8:46     ` Maxime Coquelin
2021-01-06 16:18   ` David Marchand
2021-01-15 11:10     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 23/40] net/virtio: make Vhost-user req sender consistent Maxime Coquelin
2021-01-06 11:50   ` Xia, Chenbo
2021-01-15  9:47     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 24/40] net/virtio: add Virtio-user ops to set owner Maxime Coquelin
2021-01-06 11:50   ` Xia, Chenbo
2020-12-20 21:13 ` [dpdk-dev] [PATCH 25/40] net/virtio: add Virtio-user features ops Maxime Coquelin
2021-01-06 11:54   ` Xia, Chenbo
2021-01-13 13:43     ` Adrian Moreno
2021-01-13 13:54       ` Maxime Coquelin
2021-01-15 14:19       ` Maxime Coquelin
2021-01-13 13:57   ` Adrian Moreno
2021-01-15 14:29     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 26/40] net/virtio: add Virtio-user protocol " Maxime Coquelin
2021-01-06 11:55   ` Xia, Chenbo
2020-12-20 21:13 ` [dpdk-dev] [PATCH 27/40] net/virtio: add Virtio-user memory tables ops Maxime Coquelin
2021-01-06 11:57   ` Xia, Chenbo
2021-01-15  9:57     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 28/40] net/virtio: add Virtio-user vring setting ops Maxime Coquelin
2021-01-05 21:24   ` David Marchand
2021-01-06 12:01   ` Xia, Chenbo
2021-01-15 10:12     ` Maxime Coquelin
2021-01-06 12:03   ` Xia, Chenbo
2021-01-15 10:15     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 29/40] net/virtio: add Virtio-user vring file ops Maxime Coquelin
2021-01-05 21:24   ` David Marchand
2021-01-06 12:04   ` Xia, Chenbo
2021-01-15 10:17     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 30/40] net/virtio: add Virtio-user vring address ops Maxime Coquelin
2021-01-06 12:06   ` Xia, Chenbo
2021-01-15 10:19     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 31/40] net/virtio: add Virtio-user status ops Maxime Coquelin
2021-01-06 12:09   ` Xia, Chenbo
2021-01-15 10:48     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 32/40] net/virtio: remove useless request ops Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 33/40] net/virtio: improve Virtio-user errors handling Maxime Coquelin
2021-01-07  2:26   ` Xia, Chenbo
2021-01-15 11:09     ` Maxime Coquelin
2020-12-20 21:13 ` [dpdk-dev] [PATCH 34/40] net/virtio: move Vhost-user reqs to Vhost-user backend Maxime Coquelin
2020-12-20 21:14 ` Maxime Coquelin [this message]
2021-01-07  3:20   ` [dpdk-dev] [PATCH 35/40] net/virtio: make server mode blocking Xia, Chenbo
2021-01-15 11:13     ` Maxime Coquelin
2020-12-20 21:14 ` [dpdk-dev] [PATCH 36/40] net/virtio: move protocol features to Vhost-user Maxime Coquelin
2020-12-20 21:14 ` [dpdk-dev] [PATCH 37/40] net/virtio: introduce backend data Maxime Coquelin
2021-01-05 21:26   ` David Marchand
2021-01-13 17:18   ` Adrian Moreno
2021-01-15 16:49     ` Maxime Coquelin
2020-12-20 21:14 ` [dpdk-dev] [PATCH 38/40] net/virtio: move Vhost-user specifics to its backend Maxime Coquelin
2021-01-07  6:32   ` Xia, Chenbo
2021-01-15 12:03     ` Maxime Coquelin
2020-12-20 21:14 ` [dpdk-dev] [PATCH 39/40] net/virtio: move Vhost-kernel data " Maxime Coquelin
2021-01-07  6:42   ` Xia, Chenbo
2021-01-11  8:02   ` Xia, Chenbo
2021-01-15 11:54     ` Maxime Coquelin
2021-01-18 20:36     ` Maxime Coquelin
2020-12-20 21:14 ` [dpdk-dev] [PATCH 40/40] net/virtio: move Vhost-vDPA " Maxime Coquelin
2020-12-22 15:20   ` Maxime Coquelin
2021-01-07  6:50   ` Xia, Chenbo
2021-01-15 12:08     ` Maxime Coquelin
2021-01-11  8:05   ` Xia, Chenbo
2020-12-21 10:58 ` [dpdk-dev] [PATCH 00/40] net/virtio: Virtio PMD rework Maxime Coquelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201220211405.313012-36-maxime.coquelin@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=amorenoz@redhat.com \
    --cc=chenbo.xia@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).