Re: [dpdk-dev] [PATCH] e1000: fix setting of VF MAC address

["Lu, Wenzhuo" <wenzhuo.lu@intel.com>]

Hi Bernard,


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bernard Iremonger
> Sent: Thursday, March 3, 2016 12:09 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] e1000: fix setting of VF MAC address
> 
> Allow reprogramming of the RAR with a zero mac address, to ensure that the VF
> traffic goes to the PF after stop, close and detach of the VF.
> 
> Fixes: be2d648a2dd3 ("igb: add PF support")
> Fixes: d82170d27918 ("igb: add VF support")
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  drivers/net/e1000/igb_ethdev.c | 12 +++++++++++-
>  drivers/net/e1000/igb_pf.c     |  8 +++++---
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 4ed5e95..f1044b7 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -1,7 +1,7 @@
>  /*-
>   *   BSD LICENSE
>   *
> - *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -2819,6 +2819,7 @@ igbvf_dev_close(struct rte_eth_dev *dev)
>  	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	struct e1000_adapter *adapter =
>  		E1000_DEV_PRIVATE(dev->data->dev_private);
> +	struct ether_addr addr;
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> @@ -2827,6 +2828,15 @@ igbvf_dev_close(struct rte_eth_dev *dev)
>  	igbvf_dev_stop(dev);
>  	adapter->stopped = 1;
>  	igb_dev_free_queues(dev);
> +
> +	/**
> +	 * reprogram the RAR with a zero mac address,
> +	 * to ensure that the VF traffic goes to the PF
> +	 * after stop, close and detach of the VF.
> +	 **/
> +
> +	memset(&addr, 0, sizeof(addr));
> +	igbvf_default_mac_addr_set(dev, &addr);
>  }
> 
>  static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on) diff --git
> a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c index
> 1d00dda..95204e9 100644
> --- a/drivers/net/e1000/igb_pf.c
> +++ b/drivers/net/e1000/igb_pf.c
> @@ -1,7 +1,7 @@
>  /*-
>   *   BSD LICENSE
>   *
> - *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -332,8 +332,10 @@ igb_vf_set_mac_addr(struct rte_eth_dev *dev,
> uint32_t vf, uint32_t *msgbuf)
>  	int rar_entry = hw->mac.rar_entry_count - (vf + 1);
>  	uint8_t *new_mac = (uint8_t *)(&msgbuf[1]);
> 
> -	if (is_valid_assigned_ether_addr((struct ether_addr*)new_mac)) {
> -		rte_memcpy(vfinfo[vf].vf_mac_addresses, new_mac, 6);
> +	if (is_unicast_ether_addr((struct ether_addr *)new_mac)) {
> +		if (!is_zero_ether_addr((struct ether_addr *)new_mac))
> +			rte_memcpy(vfinfo[vf].vf_mac_addresses, new_mac,
> +				sizeof(vfinfo[vf].vf_mac_addresses));
>  		hw->mac.ops.rar_set(hw, new_mac, rar_entry);
If the new mac is 0, after this, the rar is 0, but the address stored in vfinfo is not changed and surely not 0. Right?
So, they're not align with each other. Could it cause some problem?

>  		return 0;
>  	}
> --
> 2.6.3