[PATCH 1/5] interrupts: fix number of bytes read for vdev

[PATCH 1/5] interrupts: fix number of bytes read for vdev

[skoteshwar]

From: Satha Rao <skoteshwar@marvell.com>

Correct the number of bytes to read during vdev interrupt processing.
The driver sets this value, and the rxtx interrupt handling already
performs this correctly.

Fixes: 99998feec90 ("eal/linux: add interrupt type for vdev")
Cc: stable@dpdk.org

Signed-off-by: Satha Rao <skoteshwar@marvell.com>
---
 lib/eal/linux/eal_interrupts.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 6436f796eb..80eef7ec4b 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -949,6 +949,10 @@ eal_intr_process_interrupts(struct epoll_event *events, int nfds)
 			break;
 #endif
 		case RTE_INTR_HANDLE_VDEV:
+			/* For vdev, number of bytes to read is set by driver */
+			bytes_read = rte_intr_efd_counter_size_get(src->intr_handle);
+			call = true;
+			break;
 		case RTE_INTR_HANDLE_EXT:
 			bytes_read = 0;
 			call = true;
-- 
2.25.1

[PATCH 2/5] net/virtio-user: get link duplex and speed

[skoteshwar]

From: Satha Rao <skoteshwar@marvell.com>

This patch extends the virtio_user_read_dev_config API to retrieve link
speed and duplex settings if the device features support
VIRTIO_NET_F_SPEED_DUPLEX.

Signed-off-by: Satha Rao <skoteshwar@marvell.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 22 ++++++++++++++++++-
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
 drivers/net/virtio/virtio_user_ethdev.c       |  6 +++++
 3 files changed, 29 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 2997d2bd26..91ad1312f7 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -468,6 +468,25 @@ virtio_user_dev_init_mac(struct virtio_user_dev *dev, const char *mac)
 	PMD_DRV_LOG(INFO, "(%s) MAC %s specified", dev->path, buf);
 }
 
+int
+virtio_user_dev_get_speed_duplex_config(struct virtio_user_dev *dev, void *dst, size_t offset,
+					int length)
+{
+	int ret = 0;
+
+	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_SPEED_DUPLEX)))
+		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 speed/duplex config in device", dev->path);
+
+	return ret;
+}
+
 static int
 virtio_user_dev_init_notify(struct virtio_user_dev *dev)
 {
@@ -726,7 +745,8 @@ virtio_user_free_vrings(struct virtio_user_dev *dev)
 	 1ULL << VIRTIO_F_RING_PACKED		|	\
 	 1ULL << VIRTIO_F_NOTIFICATION_DATA	|	\
 	 1ULL << VIRTIO_F_ORDER_PLATFORM        |       \
-	 1ULL << VIRTIO_NET_F_RSS)
+	 1ULL << VIRTIO_NET_F_RSS               |       \
+	 1ULL << VIRTIO_NET_F_SPEED_DUPLEX)
 
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 66400b3b62..57d75d1c53 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -87,6 +87,8 @@ 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);
+int virtio_user_dev_get_speed_duplex_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 bf29f0dacd..d431b4521b 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -55,6 +55,12 @@ virtio_user_read_dev_config(struct virtio_hw *hw, size_t offset,
 
 	if (offset >= offsetof(struct virtio_net_config, rss_max_key_size))
 		virtio_user_dev_get_rss_config(dev, dst, offset, length);
+
+	if (offset == offsetof(struct virtio_net_config, speed))
+		virtio_user_dev_get_speed_duplex_config(dev, dst, offset, length);
+
+	if (offset == offsetof(struct virtio_net_config, duplex))
+		virtio_user_dev_get_speed_duplex_config(dev, dst, offset, length);
 }
 
 static void
-- 
2.25.1

[PATCH 3/5] net/virtio: add set config callback

[skoteshwar]

From: Satha Rao <skoteshwar@marvell.com>

This patch enables the set config callback handler.

Signed-off-by: Satha Rao <skoteshwar@marvell.com>
---
 drivers/net/virtio/virtio_user/vhost.h      |  1 +
 drivers/net/virtio/virtio_user/vhost_vdpa.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index eee3a4bc47..45ae56e3b5 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -92,6 +92,7 @@ struct virtio_user_backend_ops {
 	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);
+	int (*set_config_call)(struct virtio_user_dev *dev, int fd);
 };
 
 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 bc3e2a9af5..5c610664b6 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -48,6 +48,7 @@ struct vhost_vdpa_data {
 #define VHOST_VDPA_SET_VRING_ENABLE _IOW(VHOST_VIRTIO, 0x75, struct vhost_vring_state)
 #define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
 #define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64)
+#define VHOST_SET_CONFIG_CALL      _IOW(VHOST_VIRTIO, 0x77, int)
 
 /* no alignment requirement */
 struct vhost_iotlb_msg {
@@ -509,6 +510,14 @@ vhost_vdpa_set_config(struct virtio_user_dev *dev, const uint8_t *data, uint32_t
 	return ret;
 }
 
+static int
+vhost_vdpa_set_config_call(struct virtio_user_dev *dev, int fd)
+{
+	struct vhost_vdpa_data *data = dev->backend_data;
+
+	return vhost_vdpa_ioctl(data->vhostfd, VHOST_SET_CONFIG_CALL, &fd);
+}
+
 /**
  * Set up environment to talk with a vhost vdpa backend.
  *
@@ -706,6 +715,7 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.set_status = vhost_vdpa_set_status,
 	.get_config = vhost_vdpa_get_config,
 	.set_config = vhost_vdpa_set_config,
+	.set_config_call = vhost_vdpa_set_config_call,
 	.cvq_enable = vhost_vdpa_cvq_enable,
 	.enable_qp = vhost_vdpa_enable_queue_pair,
 	.dma_map = vhost_vdpa_dma_map_batch,
-- 
2.25.1

[PATCH 4/5] net/virtio: implement update link state API

[skoteshwar]

From: Satha Rao <skoteshwar@marvell.com>

This patch introduces the vhost_vdpa_update_link_state API to manage the
link state updates.

Signed-off-by: Satha Rao <skoteshwar@marvell.com>
---
 drivers/net/virtio/virtio_user/vhost_vdpa.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index 5c610664b6..e8aea54000 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -619,9 +619,17 @@ vhost_vdpa_get_backend_features(uint64_t *features)
 }
 
 static int
-vhost_vdpa_update_link_state(struct virtio_user_dev *dev __rte_unused)
+vhost_vdpa_update_link_state(struct virtio_user_dev *dev)
 {
-	/* Nothing to update (for now?) */
+	uint16_t status;
+	int rc, offset;
+
+	offset = offsetof(struct virtio_net_config, status);
+	rc = vhost_vdpa_get_config(dev, (uint8_t *)&status, offset, sizeof(status));
+	if (rc)
+		return rc;
+	dev->net_status = status & VIRTIO_NET_S_LINK_UP;
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH 5/5] net/virtio-user: support vDPA configuration callback

[skoteshwar]

From: Satha Rao <skoteshwar@marvell.com>

Extended the vhost_vdpa_get_intr_fd API to create an event and
register for configuration callbacks with the vDPA backend. This
enhancement allows the virtio-user driver to handle configuration
changes more effectively.

Signed-off-by: Satha Rao <skoteshwar@marvell.com>
---
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 18 ++++++++++--
 .../net/virtio/virtio_user/virtio_user_dev.c  | 29 +++++++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 ++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index e8aea54000..c9eaffce16 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -9,6 +9,7 @@
 #include <fcntl.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <sys/eventfd.h>
 
 #include <rte_memory.h>
 
@@ -634,10 +635,21 @@ vhost_vdpa_update_link_state(struct virtio_user_dev *dev)
 }
 
 static int
-vhost_vdpa_get_intr_fd(struct virtio_user_dev *dev __rte_unused)
+vhost_vdpa_get_intr_fd(struct virtio_user_dev *dev)
 {
-	/* No link state interrupt with Vhost-vDPA */
-	return -1;
+	int fd;
+
+	if (dev->cfg_epfd)
+		return dev->cfg_epfd;
+
+	fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+	if (fd < 0) {
+		PMD_DRV_LOG(ERR, "failed to create fd error, %s", strerror(errno));
+		fd = -1;
+	}
+	dev->cfg_epfd = fd;
+
+	return fd;
 }
 
 static int
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 91ad1312f7..ea00eb50f5 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -487,6 +487,21 @@ virtio_user_dev_get_speed_duplex_config(struct virtio_user_dev *dev, void *dst,
 	return ret;
 }
 
+int
+virtio_user_dev_set_config_call(struct virtio_user_dev *dev, int fd)
+{
+	int ret = 0;
+
+	if (!dev->ops->set_config_call)
+		return -ENOTSUP;
+
+	ret = dev->ops->set_config_call(dev, fd);
+	if (ret)
+		PMD_DRV_LOG(ERR, "(%s) Failed to set config call in device", dev->path);
+
+	return ret;
+}
+
 static int
 virtio_user_dev_init_notify(struct virtio_user_dev *dev)
 {
@@ -769,6 +784,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
 	dev->unsupported_features = 0;
 	dev->backend_type = backend_type;
 	dev->ifname = *ifname;
+	dev->cfg_epfd = 0;
 
 	if (virtio_user_dev_setup(dev) < 0) {
 		PMD_INIT_LOG(ERR, "(%s) backend set up fails", dev->path);
@@ -863,6 +879,15 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
 		}
 	}
 
+	/* vhost vdpa register valid fd to handle config callback */
+	if (dev->cfg_epfd) {
+		struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->hw.port_id];
+
+		if (rte_intr_efd_counter_size_set(eth_dev->intr_handle, 8))
+			return -rte_errno;
+		virtio_user_dev_set_config_call(dev, dev->cfg_epfd);
+	}
+
 	*ifname = NULL;
 	return 0;
 
@@ -892,6 +917,10 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev)
 
 	virtio_user_free_vrings(dev);
 
+	if (dev->cfg_epfd) {
+		close(dev->cfg_epfd);
+		dev->cfg_epfd = 0;
+	}
 	free(dev->ifname);
 
 	if (dev->is_server)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 57d75d1c53..1ad93521e3 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -31,6 +31,7 @@ struct virtio_user_dev {
 
 	int		*callfds;
 	int		*kickfds;
+	int		cfg_epfd; /* config callback interrupt */
 	int		mac_specified;
 	uint16_t	max_queue_pairs;
 	uint16_t	queue_pairs;
@@ -89,6 +90,7 @@ int virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst, size_
 				   int length);
 int virtio_user_dev_get_speed_duplex_config(struct virtio_user_dev *dev, void *dst,
 					    size_t offset, int length);
+int virtio_user_dev_set_config_call(struct virtio_user_dev *dev, int fd);
 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[];
-- 
2.25.1

Re: [PATCH 1/5] interrupts: fix number of bytes read for vdev

[Stephen Hemminger]

On Mon, 7 Oct 2024 20:33:59 +0530
<skoteshwar@marvell.com> wrote:

> From: Satha Rao <skoteshwar@marvell.com>
> 
> Correct the number of bytes to read during vdev interrupt processing.
> The driver sets this value, and the rxtx interrupt handling already
> performs this correctly.
> 
> Fixes: 99998feec90 ("eal/linux: add interrupt type for vdev")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Satha Rao <skoteshwar@marvell.com>

Not sure how this works.
The code in eal_handle_interrupts sits on a epoll_wait() waiting
for next interrupt.

eal_inter_process_interrupts() is called.
The switch you modified decides what needs to be read to clear the interrupt.
If you change RTE_INTR_HANDLE_VDEV to read bytes from the eventfd 
by setting bytes_read then the code in this function will read the counter
value from the event fd immediately.

Then when eal_intr_proc_rxtx_intr() is called it will decide to read again.
And since the event is cleared, this read will get EWOULDBLOCK.

What exactly is the issue? How to reproduce it?

Also setting call = true in the switch is wrong.
The code later in eal_intr_process_interrupts that handles bytes_read > 0
already sets call = true if interrupt was cleared from eventfd.