Re: [PATCH net] net: dsa: fix VLAN 0 filter imbalance when toggling filtering

[Vladimir Oltean]

On Fri, Apr 25, 2025 at 09:52:13AM +0200, Jonas Gorski wrote:
> I gave it a test with a vlan_filtering bridge with no PVID / egress
> untagged vlan defined on a pure software bridge, and STP continued to
> work fine.

STP is not part of the bridge data path, it is control path. The PVID
rules don't apply to it.
In software terms, br_handle_frame() returns RX_HANDLER_PASS for it, it
doesn't go through br_handle_vlan().

> So in a sense, VLAN 0 is needed, as we still need to allow
> untagged traffic to be received regardless of a PVID egress untagged
> VLAN being defined.

When we are talking about the hardware data path of a switchdev port,
that is debatable as well, since many switchdevs have built-in packet
traps which again bypass the VLAN table (a function specific to the
switching layer, like learning, STP state etc). I would argue that the
presence of VID 0 in the RX filtering table is irrelevant for STP as far
as switchdevs are concerned.

> But we shouldn't forward it (except to the cpu port) unless it is part
> of a PVID egress untagged VLAN. This is the tricky part. If (dsa)
> switch drivers ensure that untagged traffic always reaches the cpu
> port, then we can ignore VLAN 0.
> 
> So I think this boils down to that dsa needs a way to pass on to
> drivers whether a VLAN should be forwarded to other members or not
> when adding it to a port.

That can be done (add a struct dsa_db argument to port_vlan_add(),
signifying whether it is a port VLAN or a bridge VLAN), but I haven't
come across switches which can make the distinction. It would require
mapping the same VID, coming from different ports, to different hardware
FIDs.

> Currently, from a dsa driver perspective, the following two scenarios
> would be indistinguishable:
> 
> $ ip link add br0 type bridge vlan_filtering 1
> $ ip link set sw1p1 master br0
> $ ip link set sw1p2 master br0
> $ bridge vlan add dev sw1p1 vid 10
> $ bridge vlan add dev sw2p1 vid 10
> 
> and
> 
> $ ip link add br0 type bridge vlan_filtering 1
> $ ip link set sw1p1 master br0
> $ ip link set sw1p2 master br0
> $ ip link add sw1p1.10 link sw1p1 type vlan id 10
> $ ip link add sw1p2.10 link sw1p2 type vlan id 10
> 
> But in the second case, swp1p1 and sw1p2 should be isolated.
> 
> This is because vlan filters and bridge vlans result in the same
> port_vlan_add() call, with no way of the driver to tell from where the
> call comes from.
> 
> And yes, this is something that is probably hard to configure for many
> smaller embedded switch chips. E.g. b53 supported switches do not have
> forward/flood/etc masks per VLAN, so some cheating/workaround is
> needed here. switchdev.rst says to fall back to software forwarding if
> there is no other way. I have some ideas, but I will first need to
> verify that they work ... .

We have insufficient coverage in dsa_user_prechangeupper_sanity_check()
and dsa_port_can_apply_vlan_filtering(), but we should add another
restriction for this: 8021q uppers with the same VID should not be
installed to ports spanning the same VLAN-aware bridge. And there should
be a new test for it in tools/testing/selftests/net/forwarding/no_forwarding.sh.

The restriction can be selectively lifted if there ever appear drivers
which can make the distinction you are talking about, but I don't think
that any of them can, at the moment.