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] [day] [month] [year] [list]

Date:	Sat, 7 Jun 2014 13:59:15 +0800
From:	Ding Tianhong <dingtianhong@...wei.com>
To:	Vlad Yasevich <vyasevich@...il.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 2014/6/7 1:11, Vlad Yasevich wrote:
> 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.
> 

Yes, I miss it, the new mac address should be set to the lowerdev by ndo_dfwd_add_station(),
otherwise the lowerdev still use the old mac address and the macvlan could not work again,
so I need to rethink this patch and resend, thanks, vlad.

Ding

> -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