[PATCH 1/2] net/virtio-user: improve kick performance with notification area mapping

[PATCH 1/2] net/virtio-user: improve kick performance with notification area mapping

[Srujana Challa]

This patch introduces new virtio-user callback to map the vq
notification area and implements it for the vhost-vDPA backend.
This is simply done by using mmap()/munmap() for
the vhost-vDPA fd.

This patch also adds a parameter for configuring feature bit
VIRTIO_NET_F_NOTIFICATION_DATA. If feature is disabled, also
update corresponding unsupported feature bit. And also adds
code to write to queue notify address in notify callback.
This will help in increasing the kick performance.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 doc/guides/nics/virtio.rst                    |  5 ++
 drivers/net/virtio/virtio_user/vhost.h        |  1 +
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 56 ++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  | 52 +++++++++++++++--
 .../net/virtio/virtio_user/virtio_user_dev.h  |  5 +-
 drivers/net/virtio/virtio_user_ethdev.c       | 57 ++++++++++++++++---
 6 files changed, 162 insertions(+), 14 deletions(-)

diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
index c22ce56a02..11dd6359e5 100644
--- a/doc/guides/nics/virtio.rst
+++ b/doc/guides/nics/virtio.rst
@@ -349,6 +349,11 @@ Below devargs are supported by the virtio-user vdev:
     election.
     (Default: 0 (disabled))
 
+#.  ``notification_data``:
+
+    It is used to enable virtio device notification data feature.
+    (Default: 1 (enabled))
+
 Virtio paths Selection and Usage
 --------------------------------
 
diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index f817cab77a..1bce00c7ac 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -90,6 +90,7 @@ struct virtio_user_backend_ops {
 	int (*server_disconnect)(struct virtio_user_dev *dev);
 	int (*server_reconnect)(struct virtio_user_dev *dev);
 	int (*get_intr_fd)(struct virtio_user_dev *dev);
+	int (*map_notification_area)(struct virtio_user_dev *dev, bool map);
 };
 
 extern struct virtio_user_backend_ops virtio_ops_user;
diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index 2c36b26224..1eb0f9ec48 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -5,6 +5,7 @@
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -622,6 +623,60 @@ vhost_vdpa_get_intr_fd(struct virtio_user_dev *dev __rte_unused)
 	return -1;
 }
 
+static int
+unmap_notification_area(struct virtio_user_dev *dev, int nr_vrings)
+{
+	int i;
+
+	for (i = 0; i < nr_vrings; i++) {
+		if (dev->notify_area[i])
+			munmap(dev->notify_area[i], getpagesize());
+	}
+	free(dev->notify_area);
+
+	return 0;
+}
+
+static int
+vhost_vdpa_map_notification_area(struct virtio_user_dev *dev, bool map)
+{
+	struct vhost_vdpa_data *data = dev->backend_data;
+	int nr_vrings, i, page_size = getpagesize();
+	uint16_t **notify_area;
+
+	nr_vrings = dev->max_queue_pairs * 2;
+	if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
+		nr_vrings++;
+
+	if (!map)
+		return unmap_notification_area(dev, nr_vrings);
+
+	notify_area = malloc(nr_vrings * sizeof(*notify_area));
+	if (!notify_area) {
+		PMD_DRV_LOG(ERR, "(%s) Failed to allocate notify area array", dev->path);
+		return -1;
+	}
+	for (i = 0; i < nr_vrings; i++) {
+		notify_area[i] = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED | MAP_FILE,
+				      data->vhostfd, i * page_size);
+		if (notify_area[i] == MAP_FAILED) {
+			PMD_DRV_LOG(ERR, "(%s) Map failed for notify address of queue %d\n",
+				    dev->path, i);
+			goto map_err;
+		}
+	}
+	dev->notify_area = notify_area;
+
+	return 0;
+
+map_err:
+	i--;
+	for (; i >= 0; i--)
+		munmap(notify_area[i], page_size);
+	free(notify_area);
+	return -1;
+}
+
 struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.setup = vhost_vdpa_setup,
 	.destroy = vhost_vdpa_destroy,
@@ -646,4 +701,5 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.dma_unmap = vhost_vdpa_dma_unmap_batch,
 	.update_link_state = vhost_vdpa_update_link_state,
 	.get_intr_fd = vhost_vdpa_get_intr_fd,
+	.map_notification_area = vhost_vdpa_map_notification_area,
 };
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index af1f8c8237..578877d089 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -18,6 +18,7 @@
 #include <rte_string_fns.h>
 #include <rte_eal_memconfig.h>
 #include <rte_malloc.h>
+#include <rte_io.h>
 
 #include "vhost.h"
 #include "virtio_user_dev.h"
@@ -413,6 +414,12 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
 		dev->callfds[i] = callfd;
 		dev->kickfds[i] = kickfd;
 	}
+	dev->notify_area_mapped = false;
+	if (dev->ops->map_notification_area) {
+		if (dev->ops->map_notification_area(dev, true))
+			goto err;
+		dev->notify_area_mapped = true;
+	}
 
 	return 0;
 err:
@@ -445,6 +452,11 @@ virtio_user_dev_uninit_notify(struct virtio_user_dev *dev)
 			dev->callfds[i] = -1;
 		}
 	}
+	if (dev->ops->map_notification_area && dev->notify_area_mapped) {
+		/* Unmap notification area */
+		dev->ops->map_notification_area(dev, false);
+		dev->notify_area_mapped = false;
+	}
 }
 
 static int
@@ -674,12 +686,14 @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
 	 1ULL << VIRTIO_F_IN_ORDER		|	\
 	 1ULL << VIRTIO_F_VERSION_1		|	\
-	 1ULL << VIRTIO_F_RING_PACKED)
+	 1ULL << VIRTIO_F_RING_PACKED		|	\
+	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
 		     int cq, int queue_size, const char *mac, char **ifname,
 		     int server, int mrg_rxbuf, int in_order, int packed_vq,
+		     int notification_data,
 		     enum virtio_user_backend_type backend_type)
 {
 	uint64_t backend_features;
@@ -737,6 +751,9 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
 	if (!packed_vq)
 		dev->unsupported_features |= (1ull << VIRTIO_F_RING_PACKED);
 
+	if (!notification_data)
+		dev->unsupported_features |= (1ull << VIRTIO_F_NOTIFICATION_DATA);
+
 	if (dev->mac_specified)
 		dev->frontend_features |= (1ull << VIRTIO_NET_F_MAC);
 	else
@@ -1039,11 +1056,35 @@ static void
 virtio_user_control_queue_notify(struct virtqueue *vq, void *cookie)
 {
 	struct virtio_user_dev *dev = cookie;
-	uint64_t buf = 1;
+	uint64_t notify_data = 1;
+
+	if (!dev->notify_area_mapped) {
+		if (write(dev->kickfds[vq->vq_queue_index], &notify_data, sizeof(notify_data)) < 0)
+			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
+				    strerror(errno));
+		return;
+	} else if (!virtio_with_feature(&dev->hw, VIRTIO_F_NOTIFICATION_DATA)) {
+		rte_write16(vq->vq_queue_index, vq->notify_addr);
+		return;
+	}
 
-	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
-		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
-			    strerror(errno));
+	if (virtio_with_packed_queue(&dev->hw)) {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:30]: avail index
+		 * Bit[31]: avail wrap counter
+		 */
+		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
+				VRING_PACKED_DESC_F_AVAIL)) << 31) |
+				((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	} else {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:31]: avail index
+		 */
+		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	}
+	rte_write32(notify_data, vq->notify_addr);
 }
 
 int
@@ -1062,6 +1103,7 @@ virtio_user_dev_create_shadow_cvq(struct virtio_user_dev *dev, struct virtqueue
 
 	scvq->cq.notify_queue = &virtio_user_control_queue_notify;
 	scvq->cq.notify_cookie = dev;
+	scvq->notify_addr = vq->notify_addr;
 	dev->scvq = scvq;
 
 	return 0;
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 7323d88302..29ec386da5 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -64,6 +64,9 @@ struct virtio_user_dev {
 	struct virtqueue	*scvq;
 
 	void *backend_data;
+
+	bool notify_area_mapped;
+	uint16_t **notify_area;
 };
 
 int virtio_user_dev_set_features(struct virtio_user_dev *dev);
@@ -72,7 +75,7 @@ int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t 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, int notification_data,
 			 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);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 3a31642899..241465ecdd 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -18,6 +18,7 @@
 #include <bus_vdev_driver.h>
 #include <rte_alarm.h>
 #include <rte_cycles.h>
+#include <rte_io.h>
 
 #include "virtio_ethdev.h"
 #include "virtio_logs.h"
@@ -232,6 +233,7 @@ virtio_user_setup_queue(struct virtio_hw *hw, struct virtqueue *vq)
 	else
 		virtio_user_setup_queue_split(vq, dev);
 
+	vq->notify_addr = dev->notify_area[vq->vq_queue_index];
 	if (dev->hw_cvq && hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq))
 		return virtio_user_dev_create_shadow_cvq(dev, vq);
 
@@ -262,8 +264,8 @@ virtio_user_del_queue(struct virtio_hw *hw, struct virtqueue *vq)
 static void
 virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 {
-	uint64_t buf = 1;
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
+	uint64_t notify_data = 1;
 
 	if (hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq)) {
 		virtio_user_handle_cq(dev, vq->vq_queue_index);
@@ -271,9 +273,34 @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 		return;
 	}
 
-	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
-		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
-			    strerror(errno));
+	if (!dev->notify_area_mapped) {
+		if (write(dev->kickfds[vq->vq_queue_index], &notify_data,
+			  sizeof(notify_data)) < 0)
+			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
+				    strerror(errno));
+		return;
+	} else if (!virtio_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA)) {
+		rte_write16(vq->vq_queue_index, vq->notify_addr);
+		return;
+	}
+
+	if (virtio_with_packed_queue(hw)) {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:30]: avail index
+		 * Bit[31]: avail wrap counter
+		 */
+		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
+				VRING_PACKED_DESC_F_AVAIL)) << 31) |
+				((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	} else {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:31]: avail index
+		 */
+		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	}
+	rte_write32(notify_data, vq->notify_addr);
 }
 
 static int
@@ -329,6 +356,8 @@ static const char *valid_args[] = {
 	VIRTIO_USER_ARG_SPEED,
 #define VIRTIO_USER_ARG_VECTORIZED     "vectorized"
 	VIRTIO_USER_ARG_VECTORIZED,
+#define VIRTIO_USER_ARG_NOTIFICATION_DATA  "notification_data"
+	VIRTIO_USER_ARG_NOTIFICATION_DATA,
 	NULL
 };
 
@@ -480,6 +509,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 	uint64_t in_order = 1;
 	uint64_t packed_vq = 0;
 	uint64_t vectorized = 0;
+	uint64_t notification_data = 1;
 	char *path = NULL;
 	char *ifname = NULL;
 	char *mac_addr = NULL;
@@ -637,6 +667,15 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 		}
 	}
 
+	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_NOTIFICATION_DATA) == 1) {
+		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_NOTIFICATION_DATA,
+				       &get_integer_arg, &notification_data) < 0) {
+			PMD_INIT_LOG(ERR, "error to parse %s",
+				     VIRTIO_USER_ARG_NOTIFICATION_DATA);
+			goto end;
+		}
+	}
+
 	eth_dev = virtio_user_eth_dev_alloc(vdev);
 	if (!eth_dev) {
 		PMD_INIT_LOG(ERR, "virtio_user fails to alloc device");
@@ -645,9 +684,10 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
 
 	dev = eth_dev->data->dev_private;
 	hw = &dev->hw;
-	if (virtio_user_dev_init(dev, path, (uint16_t)queues, cq,
-			 queue_size, mac_addr, &ifname, server_mode,
-			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
+	if (virtio_user_dev_init(dev, path, (uint16_t)queues, cq, queue_size,
+				 mac_addr, &ifname, server_mode, mrg_rxbuf,
+				 in_order, packed_vq, notification_data,
+				 backend_type) < 0) {
 		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
 		virtio_user_eth_dev_free(eth_dev);
 		goto end;
@@ -784,4 +824,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
 	"in_order=<0|1> "
 	"packed_vq=<0|1> "
 	"speed=<int> "
-	"vectorized=<0|1>");
+	"vectorized=<0|1> "
+	"notification_data=<0|1> ");
-- 
2.25.1

[PATCH 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Srujana Challa]

This patch introduces new function to get rss device config
and adds code to forward the RSS control command to backend
through hw control queue if RSS feature is negotiated.
This patch will help to negotiate VIRTIO_NET_F_RSS feature
if vhost-vdpa backend supports RSS in HW.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 578877d089..b0937c9df9 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -304,6 +304,24 @@ virtio_user_dev_init_max_queue_pairs(struct virtio_user_dev *dev, uint32_t user_
 	return 0;
 }
 
+int
+virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst, size_t offset, int length)
+{
+	int ret = 0;
+
+	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_RSS)))
+		return -ENOTSUP;
+
+	if (!dev->ops->get_config)
+		return -ENOTSUP;
+
+	ret = dev->ops->get_config(dev, dst, offset, length);
+	if (ret)
+		PMD_DRV_LOG(ERR, "(%s) Failed to get rss config in device", dev->path);
+
+	return ret;
+}
+
 int
 virtio_user_dev_set_mac(struct virtio_user_dev *dev)
 {
@@ -687,7 +705,8 @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_F_IN_ORDER		|	\
 	 1ULL << VIRTIO_F_VERSION_1		|	\
 	 1ULL << VIRTIO_F_RING_PACKED		|	\
-	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
+	 1ULL << VIRTIO_F_NOTIFICATION_DATA	|	\
+	 1ULL << VIRTIO_NET_F_RSS)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
@@ -903,6 +922,11 @@ virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct vring *vri
 
 		queues = *(uint16_t *)(uintptr_t)vring->desc[idx_data].addr;
 		status = virtio_user_handle_mq(dev, queues);
+	} else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
+		struct virtio_net_ctrl_rss *rss;
+
+		rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring->desc[idx_data].addr;
+		status = virtio_user_handle_mq(dev, rss->max_tx_vq);
 	} else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
 		   hdr->class == VIRTIO_NET_CTRL_MAC ||
 		   hdr->class == VIRTIO_NET_CTRL_VLAN) {
@@ -964,6 +988,11 @@ virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
 		queues = *(uint16_t *)(uintptr_t)
 				vring->desc[idx_data].addr;
 		status = virtio_user_handle_mq(dev, queues);
+	} else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
+		struct virtio_net_ctrl_rss *rss;
+
+		rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring->desc[idx_data].addr;
+		status = virtio_user_handle_mq(dev, rss->max_tx_vq);
 	} else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
 		   hdr->class == VIRTIO_NET_CTRL_MAC ||
 		   hdr->class == VIRTIO_NET_CTRL_VLAN) {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 29ec386da5..39b3eec0f2 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -86,6 +86,8 @@ int virtio_user_dev_update_status(struct virtio_user_dev *dev);
 int virtio_user_dev_update_link_state(struct virtio_user_dev *dev);
 int virtio_user_dev_set_mac(struct virtio_user_dev *dev);
 int virtio_user_dev_get_mac(struct virtio_user_dev *dev);
+int virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst, size_t offset,
+				   int length);
 void virtio_user_dev_delayed_disconnect_handler(void *param);
 int virtio_user_dev_server_reconnect(struct virtio_user_dev *dev);
 extern const char * const virtio_user_backend_strings[];
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 241465ecdd..6c10e8f6c0 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -52,6 +52,9 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 
 	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
 		*(uint16_t *)dst = dev->max_queue_pairs;
+
+	if (offset >= offsetof(struct virtio_net_config, rss_max_key_size))
+		virtio_user_dev_get_rss_config(dev, dst, offset, length);
 }
 
 static void
-- 
2.25.1

RE: [EXT] [PATCH 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Srujana Challa]

Ping

> ----------------------------------------------------------------------
> This patch introduces new function to get rss device config and adds code to
> forward the RSS control command to backend through hw control queue if
> RSS feature is negotiated.
> This patch will help to negotiate VIRTIO_NET_F_RSS feature if vhost-vdpa
> backend supports RSS in HW.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
> .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
>  drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 578877d089..b0937c9df9 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -304,6 +304,24 @@ virtio_user_dev_init_max_queue_pairs(struct
> virtio_user_dev *dev, uint32_t user_
>  	return 0;
>  }
> 
> +int
> +virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst,
> +size_t offset, int length) {
> +	int ret = 0;
> +
> +	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_RSS)))
> +		return -ENOTSUP;
> +
> +	if (!dev->ops->get_config)
> +		return -ENOTSUP;
> +
> +	ret = dev->ops->get_config(dev, dst, offset, length);
> +	if (ret)
> +		PMD_DRV_LOG(ERR, "(%s) Failed to get rss config in device",
> +dev->path);
> +
> +	return ret;
> +}
> +
>  int
>  virtio_user_dev_set_mac(struct virtio_user_dev *dev)  { @@ -687,7 +705,8
> @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
>  	 1ULL << VIRTIO_F_IN_ORDER		|	\
>  	 1ULL << VIRTIO_F_VERSION_1		|	\
>  	 1ULL << VIRTIO_F_RING_PACKED		|	\
> -	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
> +	 1ULL << VIRTIO_F_NOTIFICATION_DATA	|	\
> +	 1ULL << VIRTIO_NET_F_RSS)
> 
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t
> queues, @@ -903,6 +922,11 @@ virtio_user_handle_ctrl_msg_split(struct
> virtio_user_dev *dev, struct vring *vri
> 
>  		queues = *(uint16_t *)(uintptr_t)vring->desc[idx_data].addr;
>  		status = virtio_user_handle_mq(dev, queues);
> +	} else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd ==
> VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
> +		struct virtio_net_ctrl_rss *rss;
> +
> +		rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring-
> >desc[idx_data].addr;
> +		status = virtio_user_handle_mq(dev, rss->max_tx_vq);
>  	} else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
>  		   hdr->class == VIRTIO_NET_CTRL_MAC ||
>  		   hdr->class == VIRTIO_NET_CTRL_VLAN) { @@ -964,6
> +988,11 @@ virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev
> *dev,
>  		queues = *(uint16_t *)(uintptr_t)
>  				vring->desc[idx_data].addr;
>  		status = virtio_user_handle_mq(dev, queues);
> +	} else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd ==
> VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
> +		struct virtio_net_ctrl_rss *rss;
> +
> +		rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring-
> >desc[idx_data].addr;
> +		status = virtio_user_handle_mq(dev, rss->max_tx_vq);
>  	} else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
>  		   hdr->class == VIRTIO_NET_CTRL_MAC ||
>  		   hdr->class == VIRTIO_NET_CTRL_VLAN) { diff --git
> a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 29ec386da5..39b3eec0f2 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -86,6 +86,8 @@ int virtio_user_dev_update_status(struct
> virtio_user_dev *dev);  int virtio_user_dev_update_link_state(struct
> virtio_user_dev *dev);  int virtio_user_dev_set_mac(struct virtio_user_dev
> *dev);  int virtio_user_dev_get_mac(struct virtio_user_dev *dev);
> +int virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst,
> size_t offset,
> +				   int length);
>  void virtio_user_dev_delayed_disconnect_handler(void *param);  int
> virtio_user_dev_server_reconnect(struct virtio_user_dev *dev);  extern const
> char * const virtio_user_backend_strings[]; diff --git
> a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 241465ecdd..6c10e8f6c0 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -52,6 +52,9 @@ virtio_user_read_dev_config(struct virtio_hw *hw,
> size_t offset,
> 
>  	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
>  		*(uint16_t *)dst = dev->max_queue_pairs;
> +
> +	if (offset >= offsetof(struct virtio_net_config, rss_max_key_size))
> +		virtio_user_dev_get_rss_config(dev, dst, offset, length);
>  }
> 
>  static void
> --
> 2.25.1

Re: [PATCH 1/2] net/virtio-user: improve kick performance with notification area mapping

[Maxime Coquelin]

Hi Srujana,

Thanks for your contribution!
Is it possible to provide information on which hardware it can be tested
on?

On 12/8/23 06:31, Srujana Challa wrote:
> This patch introduces new virtio-user callback to map the vq
> notification area and implements it for the vhost-vDPA backend.
> This is simply done by using mmap()/munmap() for
> the vhost-vDPA fd.
> 
> This patch also adds a parameter for configuring feature bit
> VIRTIO_NET_F_NOTIFICATION_DATA. If feature is disabled, also

VIRTIO_F_NOTIFICATION_DATA*

> update corresponding unsupported feature bit. And also adds
> code to write to queue notify address in notify callback.
> This will help in increasing the kick performance.

Do you have any number about the performance improvement?

> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>   doc/guides/nics/virtio.rst                    |  5 ++
>   drivers/net/virtio/virtio_user/vhost.h        |  1 +
>   drivers/net/virtio/virtio_user/vhost_vdpa.c   | 56 ++++++++++++++++++
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 52 +++++++++++++++--
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  5 +-
>   drivers/net/virtio/virtio_user_ethdev.c       | 57 ++++++++++++++++---
>   6 files changed, 162 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
> index c22ce56a02..11dd6359e5 100644
> --- a/doc/guides/nics/virtio.rst
> +++ b/doc/guides/nics/virtio.rst
> @@ -349,6 +349,11 @@ Below devargs are supported by the virtio-user vdev:
>       election.
>       (Default: 0 (disabled))
>   
> +#.  ``notification_data``:
> +
> +    It is used to enable virtio device notification data feature.
> +    (Default: 1 (enabled))

Is there any reason currently to make it configurable?

As it is enabled by default, I guess we want it to be negociated if the
device supports it.

Let's remove the devarg for now, and we can revisit if the need arise?

> +
>   Virtio paths Selection and Usage
>   --------------------------------
>   
> diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
> index f817cab77a..1bce00c7ac 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -90,6 +90,7 @@ struct virtio_user_backend_ops {
>   	int (*server_disconnect)(struct virtio_user_dev *dev);
>   	int (*server_reconnect)(struct virtio_user_dev *dev);
>   	int (*get_intr_fd)(struct virtio_user_dev *dev);
> +	int (*map_notification_area)(struct virtio_user_dev *dev, bool map);
>   };
>   
>   extern struct virtio_user_backend_ops virtio_ops_user;
> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> index 2c36b26224..1eb0f9ec48 100644
> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> @@ -5,6 +5,7 @@
>   #include <sys/ioctl.h>
>   #include <sys/types.h>
>   #include <sys/stat.h>
> +#include <sys/mman.h>
>   #include <fcntl.h>
>   #include <stdlib.h>
>   #include <unistd.h>
> @@ -622,6 +623,60 @@ vhost_vdpa_get_intr_fd(struct virtio_user_dev *dev __rte_unused)
>   	return -1;
>   }
>   
> +static int
> +unmap_notification_area(struct virtio_user_dev *dev, int nr_vrings)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_vrings; i++) {
> +		if (dev->notify_area[i])
> +			munmap(dev->notify_area[i], getpagesize());
> +	}
> +	free(dev->notify_area);
> +
> +	return 0;
> +}
> +
> +static int
> +vhost_vdpa_map_notification_area(struct virtio_user_dev *dev, bool map)
> +{
> +	struct vhost_vdpa_data *data = dev->backend_data;
> +	int nr_vrings, i, page_size = getpagesize();
> +	uint16_t **notify_area;
> +
> +	nr_vrings = dev->max_queue_pairs * 2;
> +	if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
> +		nr_vrings++;
> +
> +	if (!map)
> +		return unmap_notification_area(dev, nr_vrings);
> +
> +	notify_area = malloc(nr_vrings * sizeof(*notify_area));
> +	if (!notify_area) {
> +		PMD_DRV_LOG(ERR, "(%s) Failed to allocate notify area array", dev->path);
> +		return -1;
> +	}

Add new line here.

> +	for (i = 0; i < nr_vrings; i++) {
> +		notify_area[i] = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED | MAP_FILE,
> +				      data->vhostfd, i * page_size);
> +		if (notify_area[i] == MAP_FAILED) {
> +			PMD_DRV_LOG(ERR, "(%s) Map failed for notify address of queue %d\n",
> +				    dev->path, i);
> +			goto map_err;
> +		}
> +	}
> +	dev->notify_area = notify_area;
> +
> +	return 0;
> +
> +map_err:
> +	i--;

I would move this before the goto map_err;

> +	for (; i >= 0; i--)
> +		munmap(notify_area[i], page_size);
> +	free(notify_area);

New line here.

> +	return -1;
> +}
> +
>   struct virtio_user_backend_ops virtio_ops_vdpa = {
>   	.setup = vhost_vdpa_setup,
>   	.destroy = vhost_vdpa_destroy,
> @@ -646,4 +701,5 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
>   	.dma_unmap = vhost_vdpa_dma_unmap_batch,
>   	.update_link_state = vhost_vdpa_update_link_state,
>   	.get_intr_fd = vhost_vdpa_get_intr_fd,
> +	.map_notification_area = vhost_vdpa_map_notification_area,

I wonder if it wouldn't be cleaner to have one cb for map, one for
unmap. WDYT?

>   };
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index af1f8c8237..578877d089 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -18,6 +18,7 @@
>   #include <rte_string_fns.h>
>   #include <rte_eal_memconfig.h>
>   #include <rte_malloc.h>
> +#include <rte_io.h>
>   
>   #include "vhost.h"
>   #include "virtio_user_dev.h"
> @@ -413,6 +414,12 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
>   		dev->callfds[i] = callfd;
>   		dev->kickfds[i] = kickfd;
>   	}
> +	dev->notify_area_mapped = false;
Can't we just check on dev->notify_area instead of introducing a new
field?

> +	if (dev->ops->map_notification_area) {
> +		if (dev->ops->map_notification_area(dev, true))
> +			goto err;
> +		dev->notify_area_mapped = true;
> +	}
>   
>   	return 0;
>   err:
> @@ -445,6 +452,11 @@ virtio_user_dev_uninit_notify(struct virtio_user_dev *dev)
>   			dev->callfds[i] = -1;
>   		}
>   	}
> +	if (dev->ops->map_notification_area && dev->notify_area_mapped) {
> +		/* Unmap notification area */
> +		dev->ops->map_notification_area(dev, false);
> +		dev->notify_area_mapped = false;
> +	}
>   }
>   
>   static int
> @@ -674,12 +686,14 @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
>   	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
>   	 1ULL << VIRTIO_F_IN_ORDER		|	\
>   	 1ULL << VIRTIO_F_VERSION_1		|	\
> -	 1ULL << VIRTIO_F_RING_PACKED)
> +	 1ULL << VIRTIO_F_RING_PACKED		|	\
> +	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
>   
>   int
>   virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
>   		     int cq, int queue_size, const char *mac, char **ifname,
>   		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> +		     int notification_data,
>   		     enum virtio_user_backend_type backend_type)
>   {
>   	uint64_t backend_features;
> @@ -737,6 +751,9 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
>   	if (!packed_vq)
>   		dev->unsupported_features |= (1ull << VIRTIO_F_RING_PACKED);
>   
> +	if (!notification_data)
> +		dev->unsupported_features |= (1ull << VIRTIO_F_NOTIFICATION_DATA);
> +
>   	if (dev->mac_specified)
>   		dev->frontend_features |= (1ull << VIRTIO_NET_F_MAC);
>   	else
> @@ -1039,11 +1056,35 @@ static void
>   virtio_user_control_queue_notify(struct virtqueue *vq, void *cookie)
>   {
>   	struct virtio_user_dev *dev = cookie;
> -	uint64_t buf = 1;
> +	uint64_t notify_data = 1;
> +
> +	if (!dev->notify_area_mapped) {
> +		if (write(dev->kickfds[vq->vq_queue_index], &notify_data, sizeof(notify_data)) < 0)
> +			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> +				    strerror(errno));
> +		return;
> +	} else if (!virtio_with_feature(&dev->hw, VIRTIO_F_NOTIFICATION_DATA)) {
> +		rte_write16(vq->vq_queue_index, vq->notify_addr);
> +		return;
> +	}
>   
> -	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
> -		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> -			    strerror(errno));
> +	if (virtio_with_packed_queue(&dev->hw)) {
> +		/* Bit[0:15]: vq queue index
> +		 * Bit[16:30]: avail index
> +		 * Bit[31]: avail wrap counter
> +		 */
> +		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
> +				VRING_PACKED_DESC_F_AVAIL)) << 31) |
> +				((uint32_t)vq->vq_avail_idx << 16) |
> +				vq->vq_queue_index;
> +	} else {
> +		/* Bit[0:15]: vq queue index
> +		 * Bit[16:31]: avail index
> +		 */
> +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> +				vq->vq_queue_index;
> +	}
> +	rte_write32(notify_data, vq->notify_addr);
>   }
>   
>   int
> @@ -1062,6 +1103,7 @@ virtio_user_dev_create_shadow_cvq(struct virtio_user_dev *dev, struct virtqueue
>   
>   	scvq->cq.notify_queue = &virtio_user_control_queue_notify;
>   	scvq->cq.notify_cookie = dev;
> +	scvq->notify_addr = vq->notify_addr;
>   	dev->scvq = scvq;
>   
>   	return 0;
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 7323d88302..29ec386da5 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -64,6 +64,9 @@ struct virtio_user_dev {
>   	struct virtqueue	*scvq;
>   
>   	void *backend_data;
> +
> +	bool notify_area_mapped;
> +	uint16_t **notify_area;
>   };
>   
>   int virtio_user_dev_set_features(struct virtio_user_dev *dev);
> @@ -72,7 +75,7 @@ int virtio_user_stop_device(struct virtio_user_dev *dev);
>   int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t 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, int notification_data,
>   			 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);
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
> index 3a31642899..241465ecdd 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -18,6 +18,7 @@
>   #include <bus_vdev_driver.h>
>   #include <rte_alarm.h>
>   #include <rte_cycles.h>
> +#include <rte_io.h>
>   
>   #include "virtio_ethdev.h"
>   #include "virtio_logs.h"
> @@ -232,6 +233,7 @@ virtio_user_setup_queue(struct virtio_hw *hw, struct virtqueue *vq)
>   	else
>   		virtio_user_setup_queue_split(vq, dev);
>   
> +	vq->notify_addr = dev->notify_area[vq->vq_queue_index];
>   	if (dev->hw_cvq && hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq))
>   		return virtio_user_dev_create_shadow_cvq(dev, vq);
>   
> @@ -262,8 +264,8 @@ virtio_user_del_queue(struct virtio_hw *hw, struct virtqueue *vq)
>   static void
>   virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
>   {
> -	uint64_t buf = 1;
>   	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> +	uint64_t notify_data = 1;
>   
>   	if (hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq)) {
>   		virtio_user_handle_cq(dev, vq->vq_queue_index);
> @@ -271,9 +273,34 @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
>   		return;
>   	}
>   
> -	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
> -		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> -			    strerror(errno));
> +	if (!dev->notify_area_mapped) {
> +		if (write(dev->kickfds[vq->vq_queue_index], &notify_data,
> +			  sizeof(notify_data)) < 0)
> +			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> +				    strerror(errno));
> +		return;
> +	} else if (!virtio_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA)) {
> +		rte_write16(vq->vq_queue_index, vq->notify_addr);
> +		return;
> +	}
> +
> +	if (virtio_with_packed_queue(hw)) {
> +		/* Bit[0:15]: vq queue index
> +		 * Bit[16:30]: avail index
> +		 * Bit[31]: avail wrap counter
> +		 */
> +		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
> +				VRING_PACKED_DESC_F_AVAIL)) << 31) |
> +				((uint32_t)vq->vq_avail_idx << 16) |
> +				vq->vq_queue_index;
> +	} else {
> +		/* Bit[0:15]: vq queue index
> +		 * Bit[16:31]: avail index
> +		 */
> +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> +				vq->vq_queue_index;
> +	}
> +	rte_write32(notify_data, vq->notify_addr);
>   }
>   
>   static int
> @@ -329,6 +356,8 @@ static const char *valid_args[] = {
>   	VIRTIO_USER_ARG_SPEED,
>   #define VIRTIO_USER_ARG_VECTORIZED     "vectorized"
>   	VIRTIO_USER_ARG_VECTORIZED,
> +#define VIRTIO_USER_ARG_NOTIFICATION_DATA  "notification_data"
> +	VIRTIO_USER_ARG_NOTIFICATION_DATA,
>   	NULL
>   };
>   
> @@ -480,6 +509,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
>   	uint64_t in_order = 1;
>   	uint64_t packed_vq = 0;
>   	uint64_t vectorized = 0;
> +	uint64_t notification_data = 1;
>   	char *path = NULL;
>   	char *ifname = NULL;
>   	char *mac_addr = NULL;
> @@ -637,6 +667,15 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
>   		}
>   	}
>   
> +	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_NOTIFICATION_DATA) == 1) {
> +		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_NOTIFICATION_DATA,
> +				       &get_integer_arg, &notification_data) < 0) {
> +			PMD_INIT_LOG(ERR, "error to parse %s",
> +				     VIRTIO_USER_ARG_NOTIFICATION_DATA);
> +			goto end;
> +		}
> +	}
> +
>   	eth_dev = virtio_user_eth_dev_alloc(vdev);
>   	if (!eth_dev) {
>   		PMD_INIT_LOG(ERR, "virtio_user fails to alloc device");
> @@ -645,9 +684,10 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
>   
>   	dev = eth_dev->data->dev_private;
>   	hw = &dev->hw;
> -	if (virtio_user_dev_init(dev, path, (uint16_t)queues, cq,
> -			 queue_size, mac_addr, &ifname, server_mode,
> -			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
> +	if (virtio_user_dev_init(dev, path, (uint16_t)queues, cq, queue_size,
> +				 mac_addr, &ifname, server_mode, mrg_rxbuf,
> +				 in_order, packed_vq, notification_data,
> +				 backend_type) < 0) {
>   		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>   		virtio_user_eth_dev_free(eth_dev);
>   		goto end;
> @@ -784,4 +824,5 @@ RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
>   	"in_order=<0|1> "
>   	"packed_vq=<0|1> "
>   	"speed=<int> "
> -	"vectorized=<0|1>");
> +	"vectorized=<0|1> "
> +	"notification_data=<0|1> ");

Re: [PATCH 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Maxime Coquelin]

Hi,

On 12/8/23 06:31, Srujana Challa wrote:
> This patch introduces new function to get rss device config
> and adds code to forward the RSS control command to backend
> through hw control queue if RSS feature is negotiated.
> This patch will help to negotiate VIRTIO_NET_F_RSS feature
> if vhost-vdpa backend supports RSS in HW.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
>   drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
>   3 files changed, 35 insertions(+), 1 deletion(-)
> 

Nice! Same question as on patch 1, I would be interested in knowing
which hardware supports this feature.

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

Thanks,
Maxime

RE: [EXT] Re: [PATCH 1/2] net/virtio-user: improve kick performance with notification area mapping

[Srujana Challa]

> Hi Srujana,
> 
> Thanks for your contribution!
> Is it possible to provide information on which hardware it can be tested on?
It can be tested on Marvell's Octeon DPU.

> 
> On 12/8/23 06:31, Srujana Challa wrote:
> > This patch introduces new virtio-user callback to map the vq
> > notification area and implements it for the vhost-vDPA backend.
> > This is simply done by using mmap()/munmap() for the vhost-vDPA fd.
> >
> > This patch also adds a parameter for configuring feature bit
> > VIRTIO_NET_F_NOTIFICATION_DATA. If feature is disabled, also
> 
> VIRTIO_F_NOTIFICATION_DATA*
> 
> > update corresponding unsupported feature bit. And also adds code to
> > write to queue notify address in notify callback.
> > This will help in increasing the kick performance.
> 
> Do you have any number about the performance improvement?

We don't have exact comparison with the performance data, we are mainly supporting
notification data with notify area which enables direct data path notifications and
avoids intermediate checks in-between.

> 
> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > ---
> >   doc/guides/nics/virtio.rst                    |  5 ++
> >   drivers/net/virtio/virtio_user/vhost.h        |  1 +
> >   drivers/net/virtio/virtio_user/vhost_vdpa.c   | 56 ++++++++++++++++++
> >   .../net/virtio/virtio_user/virtio_user_dev.c  | 52 +++++++++++++++--
> >   .../net/virtio/virtio_user/virtio_user_dev.h  |  5 +-
> >   drivers/net/virtio/virtio_user_ethdev.c       | 57 ++++++++++++++++---
> >   6 files changed, 162 insertions(+), 14 deletions(-)
> >
> > diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst
> > index c22ce56a02..11dd6359e5 100644
> > --- a/doc/guides/nics/virtio.rst
> > +++ b/doc/guides/nics/virtio.rst
> > @@ -349,6 +349,11 @@ Below devargs are supported by the virtio-user
> vdev:
> >       election.
> >       (Default: 0 (disabled))
> >
> > +#.  ``notification_data``:
> > +
> > +    It is used to enable virtio device notification data feature.
> > +    (Default: 1 (enabled))
> 
> Is there any reason currently to make it configurable?
> 
> As it is enabled by default, I guess we want it to be negociated if the device
> supports it.
> 
> Let's remove the devarg for now, and we can revisit if the need arise?
Ack.

> 
> > +
> >   Virtio paths Selection and Usage
> >   --------------------------------
> >
> > diff --git a/drivers/net/virtio/virtio_user/vhost.h
> > b/drivers/net/virtio/virtio_user/vhost.h
> > index f817cab77a..1bce00c7ac 100644
> > --- a/drivers/net/virtio/virtio_user/vhost.h
> > +++ b/drivers/net/virtio/virtio_user/vhost.h
> > @@ -90,6 +90,7 @@ struct virtio_user_backend_ops {
> >   	int (*server_disconnect)(struct virtio_user_dev *dev);
> >   	int (*server_reconnect)(struct virtio_user_dev *dev);
> >   	int (*get_intr_fd)(struct virtio_user_dev *dev);
> > +	int (*map_notification_area)(struct virtio_user_dev *dev, bool
> map);
> >   };
> >
> >   extern struct virtio_user_backend_ops virtio_ops_user; diff --git
> > a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> > b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> > index 2c36b26224..1eb0f9ec48 100644
> > --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> > +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> > @@ -5,6 +5,7 @@
> >   #include <sys/ioctl.h>
> >   #include <sys/types.h>
> >   #include <sys/stat.h>
> > +#include <sys/mman.h>
> >   #include <fcntl.h>
> >   #include <stdlib.h>
> >   #include <unistd.h>
> > @@ -622,6 +623,60 @@ vhost_vdpa_get_intr_fd(struct virtio_user_dev
> *dev __rte_unused)
> >   	return -1;
> >   }
> >
> > +static int
> > +unmap_notification_area(struct virtio_user_dev *dev, int nr_vrings) {
> > +	int i;
> > +
> > +	for (i = 0; i < nr_vrings; i++) {
> > +		if (dev->notify_area[i])
> > +			munmap(dev->notify_area[i], getpagesize());
> > +	}
> > +	free(dev->notify_area);
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +vhost_vdpa_map_notification_area(struct virtio_user_dev *dev, bool
> > +map) {
> > +	struct vhost_vdpa_data *data = dev->backend_data;
> > +	int nr_vrings, i, page_size = getpagesize();
> > +	uint16_t **notify_area;
> > +
> > +	nr_vrings = dev->max_queue_pairs * 2;
> > +	if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
> > +		nr_vrings++;
> > +
> > +	if (!map)
> > +		return unmap_notification_area(dev, nr_vrings);
> > +
> > +	notify_area = malloc(nr_vrings * sizeof(*notify_area));
> > +	if (!notify_area) {
> > +		PMD_DRV_LOG(ERR, "(%s) Failed to allocate notify area
> array", dev->path);
> > +		return -1;
> > +	}
> 
> Add new line here.
> 
> > +	for (i = 0; i < nr_vrings; i++) {
> > +		notify_area[i] = mmap(NULL, page_size, PROT_WRITE,
> MAP_SHARED | MAP_FILE,
> > +				      data->vhostfd, i * page_size);
> > +		if (notify_area[i] == MAP_FAILED) {
> > +			PMD_DRV_LOG(ERR, "(%s) Map failed for notify
> address of queue %d\n",
> > +				    dev->path, i);
> > +			goto map_err;
> > +		}
> > +	}
> > +	dev->notify_area = notify_area;
> > +
> > +	return 0;
> > +
> > +map_err:
> > +	i--;
> 
> I would move this before the goto map_err;
> 
> > +	for (; i >= 0; i--)
> > +		munmap(notify_area[i], page_size);
> > +	free(notify_area);
> 
> New line here.
> 
> > +	return -1;
> > +}
> > +
> >   struct virtio_user_backend_ops virtio_ops_vdpa = {
> >   	.setup = vhost_vdpa_setup,
> >   	.destroy = vhost_vdpa_destroy,
> > @@ -646,4 +701,5 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
> >   	.dma_unmap = vhost_vdpa_dma_unmap_batch,
> >   	.update_link_state = vhost_vdpa_update_link_state,
> >   	.get_intr_fd = vhost_vdpa_get_intr_fd,
> > +	.map_notification_area = vhost_vdpa_map_notification_area,
> 
> I wonder if it wouldn't be cleaner to have one cb for map, one for unmap.
> WDYT?
I will add one more cb for unmap in the next version.

> 
> >   };
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index af1f8c8237..578877d089 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -18,6 +18,7 @@
> >   #include <rte_string_fns.h>
> >   #include <rte_eal_memconfig.h>
> >   #include <rte_malloc.h>
> > +#include <rte_io.h>
> >
> >   #include "vhost.h"
> >   #include "virtio_user_dev.h"
> > @@ -413,6 +414,12 @@ virtio_user_dev_init_notify(struct virtio_user_dev
> *dev)
> >   		dev->callfds[i] = callfd;
> >   		dev->kickfds[i] = kickfd;
> >   	}
> > +	dev->notify_area_mapped = false;
> Can't we just check on dev->notify_area instead of introducing a new field?
> 
> > +	if (dev->ops->map_notification_area) {
> > +		if (dev->ops->map_notification_area(dev, true))
> > +			goto err;
> > +		dev->notify_area_mapped = true;
> > +	}
> >
> >   	return 0;
> >   err:
> > @@ -445,6 +452,11 @@ virtio_user_dev_uninit_notify(struct
> virtio_user_dev *dev)
> >   			dev->callfds[i] = -1;
> >   		}
> >   	}
> > +	if (dev->ops->map_notification_area && dev-
> >notify_area_mapped) {
> > +		/* Unmap notification area */
> > +		dev->ops->map_notification_area(dev, false);
> > +		dev->notify_area_mapped = false;
> > +	}
> >   }
> >
> >   static int
> > @@ -674,12 +686,14 @@ virtio_user_free_vrings(struct virtio_user_dev
> *dev)
> >   	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
> >   	 1ULL << VIRTIO_F_IN_ORDER		|	\
> >   	 1ULL << VIRTIO_F_VERSION_1		|	\
> > -	 1ULL << VIRTIO_F_RING_PACKED)
> > +	 1ULL << VIRTIO_F_RING_PACKED		|	\
> > +	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
> >
> >   int
> >   virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t
> queues,
> >   		     int cq, int queue_size, const char *mac, char **ifname,
> >   		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> > +		     int notification_data,
> >   		     enum virtio_user_backend_type backend_type)
> >   {
> >   	uint64_t backend_features;
> > @@ -737,6 +751,9 @@ virtio_user_dev_init(struct virtio_user_dev *dev,
> char *path, uint16_t queues,
> >   	if (!packed_vq)
> >   		dev->unsupported_features |= (1ull <<
> VIRTIO_F_RING_PACKED);
> >
> > +	if (!notification_data)
> > +		dev->unsupported_features |= (1ull <<
> VIRTIO_F_NOTIFICATION_DATA);
> > +
> >   	if (dev->mac_specified)
> >   		dev->frontend_features |= (1ull << VIRTIO_NET_F_MAC);
> >   	else
> > @@ -1039,11 +1056,35 @@ static void
> >   virtio_user_control_queue_notify(struct virtqueue *vq, void *cookie)
> >   {
> >   	struct virtio_user_dev *dev = cookie;
> > -	uint64_t buf = 1;
> > +	uint64_t notify_data = 1;
> > +
> > +	if (!dev->notify_area_mapped) {
> > +		if (write(dev->kickfds[vq->vq_queue_index], &notify_data,
> sizeof(notify_data)) < 0)
> > +			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> > +				    strerror(errno));
> > +		return;
> > +	} else if (!virtio_with_feature(&dev->hw,
> VIRTIO_F_NOTIFICATION_DATA)) {
> > +		rte_write16(vq->vq_queue_index, vq->notify_addr);
> > +		return;
> > +	}
> >
> > -	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
> > -		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> > -			    strerror(errno));
> > +	if (virtio_with_packed_queue(&dev->hw)) {
> > +		/* Bit[0:15]: vq queue index
> > +		 * Bit[16:30]: avail index
> > +		 * Bit[31]: avail wrap counter
> > +		 */
> > +		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
> > +				VRING_PACKED_DESC_F_AVAIL)) << 31) |
> > +				((uint32_t)vq->vq_avail_idx << 16) |
> > +				vq->vq_queue_index;
> > +	} else {
> > +		/* Bit[0:15]: vq queue index
> > +		 * Bit[16:31]: avail index
> > +		 */
> > +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> > +				vq->vq_queue_index;
> > +	}
> > +	rte_write32(notify_data, vq->notify_addr);
> >   }
> >
> >   int
> > @@ -1062,6 +1103,7 @@ virtio_user_dev_create_shadow_cvq(struct
> > virtio_user_dev *dev, struct virtqueue
> >
> >   	scvq->cq.notify_queue = &virtio_user_control_queue_notify;
> >   	scvq->cq.notify_cookie = dev;
> > +	scvq->notify_addr = vq->notify_addr;
> >   	dev->scvq = scvq;
> >
> >   	return 0;
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > index 7323d88302..29ec386da5 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > @@ -64,6 +64,9 @@ struct virtio_user_dev {
> >   	struct virtqueue	*scvq;
> >
> >   	void *backend_data;
> > +
> > +	bool notify_area_mapped;
> > +	uint16_t **notify_area;
> >   };
> >
> >   int virtio_user_dev_set_features(struct virtio_user_dev *dev); @@
> > -72,7 +75,7 @@ int virtio_user_stop_device(struct virtio_user_dev *dev);
> >   int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t
> 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, int notification_data,
> >   			 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); diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> > b/drivers/net/virtio/virtio_user_ethdev.c
> > index 3a31642899..241465ecdd 100644
> > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > @@ -18,6 +18,7 @@
> >   #include <bus_vdev_driver.h>
> >   #include <rte_alarm.h>
> >   #include <rte_cycles.h>
> > +#include <rte_io.h>
> >
> >   #include "virtio_ethdev.h"
> >   #include "virtio_logs.h"
> > @@ -232,6 +233,7 @@ virtio_user_setup_queue(struct virtio_hw *hw,
> struct virtqueue *vq)
> >   	else
> >   		virtio_user_setup_queue_split(vq, dev);
> >
> > +	vq->notify_addr = dev->notify_area[vq->vq_queue_index];
> >   	if (dev->hw_cvq && hw->cvq && (virtnet_cq_to_vq(hw->cvq) ==
> vq))
> >   		return virtio_user_dev_create_shadow_cvq(dev, vq);
> >
> > @@ -262,8 +264,8 @@ virtio_user_del_queue(struct virtio_hw *hw, struct
> virtqueue *vq)
> >   static void
> >   virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
> >   {
> > -	uint64_t buf = 1;
> >   	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> > +	uint64_t notify_data = 1;
> >
> >   	if (hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq)) {
> >   		virtio_user_handle_cq(dev, vq->vq_queue_index); @@ -
> 271,9 +273,34
> > @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
> >   		return;
> >   	}
> >
> > -	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
> > -		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> > -			    strerror(errno));
> > +	if (!dev->notify_area_mapped) {
> > +		if (write(dev->kickfds[vq->vq_queue_index], &notify_data,
> > +			  sizeof(notify_data)) < 0)
> > +			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
> > +				    strerror(errno));
> > +		return;
> > +	} else if (!virtio_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA))
> {
> > +		rte_write16(vq->vq_queue_index, vq->notify_addr);
> > +		return;
> > +	}
> > +
> > +	if (virtio_with_packed_queue(hw)) {
> > +		/* Bit[0:15]: vq queue index
> > +		 * Bit[16:30]: avail index
> > +		 * Bit[31]: avail wrap counter
> > +		 */
> > +		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
> > +				VRING_PACKED_DESC_F_AVAIL)) << 31) |
> > +				((uint32_t)vq->vq_avail_idx << 16) |
> > +				vq->vq_queue_index;
> > +	} else {
> > +		/* Bit[0:15]: vq queue index
> > +		 * Bit[16:31]: avail index
> > +		 */
> > +		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
> > +				vq->vq_queue_index;
> > +	}
> > +	rte_write32(notify_data, vq->notify_addr);
> >   }
> >
> >   static int
> > @@ -329,6 +356,8 @@ static const char *valid_args[] = {
> >   	VIRTIO_USER_ARG_SPEED,
> >   #define VIRTIO_USER_ARG_VECTORIZED     "vectorized"
> >   	VIRTIO_USER_ARG_VECTORIZED,
> > +#define VIRTIO_USER_ARG_NOTIFICATION_DATA  "notification_data"
> > +	VIRTIO_USER_ARG_NOTIFICATION_DATA,
> >   	NULL
> >   };
> >
> > @@ -480,6 +509,7 @@ virtio_user_pmd_probe(struct rte_vdev_device
> *vdev)
> >   	uint64_t in_order = 1;
> >   	uint64_t packed_vq = 0;
> >   	uint64_t vectorized = 0;
> > +	uint64_t notification_data = 1;
> >   	char *path = NULL;
> >   	char *ifname = NULL;
> >   	char *mac_addr = NULL;
> > @@ -637,6 +667,15 @@ virtio_user_pmd_probe(struct rte_vdev_device
> *vdev)
> >   		}
> >   	}
> >
> > +	if (rte_kvargs_count(kvlist,
> VIRTIO_USER_ARG_NOTIFICATION_DATA) == 1) {
> > +		if (rte_kvargs_process(kvlist,
> VIRTIO_USER_ARG_NOTIFICATION_DATA,
> > +				       &get_integer_arg, &notification_data) <
> 0) {
> > +			PMD_INIT_LOG(ERR, "error to parse %s",
> > +
> VIRTIO_USER_ARG_NOTIFICATION_DATA);
> > +			goto end;
> > +		}
> > +	}
> > +
> >   	eth_dev = virtio_user_eth_dev_alloc(vdev);
> >   	if (!eth_dev) {
> >   		PMD_INIT_LOG(ERR, "virtio_user fails to alloc device"); @@ -
> 645,9
> > +684,10 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
> >
> >   	dev = eth_dev->data->dev_private;
> >   	hw = &dev->hw;
> > -	if (virtio_user_dev_init(dev, path, (uint16_t)queues, cq,
> > -			 queue_size, mac_addr, &ifname, server_mode,
> > -			 mrg_rxbuf, in_order, packed_vq, backend_type) <
> 0) {
> > +	if (virtio_user_dev_init(dev, path, (uint16_t)queues, cq, queue_size,
> > +				 mac_addr, &ifname, server_mode,
> mrg_rxbuf,
> > +				 in_order, packed_vq, notification_data,
> > +				 backend_type) < 0) {
> >   		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
> >   		virtio_user_eth_dev_free(eth_dev);
> >   		goto end;
> > @@ -784,4 +824,5 @@
> RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user,
> >   	"in_order=<0|1> "
> >   	"packed_vq=<0|1> "
> >   	"speed=<int> "
> > -	"vectorized=<0|1>");
> > +	"vectorized=<0|1> "
> > +	"notification_data=<0|1> ");

Thanks for the review and feedback. I will send the next version with suggested changes.

Thanks,
Srujana.

RE: [EXT] Re: [PATCH 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Srujana Challa]

> Hi,
> 
> On 12/8/23 06:31, Srujana Challa wrote:
> > This patch introduces new function to get rss device config and adds
> > code to forward the RSS control command to backend through hw control
> > queue if RSS feature is negotiated.
> > This patch will help to negotiate VIRTIO_NET_F_RSS feature if
> > vhost-vdpa backend supports RSS in HW.
> >
> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > ---
> >   .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
> >   .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
> >   drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
> >   3 files changed, 35 insertions(+), 1 deletion(-)
> >
> 
> Nice! Same question as on patch 1, I would be interested in knowing which
> hardware supports this feature.

Marvell's Octeon DPU supports this feature.
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime

[PATCH v2 1/2] net/virtio-user: improve kick performance with notification area mapping

[Srujana Challa]

This patch introduces new virtio-user callbacks to map the vq
notification area and implements it for the vhost-vDPA backend.
This is simply done by using mmap()/munmap() for the vhost-vDPA fd.

And also adds code to write to queue notify address in notify callback.
This will help in increasing the kick performance.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 drivers/net/virtio/virtio_user/vhost.h        |  2 +
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 68 +++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  | 43 ++++++++++--
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
 drivers/net/virtio/virtio_user_ethdev.c       | 37 ++++++++--
 5 files changed, 143 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index f817cab77a..eee3a4bc47 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -90,6 +90,8 @@ struct virtio_user_backend_ops {
 	int (*server_disconnect)(struct virtio_user_dev *dev);
 	int (*server_reconnect)(struct virtio_user_dev *dev);
 	int (*get_intr_fd)(struct virtio_user_dev *dev);
+	int (*map_notification_area)(struct virtio_user_dev *dev);
+	int (*unmap_notification_area)(struct virtio_user_dev *dev);
 };
 
 extern struct virtio_user_backend_ops virtio_ops_user;
diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index 2c36b26224..3246b74e13 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -5,6 +5,7 @@
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/mman.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -622,6 +623,71 @@ vhost_vdpa_get_intr_fd(struct virtio_user_dev *dev __rte_unused)
 	return -1;
 }
 
+static int
+vhost_vdpa_get_nr_vrings(struct virtio_user_dev *dev)
+{
+	int nr_vrings = dev->max_queue_pairs * 2;
+
+	if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
+		nr_vrings += 1;
+
+	return nr_vrings;
+}
+
+static int
+vhost_vdpa_unmap_notification_area(struct virtio_user_dev *dev)
+{
+	int i, nr_vrings;
+
+	nr_vrings = vhost_vdpa_get_nr_vrings(dev);
+
+	for (i = 0; i < nr_vrings; i++) {
+		if (dev->notify_area[i])
+			munmap(dev->notify_area[i], getpagesize());
+	}
+	free(dev->notify_area);
+	dev->notify_area = NULL;
+
+	return 0;
+}
+
+static int
+vhost_vdpa_map_notification_area(struct virtio_user_dev *dev)
+{
+	struct vhost_vdpa_data *data = dev->backend_data;
+	int nr_vrings, i, page_size = getpagesize();
+	uint16_t **notify_area;
+
+	nr_vrings = vhost_vdpa_get_nr_vrings(dev);
+
+	notify_area = malloc(nr_vrings * sizeof(*notify_area));
+	if (!notify_area) {
+		PMD_DRV_LOG(ERR, "(%s) Failed to allocate notify area array", dev->path);
+		return -1;
+	}
+
+	for (i = 0; i < nr_vrings; i++) {
+		notify_area[i] = mmap(NULL, page_size, PROT_WRITE, MAP_SHARED | MAP_FILE,
+				      data->vhostfd, i * page_size);
+		if (notify_area[i] == MAP_FAILED) {
+			PMD_DRV_LOG(ERR, "(%s) Map failed for notify address of queue %d\n",
+				    dev->path, i);
+			i--;
+			goto map_err;
+		}
+	}
+	dev->notify_area = notify_area;
+
+	return 0;
+
+map_err:
+	for (; i >= 0; i--)
+		munmap(notify_area[i], page_size);
+	free(notify_area);
+
+	return -1;
+}
+
 struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.setup = vhost_vdpa_setup,
 	.destroy = vhost_vdpa_destroy,
@@ -646,4 +712,6 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.dma_unmap = vhost_vdpa_dma_unmap_batch,
 	.update_link_state = vhost_vdpa_update_link_state,
 	.get_intr_fd = vhost_vdpa_get_intr_fd,
+	.map_notification_area = vhost_vdpa_map_notification_area,
+	.unmap_notification_area = vhost_vdpa_unmap_notification_area,
 };
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index af1f8c8237..15b75dd802 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -18,6 +18,7 @@
 #include <rte_string_fns.h>
 #include <rte_eal_memconfig.h>
 #include <rte_malloc.h>
+#include <rte_io.h>
 
 #include "vhost.h"
 #include "virtio_user_dev.h"
@@ -414,6 +415,10 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
 		dev->kickfds[i] = kickfd;
 	}
 
+	if (dev->ops->map_notification_area)
+		if (dev->ops->map_notification_area(dev))
+			goto err;
+
 	return 0;
 err:
 	for (j = 0; j < i; j++) {
@@ -445,6 +450,8 @@ virtio_user_dev_uninit_notify(struct virtio_user_dev *dev)
 			dev->callfds[i] = -1;
 		}
 	}
+	if (dev->ops->unmap_notification_area && dev->notify_area)
+		dev->ops->unmap_notification_area(dev);
 }
 
 static int
@@ -674,7 +681,8 @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
 	 1ULL << VIRTIO_F_IN_ORDER		|	\
 	 1ULL << VIRTIO_F_VERSION_1		|	\
-	 1ULL << VIRTIO_F_RING_PACKED)
+	 1ULL << VIRTIO_F_RING_PACKED		|	\
+	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
@@ -1039,11 +1047,35 @@ static void
 virtio_user_control_queue_notify(struct virtqueue *vq, void *cookie)
 {
 	struct virtio_user_dev *dev = cookie;
-	uint64_t buf = 1;
+	uint64_t notify_data = 1;
+
+	if (!dev->notify_area) {
+		if (write(dev->kickfds[vq->vq_queue_index], &notify_data, sizeof(notify_data)) < 0)
+			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
+				    strerror(errno));
+		return;
+	} else if (!virtio_with_feature(&dev->hw, VIRTIO_F_NOTIFICATION_DATA)) {
+		rte_write16(vq->vq_queue_index, vq->notify_addr);
+		return;
+	}
 
-	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
-		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
-			    strerror(errno));
+	if (virtio_with_packed_queue(&dev->hw)) {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:30]: avail index
+		 * Bit[31]: avail wrap counter
+		 */
+		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
+				VRING_PACKED_DESC_F_AVAIL)) << 31) |
+				((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	} else {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:31]: avail index
+		 */
+		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	}
+	rte_write32(notify_data, vq->notify_addr);
 }
 
 int
@@ -1062,6 +1094,7 @@ virtio_user_dev_create_shadow_cvq(struct virtio_user_dev *dev, struct virtqueue
 
 	scvq->cq.notify_queue = &virtio_user_control_queue_notify;
 	scvq->cq.notify_cookie = dev;
+	scvq->notify_addr = vq->notify_addr;
 	dev->scvq = scvq;
 
 	return 0;
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 7323d88302..671ba50c03 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -64,6 +64,8 @@ struct virtio_user_dev {
 	struct virtqueue	*scvq;
 
 	void *backend_data;
+
+	uint16_t **notify_area;
 };
 
 int virtio_user_dev_set_features(struct virtio_user_dev *dev);
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 3a31642899..aae4c53f32 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -18,6 +18,7 @@
 #include <bus_vdev_driver.h>
 #include <rte_alarm.h>
 #include <rte_cycles.h>
+#include <rte_io.h>
 
 #include "virtio_ethdev.h"
 #include "virtio_logs.h"
@@ -232,6 +233,9 @@ virtio_user_setup_queue(struct virtio_hw *hw, struct virtqueue *vq)
 	else
 		virtio_user_setup_queue_split(vq, dev);
 
+	if (dev->notify_area)
+		vq->notify_addr = dev->notify_area[vq->vq_queue_index];
+
 	if (dev->hw_cvq && hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq))
 		return virtio_user_dev_create_shadow_cvq(dev, vq);
 
@@ -262,8 +266,8 @@ virtio_user_del_queue(struct virtio_hw *hw, struct virtqueue *vq)
 static void
 virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 {
-	uint64_t buf = 1;
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
+	uint64_t notify_data = 1;
 
 	if (hw->cvq && (virtnet_cq_to_vq(hw->cvq) == vq)) {
 		virtio_user_handle_cq(dev, vq->vq_queue_index);
@@ -271,9 +275,34 @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
 		return;
 	}
 
-	if (write(dev->kickfds[vq->vq_queue_index], &buf, sizeof(buf)) < 0)
-		PMD_DRV_LOG(ERR, "failed to kick backend: %s",
-			    strerror(errno));
+	if (!dev->notify_area) {
+		if (write(dev->kickfds[vq->vq_queue_index], &notify_data,
+			  sizeof(notify_data)) < 0)
+			PMD_DRV_LOG(ERR, "failed to kick backend: %s",
+				    strerror(errno));
+		return;
+	} else if (!virtio_with_feature(hw, VIRTIO_F_NOTIFICATION_DATA)) {
+		rte_write16(vq->vq_queue_index, vq->notify_addr);
+		return;
+	}
+
+	if (virtio_with_packed_queue(hw)) {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:30]: avail index
+		 * Bit[31]: avail wrap counter
+		 */
+		notify_data = ((uint32_t)(!!(vq->vq_packed.cached_flags &
+				VRING_PACKED_DESC_F_AVAIL)) << 31) |
+				((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	} else {
+		/* Bit[0:15]: vq queue index
+		 * Bit[16:31]: avail index
+		 */
+		notify_data = ((uint32_t)vq->vq_avail_idx << 16) |
+				vq->vq_queue_index;
+	}
+	rte_write32(notify_data, vq->notify_addr);
 }
 
 static int
-- 
2.25.1

[PATCH v2 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Srujana Challa]

This patch introduces new function to get rss device config
and adds code to forward the RSS control command to backend
through hw control queue if RSS feature is negotiated.
This patch will help to negotiate VIRTIO_NET_F_RSS feature
if vhost-vdpa backend supports RSS in HW.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 15b75dd802..d395fc1676 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -304,6 +304,24 @@ virtio_user_dev_init_max_queue_pairs(struct virtio_user_dev *dev, uint32_t user_
 	return 0;
 }
 
+int
+virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst, size_t offset, int length)
+{
+	int ret = 0;
+
+	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_RSS)))
+		return -ENOTSUP;
+
+	if (!dev->ops->get_config)
+		return -ENOTSUP;
+
+	ret = dev->ops->get_config(dev, dst, offset, length);
+	if (ret)
+		PMD_DRV_LOG(ERR, "(%s) Failed to get rss config in device", dev->path);
+
+	return ret;
+}
+
 int
 virtio_user_dev_set_mac(struct virtio_user_dev *dev)
 {
@@ -682,7 +700,8 @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_F_IN_ORDER		|	\
 	 1ULL << VIRTIO_F_VERSION_1		|	\
 	 1ULL << VIRTIO_F_RING_PACKED		|	\
-	 1ULL << VIRTIO_F_NOTIFICATION_DATA)
+	 1ULL << VIRTIO_F_NOTIFICATION_DATA	|	\
+	 1ULL << VIRTIO_NET_F_RSS)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
@@ -894,6 +913,11 @@ virtio_user_handle_ctrl_msg_split(struct virtio_user_dev *dev, struct vring *vri
 
 		queues = *(uint16_t *)(uintptr_t)vring->desc[idx_data].addr;
 		status = virtio_user_handle_mq(dev, queues);
+	} else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
+		struct virtio_net_ctrl_rss *rss;
+
+		rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring->desc[idx_data].addr;
+		status = virtio_user_handle_mq(dev, rss->max_tx_vq);
 	} else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
 		   hdr->class == VIRTIO_NET_CTRL_MAC ||
 		   hdr->class == VIRTIO_NET_CTRL_VLAN) {
@@ -955,6 +979,11 @@ virtio_user_handle_ctrl_msg_packed(struct virtio_user_dev *dev,
 		queues = *(uint16_t *)(uintptr_t)
 				vring->desc[idx_data].addr;
 		status = virtio_user_handle_mq(dev, queues);
+	} else if (hdr->class == VIRTIO_NET_CTRL_MQ && hdr->cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
+		struct virtio_net_ctrl_rss *rss;
+
+		rss = (struct virtio_net_ctrl_rss *)(uintptr_t)vring->desc[idx_data].addr;
+		status = virtio_user_handle_mq(dev, rss->max_tx_vq);
 	} else if (hdr->class == VIRTIO_NET_CTRL_RX  ||
 		   hdr->class == VIRTIO_NET_CTRL_MAC ||
 		   hdr->class == VIRTIO_NET_CTRL_VLAN) {
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 671ba50c03..66400b3b62 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -85,6 +85,8 @@ int virtio_user_dev_update_status(struct virtio_user_dev *dev);
 int virtio_user_dev_update_link_state(struct virtio_user_dev *dev);
 int virtio_user_dev_set_mac(struct virtio_user_dev *dev);
 int virtio_user_dev_get_mac(struct virtio_user_dev *dev);
+int virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst, size_t offset,
+				   int length);
 void virtio_user_dev_delayed_disconnect_handler(void *param);
 int virtio_user_dev_server_reconnect(struct virtio_user_dev *dev);
 extern const char * const virtio_user_backend_strings[];
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index aae4c53f32..bf9de36d8f 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -52,6 +52,9 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 
 	if (offset == offsetof(struct virtio_net_config, max_virtqueue_pairs))
 		*(uint16_t *)dst = dev->max_queue_pairs;
+
+	if (offset >= offsetof(struct virtio_net_config, rss_max_key_size))
+		virtio_user_dev_get_rss_config(dev, dst, offset, length);
 }
 
 static void
-- 
2.25.1

Re: [PATCH v2 1/2] net/virtio-user: improve kick performance with notification area mapping

[Maxime Coquelin]

On 1/23/24 11:55, Srujana Challa wrote:
> This patch introduces new virtio-user callbacks to map the vq
> notification area and implements it for the vhost-vDPA backend.
> This is simply done by using mmap()/munmap() for the vhost-vDPA fd.
> 
> And also adds code to write to queue notify address in notify callback.
> This will help in increasing the kick performance.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>   drivers/net/virtio/virtio_user/vhost.h        |  2 +
>   drivers/net/virtio/virtio_user/vhost_vdpa.c   | 68 +++++++++++++++++++
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 43 ++++++++++--
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
>   drivers/net/virtio/virtio_user_ethdev.c       | 37 ++++++++--
>   5 files changed, 143 insertions(+), 9 deletions(-)
> 

For next time, please add a changelog either in a cover letter, or in
the patch just before the stats.

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

Thanks,
Maxime

Re: [PATCH v2 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Maxime Coquelin]

On 1/23/24 11:55, Srujana Challa wrote:
> This patch introduces new function to get rss device config
> and adds code to forward the RSS control command to backend
> through hw control queue if RSS feature is negotiated.
> This patch will help to negotiate VIRTIO_NET_F_RSS feature
> if vhost-vdpa backend supports RSS in HW.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
>   drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
>   3 files changed, 35 insertions(+), 1 deletion(-)
> 

Reporting the R-by I gave on V1. Please next time apply R-By and A-by
from previous revision when the patch did not change.

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

Thanks,
Maxime

Re: [PATCH v2 1/2] net/virtio-user: improve kick performance with notification area mapping

[Maxime Coquelin]

On 1/23/24 11:55, Srujana Challa wrote:
> This patch introduces new virtio-user callbacks to map the vq
> notification area and implements it for the vhost-vDPA backend.
> This is simply done by using mmap()/munmap() for the vhost-vDPA fd.
> 
> And also adds code to write to queue notify address in notify callback.
> This will help in increasing the kick performance.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>   drivers/net/virtio/virtio_user/vhost.h        |  2 +
>   drivers/net/virtio/virtio_user/vhost_vdpa.c   | 68 +++++++++++++++++++
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 43 ++++++++++--
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
>   drivers/net/virtio/virtio_user_ethdev.c       | 37 ++++++++--
>   5 files changed, 143 insertions(+), 9 deletions(-)
> 

Applied to next-virtio tree.

Thanks,
Maxime

Re: [PATCH v2 2/2] net/virtio-user: add VIRTIO_NET_F_RSS to supported features

[Maxime Coquelin]

On 1/23/24 11:55, Srujana Challa wrote:
> This patch introduces new function to get rss device config
> and adds code to forward the RSS control command to backend
> through hw control queue if RSS feature is negotiated.
> This patch will help to negotiate VIRTIO_NET_F_RSS feature
> if vhost-vdpa backend supports RSS in HW.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>   .../net/virtio/virtio_user/virtio_user_dev.c  | 31 ++++++++++++++++++-
>   .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
>   drivers/net/virtio/virtio_user_ethdev.c       |  3 ++
>   3 files changed, 35 insertions(+), 1 deletion(-)
> 

Applied to next-virtio tree.

Thanks,
Maxime