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

[DENG Qingfang <dqfext@gmail.com>]

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 <olteanv@gmail.com> wrote:
> 
> > > 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.
> 
> > 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.
> 
> > 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.