diff mbox

[net-next,1/4] macvlan: don't update the uc and vlan list for L2 forwarding offload

Message ID 1401951028-9800-2-git-send-email-dingtianhong@huawei.com
State New
Headers show

Commit Message

Ding Tianhong June 5, 2014, 6:50 a.m. UTC
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.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 drivers/net/macvlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vlad Yasevich June 5, 2014, 1:52 p.m. UTC | #1
On 06/05/2014 02:50 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.
> 

Are you sure about this?  How would the lower dev receive traffic
destined to the new HW address if it is not in the device unicast filter
list?

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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Fastabend June 5, 2014, 2:12 p.m. UTC | #2
On 6/5/2014 6:52 AM, Vlad Yasevich wrote:
> On 06/05/2014 02:50 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.
>>
>
> Are you sure about this?  How would the lower dev receive traffic
> destined to the new HW address if it is not in the device unicast filter
> list?

I don't think the offload path works as it is either though (need to
test). The issue is sync'ing with the lowerdev doesn't give the lowerdev
any information on which fwd_priv to set so the lowerdev sets the normal
path with the address. It won't "break" traffic just receive flows wont
use the direct hardware queues. This will be a bigger issue once we get
support for metering and hardware ACLs.

Agreed though, Ding how does this work? Looks like a step back to me. At
very least can we get the commit message to explain this a bit better.

.John

>> 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 {
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

Patch

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 {