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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Sep 2021 14:29:33 +0800
From:   DENG Qingfang <dqfext@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Vladimir Oltean <olteanv@...il.com>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        Mauri Sandberg <sandberg@...lfence.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>
Subject: 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 <olteanv@...il.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.