All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Moreno <amorenoz@redhat.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	dev@dpdk.org, chenbo.xia@intel.com, david.marchand@redhat.com
Subject: Re: [dpdk-dev] [RFC 3/3] net/virtio: add MAC device config getter and setter
Date: Fri, 16 Apr 2021 11:27:23 +0200	[thread overview]
Message-ID: <3997731b-29cd-5e1f-fc9f-b31b7cb5abe3@redhat.com> (raw)
In-Reply-To: <20210318223526.168614-4-maxime.coquelin@redhat.com>



On 3/18/21 11:35 PM, Maxime Coquelin wrote:
> 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  | 78 ++++++++++++++++---
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  2 +
>  drivers/net/virtio/virtio_user_ethdev.c       |  7 +-
>  3 files changed, 74 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 8757a23f6e..61517692b3 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -259,20 +259,76 @@ 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);
> +
> +	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);
>  
> -	if (rte_ether_unformat_addr(mac, &tmp) == 0) {
> -		memcpy(dev->mac_addr, &tmp, RTE_ETHER_ADDR_LEN);
> +	return ret;
> +}
> +
> +static void
> +virtio_user_dev_init_mac(struct virtio_user_dev *dev, const char *mac)
> +{
> +	struct rte_ether_addr cmdline_mac;
> +	int ret;
> +
> +	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)
> +			return;
> +
> +		if (memcmp(&cmdline_mac, dev->mac_addr, RTE_ETHER_ADDR_LEN))
> +			PMD_DRV_LOG(INFO, "(%s) Device MAC update failed\n", dev->path);

Maybe log the mac that we've read if it differs from the one we tried to write?

>  	} 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\n",
> +					dev->path);
> +		else
> +			dev->mac_specified = 1;
>  	}
>  }
>  
> @@ -508,8 +564,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;
> @@ -537,6 +591,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 8a62f7ea79..03bcf95970 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -78,6 +78,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_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 bb36316186..34c8e7a9b5 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
> 

-- 
Adrián Moreno


  reply	other threads:[~2021-04-16  9:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-18 22:35 [dpdk-dev] [RFC 0/3] net/virtio: add vdpa device config support Maxime Coquelin
2021-03-18 22:35 ` [dpdk-dev] [RFC 1/3] net/virtio: keep device and frontend features separated Maxime Coquelin
2021-03-18 22:35 ` [dpdk-dev] [RFC 2/3] net/virtio: add device config support to vDPA Maxime Coquelin
2021-03-18 22:35 ` [dpdk-dev] [RFC 3/3] net/virtio: add MAC device config getter and setter Maxime Coquelin
2021-04-16  9:27   ` Adrian Moreno [this message]
2021-04-19  6:24   ` Xia, Chenbo
2021-06-03 14:28     ` Maxime Coquelin
2021-06-08  5:29       ` Xia, Chenbo
2021-06-08  6:22         ` Maxime Coquelin
2021-04-16  7:28 ` [dpdk-dev] [RFC 0/3] net/virtio: add vdpa device config support Adrian Moreno
2021-04-16  8:10   ` Maxime Coquelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3997731b-29cd-5e1f-fc9f-b31b7cb5abe3@redhat.com \
    --to=amorenoz@redhat.com \
    --cc=chenbo.xia@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=maxime.coquelin@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.