Subject: Re: [PATCH] net/vlan: withdraw VLAN ID attribute from GVRP on VLAN device stop

From: "Jorge Boncompte [DTI2]" <[email-masked]>
Date: Mon, 26 Mar 2012 17:14:18 +0200

El 26/03/2012 15:38, Ward, David - 0663 - MITLL escribió:
> On 26/03/12 07:29, Jorge Boncompte [DTI2] wrote:
>> El 26/03/2012 0:43, David Ward escribió:
>>> When a VLAN device is stopped which has VLAN_FLAG_GVRP set, the VLAN
>>> ID attribute that was previously declared by GVRP must be withdrawn.
>>>
>> 	Hmm, maybe I am missing something but I think it only makes sense to withdrawn
>> the attribute when you delete the interface, and vlan_dev_stop() it's called if
>> you just put the interface down. It's better for the network convergence to not
>> signal the switches just for this. IMHO.
> 
> If I bring a VLAN interface down, then I stop participating in the 
> VLAN.  If my NIC still receives traffic for the VLAN, I drop it.  So to 
> remove unnecessary load on the (potentially shared) network link and 
> remove unnecessary local processing by the kernel of packets I know I am 
> going to drop, I should tell the switch that I am no longer interested 
> in receiving the VLAN traffic.  Which is the whole point of GVRP.  Right?

	In the non-GVRP case, as far as i can see, you still receive the traffic for
that VLAN and the kernel drops it. Maybe is that I think that a downed interface
it's more a transient state, and administratively choosen one. If you don't want
to participate in that VLAN, you always can disable GVRP on it or delete the
interface.

> In any case, we currently register the attribute when the interface is 
> brought up, not when it is created.

	And that makes sense to me, if you have never uped the interface you have never
participated in that VLAN.

>  However we do it, the attribute
> declaration/withdrawal should be symmetric.
> 
>>
>>> Signed-off-by: David Ward<[email-masked]>
>>> ---
>>>   net/8021q/vlan_dev.c |    3 +++
>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>>> index 9988d4a..df86dd0 100644
>>> --- a/net/8021q/vlan_dev.c
>>> +++ b/net/8021q/vlan_dev.c
>>> @@ -319,6 +319,9 @@ static int vlan_dev_stop(struct net_device *dev)
>>>   	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>>>   	struct net_device *real_dev = vlan->real_dev;
>>>
>>> +	if (vlan->flags&  VLAN_FLAG_GVRP)
>>> +		vlan_gvrp_request_leave(dev);
>>> +
>>>   	dev_mc_unsync(real_dev, dev);
>>>   	dev_uc_unsync(real_dev, dev);
>>>   	if (dev->flags&  IFF_ALLMULTI)--
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email-masked]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
>