Re: [RFC 3/3] net: dsa: mv88e6xxx: fix vlan setup

From: Ido Schimmel <idosch@idosch.org>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>,
	netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Jiri Pirko <jiri@resnulli.us>, Ido Schimmel <idosch@mellanox.com>,
	"David S. Miller" <davem@davemloft.net>,
	Ivan Vecera <ivecera@redhat.com>,
	Vivien Didelot <vivien.didelot@gmail.com>
Subject: Re: [RFC 3/3] net: dsa: mv88e6xxx: fix vlan setup
Date: Tue, 24 Dec 2019 10:30:45 +0200	[thread overview]
Message-ID: <20191224083045.GA895380@splinter> (raw)
In-Reply-To: <562d2a65-8361-9361-f761-082ace3a77bc@gmail.com>

On Mon, Dec 23, 2019 at 10:02:12AM -0800, Florian Fainelli wrote:
> +Ido,
> 
> On 12/22/2019 11:24 AM, Russell King wrote:
> > DSA assumes that a bridge which has vlan filtering disabled is not
> > vlan aware, and ignores all vlan configuration. However, the kernel
> > software bridge code allows configuration in this state.
> > 
> > This causes the kernel's idea of the bridge vlan state and the
> > hardware state to disagree, so "bridge vlan show" indicates a correct
> > configuration but the hardware lacks all configuration. Even worse,
> > enabling vlan filtering on a DSA bridge immediately blocks all traffic
> > which, given the output of "bridge vlan show", is very confusing.
> > 
> > Provide an option that drivers can set to indicate they want to receive
> > vlan configuration even when vlan filtering is disabled. This is safe
> > for Marvell DSA bridges, which do not look up ingress traffic in the
> > VTU if the port is in 8021Q disabled state. Whether this change is
> > suitable for all DSA bridges is not known.
> 
> s/DSA bridges/DSA switches/ ?
> 
> this is also safe to do with b53 switches in fact this is even desirable
> because VLAN filtering is a global attribute so if you have at least one
> bridge that spans one of your switch ports and that bridge requests
> vlan_filtering=1, you *must* have a valid VID entry for the non-bridged
> ports, or the bridged ports with vlan_filtering=0 otherwise there is no
> default VID entry programmed to ingress the frame. Today this is
> achieved by making sure that the default untagged VID 0 for non bridged
> ports is always programmed at start-up and the switch is always
> configured with VLAN awareness.
> 
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> This ties in with this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2ea7a679ca2abd251c1ec03f20508619707e1749
> 
> which I believe is still correct in the sense that with Linux a bridge
> with vlan_filtering=0 also means that the bridge is not VLAN aware. Ido,
> Jiri, do you disagree?

Hi Florian,

Yes, vlan_filtering=0 means that the bridge is not VLAN aware. It's not
only about filtering at ingress / egress. The man page says "When
disabled, the bridge will not consider the VLAN tag when handling
packets." It also affects the VLAN with which FDB entries are learned
and the VLAN used for FDB lookup.

It is really problematic for switch drivers to properly support the
dynamic toggling of this option. This is why mlxsw forbids this
toggling. Either you create the bridge with VLAN filtering disabled or
enabled. Assuming I'm reading Cumulus documentation correctly, it seems
they enforce the same behavior:

https://docs.cumulusnetworks.com/cumulus-linux/Layer-2/Ethernet-Bridging-VLANs/VLAN-aware-Bridge-Mode/#convert-bridges-between-supported-modes

> This seems to be coming every now and then, so maybe it is time to
> revisit this documentation patch:
> 
> https://github.com/ffainelli/linux/commit/3fe61b1722a3b79d2e317a812c54f3afc902e5b0

Yes, I remember reviewing it. Not sure why you didn't send another
version :)