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
| ||
|
Message-ID: <20220718162434.72fqamkv4v274tny@skbuf> Date: Mon, 18 Jul 2022 16:24:35 +0000 From: Vladimir Oltean <vladimir.oltean@....com> To: "Arun.Ramadoss@...rochip.com" <Arun.Ramadoss@...rochip.com> CC: Claudiu Manoil <claudiu.manoil@....com>, "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>, "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>, "vivien.didelot@...il.com" <vivien.didelot@...il.com>, "andrew@...n.ch" <andrew@...n.ch>, "idosch@...dia.com" <idosch@...dia.com>, "linux@...pel-privat.de" <linux@...pel-privat.de>, "petrm@...dia.com" <petrm@...dia.com>, "f.fainelli@...il.com" <f.fainelli@...il.com>, "hauke@...ke-m.de" <hauke@...ke-m.de>, "martin.blumenstingl@...glemail.com" <martin.blumenstingl@...glemail.com>, Xiaoliang Yang <xiaoliang.yang_1@....com>, "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>, "edumazet@...gle.com" <edumazet@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Woojung.Huh@...rochip.com" <Woojung.Huh@...rochip.com>, "davem@...emloft.net" <davem@...emloft.net> Subject: Re: [RFC PATCH net-next 3/3] net: dsa: never skip VLAN configuration Hi Arun, On Mon, Jul 18, 2022 at 02:34:33PM +0000, Arun.Ramadoss@...rochip.com wrote: > Hi Vladimir, > There was a mistake in our testing on the latest code base of net-next. > Today we tried in the latest net-next and following are the > observation. > > Scenario 1: Before applying the patch > ------------------------------ > ip link set dev br0 type bridge vlan_filtering 0 > > bridge vlan add vid 10 dev lan1 pvid untagged > bridge vlan add vid 10 dev lan2 pvid untagged "bridge vlan add" on lan2 was never part of the test instructions. > > We got warning skipping configuration of VLAN and ksz_port_vlan_add() > is not called. > > Packet is received in Host2 when transmitted from Host1. So there is no > breakage in the forwarding. Nonetheless it's good to know that the control scenario behaves as expected, i.e. the VLANs are skipped while VLAN-unaware (and therefore, the port hardware PVID remains what it was). > Scenario 2: After applying the patch > ---------------------------- > ip link set dev br0 type bridge vlan_filtering 0 > > bridge vlan add vid 10 dev lan1 pvid untagged > > --> Packet is not received in the Host2 The problem is exactly here. The driver should pass packets at this stage. This is because the bridge is still VLAN-unaware, despite its VLAN-aware pvid VLAN having been changed from 1 (vlan_default_pvid) to 10. > bridge vlan add vid 10 dev lan2 pvid untagged > > --> packet is received in the Host2 This only goes to prove that the VLAN in which the switch processes traffic while VLAN-unaware is the PVID of the port. So when the PVID on the ingress and egress ports matches, forwarding is naturally restored. > bridge vlan del vid 10 dev lan1 > > --> packet is received in the Host2 > > bridge vlan del vid 10 dev lan2 > > --> packet is received in the Host2 > > * Let us know, do we need to test anything further on this. Yes, now you need to go fix the driver :) Please read the comments from this patch and the series in general (including cover letter), they point to similar issues in other drivers and to commits which have solved them. I need the ksz driver to work properly before I can delete the configure_vlan_while_not_filtering workaround. Generally speaking, the PVID needs to be committed to hardware based on a smarter logic, see this for example (and all the places from which it is called): static int mv88e6xxx_port_commit_pvid(struct mv88e6xxx_chip *chip, int port) { struct dsa_port *dp = dsa_to_port(chip->ds, port); struct net_device *br = dsa_port_bridge_dev_get(dp); struct mv88e6xxx_port *p = &chip->ports[port]; u16 pvid = MV88E6XXX_VID_STANDALONE; // Dedicated PVID for standalone mode bool drop_untagged = false; int err; if (br) { if (br_vlan_enabled(br)) { pvid = p->bridge_pvid.vid; // PVID is inherited from bridge only if the bridge is *currently* VLAN-aware drop_untagged = !p->bridge_pvid.valid; } else { pvid = MV88E6XXX_VID_BRIDGED; // Dedicated PVID for VLAN-unaware bridging } } err = mv88e6xxx_port_set_pvid(chip, port, pvid); if (err) return err; return mv88e6xxx_port_drop_untagged(chip, port, drop_untagged); } Additionally, DaveM has just merged some DSA documentation updates which I think are very relevant to this discussion. See the new "Address databases" chapter for a review of how things are supposed to actually work when done carefully: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/Documentation/networking/dsa/dsa.rst#n730 Thanks!
Powered by blists - more mailing lists