Message-ID: <20250428100711.cushq4ok3s2n5fxm@skbuf>
Date: Mon, 28 Apr 2025 13:07:11 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jonas Gorski <jonas.gorski@...il.com>
Cc: Nikolay Aleksandrov <razor@...ckwall.org>,
	Ido Schimmel <idosch@...dia.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, Andrew Lunn <andrew@...n.ch>,
	bridge@...ts.linux.dev, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net 2/2] net: dsa: propagate brentry flag changes

On Sat, Apr 19, 2025 at 12:52:09PM +0200, Jonas Gorski wrote:
> On Mon, Apr 14, 2025 at 5:07 PM Vladimir Oltean <vladimir.oltean@....com> wrote:
> > You'd have to ask yourself how do you even expect DSA to react and sort
> > this out, between the bridge direction wanting the VLAN untagged and the
> > 8021q direction wanting it tagged.
> 
> Unless the switch chip supports passing frames as is to the cpu port
> (and only there), the only winning move is likely to keep the cpu port
> tagged in all cases, and untag in software as needed. I guess this is
> what untag_vlan_aware_bridge_pvid is for.
> 
> As a side note, b53 datasheets state that all vlans are always tagged
> on egress and ignore the untag map for the management port, but that
> is clearly not the case for most devices lol.

So it's in fact not a problem, am I reading this right?

> I am still a bit confused by untag_bridge_pvid, or more specifically
> dsa_software_untag_vlan_unaware_bridge(). I would have expected a non
> vlan_filtering bridge to ignore any vlan configuration, including
> PVID. Or rather the PVID is implicit by a port being bridged with vlan
> uppers of a certain vid (e.g. port1.10 + port2 => port2 has PVID 10),
> but not explicitly via the bridge vlan configuration.

I think dsa_software_untag_vlan_unaware_bridge() exists in this form
only as a result of the misunderstanding that the bridge PVID must not
be committed to hardware in vlan_filtering=0 mode. Otherwise, it is a
coincidence that the bridge PVID == the VID added by hardware to packets
in VLAN-unaware bridging mode.

See the reference to ds->ops->get_private_vid() if you need to pop a
driver-specific VID from packets received from VLAN-unaware bridge ports,
VID which is not necessarily equal to the bridge PVID.

> > > I guess the proper fix for b53 is probably to always have a vlan tag
> > > on the cpu port (except for the special vlan 0 for untagged traffic on
> > > ports with no PVID), and enable untag_vlan_aware_bridge_pvid.
> >
> > What's the story with the ports with no PVID, and VID 0?
> > In Documentation/networking/switchdev.rst, it is said that VLAN
> > filtering bridge ports should drop untagged and VID 0 tagged RX packets
> > when there is no pvid.
> 
> Hm, that's not what the code does:
> 
> With a vlan_filtering bridge VID 0 always gets added to bridge ports
> if they get set up:
> 
> The order is:
> 
> 1. filtering is enabled on the port => NETIF_F_HW_VLAN_CTAG_FILTER is set
> 2. on if up, vlan_device_event() sees that NETIF_F_HW_VLAN_CTAG_FILTER
> is enabled and calls vlan_vid_add(dev, .., 0)
> 3. switchdev/dsa passes this on to the dsa driver via port_vlan_add()
> 4. all bridge ports being up are now members of vlan 0/have vlan 0 enabled.
> 
> Not sure if this is intended/expected behavior, but this enables
> untagged rx at least with b53. And since b53 can't restrict forwarding
> on a per-vlan base, likely enables forwarding between all bridge ports
> for untagged traffic (until a PVID vlan is configured on the bridge,
> then the untagged traffic is moved to a different port). This part is
> likely not intended.
> 
> My first guess was that this is intentional to allow STP & co to work
> regardless if there is a PVID/egress untagged VLAN configured. Though
> this will likely also require preventing of forwarding unless this is
> a configured bridge vlan. Currently trying to read 802.1Q-2022 to see
> if I find anything clearly stating how this should (not) work ... .

Not replying to this, as this discussion continued in the other thread.

> > > Makes the think the cpu port always tagged should be the default
> > > behavior. It's easy to strip a vlan tag that shouldn't be there, but
> > > hard to add one that's missing, especially since in contrast to PVID
> > > you can have more than one vlan as egress untagged.
> >
> > I agree and I would like to see b53 converge towards that. But changing
> > the default by unsetting this flag in DSA could be a breaking change, we
> > should be careful, and definitely only consider that for net-next.
> >
> > b53 already sets a form (the deprecated form) of ds->untag_bridge_pvid,
> > someone with hardware should compare its behavior to the issues
> > documented in dsa_software_untag_vlan_unaware_bridge(), and, if
> > necessary, transition it to ds->untag_vlan_aware_bridge_pvid or perhaps
> > something else.
> 
> This is for the case when you have a b53 switch behind a b53 switch, e.g.
> 
> sw0.port1..4 user ports
> sw0.cpu -> sw1.port1
> sw1.port2..4 user ports
> sw1.cpu -> eth0
> 
> and you have user ports on both switches. Due to the way the broadcom
> tag works, if sw0 would be brcm tagged, then sw1 wouldn't see the vlan
> tag anymore on rx (since there will now be the brcm tag), and all
> traffic would go to the pvid of sw1.port1, thus any forwarding between
> ports of sw0 and sw1 would have to be done in software.

Can sw0 be brcm-tagged in another way that allows sw1 to see VLAN tags?
Like with brcm-legacy perhaps?

> In this case, sw0 needs to have its traffic always tagged. Also sw1
> needs to keep all vlans on port1 egress tagged, not sure if we
> actually take care of that ... (maybe the dsa code does that for us,
> didn't check).

No, this would be driver-level configuration. DSA doesn't alter the
egress-tagged flag of bridge VLANs.

> Although forwarding works between user ports of sw0 and sw1, from the
> linux perspective we would never receive any traffic from sw0's ports,
> since we only have sw1's tag, and that one will say sw1.port1.
> 
> So while you may be able to configure vlans and forwarding, probably
> everything else won't work as expected. Like when you want to send out
> a packet on a certain port.
> 
> Though I assume this isn't a special issue for b53, and rather a
> common issue of chained switch chips, and b53 just acknowledges it /
> tries to work around it. I know that marvell (E)DSA can handle this,
> but I wouldn't be surprised if many others can't.

For the record, we have a non-proprietary DSA tagging infrastructure
(tag_8021q.c) which supports cross-chip bridging, used for 3 drivers
already. I know Florian has looked at it in the past, but due to various
reasons couldn't immediately find a way to make use of it for b53.
It could be interesting though.

> Looking at devices in OpenWrt that are affected by this, these device
> have exactly one user port on sw1, and that one is mostly used as a
> wan port, so as a stand-alone port, not a bridge port. One could argue
> here that proper functioning of the "outer" switch's ports is more
> important than being able to forward frames in hardware to the wan
> port.
> 
> Unfortunately I do not have a device at hand, might need to figure out
> if I can get my hands on one ... .

Generally, switches which support a cascading topology have the necessary
prerequisites taken care of at the tagging protocol level. Two switches
from the same vendor which are connected together are more likely to
form a single switch tree, and the switch ID from the packet indicates
which device from the tree does the packet belong to. There's a single
DSA tag in this situation.

On the other hand you have tag stacking, where a DSA user port is the
conduit of a downstream DSA switch. There are 2 single-switch trees in
that case. That also tends to work, usually, though you wouldn't expect
hardware-accelerated forwarding between the top and bottom switch
(software needs to perform tag adaptation for the general case).
There are recorded cases of tagging protocols "ocelot" and "ocelot-8021q"
working just fine as DSA conduits for "sja1105".

For the stacked b53 case, it looks like due to hardware limitations, the
tag stacking solution was partially adopted, but with b53 specificities.
You have hardware-accelerated cross-chip bridging (which you wouldn't
normally expect) and one of the switches has tagging protocol "none"
(which again you wouldn't expect). See if maybe a single tree with a
custom "brcm-8021q" tagging protocol is something feasible.

Powered by blists - more mailing lists