DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support
@ 2021-06-17 14:17 Maxime Coquelin
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 1/3] net/virtio: keep device and frontend features separated Maxime Coquelin
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Maxime Coquelin @ 2021-06-17 14:17 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand; +Cc: Maxime Coquelin

This patch adds vDPA device config space requests support.
For now, it only adds MAC address get and set. It may be
extended in next revision to support other configs like
link state.

Regarding the MAC selection strategy, if devargs MAC address
is set by the user and valid, the driver tries to store it
in the device config space, then it reads the MAC address
back from the device config, which will be used. If not set
in devargs or invalid, it tries to read it from the device.
If it fails, a random MAC will be used.

I'm interrested to know your feedback on this strategy.

It has been tested with vDPA simulator, which only supports
getting the MAC address, and witch CX6 which supports neither
getting or setting MAC address (and so devarg or random MAC is
used). IFCVF driver seems to support both getting and setting
the MAC, I have a try with it before next revision.

Changes since v2:
-----------------
- Remove unnecessary "\n" in error logging (Chenbo).

Changes since RFC:
------------------
- Rebase
- Improve logging by printing used MAC address when specified
  by the user or the device (Adrian, Chenbo)


Maxime Coquelin (3):
  net/virtio: keep device and frontend features separated
  net/virtio: add device config support to vDPA
  net/virtio: add MAC device config getter and setter

 drivers/net/virtio/virtio_user/vhost.h        |  3 +
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 69 ++++++++++++++
 .../net/virtio/virtio_user/virtio_user_dev.c  | 95 +++++++++++++++----
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
 drivers/net/virtio/virtio_user_ethdev.c       | 12 ++-
 5 files changed, 158 insertions(+), 23 deletions(-)

-- 
2.31.1


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

* [dpdk-dev] [PATCH v2 1/3] net/virtio: keep device and frontend features separated
  2021-06-17 14:17 [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Maxime Coquelin
@ 2021-06-17 14:17 ` Maxime Coquelin
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 2/3] net/virtio: add device config support to vDPA Maxime Coquelin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Maxime Coquelin @ 2021-06-17 14:17 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand; +Cc: Maxime Coquelin

This patch is preliminary rework to add support for getting
and setting device's config space.

In order to get or set a device config such as its MAC address,
we need to know whether the device itself support the feature,
or if it is emulated by the frontend.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
---
 drivers/net/virtio/virtio_user/virtio_user_dev.c | 10 ++--------
 drivers/net/virtio/virtio_user_ethdev.c          |  5 +++--
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 364f43e21c..ed55cd7524 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -573,11 +573,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
 
-	/*
-	 * Device features =
-	 *     (frontend_features | backend_features) & ~unsupported_features;
-	 */
-	dev->device_features |= dev->frontend_features;
+	dev->frontend_features &= ~dev->unsupported_features;
 	dev->device_features &= ~dev->unsupported_features;
 
 	if (rte_mem_event_callback_register(VIRTIO_USER_MEM_EVENT_CLB_NAME,
@@ -980,12 +976,10 @@ virtio_user_dev_server_reconnect(struct virtio_user_dev *dev)
 		return -1;
 	}
 
-	dev->device_features |= dev->frontend_features;
-
 	/* unmask vhost-user unsupported features */
 	dev->device_features &= ~(dev->unsupported_features);
 
-	dev->features &= dev->device_features;
+	dev->features &= (dev->device_features | dev->frontend_features);
 
 	/* For packed ring, resetting queues is required in reconnection. */
 	if (virtio_with_packed_queue(hw) &&
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index e85906e9eb..3ecbb4184a 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -110,7 +110,8 @@ virtio_user_get_features(struct virtio_hw *hw)
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	/* unmask feature bits defined in vhost user protocol */
-	return dev->device_features & VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
+	return (dev->device_features | dev->frontend_features) &
+		VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
 }
 
 static void
@@ -118,7 +119,7 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t features)
 {
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
-	dev->features = features & dev->device_features;
+	dev->features = features & (dev->device_features | dev->frontend_features);
 }
 
 static int
-- 
2.31.1


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

* [dpdk-dev] [PATCH v2 2/3] net/virtio: add device config support to vDPA
  2021-06-17 14:17 [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Maxime Coquelin
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 1/3] net/virtio: keep device and frontend features separated Maxime Coquelin
@ 2021-06-17 14:17 ` Maxime Coquelin
  2021-06-23  7:53   ` Xia, Chenbo
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 3/3] net/virtio: add MAC device config getter and setter Maxime Coquelin
  2021-06-23  8:08 ` [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Xia, Chenbo
  3 siblings, 1 reply; 7+ messages in thread
From: Maxime Coquelin @ 2021-06-17 14:17 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand; +Cc: Maxime Coquelin

This patch introduces two virtio-user callbacks to get
and set device's config, and implements it for vDPA
backends.

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

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index c49e88036d..dfbf6be033 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -79,6 +79,9 @@ struct virtio_user_backend_ops {
 	int (*set_vring_addr)(struct virtio_user_dev *dev, struct vhost_vring_addr *addr);
 	int (*get_status)(struct virtio_user_dev *dev, uint8_t *status);
 	int (*set_status)(struct virtio_user_dev *dev, uint8_t status);
+	int (*get_config)(struct virtio_user_dev *dev, uint8_t *data, uint32_t off, uint32_t len);
+	int (*set_config)(struct virtio_user_dev *dev, const uint8_t *data, uint32_t off,
+			uint32_t len);
 	int (*enable_qp)(struct virtio_user_dev *dev, uint16_t pair_idx, int enable);
 	int (*dma_map)(struct virtio_user_dev *dev, void *addr, uint64_t iova, size_t len);
 	int (*dma_unmap)(struct virtio_user_dev *dev, void *addr, uint64_t iova, size_t len);
diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index e2d6d3504d..3d65f0079a 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -41,6 +41,8 @@ struct vhost_vdpa_data {
 #define VHOST_VDPA_GET_DEVICE_ID _IOR(VHOST_VIRTIO, 0x70, __u32)
 #define VHOST_VDPA_GET_STATUS _IOR(VHOST_VIRTIO, 0x71, __u8)
 #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8)
+#define VHOST_VDPA_GET_CONFIG _IOR(VHOST_VIRTIO, 0x73, struct vhost_vdpa_config)
+#define VHOST_VDPA_SET_CONFIG _IOW(VHOST_VIRTIO, 0x74, struct vhost_vdpa_config)
 #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)
@@ -65,6 +67,12 @@ struct vhost_iotlb_msg {
 
 #define VHOST_IOTLB_MSG_V2 0x2
 
+struct vhost_vdpa_config {
+	uint32_t off;
+	uint32_t len;
+	uint8_t buf[0];
+};
+
 struct vhost_msg {
 	uint32_t type;
 	uint32_t reserved;
@@ -440,6 +448,65 @@ vhost_vdpa_set_status(struct virtio_user_dev *dev, uint8_t status)
 	return vhost_vdpa_ioctl(data->vhostfd, VHOST_VDPA_SET_STATUS, &status);
 }
 
+static int
+vhost_vdpa_get_config(struct virtio_user_dev *dev, uint8_t *data, uint32_t off, uint32_t len)
+{
+	struct vhost_vdpa_data *vdpa_data = dev->backend_data;
+	struct vhost_vdpa_config *config;
+	int ret = 0;
+
+	config = malloc(sizeof(*config) + len);
+	if (!config) {
+		PMD_DRV_LOG(ERR, "Failed to allocate vDPA config data");
+		return -1;
+	}
+
+	config->off = off;
+	config->len = len;
+
+	ret = vhost_vdpa_ioctl(vdpa_data->vhostfd, VHOST_VDPA_GET_CONFIG, config);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Failed to get vDPA config (offset 0x%x, len 0x%x)", off, len);
+		ret = -1;
+		goto out;
+	}
+
+	memcpy(data, config->buf, len);
+out:
+	free(config);
+
+	return ret;
+}
+
+static int
+vhost_vdpa_set_config(struct virtio_user_dev *dev, const uint8_t *data, uint32_t off, uint32_t len)
+{
+	struct vhost_vdpa_data *vdpa_data = dev->backend_data;
+	struct vhost_vdpa_config *config;
+	int ret = 0;
+
+	config = malloc(sizeof(*config) + len);
+	if (!config) {
+		PMD_DRV_LOG(ERR, "Failed to allocate vDPA config data");
+		return -1;
+	}
+
+	config->off = off;
+	config->len = len;
+
+	memcpy(config->buf, data, len);
+
+	ret = vhost_vdpa_ioctl(vdpa_data->vhostfd, VHOST_VDPA_SET_CONFIG, config);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Failed to set vDPA config (offset 0x%x, len 0x%x)", off, len);
+		ret = -1;
+	}
+
+	free(config);
+
+	return ret;
+}
+
 /**
  * Set up environment to talk with a vhost vdpa backend.
  *
@@ -559,6 +626,8 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.set_vring_addr = vhost_vdpa_set_vring_addr,
 	.get_status = vhost_vdpa_get_status,
 	.set_status = vhost_vdpa_set_status,
+	.get_config = vhost_vdpa_get_config,
+	.set_config = vhost_vdpa_set_config,
 	.enable_qp = vhost_vdpa_enable_queue_pair,
 	.dma_map = vhost_vdpa_dma_map_batch,
 	.dma_unmap = vhost_vdpa_dma_unmap_batch,
-- 
2.31.1


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

* [dpdk-dev] [PATCH v2 3/3] net/virtio: add MAC device config getter and setter
  2021-06-17 14:17 [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Maxime Coquelin
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 1/3] net/virtio: keep device and frontend features separated Maxime Coquelin
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 2/3] net/virtio: add device config support to vDPA Maxime Coquelin
@ 2021-06-17 14:17 ` Maxime Coquelin
  2021-06-23  7:54   ` Xia, Chenbo
  2021-06-23  8:08 ` [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Xia, Chenbo
  3 siblings, 1 reply; 7+ messages in thread
From: Maxime Coquelin @ 2021-06-17 14:17 UTC (permalink / raw)
  To: dev, chenbo.xia, amorenoz, david.marchand; +Cc: Maxime Coquelin

This patch uses the new device config ops to get and set
the MAC address if supported.

If a valid MAC address is passed as devarg of the
Virtio-user PMD, the driver will try to store it in the
device config space. Otherwise the one provided in
the device config space will be used, if available.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 85 ++++++++++++++++---
 .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
 drivers/net/virtio/virtio_user_ethdev.c       |  7 +-
 3 files changed, 81 insertions(+), 13 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index ed55cd7524..1cd1e95f45 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -260,21 +260,84 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
 	return -1;
 }
 
-static inline void
-parse_mac(struct virtio_user_dev *dev, const char *mac)
+int
+virtio_user_dev_set_mac(struct virtio_user_dev *dev)
 {
-	struct rte_ether_addr tmp;
+	int ret = 0;
 
-	if (!mac)
-		return;
+	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_MAC)))
+		return -ENOTSUP;
+
+	if (!dev->ops->set_config)
+		return -ENOTSUP;
+
+	ret = dev->ops->set_config(dev, dev->mac_addr,
+			offsetof(struct virtio_net_config, mac),
+			RTE_ETHER_ADDR_LEN);
+	if (ret)
+		PMD_DRV_LOG(ERR, "(%s) Failed to set MAC address in device", dev->path);
+
+	return ret;
+}
+
+int
+virtio_user_dev_get_mac(struct virtio_user_dev *dev)
+{
+	int ret = 0;
+
+	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_MAC)))
+		return -ENOTSUP;
+
+	if (!dev->ops->get_config)
+		return -ENOTSUP;
+
+	ret = dev->ops->get_config(dev, dev->mac_addr,
+			offsetof(struct virtio_net_config, mac),
+			RTE_ETHER_ADDR_LEN);
+	if (ret)
+		PMD_DRV_LOG(ERR, "(%s) Failed to get MAC address from device", dev->path);
+
+	return ret;
+}
+
+static void
+virtio_user_dev_init_mac(struct virtio_user_dev *dev, const char *mac)
+{
+	struct rte_ether_addr cmdline_mac;
+	char buf[RTE_ETHER_ADDR_FMT_SIZE];
+	int ret;
 
-	if (rte_ether_unformat_addr(mac, &tmp) == 0) {
-		memcpy(dev->mac_addr, &tmp, RTE_ETHER_ADDR_LEN);
+	if (mac && rte_ether_unformat_addr(mac, &cmdline_mac) == 0) {
+		/*
+		 * MAC address was passed from command-line, try to store
+		 * it in the device if it supports it. Otherwise try to use
+		 * the device one.
+		 */
+		memcpy(dev->mac_addr, &cmdline_mac, RTE_ETHER_ADDR_LEN);
 		dev->mac_specified = 1;
+
+		/* Setting MAC may fail, continue to get the device one in this case */
+		virtio_user_dev_set_mac(dev);
+		ret = virtio_user_dev_get_mac(dev);
+		if (ret == -ENOTSUP)
+			goto out;
+
+		if (memcmp(&cmdline_mac, dev->mac_addr, RTE_ETHER_ADDR_LEN))
+			PMD_DRV_LOG(INFO, "(%s) Device MAC update failed", dev->path);
 	} else {
-		/* ignore the wrong mac, use random mac */
-		PMD_DRV_LOG(ERR, "wrong format of mac: %s", mac);
+		ret = virtio_user_dev_get_mac(dev);
+		if (ret) {
+			PMD_DRV_LOG(ERR, "(%s) No valid MAC in devargs or device, use random",
+					dev->path);
+			return;
+		}
+
+		dev->mac_specified = 1;
 	}
+out:
+	rte_ether_format_addr(buf, RTE_ETHER_ADDR_FMT_SIZE,
+			(struct rte_ether_addr *)dev->mac_addr);
+	PMD_DRV_LOG(INFO, "(%s) MAC %s specified", dev->path, buf);
 }
 
 static int
@@ -509,8 +572,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	dev->unsupported_features = 0;
 	dev->backend_type = backend_type;
 
-	parse_mac(dev, mac);
-
 	if (*ifname) {
 		dev->ifname = *ifname;
 		*ifname = NULL;
@@ -538,6 +599,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
+	virtio_user_dev_init_mac(dev, mac);
+
 	if (!mrg_rxbuf)
 		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MRG_RXBUF);
 
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 58ad5198b6..819f6463ba 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -77,6 +77,8 @@ uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
 int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status);
 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);
 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 3ecbb4184a..90fcd6e7cc 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -60,12 +60,15 @@ virtio_user_write_dev_config(struct virtio_hw *hw, size_t offset,
 	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
 	if ((offset == offsetof(struct virtio_net_config, mac)) &&
-	    (length == RTE_ETHER_ADDR_LEN))
+	    (length == RTE_ETHER_ADDR_LEN)) {
 		for (i = 0; i < RTE_ETHER_ADDR_LEN; ++i)
 			dev->mac_addr[i] = ((const uint8_t *)src)[i];
-	else
+		virtio_user_dev_set_mac(dev);
+		virtio_user_dev_get_mac(dev);
+	} else {
 		PMD_DRV_LOG(ERR, "not supported offset=%zu, len=%d",
 			    offset, length);
+	}
 }
 
 static void
-- 
2.31.1


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

* Re: [dpdk-dev] [PATCH v2 2/3] net/virtio: add device config support to vDPA
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 2/3] net/virtio: add device config support to vDPA Maxime Coquelin
@ 2021-06-23  7:53   ` Xia, Chenbo
  0 siblings, 0 replies; 7+ messages in thread
From: Xia, Chenbo @ 2021-06-23  7:53 UTC (permalink / raw)
  To: Maxime Coquelin, dev, amorenoz, david.marchand

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, June 17, 2021 10:17 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 2/3] net/virtio: add device config support to vDPA
> 
> This patch introduces two virtio-user callbacks to get
> and set device's config, and implements it for vDPA
> backends.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost.h      |  3 +
>  drivers/net/virtio/virtio_user/vhost_vdpa.c | 69 +++++++++++++++++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index c49e88036d..dfbf6be033 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -79,6 +79,9 @@ struct virtio_user_backend_ops {
>  	int (*set_vring_addr)(struct virtio_user_dev *dev, struct
> vhost_vring_addr *addr);
>  	int (*get_status)(struct virtio_user_dev *dev, uint8_t *status);
>  	int (*set_status)(struct virtio_user_dev *dev, uint8_t status);
> +	int (*get_config)(struct virtio_user_dev *dev, uint8_t *data,
> uint32_t off, uint32_t len);
> +	int (*set_config)(struct virtio_user_dev *dev, const uint8_t *data,
> uint32_t off,
> +			uint32_t len);
>  	int (*enable_qp)(struct virtio_user_dev *dev, uint16_t pair_idx, int
> enable);
>  	int (*dma_map)(struct virtio_user_dev *dev, void *addr, uint64_t
> iova, size_t len);
>  	int (*dma_unmap)(struct virtio_user_dev *dev, void *addr, uint64_t
> iova, size_t len);
> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> index e2d6d3504d..3d65f0079a 100644
> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> @@ -41,6 +41,8 @@ struct vhost_vdpa_data {
>  #define VHOST_VDPA_GET_DEVICE_ID _IOR(VHOST_VIRTIO, 0x70, __u32)
>  #define VHOST_VDPA_GET_STATUS _IOR(VHOST_VIRTIO, 0x71, __u8)
>  #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8)
> +#define VHOST_VDPA_GET_CONFIG _IOR(VHOST_VIRTIO, 0x73, struct
> vhost_vdpa_config)
> +#define VHOST_VDPA_SET_CONFIG _IOW(VHOST_VIRTIO, 0x74, struct
> vhost_vdpa_config)
>  #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)
> @@ -65,6 +67,12 @@ struct vhost_iotlb_msg {
> 
>  #define VHOST_IOTLB_MSG_V2 0x2
> 
> +struct vhost_vdpa_config {
> +	uint32_t off;
> +	uint32_t len;
> +	uint8_t buf[0];
> +};
> +
>  struct vhost_msg {
>  	uint32_t type;
>  	uint32_t reserved;
> @@ -440,6 +448,65 @@ vhost_vdpa_set_status(struct virtio_user_dev *dev,
> uint8_t status)
>  	return vhost_vdpa_ioctl(data->vhostfd, VHOST_VDPA_SET_STATUS,
> &status);
>  }
> 
> +static int
> +vhost_vdpa_get_config(struct virtio_user_dev *dev, uint8_t *data,
> uint32_t off, uint32_t len)
> +{
> +	struct vhost_vdpa_data *vdpa_data = dev->backend_data;
> +	struct vhost_vdpa_config *config;
> +	int ret = 0;
> +
> +	config = malloc(sizeof(*config) + len);
> +	if (!config) {
> +		PMD_DRV_LOG(ERR, "Failed to allocate vDPA config data");
> +		return -1;
> +	}
> +
> +	config->off = off;
> +	config->len = len;
> +
> +	ret = vhost_vdpa_ioctl(vdpa_data->vhostfd, VHOST_VDPA_GET_CONFIG,
> config);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "Failed to get vDPA config (offset 0x%x, len
> 0x%x)", off, len);
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	memcpy(data, config->buf, len);
> +out:
> +	free(config);
> +
> +	return ret;
> +}
> +
> +static int
> +vhost_vdpa_set_config(struct virtio_user_dev *dev, const uint8_t *data,
> uint32_t off, uint32_t len)
> +{
> +	struct vhost_vdpa_data *vdpa_data = dev->backend_data;
> +	struct vhost_vdpa_config *config;
> +	int ret = 0;
> +
> +	config = malloc(sizeof(*config) + len);
> +	if (!config) {
> +		PMD_DRV_LOG(ERR, "Failed to allocate vDPA config data");
> +		return -1;
> +	}
> +
> +	config->off = off;
> +	config->len = len;
> +
> +	memcpy(config->buf, data, len);
> +
> +	ret = vhost_vdpa_ioctl(vdpa_data->vhostfd, VHOST_VDPA_SET_CONFIG,
> config);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "Failed to set vDPA config (offset 0x%x, len
> 0x%x)", off, len);
> +		ret = -1;
> +	}
> +
> +	free(config);
> +
> +	return ret;
> +}
> +
>  /**
>   * Set up environment to talk with a vhost vdpa backend.
>   *
> @@ -559,6 +626,8 @@ struct virtio_user_backend_ops virtio_ops_vdpa = {
>  	.set_vring_addr = vhost_vdpa_set_vring_addr,
>  	.get_status = vhost_vdpa_get_status,
>  	.set_status = vhost_vdpa_set_status,
> +	.get_config = vhost_vdpa_get_config,
> +	.set_config = vhost_vdpa_set_config,
>  	.enable_qp = vhost_vdpa_enable_queue_pair,
>  	.dma_map = vhost_vdpa_dma_map_batch,
>  	.dma_unmap = vhost_vdpa_dma_unmap_batch,
> --
> 2.31.1

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

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

* Re: [dpdk-dev] [PATCH v2 3/3] net/virtio: add MAC device config getter and setter
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 3/3] net/virtio: add MAC device config getter and setter Maxime Coquelin
@ 2021-06-23  7:54   ` Xia, Chenbo
  0 siblings, 0 replies; 7+ messages in thread
From: Xia, Chenbo @ 2021-06-23  7:54 UTC (permalink / raw)
  To: Maxime Coquelin, dev, amorenoz, david.marchand

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, June 17, 2021 10:17 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 3/3] net/virtio: add MAC device config getter and
> setter
> 
> This patch uses the new device config ops to get and set
> the MAC address if supported.
> 
> If a valid MAC address is passed as devarg of the
> Virtio-user PMD, the driver will try to store it in the
> device config space. Otherwise the one provided in
> the device config space will be used, if available.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 85 ++++++++++++++++---
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
>  drivers/net/virtio/virtio_user_ethdev.c       |  7 +-
>  3 files changed, 81 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index ed55cd7524..1cd1e95f45 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -260,21 +260,84 @@ int virtio_user_stop_device(struct virtio_user_dev
> *dev)
>  	return -1;
>  }
> 
> -static inline void
> -parse_mac(struct virtio_user_dev *dev, const char *mac)
> +int
> +virtio_user_dev_set_mac(struct virtio_user_dev *dev)
>  {
> -	struct rte_ether_addr tmp;
> +	int ret = 0;
> 
> -	if (!mac)
> -		return;
> +	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_MAC)))
> +		return -ENOTSUP;
> +
> +	if (!dev->ops->set_config)
> +		return -ENOTSUP;
> +
> +	ret = dev->ops->set_config(dev, dev->mac_addr,
> +			offsetof(struct virtio_net_config, mac),
> +			RTE_ETHER_ADDR_LEN);
> +	if (ret)
> +		PMD_DRV_LOG(ERR, "(%s) Failed to set MAC address in device",
> dev->path);
> +
> +	return ret;
> +}
> +
> +int
> +virtio_user_dev_get_mac(struct virtio_user_dev *dev)
> +{
> +	int ret = 0;
> +
> +	if (!(dev->device_features & (1ULL << VIRTIO_NET_F_MAC)))
> +		return -ENOTSUP;
> +
> +	if (!dev->ops->get_config)
> +		return -ENOTSUP;
> +
> +	ret = dev->ops->get_config(dev, dev->mac_addr,
> +			offsetof(struct virtio_net_config, mac),
> +			RTE_ETHER_ADDR_LEN);
> +	if (ret)
> +		PMD_DRV_LOG(ERR, "(%s) Failed to get MAC address from device",
> dev->path);
> +
> +	return ret;
> +}
> +
> +static void
> +virtio_user_dev_init_mac(struct virtio_user_dev *dev, const char *mac)
> +{
> +	struct rte_ether_addr cmdline_mac;
> +	char buf[RTE_ETHER_ADDR_FMT_SIZE];
> +	int ret;
> 
> -	if (rte_ether_unformat_addr(mac, &tmp) == 0) {
> -		memcpy(dev->mac_addr, &tmp, RTE_ETHER_ADDR_LEN);
> +	if (mac && rte_ether_unformat_addr(mac, &cmdline_mac) == 0) {
> +		/*
> +		 * MAC address was passed from command-line, try to store
> +		 * it in the device if it supports it. Otherwise try to use
> +		 * the device one.
> +		 */
> +		memcpy(dev->mac_addr, &cmdline_mac, RTE_ETHER_ADDR_LEN);
>  		dev->mac_specified = 1;
> +
> +		/* Setting MAC may fail, continue to get the device one in
> this case */
> +		virtio_user_dev_set_mac(dev);
> +		ret = virtio_user_dev_get_mac(dev);
> +		if (ret == -ENOTSUP)
> +			goto out;
> +
> +		if (memcmp(&cmdline_mac, dev->mac_addr, RTE_ETHER_ADDR_LEN))
> +			PMD_DRV_LOG(INFO, "(%s) Device MAC update failed", dev-
> >path);
>  	} else {
> -		/* ignore the wrong mac, use random mac */
> -		PMD_DRV_LOG(ERR, "wrong format of mac: %s", mac);
> +		ret = virtio_user_dev_get_mac(dev);
> +		if (ret) {
> +			PMD_DRV_LOG(ERR, "(%s) No valid MAC in devargs or device,
> use random",
> +					dev->path);
> +			return;
> +		}
> +
> +		dev->mac_specified = 1;
>  	}
> +out:
> +	rte_ether_format_addr(buf, RTE_ETHER_ADDR_FMT_SIZE,
> +			(struct rte_ether_addr *)dev->mac_addr);
> +	PMD_DRV_LOG(INFO, "(%s) MAC %s specified", dev->path, buf);
>  }
> 
>  static int
> @@ -509,8 +572,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	dev->unsupported_features = 0;
>  	dev->backend_type = backend_type;
> 
> -	parse_mac(dev, mac);
> -
>  	if (*ifname) {
>  		dev->ifname = *ifname;
>  		*ifname = NULL;
> @@ -538,6 +599,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  		return -1;
>  	}
> 
> +	virtio_user_dev_init_mac(dev, mac);
> +
>  	if (!mrg_rxbuf)
>  		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MRG_RXBUF);
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 58ad5198b6..819f6463ba 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -77,6 +77,8 @@ uint8_t virtio_user_handle_mq(struct virtio_user_dev
> *dev, uint16_t q_pairs);
>  int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t
> status);
>  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);
>  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 3ecbb4184a..90fcd6e7cc 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -60,12 +60,15 @@ virtio_user_write_dev_config(struct virtio_hw *hw,
> size_t offset,
>  	struct virtio_user_dev *dev = virtio_user_get_dev(hw);
> 
>  	if ((offset == offsetof(struct virtio_net_config, mac)) &&
> -	    (length == RTE_ETHER_ADDR_LEN))
> +	    (length == RTE_ETHER_ADDR_LEN)) {
>  		for (i = 0; i < RTE_ETHER_ADDR_LEN; ++i)
>  			dev->mac_addr[i] = ((const uint8_t *)src)[i];
> -	else
> +		virtio_user_dev_set_mac(dev);
> +		virtio_user_dev_get_mac(dev);
> +	} else {
>  		PMD_DRV_LOG(ERR, "not supported offset=%zu, len=%d",
>  			    offset, length);
> +	}
>  }
> 
>  static void
> --
> 2.31.1

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

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

* Re: [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support
  2021-06-17 14:17 [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Maxime Coquelin
                   ` (2 preceding siblings ...)
  2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 3/3] net/virtio: add MAC device config getter and setter Maxime Coquelin
@ 2021-06-23  8:08 ` Xia, Chenbo
  3 siblings, 0 replies; 7+ messages in thread
From: Xia, Chenbo @ 2021-06-23  8:08 UTC (permalink / raw)
  To: Maxime Coquelin, dev, amorenoz, david.marchand

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, June 17, 2021 10:17 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v2 0/3] net/virtio: add vdpa device config support
> 
> This patch adds vDPA device config space requests support.
> For now, it only adds MAC address get and set. It may be
> extended in next revision to support other configs like
> link state.
> 
>  drivers/net/virtio/virtio_user/vhost.h        |  3 +
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 69 ++++++++++++++
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 95 +++++++++++++++----
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
>  drivers/net/virtio/virtio_user_ethdev.c       | 12 ++-
>  5 files changed, 158 insertions(+), 23 deletions(-)
> 
> --
> 2.31.1

Series applied to next-virtio/main. Thanks


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

end of thread, other threads:[~2021-06-23  8:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 14:17 [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Maxime Coquelin
2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 1/3] net/virtio: keep device and frontend features separated Maxime Coquelin
2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 2/3] net/virtio: add device config support to vDPA Maxime Coquelin
2021-06-23  7:53   ` Xia, Chenbo
2021-06-17 14:17 ` [dpdk-dev] [PATCH v2 3/3] net/virtio: add MAC device config getter and setter Maxime Coquelin
2021-06-23  7:54   ` Xia, Chenbo
2021-06-23  8:08 ` [dpdk-dev] [PATCH v2 0/3] net/virtio: add vdpa device config support Xia, Chenbo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).