Re: [RFC PATCH net-next 1/3] selftests: forwarding: add a vlan_deletion test to bridge_vlan_unaware

[Ido Schimmel <idosch@nvidia.com>]

On Tue, Jul 05, 2022 at 08:31:12PM +0300, Vladimir Oltean wrote:
> Historically, DSA drivers have seen problems with the model in which
> bridge VLANs work, particularly with them being offloaded to switchdev
> asynchronously relative to when they become active (vlan_filtering=1).
> 
> This switchdev API peculiarity was papered over by commit 2ea7a679ca2a
> ("net: dsa: Don't add vlans when vlan filtering is disabled"), which
> introduced other problems, fixed by commit 54a0ed0df496 ("net: dsa:
> provide an option for drivers to always receive bridge VLANs") through
> an opt-in ds->configure_vlan_while_not_filtering bool (which later
> became an opt-out).
> 
> The point is that some DSA drivers still skip VLAN configuration while
> VLAN-unaware, and there is a desire to get rid of that behavior.
> 
> It's hard to deduce from the wording "at least one corner case" what
> Andrew saw, but my best guess is that there is a discrepancy of meaning
> between bridge pvid and hardware port pvid which caused breakage.
> 
> On one side, the Linux bridge with vlan_filtering=0 is completely
> VLAN-unaware, and will accept and process a packet the same way
> irrespective of the VLAN groups on the ports or the bridge itself
> (there may not even be a pvid, and this makes no difference).
> 
> On the other hand, DSA switches still do VLAN processing internally,
> even with vlan_filtering disabled, but they are expected to classify all
> packets to the port pvid. That pvid shouldn't be confused with the
> bridge pvid, and there lies the problem.
> 
> When a switch port is under a VLAN-unaware bridge, the hardware pvid
> must be explicitly managed by the driver to classify all received
> packets to it, regardless of bridge VLAN groups. When under a VLAN-aware
> bridge, the hardware pvid must be synchronized to the bridge port pvid.
> To do this correctly, the pattern is unfortunately a bit complicated,
> and involves hooking the pvid change logic into quite a few places
> (the ones that change the input variables which determine the value to
> use as hardware pvid for a port). See mv88e6xxx_port_commit_pvid(),
> sja1105_commit_pvid(), ocelot_port_set_pvid() etc.
> 
> The point is that not all drivers used to do that, especially in older
> kernels. If a driver is to blindly program a bridge pvid VLAN received
> from switchdev while it's VLAN-unaware, this might in turn change the
> hardware pvid used by a VLAN-unaware bridge port, which might result in
> packet loss depending which other ports have that pvid too (in that same
> note, it might also go unnoticed).
> 
> To capture that condition, it is sufficient to take a VLAN-unaware
> bridge and change the [VLAN-aware] bridge pvid on a single port, to a
> VID that isn't present on any other port. This shouldn't have absolutely
> any effect on packet classification or forwarding. However, broken
> drivers will take the bait, and change their PVID to 3, causing packet
> loss.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Tested-by: Ido Schimmel <idosch@nvidia.com>