Re: [PATCH net-next 2/3] net: dsa: mv88e6xxx: fix duplicate vlan warning

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

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
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up