netdev - Re: [PATCH net-next v2 1/3] macvlan: don't update the uc and vlan list for L2 forwarding offload

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]

Date:	Fri, 06 Jun 2014 13:11:34 -0400
From:	Vlad Yasevich <vyasevich@...il.com>
To:	Ding Tianhong <dingtianhong@...wei.com>, kaber@...sh.net,
	davem@...emloft.net, edumazet@...gle.com, vyasevic@...hat.com,
	makita.toshiaki@....ntt.co.jp
CC:	netdev@...r.kernel.org, John Fastabend <john.r.fastabend@...el.com>
Subject: Re: [PATCH net-next v2 1/3] macvlan: don't update the uc and vlan
 list for L2 forwarding offload

On 06/05/2014 11:45 PM, Ding Tianhong wrote:
> On 2014/6/5 23:09, Vlad Yasevich wrote:
>> On 06/05/2014 08:01 AM, Ding Tianhong wrote:
>>> If lowerdev supports L2 forwarding offload, no need to set mac address
>>> to uc list and vlan list, so also don't do that when the macvlan mac address
>>> changes.
>>
>> Same issue as in v1.  Please explain how this actually works, since the
>> new address never makes it to the hw.
>>
> 
> Hi, vlad:
> 
> The fwd only support 82599 nic, and when fwd_priv is true, this means the lowerdev support L2 forwarding offload, and will set the macvlan
> to the ixgbe_fwd_adapter->netdev, and then it is clear that the lowerdev could know that the the upper device is macvlan and could handle
> the skb to the upperdev.
> 
> If I miss something, please remind me, thanks.

You've just changed the mac address of the macvlan device, but the
address is not written to the card's RAR registers.
So what allows the card to receive the traffic and place it into the
queue in the first place?

I can see it possibly working if the card is in promiscuous mode.

Also, I think John is right in that if we simply do dev_uc_add()
to add the address to the card, we'll unmap the address from the vmdqs
and lose the acceleration.  This needs more thought.

-vlad

> 
> Ding
> 
>> -vlad
>>>
>>> Signed-off-by: Ding Tianhong <dingtianhong@...wei.com>
>>> ---
>>>  drivers/net/macvlan.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>> index 453d55a..c3a54a6 100644
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr)
>>>  	struct net_device *lowerdev = vlan->lowerdev;
>>>  	int err;
>>>  
>>> -	if (!(dev->flags & IFF_UP)) {
>>> +	if (!(dev->flags & IFF_UP) || vlan->fwd_priv) {
>>>  		/* Just copy in the new address */
>>>  		ether_addr_copy(dev->dev_addr, addr);
>>>  	} else {
>>>
>>
>>
>>
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html