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

Message ID 20240123105523.751151-1-schalla@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series [v2,1/2] net/virtio-user: improve kick performance with notification area mapping |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Srujana Challa Jan. 23, 2024, 10:55 a.m. UTC
  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(-)
  

Comments

Maxime Coquelin Feb. 5, 2024, 10:30 a.m. UTC | #1
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
  
Maxime Coquelin Feb. 6, 2024, 2:57 p.m. UTC | #2
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
  

Patch

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