[PATCH net-next] net: dsa: mt7530: manually set up VLAN ID 0

The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After
the blamed commit, VLAN ID 0 won't be set up anymore, breaking software
bridging fallback on VLAN-unaware bridges.

Manually set up VLAN ID 0 to fix this.

Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
 drivers/net/dsa/mt7530.c | 25 +++++++++++++++++++++++++
 drivers/net/dsa/mt7530.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index d757d9dcba51..d0cba2d1cd68 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c

@@ -1599,6 +1599,21 @@ mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
 	mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, vid);
 }
 
+static int
+mt7530_setup_vlan0(struct mt7530_priv *priv)
+{
+	u32 val;
+
+	/* Validate the entry with independent learning, keep the original
+	 * ingress tag attribute.
+	 */
+	val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) |
+	      VLAN_VALID;
+	mt7530_write(priv, MT7530_VAWD1, val);
+
+	return mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, 0);
+}
+
 static int
 mt7530_port_vlan_add(struct dsa_switch *ds, int port,
 		     const struct switchdev_obj_port_vlan *vlan,

@@ -2174,6 +2189,11 @@ mt7530_setup(struct dsa_switch *ds)
 			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
 	}
 
+	/* Setup VLAN ID 0 for VLAN-unaware bridges */
+	ret = mt7530_setup_vlan0(priv);
+	if (ret)
+		return ret;
+
 	/* Setup port 5 */
 	priv->p5_intf_sel = P5_DISABLED;
 	interface = PHY_INTERFACE_MODE_NA;

@@ -2346,6 +2366,11 @@ mt7531_setup(struct dsa_switch *ds)
 			   PVC_EG_TAG(MT7530_VLAN_EG_CONSISTENT));
 	}
 
+	/* Setup VLAN ID 0 for VLAN-unaware bridges */
+	ret = mt7530_setup_vlan0(priv);
+	if (ret)
+		return ret;
+
 	ds->assisted_learning_on_cpu_port = true;
 	ds->mtu_enforcement_ingress = true;
 

diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index fe4cd2ac26d0..91508e2feef9 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h

@@ -145,6 +145,8 @@ enum mt7530_vlan_cmd {
 #define  PORT_STAG			BIT(31)
 /* Independent VLAN Learning */
 #define  IVL_MAC			BIT(30)
+/* Egress Tag Consistent */
+#define  EG_CON				BIT(29)
 /* Per VLAN Egress Tag Control */
 #define  VTAG_EN			BIT(28)
 /* VLAN Member Control */

-- 
2.25.1

On Wed, Aug 25, 2021 at 12:52:52AM +0800, DENG Qingfang wrote:

The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After
the blamed commit, VLAN ID 0 won't be set up anymore, breaking software
bridging fallback on VLAN-unaware bridges.

Manually set up VLAN ID 0 to fix this.

Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---

I understand that this is how you noticed the issue, but please remember
that one can always compile a kernel with CONFIG_VLAN_8021Q=n. So the
issue predates my patch by much longer. You might reconsider the Fixes:
tag in light of this, maybe the patch needs to be sent to stable.

FID_BRIDGED?

On Tue, Aug 24, 2021 at 07:57:42PM +0300, Vladimir Oltean wrote:

quoted

+static int
+mt7530_setup_vlan0(struct mt7530_priv *priv)
+{
+	u32 val;
+
+	/* Validate the entry with independent learning, keep the original
+	 * ingress tag attribute.
+	 */
+	val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) |

FID_BRIDGED?

yes, FID_BRIDGED.

Please confirm that the Fixes: tag is the one you intend. The patch appears fine otherwise.

On Tue, Aug 24, 2021 at 07:57:42PM +0300, Vladimir Oltean wrote:

I understand that this is how you noticed the issue, but please remember
that one can always compile a kernel with CONFIG_VLAN_8021Q=n. So the
issue predates my patch by much longer. You might reconsider the Fixes:
tag in light of this, maybe the patch needs to be sent to stable.

Okay. So the Fixes tag should be 6087175b7991, which initially adds the
software fallback support for mt7530.

quoted

+static int
+mt7530_setup_vlan0(struct mt7530_priv *priv)
+{
+	u32 val;
+
+	/* Validate the entry with independent learning, keep the original
+	 * ingress tag attribute.
+	 */
+	val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) |

FID_BRIDGED?

What's wrong with that?

quoted

+	      VLAN_VALID;
+	mt7530_write(priv, MT7530_VAWD1, val);
+
+	return mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, 0);
+}

On Wed, Aug 25, 2021 at 01:32:37AM +0800, DENG Qingfang wrote:

On Tue, Aug 24, 2021 at 07:57:42PM +0300, Vladimir Oltean wrote:

quoted

I understand that this is how you noticed the issue, but please remember
that one can always compile a kernel with CONFIG_VLAN_8021Q=n. So the
issue predates my patch by much longer. You might reconsider the Fixes:
tag in light of this, maybe the patch needs to be sent to stable.

Okay. So the Fixes tag should be 6087175b7991, which initially adds the
software fallback support for mt7530.

Ok. Did the old code not need VLAN 0 for VLAN-unaware ports, or are you
saying that since the VLAN table lookup was bypassed completely in the
old code, 'no VLAN 0' was an inconsequential error?

I think it's the latter. Just wanted to make sure. So that means, either
this Fixes: tag or the other, the patch still belongs to net-next. From
my side you shouldn't need to resend.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

quoted

quoted

+static int
+mt7530_setup_vlan0(struct mt7530_priv *priv)
+{
+	u32 val;
+
+	/* Validate the entry with independent learning, keep the original
+	 * ingress tag attribute.
+	 */
+	val = IVL_MAC | EG_CON | PORT_MEM(MT7530_ALL_MEMBERS) | FID(FID_BRIDGED) |

FID_BRIDGED?

What's wrong with that?

Nothing, I had a senior moment and I forgot how mt7530 sets up things.

On 8/24/2021 6:52 PM, DENG Qingfang wrote:

The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After
the blamed commit, VLAN ID 0 won't be set up anymore, breaking software
bridging fallback on VLAN-unaware bridges.

Manually set up VLAN ID 0 to fix this.

Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

On Tue, Aug 24, 2021 at 08:37:14PM +0300, Vladimir Oltean wrote:

On Wed, Aug 25, 2021 at 01:32:37AM +0800, DENG Qingfang wrote:

quoted

Okay. So the Fixes tag should be 6087175b7991, which initially adds the
software fallback support for mt7530.

Ok. Did the old code not need VLAN 0 for VLAN-unaware ports, or are you
saying that since the VLAN table lookup was bypassed completely in the
old code, 'no VLAN 0' was an inconsequential error?

I think it's the latter. Just wanted to make sure. So that means, either
this Fixes: tag or the other, the patch still belongs to net-next. From
my side you shouldn't need to resend.

You're right. The old code does not use VLAN table lookup for VLAN-unaware
ports, and the current code set VLAN-unaware ports to fallback mode so
missing VLAN 0 will only make them fallback to SVL.

Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Wed, 25 Aug 2021 00:52:52 +0800 you wrote:

The driver was relying on dsa_slave_vlan_rx_add_vid to add VLAN ID 0. After
the blamed commit, VLAN ID 0 won't be set up anymore, breaking software
bridging fallback on VLAN-unaware bridges.

Manually set up VLAN ID 0 to fix this.

Fixes: 06cfb2df7eb0 ("net: dsa: don't advertise 'rx-vlan-filter' when not needed")
Signed-off-by: DENG Qingfang <dqfext@gmail.com>

[...]

Here is the summary with links:
  - [net-next] net: dsa: mt7530: manually set up VLAN ID 0
    https://git.kernel.org/netdev/net-next/c/1ca8a193cade

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html