Re: [PATCH v2 2/2] hv_netvsc: pair VF based on serial number

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Fri, Sep 14, 2018 at 12:54:57PM -0700, Stephen Hemminger wrote:
> Matching network device based on MAC address is problematic
> since a non VF network device can be creted with a duplicate MAC
> address causing confusion and problems.  The VMBus API does provide
> a serial number that is a better matching method.
> 
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
>  drivers/net/hyperv/netvsc.c     |  3 ++
>  drivers/net/hyperv/netvsc_drv.c | 58 +++++++++++++++++++--------------
>  2 files changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 31c3d77b4733..fe01e141c8f8 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -1203,6 +1203,9 @@ static void netvsc_send_vf(struct net_device *ndev,
>  
>  	net_device_ctx->vf_alloc = nvmsg->msg.v4_msg.vf_assoc.allocated;
>  	net_device_ctx->vf_serial = nvmsg->msg.v4_msg.vf_assoc.serial;
> +	netdev_info(ndev, "VF slot %u %s\n",
> +		    net_device_ctx->vf_serial,
> +		    net_device_ctx->vf_alloc ? "added" : "removed");
>  }
>  
>  static  void netvsc_receive_inband(struct net_device *ndev,
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1121a1ec407c..9dedc1463e88 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1894,20 +1894,6 @@ static void netvsc_link_change(struct work_struct *w)
>  	rtnl_unlock();
>  }
>  
> -static struct net_device *get_netvsc_bymac(const u8 *mac)
> -{
> -	struct net_device_context *ndev_ctx;
> -
> -	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
> -		struct net_device *dev = hv_get_drvdata(ndev_ctx->device_ctx);
> -
> -		if (ether_addr_equal(mac, dev->perm_addr))
> -			return dev;
> -	}
> -
> -	return NULL;
> -}
> -
>  static struct net_device *get_netvsc_byref(struct net_device *vf_netdev)
>  {
>  	struct net_device_context *net_device_ctx;
> @@ -2036,26 +2022,48 @@ static void netvsc_vf_setup(struct work_struct *w)
>  	rtnl_unlock();
>  }
>  
> +/* Find netvsc by VMBus serial number.
> + * The PCI hyperv controller records the serial number as the slot.
> + */
> +static struct net_device *get_netvsc_byslot(const struct net_device *vf_netdev)
> +{
> +	struct device *parent = vf_netdev->dev.parent;
> +	struct net_device_context *ndev_ctx;
> +	struct pci_dev *pdev;
> +
> +	if (!parent || !dev_is_pci(parent))
> +		return NULL; /* not a PCI device */
> +
> +	pdev = to_pci_dev(parent);
> +	if (!pdev->slot) {
> +		netdev_notice(vf_netdev, "no PCI slot information\n");
> +		return NULL;
> +	}
> +
> +	list_for_each_entry(ndev_ctx, &netvsc_dev_list, list) {
> +		if (!ndev_ctx->vf_alloc)
> +			continue;
> +
> +		if (ndev_ctx->vf_serial == pdev->slot->number)
> +			return hv_get_drvdata(ndev_ctx->device_ctx);

In patch 1, pdev->slot->number is set to:

PCI_SLOT(wslot_to_devfn(hpdev->desc.win_slot.slot))

so I assume vf_serial is initialized (I have no knowledge of VMBUS and
hyper-V internals) to that value, somehow.

I also do not know how the wslot stuff is handled but I assume 5 bits
(ie dev bits in devfn) are enough.

BTW, I have noticed this patch (and patch 1) are already in -next so I will
drop them from the PCI patch queue.

Lorenzo

> +	}
> +
> +	netdev_notice(vf_netdev,
> +		      "no netdev found for slot %u\n", pdev->slot->number);
> +	return NULL;
> +}
> +
>  static int netvsc_register_vf(struct net_device *vf_netdev)
>  {
> -	struct net_device *ndev;
>  	struct net_device_context *net_device_ctx;
> -	struct device *pdev = vf_netdev->dev.parent;
>  	struct netvsc_device *netvsc_dev;
> +	struct net_device *ndev;
>  	int ret;
>  
>  	if (vf_netdev->addr_len != ETH_ALEN)
>  		return NOTIFY_DONE;
>  
> -	if (!pdev || !dev_is_pci(pdev) || dev_is_pf(pdev))
> -		return NOTIFY_DONE;
> -
> -	/*
> -	 * We will use the MAC address to locate the synthetic interface to
> -	 * associate with the VF interface. If we don't find a matching
> -	 * synthetic interface, move on.
> -	 */
> -	ndev = get_netvsc_bymac(vf_netdev->perm_addr);
> +	ndev = get_netvsc_byslot(vf_netdev);
>  	if (!ndev)
>  		return NOTIFY_DONE;
>  
> -- 
> 2.18.0
>