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

On Tue, Jun 07, 2011 at 05:17:27PM +0200, Patrick McHardy wrote:

On 05.06.2011 23:28, David Miller wrote:

quoted

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().

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 :(
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.

-- 
Jiri Bohac [off-list ref]
SUSE Labs, SUSE CZ