Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Tuesday, June 8, 2021 10:14 PM
> To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>; amore...@redhat.com;
> david.march...@redhat.com
> Cc: Maxime Coquelin <maxime.coque...@redhat.com>
> Subject: [PATCH 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.coque...@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..5c9f142024 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\n",
> dev->path);
No need to add '\n'. And same for below 'PMD_DRV_LOG'
Thanks,
Chenbo
> +
> + 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\n",
> 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
