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

[Semih Baskan]

Hi Vladimir,

> But it doesn't, at least not in a sane way. If it still only accepts
> those VLANs that have been added to filters by higher layers, it's not
> VLAN-unaware. If a solution is not found to the problem, the driver must
> reject operation as VLAN-unaware.

You are right that the port is not VLAN-unaware. It cannot be: the VID
lookup on this silicon cannot be turned off without the ARL loss from
the other subthread. My mistake in the previous mail was arguing around
that instead of saying it.

Let me first correct a history error I made, because the accurate
version actually simplifies the discussion.

> I fail to see how commit 06cfb2df7eb0 ("net: dsa: don't advertise
> 'rx-vlan-filter' when not needed") could have caused a regression in
> VLAN-unaware bridging on your b53 switch.

Correct, it could not, and my sentence attributing transparent bridging
to the same change was wrong. b53 has had dev->vlan_enabled = true at
switch allocation since before v5.15, and b53_vlan_filtering() has
never turned the table off. So arbitrary-VID transparency was already
impossible in v5.14, the last working kernel of the bug report. There
was no coincidence and no bridging regression: what worked in v5.14
and stopped in v5.15 is exactly one pipeline, the standalone 8021q
upper. In v5.14 the feature bit was advertised unconditionally, the
VIDs were delivered, and .port_vlan_add programmed them into the
always-on table. 06cfb2df7eb0 stopped the delivery and f089652b6b16
stopped the programming. The series restores that v5.14 pipeline for
drivers that opt in, and nothing else. That is why the Fixes tag
points where it points.

> ..which you just said earlier that they don't work either way?!

You are right, that sentence conflated two things. Forcing
vlan_filtering=1 globally would not hurt tagged transparency, which is
dead either way. What it changes is what works today: untagged
forwarding becomes subject to bridge VLAN semantics the user never
configured, the reported vlan_filtering state of every bridge on the
switch flips under the user, and b53 switches its ingress mode to
VC4_ING_VID_VIO_DROP plus VC5_DROP_VTABLE_MISS chip-wide.

> The only distinction I'm seeing is that in hellcreek, VLAN-unaware mode
> works, and in your b53 model it doesn't.

Yes, and I think the three reasons you listed in 06cfb2df7eb0's commit
message describe the difference better than my commit message did.
hellcreek is case 3: its hardware can be VLAN-unaware, but standalone
separation depends on unique VLANs, so its flag also forces the
vlan_filtering state, and that forcing is correct there. b53 after
v5.14 is case 1, "the standalone ports would otherwise drop
VLAN-tagged traffic", with one difference: in case 1 the VLAN
awareness that causes the dropping is held on by a VLAN-aware bridge
elsewhere on the switch, while on b53 it is held on by the silicon
itself, permanently. The new flag registers exactly that: a driver
whose ports always satisfy case 1's condition, with no bridge
required. It deliberately does not force the vlan_filtering state the
way needs_standalone_vlan_filtering does, because that forcing solves
hellcreek's separation problem, which b53 does not have.

> You need to skip dsa_user_manage_vlan_filtering() because you want to
> keep VLAN filters you need while lying to higher layers that you don't
> need them.

I would describe it as the opposite. 'rx-vlan-filter: on' is the true
statement about this hardware: it filters, always, and cannot do
otherwise. What lies to higher layers is the current mainline
behaviour, which reports the port as not filtering and then drops
every unsubscribed VID in silicon. The flag makes the feature bit
match the silicon; the skip then only stops the core from toggling a
bit that describes an invariant property.

> With vlan_filtering/NETIF_F_HW_VLAN_CTAG_FILTER set to 0, no one *has*
> to reoffload the VLAN filters, because the hardware shouldn't need them.

Agreed, in the software model no one has to. The measurement was not an
argument that the software model is wrong; it shows what breaks
mechanically if the flag advertises the feature permanently but the
core still clears it on toggles. It is internal consistency of this
approach, not a requirement I claim the model imposes.

Which leaves the real question, what to do with hardware like this.
I see three options.

1. Reject VLAN-unaware operation, as you suggest. Honest, but on b53 it
   refuses the default configuration of every deployed OpenWrt board on
   this platform, where the LAN bridge is vlan_filtering=0. That turns
   a working untagged setup into a broken one on a kernel update.

2. Force VLAN awareness, the hellcreek way. No new core code, but it
   flips the reported vlan_filtering state under the user, changes
   untagged handling switch-wide, and enables the drop modes above on
   every port. Whether that cost is acceptable for this user base is
   really a question for Jonas and Florian.

3. This series: report the filtering that the silicon actually does,
   restore the v5.14 delivery pipeline behind an opt-in, and change
   nothing else. Untagged setups keep working unchanged, VLAN-unaware
   bridges keep their (already partial) behaviour unchanged, and the
   one regression users actually reported is fixed in a way that can
   go to stable.

I implemented 3 because it is the only one where no deployed
configuration changes behaviour. If the consensus is that 1 or 2 is
the right model for b53 despite the fallout, I will help make and test
that on the hardware I have, but I do not think it can be the net fix
for the regression.

Best regards,
Semih