[PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

Hi,

This is a repost of the previously posted RFC back in December, which
did not get fully reviewed.  I've dropped the RFC tag this time as no
one really found anything too problematical in the RFC posting.

I've been trying to configure DSA for VLANs and not having much success.
The setup is quite simple:

- The main network is untagged
- The wifi network is a vlan tagged with id $VN running over the main
  network.

I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
setup to provide wifi access to the vlan $VN network, while the switch
is also part of the main network.

However, I'm encountering problems:

1) vlan support in DSA has a different behaviour from the Linux
   software bridge implementation.

    # bridge vlan
    port    vlan ids
    lan1     1 PVID Egress Untagged
    ...

   shows the default setup - the bridge ports are all configured for
   vlan 1, untagged egress, and vlan 1 as the port vid.  Issuing:

    # ip li set dev br0 type bridge vlan_filtering 1

   with no other vlan configuration commands on a Linux software bridge
   continues to allow untagged traffic to flow across the bridge.
   
   This difference in behaviour is because the MV88E6xxx VTU is
   completely empty - because net/dsa ignores all vlan settings for
   a port if br_vlan_enabled(dp->bridge_dev) is false - this reflects
   the vlan filtering state of the bridge, not whether the bridge is
   vlan aware.
   
   What this means is that attempting to configure the bridge port
   vlans before enabling vlan filtering works for Linux software
   bridges, but fails for DSA bridges.

2) Assuming the above is sorted, we move on to the next issue, which
   is altogether more weird.  Let's take a setup where we have a
   DSA bridge with lan1..6 in a bridge device, br0, with vlan
   filtering enabled.  lan1 is the upstream port, lan2 is a downstream
   port that also wants to see traffic on vlan id $VN.

   Both lan1 and lan2 are configured for that:

     # bridge vlan add vid $VN dev lan1
     # bridge vlan add vid $VN dev lan2
     # ip li set br0 type bridge vlan_filtering 1

   Untagged traffic can now pass between all the six lan ports, and
   vlan $VN between lan1 and lan2 only.  The MV88E6xxx 8021q_mode
   debugfs file shows all lan ports are in mode "secure" - this is
   important!  /sys/class/net/br0/bridge/vlan_filtering contains 1.

   tcpdumping from another machine on lan4 shows that no $VN traffic
   reaches it.  Everything seems to be working correctly...
   
   In order to further bridge vlan $VN traffic to hostapd's wifi
   interface, things get a little more complex - we can't add hostapd's
   wifi interface to br0 directly, because hostapd will bring up the
   wifi interface and leak the main, untagged traffic onto the wifi.
   (hostapd does have vlan support, but only as a dynamic per-client
   thing, and there's no hooks I can see to allow script-based config
   of the network setup before hostapd up's the wifi interface.)

   So, what I tried was:

     # ip li add link br0 name br0.$VN type vlan id $VN
     # bridge vlan add vid $VN dev br0 self
     # ip li set dev br0.$VN up

   So far so good, we get a vlan interface on top of the bridge, and
   tcpdumping it shows we get traffic.  The 8021q_mode file has not
   changed state.  Everything still seems to be correct.

     # bridge addbr br1

   Still nothing has changed.

     # bridge addif br1 br0.$VN

   And now the 8021q_mode debugfs file shows that all ports are now in
   "disabled" mode, but /sys/class/net/br0/bridge/vlan_filtering still
   contains '1'.  In other words, br0 still thinks vlan filtering is
   enabled, but the hardware has had vlan filtering disabled.

   Adding some stack traces to an appropriate point indicates that this
   is because __switchdev_handle_port_attr_set() recurses down through
   the tree of interfaces, skipping over the vlan interface, applying
   br1's configuration to br0's ports.

   This surely can not be right - surely
   __switchdev_handle_port_attr_set() and similar should stop recursing
   down through another master bridge device?  There are probably other
   network device classes that switchdev shouldn't recurse down too.

   I've considered whether switchdev is the right level to do it, and
   I think it is - as we want the check/set callbacks to be called for
   the top level device even if it is a master bridge device, but we
   don't want to recurse through a lower master bridge device.

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

[PATCH net-next 1/3] net: switchdev: do not propagate bridge updates across bridges

[Russell King]

When configuring a tree of independent bridges, propagating changes
from the upper bridge across a bridge master to the lower bridge
ports brings surprises.

For example, a lower bridge may have vlan filtering enabled.  It
may have a vlan interface attached to the bridge master, which may
then be incorporated into another bridge.  As soon as the lower
bridge vlan interface is attached to the upper bridge, the lower
bridge has vlan filtering disabled.

This occurs because switchdev recursively applies its changes to
all lower devices no matter what.

Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Tested-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 net/switchdev/switchdev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 60630762a748..f25604d68337 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -475,6 +475,9 @@ static int __switchdev_handle_port_obj_add(struct net_device *dev,
 	 * necessary to go through this helper.
 	 */
 	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		if (netif_is_bridge_master(lower_dev))
+			continue;
+
 		err = __switchdev_handle_port_obj_add(lower_dev, port_obj_info,
 						      check_cb, add_cb);
 		if (err && err != -EOPNOTSUPP)
@@ -526,6 +529,9 @@ static int __switchdev_handle_port_obj_del(struct net_device *dev,
 	 * necessary to go through this helper.
 	 */
 	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		if (netif_is_bridge_master(lower_dev))
+			continue;
+
 		err = __switchdev_handle_port_obj_del(lower_dev, port_obj_info,
 						      check_cb, del_cb);
 		if (err && err != -EOPNOTSUPP)
@@ -576,6 +582,9 @@ static int __switchdev_handle_port_attr_set(struct net_device *dev,
 	 * necessary to go through this helper.
 	 */
 	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		if (netif_is_bridge_master(lower_dev))
+			continue;
+
 		err = __switchdev_handle_port_attr_set(lower_dev, port_attr_info,
 						       check_cb, set_cb);
 		if (err && err != -EOPNOTSUPP)
-- 
2.20.1

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

[Russell King]

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

[PATCH net-next 3/3] net: dsa: mv88e6xxx: fix vlan setup

[Russell King]

DSA assumes that a bridge which has vlan filtering disabled is not
vlan aware, and ignores all vlan configuration. However, the kernel
software bridge code allows configuration in this state.

This causes the kernel's idea of the bridge vlan state and the
hardware state to disagree, so "bridge vlan show" indicates a correct
configuration but the hardware lacks all configuration. Even worse,
enabling vlan filtering on a DSA bridge immediately blocks all traffic
which, given the output of "bridge vlan show", is very confusing.

Provide an option that drivers can set to indicate they want to receive
vlan configuration even when vlan filtering is disabled. This is safe
for Marvell DSA bridges, which do not look up ingress traffic in the
VTU if the port is in 8021Q disabled state. Whether this change is
suitable for all DSA bridges is not known.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/dsa/mv88e6xxx/chip.c |  1 +
 include/net/dsa.h                |  1 +
 net/dsa/slave.c                  | 12 ++++++++----
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 629eb7bbbb23..e656e571ef7d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2934,6 +2934,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 
 	chip->ds = ds;
 	ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
+	ds->vlan_bridge_vtu = true;
 
 	mv88e6xxx_reg_lock(chip);
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 63495e3443ac..d3a826646e8e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -273,6 +273,7 @@ struct dsa_switch {
 	 * settings on ports if not hardware-supported
 	 */
 	bool			vlan_filtering_is_global;
+	bool			vlan_bridge_vtu;
 
 	/* In case vlan_filtering_is_global is set, the VLAN awareness state
 	 * should be retrieved from here and not from the per-port settings.
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 088c886e609e..534d511b349e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -318,7 +318,8 @@ static int dsa_slave_vlan_add(struct net_device *dev,
 	if (obj->orig_dev != dev)
 		return -EOPNOTSUPP;
 
-	if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
+	if (dp->bridge_dev && !dp->ds->vlan_bridge_vtu &&
+	    !br_vlan_enabled(dp->bridge_dev))
 		return 0;
 
 	vlan = *SWITCHDEV_OBJ_PORT_VLAN(obj);
@@ -385,7 +386,8 @@ static int dsa_slave_vlan_del(struct net_device *dev,
 	if (obj->orig_dev != dev)
 		return -EOPNOTSUPP;
 
-	if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
+	if (dp->bridge_dev && !dp->ds->vlan_bridge_vtu &&
+	    !br_vlan_enabled(dp->bridge_dev))
 		return 0;
 
 	/* Do not deprogram the CPU port as it may be shared with other user
@@ -1106,7 +1108,8 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
 	 * need to emulate the switchdev prepare + commit phase.
 	 */
 	if (dp->bridge_dev) {
-		if (!br_vlan_enabled(dp->bridge_dev))
+		if (!dp->ds->vlan_bridge_vtu &&
+		    !br_vlan_enabled(dp->bridge_dev))
 			return 0;
 
 		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
@@ -1140,7 +1143,8 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
 	 * need to emulate the switchdev prepare + commit phase.
 	 */
 	if (dp->bridge_dev) {
-		if (!br_vlan_enabled(dp->bridge_dev))
+		if (!dp->ds->vlan_bridge_vtu &&
+		    !br_vlan_enabled(dp->bridge_dev))
 			return 0;
 
 		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
-- 
2.20.1

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

[Russell King - ARM Linux admin]

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

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

[Andrew Lunn]

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

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

[Russell King - ARM Linux admin]

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

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

Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: fix vlan setup

[Vivien Didelot]

Hi Russell,

On Tue, 18 Feb 2020 11:46:20 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> DSA assumes that a bridge which has vlan filtering disabled is not
> vlan aware, and ignores all vlan configuration. However, the kernel
> software bridge code allows configuration in this state.
> 
> This causes the kernel's idea of the bridge vlan state and the
> hardware state to disagree, so "bridge vlan show" indicates a correct
> configuration but the hardware lacks all configuration. Even worse,
> enabling vlan filtering on a DSA bridge immediately blocks all traffic
> which, given the output of "bridge vlan show", is very confusing.
> 
> Provide an option that drivers can set to indicate they want to receive
> vlan configuration even when vlan filtering is disabled. This is safe
> for Marvell DSA bridges, which do not look up ingress traffic in the
> VTU if the port is in 8021Q disabled state. Whether this change is
> suitable for all DSA bridges is not known.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c |  1 +
>  include/net/dsa.h                |  1 +
>  net/dsa/slave.c                  | 12 ++++++++----
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 629eb7bbbb23..e656e571ef7d 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2934,6 +2934,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>  
>  	chip->ds = ds;
>  	ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
> +	ds->vlan_bridge_vtu = true;
>  
>  	mv88e6xxx_reg_lock(chip);
>  
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 63495e3443ac..d3a826646e8e 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -273,6 +273,7 @@ struct dsa_switch {
>  	 * settings on ports if not hardware-supported
>  	 */
>  	bool			vlan_filtering_is_global;
> +	bool			vlan_bridge_vtu;
>  
>  	/* In case vlan_filtering_is_global is set, the VLAN awareness state
>  	 * should be retrieved from here and not from the per-port settings.
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 088c886e609e..534d511b349e 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -318,7 +318,8 @@ static int dsa_slave_vlan_add(struct net_device *dev,
>  	if (obj->orig_dev != dev)
>  		return -EOPNOTSUPP;
>  
> -	if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
> +	if (dp->bridge_dev && !dp->ds->vlan_bridge_vtu &&
> +	    !br_vlan_enabled(dp->bridge_dev))
>  		return 0;
>  
>  	vlan = *SWITCHDEV_OBJ_PORT_VLAN(obj);
> @@ -385,7 +386,8 @@ static int dsa_slave_vlan_del(struct net_device *dev,
>  	if (obj->orig_dev != dev)
>  		return -EOPNOTSUPP;
>  
> -	if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
> +	if (dp->bridge_dev && !dp->ds->vlan_bridge_vtu &&
> +	    !br_vlan_enabled(dp->bridge_dev))
>  		return 0;
>  
>  	/* Do not deprogram the CPU port as it may be shared with other user
> @@ -1106,7 +1108,8 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
>  	 * need to emulate the switchdev prepare + commit phase.
>  	 */
>  	if (dp->bridge_dev) {
> -		if (!br_vlan_enabled(dp->bridge_dev))
> +		if (!dp->ds->vlan_bridge_vtu &&
> +		    !br_vlan_enabled(dp->bridge_dev))
>  			return 0;
>  
>  		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
> @@ -1140,7 +1143,8 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
>  	 * need to emulate the switchdev prepare + commit phase.
>  	 */
>  	if (dp->bridge_dev) {
> -		if (!br_vlan_enabled(dp->bridge_dev))
> +		if (!dp->ds->vlan_bridge_vtu &&
> +		    !br_vlan_enabled(dp->bridge_dev))
>  			return 0;
>  
>  		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the

It is confusing to add a Marvell specific term (VTU) in the generic dsa_switch
structure to bypass the fact that VLAN configuration in hardware was already
bypassed for some reasons until vlan filtering is turned on. As you said,
simply offloading the VLAN configuration in hardware and only turning the
ports' 802.1Q mode to Secure once vlan_filtering is flipped to 1 should work
in theory for both VLAN filtering aware and unaware scenarios, but this was
causing problems if I'm not mistaken, I'll try to dig this out.

In the meantime, does the issue you're trying to solve here happens if you
create a vlan-filtering aware bridge in the first place, before any VLAN
configuration? i.e.:

    # ip link add name br0 type bridge vlan_filtering 1
    # ip link set master br0 dev lan1 up
    # bridge vlan add ...


Thank you,
Vivien

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Florian Fainelli]

On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> Hi,
> 
> This is a repost of the previously posted RFC back in December, which
> did not get fully reviewed.  I've dropped the RFC tag this time as no
> one really found anything too problematical in the RFC posting.
> 
> I've been trying to configure DSA for VLANs and not having much success.
> The setup is quite simple:
> 
> - The main network is untagged
> - The wifi network is a vlan tagged with id $VN running over the main
>   network.
> 
> I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> setup to provide wifi access to the vlan $VN network, while the switch
> is also part of the main network.
> 
> However, I'm encountering problems:
> 
> 1) vlan support in DSA has a different behaviour from the Linux
>    software bridge implementation.
> 
>     # bridge vlan
>     port    vlan ids
>     lan1     1 PVID Egress Untagged
>     ...
> 
>    shows the default setup - the bridge ports are all configured for
>    vlan 1, untagged egress, and vlan 1 as the port vid.  Issuing:
> 
>     # ip li set dev br0 type bridge vlan_filtering 1
> 
>    with no other vlan configuration commands on a Linux software bridge
>    continues to allow untagged traffic to flow across the bridge.
>    
>    This difference in behaviour is because the MV88E6xxx VTU is
>    completely empty - because net/dsa ignores all vlan settings for
>    a port if br_vlan_enabled(dp->bridge_dev) is false - this reflects
>    the vlan filtering state of the bridge, not whether the bridge is
>    vlan aware.
>    
>    What this means is that attempting to configure the bridge port
>    vlans before enabling vlan filtering works for Linux software
>    bridges, but fails for DSA bridges.

FWIW, because VLAN filtering is global with b53 switches, we do actually
program every port with a valid VLAN entry into VID 0 which ensures that
standalone (non-bridged) ports, with, or without VLAN interfaces on top
(e.g.: sw0p0.N) continue to work as soon as another port gets enslaved
into a bridge that does request vlan_filtering.

> 
> 2) Assuming the above is sorted, we move on to the next issue, which
>    is altogether more weird.  Let's take a setup where we have a
>    DSA bridge with lan1..6 in a bridge device, br0, with vlan
>    filtering enabled.  lan1 is the upstream port, lan2 is a downstream
>    port that also wants to see traffic on vlan id $VN.
> 
>    Both lan1 and lan2 are configured for that:
> 
>      # bridge vlan add vid $VN dev lan1
>      # bridge vlan add vid $VN dev lan2
>      # ip li set br0 type bridge vlan_filtering 1
> 
>    Untagged traffic can now pass between all the six lan ports, and
>    vlan $VN between lan1 and lan2 only.  The MV88E6xxx 8021q_mode
>    debugfs file shows all lan ports are in mode "secure" - this is
>    important!  /sys/class/net/br0/bridge/vlan_filtering contains 1.
> 
>    tcpdumping from another machine on lan4 shows that no $VN traffic
>    reaches it.  Everything seems to be working correctly...
>    
>    In order to further bridge vlan $VN traffic to hostapd's wifi
>    interface, things get a little more complex - we can't add hostapd's
>    wifi interface to br0 directly, because hostapd will bring up the
>    wifi interface and leak the main, untagged traffic onto the wifi.
>    (hostapd does have vlan support, but only as a dynamic per-client
>    thing, and there's no hooks I can see to allow script-based config
>    of the network setup before hostapd up's the wifi interface.)
> 
>    So, what I tried was:
> 
>      # ip li add link br0 name br0.$VN type vlan id $VN
>      # bridge vlan add vid $VN dev br0 self
>      # ip li set dev br0.$VN up
> 
>    So far so good, we get a vlan interface on top of the bridge, and
>    tcpdumping it shows we get traffic.  The 8021q_mode file has not
>    changed state.  Everything still seems to be correct.
> 
>      # bridge addbr br1
> 
>    Still nothing has changed.
> 
>      # bridge addif br1 br0.$VN
> 
>    And now the 8021q_mode debugfs file shows that all ports are now in
>    "disabled" mode, but /sys/class/net/br0/bridge/vlan_filtering still
>    contains '1'.  In other words, br0 still thinks vlan filtering is
>    enabled, but the hardware has had vlan filtering disabled.
> 
>    Adding some stack traces to an appropriate point indicates that this
>    is because __switchdev_handle_port_attr_set() recurses down through
>    the tree of interfaces, skipping over the vlan interface, applying
>    br1's configuration to br0's ports.
> 
>    This surely can not be right - surely
>    __switchdev_handle_port_attr_set() and similar should stop recursing
>    down through another master bridge device?  There are probably other
>    network device classes that switchdev shouldn't recurse down too.
> 
>    I've considered whether switchdev is the right level to do it, and
>    I think it is - as we want the check/set callbacks to be called for
>    the top level device even if it is a master bridge device, but we
>    don't want to recurse through a lower master bridge device.
> 


-- 
Florian

Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: fix vlan setup

[Florian Fainelli]

On 2/18/20 11:09 AM, Vivien Didelot wrote:
> Hi Russell,
> 
> On Tue, 18 Feb 2020 11:46:20 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
>> DSA assumes that a bridge which has vlan filtering disabled is not
>> vlan aware, and ignores all vlan configuration. However, the kernel
>> software bridge code allows configuration in this state.
>>
>> This causes the kernel's idea of the bridge vlan state and the
>> hardware state to disagree, so "bridge vlan show" indicates a correct
>> configuration but the hardware lacks all configuration. Even worse,
>> enabling vlan filtering on a DSA bridge immediately blocks all traffic
>> which, given the output of "bridge vlan show", is very confusing.
>>
>> Provide an option that drivers can set to indicate they want to receive
>> vlan configuration even when vlan filtering is disabled. This is safe
>> for Marvell DSA bridges, which do not look up ingress traffic in the
>> VTU if the port is in 8021Q disabled state. Whether this change is
>> suitable for all DSA bridges is not known.
>>
>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>> ---
>>  drivers/net/dsa/mv88e6xxx/chip.c |  1 +
>>  include/net/dsa.h                |  1 +
>>  net/dsa/slave.c                  | 12 ++++++++----
>>  3 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 629eb7bbbb23..e656e571ef7d 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -2934,6 +2934,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
>>  
>>  	chip->ds = ds;
>>  	ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
>> +	ds->vlan_bridge_vtu = true;
>>  
>>  	mv88e6xxx_reg_lock(chip);
>>  
>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>> index 63495e3443ac..d3a826646e8e 100644
>> --- a/include/net/dsa.h
>> +++ b/include/net/dsa.h
>> @@ -273,6 +273,7 @@ struct dsa_switch {
>>  	 * settings on ports if not hardware-supported
>>  	 */
>>  	bool			vlan_filtering_is_global;
>> +	bool			vlan_bridge_vtu;
>>  
>>  	/* In case vlan_filtering_is_global is set, the VLAN awareness state
>>  	 * should be retrieved from here and not from the per-port settings.
>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>> index 088c886e609e..534d511b349e 100644
>> --- a/net/dsa/slave.c
>> +++ b/net/dsa/slave.c
>> @@ -318,7 +318,8 @@ static int dsa_slave_vlan_add(struct net_device *dev,
>>  	if (obj->orig_dev != dev)
>>  		return -EOPNOTSUPP;
>>  
>> -	if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
>> +	if (dp->bridge_dev && !dp->ds->vlan_bridge_vtu &&
>> +	    !br_vlan_enabled(dp->bridge_dev))
>>  		return 0;
>>  
>>  	vlan = *SWITCHDEV_OBJ_PORT_VLAN(obj);
>> @@ -385,7 +386,8 @@ static int dsa_slave_vlan_del(struct net_device *dev,
>>  	if (obj->orig_dev != dev)
>>  		return -EOPNOTSUPP;
>>  
>> -	if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
>> +	if (dp->bridge_dev && !dp->ds->vlan_bridge_vtu &&
>> +	    !br_vlan_enabled(dp->bridge_dev))
>>  		return 0;
>>  
>>  	/* Do not deprogram the CPU port as it may be shared with other user
>> @@ -1106,7 +1108,8 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
>>  	 * need to emulate the switchdev prepare + commit phase.
>>  	 */
>>  	if (dp->bridge_dev) {
>> -		if (!br_vlan_enabled(dp->bridge_dev))
>> +		if (!dp->ds->vlan_bridge_vtu &&
>> +		    !br_vlan_enabled(dp->bridge_dev))
>>  			return 0;
>>  
>>  		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
>> @@ -1140,7 +1143,8 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
>>  	 * need to emulate the switchdev prepare + commit phase.
>>  	 */
>>  	if (dp->bridge_dev) {
>> -		if (!br_vlan_enabled(dp->bridge_dev))
>> +		if (!dp->ds->vlan_bridge_vtu &&
>> +		    !br_vlan_enabled(dp->bridge_dev))
>>  			return 0;
>>  
>>  		/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
> 
> It is confusing to add a Marvell specific term (VTU) in the generic dsa_switch
> structure to bypass the fact that VLAN configuration in hardware was already
> bypassed for some reasons until vlan filtering is turned on. As you said,
> simply offloading the VLAN configuration in hardware and only turning the
> ports' 802.1Q mode to Secure once vlan_filtering is flipped to 1 should work
> in theory for both VLAN filtering aware and unaware scenarios, but this was
> causing problems if I'm not mistaken, I'll try to dig this out.
> 
> In the meantime, does the issue you're trying to solve here happens if you
> create a vlan-filtering aware bridge in the first place, before any VLAN
> configuration? i.e.:
> 
>     # ip link add name br0 type bridge vlan_filtering 1
>     # ip link set master br0 dev lan1 up
>     # bridge vlan add ...

That will work okay because then you do get around the restrictions
added by 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa: Don't add
vlans when vlan filtering is disabled") and you will get VLAN objects
programming request to flow down your DSA driver. AFAICT, mlxsw
specifically prevents to toggle vlan_filtering at runtime for that
reason, because the VLAN objects notification are not "sent again" while
you toggle vlan_filtering. I really wonder if we should not do the same
in DSA as runtime toggling is of questionable use beyond just testing.

Russell, in your tests/examples, did the tagged traffic of $VN continue
to work after you toggled vlan_filtering on? If so, that must be because
on a bridge with VLAN filtering disabled, we still ended up calling down
to the lan1..6 ndo_vlan_rx_add_vid() and so we do have VLAN entries
programmed for $VN.
-- 
Florian

Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: fix vlan setup

[Russell King - ARM Linux admin]

On Tue, Feb 18, 2020 at 11:34:38AM -0800, Florian Fainelli wrote:
> Russell, in your tests/examples, did the tagged traffic of $VN continue
> to work after you toggled vlan_filtering on? If so, that must be because
> on a bridge with VLAN filtering disabled, we still ended up calling down
> to the lan1..6 ndo_vlan_rx_add_vid() and so we do have VLAN entries
> programmed for $VN.

From what I remember, _all_ traffic was blocked because the VTU
was completely empty when vlan filtering is turned on.

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

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Florian Fainelli]

On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> Hi,
> 
> This is a repost of the previously posted RFC back in December, which
> did not get fully reviewed.  I've dropped the RFC tag this time as no
> one really found anything too problematical in the RFC posting.
> 
> I've been trying to configure DSA for VLANs and not having much success.
> The setup is quite simple:
> 
> - The main network is untagged
> - The wifi network is a vlan tagged with id $VN running over the main
>   network.
> 
> I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> setup to provide wifi access to the vlan $VN network, while the switch
> is also part of the main network.

Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
Don't add vlans when vlan filtering is disabled")? If a driver wants to
veto the programming of VLANs while it has ports enslaved to a bridge
that does not have VLAN filtering, it should have enough information to
not do that operation.
-- 
Florian

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
> On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> > Hi,
> > 
> > This is a repost of the previously posted RFC back in December, which
> > did not get fully reviewed.  I've dropped the RFC tag this time as no
> > one really found anything too problematical in the RFC posting.
> > 
> > I've been trying to configure DSA for VLANs and not having much success.
> > The setup is quite simple:
> > 
> > - The main network is untagged
> > - The wifi network is a vlan tagged with id $VN running over the main
> >   network.
> > 
> > I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> > setup to provide wifi access to the vlan $VN network, while the switch
> > is also part of the main network.
> 
> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> Don't add vlans when vlan filtering is disabled")? If a driver wants to
> veto the programming of VLANs while it has ports enslaved to a bridge
> that does not have VLAN filtering, it should have enough information to
> not do that operation.

I do not have the knowledge to know whether reverting that commit
would be appropriate; I do not know how the non-Marvell switches will
behave with such a revert - what was the reason for the commit in
the first place?

The commit says:

    This fixes at least one corner case. There are still issues in other
    corners, such as when vlan_filtering is later enabled.

but it doesn't say what that corner case was.  So, presumably reverting
it will cause a regression of whatever that corner case was...

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

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Florian Fainelli]

On 2/18/20 4:17 PM, Russell King - ARM Linux admin wrote:
> On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
>> On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
>>> Hi,
>>>
>>> This is a repost of the previously posted RFC back in December, which
>>> did not get fully reviewed.  I've dropped the RFC tag this time as no
>>> one really found anything too problematical in the RFC posting.
>>>
>>> I've been trying to configure DSA for VLANs and not having much success.
>>> The setup is quite simple:
>>>
>>> - The main network is untagged
>>> - The wifi network is a vlan tagged with id $VN running over the main
>>>   network.
>>>
>>> I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
>>> setup to provide wifi access to the vlan $VN network, while the switch
>>> is also part of the main network.
>>
>> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
>> Don't add vlans when vlan filtering is disabled")? If a driver wants to
>> veto the programming of VLANs while it has ports enslaved to a bridge
>> that does not have VLAN filtering, it should have enough information to
>> not do that operation.
> 
> I do not have the knowledge to know whether reverting that commit
> would be appropriate; I do not know how the non-Marvell switches will
> behave with such a revert - what was the reason for the commit in
> the first place?
> 
> The commit says:
> 
>     This fixes at least one corner case. There are still issues in other
>     corners, such as when vlan_filtering is later enabled.
> 
> but it doesn't say what that corner case was.  So, presumably reverting
> it will cause a regression of whatever that corner case was...

Andrew, can you provide more details on what prompted you to do this in
the first place?
-- 
Florian

Re: [PATCH net-next 3/3] net: dsa: mv88e6xxx: fix vlan setup

[Vivien Didelot]

Hi Russell,

On Tue, 18 Feb 2020 23:48:06 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Tue, Feb 18, 2020 at 11:34:38AM -0800, Florian Fainelli wrote:
> > Russell, in your tests/examples, did the tagged traffic of $VN continue
> > to work after you toggled vlan_filtering on? If so, that must be because
> > on a bridge with VLAN filtering disabled, we still ended up calling down
> > to the lan1..6 ndo_vlan_rx_add_vid() and so we do have VLAN entries
> > programmed for $VN.
> 
> From what I remember, _all_ traffic was blocked because the VTU
> was completely empty when vlan filtering is turned on.

Not sure if the default PVID is 0 or 1, but one of them is ignored
somehow. Setting up a default PVID in the first place is a way to ensure
the programmation of the VTU, like in the following example:

    # ip link add name br0 type bridge vlan_filtering 1 vlan_default_pvid 42
    # ip link set master br0 dev lan2 up
    # cat /sys/kernel/debug/mv88e6xxx/sw0/vtu 
    vid 42	fid 1	sid 0	dpv 0 unmodified 2 untagged 10 unmodified

Russell can you test this? If things work out for you when setting up a VLAN
filtering aware bridge with your own default PVID before enslaving DSA ports,
then the problem is narrowed to propagating the default bridge configuration
(default PVID and configuration after enabling VLAN filtering).


Thanks,

	Vivien

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Vivien Didelot]

On Wed, 19 Feb 2020 00:17:37 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
> > On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> > > Hi,
> > > 
> > > This is a repost of the previously posted RFC back in December, which
> > > did not get fully reviewed.  I've dropped the RFC tag this time as no
> > > one really found anything too problematical in the RFC posting.
> > > 
> > > I've been trying to configure DSA for VLANs and not having much success.
> > > The setup is quite simple:
> > > 
> > > - The main network is untagged
> > > - The wifi network is a vlan tagged with id $VN running over the main
> > >   network.
> > > 
> > > I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> > > setup to provide wifi access to the vlan $VN network, while the switch
> > > is also part of the main network.
> > 
> > Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> > Don't add vlans when vlan filtering is disabled")? If a driver wants to
> > veto the programming of VLANs while it has ports enslaved to a bridge
> > that does not have VLAN filtering, it should have enough information to
> > not do that operation.
> 
> I do not have the knowledge to know whether reverting that commit
> would be appropriate; I do not know how the non-Marvell switches will
> behave with such a revert - what was the reason for the commit in
> the first place?
> 
> The commit says:
> 
>     This fixes at least one corner case. There are still issues in other
>     corners, such as when vlan_filtering is later enabled.
> 
> but it doesn't say what that corner case was.  So, presumably reverting
> it will cause a regression of whatever that corner case was...

It is hard to care about regression when we have no idea what these "corner
cases" are. Also things like ds->vlan_filtering* were added after if I'm
not mistaken, so the drivers might have enough information now to adapt to
or reject some unsupported bridge offload.

Getting rid of this limitation would definitely be another approach worth
trying.


Thanks,

	Vivien

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Andrew Lunn]

On Wed, Feb 19, 2020 at 12:17:37AM +0000, Russell King - ARM Linux admin wrote:
> On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
> > On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> > > Hi,
> > > 
> > > This is a repost of the previously posted RFC back in December, which
> > > did not get fully reviewed.  I've dropped the RFC tag this time as no
> > > one really found anything too problematical in the RFC posting.
> > > 
> > > I've been trying to configure DSA for VLANs and not having much success.
> > > The setup is quite simple:
> > > 
> > > - The main network is untagged
> > > - The wifi network is a vlan tagged with id $VN running over the main
> > >   network.
> > > 
> > > I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> > > setup to provide wifi access to the vlan $VN network, while the switch
> > > is also part of the main network.
> > 
> > Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> > Don't add vlans when vlan filtering is disabled")? If a driver wants to
> > veto the programming of VLANs while it has ports enslaved to a bridge
> > that does not have VLAN filtering, it should have enough information to
> > not do that operation.
> 
> I do not have the knowledge to know whether reverting that commit
> would be appropriate; I do not know how the non-Marvell switches will
> behave with such a revert - what was the reason for the commit in
> the first place?
> 
> The commit says:
> 
>     This fixes at least one corner case. There are still issues in other
>     corners, such as when vlan_filtering is later enabled.
> 
> but it doesn't say what that corner case was.  So, presumably reverting
> it will cause a regression of whatever that corner case was...

Yes, sorry, bad commit message. I'm not too sure, but it could of been
that the switch was adding the VLANs to its tables, even though it
should not because filtering is disabled. And i also think the default
VLAN was not defined at that point, it only gets defined when
vlan_filtering is enabled?

       Andrew

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Wed, Feb 19, 2020 at 04:47:30AM +0100, Andrew Lunn wrote:
> On Wed, Feb 19, 2020 at 12:17:37AM +0000, Russell King - ARM Linux admin wrote:
> > On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
> > > On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> > > > Hi,
> > > > 
> > > > This is a repost of the previously posted RFC back in December, which
> > > > did not get fully reviewed.  I've dropped the RFC tag this time as no
> > > > one really found anything too problematical in the RFC posting.
> > > > 
> > > > I've been trying to configure DSA for VLANs and not having much success.
> > > > The setup is quite simple:
> > > > 
> > > > - The main network is untagged
> > > > - The wifi network is a vlan tagged with id $VN running over the main
> > > >   network.
> > > > 
> > > > I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> > > > setup to provide wifi access to the vlan $VN network, while the switch
> > > > is also part of the main network.
> > > 
> > > Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> > > Don't add vlans when vlan filtering is disabled")? If a driver wants to
> > > veto the programming of VLANs while it has ports enslaved to a bridge
> > > that does not have VLAN filtering, it should have enough information to
> > > not do that operation.
> > 
> > I do not have the knowledge to know whether reverting that commit
> > would be appropriate; I do not know how the non-Marvell switches will
> > behave with such a revert - what was the reason for the commit in
> > the first place?
> > 
> > The commit says:
> > 
> >     This fixes at least one corner case. There are still issues in other
> >     corners, such as when vlan_filtering is later enabled.
> > 
> > but it doesn't say what that corner case was.  So, presumably reverting
> > it will cause a regression of whatever that corner case was...
> 
> Yes, sorry, bad commit message. I'm not too sure, but it could of been
> that the switch was adding the VLANs to its tables, even though it
> should not because filtering is disabled. And i also think the default
> VLAN was not defined at that point, it only gets defined when
> vlan_filtering is enabled?

It's been too long since I researched all these details, but I seem
to remember that in the Linux software bridge, vlan 1 is always
present even when vlan filtering is not enabled.

Looking at br_vlan_init():

        br->default_pvid = 1;

and nbp_vlan_init() propagates that irrespective of the bridge vlan
enable state to switchdev.  nbp_vlan_init() is called whenever any
interface is added to a bridge (in br_add_if()).

As I believe I mentioned somewhere in the commit messages or covering
message, for at least some of the Marvell DSA switches, it is safe to
add VTU entries - they do not even look at the VTU when the port has
802.1Q disabled.  Whether that is true for all Marvell's DSA switches
I don't know without trawling every functional spec, and I was hoping
that you guys would know.  I guess I need to trawl the specs.

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

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Vivien Didelot]

Hi Russell,

On Wed, 19 Feb 2020 09:19:00 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Wed, Feb 19, 2020 at 04:47:30AM +0100, Andrew Lunn wrote:
> > On Wed, Feb 19, 2020 at 12:17:37AM +0000, Russell King - ARM Linux admin wrote:
> > > On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
> > > > On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
> > > > > Hi,
> > > > > 
> > > > > This is a repost of the previously posted RFC back in December, which
> > > > > did not get fully reviewed.  I've dropped the RFC tag this time as no
> > > > > one really found anything too problematical in the RFC posting.
> > > > > 
> > > > > I've been trying to configure DSA for VLANs and not having much success.
> > > > > The setup is quite simple:
> > > > > 
> > > > > - The main network is untagged
> > > > > - The wifi network is a vlan tagged with id $VN running over the main
> > > > >   network.
> > > > > 
> > > > > I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
> > > > > setup to provide wifi access to the vlan $VN network, while the switch
> > > > > is also part of the main network.
> > > > 
> > > > Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> > > > Don't add vlans when vlan filtering is disabled")? If a driver wants to
> > > > veto the programming of VLANs while it has ports enslaved to a bridge
> > > > that does not have VLAN filtering, it should have enough information to
> > > > not do that operation.
> > > 
> > > I do not have the knowledge to know whether reverting that commit
> > > would be appropriate; I do not know how the non-Marvell switches will
> > > behave with such a revert - what was the reason for the commit in
> > > the first place?
> > > 
> > > The commit says:
> > > 
> > >     This fixes at least one corner case. There are still issues in other
> > >     corners, such as when vlan_filtering is later enabled.
> > > 
> > > but it doesn't say what that corner case was.  So, presumably reverting
> > > it will cause a regression of whatever that corner case was...
> > 
> > Yes, sorry, bad commit message. I'm not too sure, but it could of been
> > that the switch was adding the VLANs to its tables, even though it
> > should not because filtering is disabled. And i also think the default
> > VLAN was not defined at that point, it only gets defined when
> > vlan_filtering is enabled?
> 
> It's been too long since I researched all these details, but I seem
> to remember that in the Linux software bridge, vlan 1 is always
> present even when vlan filtering is not enabled.
> 
> Looking at br_vlan_init():
> 
>         br->default_pvid = 1;
> 
> and nbp_vlan_init() propagates that irrespective of the bridge vlan
> enable state to switchdev.  nbp_vlan_init() is called whenever any
> interface is added to a bridge (in br_add_if()).
> 
> As I believe I mentioned somewhere in the commit messages or covering
> message, for at least some of the Marvell DSA switches, it is safe to
> add VTU entries - they do not even look at the VTU when the port has
> 802.1Q disabled.  Whether that is true for all Marvell's DSA switches
> I don't know without trawling every functional spec, and I was hoping
> that you guys would know.  I guess I need to trawl the specs.

Some switches like the Marvell 88E6060 don't have a VTU, so programming the
default PVID would return -EOPNOTSUPP. Switches supporting only global VLAN
filtering cannot have a VLAN filtering aware bridge as well as a non VLAN
filtering aware bridge spanning their ports at the same time. But all this
shouldn't be a problem because drivers inform the stack whether they support
ds->vlan_filtering per-port, globally or not. We should simply reject the
operation when vlan_filtering is being enabled on unsupported hardware.

Linux bridge is the reference for the implementation of an Ethernet bridge,
if it programs VLAN entries, supported DSA hardware must do so. I'm not a
fan of having our own bridge logic in DSA, so the limitation implemented by
2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is disabled")
needs to go in my opinion.


Thanks,

	Vivien

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Florian Fainelli]

On 2/19/20 10:07 AM, Vivien Didelot wrote:
> Hi Russell,
> 
> On Wed, 19 Feb 2020 09:19:00 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
>> On Wed, Feb 19, 2020 at 04:47:30AM +0100, Andrew Lunn wrote:
>>> On Wed, Feb 19, 2020 at 12:17:37AM +0000, Russell King - ARM Linux admin wrote:
>>>> On Tue, Feb 18, 2020 at 04:00:08PM -0800, Florian Fainelli wrote:
>>>>> On 2/18/20 3:45 AM, Russell King - ARM Linux admin wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This is a repost of the previously posted RFC back in December, which
>>>>>> did not get fully reviewed.  I've dropped the RFC tag this time as no
>>>>>> one really found anything too problematical in the RFC posting.
>>>>>>
>>>>>> I've been trying to configure DSA for VLANs and not having much success.
>>>>>> The setup is quite simple:
>>>>>>
>>>>>> - The main network is untagged
>>>>>> - The wifi network is a vlan tagged with id $VN running over the main
>>>>>>   network.
>>>>>>
>>>>>> I have an Armada 388 Clearfog with a PCIe wifi card which I'm trying to
>>>>>> setup to provide wifi access to the vlan $VN network, while the switch
>>>>>> is also part of the main network.
>>>>>
>>>>> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
>>>>> Don't add vlans when vlan filtering is disabled")? If a driver wants to
>>>>> veto the programming of VLANs while it has ports enslaved to a bridge
>>>>> that does not have VLAN filtering, it should have enough information to
>>>>> not do that operation.
>>>>
>>>> I do not have the knowledge to know whether reverting that commit
>>>> would be appropriate; I do not know how the non-Marvell switches will
>>>> behave with such a revert - what was the reason for the commit in
>>>> the first place?
>>>>
>>>> The commit says:
>>>>
>>>>     This fixes at least one corner case. There are still issues in other
>>>>     corners, such as when vlan_filtering is later enabled.
>>>>
>>>> but it doesn't say what that corner case was.  So, presumably reverting
>>>> it will cause a regression of whatever that corner case was...
>>>
>>> Yes, sorry, bad commit message. I'm not too sure, but it could of been
>>> that the switch was adding the VLANs to its tables, even though it
>>> should not because filtering is disabled. And i also think the default
>>> VLAN was not defined at that point, it only gets defined when
>>> vlan_filtering is enabled?
>>
>> It's been too long since I researched all these details, but I seem
>> to remember that in the Linux software bridge, vlan 1 is always
>> present even when vlan filtering is not enabled.
>>
>> Looking at br_vlan_init():
>>
>>         br->default_pvid = 1;
>>
>> and nbp_vlan_init() propagates that irrespective of the bridge vlan
>> enable state to switchdev.  nbp_vlan_init() is called whenever any
>> interface is added to a bridge (in br_add_if()).
>>
>> As I believe I mentioned somewhere in the commit messages or covering
>> message, for at least some of the Marvell DSA switches, it is safe to
>> add VTU entries - they do not even look at the VTU when the port has
>> 802.1Q disabled.  Whether that is true for all Marvell's DSA switches
>> I don't know without trawling every functional spec, and I was hoping
>> that you guys would know.  I guess I need to trawl the specs.
> 
> Some switches like the Marvell 88E6060 don't have a VTU, so programming the
> default PVID would return -EOPNOTSUPP. Switches supporting only global VLAN
> filtering cannot have a VLAN filtering aware bridge as well as a non VLAN
> filtering aware bridge spanning their ports at the same time. But all this
> shouldn't be a problem because drivers inform the stack whether they support
> ds->vlan_filtering per-port, globally or not. We should simply reject the
> operation when vlan_filtering is being enabled on unsupported hardware.
> 
> Linux bridge is the reference for the implementation of an Ethernet bridge,
> if it programs VLAN entries, supported DSA hardware must do so. I'm not a
> fan of having our own bridge logic in DSA, so the limitation implemented by
> 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is disabled")
> needs to go in my opinion.

Agreed.

This also helps with switches who only support the creation of broadcast
domains via VLANs (not the case with b53 and mv88e6xxx AFAICT they have
specific egress vector controls). Because then you could put each
standalone port in say, VID (4094 - port number), and once enslaved in a
bridge, have them in VID 1 to maintain broadcast domains, whether the
bridge has VLAN filtering or not.
-- 
Florian

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Vladimir Oltean]

On Wed, 19 Feb 2020 at 02:02, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> Don't add vlans when vlan filtering is disabled")? If a driver wants to
> veto the programming of VLANs while it has ports enslaved to a bridge
> that does not have VLAN filtering, it should have enough information to
> not do that operation.
> --
> Florian

It would be worth mentioning that for sja1105 and hypothetical other
users of DSA_TAG_PROTO_8021Q, DSA doing that automatically was
helpful. VLAN manipulations are still being done from tag_8021q.c for
the purpose of DSA tagging, but the fact that the VLAN EtherType is
not 0x8100 means that from the perspective of real VLAN traffic, the
switch is VLAN unaware. DSA was the easiest place to disseminate
between VLAN requests of its own and VLAN requests coming from
switchdev.
Without that logic in DSA, a vlan-unaware bridge would be able to
destroy the configuration done for source port decoding.
Just saying - with enough logic in .port_vlan_prepare, I should still
be able to accept only what's whitelisted to work for tagging, and
then it won't matter who issued that VLAN command.

Thanks,
-Vladimir

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Florian Fainelli]

On 2/19/20 10:52 AM, Vladimir Oltean wrote:
> On Wed, 19 Feb 2020 at 02:02, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
>> Don't add vlans when vlan filtering is disabled")? If a driver wants to
>> veto the programming of VLANs while it has ports enslaved to a bridge
>> that does not have VLAN filtering, it should have enough information to
>> not do that operation.
>> --
>> Florian
> 
> It would be worth mentioning that for sja1105 and hypothetical other
> users of DSA_TAG_PROTO_8021Q, DSA doing that automatically was
> helpful. VLAN manipulations are still being done from tag_8021q.c for
> the purpose of DSA tagging, but the fact that the VLAN EtherType is
> not 0x8100 means that from the perspective of real VLAN traffic, the
> switch is VLAN unaware. DSA was the easiest place to disseminate
> between VLAN requests of its own and VLAN requests coming from
> switchdev.
> Without that logic in DSA, a vlan-unaware bridge would be able to
> destroy the configuration done for source port decoding.
> Just saying - with enough logic in .port_vlan_prepare, I should still
> be able to accept only what's whitelisted to work for tagging, and
> then it won't matter who issued that VLAN command.

I suppose I am fine with Russell's approach, but maybe its meaning
should be reversed, that is, you get VLAN objects notifications by
default for a  VLAN unaware bridge and if you do set a specific
dsa_switch flag, then you do not get those.
-- 
Florian

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Wed, Feb 19, 2020 at 11:18:17AM -0800, Florian Fainelli wrote:
> On 2/19/20 10:52 AM, Vladimir Oltean wrote:
> > On Wed, 19 Feb 2020 at 02:02, Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>
> >> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
> >> Don't add vlans when vlan filtering is disabled")? If a driver wants to
> >> veto the programming of VLANs while it has ports enslaved to a bridge
> >> that does not have VLAN filtering, it should have enough information to
> >> not do that operation.
> >> --
> >> Florian
> > 
> > It would be worth mentioning that for sja1105 and hypothetical other
> > users of DSA_TAG_PROTO_8021Q, DSA doing that automatically was
> > helpful. VLAN manipulations are still being done from tag_8021q.c for
> > the purpose of DSA tagging, but the fact that the VLAN EtherType is
> > not 0x8100 means that from the perspective of real VLAN traffic, the
> > switch is VLAN unaware. DSA was the easiest place to disseminate
> > between VLAN requests of its own and VLAN requests coming from
> > switchdev.
> > Without that logic in DSA, a vlan-unaware bridge would be able to
> > destroy the configuration done for source port decoding.
> > Just saying - with enough logic in .port_vlan_prepare, I should still
> > be able to accept only what's whitelisted to work for tagging, and
> > then it won't matter who issued that VLAN command.
> 
> I suppose I am fine with Russell's approach, but maybe its meaning
> should be reversed, that is, you get VLAN objects notifications by
> default for a  VLAN unaware bridge and if you do set a specific
> dsa_switch flag, then you do not get those.

If we reverse it, I'll need someone to tell me which DSA switches
should not get the vlan object notifications.  Maybe also in that
case, we should deny the ability to toggle the state of
vlan_filtering as well?

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

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Wed, Feb 19, 2020 at 01:07:07PM -0500, Vivien Didelot wrote:
> Hi Russell,
> 
> Some switches like the Marvell 88E6060 don't have a VTU, so programming the
> default PVID would return -EOPNOTSUPP.

The 88e6060 has its own driver separate from mv88e6xxx.

> Switches supporting only global VLAN
> filtering cannot have a VLAN filtering aware bridge as well as a non VLAN
> filtering aware bridge spanning their ports at the same time. But all this
> shouldn't be a problem because drivers inform the stack whether they support
> ds->vlan_filtering per-port, globally or not. We should simply reject the
> operation when vlan_filtering is being enabled on unsupported hardware.
> 
> Linux bridge is the reference for the implementation of an Ethernet bridge,
> if it programs VLAN entries, supported DSA hardware must do so. I'm not a
> fan of having our own bridge logic in DSA, so the limitation implemented by
> 2ea7a679ca2a ("net: dsa: Don't add vlans when vlan filtering is disabled")
> needs to go in my opinion.

... which is basically what patch 3 is doing, but in a per-driver
manner.

The checks introduced in 2ea7a679ca2a ("net: dsa: Don't add vlans when
vlan filtering is disabled") were raised up a level by c5335d737ff3
("net: dsa: check bridge VLAN in slave operations") to their present
positions, which are then touched by my patch 3 to make the checks
conditional.

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

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

[Russell King - ARM Linux admin]

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?

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

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Florian Fainelli]

On 2/19/20 3:15 PM, Russell King - ARM Linux admin wrote:
> On Wed, Feb 19, 2020 at 11:18:17AM -0800, Florian Fainelli wrote:
>> On 2/19/20 10:52 AM, Vladimir Oltean wrote:
>>> On Wed, 19 Feb 2020 at 02:02, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>
>>>> Why not just revert 2ea7a679ca2abd251c1ec03f20508619707e1749 ("net: dsa:
>>>> Don't add vlans when vlan filtering is disabled")? If a driver wants to
>>>> veto the programming of VLANs while it has ports enslaved to a bridge
>>>> that does not have VLAN filtering, it should have enough information to
>>>> not do that operation.
>>>> --
>>>> Florian
>>>
>>> It would be worth mentioning that for sja1105 and hypothetical other
>>> users of DSA_TAG_PROTO_8021Q, DSA doing that automatically was
>>> helpful. VLAN manipulations are still being done from tag_8021q.c for
>>> the purpose of DSA tagging, but the fact that the VLAN EtherType is
>>> not 0x8100 means that from the perspective of real VLAN traffic, the
>>> switch is VLAN unaware. DSA was the easiest place to disseminate
>>> between VLAN requests of its own and VLAN requests coming from
>>> switchdev.
>>> Without that logic in DSA, a vlan-unaware bridge would be able to
>>> destroy the configuration done for source port decoding.
>>> Just saying - with enough logic in .port_vlan_prepare, I should still
>>> be able to accept only what's whitelisted to work for tagging, and
>>> then it won't matter who issued that VLAN command.
>>
>> I suppose I am fine with Russell's approach, but maybe its meaning
>> should be reversed, that is, you get VLAN objects notifications by
>> default for a  VLAN unaware bridge and if you do set a specific
>> dsa_switch flag, then you do not get those.
> 
> If we reverse it, I'll need someone to tell me which DSA switches
> should not get the vlan object notifications.  Maybe also in that
> case, we should deny the ability to toggle the state of
> vlan_filtering as well?
> 

Let's get your patch series merged. If you re-spin while addressing
Vivien's comment not to use the term "vtu", I think I would be fine with
the current approach of having to go after each driver and enabling them
where necessary.

Thanks
-- 
Florian

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> Let's get your patch series merged. If you re-spin while addressing
> Vivien's comment not to use the term "vtu", I think I would be fine with
> the current approach of having to go after each driver and enabling them
> where necessary.

The question then becomes what to call it.  "always_allow_vlans" or
"always_allow_vlan_config" maybe?

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

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > Let's get your patch series merged. If you re-spin while addressing
> > Vivien's comment not to use the term "vtu", I think I would be fine with
> > the current approach of having to go after each driver and enabling them
> > where necessary.
> 
> The question then becomes what to call it.  "always_allow_vlans" or
> "always_allow_vlan_config" maybe?

Please note that I still have this patch pending (i.o.w., the problem
with vlans remains unfixed) as I haven't received a reply to this,
although the first two patches have been merged.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Mon, Mar 16, 2020 at 11:15:24AM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> > On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > > Let's get your patch series merged. If you re-spin while addressing
> > > Vivien's comment not to use the term "vtu", I think I would be fine with
> > > the current approach of having to go after each driver and enabling them
> > > where necessary.
> > 
> > The question then becomes what to call it.  "always_allow_vlans" or
> > "always_allow_vlan_config" maybe?
> 
> Please note that I still have this patch pending (i.o.w., the problem
> with vlans remains unfixed) as I haven't received a reply to this,
> although the first two patches have been merged.

Okay, I think three and a half weeks is way beyond a reasonable time
period to expect any kind of reply.

Since no one seems to have any idea what to name this, but can only
offer "we don't like the vtu" term, it's difficult to see what would
actually be acceptable.  So, I propose that we go with the existing
naming.

If you only know what you don't want, but not what you want, and aren't
even willing to discuss it, it makes it very much impossible to
progress.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Vladimir Oltean]

Hi Russell,

On Tue, 17 Mar 2020 at 14:00, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Mon, Mar 16, 2020 at 11:15:24AM +0000, Russell King - ARM Linux admin wrote:
> > On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> > > On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > > > Let's get your patch series merged. If you re-spin while addressing
> > > > Vivien's comment not to use the term "vtu", I think I would be fine with
> > > > the current approach of having to go after each driver and enabling them
> > > > where necessary.
> > >
> > > The question then becomes what to call it.  "always_allow_vlans" or
> > > "always_allow_vlan_config" maybe?
> >
> > Please note that I still have this patch pending (i.o.w., the problem
> > with vlans remains unfixed) as I haven't received a reply to this,
> > although the first two patches have been merged.
>
> Okay, I think three and a half weeks is way beyond a reasonable time
> period to expect any kind of reply.
>
> Since no one seems to have any idea what to name this, but can only
> offer "we don't like the vtu" term, it's difficult to see what would
> actually be acceptable.  So, I propose that we go with the existing
> naming.
>
> If you only know what you don't want, but not what you want, and aren't
> even willing to discuss it, it makes it very much impossible to
> progress.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

As I said, I know why I need this blocking of bridge vlan
configuration for sja1105, but not more. For sja1105 in particular, I
fully admit that the hardware is quirky, but I can work around that
within the driver. The concern is for the other drivers where we don't
really "remember" why this workaround is in place. I think, while your
patch is definitely a punctual fix for one case that doesn't need the
workaround, it might be better for maintenance to just see exactly
what breaks, instead of introducing this opaque property.
While I don't want to speak on behalf of the maintainers, I think that
may be at least part of the reason why there is little progress being
made. Introducing some breakage which is going to be fixed better next
time might be the more appropriate thing to do.

Thanks,
-Vladimir

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Tue, Mar 17, 2020 at 04:21:00PM +0200, Vladimir Oltean wrote:
> Hi Russell,
> 
> On Tue, 17 Mar 2020 at 14:00, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Mon, Mar 16, 2020 at 11:15:24AM +0000, Russell King - ARM Linux admin wrote:
> > > On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> > > > On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > > > > Let's get your patch series merged. If you re-spin while addressing
> > > > > Vivien's comment not to use the term "vtu", I think I would be fine with
> > > > > the current approach of having to go after each driver and enabling them
> > > > > where necessary.
> > > >
> > > > The question then becomes what to call it.  "always_allow_vlans" or
> > > > "always_allow_vlan_config" maybe?
> > >
> > > Please note that I still have this patch pending (i.o.w., the problem
> > > with vlans remains unfixed) as I haven't received a reply to this,
> > > although the first two patches have been merged.
> >
> > Okay, I think three and a half weeks is way beyond a reasonable time
> > period to expect any kind of reply.
> >
> > Since no one seems to have any idea what to name this, but can only
> > offer "we don't like the vtu" term, it's difficult to see what would
> > actually be acceptable.  So, I propose that we go with the existing
> > naming.
> >
> > If you only know what you don't want, but not what you want, and aren't
> > even willing to discuss it, it makes it very much impossible to
> > progress.
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> 
> As I said, I know why I need this blocking of bridge vlan
> configuration for sja1105, but not more. For sja1105 in particular, I
> fully admit that the hardware is quirky, but I can work around that
> within the driver. The concern is for the other drivers where we don't
> really "remember" why this workaround is in place. I think, while your
> patch is definitely a punctual fix for one case that doesn't need the
> workaround, it might be better for maintenance to just see exactly
> what breaks, instead of introducing this opaque property.
> While I don't want to speak on behalf of the maintainers, I think that
> may be at least part of the reason why there is little progress being
> made. Introducing some breakage which is going to be fixed better next
> time might be the more appropriate thing to do.

The conclusion on 21st February was that all patches should be merged,
complete with the boolean control, but there was an open question about
the name of the boolean used to enable this behaviour.

That question has not been resolved, so I'm trying to re-open discussion
of that point.  I've re-posted the patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Vivien Didelot]

Hi Russell,

On Tue, 17 Mar 2020 15:12:38 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Tue, Mar 17, 2020 at 04:21:00PM +0200, Vladimir Oltean wrote:
> > Hi Russell,
> > 
> > On Tue, 17 Mar 2020 at 14:00, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Mon, Mar 16, 2020 at 11:15:24AM +0000, Russell King - ARM Linux admin wrote:
> > > > On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> > > > > On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > > > > > Let's get your patch series merged. If you re-spin while addressing
> > > > > > Vivien's comment not to use the term "vtu", I think I would be fine with
> > > > > > the current approach of having to go after each driver and enabling them
> > > > > > where necessary.
> > > > >
> > > > > The question then becomes what to call it.  "always_allow_vlans" or
> > > > > "always_allow_vlan_config" maybe?
> > > >
> > > > Please note that I still have this patch pending (i.o.w., the problem
> > > > with vlans remains unfixed) as I haven't received a reply to this,
> > > > although the first two patches have been merged.
> > >
> > > Okay, I think three and a half weeks is way beyond a reasonable time
> > > period to expect any kind of reply.
> > >
> > > Since no one seems to have any idea what to name this, but can only
> > > offer "we don't like the vtu" term, it's difficult to see what would
> > > actually be acceptable.  So, I propose that we go with the existing
> > > naming.
> > >
> > > If you only know what you don't want, but not what you want, and aren't
> > > even willing to discuss it, it makes it very much impossible to
> > > progress.
> > >
> > > --
> > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> > 
> > As I said, I know why I need this blocking of bridge vlan
> > configuration for sja1105, but not more. For sja1105 in particular, I
> > fully admit that the hardware is quirky, but I can work around that
> > within the driver. The concern is for the other drivers where we don't
> > really "remember" why this workaround is in place. I think, while your
> > patch is definitely a punctual fix for one case that doesn't need the
> > workaround, it might be better for maintenance to just see exactly
> > what breaks, instead of introducing this opaque property.
> > While I don't want to speak on behalf of the maintainers, I think that
> > may be at least part of the reason why there is little progress being
> > made. Introducing some breakage which is going to be fixed better next
> > time might be the more appropriate thing to do.
> 
> The conclusion on 21st February was that all patches should be merged,
> complete with the boolean control, but there was an open question about
> the name of the boolean used to enable this behaviour.
> 
> That question has not been resolved, so I'm trying to re-open discussion
> of that point.  I've re-posted the patch.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

In response to your 3/3 patch, I suggested commands to test setting up a
VLAN filtering aware bridge with your own default PVID before enslaving
DSA ports. Unfortunately you left this unanswered. I think this would be
much more interesting in order to tackle the issue you're having here with
mv88e6xxx and bridge, instead of pointing out the lack of response regarding
an alternative boolean name. That being said, "force_vlan_programming",
"always_allow_vlans", or whatever explicit non-"vtu" term is fine by me.


Thanks,

	Vivien

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Russell King - ARM Linux admin]

On Tue, Mar 17, 2020 at 02:49:06PM -0400, Vivien Didelot wrote:
> Hi Russell,
> 
> On Tue, 17 Mar 2020 15:12:38 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > On Tue, Mar 17, 2020 at 04:21:00PM +0200, Vladimir Oltean wrote:
> > > Hi Russell,
> > > 
> > > On Tue, 17 Mar 2020 at 14:00, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Mon, Mar 16, 2020 at 11:15:24AM +0000, Russell King - ARM Linux admin wrote:
> > > > > On Fri, Feb 21, 2020 at 12:21:10AM +0000, Russell King - ARM Linux admin wrote:
> > > > > > On Thu, Feb 20, 2020 at 10:56:17AM -0800, Florian Fainelli wrote:
> > > > > > > Let's get your patch series merged. If you re-spin while addressing
> > > > > > > Vivien's comment not to use the term "vtu", I think I would be fine with
> > > > > > > the current approach of having to go after each driver and enabling them
> > > > > > > where necessary.
> > > > > >
> > > > > > The question then becomes what to call it.  "always_allow_vlans" or
> > > > > > "always_allow_vlan_config" maybe?
> > > > >
> > > > > Please note that I still have this patch pending (i.o.w., the problem
> > > > > with vlans remains unfixed) as I haven't received a reply to this,
> > > > > although the first two patches have been merged.
> > > >
> > > > Okay, I think three and a half weeks is way beyond a reasonable time
> > > > period to expect any kind of reply.
> > > >
> > > > Since no one seems to have any idea what to name this, but can only
> > > > offer "we don't like the vtu" term, it's difficult to see what would
> > > > actually be acceptable.  So, I propose that we go with the existing
> > > > naming.
> > > >
> > > > If you only know what you don't want, but not what you want, and aren't
> > > > even willing to discuss it, it makes it very much impossible to
> > > > progress.
> > > >
> > > > --
> > > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> > > 
> > > As I said, I know why I need this blocking of bridge vlan
> > > configuration for sja1105, but not more. For sja1105 in particular, I
> > > fully admit that the hardware is quirky, but I can work around that
> > > within the driver. The concern is for the other drivers where we don't
> > > really "remember" why this workaround is in place. I think, while your
> > > patch is definitely a punctual fix for one case that doesn't need the
> > > workaround, it might be better for maintenance to just see exactly
> > > what breaks, instead of introducing this opaque property.
> > > While I don't want to speak on behalf of the maintainers, I think that
> > > may be at least part of the reason why there is little progress being
> > > made. Introducing some breakage which is going to be fixed better next
> > > time might be the more appropriate thing to do.
> > 
> > The conclusion on 21st February was that all patches should be merged,
> > complete with the boolean control, but there was an open question about
> > the name of the boolean used to enable this behaviour.
> > 
> > That question has not been resolved, so I'm trying to re-open discussion
> > of that point.  I've re-posted the patch.
> > 
> > -- 
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
> 
> In response to your 3/3 patch, I suggested commands to test setting up a
> VLAN filtering aware bridge with your own default PVID before enslaving
> DSA ports. Unfortunately you left this unanswered.

I don't believe I left it unanswered.  However, I'm not about to rip
apart my network to try an experiment with specific set of commands.
I did, however, experiment a lot to work out what was going on, so I
already have the knowledge.

I believe I explained in the series description that the problem only
happens when vlan filtering is enabled with a pre-existing vlan
configuration present, even the default configuration.

Enabling vlan filtering *immediately* blocks all traffic on the Marvell
switch, whether it has vlan tags or not.  Any *new* vlan modifications
then get entered into the VTU, and the Marvell switch then behaves
accordingly with those new entries as one would expect.

Any setup done *before* vlan filtering is enabled is not present in the
VTU, and so remains non-existent as far as the DSA switch is concerned.

As soon as vlan filtering is enabled, the ports are switched to "802.1Q
secure" state, which means:

- The switch will consult the VTU for the incoming packet; if no entry
  is found for the vlan number either tagged to the packet, or the
  default vlan if not, then the packet will be discarded by the switch.

So, the setup done *before* vlan filtering is enabled, which is not
programmed into the VTU, results in that traffic being lost.

> I think this would be
> much more interesting in order to tackle the issue you're having here with
> mv88e6xxx and bridge, instead of pointing out the lack of response regarding
> an alternative boolean name.

It is my understanding that Florian actively wants this merged.  No
one objected to his email.

It seems there's a disconnect *between* the DSA maintainers - I think
you need to be more effectively communicating with each other and
reading each other's emails, and pro-actively replying to stuff you
may have other views on.

> That being said, "force_vlan_programming",
> "always_allow_vlans", or whatever explicit non-"vtu" term is fine by me.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

Re: [PATCH net-next 0/3] VLANs, DSA switches and multiple bridges

[Vivien Didelot]

On Tue, 17 Mar 2020 21:24:53 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> > In response to your 3/3 patch, I suggested commands to test setting up a
> > VLAN filtering aware bridge with your own default PVID before enslaving
> > DSA ports. Unfortunately you left this unanswered.
> 
> I don't believe I left it unanswered.  However, I'm not about to rip
> apart my network to try an experiment with specific set of commands.

In mail 3/3 I suggested to run the following snippet to configure the bridge
at creation time so that we can see clearly if the problem still occurs:

    # ip link add name br0 type bridge vlan_filtering 1 vlan_default_pvid 42
    # ip link set master br0 dev lan2 up
    # cat /sys/kernel/debug/mv88e6xxx/sw0/vtu
    vid 42      fid 1   sid 0   dpv 0 unmodified 2 untagged 10 unmodified

You skipped this, last email without reply, this feels pretty unanswered to me.

But whatever, I don't want these two commands to rip apart your network.

> It is my understanding that Florian actively wants this merged.  No
> one objected to his email.
> 
> It seems there's a disconnect *between* the DSA maintainers - I think
> you need to be more effectively communicating with each other and
> reading each other's emails, and pro-actively replying to stuff you
> may have other views on.

I'm not sure to understand what you're assuming here. As Florian said, your
patch is good to go as long as you change the boolean name to something
generic not containing "vtu", which is Marvell specific. If you really
need us to choose, then go with "force_vlan_programming" or one of your
suggestions. What matters here is that a non-mv88e6xxx user can clearly
understand what this boolean does.


Thank you,

	Vivien