netdev - Re: [net-next 2/9] i40e: make visible changed vf mac on host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9a3a4675-b031-7666-f259-978d18b6db19@pensando.io>
Date:   Thu, 1 Aug 2019 17:11:06 -0700
From:   Shannon Nelson <snelson@...sando.io>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net
Cc:     Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
        netdev@...r.kernel.org, nhorman@...hat.com, sassmann@...hat.com,
        Andrew Bowers <andrewx.bowers@...el.com>
Subject: Re: [net-next 2/9] i40e: make visible changed vf mac on host

On 8/1/19 1:51 PM, Jeff Kirsher wrote:
> From: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>
> This patch makes changed VM mac address visible on host via
> ip link show command. This problem is fixed by copying last
> unicast mac filter to vf->default_lan_addr.addr. Without
> this patch if VF MAC was not set from host side and
> if you run ip link show $pf, on host side you'd always
> see a zero MAC, not the real VF MAC that VF assigned to
> itself.
>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> Tested-by: Andrew Bowers <andrewx.bowers@...el.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index 02b09a8ad54c..21f7ac514d1f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -2629,6 +2629,9 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>   			} else {
>   				vf->num_mac++;
>   			}
> +			if (is_valid_ether_addr(al->list[i].addr))
> +				ether_addr_copy(vf->default_lan_addr.addr,
> +						al->list[i].addr);
>   		}
>   	}
>   	spin_unlock_bh(&vsi->mac_filter_hash_lock);

Since this copy is done inside the for-loop, it looks like you are 
copying every address in the list, not just the last one.  This seems 
wasteful and unnecessary.

Since it is possible, altho' unlikely, that the filter sync that happens 
a little later could fail, might it be better to do the copy after you 
know that the sync has succeeded?

Why is the last mac chosen for display rather than the first?  Is there 
anything special about the last mac as opposed to the first mac?

sln