netdev - Re: [PATCH 1/2] vlan: only create special VLAN 0 once

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]

Message-ID: <4DEEAB41.1020306@trash.net>
Date:	Wed, 08 Jun 2011 00:50:41 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Jiri Bohac <jbohac@...e.cz>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	pedro.netdev@...devamos.com
Subject: Re: [PATCH 1/2] vlan: only create special VLAN 0 once

On 07.06.2011 18:41, Jiri Bohac wrote:
> On Tue, Jun 07, 2011 at 05:17:27PM +0200, Patrick McHardy wrote:
>> On 05.06.2011 23:28, David Miller wrote:
>>> I am pretty sure that the hardware device driver methods that
>>> implement ndo_vlan_rx_add_vid() assume that the device is up.
>>> Because most drivers completely reset the chip when the
>>> interface is brought up and this will likely clear out the
>>> VLAN ID tables in the chip.
>>>
>>
>> Good point.
>>
>> I don't think this approach works very well at all since
>> some drivers don't do incremental updates, but iterate over
>> the registered VLAN group when constructing filters. The
>> group is not created until the first real VLAN device is
>> registered however.
>>
>> Based on a quick grep (may have missed some):
>>
>> - via_velocity, mlx4, starfire: will do nothing
>>
>> - benet, igb, vxge, igbvf, ixgbevf, e1000e: would oops on
>>   rx_kill_vid due to unnecessary vlan_group_set_device()
> 
> which approach do you mean? David's or mine? I suppose you mean
> David's, because I did not call rx_kill_vid().

Neither, the approach how adding VID 0 was implemented. At
least the first list of drivers mentioned above will do
nothing, the second one would oops if we removed the VID
on NETDEV_DOWN without adding a real VLAN device.

>> The assumption of the drivers that a VLAN group exists
>> before the first VID is configured is reasonable in my
>> opinion, a lot of them also don't even configure VLAN
>> filtering until the VLAN group is registered.
> 
> So this is broken already since ad1afb00 :(

Exactly.

> The assumption broke the bonding driver and this got fixed by
> f35188fa, btw.
> 
>> So I think a good solution would be to make sure all
>> drivers don't enable VLAN filtering before the first
>> VLAN is actually registered and do the automatic
>> registration of VID 0 once the first real VLAN device
>> is created.
> 
> But this behaviour is not what was intended by ad1afb00.
> The VID 0 needs to be registered by default, to make 8021p work.
> Even without any real VLAN devices created.

I'm not sure what exactly you're referring to. HW VLAN filters
are (or should be) usually only activated when the first
VLAN device is registered, so this change has no effect.

If drivers behave that way *and* the VID 0 is only registered
automatically when the first real device is configured,
this should provide the desired behaviour.

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