| Message ID | E1j41KQ-0002uy-TQ@rmk-PC.armlinux.org.uk |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | VLANs, DSA switches and multiple bridges | expand |
On Tue, Feb 18, 2020 at 11:46:14AM +0000, Russell King wrote: > When setting VLANs on DSA switches, the VLAN is added to both the port > concerned as well as the CPU port by dsa_slave_vlan_add(). If multiple > ports are configured with the same VLAN ID, this triggers a warning on > the CPU port. > > Avoid this warning for CPU ports. > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> Note that there is still something not right. On the ZII dev rev B, setting up a bridge across all the switch ports, I get: [ 29.526408] device lan0 entered promiscuous mode [ 29.812389] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 29.837032] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 29.855132] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 29.870246] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 30.201951] device lan1 entered promiscuous mode [ 30.217172] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.226663] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.235366] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.243355] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 30.251970] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.262759] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.271104] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.278437] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 30.444394] device lan2 entered promiscuous mode [ 30.459404] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.468032] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.476725] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.484390] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 30.493135] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.503920] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.512208] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.519546] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 30.685926] device lan3 entered promiscuous mode [ 30.697173] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.710499] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.719215] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.727069] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 30.735674] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.746472] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.755567] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.762909] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 30.931879] device lan4 entered promiscuous mode [ 30.942491] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.955862] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 30.965332] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 30.973110] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 30.981817] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 30.992666] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.001044] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.008382] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 31.180919] device lan5 entered promiscuous mode [ 31.191329] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.205077] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.213782] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.222598] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 31.231304] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.243705] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.254125] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.262043] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 31.449715] device lan6 entered promiscuous mode [ 31.459350] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.468345] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.477056] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.488690] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 31.497379] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.508209] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.516494] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.523956] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 31.670892] device lan7 entered promiscuous mode [ 31.680315] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.689037] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.698480] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.709288] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 31.718807] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.729622] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.737996] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.745465] mv88e6085 0.4:00: p9: already a member of VLAN 1 ... [ 31.890353] device lan8 entered promiscuous mode [ 31.899985] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.909517] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.918255] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.928986] mv88e6085 0.4:00: p9: already a member of VLAN 1 [ 31.937703] mv88e6085 0.1:00: p5: already a member of VLAN 1 [ 31.948657] mv88e6085 0.2:00: p5: already a member of VLAN 1 [ 31.957027] mv88e6085 0.2:00: p6: already a member of VLAN 1 [ 31.964368] mv88e6085 0.4:00: p9: already a member of VLAN 1 So there's still something not right. The setup is not non-standard, it's just a bridge across all the lan ports. My guess is this patch needs to be extended to DSA ports as well as CPU ports? > --- > drivers/net/dsa/mv88e6xxx/chip.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c > index 4ec09cc8dcdc..629eb7bbbb23 100644 > --- a/drivers/net/dsa/mv88e6xxx/chip.c > +++ b/drivers/net/dsa/mv88e6xxx/chip.c > @@ -1795,7 +1795,7 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid) > } > > static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port, > - u16 vid, u8 member) > + u16 vid, u8 member, bool warn) > { > const u8 non_member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER; > struct mv88e6xxx_vtu_entry vlan; > @@ -1840,7 +1840,7 @@ static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port, > err = mv88e6xxx_vtu_loadpurge(chip, &vlan); > if (err) > return err; > - } else { > + } else if (warn) { > dev_info(chip->dev, "p%d: already a member of VLAN %d\n", > port, vid); > } > @@ -1854,6 +1854,7 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, > struct mv88e6xxx_chip *chip = ds->priv; > bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; > bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; > + bool warn; > u8 member; > u16 vid; > > @@ -1867,10 +1868,15 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, > else > member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_TAGGED; > > + /* net/dsa/slave.c will call dsa_port_vlan_add() for the affected port > + * and then the CPU port. Do not warn for duplicates for the CPU port. > + */ > + warn = !dsa_is_cpu_port(ds, port); > + > mv88e6xxx_reg_lock(chip); > > for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) > - if (mv88e6xxx_port_vlan_join(chip, port, vid, member)) > + if (mv88e6xxx_port_vlan_join(chip, port, vid, member, warn)) > dev_err(ds->dev, "p%d: failed to add VLAN %d%c\n", port, > vid, untagged ? 'u' : 't'); > > -- > 2.20.1 > >
On Tue, Feb 18, 2020 at 11:51:57AM +0000, Russell King - ARM Linux admin wrote: > On Tue, Feb 18, 2020 at 11:46:14AM +0000, Russell King wrote: > > When setting VLANs on DSA switches, the VLAN is added to both the port > > concerned as well as the CPU port by dsa_slave_vlan_add(). If multiple > > ports are configured with the same VLAN ID, this triggers a warning on > > the CPU port. > > > > Avoid this warning for CPU ports. > > > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > Note that there is still something not right. On the ZII dev rev B, > setting up a bridge across all the switch ports, I get: Hi Russell FYI: You need to be a little careful with VLANs on rev B. The third switch does not have the PVT hardware. So VLANs are going to 'leak' when they cross the DSA link to that switch. I will look at these patches later today. Andrew
On Tue, Feb 18, 2020 at 05:27:50PM +0100, Andrew Lunn wrote: > On Tue, Feb 18, 2020 at 11:51:57AM +0000, Russell King - ARM Linux admin wrote: > > On Tue, Feb 18, 2020 at 11:46:14AM +0000, Russell King wrote: > > > When setting VLANs on DSA switches, the VLAN is added to both the port > > > concerned as well as the CPU port by dsa_slave_vlan_add(). If multiple > > > ports are configured with the same VLAN ID, this triggers a warning on > > > the CPU port. > > > > > > Avoid this warning for CPU ports. > > > > > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > > > Note that there is still something not right. On the ZII dev rev B, > > setting up a bridge across all the switch ports, I get: > > Hi Russell > > FYI: You need to be a little careful with VLANs on rev B. The third > switch does not have the PVT hardware. So VLANs are going to 'leak' > when they cross the DSA link to that switch. However, I'm not using VLAN configuration on any of the ZII boards. As I stated, I'm just setting up a bridge. It isn't even vlan aware: iface br0 inet manual bridge-ports lan0 lan1 lan2 lan3 lan4 lan5 lan6 lan7 lan8 bridge-maxwait 0
On Tue, Feb 18, 2020 at 05:27:50PM +0100, Andrew Lunn wrote: > On Tue, Feb 18, 2020 at 11:51:57AM +0000, Russell King - ARM Linux admin wrote: > > On Tue, Feb 18, 2020 at 11:46:14AM +0000, Russell King wrote: > > > When setting VLANs on DSA switches, the VLAN is added to both the port > > > concerned as well as the CPU port by dsa_slave_vlan_add(). If multiple > > > ports are configured with the same VLAN ID, this triggers a warning on > > > the CPU port. > > > > > > Avoid this warning for CPU ports. > > > > > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> > > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> > > > > Note that there is still something not right. On the ZII dev rev B, > > setting up a bridge across all the switch ports, I get: > > Hi Russell > > FYI: You need to be a little careful with VLANs on rev B. The third > switch does not have the PVT hardware. So VLANs are going to 'leak' > when they cross the DSA link to that switch. I'm not sure I fully understand what you're saying or the mechanism behind it. From what I can see, the 88E6352 and the 88E6185 both contain a VTU which is capable of taking an ingressing frame and restricting which ports it can egress from. If a frame ingresses on one 88E6352, passed across to the other 88E6352, and finally to the 88E6185, doesn't each switch look up in its own VTU which ports to egress the packet from, which should include the DSA ports, so it can then be passed to the other switches? And doesn't the VTU on each switch define which ports the frame is allowed to egress out of? From what I can see, setting up a bridge across all lan ports on the Zii rev B, then enabling vlan filtering, and then allowing VID V on lan0 and lan8 (one port on each 88E6352, passed across to the other 88E6352, and finally to the 88E6185 which has the other port on) results in VID V frames passed correctly across, and are received appropriately. Untagged traffic continues to be received appropriately. Removing VID V from lan8 (the port I'm monitoring) results in VID V traffic no longer sent out via lan8. So, it seems to work as one would expect. What am I missing?
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 4ec09cc8dcdc..629eb7bbbb23 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1795,7 +1795,7 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid) } static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port, - u16 vid, u8 member) + u16 vid, u8 member, bool warn) { const u8 non_member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER; struct mv88e6xxx_vtu_entry vlan; @@ -1840,7 +1840,7 @@ static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port, err = mv88e6xxx_vtu_loadpurge(chip, &vlan); if (err) return err; - } else { + } else if (warn) { dev_info(chip->dev, "p%d: already a member of VLAN %d\n", port, vid); } @@ -1854,6 +1854,7 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, struct mv88e6xxx_chip *chip = ds->priv; bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED; bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID; + bool warn; u8 member; u16 vid; @@ -1867,10 +1868,15 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, else member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_TAGGED; + /* net/dsa/slave.c will call dsa_port_vlan_add() for the affected port + * and then the CPU port. Do not warn for duplicates for the CPU port. + */ + warn = !dsa_is_cpu_port(ds, port); + mv88e6xxx_reg_lock(chip); for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) - if (mv88e6xxx_port_vlan_join(chip, port, vid, member)) + if (mv88e6xxx_port_vlan_join(chip, port, vid, member, warn)) dev_err(ds->dev, "p%d: failed to add VLAN %d%c\n", port, vid, untagged ? 'u' : 't');