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

[Ding Tianhong <dingtianhong@huawei.com>]

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@huawei.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 {
>>>>
>>>
>>>
>>>
>>
>>
> 
> 
> .
>