DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend
@ 2020-09-29 16:13 Maxime Coquelin
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops Maxime Coquelin
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:13 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

vhost-vDPA is a new vhost backend type introduced by vDPA kernel
framework, which provides abstruction to the vDPA devices and
exposes to userspace a unified control interface through char devs.

This patch set adds vhost-vdpa backend type to the virtio_user.
A set of vhost-vdpa specific ops callbacks are attached to the
virtio_user according to the runtime dynamic check result of the
backend type. DMA memory map/unmap callbacks are added to both
vdev bus driver and virtio_user pmd to support address mapping.
In addition, minor fixes to existing virtio control path are also
implemented to make the new backend work.

This is a collaborative work done with Patrick Fu from Intel and
Adrian Moreno from Red Hat. Thanks to them for their contributions.

The series has been tested with vdpasim and Intel IFC Kernel vDPA
drivers, and more lightly with Mellanox mlx5_vdpa on ConnectX-6 Dx.

Changes in v3:
--------------
 * Fix 32bit builds (CI & Chenbo)
 * Fix checkpatch

Changes in v2:
--------------
 * Split backend-type patch (Adrian)
 * Fix get_status size (Chenbo)
 * Various minro fixes (Chenbo)

Adrian Moreno (1):
  net/virtio: move backend type selection to ethdev

Maxime Coquelin (7):
  bus/vdev: add DMA mapping ops
  net/virtio: introduce DMA ops
  net/virtio: introduce Vhost-vDPA backend type
  net/virtio: check protocol feature in user backend
  net/virtio: adapt Virtio-user status size
  net/virtio: split virtio-user start
  net/virtio: introduce Vhost-vDPA backend

 drivers/bus/vdev/rte_bus_vdev.h               |  46 ++-
 drivers/bus/vdev/vdev.c                       |  52 +++
 drivers/net/virtio/meson.build                |   1 +
 drivers/net/virtio/virtio_user/vhost.h        |   5 +
 drivers/net/virtio/virtio_user/vhost_user.c   |   6 +-
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 298 ++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  | 117 ++++---
 .../net/virtio/virtio_user/virtio_user_dev.h  |  13 +-
 drivers/net/virtio/virtio_user_ethdev.c       | 126 +++++++-
 9 files changed, 607 insertions(+), 57 deletions(-)
 create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c

-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
@ 2020-09-29 16:13 ` Maxime Coquelin
  2020-09-30  5:47   ` Xia, Chenbo
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 2/8] net/virtio: introduce DMA ops Maxime Coquelin
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:13 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

Add DMA map/unmap operation callbacks to the vdev bus, which
could be used by DMA capable vdev drivers.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/bus/vdev/rte_bus_vdev.h | 46 +++++++++++++++++++++++++++--
 drivers/bus/vdev/vdev.c         | 52 +++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
index 78a032cea8..d14eeb41b0 100644
--- a/drivers/bus/vdev/rte_bus_vdev.h
+++ b/drivers/bus/vdev/rte_bus_vdev.h
@@ -63,14 +63,54 @@ typedef int (rte_vdev_probe_t)(struct rte_vdev_device *dev);
  */
 typedef int (rte_vdev_remove_t)(struct rte_vdev_device *dev);
 
+/**
+ * Driver-specific DMA mapping. After a successful call the device
+ * will be able to read/write from/to this segment.
+ *
+ * @param dev
+ *   Pointer to the Virtual device.
+ * @param addr
+ *   Starting virtual address of memory to be mapped.
+ * @param iova
+ *   Starting IOVA address of memory to be mapped.
+ * @param len
+ *   Length of memory segment being mapped.
+ * @return
+ *   - 0 On success.
+ *   - Negative value and rte_errno is set otherwise.
+ */
+typedef int (rte_vdev_dma_map_t)(struct rte_vdev_device *dev, void *addr,
+			    uint64_t iova, size_t len);
+
+/**
+ * Driver-specific DMA un-mapping. After a successful call the device
+ * will not be able to read/write from/to this segment.
+ *
+ * @param dev
+ *   Pointer to the Virtual device.
+ * @param addr
+ *   Starting virtual address of memory to be unmapped.
+ * @param iova
+ *   Starting IOVA address of memory to be unmapped.
+ * @param len
+ *   Length of memory segment being unmapped.
+ * @return
+ *   - 0 On success.
+ *   - Negative value and rte_errno is set otherwise.
+ */
+typedef int (rte_vdev_dma_unmap_t)(struct rte_vdev_device *dev, void *addr,
+			      uint64_t iova, size_t len);
+
 /**
  * A virtual device driver abstraction.
  */
 struct rte_vdev_driver {
 	TAILQ_ENTRY(rte_vdev_driver) next; /**< Next in list. */
-	struct rte_driver driver;      /**< Inherited general driver. */
-	rte_vdev_probe_t *probe;       /**< Virtual device probe function. */
-	rte_vdev_remove_t *remove;     /**< Virtual device remove function. */
+	struct rte_driver driver;        /**< Inherited general driver. */
+	rte_vdev_probe_t *probe;         /**< Virtual device probe function. */
+	rte_vdev_remove_t *remove;       /**< Virtual device remove function. */
+	rte_vdev_dma_map_t *dma_map;     /**< Virtual device DMA map function. */
+	rte_vdev_dma_unmap_t *dma_unmap; /**< Virtual device DMA unmap function. */
 };
 
 /**
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index d746149a2e..0e94ea9d26 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -134,6 +134,56 @@ vdev_parse(const char *name, void *addr)
 	return driver == NULL;
 }
 
+static int
+vdev_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t len)
+{
+	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
+	const struct rte_vdev_driver *driver;
+
+	if (!vdev) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+
+	if (!vdev->device.driver) {
+		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
+		return 1;
+	}
+
+	driver = container_of(vdev->device.driver, const struct rte_vdev_driver,
+			driver);
+
+	if (driver->dma_map)
+		return driver->dma_map(vdev, addr, iova, len);
+
+	return 0;
+}
+
+static int
+vdev_dma_unmap(struct rte_device *dev, void *addr, uint64_t iova, size_t len)
+{
+	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
+	const struct rte_vdev_driver *driver;
+
+	if (!vdev) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+
+	if (!vdev->device.driver) {
+		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
+		return 1;
+	}
+
+	driver = container_of(vdev->device.driver, const struct rte_vdev_driver,
+			driver);
+
+	if (driver->dma_unmap)
+		return driver->dma_unmap(vdev, addr, iova, len);
+
+	return 0;
+}
+
 static int
 vdev_probe_all_drivers(struct rte_vdev_device *dev)
 {
@@ -551,6 +601,8 @@ static struct rte_bus rte_vdev_bus = {
 	.plug = vdev_plug,
 	.unplug = vdev_unplug,
 	.parse = vdev_parse,
+	.dma_map = vdev_dma_map,
+	.dma_unmap = vdev_dma_unmap,
 	.dev_iterate = rte_vdev_dev_iterate,
 };
 
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 2/8] net/virtio: introduce DMA ops
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops Maxime Coquelin
@ 2020-09-29 16:13 ` Maxime Coquelin
  2020-09-30  5:47   ` Xia, Chenbo
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev Maxime Coquelin
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:13 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

Add DMA map/unmap callbacks to the virtio_user pmd, which could
be leveraged by vdev bus driver to map memory for backend
devices with DMA capability.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost.h  |  4 ++
 drivers/net/virtio/virtio_user_ethdev.c | 54 +++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index 8f49ef4574..2e71995a79 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -105,6 +105,10 @@ struct virtio_user_backend_ops {
 	int (*enable_qp)(struct virtio_user_dev *dev,
 			 uint16_t pair_idx,
 			 int enable);
+	int (*dma_map)(struct virtio_user_dev *dev, void *addr,
+				  uint64_t iova, size_t len);
+	int (*dma_unmap)(struct virtio_user_dev *dev, void *addr,
+				  uint64_t iova, size_t len);
 };
 
 extern struct virtio_user_backend_ops virtio_ops_user;
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 87f6cb6950..60d17af888 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -818,9 +818,63 @@ virtio_user_pmd_remove(struct rte_vdev_device *vdev)
 	return 0;
 }
 
+static int virtio_user_pmd_dma_map(struct rte_vdev_device *vdev, void *addr,
+		uint64_t iova, size_t len)
+{
+	const char *name;
+	struct rte_eth_dev *eth_dev;
+	struct virtio_user_dev *dev;
+	struct virtio_hw *hw;
+
+	if (!vdev)
+		return -EINVAL;
+
+	name = rte_vdev_device_name(vdev);
+	eth_dev = rte_eth_dev_allocated(name);
+	/* Port has already been released by close. */
+	if (!eth_dev)
+		return 0;
+
+	hw = (struct virtio_hw *)eth_dev->data->dev_private;
+	dev = hw->virtio_user_dev;
+
+	if (dev->ops->dma_map)
+		return dev->ops->dma_map(dev, addr, iova, len);
+
+	return 0;
+}
+
+static int virtio_user_pmd_dma_unmap(struct rte_vdev_device *vdev, void *addr,
+		uint64_t iova, size_t len)
+{
+	const char *name;
+	struct rte_eth_dev *eth_dev;
+	struct virtio_user_dev *dev;
+	struct virtio_hw *hw;
+
+	if (!vdev)
+		return -EINVAL;
+
+	name = rte_vdev_device_name(vdev);
+	eth_dev = rte_eth_dev_allocated(name);
+	/* Port has already been released by close. */
+	if (!eth_dev)
+		return 0;
+
+	hw = (struct virtio_hw *)eth_dev->data->dev_private;
+	dev = hw->virtio_user_dev;
+
+	if (dev->ops->dma_unmap)
+		return dev->ops->dma_unmap(dev, addr, iova, len);
+
+	return 0;
+}
+
 static struct rte_vdev_driver virtio_user_driver = {
 	.probe = virtio_user_pmd_probe,
 	.remove = virtio_user_pmd_remove,
+	.dma_map = virtio_user_pmd_dma_map,
+	.dma_unmap = virtio_user_pmd_dma_unmap,
 };
 
 RTE_PMD_REGISTER_VDEV(net_virtio_user, virtio_user_driver);
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops Maxime Coquelin
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 2/8] net/virtio: introduce DMA ops Maxime Coquelin
@ 2020-09-29 16:13 ` Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
  2020-10-16  5:42   ` Wang, Yinan
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type Maxime Coquelin
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:13 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

From: Adrian Moreno <amorenoz@redhat.com>

This is a preparation patch with no functional change.

Use an enum instead of a boolean for the backend type.
Move the detection logic to the ethdev layer (where it is needed for the
first time).
The virtio_user_dev stores the backend type in the virtio_user_dev
struct so the type is only determined once

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
 .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
 drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
 3 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 2a0c861085..b79a9f84aa 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
 	return 0;
 }
 
-int
-is_vhost_user_by_type(const char *path)
-{
-	struct stat sb;
-
-	if (stat(path, &sb) == -1)
-		return 0;
-
-	return S_ISSOCK(sb.st_mode);
-}
-
 int
 virtio_user_start_device(struct virtio_user_dev *dev)
 {
@@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	rte_mcfg_mem_read_lock();
 	pthread_mutex_lock(&dev->mutex);
 
-	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
+			dev->vhostfd < 0)
 		goto error;
 
 	/* Step 0: tell vhost to create queues */
@@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 	dev->tapfds = NULL;
 
 	if (dev->is_server) {
-		if (access(dev->path, F_OK) == 0 &&
-		    !is_vhost_user_by_type(dev->path)) {
-			PMD_DRV_LOG(ERR, "Server mode doesn't support vhost-kernel!");
+		if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
+			PMD_DRV_LOG(ERR, "Server mode only supports vhost-user!");
 			return -1;
 		}
 		dev->ops = &virtio_ops_user;
 	} else {
-		if (is_vhost_user_by_type(dev->path)) {
+		if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
 			dev->ops = &virtio_ops_user;
-		} else {
+		} else if (dev->backend_type ==
+					VIRTIO_USER_BACKEND_VHOST_KERNEL) {
 			dev->ops = &virtio_ops_kernel;
 
 			dev->vhostfds = malloc(dev->max_queue_pairs *
@@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		     int cq, int queue_size, const char *mac, char **ifname,
-		     int server, int mrg_rxbuf, int in_order, int packed_vq)
+		     int server, int mrg_rxbuf, int in_order, int packed_vq,
+		     enum virtio_user_backend_type backend_type)
 {
 	uint64_t protocol_features = 0;
 
@@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	dev->frontend_features = 0;
 	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
 	dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
+	dev->backend_type = backend_type;
+
 	parse_mac(dev, mac);
 
 	if (*ifname) {
@@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
-	if (!is_vhost_user_by_type(dev->path))
+	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->unsupported_features |=
 			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
 
@@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	}
 
 	/* The backend will not report this feature, we add it explicitly */
-	if (is_vhost_user_by_type(dev->path))
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
 
 	/*
@@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 	uint64_t arg = status;
 
 	/* Vhost-user only for now */
-	if (!is_vhost_user_by_type(dev->path))
+	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
 	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
@@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 	int err;
 
 	/* Vhost-user only for now */
-	if (!is_vhost_user_by_type(dev->path))
+	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
 	if (!(dev->protocol_features & (1UL << VHOST_USER_PROTOCOL_F_STATUS)))
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 9377d5ba66..575bf430c0 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -10,6 +10,12 @@
 #include "../virtio_pci.h"
 #include "../virtio_ring.h"
 
+enum virtio_user_backend_type {
+	VIRTIO_USER_BACKEND_UNKNOWN,
+	VIRTIO_USER_BACKEND_VHOST_USER,
+	VIRTIO_USER_BACKEND_VHOST_KERNEL,
+};
+
 struct virtio_user_queue {
 	uint16_t used_idx;
 	bool avail_wrap_counter;
@@ -17,6 +23,7 @@ struct virtio_user_queue {
 };
 
 struct virtio_user_dev {
+	enum virtio_user_backend_type backend_type;
 	/* for vhost_user backend */
 	int		vhostfd;
 	int		listenfd;   /* listening fd */
@@ -60,13 +67,13 @@ struct virtio_user_dev {
 	bool		started;
 };
 
-int is_vhost_user_by_type(const char *path);
 int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 			 int cq, int queue_size, const char *mac, char **ifname,
 			 int server, int mrg_rxbuf, int in_order,
-			 int packed_vq);
+			 int packed_vq,
+			 enum virtio_user_backend_type backend_type);
 void virtio_user_dev_uninit(struct virtio_user_dev *dev);
 void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
 void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 60d17af888..38b49bad5f 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -6,6 +6,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <sys/stat.h>
 #include <sys/socket.h>
 
 #include <rte_malloc.h>
@@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
 	return -errno;
 }
 
+static enum virtio_user_backend_type
+virtio_user_backend_type(const char *path)
+{
+	struct stat sb;
+
+	if (stat(path, &sb) == -1)
+		return VIRTIO_USER_BACKEND_UNKNOWN;
+
+	return S_ISSOCK(sb.st_mode) ?
+		VIRTIO_USER_BACKEND_VHOST_USER :
+		VIRTIO_USER_BACKEND_VHOST_KERNEL;
+}
+
 static struct rte_eth_dev *
 virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
 {
@@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	struct rte_kvargs *kvlist = NULL;
 	struct rte_eth_dev *eth_dev;
 	struct virtio_hw *hw;
+	enum virtio_user_backend_type backend_type = VIRTIO_USER_BACKEND_UNKNOWN;
 	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
 	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
 	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
@@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 		goto end;
 	}
 
+	backend_type = virtio_user_backend_type(path);
+	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
+		PMD_INIT_LOG(ERR,
+			     "unable to determine backend type for path %s",
+			path);
+		goto end;
+	}
+
+
 	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
-		if (is_vhost_user_by_type(path)) {
+		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
 			PMD_INIT_LOG(ERR,
 				"arg %s applies only to vhost-kernel backend",
 				VIRTIO_USER_ARG_INTERFACE_NAME);
@@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	hw = eth_dev->data->dev_private;
 	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
 			 queue_size, mac_addr, &ifname, server_mode,
-			 mrg_rxbuf, in_order, packed_vq) < 0) {
+			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
 		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
 		virtio_user_eth_dev_free(eth_dev);
 		goto end;
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
                   ` (2 preceding siblings ...)
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev Maxime Coquelin
@ 2020-09-29 16:14 ` Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 5/8] net/virtio: check protocol feature in user backend Maxime Coquelin
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:14 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

Backend type is determined by checking char-device major numbers

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
 drivers/net/virtio/virtio_user_ethdev.c       | 48 +++++++++++++++++--
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 575bf430c0..1c8c98b1cd 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -14,6 +14,7 @@ enum virtio_user_backend_type {
 	VIRTIO_USER_BACKEND_UNKNOWN,
 	VIRTIO_USER_BACKEND_VHOST_USER,
 	VIRTIO_USER_BACKEND_VHOST_KERNEL,
+	VIRTIO_USER_BACKEND_VHOST_VDPA,
 };
 
 struct virtio_user_queue {
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 38b49bad5f..3a51afd711 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -6,7 +6,9 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <linux/major.h>
 #include <sys/stat.h>
+#include <sys/sysmacros.h>
 #include <sys/socket.h>
 
 #include <rte_malloc.h>
@@ -519,17 +521,55 @@ get_integer_arg(const char *key __rte_unused,
 	return -errno;
 }
 
+static uint32_t
+vdpa_dynamic_major_num(void)
+{
+	FILE *fp;
+	char *line = NULL;
+	size_t size;
+	char name[11];
+	bool found = false;
+	uint32_t num;
+
+	fp = fopen("/proc/devices", "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "Cannot open /proc/devices: %s",
+			     strerror(errno));
+		return UNNAMED_MAJOR;
+	}
+
+	while (getline(&line, &size, fp) > 0) {
+		char *stripped = line + strspn(line, " ");
+		if ((sscanf(stripped, "%u %10s", &num, name) == 2) &&
+		    (strncmp(name, "vhost-vdpa", 10) == 0)) {
+			found = true;
+			break;
+		}
+	}
+	fclose(fp);
+	return found ? num : UNNAMED_MAJOR;
+}
+
 static enum virtio_user_backend_type
 virtio_user_backend_type(const char *path)
 {
 	struct stat sb;
 
-	if (stat(path, &sb) == -1)
+	if (stat(path, &sb) == -1) {
+		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
+			     strerror(errno));
 		return VIRTIO_USER_BACKEND_UNKNOWN;
+	}
 
-	return S_ISSOCK(sb.st_mode) ?
-		VIRTIO_USER_BACKEND_VHOST_USER :
-		VIRTIO_USER_BACKEND_VHOST_KERNEL;
+	if (S_ISSOCK(sb.st_mode)) {
+		return VIRTIO_USER_BACKEND_VHOST_USER;
+	} else if (S_ISCHR(sb.st_mode)) {
+		if (major(sb.st_rdev) == MISC_MAJOR)
+			return VIRTIO_USER_BACKEND_VHOST_KERNEL;
+		if (major(sb.st_rdev) == vdpa_dynamic_major_num())
+			return VIRTIO_USER_BACKEND_VHOST_VDPA;
+	}
+	return VIRTIO_USER_BACKEND_UNKNOWN;
 }
 
 static struct rte_eth_dev *
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 5/8] net/virtio: check protocol feature in user backend
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
                   ` (3 preceding siblings ...)
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type Maxime Coquelin
@ 2020-09-29 16:14 ` Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 6/8] net/virtio: adapt Virtio-user status size Maxime Coquelin
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:14 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

When sending set status message, move protocol feature check
to vhost_user to be compatible with different backend types.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost_user.c      | 6 +++++-
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index 12b6c7dbcf..ef290c357b 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -277,9 +277,13 @@ vhost_user_sock(struct virtio_user_dev *dev,
 	msg.size = 0;
 
 	switch (req) {
+	case VHOST_USER_GET_STATUS:
+		if (!(dev->protocol_features &
+				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+			return 0;
+		/* Fallthrough */
 	case VHOST_USER_GET_FEATURES:
 	case VHOST_USER_GET_PROTOCOL_FEATURES:
-	case VHOST_USER_GET_STATUS:
 		need_reply = 1;
 		break;
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index b79a9f84aa..d7cd6b0346 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -788,9 +788,6 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
-	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
-		return 0;
-
 	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
@@ -811,9 +808,6 @@ virtio_user_update_status(struct virtio_user_dev *dev)
 	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
-	if (!(dev->protocol_features & (1UL << VHOST_USER_PROTOCOL_F_STATUS)))
-		return 0;
-
 	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
 	if (err) {
 		PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err,
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 6/8] net/virtio: adapt Virtio-user status size
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
                   ` (4 preceding siblings ...)
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 5/8] net/virtio: check protocol feature in user backend Maxime Coquelin
@ 2020-09-29 16:14 ` Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 7/8] net/virtio: split virtio-user start Maxime Coquelin
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:14 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

Set proper payload size for set/get status message. The payload
size varies according to backend types.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Patrick Fu <patrick.fu@intel.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 34 +++++++++++++------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index d7cd6b0346..ded44bf32b 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -784,11 +784,15 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 	int ret;
 	uint64_t arg = status;
 
-	/* Vhost-user only for now */
-	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
+		ret = dev->ops->send_request(dev,
+				VHOST_USER_SET_STATUS, &arg);
+	else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)
+		ret = dev->ops->send_request(dev,
+				VHOST_USER_SET_STATUS, &status);
+	else
 		return 0;
 
-	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
 			     strerror(errno));
@@ -802,24 +806,32 @@ int
 virtio_user_update_status(struct virtio_user_dev *dev)
 {
 	uint64_t ret;
+	uint8_t status;
 	int err;
 
-	/* Vhost-user only for now */
-	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
+		err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
+		if (!err && ret > UINT8_MAX) {
+			PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
+					"response 0x%" PRIx64 "\n", ret);
+			return -1;
+		}
+
+		status = ret;
+	} else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) {
+		err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS,
+				&status);
+	} else {
 		return 0;
+	}
 
-	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
 	if (err) {
 		PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err,
 			     strerror(errno));
 		return -1;
 	}
-	if (ret > UINT8_MAX) {
-		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS response 0x%" PRIx64 "\n", ret);
-		return -1;
-	}
 
-	dev->status = ret;
+	dev->status = status;
 	PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n"
 			"\t-RESET: %u\n"
 			"\t-ACKNOWLEDGE: %u\n"
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 7/8] net/virtio: split virtio-user start
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
                   ` (5 preceding siblings ...)
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 6/8] net/virtio: adapt Virtio-user status size Maxime Coquelin
@ 2020-09-29 16:14 ` Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
  2020-09-30 16:22 ` [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:14 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

Move feature bit settings in device start out as an standalone
function, so that feature bit could be negotiated at device
feature_ok status.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 50 ++++++++++++-------
 .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
 drivers/net/virtio/virtio_user_ethdev.c       |  4 ++
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index ded44bf32b..63424656e3 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -112,25 +112,11 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
 }
 
 int
-virtio_user_start_device(struct virtio_user_dev *dev)
+virtio_user_dev_set_features(struct virtio_user_dev *dev)
 {
 	uint64_t features;
-	int ret;
+	int ret = -1;
 
-	/*
-	 * XXX workaround!
-	 *
-	 * We need to make sure that the locks will be
-	 * taken in the correct order to avoid deadlocks.
-	 *
-	 * Before releasing this lock, this thread should
-	 * not trigger any memory hotplug events.
-	 *
-	 * This is a temporary workaround, and should be
-	 * replaced when we get proper supports from the
-	 * memory subsystem in the future.
-	 */
-	rte_mcfg_mem_read_lock();
 	pthread_mutex_lock(&dev->mutex);
 
 	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
@@ -141,10 +127,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0)
 		goto error;
 
-	/* Step 1: negotiate protocol features & set features */
 	features = dev->features;
 
-
 	/* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */
 	features &= ~(1ull << VIRTIO_NET_F_MAC);
 	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know */
@@ -154,6 +138,36 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 	if (ret < 0)
 		goto error;
 	PMD_DRV_LOG(INFO, "set features: %" PRIx64, features);
+error:
+	pthread_mutex_unlock(&dev->mutex);
+
+	return ret;
+}
+
+int
+virtio_user_start_device(struct virtio_user_dev *dev)
+{
+	int ret;
+
+	/*
+	 * XXX workaround!
+	 *
+	 * We need to make sure that the locks will be
+	 * taken in the correct order to avoid deadlocks.
+	 *
+	 * Before releasing this lock, this thread should
+	 * not trigger any memory hotplug events.
+	 *
+	 * This is a temporary workaround, and should be
+	 * replaced when we get proper supports from the
+	 * memory subsystem in the future.
+	 */
+	rte_mcfg_mem_read_lock();
+	pthread_mutex_lock(&dev->mutex);
+
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
+			dev->vhostfd < 0)
+		goto error;
 
 	/* Step 2: share memory regions */
 	ret = dev->ops->send_request(dev, VHOST_USER_SET_MEM_TABLE, NULL);
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 1c8c98b1cd..3e9d1a1eb3 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -68,6 +68,7 @@ struct virtio_user_dev {
 	bool		started;
 };
 
+int virtio_user_dev_set_features(struct virtio_user_dev *dev);
 int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 3a51afd711..f56fc238c4 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -269,7 +269,11 @@ static void
 virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
 {
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
+	uint8_t old_status = dev->status;
 
+	if (status & VIRTIO_CONFIG_STATUS_FEATURES_OK &&
+			~old_status & VIRTIO_CONFIG_STATUS_FEATURES_OK)
+		virtio_user_dev_set_features(dev);
 	if (status & VIRTIO_CONFIG_STATUS_DRIVER_OK)
 		virtio_user_start_device(dev);
 	else if (status == VIRTIO_CONFIG_STATUS_RESET)
-- 
2.26.2


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

* [dpdk-dev] [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
                   ` (6 preceding siblings ...)
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 7/8] net/virtio: split virtio-user start Maxime Coquelin
@ 2020-09-29 16:14 ` Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
  2020-09-30 16:22 ` [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
  8 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-29 16:14 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz; +Cc: Maxime Coquelin

vhost-vDPA is a new virtio backend type introduced by vDPA kernel
framework, which provides abstruction to the vDPA devices and
exposes an unified control interface through a char dev.

This patch adds support to the vhost-vDPA backend. As similar to
the existing vhost kernel backend, a set of virtio_user ops were
introduced for vhost-vDPA backend to handle device specific operations
such as:
 - device setup
 - ioctl message handling
 - queue pair enabling
 - dma map/unmap
vDPA relevant ioctl codes and data structures are also defined in
this patch.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/meson.build                |   1 +
 drivers/net/virtio/virtio_user/vhost.h        |   1 +
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 298 ++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  |   6 +
 4 files changed, 306 insertions(+)
 create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c

diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
index 3fd6051f4b..eaed46373d 100644
--- a/drivers/net/virtio/meson.build
+++ b/drivers/net/virtio/meson.build
@@ -42,6 +42,7 @@ if is_linux
 		'virtio_user/vhost_kernel.c',
 		'virtio_user/vhost_kernel_tap.c',
 		'virtio_user/vhost_user.c',
+		'virtio_user/vhost_vdpa.c',
 		'virtio_user/virtio_user_dev.c')
 	deps += ['bus_vdev']
 endif
diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index 2e71995a79..210a3704e7 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -113,5 +113,6 @@ struct virtio_user_backend_ops {
 
 extern struct virtio_user_backend_ops virtio_ops_user;
 extern struct virtio_user_backend_ops virtio_ops_kernel;
+extern struct virtio_user_backend_ops virtio_ops_vdpa;
 
 #endif
diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
new file mode 100644
index 0000000000..c7b9349fc8
--- /dev/null
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -0,0 +1,298 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Red Hat Inc.
+ */
+
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <rte_memory.h>
+
+#include "vhost.h"
+#include "virtio_user_dev.h"
+
+/* vhost kernel & vdpa ioctls */
+#define VHOST_VIRTIO 0xAF
+#define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64)
+#define VHOST_SET_FEATURES _IOW(VHOST_VIRTIO, 0x00, __u64)
+#define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
+#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
+#define VHOST_SET_MEM_TABLE _IOW(VHOST_VIRTIO, 0x03, void *)
+#define VHOST_SET_LOG_BASE _IOW(VHOST_VIRTIO, 0x04, __u64)
+#define VHOST_SET_LOG_FD _IOW(VHOST_VIRTIO, 0x07, int)
+#define VHOST_SET_VRING_NUM _IOW(VHOST_VIRTIO, 0x10, struct vhost_vring_state)
+#define VHOST_SET_VRING_ADDR _IOW(VHOST_VIRTIO, 0x11, struct vhost_vring_addr)
+#define VHOST_SET_VRING_BASE _IOW(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
+#define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
+#define VHOST_SET_VRING_KICK _IOW(VHOST_VIRTIO, 0x20, struct vhost_vring_file)
+#define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct vhost_vring_file)
+#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct vhost_vring_file)
+#define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct vhost_vring_file)
+#define VHOST_VDPA_GET_DEVICE_ID _IOR(VHOST_VIRTIO, 0x70, __u32)
+#define VHOST_VDPA_GET_STATUS _IOR(VHOST_VIRTIO, 0x71, __u8)
+#define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8)
+#define VHOST_VDPA_SET_VRING_ENABLE	_IOW(VHOST_VIRTIO, 0x75, \
+					     struct vhost_vring_state)
+
+static uint64_t vhost_req_user_to_vdpa[] = {
+	[VHOST_USER_SET_OWNER] = VHOST_SET_OWNER,
+	[VHOST_USER_RESET_OWNER] = VHOST_RESET_OWNER,
+	[VHOST_USER_SET_FEATURES] = VHOST_SET_FEATURES,
+	[VHOST_USER_GET_FEATURES] = VHOST_GET_FEATURES,
+	[VHOST_USER_SET_VRING_CALL] = VHOST_SET_VRING_CALL,
+	[VHOST_USER_SET_VRING_NUM] = VHOST_SET_VRING_NUM,
+	[VHOST_USER_SET_VRING_BASE] = VHOST_SET_VRING_BASE,
+	[VHOST_USER_GET_VRING_BASE] = VHOST_GET_VRING_BASE,
+	[VHOST_USER_SET_VRING_ADDR] = VHOST_SET_VRING_ADDR,
+	[VHOST_USER_SET_VRING_KICK] = VHOST_SET_VRING_KICK,
+	[VHOST_USER_SET_MEM_TABLE] = VHOST_SET_MEM_TABLE,
+	[VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS,
+	[VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS,
+	[VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE,
+};
+
+/* no alignment requirement */
+struct vhost_iotlb_msg {
+	uint64_t iova;
+	uint64_t size;
+	uint64_t uaddr;
+#define VHOST_ACCESS_RO      0x1
+#define VHOST_ACCESS_WO      0x2
+#define VHOST_ACCESS_RW      0x3
+	uint8_t perm;
+#define VHOST_IOTLB_MISS           1
+#define VHOST_IOTLB_UPDATE         2
+#define VHOST_IOTLB_INVALIDATE     3
+#define VHOST_IOTLB_ACCESS_FAIL    4
+	uint8_t type;
+};
+
+#define VHOST_IOTLB_MSG_V2 0x2
+
+struct vhost_msg {
+	uint32_t type;
+	uint32_t reserved;
+	union {
+		struct vhost_iotlb_msg iotlb;
+		uint8_t padding[64];
+	};
+};
+
+static int
+vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr,
+				  uint64_t iova, size_t len)
+{
+	struct vhost_msg msg = {};
+
+	msg.type = VHOST_IOTLB_MSG_V2;
+	msg.iotlb.type = VHOST_IOTLB_UPDATE;
+	msg.iotlb.iova = iova;
+	msg.iotlb.uaddr = (uint64_t)(uintptr_t)addr;
+	msg.iotlb.size = len;
+	msg.iotlb.perm = VHOST_ACCESS_RW;
+
+	if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
+		PMD_DRV_LOG(ERR, "Failed to send IOTLB update (%s)",
+				strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+static int
+vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, __rte_unused void *addr,
+				  uint64_t iova, size_t len)
+{
+	struct vhost_msg msg = {};
+
+	msg.type = VHOST_IOTLB_MSG_V2;
+	msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
+	msg.iotlb.iova = iova;
+	msg.iotlb.size = len;
+
+	if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
+		PMD_DRV_LOG(ERR, "Failed to send IOTLB invalidate (%s)",
+				strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
+
+static int
+vhost_vdpa_map_contig(const struct rte_memseg_list *msl,
+		const struct rte_memseg *ms, size_t len, void *arg)
+{
+	struct virtio_user_dev *dev = arg;
+
+	if (msl->external)
+		return 0;
+
+	return vhost_vdpa_dma_map(dev, ms->addr, ms->iova, len);
+}
+
+static int
+vhost_vdpa_map(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
+		void *arg)
+{
+	struct virtio_user_dev *dev = arg;
+
+	/* skip external memory that isn't a heap */
+	if (msl->external && !msl->heap)
+		return 0;
+
+	/* skip any segments with invalid IOVA addresses */
+	if (ms->iova == RTE_BAD_IOVA)
+		return 0;
+
+	/* if IOVA mode is VA, we've already mapped the internal segments */
+	if (!msl->external && rte_eal_iova_mode() == RTE_IOVA_VA)
+		return 0;
+
+	return vhost_vdpa_dma_map(dev, ms->addr, ms->iova, ms->len);
+}
+
+static int
+vhost_vdpa_dma_map_all(struct virtio_user_dev *dev)
+{
+	vhost_vdpa_dma_unmap(dev, NULL, 0, SIZE_MAX);
+
+	if (rte_eal_iova_mode() == RTE_IOVA_VA) {
+		/* with IOVA as VA mode, we can get away with mapping contiguous
+		 * chunks rather than going page-by-page.
+		 */
+		int ret = rte_memseg_contig_walk_thread_unsafe(
+				vhost_vdpa_map_contig, dev);
+		if (ret)
+			return ret;
+		/* we have to continue the walk because we've skipped the
+		 * external segments during the config walk.
+		 */
+	}
+	return rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev);
+}
+
+/* with below features, vhost vdpa does not need to do the checksum and TSO,
+ * these info will be passed to virtio_user through virtio net header.
+ */
+#define VHOST_VDPA_GUEST_OFFLOADS_MASK	\
+	((1ULL << VIRTIO_NET_F_GUEST_CSUM) |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_TSO4) |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_TSO6) |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_ECN)  |	\
+	 (1ULL << VIRTIO_NET_F_GUEST_UFO))
+
+#define VHOST_VDPA_HOST_OFFLOADS_MASK		\
+	((1ULL << VIRTIO_NET_F_HOST_TSO4) |	\
+	 (1ULL << VIRTIO_NET_F_HOST_TSO6) |	\
+	 (1ULL << VIRTIO_NET_F_CSUM))
+
+static int
+vhost_vdpa_ioctl(struct virtio_user_dev *dev,
+		   enum vhost_user_request req,
+		   void *arg)
+{
+	int ret = -1;
+	uint64_t req_vdpa;
+
+	PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]);
+
+	req_vdpa = vhost_req_user_to_vdpa[req];
+
+	if (req_vdpa == VHOST_SET_MEM_TABLE)
+		return vhost_vdpa_dma_map_all(dev);
+
+	if (req_vdpa == VHOST_SET_FEATURES) {
+		/* WORKAROUND */
+		*(uint64_t *)arg |= 1ULL << VIRTIO_F_IOMMU_PLATFORM;
+
+		/* Multiqueue not supported for now */
+		*(uint64_t *)arg &= ~(1ULL << VIRTIO_NET_F_MQ);
+	}
+
+	switch (req_vdpa) {
+	case VHOST_SET_VRING_NUM:
+	case VHOST_SET_VRING_ADDR:
+	case VHOST_SET_VRING_BASE:
+	case VHOST_GET_VRING_BASE:
+	case VHOST_SET_VRING_KICK:
+	case VHOST_SET_VRING_CALL:
+		PMD_DRV_LOG(DEBUG, "vhostfd=%d, index=%u",
+			    dev->vhostfd, *(unsigned int *)arg);
+		break;
+	default:
+		break;
+	}
+
+	ret = ioctl(dev->vhostfd, req_vdpa, arg);
+	if (ret < 0)
+		PMD_DRV_LOG(ERR, "%s failed: %s",
+			    vhost_msg_strings[req], strerror(errno));
+
+	return ret;
+}
+
+/**
+ * Set up environment to talk with a vhost vdpa backend.
+ *
+ * @return
+ *   - (-1) if fail to set up;
+ *   - (>=0) if successful.
+ */
+static int
+vhost_vdpa_setup(struct virtio_user_dev *dev)
+{
+	uint32_t did = (uint32_t)-1;
+
+	dev->vhostfd = open(dev->path, O_RDWR);
+	if (dev->vhostfd < 0) {
+		PMD_DRV_LOG(ERR, "Failed to open %s: %s\n",
+				dev->path, strerror(errno));
+		return -1;
+	}
+
+	if (ioctl(dev->vhostfd, VHOST_VDPA_GET_DEVICE_ID, &did) < 0 ||
+			did != VIRTIO_ID_NETWORK) {
+		PMD_DRV_LOG(ERR, "Invalid vdpa device ID: %u\n", did);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int
+vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
+			       uint16_t pair_idx,
+			       int enable)
+{
+	int i;
+
+	if (dev->qp_enabled[pair_idx] == enable)
+		return 0;
+
+	for (i = 0; i < 2; ++i) {
+		struct vhost_vring_state state = {
+			.index = pair_idx * 2 + i,
+			.num   = enable,
+		};
+
+		if (vhost_vdpa_ioctl(dev, VHOST_USER_SET_VRING_ENABLE, &state))
+			return -1;
+	}
+
+	dev->qp_enabled[pair_idx] = enable;
+
+	return 0;
+}
+
+struct virtio_user_backend_ops virtio_ops_vdpa = {
+	.setup = vhost_vdpa_setup,
+	.send_request = vhost_vdpa_ioctl,
+	.enable_qp = vhost_vdpa_enable_queue_pair,
+	.dma_map = vhost_vdpa_dma_map,
+	.dma_unmap = vhost_vdpa_dma_unmap,
+};
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 63424656e3..3681758ef1 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -389,6 +389,12 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
 				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, "Unknown backend type");
+			return -1;
 		}
 	}
 
-- 
2.26.2


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

* Re: [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops Maxime Coquelin
@ 2020-09-30  5:47   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:47 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 1/8] bus/vdev: add DMA mapping ops
> 
> Add DMA map/unmap operation callbacks to the vdev bus, which
> could be used by DMA capable vdev drivers.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/bus/vdev/rte_bus_vdev.h | 46 +++++++++++++++++++++++++++--
>  drivers/bus/vdev/vdev.c         | 52 +++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/vdev/rte_bus_vdev.h
> b/drivers/bus/vdev/rte_bus_vdev.h
> index 78a032cea8..d14eeb41b0 100644
> --- a/drivers/bus/vdev/rte_bus_vdev.h
> +++ b/drivers/bus/vdev/rte_bus_vdev.h
> @@ -63,14 +63,54 @@ typedef int (rte_vdev_probe_t)(struct rte_vdev_device
> *dev);
>   */
>  typedef int (rte_vdev_remove_t)(struct rte_vdev_device *dev);
> 
> +/**
> + * Driver-specific DMA mapping. After a successful call the device
> + * will be able to read/write from/to this segment.
> + *
> + * @param dev
> + *   Pointer to the Virtual device.
> + * @param addr
> + *   Starting virtual address of memory to be mapped.
> + * @param iova
> + *   Starting IOVA address of memory to be mapped.
> + * @param len
> + *   Length of memory segment being mapped.
> + * @return
> + *   - 0 On success.
> + *   - Negative value and rte_errno is set otherwise.
> + */
> +typedef int (rte_vdev_dma_map_t)(struct rte_vdev_device *dev, void *addr,
> +			    uint64_t iova, size_t len);
> +
> +/**
> + * Driver-specific DMA un-mapping. After a successful call the device
> + * will not be able to read/write from/to this segment.
> + *
> + * @param dev
> + *   Pointer to the Virtual device.
> + * @param addr
> + *   Starting virtual address of memory to be unmapped.
> + * @param iova
> + *   Starting IOVA address of memory to be unmapped.
> + * @param len
> + *   Length of memory segment being unmapped.
> + * @return
> + *   - 0 On success.
> + *   - Negative value and rte_errno is set otherwise.
> + */
> +typedef int (rte_vdev_dma_unmap_t)(struct rte_vdev_device *dev, void
> *addr,
> +			      uint64_t iova, size_t len);
> +
>  /**
>   * A virtual device driver abstraction.
>   */
>  struct rte_vdev_driver {
>  	TAILQ_ENTRY(rte_vdev_driver) next; /**< Next in list. */
> -	struct rte_driver driver;      /**< Inherited general driver. */
> -	rte_vdev_probe_t *probe;       /**< Virtual device probe function.
> */
> -	rte_vdev_remove_t *remove;     /**< Virtual device remove function.
> */
> +	struct rte_driver driver;        /**< Inherited general driver. */
> +	rte_vdev_probe_t *probe;         /**< Virtual device probe function.
> */
> +	rte_vdev_remove_t *remove;       /**< Virtual device remove function.
> */
> +	rte_vdev_dma_map_t *dma_map;     /**< Virtual device DMA map
> function. */
> +	rte_vdev_dma_unmap_t *dma_unmap; /**< Virtual device DMA unmap
> function. */
>  };
> 
>  /**
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index d746149a2e..0e94ea9d26 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -134,6 +134,56 @@ vdev_parse(const char *name, void *addr)
>  	return driver == NULL;
>  }
> 
> +static int
> +vdev_dma_map(struct rte_device *dev, void *addr, uint64_t iova, size_t
> len)
> +{
> +	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
> +	const struct rte_vdev_driver *driver;
> +
> +	if (!vdev) {
> +		rte_errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if (!vdev->device.driver) {
> +		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
> +		return 1;
> +	}
> +
> +	driver = container_of(vdev->device.driver, const struct
> rte_vdev_driver,
> +			driver);
> +
> +	if (driver->dma_map)
> +		return driver->dma_map(vdev, addr, iova, len);
> +
> +	return 0;
> +}
> +
> +static int
> +vdev_dma_unmap(struct rte_device *dev, void *addr, uint64_t iova, size_t
> len)
> +{
> +	struct rte_vdev_device *vdev = RTE_DEV_TO_VDEV(dev);
> +	const struct rte_vdev_driver *driver;
> +
> +	if (!vdev) {
> +		rte_errno = EINVAL;
> +		return -1;
> +	}
> +
> +	if (!vdev->device.driver) {
> +		VDEV_LOG(DEBUG, "no driver attach to device %s", dev->name);
> +		return 1;
> +	}
> +
> +	driver = container_of(vdev->device.driver, const struct
> rte_vdev_driver,
> +			driver);
> +
> +	if (driver->dma_unmap)
> +		return driver->dma_unmap(vdev, addr, iova, len);
> +
> +	return 0;
> +}
> +
>  static int
>  vdev_probe_all_drivers(struct rte_vdev_device *dev)
>  {
> @@ -551,6 +601,8 @@ static struct rte_bus rte_vdev_bus = {
>  	.plug = vdev_plug,
>  	.unplug = vdev_unplug,
>  	.parse = vdev_parse,
> +	.dma_map = vdev_dma_map,
> +	.dma_unmap = vdev_dma_unmap,
>  	.dev_iterate = rte_vdev_dev_iterate,
>  };
> 
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 2/8] net/virtio: introduce DMA ops
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 2/8] net/virtio: introduce DMA ops Maxime Coquelin
@ 2020-09-30  5:47   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:47 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 2/8] net/virtio: introduce DMA ops
> 
> Add DMA map/unmap callbacks to the virtio_user pmd, which could
> be leveraged by vdev bus driver to map memory for backend
> devices with DMA capability.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost.h  |  4 ++
>  drivers/net/virtio/virtio_user_ethdev.c | 54 +++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 8f49ef4574..2e71995a79 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -105,6 +105,10 @@ struct virtio_user_backend_ops {
>  	int (*enable_qp)(struct virtio_user_dev *dev,
>  			 uint16_t pair_idx,
>  			 int enable);
> +	int (*dma_map)(struct virtio_user_dev *dev, void *addr,
> +				  uint64_t iova, size_t len);
> +	int (*dma_unmap)(struct virtio_user_dev *dev, void *addr,
> +				  uint64_t iova, size_t len);
>  };
> 
>  extern struct virtio_user_backend_ops virtio_ops_user;
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 87f6cb6950..60d17af888 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -818,9 +818,63 @@ virtio_user_pmd_remove(struct rte_vdev_device *vdev)
>  	return 0;
>  }
> 
> +static int virtio_user_pmd_dma_map(struct rte_vdev_device *vdev, void
> *addr,
> +		uint64_t iova, size_t len)
> +{
> +	const char *name;
> +	struct rte_eth_dev *eth_dev;
> +	struct virtio_user_dev *dev;
> +	struct virtio_hw *hw;
> +
> +	if (!vdev)
> +		return -EINVAL;
> +
> +	name = rte_vdev_device_name(vdev);
> +	eth_dev = rte_eth_dev_allocated(name);
> +	/* Port has already been released by close. */
> +	if (!eth_dev)
> +		return 0;
> +
> +	hw = (struct virtio_hw *)eth_dev->data->dev_private;
> +	dev = hw->virtio_user_dev;
> +
> +	if (dev->ops->dma_map)
> +		return dev->ops->dma_map(dev, addr, iova, len);
> +
> +	return 0;
> +}
> +
> +static int virtio_user_pmd_dma_unmap(struct rte_vdev_device *vdev, void
> *addr,
> +		uint64_t iova, size_t len)
> +{
> +	const char *name;
> +	struct rte_eth_dev *eth_dev;
> +	struct virtio_user_dev *dev;
> +	struct virtio_hw *hw;
> +
> +	if (!vdev)
> +		return -EINVAL;
> +
> +	name = rte_vdev_device_name(vdev);
> +	eth_dev = rte_eth_dev_allocated(name);
> +	/* Port has already been released by close. */
> +	if (!eth_dev)
> +		return 0;
> +
> +	hw = (struct virtio_hw *)eth_dev->data->dev_private;
> +	dev = hw->virtio_user_dev;
> +
> +	if (dev->ops->dma_unmap)
> +		return dev->ops->dma_unmap(dev, addr, iova, len);
> +
> +	return 0;
> +}
> +
>  static struct rte_vdev_driver virtio_user_driver = {
>  	.probe = virtio_user_pmd_probe,
>  	.remove = virtio_user_pmd_remove,
> +	.dma_map = virtio_user_pmd_dma_map,
> +	.dma_unmap = virtio_user_pmd_dma_unmap,
>  };
> 
>  RTE_PMD_REGISTER_VDEV(net_virtio_user, virtio_user_driver);
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev Maxime Coquelin
@ 2020-09-30  5:49   ` Xia, Chenbo
  2020-10-16  5:42   ` Wang, Yinan
  1 sibling, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:49 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
> 
> From: Adrian Moreno <amorenoz@redhat.com>
> 
> This is a preparation patch with no functional change.
> 
> Use an enum instead of a boolean for the backend type.
> Move the detection logic to the ethdev layer (where it is needed for the
> first time).
> The virtio_user_dev stores the backend type in the virtio_user_dev
> struct so the type is only determined once
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>  3 files changed, 50 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 2a0c861085..b79a9f84aa 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>  	return 0;
>  }
> 
> -int
> -is_vhost_user_by_type(const char *path)
> -{
> -	struct stat sb;
> -
> -	if (stat(path, &sb) == -1)
> -		return 0;
> -
> -	return S_ISSOCK(sb.st_mode);
> -}
> -
>  int
>  virtio_user_start_device(struct virtio_user_dev *dev)
>  {
> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>  	rte_mcfg_mem_read_lock();
>  	pthread_mutex_lock(&dev->mutex);
> 
> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> +			dev->vhostfd < 0)
>  		goto error;
> 
>  	/* Step 0: tell vhost to create queues */
> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  	dev->tapfds = NULL;
> 
>  	if (dev->is_server) {
> -		if (access(dev->path, F_OK) == 0 &&
> -		    !is_vhost_user_by_type(dev->path)) {
> -			PMD_DRV_LOG(ERR, "Server mode doesn't support vhost-
> kernel!");
> +		if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
> +			PMD_DRV_LOG(ERR, "Server mode only supports vhost-
> user!");
>  			return -1;
>  		}
>  		dev->ops = &virtio_ops_user;
>  	} else {
> -		if (is_vhost_user_by_type(dev->path)) {
> +		if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
>  			dev->ops = &virtio_ops_user;
> -		} else {
> +		} else if (dev->backend_type ==
> +					VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			dev->ops = &virtio_ops_kernel;
> 
>  			dev->vhostfds = malloc(dev->max_queue_pairs *
> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		     int cq, int queue_size, const char *mac, char **ifname,
> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> +		     enum virtio_user_backend_type backend_type)
>  {
>  	uint64_t protocol_features = 0;
> 
> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	dev->frontend_features = 0;
>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>  	dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> +	dev->backend_type = backend_type;
> +
>  	parse_mac(dev, mac);
> 
>  	if (*ifname) {
> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  		return -1;
>  	}
> 
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->unsupported_features |=
>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> 
> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	}
> 
>  	/* The backend will not report this feature, we add it explicitly */
> -	if (is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> 
>  	/*
> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> *dev, uint8_t status)
>  	uint64_t arg = status;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  	int err;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1UL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 9377d5ba66..575bf430c0 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -10,6 +10,12 @@
>  #include "../virtio_pci.h"
>  #include "../virtio_ring.h"
> 
> +enum virtio_user_backend_type {
> +	VIRTIO_USER_BACKEND_UNKNOWN,
> +	VIRTIO_USER_BACKEND_VHOST_USER,
> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> +};
> +
>  struct virtio_user_queue {
>  	uint16_t used_idx;
>  	bool avail_wrap_counter;
> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>  };
> 
>  struct virtio_user_dev {
> +	enum virtio_user_backend_type backend_type;
>  	/* for vhost_user backend */
>  	int		vhostfd;
>  	int		listenfd;   /* listening fd */
> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>  	bool		started;
>  };
> 
> -int is_vhost_user_by_type(const char *path);
>  int virtio_user_start_device(struct virtio_user_dev *dev);
>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
> queues,
>  			 int cq, int queue_size, const char *mac, char **ifname,
>  			 int server, int mrg_rxbuf, int in_order,
> -			 int packed_vq);
> +			 int packed_vq,
> +			 enum virtio_user_backend_type backend_type);
>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t
> queue_idx);
>  void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 60d17af888..38b49bad5f 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -6,6 +6,7 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <sys/stat.h>
>  #include <sys/socket.h>
> 
>  #include <rte_malloc.h>
> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>  	return -errno;
>  }
> 
> +static enum virtio_user_backend_type
> +virtio_user_backend_type(const char *path)
> +{
> +	struct stat sb;
> +
> +	if (stat(path, &sb) == -1)
> +		return VIRTIO_USER_BACKEND_UNKNOWN;
> +
> +	return S_ISSOCK(sb.st_mode) ?
> +		VIRTIO_USER_BACKEND_VHOST_USER :
> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> +}
> +
>  static struct rte_eth_dev *
>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>  {
> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	struct rte_kvargs *kvlist = NULL;
>  	struct rte_eth_dev *eth_dev;
>  	struct virtio_hw *hw;
> +	enum virtio_user_backend_type backend_type =
> VIRTIO_USER_BACKEND_UNKNOWN;
>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  		goto end;
>  	}
> 
> +	backend_type = virtio_user_backend_type(path);
> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> +		PMD_INIT_LOG(ERR,
> +			     "unable to determine backend type for path %s",
> +			path);
> +		goto end;
> +	}
> +
> +
>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
> -		if (is_vhost_user_by_type(path)) {
> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			PMD_INIT_LOG(ERR,
>  				"arg %s applies only to vhost-kernel backend",
>  				VIRTIO_USER_ARG_INTERFACE_NAME);
> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	hw = eth_dev->data->dev_private;
>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>  			 queue_size, mac_addr, &ifname, server_mode,
> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>  		virtio_user_eth_dev_free(eth_dev);
>  		goto end;
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type Maxime Coquelin
@ 2020-09-30  5:49   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:49 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type
> 
> Backend type is determined by checking char-device major numbers
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
>  drivers/net/virtio/virtio_user_ethdev.c       | 48 +++++++++++++++++--
>  2 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 575bf430c0..1c8c98b1cd 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -14,6 +14,7 @@ enum virtio_user_backend_type {
>  	VIRTIO_USER_BACKEND_UNKNOWN,
>  	VIRTIO_USER_BACKEND_VHOST_USER,
>  	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> +	VIRTIO_USER_BACKEND_VHOST_VDPA,
>  };
> 
>  struct virtio_user_queue {
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 38b49bad5f..3a51afd711 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -6,7 +6,9 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <linux/major.h>
>  #include <sys/stat.h>
> +#include <sys/sysmacros.h>
>  #include <sys/socket.h>
> 
>  #include <rte_malloc.h>
> @@ -519,17 +521,55 @@ get_integer_arg(const char *key __rte_unused,
>  	return -errno;
>  }
> 
> +static uint32_t
> +vdpa_dynamic_major_num(void)
> +{
> +	FILE *fp;
> +	char *line = NULL;
> +	size_t size;
> +	char name[11];
> +	bool found = false;
> +	uint32_t num;
> +
> +	fp = fopen("/proc/devices", "r");
> +	if (fp == NULL) {
> +		PMD_INIT_LOG(ERR, "Cannot open /proc/devices: %s",
> +			     strerror(errno));
> +		return UNNAMED_MAJOR;
> +	}
> +
> +	while (getline(&line, &size, fp) > 0) {
> +		char *stripped = line + strspn(line, " ");
> +		if ((sscanf(stripped, "%u %10s", &num, name) == 2) &&
> +		    (strncmp(name, "vhost-vdpa", 10) == 0)) {
> +			found = true;
> +			break;
> +		}
> +	}
> +	fclose(fp);
> +	return found ? num : UNNAMED_MAJOR;
> +}
> +
>  static enum virtio_user_backend_type
>  virtio_user_backend_type(const char *path)
>  {
>  	struct stat sb;
> 
> -	if (stat(path, &sb) == -1)
> +	if (stat(path, &sb) == -1) {
> +		PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path,
> +			     strerror(errno));
>  		return VIRTIO_USER_BACKEND_UNKNOWN;
> +	}
> 
> -	return S_ISSOCK(sb.st_mode) ?
> -		VIRTIO_USER_BACKEND_VHOST_USER :
> -		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> +	if (S_ISSOCK(sb.st_mode)) {
> +		return VIRTIO_USER_BACKEND_VHOST_USER;
> +	} else if (S_ISCHR(sb.st_mode)) {
> +		if (major(sb.st_rdev) == MISC_MAJOR)
> +			return VIRTIO_USER_BACKEND_VHOST_KERNEL;
> +		if (major(sb.st_rdev) == vdpa_dynamic_major_num())
> +			return VIRTIO_USER_BACKEND_VHOST_VDPA;
> +	}
> +	return VIRTIO_USER_BACKEND_UNKNOWN;
>  }
> 
>  static struct rte_eth_dev *
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 5/8] net/virtio: check protocol feature in user backend
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 5/8] net/virtio: check protocol feature in user backend Maxime Coquelin
@ 2020-09-30  5:49   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:49 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 5/8] net/virtio: check protocol feature in user backend
> 
> When sending set status message, move protocol feature check
> to vhost_user to be compatible with different backend types.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost_user.c      | 6 +++++-
>  drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ------
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index 12b6c7dbcf..ef290c357b 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -277,9 +277,13 @@ vhost_user_sock(struct virtio_user_dev *dev,
>  	msg.size = 0;
> 
>  	switch (req) {
> +	case VHOST_USER_GET_STATUS:
> +		if (!(dev->protocol_features &
> +				(1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
> +			return 0;
> +		/* Fallthrough */
>  	case VHOST_USER_GET_FEATURES:
>  	case VHOST_USER_GET_PROTOCOL_FEATURES:
> -	case VHOST_USER_GET_STATUS:
>  		need_reply = 1;
>  		break;
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index b79a9f84aa..d7cd6b0346 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -788,9 +788,6 @@ virtio_user_send_status_update(struct virtio_user_dev
> *dev, uint8_t status)
>  	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
> -	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> -		return 0;
> -
>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
>  	if (ret) {
>  		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
> @@ -811,9 +808,6 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
> -	if (!(dev->protocol_features & (1UL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> -		return 0;
> -
>  	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
>  	if (err) {
>  		PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err,
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 6/8] net/virtio: adapt Virtio-user status size
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 6/8] net/virtio: adapt Virtio-user status size Maxime Coquelin
@ 2020-09-30  5:49   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:49 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 6/8] net/virtio: adapt Virtio-user status size
> 
> Set proper payload size for set/get status message. The payload
> size varies according to backend types.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Patrick Fu <patrick.fu@intel.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 34 +++++++++++++------
>  1 file changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index d7cd6b0346..ded44bf32b 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -784,11 +784,15 @@ virtio_user_send_status_update(struct
> virtio_user_dev *dev, uint8_t status)
>  	int ret;
>  	uint64_t arg = status;
> 
> -	/* Vhost-user only for now */
> -	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> +		ret = dev->ops->send_request(dev,
> +				VHOST_USER_SET_STATUS, &arg);
> +	else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)
> +		ret = dev->ops->send_request(dev,
> +				VHOST_USER_SET_STATUS, &status);
> +	else
>  		return 0;
> 
> -	ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg);
>  	if (ret) {
>  		PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret,
>  			     strerror(errno));
> @@ -802,24 +806,32 @@ int
>  virtio_user_update_status(struct virtio_user_dev *dev)
>  {
>  	uint64_t ret;
> +	uint8_t status;
>  	int err;
> 
> -	/* Vhost-user only for now */
> -	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
> +		err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
> +		if (!err && ret > UINT8_MAX) {
> +			PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS "
> +					"response 0x%" PRIx64 "\n", ret);
> +			return -1;
> +		}
> +
> +		status = ret;
> +	} else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) {
> +		err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS,
> +				&status);
> +	} else {
>  		return 0;
> +	}
> 
> -	err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret);
>  	if (err) {
>  		PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err,
>  			     strerror(errno));
>  		return -1;
>  	}
> -	if (ret > UINT8_MAX) {
> -		PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS response 0x%"
> PRIx64 "\n", ret);
> -		return -1;
> -	}
> 
> -	dev->status = ret;
> +	dev->status = status;
>  	PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n"
>  			"\t-RESET: %u\n"
>  			"\t-ACKNOWLEDGE: %u\n"
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 7/8] net/virtio: split virtio-user start
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 7/8] net/virtio: split virtio-user start Maxime Coquelin
@ 2020-09-30  5:49   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:49 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 7/8] net/virtio: split virtio-user start
> 
> Move feature bit settings in device start out as an standalone
> function, so that feature bit could be negotiated at device
> feature_ok status.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 50 ++++++++++++-------
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  1 +
>  drivers/net/virtio/virtio_user_ethdev.c       |  4 ++
>  3 files changed, 37 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index ded44bf32b..63424656e3 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -112,25 +112,11 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>  }
> 
>  int
> -virtio_user_start_device(struct virtio_user_dev *dev)
> +virtio_user_dev_set_features(struct virtio_user_dev *dev)
>  {
>  	uint64_t features;
> -	int ret;
> +	int ret = -1;
> 
> -	/*
> -	 * XXX workaround!
> -	 *
> -	 * We need to make sure that the locks will be
> -	 * taken in the correct order to avoid deadlocks.
> -	 *
> -	 * Before releasing this lock, this thread should
> -	 * not trigger any memory hotplug events.
> -	 *
> -	 * This is a temporary workaround, and should be
> -	 * replaced when we get proper supports from the
> -	 * memory subsystem in the future.
> -	 */
> -	rte_mcfg_mem_read_lock();
>  	pthread_mutex_lock(&dev->mutex);
> 
>  	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> @@ -141,10 +127,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>  	if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0)
>  		goto error;
> 
> -	/* Step 1: negotiate protocol features & set features */
>  	features = dev->features;
> 
> -
>  	/* Strip VIRTIO_NET_F_MAC, as MAC address is handled in vdev init */
>  	features &= ~(1ull << VIRTIO_NET_F_MAC);
>  	/* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to know
> */
> @@ -154,6 +138,36 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>  	if (ret < 0)
>  		goto error;
>  	PMD_DRV_LOG(INFO, "set features: %" PRIx64, features);
> +error:
> +	pthread_mutex_unlock(&dev->mutex);
> +
> +	return ret;
> +}
> +
> +int
> +virtio_user_start_device(struct virtio_user_dev *dev)
> +{
> +	int ret;
> +
> +	/*
> +	 * XXX workaround!
> +	 *
> +	 * We need to make sure that the locks will be
> +	 * taken in the correct order to avoid deadlocks.
> +	 *
> +	 * Before releasing this lock, this thread should
> +	 * not trigger any memory hotplug events.
> +	 *
> +	 * This is a temporary workaround, and should be
> +	 * replaced when we get proper supports from the
> +	 * memory subsystem in the future.
> +	 */
> +	rte_mcfg_mem_read_lock();
> +	pthread_mutex_lock(&dev->mutex);
> +
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> +			dev->vhostfd < 0)
> +		goto error;
> 
>  	/* Step 2: share memory regions */
>  	ret = dev->ops->send_request(dev, VHOST_USER_SET_MEM_TABLE, NULL);
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 1c8c98b1cd..3e9d1a1eb3 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -68,6 +68,7 @@ struct virtio_user_dev {
>  	bool		started;
>  };
> 
> +int virtio_user_dev_set_features(struct virtio_user_dev *dev);
>  int virtio_user_start_device(struct virtio_user_dev *dev);
>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
> queues,
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 3a51afd711..f56fc238c4 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -269,7 +269,11 @@ static void
>  virtio_user_set_status(struct virtio_hw *hw, uint8_t status)
>  {
>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> +	uint8_t old_status = dev->status;
> 
> +	if (status & VIRTIO_CONFIG_STATUS_FEATURES_OK &&
> +			~old_status & VIRTIO_CONFIG_STATUS_FEATURES_OK)
> +		virtio_user_dev_set_features(dev);
>  	if (status & VIRTIO_CONFIG_STATUS_DRIVER_OK)
>  		virtio_user_start_device(dev);
>  	else if (status == VIRTIO_CONFIG_STATUS_RESET)
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
@ 2020-09-30  5:49   ` Xia, Chenbo
  0 siblings, 0 replies; 23+ messages in thread
From: Xia, Chenbo @ 2020-09-30  5:49 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Fu, Patrick, amorenoz

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend
> 
> vhost-vDPA is a new virtio backend type introduced by vDPA kernel
> framework, which provides abstruction to the vDPA devices and
> exposes an unified control interface through a char dev.
> 
> This patch adds support to the vhost-vDPA backend. As similar to
> the existing vhost kernel backend, a set of virtio_user ops were
> introduced for vhost-vDPA backend to handle device specific operations
> such as:
>  - device setup
>  - ioctl message handling
>  - queue pair enabling
>  - dma map/unmap
> vDPA relevant ioctl codes and data structures are also defined in
> this patch.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/meson.build                |   1 +
>  drivers/net/virtio/virtio_user/vhost.h        |   1 +
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 298 ++++++++++++++++++
>  .../net/virtio/virtio_user/virtio_user_dev.c  |   6 +
>  4 files changed, 306 insertions(+)
>  create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c
> 
> diff --git a/drivers/net/virtio/meson.build
> b/drivers/net/virtio/meson.build
> index 3fd6051f4b..eaed46373d 100644
> --- a/drivers/net/virtio/meson.build
> +++ b/drivers/net/virtio/meson.build
> @@ -42,6 +42,7 @@ if is_linux
>  		'virtio_user/vhost_kernel.c',
>  		'virtio_user/vhost_kernel_tap.c',
>  		'virtio_user/vhost_user.c',
> +		'virtio_user/vhost_vdpa.c',
>  		'virtio_user/virtio_user_dev.c')
>  	deps += ['bus_vdev']
>  endif
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 2e71995a79..210a3704e7 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -113,5 +113,6 @@ struct virtio_user_backend_ops {
> 
>  extern struct virtio_user_backend_ops virtio_ops_user;
>  extern struct virtio_user_backend_ops virtio_ops_kernel;
> +extern struct virtio_user_backend_ops virtio_ops_vdpa;
> 
>  #endif
> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> new file mode 100644
> index 0000000000..c7b9349fc8
> --- /dev/null
> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> @@ -0,0 +1,298 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Red Hat Inc.
> + */
> +
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +
> +#include <rte_memory.h>
> +
> +#include "vhost.h"
> +#include "virtio_user_dev.h"
> +
> +/* vhost kernel & vdpa ioctls */
> +#define VHOST_VIRTIO 0xAF
> +#define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64)
> +#define VHOST_SET_FEATURES _IOW(VHOST_VIRTIO, 0x00, __u64)
> +#define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
> +#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
> +#define VHOST_SET_MEM_TABLE _IOW(VHOST_VIRTIO, 0x03, void *)
> +#define VHOST_SET_LOG_BASE _IOW(VHOST_VIRTIO, 0x04, __u64)
> +#define VHOST_SET_LOG_FD _IOW(VHOST_VIRTIO, 0x07, int)
> +#define VHOST_SET_VRING_NUM _IOW(VHOST_VIRTIO, 0x10, struct
> vhost_vring_state)
> +#define VHOST_SET_VRING_ADDR _IOW(VHOST_VIRTIO, 0x11, struct
> vhost_vring_addr)
> +#define VHOST_SET_VRING_BASE _IOW(VHOST_VIRTIO, 0x12, struct
> vhost_vring_state)
> +#define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct
> vhost_vring_state)
> +#define VHOST_SET_VRING_KICK _IOW(VHOST_VIRTIO, 0x20, struct
> vhost_vring_file)
> +#define VHOST_SET_VRING_CALL _IOW(VHOST_VIRTIO, 0x21, struct
> vhost_vring_file)
> +#define VHOST_SET_VRING_ERR _IOW(VHOST_VIRTIO, 0x22, struct
> vhost_vring_file)
> +#define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct
> vhost_vring_file)
> +#define VHOST_VDPA_GET_DEVICE_ID _IOR(VHOST_VIRTIO, 0x70, __u32)
> +#define VHOST_VDPA_GET_STATUS _IOR(VHOST_VIRTIO, 0x71, __u8)
> +#define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8)
> +#define VHOST_VDPA_SET_VRING_ENABLE	_IOW(VHOST_VIRTIO, 0x75, \
> +					     struct vhost_vring_state)
> +
> +static uint64_t vhost_req_user_to_vdpa[] = {
> +	[VHOST_USER_SET_OWNER] = VHOST_SET_OWNER,
> +	[VHOST_USER_RESET_OWNER] = VHOST_RESET_OWNER,
> +	[VHOST_USER_SET_FEATURES] = VHOST_SET_FEATURES,
> +	[VHOST_USER_GET_FEATURES] = VHOST_GET_FEATURES,
> +	[VHOST_USER_SET_VRING_CALL] = VHOST_SET_VRING_CALL,
> +	[VHOST_USER_SET_VRING_NUM] = VHOST_SET_VRING_NUM,
> +	[VHOST_USER_SET_VRING_BASE] = VHOST_SET_VRING_BASE,
> +	[VHOST_USER_GET_VRING_BASE] = VHOST_GET_VRING_BASE,
> +	[VHOST_USER_SET_VRING_ADDR] = VHOST_SET_VRING_ADDR,
> +	[VHOST_USER_SET_VRING_KICK] = VHOST_SET_VRING_KICK,
> +	[VHOST_USER_SET_MEM_TABLE] = VHOST_SET_MEM_TABLE,
> +	[VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS,
> +	[VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS,
> +	[VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE,
> +};
> +
> +/* no alignment requirement */
> +struct vhost_iotlb_msg {
> +	uint64_t iova;
> +	uint64_t size;
> +	uint64_t uaddr;
> +#define VHOST_ACCESS_RO      0x1
> +#define VHOST_ACCESS_WO      0x2
> +#define VHOST_ACCESS_RW      0x3
> +	uint8_t perm;
> +#define VHOST_IOTLB_MISS           1
> +#define VHOST_IOTLB_UPDATE         2
> +#define VHOST_IOTLB_INVALIDATE     3
> +#define VHOST_IOTLB_ACCESS_FAIL    4
> +	uint8_t type;
> +};
> +
> +#define VHOST_IOTLB_MSG_V2 0x2
> +
> +struct vhost_msg {
> +	uint32_t type;
> +	uint32_t reserved;
> +	union {
> +		struct vhost_iotlb_msg iotlb;
> +		uint8_t padding[64];
> +	};
> +};
> +
> +static int
> +vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr,
> +				  uint64_t iova, size_t len)
> +{
> +	struct vhost_msg msg = {};
> +
> +	msg.type = VHOST_IOTLB_MSG_V2;
> +	msg.iotlb.type = VHOST_IOTLB_UPDATE;
> +	msg.iotlb.iova = iova;
> +	msg.iotlb.uaddr = (uint64_t)(uintptr_t)addr;
> +	msg.iotlb.size = len;
> +	msg.iotlb.perm = VHOST_ACCESS_RW;
> +
> +	if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
> +		PMD_DRV_LOG(ERR, "Failed to send IOTLB update (%s)",
> +				strerror(errno));
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, __rte_unused void *addr,
> +				  uint64_t iova, size_t len)
> +{
> +	struct vhost_msg msg = {};
> +
> +	msg.type = VHOST_IOTLB_MSG_V2;
> +	msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
> +	msg.iotlb.iova = iova;
> +	msg.iotlb.size = len;
> +
> +	if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) {
> +		PMD_DRV_LOG(ERR, "Failed to send IOTLB invalidate (%s)",
> +				strerror(errno));
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +
> +static int
> +vhost_vdpa_map_contig(const struct rte_memseg_list *msl,
> +		const struct rte_memseg *ms, size_t len, void *arg)
> +{
> +	struct virtio_user_dev *dev = arg;
> +
> +	if (msl->external)
> +		return 0;
> +
> +	return vhost_vdpa_dma_map(dev, ms->addr, ms->iova, len);
> +}
> +
> +static int
> +vhost_vdpa_map(const struct rte_memseg_list *msl, const struct rte_memseg
> *ms,
> +		void *arg)
> +{
> +	struct virtio_user_dev *dev = arg;
> +
> +	/* skip external memory that isn't a heap */
> +	if (msl->external && !msl->heap)
> +		return 0;
> +
> +	/* skip any segments with invalid IOVA addresses */
> +	if (ms->iova == RTE_BAD_IOVA)
> +		return 0;
> +
> +	/* if IOVA mode is VA, we've already mapped the internal segments */
> +	if (!msl->external && rte_eal_iova_mode() == RTE_IOVA_VA)
> +		return 0;
> +
> +	return vhost_vdpa_dma_map(dev, ms->addr, ms->iova, ms->len);
> +}
> +
> +static int
> +vhost_vdpa_dma_map_all(struct virtio_user_dev *dev)
> +{
> +	vhost_vdpa_dma_unmap(dev, NULL, 0, SIZE_MAX);
> +
> +	if (rte_eal_iova_mode() == RTE_IOVA_VA) {
> +		/* with IOVA as VA mode, we can get away with mapping
> contiguous
> +		 * chunks rather than going page-by-page.
> +		 */
> +		int ret = rte_memseg_contig_walk_thread_unsafe(
> +				vhost_vdpa_map_contig, dev);
> +		if (ret)
> +			return ret;
> +		/* we have to continue the walk because we've skipped the
> +		 * external segments during the config walk.
> +		 */
> +	}
> +	return rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev);
> +}
> +
> +/* with below features, vhost vdpa does not need to do the checksum and
> TSO,
> + * these info will be passed to virtio_user through virtio net header.
> + */
> +#define VHOST_VDPA_GUEST_OFFLOADS_MASK	\
> +	((1ULL << VIRTIO_NET_F_GUEST_CSUM) |	\
> +	 (1ULL << VIRTIO_NET_F_GUEST_TSO4) |	\
> +	 (1ULL << VIRTIO_NET_F_GUEST_TSO6) |	\
> +	 (1ULL << VIRTIO_NET_F_GUEST_ECN)  |	\
> +	 (1ULL << VIRTIO_NET_F_GUEST_UFO))
> +
> +#define VHOST_VDPA_HOST_OFFLOADS_MASK		\
> +	((1ULL << VIRTIO_NET_F_HOST_TSO4) |	\
> +	 (1ULL << VIRTIO_NET_F_HOST_TSO6) |	\
> +	 (1ULL << VIRTIO_NET_F_CSUM))
> +
> +static int
> +vhost_vdpa_ioctl(struct virtio_user_dev *dev,
> +		   enum vhost_user_request req,
> +		   void *arg)
> +{
> +	int ret = -1;
> +	uint64_t req_vdpa;
> +
> +	PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]);
> +
> +	req_vdpa = vhost_req_user_to_vdpa[req];
> +
> +	if (req_vdpa == VHOST_SET_MEM_TABLE)
> +		return vhost_vdpa_dma_map_all(dev);
> +
> +	if (req_vdpa == VHOST_SET_FEATURES) {
> +		/* WORKAROUND */
> +		*(uint64_t *)arg |= 1ULL << VIRTIO_F_IOMMU_PLATFORM;
> +
> +		/* Multiqueue not supported for now */
> +		*(uint64_t *)arg &= ~(1ULL << VIRTIO_NET_F_MQ);
> +	}
> +
> +	switch (req_vdpa) {
> +	case VHOST_SET_VRING_NUM:
> +	case VHOST_SET_VRING_ADDR:
> +	case VHOST_SET_VRING_BASE:
> +	case VHOST_GET_VRING_BASE:
> +	case VHOST_SET_VRING_KICK:
> +	case VHOST_SET_VRING_CALL:
> +		PMD_DRV_LOG(DEBUG, "vhostfd=%d, index=%u",
> +			    dev->vhostfd, *(unsigned int *)arg);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	ret = ioctl(dev->vhostfd, req_vdpa, arg);
> +	if (ret < 0)
> +		PMD_DRV_LOG(ERR, "%s failed: %s",
> +			    vhost_msg_strings[req], strerror(errno));
> +
> +	return ret;
> +}
> +
> +/**
> + * Set up environment to talk with a vhost vdpa backend.
> + *
> + * @return
> + *   - (-1) if fail to set up;
> + *   - (>=0) if successful.
> + */
> +static int
> +vhost_vdpa_setup(struct virtio_user_dev *dev)
> +{
> +	uint32_t did = (uint32_t)-1;
> +
> +	dev->vhostfd = open(dev->path, O_RDWR);
> +	if (dev->vhostfd < 0) {
> +		PMD_DRV_LOG(ERR, "Failed to open %s: %s\n",
> +				dev->path, strerror(errno));
> +		return -1;
> +	}
> +
> +	if (ioctl(dev->vhostfd, VHOST_VDPA_GET_DEVICE_ID, &did) < 0 ||
> +			did != VIRTIO_ID_NETWORK) {
> +		PMD_DRV_LOG(ERR, "Invalid vdpa device ID: %u\n", did);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
> +			       uint16_t pair_idx,
> +			       int enable)
> +{
> +	int i;
> +
> +	if (dev->qp_enabled[pair_idx] == enable)
> +		return 0;
> +
> +	for (i = 0; i < 2; ++i) {
> +		struct vhost_vring_state state = {
> +			.index = pair_idx * 2 + i,
> +			.num   = enable,
> +		};
> +
> +		if (vhost_vdpa_ioctl(dev, VHOST_USER_SET_VRING_ENABLE, &state))
> +			return -1;
> +	}
> +
> +	dev->qp_enabled[pair_idx] = enable;
> +
> +	return 0;
> +}
> +
> +struct virtio_user_backend_ops virtio_ops_vdpa = {
> +	.setup = vhost_vdpa_setup,
> +	.send_request = vhost_vdpa_ioctl,
> +	.enable_qp = vhost_vdpa_enable_queue_pair,
> +	.dma_map = vhost_vdpa_dma_map,
> +	.dma_unmap = vhost_vdpa_dma_unmap,
> +};
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 63424656e3..3681758ef1 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -389,6 +389,12 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  				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, "Unknown backend type");
> +			return -1;
>  		}
>  	}
> 
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend
  2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
                   ` (7 preceding siblings ...)
  2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
@ 2020-09-30 16:22 ` Maxime Coquelin
  8 siblings, 0 replies; 23+ messages in thread
From: Maxime Coquelin @ 2020-09-30 16:22 UTC (permalink / raw)
  To: dev, chenbo.xia, patrick.fu, amorenoz



On 9/29/20 6:13 PM, Maxime Coquelin wrote:
> vhost-vDPA is a new vhost backend type introduced by vDPA kernel
> framework, which provides abstruction to the vDPA devices and
> exposes to userspace a unified control interface through char devs.
> 
> This patch set adds vhost-vdpa backend type to the virtio_user.
> A set of vhost-vdpa specific ops callbacks are attached to the
> virtio_user according to the runtime dynamic check result of the
> backend type. DMA memory map/unmap callbacks are added to both
> vdev bus driver and virtio_user pmd to support address mapping.
> In addition, minor fixes to existing virtio control path are also
> implemented to make the new backend work.
> 
> This is a collaborative work done with Patrick Fu from Intel and
> Adrian Moreno from Red Hat. Thanks to them for their contributions.
> 
> The series has been tested with vdpasim and Intel IFC Kernel vDPA
> drivers, and more lightly with Mellanox mlx5_vdpa on ConnectX-6 Dx.
> 
> Changes in v3:
> --------------
>  * Fix 32bit builds (CI & Chenbo)
>  * Fix checkpatch
> 
> Changes in v2:
> --------------
>  * Split backend-type patch (Adrian)
>  * Fix get_status size (Chenbo)
>  * Various minro fixes (Chenbo)
> 
> Adrian Moreno (1):
>   net/virtio: move backend type selection to ethdev
> 
> Maxime Coquelin (7):
>   bus/vdev: add DMA mapping ops
>   net/virtio: introduce DMA ops
>   net/virtio: introduce Vhost-vDPA backend type
>   net/virtio: check protocol feature in user backend
>   net/virtio: adapt Virtio-user status size
>   net/virtio: split virtio-user start
>   net/virtio: introduce Vhost-vDPA backend
> 
>  drivers/bus/vdev/rte_bus_vdev.h               |  46 ++-
>  drivers/bus/vdev/vdev.c                       |  52 +++
>  drivers/net/virtio/meson.build                |   1 +
>  drivers/net/virtio/virtio_user/vhost.h        |   5 +
>  drivers/net/virtio/virtio_user/vhost_user.c   |   6 +-
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 298 ++++++++++++++++++
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 117 ++++---
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  13 +-
>  drivers/net/virtio/virtio_user_ethdev.c       | 126 +++++++-
>  9 files changed, 607 insertions(+), 57 deletions(-)
>  create mode 100644 drivers/net/virtio/virtio_user/vhost_vdpa.c
> 


Applied to dpdk-next-virtio/main.

Thanks,
Maxime


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

* Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev Maxime Coquelin
  2020-09-30  5:49   ` Xia, Chenbo
@ 2020-10-16  5:42   ` Wang, Yinan
  2020-10-16  5:58     ` Wang, Yinan
  2020-10-20  1:52     ` Wang, Yinan
  1 sibling, 2 replies; 23+ messages in thread
From: Wang, Yinan @ 2020-10-16  5:42 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo, Fu, Patrick, amorenoz

Hi Adrian,

This patch introduce a regression issue that vhost-user can't launch with server mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
This issue blocked amount of regression cases, could you help to take a look?
Thanks in advance!


BR,
Yinan


> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> Sent: 2020?9?30? 0:14
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
> ethdev
> 
> From: Adrian Moreno <amorenoz@redhat.com>
> 
> This is a preparation patch with no functional change.
> 
> Use an enum instead of a boolean for the backend type.
> Move the detection logic to the ethdev layer (where it is needed for the
> first time).
> The virtio_user_dev stores the backend type in the virtio_user_dev
> struct so the type is only determined once
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>  3 files changed, 50 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 2a0c861085..b79a9f84aa 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>  	return 0;
>  }
> 
> -int
> -is_vhost_user_by_type(const char *path)
> -{
> -	struct stat sb;
> -
> -	if (stat(path, &sb) == -1)
> -		return 0;
> -
> -	return S_ISSOCK(sb.st_mode);
> -}
> -
>  int
>  virtio_user_start_device(struct virtio_user_dev *dev)
>  {
> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>  	rte_mcfg_mem_read_lock();
>  	pthread_mutex_lock(&dev->mutex);
> 
> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> +			dev->vhostfd < 0)
>  		goto error;
> 
>  	/* Step 0: tell vhost to create queues */
> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  	dev->tapfds = NULL;
> 
>  	if (dev->is_server) {
> -		if (access(dev->path, F_OK) == 0 &&
> -		    !is_vhost_user_by_type(dev->path)) {
> -			PMD_DRV_LOG(ERR, "Server mode doesn't support
> vhost-kernel!");
> +		if (dev->backend_type !=
> VIRTIO_USER_BACKEND_VHOST_USER) {
> +			PMD_DRV_LOG(ERR, "Server mode only supports
> vhost-user!");
>  			return -1;
>  		}
>  		dev->ops = &virtio_ops_user;
>  	} else {
> -		if (is_vhost_user_by_type(dev->path)) {
> +		if (dev->backend_type ==
> VIRTIO_USER_BACKEND_VHOST_USER) {
>  			dev->ops = &virtio_ops_user;
> -		} else {
> +		} else if (dev->backend_type ==
> +
> 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			dev->ops = &virtio_ops_kernel;
> 
>  			dev->vhostfds = malloc(dev->max_queue_pairs *
> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		     int cq, int queue_size, const char *mac, char **ifname,
> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> +		     enum virtio_user_backend_type backend_type)
>  {
>  	uint64_t protocol_features = 0;
> 
> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	dev->frontend_features = 0;
>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>  	dev->protocol_features =
> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> +	dev->backend_type = backend_type;
> +
>  	parse_mac(dev, mac);
> 
>  	if (*ifname) {
> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  		return -1;
>  	}
> 
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->unsupported_features |=
>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> 
> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	}
> 
>  	/* The backend will not report this feature, we add it explicitly */
> -	if (is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> 
>  	/*
> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> *dev, uint8_t status)
>  	uint64_t arg = status;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  	int err;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1UL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 9377d5ba66..575bf430c0 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -10,6 +10,12 @@
>  #include "../virtio_pci.h"
>  #include "../virtio_ring.h"
> 
> +enum virtio_user_backend_type {
> +	VIRTIO_USER_BACKEND_UNKNOWN,
> +	VIRTIO_USER_BACKEND_VHOST_USER,
> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> +};
> +
>  struct virtio_user_queue {
>  	uint16_t used_idx;
>  	bool avail_wrap_counter;
> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>  };
> 
>  struct virtio_user_dev {
> +	enum virtio_user_backend_type backend_type;
>  	/* for vhost_user backend */
>  	int		vhostfd;
>  	int		listenfd;   /* listening fd */
> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>  	bool		started;
>  };
> 
> -int is_vhost_user_by_type(const char *path);
>  int virtio_user_start_device(struct virtio_user_dev *dev);
>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  			 int cq, int queue_size, const char *mac, char **ifname,
>  			 int server, int mrg_rxbuf, int in_order,
> -			 int packed_vq);
> +			 int packed_vq,
> +			 enum virtio_user_backend_type backend_type);
>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
>  void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 60d17af888..38b49bad5f 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -6,6 +6,7 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <sys/stat.h>
>  #include <sys/socket.h>
> 
>  #include <rte_malloc.h>
> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>  	return -errno;
>  }
> 
> +static enum virtio_user_backend_type
> +virtio_user_backend_type(const char *path)
> +{
> +	struct stat sb;
> +
> +	if (stat(path, &sb) == -1)
> +		return VIRTIO_USER_BACKEND_UNKNOWN;
> +
> +	return S_ISSOCK(sb.st_mode) ?
> +		VIRTIO_USER_BACKEND_VHOST_USER :
> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> +}
> +
>  static struct rte_eth_dev *
>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>  {
> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	struct rte_kvargs *kvlist = NULL;
>  	struct rte_eth_dev *eth_dev;
>  	struct virtio_hw *hw;
> +	enum virtio_user_backend_type backend_type =
> VIRTIO_USER_BACKEND_UNKNOWN;
>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  		goto end;
>  	}
> 
> +	backend_type = virtio_user_backend_type(path);
> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> +		PMD_INIT_LOG(ERR,
> +			     "unable to determine backend type for path %s",
> +			path);
> +		goto end;
> +	}
> +
> +
>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
> {
> -		if (is_vhost_user_by_type(path)) {
> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			PMD_INIT_LOG(ERR,
>  				"arg %s applies only to vhost-kernel backend",
>  				VIRTIO_USER_ARG_INTERFACE_NAME);
> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	hw = eth_dev->data->dev_private;
>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>  			 queue_size, mac_addr, &ifname, server_mode,
> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>  		virtio_user_eth_dev_free(eth_dev);
>  		goto end;
> --
> 2.26.2


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

* Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-10-16  5:42   ` Wang, Yinan
@ 2020-10-16  5:58     ` Wang, Yinan
  2020-10-20  1:52     ` Wang, Yinan
  1 sibling, 0 replies; 23+ messages in thread
From: Wang, Yinan @ 2020-10-16  5:58 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo, Fu, Patrick, amorenoz

Correct my typo issue that vhost-user can't launch with client mode.

BR,
Yinan

> -----Original Message-----
> From: Wang, Yinan
> Sent: 2020?10?16? 13:42
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
> amorenoz@redhat.com
> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
> to ethdev
> 
> Hi Adrian,
> 
> This patch introduce a regression issue that vhost-user can't launch with server
> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
> This issue blocked amount of regression cases, could you help to take a look?
> Thanks in advance!
> 
> 
> BR,
> Yinan
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> > Sent: 2020?9?30? 0:14
> > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> > <patrick.fu@intel.com>; amorenoz@redhat.com
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
> > ethdev
> >
> > From: Adrian Moreno <amorenoz@redhat.com>
> >
> > This is a preparation patch with no functional change.
> >
> > Use an enum instead of a boolean for the backend type.
> > Move the detection logic to the ethdev layer (where it is needed for the
> > first time).
> > The virtio_user_dev stores the backend type in the virtio_user_dev
> > struct so the type is only determined once
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> > ---
> >  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
> >  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
> >  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
> >  3 files changed, 50 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 2a0c861085..b79a9f84aa 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
> >  	return 0;
> >  }
> >
> > -int
> > -is_vhost_user_by_type(const char *path)
> > -{
> > -	struct stat sb;
> > -
> > -	if (stat(path, &sb) == -1)
> > -		return 0;
> > -
> > -	return S_ISSOCK(sb.st_mode);
> > -}
> > -
> >  int
> >  virtio_user_start_device(struct virtio_user_dev *dev)
> >  {
> > @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
> >  	rte_mcfg_mem_read_lock();
> >  	pthread_mutex_lock(&dev->mutex);
> >
> > -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> > +			dev->vhostfd < 0)
> >  		goto error;
> >
> >  	/* Step 0: tell vhost to create queues */
> > @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  	dev->tapfds = NULL;
> >
> >  	if (dev->is_server) {
> > -		if (access(dev->path, F_OK) == 0 &&
> > -		    !is_vhost_user_by_type(dev->path)) {
> > -			PMD_DRV_LOG(ERR, "Server mode doesn't support
> > vhost-kernel!");
> > +		if (dev->backend_type !=
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> > +			PMD_DRV_LOG(ERR, "Server mode only supports
> > vhost-user!");
> >  			return -1;
> >  		}
> >  		dev->ops = &virtio_ops_user;
> >  	} else {
> > -		if (is_vhost_user_by_type(dev->path)) {
> > +		if (dev->backend_type ==
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> >  			dev->ops = &virtio_ops_user;
> > -		} else {
> > +		} else if (dev->backend_type ==
> > +
> > 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			dev->ops = &virtio_ops_kernel;
> >
> >  			dev->vhostfds = malloc(dev->max_queue_pairs *
> > @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  int
> >  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  		     int cq, int queue_size, const char *mac, char **ifname,
> > -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> > +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> > +		     enum virtio_user_backend_type backend_type)
> >  {
> >  	uint64_t protocol_features = 0;
> >
> > @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	dev->frontend_features = 0;
> >  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
> >  	dev->protocol_features =
> > VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> > +	dev->backend_type = backend_type;
> > +
> >  	parse_mac(dev, mac);
> >
> >  	if (*ifname) {
> > @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  		return -1;
> >  	}
> >
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->unsupported_features |=
> >  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> >
> > @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	}
> >
> >  	/* The backend will not report this feature, we add it explicitly */
> > -	if (is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> >
> >  	/*
> > @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> > *dev, uint8_t status)
> >  	uint64_t arg = status;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1ULL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
> >  	int err;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1UL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > index 9377d5ba66..575bf430c0 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > @@ -10,6 +10,12 @@
> >  #include "../virtio_pci.h"
> >  #include "../virtio_ring.h"
> >
> > +enum virtio_user_backend_type {
> > +	VIRTIO_USER_BACKEND_UNKNOWN,
> > +	VIRTIO_USER_BACKEND_VHOST_USER,
> > +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> > +};
> > +
> >  struct virtio_user_queue {
> >  	uint16_t used_idx;
> >  	bool avail_wrap_counter;
> > @@ -17,6 +23,7 @@ struct virtio_user_queue {
> >  };
> >
> >  struct virtio_user_dev {
> > +	enum virtio_user_backend_type backend_type;
> >  	/* for vhost_user backend */
> >  	int		vhostfd;
> >  	int		listenfd;   /* listening fd */
> > @@ -60,13 +67,13 @@ struct virtio_user_dev {
> >  	bool		started;
> >  };
> >
> > -int is_vhost_user_by_type(const char *path);
> >  int virtio_user_start_device(struct virtio_user_dev *dev);
> >  int virtio_user_stop_device(struct virtio_user_dev *dev);
> >  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  			 int cq, int queue_size, const char *mac, char **ifname,
> >  			 int server, int mrg_rxbuf, int in_order,
> > -			 int packed_vq);
> > +			 int packed_vq,
> > +			 enum virtio_user_backend_type backend_type);
> >  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
> >  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
> >  void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
> > diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> > b/drivers/net/virtio/virtio_user_ethdev.c
> > index 60d17af888..38b49bad5f 100644
> > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > @@ -6,6 +6,7 @@
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >  #include <fcntl.h>
> > +#include <sys/stat.h>
> >  #include <sys/socket.h>
> >
> >  #include <rte_malloc.h>
> > @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
> >  	return -errno;
> >  }
> >
> > +static enum virtio_user_backend_type
> > +virtio_user_backend_type(const char *path)
> > +{
> > +	struct stat sb;
> > +
> > +	if (stat(path, &sb) == -1)
> > +		return VIRTIO_USER_BACKEND_UNKNOWN;
> > +
> > +	return S_ISSOCK(sb.st_mode) ?
> > +		VIRTIO_USER_BACKEND_VHOST_USER :
> > +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> > +}
> > +
> >  static struct rte_eth_dev *
> >  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
> >  {
> > @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	struct rte_kvargs *kvlist = NULL;
> >  	struct rte_eth_dev *eth_dev;
> >  	struct virtio_hw *hw;
> > +	enum virtio_user_backend_type backend_type =
> > VIRTIO_USER_BACKEND_UNKNOWN;
> >  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
> >  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
> >  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> > @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  		goto end;
> >  	}
> >
> > +	backend_type = virtio_user_backend_type(path);
> > +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> > +		PMD_INIT_LOG(ERR,
> > +			     "unable to determine backend type for path %s",
> > +			path);
> > +		goto end;
> > +	}
> > +
> > +
> >  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
> > {
> > -		if (is_vhost_user_by_type(path)) {
> > +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			PMD_INIT_LOG(ERR,
> >  				"arg %s applies only to vhost-kernel backend",
> >  				VIRTIO_USER_ARG_INTERFACE_NAME);
> > @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	hw = eth_dev->data->dev_private;
> >  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
> >  			 queue_size, mac_addr, &ifname, server_mode,
> > -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> > +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
> >  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
> >  		virtio_user_eth_dev_free(eth_dev);
> >  		goto end;
> > --
> > 2.26.2


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

* Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-10-16  5:42   ` Wang, Yinan
  2020-10-16  5:58     ` Wang, Yinan
@ 2020-10-20  1:52     ` Wang, Yinan
  2020-10-20  7:11       ` Maxime Coquelin
  1 sibling, 1 reply; 23+ messages in thread
From: Wang, Yinan @ 2020-10-20  1:52 UTC (permalink / raw)
  To: Maxime Coquelin, dev, Xia, Chenbo, Fu, Patrick, amorenoz

Hi Adrian/Maxime,

Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test.
Thanks in advance!

BR,
Yinan

> -----Original Message-----
> From: Wang, Yinan
> Sent: 2020?10?16? 13:42
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
> amorenoz@redhat.com
> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
> to ethdev
> 
> Hi Adrian,
> 
> This patch introduce a regression issue that vhost-user can't launch with server
> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
> This issue blocked amount of regression cases, could you help to take a look?
> Thanks in advance!
> 
> 
> BR,
> Yinan
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> > Sent: 2020?9?30? 0:14
> > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> > <patrick.fu@intel.com>; amorenoz@redhat.com
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
> > ethdev
> >
> > From: Adrian Moreno <amorenoz@redhat.com>
> >
> > This is a preparation patch with no functional change.
> >
> > Use an enum instead of a boolean for the backend type.
> > Move the detection logic to the ethdev layer (where it is needed for the
> > first time).
> > The virtio_user_dev stores the backend type in the virtio_user_dev
> > struct so the type is only determined once
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> > ---
> >  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
> >  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
> >  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
> >  3 files changed, 50 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 2a0c861085..b79a9f84aa 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
> >  	return 0;
> >  }
> >
> > -int
> > -is_vhost_user_by_type(const char *path)
> > -{
> > -	struct stat sb;
> > -
> > -	if (stat(path, &sb) == -1)
> > -		return 0;
> > -
> > -	return S_ISSOCK(sb.st_mode);
> > -}
> > -
> >  int
> >  virtio_user_start_device(struct virtio_user_dev *dev)
> >  {
> > @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
> >  	rte_mcfg_mem_read_lock();
> >  	pthread_mutex_lock(&dev->mutex);
> >
> > -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> > +			dev->vhostfd < 0)
> >  		goto error;
> >
> >  	/* Step 0: tell vhost to create queues */
> > @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  	dev->tapfds = NULL;
> >
> >  	if (dev->is_server) {
> > -		if (access(dev->path, F_OK) == 0 &&
> > -		    !is_vhost_user_by_type(dev->path)) {
> > -			PMD_DRV_LOG(ERR, "Server mode doesn't support
> > vhost-kernel!");
> > +		if (dev->backend_type !=
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> > +			PMD_DRV_LOG(ERR, "Server mode only supports
> > vhost-user!");
> >  			return -1;
> >  		}
> >  		dev->ops = &virtio_ops_user;
> >  	} else {
> > -		if (is_vhost_user_by_type(dev->path)) {
> > +		if (dev->backend_type ==
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> >  			dev->ops = &virtio_ops_user;
> > -		} else {
> > +		} else if (dev->backend_type ==
> > +
> > 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			dev->ops = &virtio_ops_kernel;
> >
> >  			dev->vhostfds = malloc(dev->max_queue_pairs *
> > @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  int
> >  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  		     int cq, int queue_size, const char *mac, char **ifname,
> > -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> > +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> > +		     enum virtio_user_backend_type backend_type)
> >  {
> >  	uint64_t protocol_features = 0;
> >
> > @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	dev->frontend_features = 0;
> >  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
> >  	dev->protocol_features =
> > VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> > +	dev->backend_type = backend_type;
> > +
> >  	parse_mac(dev, mac);
> >
> >  	if (*ifname) {
> > @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  		return -1;
> >  	}
> >
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->unsupported_features |=
> >  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> >
> > @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	}
> >
> >  	/* The backend will not report this feature, we add it explicitly */
> > -	if (is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> >
> >  	/*
> > @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> > *dev, uint8_t status)
> >  	uint64_t arg = status;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1ULL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
> >  	int err;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1UL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > index 9377d5ba66..575bf430c0 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > @@ -10,6 +10,12 @@
> >  #include "../virtio_pci.h"
> >  #include "../virtio_ring.h"
> >
> > +enum virtio_user_backend_type {
> > +	VIRTIO_USER_BACKEND_UNKNOWN,
> > +	VIRTIO_USER_BACKEND_VHOST_USER,
> > +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> > +};
> > +
> >  struct virtio_user_queue {
> >  	uint16_t used_idx;
> >  	bool avail_wrap_counter;
> > @@ -17,6 +23,7 @@ struct virtio_user_queue {
> >  };
> >
> >  struct virtio_user_dev {
> > +	enum virtio_user_backend_type backend_type;
> >  	/* for vhost_user backend */
> >  	int		vhostfd;
> >  	int		listenfd;   /* listening fd */
> > @@ -60,13 +67,13 @@ struct virtio_user_dev {
> >  	bool		started;
> >  };
> >
> > -int is_vhost_user_by_type(const char *path);
> >  int virtio_user_start_device(struct virtio_user_dev *dev);
> >  int virtio_user_stop_device(struct virtio_user_dev *dev);
> >  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  			 int cq, int queue_size, const char *mac, char **ifname,
> >  			 int server, int mrg_rxbuf, int in_order,
> > -			 int packed_vq);
> > +			 int packed_vq,
> > +			 enum virtio_user_backend_type backend_type);
> >  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
> >  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
> >  void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
> > diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> > b/drivers/net/virtio/virtio_user_ethdev.c
> > index 60d17af888..38b49bad5f 100644
> > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > @@ -6,6 +6,7 @@
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >  #include <fcntl.h>
> > +#include <sys/stat.h>
> >  #include <sys/socket.h>
> >
> >  #include <rte_malloc.h>
> > @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
> >  	return -errno;
> >  }
> >
> > +static enum virtio_user_backend_type
> > +virtio_user_backend_type(const char *path)
> > +{
> > +	struct stat sb;
> > +
> > +	if (stat(path, &sb) == -1)
> > +		return VIRTIO_USER_BACKEND_UNKNOWN;
> > +
> > +	return S_ISSOCK(sb.st_mode) ?
> > +		VIRTIO_USER_BACKEND_VHOST_USER :
> > +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> > +}
> > +
> >  static struct rte_eth_dev *
> >  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
> >  {
> > @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	struct rte_kvargs *kvlist = NULL;
> >  	struct rte_eth_dev *eth_dev;
> >  	struct virtio_hw *hw;
> > +	enum virtio_user_backend_type backend_type =
> > VIRTIO_USER_BACKEND_UNKNOWN;
> >  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
> >  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
> >  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> > @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  		goto end;
> >  	}
> >
> > +	backend_type = virtio_user_backend_type(path);
> > +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> > +		PMD_INIT_LOG(ERR,
> > +			     "unable to determine backend type for path %s",
> > +			path);
> > +		goto end;
> > +	}
> > +
> > +
> >  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
> > {
> > -		if (is_vhost_user_by_type(path)) {
> > +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			PMD_INIT_LOG(ERR,
> >  				"arg %s applies only to vhost-kernel backend",
> >  				VIRTIO_USER_ARG_INTERFACE_NAME);
> > @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	hw = eth_dev->data->dev_private;
> >  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
> >  			 queue_size, mac_addr, &ifname, server_mode,
> > -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> > +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
> >  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
> >  		virtio_user_eth_dev_free(eth_dev);
> >  		goto end;
> > --
> > 2.26.2


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

* Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-10-20  1:52     ` Wang, Yinan
@ 2020-10-20  7:11       ` Maxime Coquelin
  2020-10-20  7:20         ` Adrian Moreno
  0 siblings, 1 reply; 23+ messages in thread
From: Maxime Coquelin @ 2020-10-20  7:11 UTC (permalink / raw)
  To: Wang, Yinan, dev, Xia, Chenbo, Fu, Patrick, amorenoz

Hi Yinan,

On 10/20/20 3:52 AM, Wang, Yinan wrote:
> Hi Adrian/Maxime,
> 
> Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test.

We are working on it. Adrian managed to reproduce it, and fixed one
part of the issue. We''l continue the debugging today.

BTW, you are mentionning automated regression tests, any chances these
test land in the Intel functionnal CI at some point? I would be great to
catch these issues before the series are merged.

Thanks!
Maxime

> Thanks in advance!
> 
> BR,
> Yinan
> 
>> -----Original Message-----
>> From: Wang, Yinan
>> Sent: 2020?10?16? 13:42
>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
>> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
>> amorenoz@redhat.com
>> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
>> to ethdev
>>
>> Hi Adrian,
>>
>> This patch introduce a regression issue that vhost-user can't launch with server
>> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
>> This issue blocked amount of regression cases, could you help to take a look?
>> Thanks in advance!
>>
>>
>> BR,
>> Yinan
>>
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>>> Sent: 2020?9?30? 0:14
>>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
>>> <patrick.fu@intel.com>; amorenoz@redhat.com
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
>>> ethdev
>>>
>>> From: Adrian Moreno <amorenoz@redhat.com>
>>>
>>> This is a preparation patch with no functional change.
>>>
>>> Use an enum instead of a boolean for the backend type.
>>> Move the detection logic to the ethdev layer (where it is needed for the
>>> first time).
>>> The virtio_user_dev stores the backend type in the virtio_user_dev
>>> struct so the type is only determined once
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>> ---
>>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>>>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>>>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>>>  3 files changed, 50 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> index 2a0c861085..b79a9f84aa 100644
>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>>>  	return 0;
>>>  }
>>>
>>> -int
>>> -is_vhost_user_by_type(const char *path)
>>> -{
>>> -	struct stat sb;
>>> -
>>> -	if (stat(path, &sb) == -1)
>>> -		return 0;
>>> -
>>> -	return S_ISSOCK(sb.st_mode);
>>> -}
>>> -
>>>  int
>>>  virtio_user_start_device(struct virtio_user_dev *dev)
>>>  {
>>> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>>>  	rte_mcfg_mem_read_lock();
>>>  	pthread_mutex_lock(&dev->mutex);
>>>
>>> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
>>> +			dev->vhostfd < 0)
>>>  		goto error;
>>>
>>>  	/* Step 0: tell vhost to create queues */
>>> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>  	dev->tapfds = NULL;
>>>
>>>  	if (dev->is_server) {
>>> -		if (access(dev->path, F_OK) == 0 &&
>>> -		    !is_vhost_user_by_type(dev->path)) {
>>> -			PMD_DRV_LOG(ERR, "Server mode doesn't support
>>> vhost-kernel!");
>>> +		if (dev->backend_type !=
>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>> +			PMD_DRV_LOG(ERR, "Server mode only supports
>>> vhost-user!");
>>>  			return -1;
>>>  		}
>>>  		dev->ops = &virtio_ops_user;
>>>  	} else {
>>> -		if (is_vhost_user_by_type(dev->path)) {
>>> +		if (dev->backend_type ==
>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>>  			dev->ops = &virtio_ops_user;
>>> -		} else {
>>> +		} else if (dev->backend_type ==
>>> +
>>> 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>  			dev->ops = &virtio_ops_kernel;
>>>
>>>  			dev->vhostfds = malloc(dev->max_queue_pairs *
>>> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>  int
>>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>  		     int cq, int queue_size, const char *mac, char **ifname,
>>> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
>>> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
>>> +		     enum virtio_user_backend_type backend_type)
>>>  {
>>>  	uint64_t protocol_features = 0;
>>>
>>> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>> *path, int queues,
>>>  	dev->frontend_features = 0;
>>>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>>>  	dev->protocol_features =
>>> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
>>> +	dev->backend_type = backend_type;
>>> +
>>>  	parse_mac(dev, mac);
>>>
>>>  	if (*ifname) {
>>> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>> *path, int queues,
>>>  		return -1;
>>>  	}
>>>
>>> -	if (!is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		dev->unsupported_features |=
>>>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
>>>
>>> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>> *path, int queues,
>>>  	}
>>>
>>>  	/* The backend will not report this feature, we add it explicitly */
>>> -	if (is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
>>>
>>>  	/*
>>> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
>>> *dev, uint8_t status)
>>>  	uint64_t arg = status;
>>>
>>>  	/* Vhost-user only for now */
>>> -	if (!is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		return 0;
>>>
>>>  	if (!(dev->protocol_features & (1ULL <<
>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>>>  	int err;
>>>
>>>  	/* Vhost-user only for now */
>>> -	if (!is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		return 0;
>>>
>>>  	if (!(dev->protocol_features & (1UL <<
>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> index 9377d5ba66..575bf430c0 100644
>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> @@ -10,6 +10,12 @@
>>>  #include "../virtio_pci.h"
>>>  #include "../virtio_ring.h"
>>>
>>> +enum virtio_user_backend_type {
>>> +	VIRTIO_USER_BACKEND_UNKNOWN,
>>> +	VIRTIO_USER_BACKEND_VHOST_USER,
>>> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
>>> +};
>>> +
>>>  struct virtio_user_queue {
>>>  	uint16_t used_idx;
>>>  	bool avail_wrap_counter;
>>> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>>>  };
>>>
>>>  struct virtio_user_dev {
>>> +	enum virtio_user_backend_type backend_type;
>>>  	/* for vhost_user backend */
>>>  	int		vhostfd;
>>>  	int		listenfd;   /* listening fd */
>>> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>>>  	bool		started;
>>>  };
>>>
>>> -int is_vhost_user_by_type(const char *path);
>>>  int virtio_user_start_device(struct virtio_user_dev *dev);
>>>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>>>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>  			 int cq, int queue_size, const char *mac, char **ifname,
>>>  			 int server, int mrg_rxbuf, int in_order,
>>> -			 int packed_vq);
>>> +			 int packed_vq,
>>> +			 enum virtio_user_backend_type backend_type);
>>>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>>>  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
>>>  void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>> index 60d17af888..38b49bad5f 100644
>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>> @@ -6,6 +6,7 @@
>>>  #include <sys/types.h>
>>>  #include <unistd.h>
>>>  #include <fcntl.h>
>>> +#include <sys/stat.h>
>>>  #include <sys/socket.h>
>>>
>>>  #include <rte_malloc.h>
>>> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>>>  	return -errno;
>>>  }
>>>
>>> +static enum virtio_user_backend_type
>>> +virtio_user_backend_type(const char *path)
>>> +{
>>> +	struct stat sb;
>>> +
>>> +	if (stat(path, &sb) == -1)
>>> +		return VIRTIO_USER_BACKEND_UNKNOWN;
>>> +
>>> +	return S_ISSOCK(sb.st_mode) ?
>>> +		VIRTIO_USER_BACKEND_VHOST_USER :
>>> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
>>> +}
>>> +
>>>  static struct rte_eth_dev *
>>>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>>>  {
>>> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>  	struct rte_kvargs *kvlist = NULL;
>>>  	struct rte_eth_dev *eth_dev;
>>>  	struct virtio_hw *hw;
>>> +	enum virtio_user_backend_type backend_type =
>>> VIRTIO_USER_BACKEND_UNKNOWN;
>>>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>>>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>>>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
>>> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>  		goto end;
>>>  	}
>>>
>>> +	backend_type = virtio_user_backend_type(path);
>>> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
>>> +		PMD_INIT_LOG(ERR,
>>> +			     "unable to determine backend type for path %s",
>>> +			path);
>>> +		goto end;
>>> +	}
>>> +
>>> +
>>>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
>>> {
>>> -		if (is_vhost_user_by_type(path)) {
>>> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>  			PMD_INIT_LOG(ERR,
>>>  				"arg %s applies only to vhost-kernel backend",
>>>  				VIRTIO_USER_ARG_INTERFACE_NAME);
>>> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>  	hw = eth_dev->data->dev_private;
>>>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>>>  			 queue_size, mac_addr, &ifname, server_mode,
>>> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
>>> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>>>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>>>  		virtio_user_eth_dev_free(eth_dev);
>>>  		goto end;
>>> --
>>> 2.26.2
> 


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

* Re: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
  2020-10-20  7:11       ` Maxime Coquelin
@ 2020-10-20  7:20         ` Adrian Moreno
  0 siblings, 0 replies; 23+ messages in thread
From: Adrian Moreno @ 2020-10-20  7:20 UTC (permalink / raw)
  To: Maxime Coquelin, Wang, Yinan, dev, Xia, Chenbo, Fu, Patrick

Hi Yinan,

Still testing, but submitting for early review:
https://patches.dpdk.org/patch/81431/

Thanks,

On 10/20/20 9:11 AM, Maxime Coquelin wrote:
> Hi Yinan,
> 
> On 10/20/20 3:52 AM, Wang, Yinan wrote:
>> Hi Adrian/Maxime,
>>
>> Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test.
> 
> We are working on it. Adrian managed to reproduce it, and fixed one
> part of the issue. We''l continue the debugging today.
> 

> BTW, you are mentionning automated regression tests, any chances these
> test land in the Intel functionnal CI at some point? I would be great to
> catch these issues before the series are merged.
> 
> Thanks!
> Maxime
> 
>> Thanks in advance!
>>
>> BR,
>> Yinan
>>
>>> -----Original Message-----
>>> From: Wang, Yinan
>>> Sent: 2020?10?16? 13:42
>>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
>>> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
>>> amorenoz@redhat.com
>>> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
>>> to ethdev
>>>
>>> Hi Adrian,
>>>
>>> This patch introduce a regression issue that vhost-user can't launch with server
>>> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
>>> This issue blocked amount of regression cases, could you help to take a look?
>>> Thanks in advance!
>>>
>>>
>>> BR,
>>> Yinan
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>>>> Sent: 2020?9?30? 0:14
>>>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
>>>> <patrick.fu@intel.com>; amorenoz@redhat.com
>>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
>>>> ethdev
>>>>
>>>> From: Adrian Moreno <amorenoz@redhat.com>
>>>>
>>>> This is a preparation patch with no functional change.
>>>>
>>>> Use an enum instead of a boolean for the backend type.
>>>> Move the detection logic to the ethdev layer (where it is needed for the
>>>> first time).
>>>> The virtio_user_dev stores the backend type in the virtio_user_dev
>>>> struct so the type is only determined once
>>>>
>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>> ---
>>>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>>>>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>>>>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>>>>  3 files changed, 50 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> index 2a0c861085..b79a9f84aa 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>>>>  	return 0;
>>>>  }
>>>>
>>>> -int
>>>> -is_vhost_user_by_type(const char *path)
>>>> -{
>>>> -	struct stat sb;
>>>> -
>>>> -	if (stat(path, &sb) == -1)
>>>> -		return 0;
>>>> -
>>>> -	return S_ISSOCK(sb.st_mode);
>>>> -}
>>>> -
>>>>  int
>>>>  virtio_user_start_device(struct virtio_user_dev *dev)
>>>>  {
>>>> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>>>>  	rte_mcfg_mem_read_lock();
>>>>  	pthread_mutex_lock(&dev->mutex);
>>>>
>>>> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
>>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
>>>> +			dev->vhostfd < 0)
>>>>  		goto error;
>>>>
>>>>  	/* Step 0: tell vhost to create queues */
>>>> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>>  	dev->tapfds = NULL;
>>>>
>>>>  	if (dev->is_server) {
>>>> -		if (access(dev->path, F_OK) == 0 &&
>>>> -		    !is_vhost_user_by_type(dev->path)) {
>>>> -			PMD_DRV_LOG(ERR, "Server mode doesn't support
>>>> vhost-kernel!");
>>>> +		if (dev->backend_type !=
>>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>>> +			PMD_DRV_LOG(ERR, "Server mode only supports
>>>> vhost-user!");
>>>>  			return -1;
>>>>  		}
>>>>  		dev->ops = &virtio_ops_user;
>>>>  	} else {
>>>> -		if (is_vhost_user_by_type(dev->path)) {
>>>> +		if (dev->backend_type ==
>>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>>>  			dev->ops = &virtio_ops_user;
>>>> -		} else {
>>>> +		} else if (dev->backend_type ==
>>>> +
>>>> 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>>  			dev->ops = &virtio_ops_kernel;
>>>>
>>>>  			dev->vhostfds = malloc(dev->max_queue_pairs *
>>>> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>>  int
>>>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>>  		     int cq, int queue_size, const char *mac, char **ifname,
>>>> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
>>>> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
>>>> +		     enum virtio_user_backend_type backend_type)
>>>>  {
>>>>  	uint64_t protocol_features = 0;
>>>>
>>>> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>>> *path, int queues,
>>>>  	dev->frontend_features = 0;
>>>>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>>>>  	dev->protocol_features =
>>>> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
>>>> +	dev->backend_type = backend_type;
>>>> +
>>>>  	parse_mac(dev, mac);
>>>>
>>>>  	if (*ifname) {
>>>> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>>> *path, int queues,
>>>>  		return -1;
>>>>  	}
>>>>
>>>> -	if (!is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		dev->unsupported_features |=
>>>>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
>>>>
>>>> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>>> *path, int queues,
>>>>  	}
>>>>
>>>>  	/* The backend will not report this feature, we add it explicitly */
>>>> -	if (is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
>>>>
>>>>  	/*
>>>> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
>>>> *dev, uint8_t status)
>>>>  	uint64_t arg = status;
>>>>
>>>>  	/* Vhost-user only for now */
>>>> -	if (!is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		return 0;
>>>>
>>>>  	if (!(dev->protocol_features & (1ULL <<
>>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>>> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>>>>  	int err;
>>>>
>>>>  	/* Vhost-user only for now */
>>>> -	if (!is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		return 0;
>>>>
>>>>  	if (!(dev->protocol_features & (1UL <<
>>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> index 9377d5ba66..575bf430c0 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> @@ -10,6 +10,12 @@
>>>>  #include "../virtio_pci.h"
>>>>  #include "../virtio_ring.h"
>>>>
>>>> +enum virtio_user_backend_type {
>>>> +	VIRTIO_USER_BACKEND_UNKNOWN,
>>>> +	VIRTIO_USER_BACKEND_VHOST_USER,
>>>> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
>>>> +};
>>>> +
>>>>  struct virtio_user_queue {
>>>>  	uint16_t used_idx;
>>>>  	bool avail_wrap_counter;
>>>> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>>>>  };
>>>>
>>>>  struct virtio_user_dev {
>>>> +	enum virtio_user_backend_type backend_type;
>>>>  	/* for vhost_user backend */
>>>>  	int		vhostfd;
>>>>  	int		listenfd;   /* listening fd */
>>>> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>>>>  	bool		started;
>>>>  };
>>>>
>>>> -int is_vhost_user_by_type(const char *path);
>>>>  int virtio_user_start_device(struct virtio_user_dev *dev);
>>>>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>>>>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>>  			 int cq, int queue_size, const char *mac, char **ifname,
>>>>  			 int server, int mrg_rxbuf, int in_order,
>>>> -			 int packed_vq);
>>>> +			 int packed_vq,
>>>> +			 enum virtio_user_backend_type backend_type);
>>>>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>>>>  void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
>>>>  void virtio_user_handle_cq_packed(struct virtio_user_dev *dev,
>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>> index 60d17af888..38b49bad5f 100644
>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>> @@ -6,6 +6,7 @@
>>>>  #include <sys/types.h>
>>>>  #include <unistd.h>
>>>>  #include <fcntl.h>
>>>> +#include <sys/stat.h>
>>>>  #include <sys/socket.h>
>>>>
>>>>  #include <rte_malloc.h>
>>>> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>>>>  	return -errno;
>>>>  }
>>>>
>>>> +static enum virtio_user_backend_type
>>>> +virtio_user_backend_type(const char *path)
>>>> +{
>>>> +	struct stat sb;
>>>> +
>>>> +	if (stat(path, &sb) == -1)
>>>> +		return VIRTIO_USER_BACKEND_UNKNOWN;
>>>> +
>>>> +	return S_ISSOCK(sb.st_mode) ?
>>>> +		VIRTIO_USER_BACKEND_VHOST_USER :
>>>> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
>>>> +}
>>>> +
>>>>  static struct rte_eth_dev *
>>>>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>>>>  {
>>>> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>>  	struct rte_kvargs *kvlist = NULL;
>>>>  	struct rte_eth_dev *eth_dev;
>>>>  	struct virtio_hw *hw;
>>>> +	enum virtio_user_backend_type backend_type =
>>>> VIRTIO_USER_BACKEND_UNKNOWN;
>>>>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>>>>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>>>>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
>>>> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>>  		goto end;
>>>>  	}
>>>>
>>>> +	backend_type = virtio_user_backend_type(path);
>>>> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
>>>> +		PMD_INIT_LOG(ERR,
>>>> +			     "unable to determine backend type for path %s",
>>>> +			path);
>>>> +		goto end;
>>>> +	}
>>>> +
>>>> +
>>>>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
>>>> {
>>>> -		if (is_vhost_user_by_type(path)) {
>>>> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>>  			PMD_INIT_LOG(ERR,
>>>>  				"arg %s applies only to vhost-kernel backend",
>>>>  				VIRTIO_USER_ARG_INTERFACE_NAME);
>>>> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>>  	hw = eth_dev->data->dev_private;
>>>>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>>>>  			 queue_size, mac_addr, &ifname, server_mode,
>>>> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
>>>> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>>>>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>>>>  		virtio_user_eth_dev_free(eth_dev);
>>>>  		goto end;
>>>> --
>>>> 2.26.2
>>
> 

-- 
Adrián Moreno


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

end of thread, other threads:[~2020-10-20  7:21 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 16:13 [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend Maxime Coquelin
2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 1/8] bus/vdev: add DMA mapping ops Maxime Coquelin
2020-09-30  5:47   ` Xia, Chenbo
2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 2/8] net/virtio: introduce DMA ops Maxime Coquelin
2020-09-30  5:47   ` Xia, Chenbo
2020-09-29 16:13 ` [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to ethdev Maxime Coquelin
2020-09-30  5:49   ` Xia, Chenbo
2020-10-16  5:42   ` Wang, Yinan
2020-10-16  5:58     ` Wang, Yinan
2020-10-20  1:52     ` Wang, Yinan
2020-10-20  7:11       ` Maxime Coquelin
2020-10-20  7:20         ` Adrian Moreno
2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 4/8] net/virtio: introduce Vhost-vDPA backend type Maxime Coquelin
2020-09-30  5:49   ` Xia, Chenbo
2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 5/8] net/virtio: check protocol feature in user backend Maxime Coquelin
2020-09-30  5:49   ` Xia, Chenbo
2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 6/8] net/virtio: adapt Virtio-user status size Maxime Coquelin
2020-09-30  5:49   ` Xia, Chenbo
2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 7/8] net/virtio: split virtio-user start Maxime Coquelin
2020-09-30  5:49   ` Xia, Chenbo
2020-09-29 16:14 ` [dpdk-dev] [PATCH v3 8/8] net/virtio: introduce Vhost-vDPA backend Maxime Coquelin
2020-09-30  5:49   ` Xia, Chenbo
2020-09-30 16:22 ` [dpdk-dev] [PATCH v3 0/8] virtio-user: introduce vhost-vdpa backend 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).