Re: [PATCH net 1/2] net: dsa: let drivers offload 8021q uppers on standalone ports

Hi Vladimir,

> Then their "vlan_filtering off" implementation is broken.

In the sense that the hardware cannot forward an arbitrary tagged frame
transparently while the table is active, and the table cannot be
deactivated without the ARL cost from the other subthread, yes. The
flag exists so the driver can compensate for exactly that.

> BTW, how is the "standalone port" behaviour different than the
> vlan_filtering=0 bridge port case? If as you say, the switch must have
> the VID in the VLAN table to send the packet to the CPU, what is
> different when that port is under a VLAN-unaware bridge such that this
> presumably does work?

It is not different, and it does not work. A tagged frame whose VID is
not in the table dies the same way when the port is under a
VLAN-unaware bridge; this hardware cannot do transparent tagged
bridging since the same v5.15 change. Jonas observed the same
limitation earlier in this thread from the other direction: on the
chips where standalone RX still works, forwarding tagged frames
between ports does not.

The reason the fix scopes to standalone ports is that they have a
finite, well-defined VID source: the 8021q uppers, reported through
the feature bit. A VLAN-unaware bridge has no such source; making it
transparent would mean programming the whole VID space, which is the
several-seconds-per-toggle variant Jonas measured and rejected in the
first thread. So the series fixes the reported regression, the
standalone PPPoE/upper case, and does not pretend to fix transparent
tagged bridging, which this hardware has not done since v5.15 either.

> If there is a problem with the vlan_filtering_is_global +
> needs_standalone_vlan_filtering combination, then hellcreek also suffers
> from it, because it does set both flags as well.

You are right, and the commit message argues this badly; I will reword
it if a v2 is wanted. vlan_filtering_is_global is not the
differentiator, hellcreek sets it too. The difference is what the
forced vlan_filtering=1 means for each driver. For hellcreek,
switch-wide VLAN awareness is the intended operating state; its
standalone traffic depends on filtering being on, and hellcreek.c
documents that unmanaged setups are not supported. The forced flip
lands it in the state it wants. For b53, vlan_filtering=1 is a
different user-visible mode for every port on the switch: untagged
frames become PVID-classified against the table, egress untagging
applies, unknown VIDs are dropped at ingress. Forcing that globally
because one port left a VLAN-unaware bridge would change the behaviour
of every other port, including members of VLAN-unaware bridges that
expect transparent operation. b53 needs the VIDs delivered while
vlan_filtering stays wherever the user put it, which is the narrower
flag.

> Why can't standalone ports tolerate the .port_vlan_filtering() call?

They do tolerate and still receive it: the ds->ops->port_vlan_filtering
call is unchanged, b53 sees every toggle and rebuilds its hardware
state from its own records. What the flag skips is only the core's
dsa_user_manage_vlan_filtering(), whose two jobs are wrong for a
switch whose feature bit is permanently on. On the way to
vlan_filtering=1 it replays VIDs that were never cleared, so
vlan_vid_add() refcounts every upper VID twice. On the way to 0 it
clears the VIDs and drops NETIF_F_HW_VLAN_CTAG_FILTER on a port that
happens to be bridged at toggle time; I measured that case on the
RT-N18U: after the port later leaves the bridge, its uppers stay dead
until reboot, because nothing re-offloads them once the feature bit is
gone. With the skip, both effects are gone and the driver derives the
hardware state from the flip itself. hellcreek does not set the new
flag, so its path through dsa_user_manage_vlan_filtering() is
unchanged.

Best regards,
Semih