[PATCH net 0/3] Fix VLAN checks for SJA1105 DSA tc-flower filters

[PATCH net 0/3] Fix VLAN checks for SJA1105 DSA tc-flower filters

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This fixes a ridiculous situation where the driver, in VLAN-unaware
mode, would refuse accepting any tc filter:

tc filter replace dev sw1p3 ingress flower skip_sw \
	dst_mac 42:be:24:9b:76:20 \
	action gate (...)
Error: sja1105: Can only gate based on {DMAC, VID, PCP}.

tc filter replace dev sw1p3 ingress protocol 802.1Q flower skip_sw \
	vlan_id 1 vlan_prio 0 dst_mac 42:be:24:9b:76:20 \
	action gate (...)
Error: sja1105: Can only gate based on DMAC.

So, without changing the VLAN awareness state, it says it doesn't want
VLAN-aware rules, and it doesn't want VLAN-unaware rules either. One
would say it's in Schrodinger's state...

Now, the situation has been made worse by commit 7f14937facdc ("net:
dsa: sja1105: keep the VLAN awareness state in a driver variable"),
which made VLAN awareness a ternary attribute, but after inspecting the
code from before that patch with a truth table, it looks like the
logical bug was there even before.

While attempting to fix this, I also noticed some leftover debugging
code in one of the places that needed to be fixed. It would have
appeared in the context of patch 3/3 anyway, so I decided to create a
patch that removes it.

Vladimir Oltean (3):
  net: dsa: sja1105: remove debugging code in sja1105_vl_gate
  net: dsa: sja1105: fix checks for VLAN state in redirect action
  net: dsa: sja1105: fix checks for VLAN state in gate action

 drivers/net/dsa/sja1105/sja1105_vl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH net 1/3] net: dsa: sja1105: remove debugging code in sja1105_vl_gate

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This shouldn't be there.

Fixes: 834f8933d5dd ("net: dsa: sja1105: implement tc-gate using time-triggered virtual links")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_vl.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index bdfd6c4e190d..32eca3e660e1 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -588,14 +588,10 @@ int sja1105_vl_gate(struct sja1105_private *priv, int port,
 
 	if (priv->vlan_state == SJA1105_VLAN_UNAWARE &&
 	    key->type != SJA1105_KEY_VLAN_UNAWARE_VL) {
-		dev_err(priv->ds->dev, "1: vlan state %d key type %d\n",
-			priv->vlan_state, key->type);
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Can only gate based on DMAC");
 		return -EOPNOTSUPP;
 	} else if (key->type != SJA1105_KEY_VLAN_AWARE_VL) {
-		dev_err(priv->ds->dev, "2: vlan state %d key type %d\n",
-			priv->vlan_state, key->type);
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Can only gate based on {DMAC, VID, PCP}");
 		return -EOPNOTSUPP;
-- 
2.25.1

[PATCH net 2/3] net: dsa: sja1105: fix checks for VLAN state in redirect action

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This action requires the VLAN awareness state of the switch to be of the
same type as the key that's being added:

- If the switch is unaware of VLAN, then the tc filter key must only
  contain the destination MAC address.
- If the switch is VLAN-aware, the key must also contain the VLAN ID and
  PCP.

But this check doesn't work unless we verify the VLAN awareness state on
both the "if" and the "else" branches.

Fixes: dfacc5a23e22 ("net: dsa: sja1105: support flow-based redirection via virtual links")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_vl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index 32eca3e660e1..a176f39a052b 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -342,7 +342,9 @@ int sja1105_vl_redirect(struct sja1105_private *priv, int port,
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Can only redirect based on DMAC");
 		return -EOPNOTSUPP;
-	} else if (key->type != SJA1105_KEY_VLAN_AWARE_VL) {
+	} else if ((priv->vlan_state == SJA1105_VLAN_BEST_EFFORT ||
+		    priv->vlan_state == SJA1105_VLAN_FILTERING_FULL) &&
+		   key->type != SJA1105_KEY_VLAN_AWARE_VL) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Can only redirect based on {DMAC, VID, PCP}");
 		return -EOPNOTSUPP;
-- 
2.25.1

[PATCH net 3/3] net: dsa: sja1105: fix checks for VLAN state in gate action

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This action requires the VLAN awareness state of the switch to be of the
same type as the key that's being added:

- If the switch is unaware of VLAN, then the tc filter key must only
  contain the destination MAC address.
- If the switch is VLAN-aware, the key must also contain the VLAN ID and
  PCP.

But this check doesn't work unless we verify the VLAN awareness state on
both the "if" and the "else" branches.

Fixes: 834f8933d5dd ("net: dsa: sja1105: implement tc-gate using time-triggered virtual links")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_vl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index a176f39a052b..0056f9c1e471 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -593,7 +593,9 @@ int sja1105_vl_gate(struct sja1105_private *priv, int port,
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Can only gate based on DMAC");
 		return -EOPNOTSUPP;
-	} else if (key->type != SJA1105_KEY_VLAN_AWARE_VL) {
+	} else if ((priv->vlan_state == SJA1105_VLAN_BEST_EFFORT ||
+		    priv->vlan_state == SJA1105_VLAN_FILTERING_FULL) &&
+		   key->type != SJA1105_KEY_VLAN_AWARE_VL) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Can only gate based on {DMAC, VID, PCP}");
 		return -EOPNOTSUPP;
-- 
2.25.1

Re: [PATCH net 0/3] Fix VLAN checks for SJA1105 DSA tc-flower filters

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Wed, 17 Jun 2020 02:58:40 +0300

> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This fixes a ridiculous situation where the driver, in VLAN-unaware
> mode, would refuse accepting any tc filter:
> 
> tc filter replace dev sw1p3 ingress flower skip_sw \
> 	dst_mac 42:be:24:9b:76:20 \
> 	action gate (...)
> Error: sja1105: Can only gate based on {DMAC, VID, PCP}.
> 
> tc filter replace dev sw1p3 ingress protocol 802.1Q flower skip_sw \
> 	vlan_id 1 vlan_prio 0 dst_mac 42:be:24:9b:76:20 \
> 	action gate (...)
> Error: sja1105: Can only gate based on DMAC.
> 
> So, without changing the VLAN awareness state, it says it doesn't want
> VLAN-aware rules, and it doesn't want VLAN-unaware rules either. One
> would say it's in Schrodinger's state...
> 
> Now, the situation has been made worse by commit 7f14937facdc ("net:
> dsa: sja1105: keep the VLAN awareness state in a driver variable"),
> which made VLAN awareness a ternary attribute, but after inspecting the
> code from before that patch with a truth table, it looks like the
> logical bug was there even before.
> 
> While attempting to fix this, I also noticed some leftover debugging
> code in one of the places that needed to be fixed. It would have
> appeared in the context of patch 3/3 anyway, so I decided to create a
> patch that removes it.

Series applied, thanks.