Re: [PATCH net-next 5/8] net: dsa: rtl8366: Disable "4K" VLANs

Hi Linus,

On Tue, Sep 14, 2021 at 01:20:14AM +0200, Linus Walleij wrote:

Hi Vladimir,

first, thanks for your help and patience. I learned a lot the recent
weeks, much thanks to your questions and explanations!

On Mon, Sep 13, 2021 at 5:34 PM Vladimir Oltean [off-list ref] wrote:

quoted

quoted

This was discovered when testing with OpenWrt that join
the LAN ports lan0 ... lan3 into a bridge and then assign
each of them into VLAN 1 with PVID set on each port: without
this patch this will not work and the bridge goes numb.

It is important to explain _why_ the switch will go "numb" and not pass
packets if the Linux bridge assigns all ports to VLAN ID 1 as pvid. It
is certainly not expected for that to happen.

Yeah it is pretty weird. What happens now is that this is a regression
when using OpenWrt userspace as it sets up the VLANs like this,

Were you running net-next kernel?

There have been major changes to DSA since 5.10, so you'd better test your
driver on net-next.

but if I boot a clean system and just manually do e.g.
ifconfig lan0 169.254.1.2 netmask 255.255.255.0 up
it works fine because the default VLANs that were set up by the
driver (removed by patch 2/8) will tag all packets using PVID and
send packets on 5 ingress and 1 egress VLANs.

quoted

The purpose of the PVID feature is specifically to classify untagged
packets to a port-based VLAN ID. So "everything is a VLAN" even for
Linux user space, not sure what you're talking about.

I think what happens is that OpenWrts userspace sets VLAN 1
for all ingress ports with PVID, so all packets from ingress ports
get tagged nicely with VID 1.

But as the CPU port is hidden inside the bridge
it can't join the CPU port into that VLAN (userspace does not
know it exist I think?) and thus no packets
can go into or out of the CPU port. But you can still pass packets
between the lan ports.

quoted

When the Linux bridge has the vlan_filtering attribute set to 1, the
hardware should follow suit by making untagged packets get classified to
the VLAN ID that the software bridge wants to see, on the ports that are
members of that bridge.

This is what it does, I think.

But the "4K" VLAN feature is so strict that it will restrict also the CPU
port from this (in hardware) with no way to turn it off.

It seems the "4K" mode is a "VLAN with filtering only mode" so no
matter whether we turned on filtering or not, the CPU port
will not see any packets from any other ports unless we add also
that port (port 5) into the VLAN.

One solution I could try would be to just add the CPU port to all
VLANs by default, but .. is that right?

The DSA core already adds the CPU port to VLAN members for you.
In file net/dsa/slave.c, function dsa_slave_vlan_add:
...
	err = dsa_port_vlan_add(dp, &vlan, extack);
	if (err)
		return err;

	/* We need the dedicated CPU port to be a member of the VLAN as well.
	 * Even though drivers often handle CPU membership in special ways,
	 * it doesn't make sense to program a PVID, so clear this flag.
	 */
	vlan.flags &= ~BRIDGE_VLAN_INFO_PVID;

	err = dsa_port_vlan_add(dp->cpu_dp, &vlan, extack);
	if (err)
		return err;
...

If it does not work, you may have done something wrong.