diff mbox series

[19.07] ipq40xx: Revert "ipq40xx: fix ethernet vlan double tagging"

Message ID 20201120102114.894966-1-baptiste@bitsofnetworks.org
State Changes Requested
Headers show
Series [19.07] ipq40xx: Revert "ipq40xx: fix ethernet vlan double tagging" | expand

Commit Message

Baptiste Jonglez Nov. 20, 2020, 10:21 a.m. UTC 
From: Baptiste Jonglez <git@bitsofnetworks.org>

This change has been causing several issues on ipq40xx devices, including:

- VLAN tagging no longer works correctly: https://bugs.openwrt.org/index.php?do=details&task_id=3239
- poor performance with tagged VLANs: https://bugs.openwrt.org/index.php?do=details&task_id=3457

See also https://forum.openwrt.org/t/vlan-tagging-on-ipq40xx-gl-b1300-no-longer-works/69569

There are have been discussions on ways to fix the issue in the links
above (including switching to DSA), but nothing that can realistically
be introduced in the 19.07 branch.

This reverts commit 8c191712558ce94 ("ipq40xx: fix ethernet vlan
double tagging")

Note that it's not a clean revert because this patch was touched in
148d59c67edd5 ("kernel: update kernel 4.14 to version 4.14.193") even
though the semantic of the patch was left unchanged.

Fixes: FS#3239
Fixes: FS#3457
---
 .../716-essedma-vlan-double-tag.patch         | 128 ------------------
 1 file changed, 128 deletions(-)
 delete mode 100644 target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-tag.patch

Comments

Adrian Schmutzler Nov. 20, 2020, 2:09 p.m. UTC | #1 
Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> On Behalf Of Baptiste Jonglez
> Sent: Freitag, 20. November 2020 11:21
> To: openwrt-devel@lists.openwrt.org; John Crispin <john@phrozen.org>
> Cc: Baptiste Jonglez <git@bitsofnetworks.org>
> Subject: [PATCH 19.07] ipq40xx: Revert "ipq40xx: fix ethernet vlan double
> tagging"
> 
> From: Baptiste Jonglez <git@bitsofnetworks.org>
> 
> This change has been causing several issues on ipq40xx devices, including:

this seems to lack a Signed-off-by?

Best

Adrian

> 
> - VLAN tagging no longer works correctly:
> https://bugs.openwrt.org/index.php?do=details&task_id=3239
> - poor performance with tagged VLANs:
> https://bugs.openwrt.org/index.php?do=details&task_id=3457
> 
> See also https://forum.openwrt.org/t/vlan-tagging-on-ipq40xx-gl-b1300-no-
> longer-works/69569
> 
> There are have been discussions on ways to fix the issue in the links above
> (including switching to DSA), but nothing that can realistically be introduced in
> the 19.07 branch.
> 
> This reverts commit 8c191712558ce94 ("ipq40xx: fix ethernet vlan double
> tagging")
> 
> Note that it's not a clean revert because this patch was touched in
> 148d59c67edd5 ("kernel: update kernel 4.14 to version 4.14.193") even
> though the semantic of the patch was left unchanged.
> 
> Fixes: FS#3239
> Fixes: FS#3457
> ---
>  .../716-essedma-vlan-double-tag.patch         | 128 ------------------
>  1 file changed, 128 deletions(-)
>  delete mode 100644 target/linux/ipq40xx/patches-4.14/716-essedma-vlan-
> double-tag.patch
> 
> diff --git a/target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-
> tag.patch b/target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-
> tag.patch
> deleted file mode 100644
> index e268351273..0000000000
> --- a/target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-tag.patch
> +++ /dev/null
> @@ -1,128 +0,0 @@
> -From: Sven Eckelmann <sven@narfation.org>
> -Date: Wed, 8 Feb 2017 16:26:00 +0100
> -Subject: [PATCH] ipq40xx: Fix ar40xx port separation
> -
> -It is currently not possible to submit (or receive) VLAN tagged frames over -
> the ar40xx PHY switch and the edma ethernet device.
> -
> -This can be worked around by disabling enable_vlan. The separation of the
> -eth0 and eth1 ports is then done by the vlan_tag information from the -
> device tree. But the ar40xx PHY switch then also has to parse the word3 -port
> bitmap (word3) from the TDP when data was received from the CPU port -
> (0).
> -
> -IssueID: #2857
> -
> -Forwarded: no
> - The ar40xx.c change was forwarded to Xiaofei Shen
> <xiaofeis@codeaurora.org>
> - (QCA). But John Crispin will rewrite the driver anyway and we have to check
> - later if this change is required in his driver too.
> ----
> - drivers/net/phy/ar40xx.c | 6 +++++-
> - 1 file changed, 5 insertions(+), 1 deletion(-)
> -
> ---- a/drivers/net/phy/ar40xx.c
> -+++ b/drivers/net/phy/ar40xx.c
> -@@ -1200,7 +1200,11 @@ ar40xx_init_port(struct ar40xx_priv *pri
> - 	ar40xx_rmw(priv, AR40XX_REG_PORT_STATUS(port),
> - 			AR40XX_PORT_AUTO_LINK_EN, 0);
> -
> --	ar40xx_write(priv, AR40XX_REG_PORT_HEADER(port), 0);
> -+	/* CPU port is setting headers to limit output ports */
> -+	if (port == 0)
> -+		ar40xx_write(priv, AR40XX_REG_PORT_HEADER(port), 0x8);
> -+	else
> -+		ar40xx_write(priv, AR40XX_REG_PORT_HEADER(port), 0);
> -
> - 	ar40xx_write(priv, AR40XX_REG_PORT_VLAN0(port), 0);
> -
> -@@ -1243,6 +1247,10 @@ ar40xx_init_globals(struct ar40xx_priv *
> - 	t = (AR40XX_PORT0_FC_THRESH_ON_DFLT << 16) |
> - 	      AR40XX_PORT0_FC_THRESH_OFF_DFLT;
> - 	ar40xx_write(priv, AR40XX_REG_PORT_FLOWCTRL_THRESH(0), t);
> -+
> -+	/* set service tag to 802.1q */
> -+	t = ETH_P_8021Q | AR40XX_ESS_SERVICE_TAG_STAG;
> -+	ar40xx_write(priv, AR40XX_ESS_SERVICE_TAG, t);
> - }
> -
> - static void
> -@@ -1568,7 +1576,11 @@ ar40xx_setup_port(struct ar40xx_priv *pr
> - 	u32 pvid = priv->vlan_id[priv->pvid[port]];
> -
> - 	if (priv->vlan) {
> --		egress = AR40XX_PORT_VLAN1_OUT_MODE_UNMOD;
> -+		if (priv->vlan_tagged & BIT(port))
> -+			egress = AR40XX_PORT_VLAN1_OUT_MODE_TAG;
> -+		else
> -+			egress =
> AR40XX_PORT_VLAN1_OUT_MODE_UNMOD;
> -+
> - 		ingress = AR40XX_IN_SECURE;
> - 	} else {
> - 		egress = AR40XX_PORT_VLAN1_OUT_MODE_UNTOUCH;
> -@@ -1579,8 +1591,17 @@ ar40xx_setup_port(struct ar40xx_priv *pr
> - 	t |= pvid << AR40XX_PORT_VLAN0_DEF_CVID_S;
> - 	ar40xx_write(priv, AR40XX_REG_PORT_VLAN0(port), t);
> -
> --	t = AR40XX_PORT_VLAN1_PORT_VLAN_PROP;
> --	t |= egress << AR40XX_PORT_VLAN1_OUT_MODE_S;
> -+	t = egress << AR40XX_PORT_VLAN1_OUT_MODE_S;
> -+
> -+	/* set CPU port to core port */
> -+	if (port == 0)
> -+		t |= AR40XX_PORT_VLAN1_CORE_PORT;
> -+
> -+	if (priv->vlan_tagged & BIT(port))
> -+		t |= AR40XX_PORT_VLAN1_PORT_VLAN_PROP;
> -+	else
> -+		t |= AR40XX_PORT_VLAN1_PORT_TLS_MODE;
> -+
> - 	ar40xx_write(priv, AR40XX_REG_PORT_VLAN1(port), t);
> -
> - 	t = members;
> ---- a/drivers/net/ethernet/qualcomm/essedma/edma_axi.c
> -+++ b/drivers/net/ethernet/qualcomm/essedma/edma_axi.c
> -@@ -970,7 +970,6 @@ static int edma_axi_probe(struct platfor
> - 		edma_netdev[i]->netdev_ops = &edma_axi_netdev_ops;
> - 		edma_netdev[i]->max_mtu = 9000;
> - 		edma_netdev[i]->features = NETIF_F_HW_CSUM |
> NETIF_F_RXCSUM
> --				      | NETIF_F_HW_VLAN_CTAG_TX
> - 				      | NETIF_F_HW_VLAN_CTAG_RX |
> NETIF_F_SG |
> - 				      NETIF_F_TSO | NETIF_F_GRO;
> - 		edma_netdev[i]->hw_features = NETIF_F_HW_CSUM |
> NETIF_F_RXCSUM |
> -@@ -982,10 +981,10 @@ static int edma_axi_probe(struct platfor
> - 					     NETIF_F_TSO | NETIF_F_GRO;
> -
> - #ifdef CONFIG_RFS_ACCEL
> --		edma_netdev[i]->features |=  NETIF_F_RXHASH |
> NETIF_F_NTUPLE;
> --		edma_netdev[i]->hw_features |=  NETIF_F_RXHASH |
> NETIF_F_NTUPLE;
> --		edma_netdev[i]->vlan_features |= NETIF_F_RXHASH |
> NETIF_F_NTUPLE;
> --		edma_netdev[i]->wanted_features |= NETIF_F_RXHASH |
> NETIF_F_NTUPLE;
> -+		edma_netdev[i]->features |=  NETIF_F_NTUPLE;
> -+		edma_netdev[i]->hw_features |=  NETIF_F_NTUPLE;
> -+		edma_netdev[i]->vlan_features |= NETIF_F_NTUPLE;
> -+		edma_netdev[i]->wanted_features |= NETIF_F_NTUPLE;
> - #endif
> - 		edma_set_ethtool_ops(edma_netdev[i]);
> -
> ---- a/drivers/net/phy/ar40xx.h
> -+++ b/drivers/net/phy/ar40xx.h
> -@@ -151,6 +151,9 @@ struct ar40xx_mib_desc {
> - #define   AR40XX_MIB_FUNC_NO_OP		0x0
> - #define   AR40XX_MIB_FUNC_FLUSH		0x1
> -
> -+#define AR40XX_ESS_SERVICE_TAG		0x48
> -+#define AR40XX_ESS_SERVICE_TAG_STAG	BIT(17)
> -+
> - #define AR40XX_REG_PORT_STATUS(_i)		(0x07c + (_i) * 4)
> - #define   AR40XX_PORT_SPEED			BITS(0, 2)
> - #define   AR40XX_PORT_STATUS_SPEED_S	0
> -@@ -179,6 +182,8 @@ struct ar40xx_mib_desc {
> - #define   AR40XX_PORT_VLAN0_DEF_CVID_S		16
> -
> - #define AR40XX_REG_PORT_VLAN1(_i)		(0x424 + (_i) * 0x8)
> -+#define   AR40XX_PORT_VLAN1_CORE_PORT		BIT(9)
> -+#define   AR40XX_PORT_VLAN1_PORT_TLS_MODE	BIT(7)
> - #define   AR40XX_PORT_VLAN1_PORT_VLAN_PROP	BIT(6)
> - #define   AR40XX_PORT_VLAN1_OUT_MODE		BITS(12, 2)
> - #define   AR40XX_PORT_VLAN1_OUT_MODE_S		12
> --
> 2.29.2
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Baptiste Jonglez Nov. 20, 2020, 2:30 p.m. UTC | #2 
Hi,

On 20-11-20, Adrian Schmutzler wrote:
> > -----Original Message-----
> > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
> > On Behalf Of Baptiste Jonglez
> > Sent: Freitag, 20. November 2020 11:21
> > To: openwrt-devel@lists.openwrt.org; John Crispin <john@phrozen.org>
> > Cc: Baptiste Jonglez <git@bitsofnetworks.org>
> > Subject: [PATCH 19.07] ipq40xx: Revert "ipq40xx: fix ethernet vlan double
> > tagging"
> > 
> > From: Baptiste Jonglez <git@bitsofnetworks.org>
> > 
> > This change has been causing several issues on ipq40xx devices, including:
> 
> this seems to lack a Signed-off-by?

You're totally right, thanks for spotting this.  I've just sent a v2.

Baptiste
Nick Nov. 20, 2020, 8:42 p.m. UTC | #3 
I added a kernel flag to differentiate between both driver versions.
https://github.com/openwrt/openwrt/pull/3596

I would backport this to 19.07 if it gets accepted.

On 11/20/20 3:30 PM, Baptiste Jonglez wrote:
> Hi,
>
> On 20-11-20, Adrian Schmutzler wrote:
>>> -----Original Message-----
>>> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org]
>>> On Behalf Of Baptiste Jonglez
>>> Sent: Freitag, 20. November 2020 11:21
>>> To: openwrt-devel@lists.openwrt.org; John Crispin <john@phrozen.org>
>>> Cc: Baptiste Jonglez <git@bitsofnetworks.org>
>>> Subject: [PATCH 19.07] ipq40xx: Revert "ipq40xx: fix ethernet vlan double
>>> tagging"
>>>
>>> From: Baptiste Jonglez <git@bitsofnetworks.org>
>>>
>>> This change has been causing several issues on ipq40xx devices, including:
>> this seems to lack a Signed-off-by?
> You're totally right, thanks for spotting this.  I've just sent a v2.
>
> Baptiste
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff mbox series

Patch

diff --git a/target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-tag.patch b/target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-tag.patch
deleted file mode 100644
index e268351273..0000000000
--- a/target/linux/ipq40xx/patches-4.14/716-essedma-vlan-double-tag.patch
+++ /dev/null
@@ -1,128 +0,0 @@ 
-From: Sven Eckelmann <sven@narfation.org>
-Date: Wed, 8 Feb 2017 16:26:00 +0100
-Subject: [PATCH] ipq40xx: Fix ar40xx port separation
-
-It is currently not possible to submit (or receive) VLAN tagged frames over
-the ar40xx PHY switch and the edma ethernet device.
-
-This can be worked around by disabling enable_vlan. The separation of the
-eth0 and eth1 ports is then done by the vlan_tag information from the
-device tree. But the ar40xx PHY switch then also has to parse the word3
-port bitmap (word3) from the TDP when data was received from the CPU port
-(0).
-
-IssueID: #2857
-
-Forwarded: no
- The ar40xx.c change was forwarded to Xiaofei Shen <xiaofeis@codeaurora.org>
- (QCA). But John Crispin will rewrite the driver anyway and we have to check
- later if this change is required in his driver too.
----
- drivers/net/phy/ar40xx.c | 6 +++++-
- 1 file changed, 5 insertions(+), 1 deletion(-)
-
---- a/drivers/net/phy/ar40xx.c
-+++ b/drivers/net/phy/ar40xx.c
-@@ -1200,7 +1200,11 @@ ar40xx_init_port(struct ar40xx_priv *pri
- 	ar40xx_rmw(priv, AR40XX_REG_PORT_STATUS(port),
- 			AR40XX_PORT_AUTO_LINK_EN, 0);
- 
--	ar40xx_write(priv, AR40XX_REG_PORT_HEADER(port), 0);
-+	/* CPU port is setting headers to limit output ports */
-+	if (port == 0)
-+		ar40xx_write(priv, AR40XX_REG_PORT_HEADER(port), 0x8);
-+	else
-+		ar40xx_write(priv, AR40XX_REG_PORT_HEADER(port), 0);
- 
- 	ar40xx_write(priv, AR40XX_REG_PORT_VLAN0(port), 0);
- 
-@@ -1243,6 +1247,10 @@ ar40xx_init_globals(struct ar40xx_priv *
- 	t = (AR40XX_PORT0_FC_THRESH_ON_DFLT << 16) |
- 	      AR40XX_PORT0_FC_THRESH_OFF_DFLT;
- 	ar40xx_write(priv, AR40XX_REG_PORT_FLOWCTRL_THRESH(0), t);
-+
-+	/* set service tag to 802.1q */
-+	t = ETH_P_8021Q | AR40XX_ESS_SERVICE_TAG_STAG;
-+	ar40xx_write(priv, AR40XX_ESS_SERVICE_TAG, t);
- }
- 
- static void
-@@ -1568,7 +1576,11 @@ ar40xx_setup_port(struct ar40xx_priv *pr
- 	u32 pvid = priv->vlan_id[priv->pvid[port]];
- 
- 	if (priv->vlan) {
--		egress = AR40XX_PORT_VLAN1_OUT_MODE_UNMOD;
-+		if (priv->vlan_tagged & BIT(port))
-+			egress = AR40XX_PORT_VLAN1_OUT_MODE_TAG;
-+		else
-+			egress = AR40XX_PORT_VLAN1_OUT_MODE_UNMOD;
-+
- 		ingress = AR40XX_IN_SECURE;
- 	} else {
- 		egress = AR40XX_PORT_VLAN1_OUT_MODE_UNTOUCH;
-@@ -1579,8 +1591,17 @@ ar40xx_setup_port(struct ar40xx_priv *pr
- 	t |= pvid << AR40XX_PORT_VLAN0_DEF_CVID_S;
- 	ar40xx_write(priv, AR40XX_REG_PORT_VLAN0(port), t);
- 
--	t = AR40XX_PORT_VLAN1_PORT_VLAN_PROP;
--	t |= egress << AR40XX_PORT_VLAN1_OUT_MODE_S;
-+	t = egress << AR40XX_PORT_VLAN1_OUT_MODE_S;
-+
-+	/* set CPU port to core port */
-+	if (port == 0)
-+		t |= AR40XX_PORT_VLAN1_CORE_PORT;
-+
-+	if (priv->vlan_tagged & BIT(port))
-+		t |= AR40XX_PORT_VLAN1_PORT_VLAN_PROP;
-+	else
-+		t |= AR40XX_PORT_VLAN1_PORT_TLS_MODE;
-+
- 	ar40xx_write(priv, AR40XX_REG_PORT_VLAN1(port), t);
- 
- 	t = members;
---- a/drivers/net/ethernet/qualcomm/essedma/edma_axi.c
-+++ b/drivers/net/ethernet/qualcomm/essedma/edma_axi.c
-@@ -970,7 +970,6 @@ static int edma_axi_probe(struct platfor
- 		edma_netdev[i]->netdev_ops = &edma_axi_netdev_ops;
- 		edma_netdev[i]->max_mtu = 9000;
- 		edma_netdev[i]->features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM
--				      | NETIF_F_HW_VLAN_CTAG_TX
- 				      | NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_SG |
- 				      NETIF_F_TSO | NETIF_F_GRO;
- 		edma_netdev[i]->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM |
-@@ -982,10 +981,10 @@ static int edma_axi_probe(struct platfor
- 					     NETIF_F_TSO | NETIF_F_GRO;
- 
- #ifdef CONFIG_RFS_ACCEL
--		edma_netdev[i]->features |=  NETIF_F_RXHASH | NETIF_F_NTUPLE;
--		edma_netdev[i]->hw_features |=  NETIF_F_RXHASH | NETIF_F_NTUPLE;
--		edma_netdev[i]->vlan_features |= NETIF_F_RXHASH | NETIF_F_NTUPLE;
--		edma_netdev[i]->wanted_features |= NETIF_F_RXHASH | NETIF_F_NTUPLE;
-+		edma_netdev[i]->features |=  NETIF_F_NTUPLE;
-+		edma_netdev[i]->hw_features |=  NETIF_F_NTUPLE;
-+		edma_netdev[i]->vlan_features |= NETIF_F_NTUPLE;
-+		edma_netdev[i]->wanted_features |= NETIF_F_NTUPLE;
- #endif
- 		edma_set_ethtool_ops(edma_netdev[i]);
- 
---- a/drivers/net/phy/ar40xx.h
-+++ b/drivers/net/phy/ar40xx.h
-@@ -151,6 +151,9 @@ struct ar40xx_mib_desc {
- #define   AR40XX_MIB_FUNC_NO_OP		0x0
- #define   AR40XX_MIB_FUNC_FLUSH		0x1
- 
-+#define AR40XX_ESS_SERVICE_TAG		0x48
-+#define AR40XX_ESS_SERVICE_TAG_STAG	BIT(17)
-+
- #define AR40XX_REG_PORT_STATUS(_i)		(0x07c + (_i) * 4)
- #define   AR40XX_PORT_SPEED			BITS(0, 2)
- #define   AR40XX_PORT_STATUS_SPEED_S	0
-@@ -179,6 +182,8 @@ struct ar40xx_mib_desc {
- #define   AR40XX_PORT_VLAN0_DEF_CVID_S		16
- 
- #define AR40XX_REG_PORT_VLAN1(_i)		(0x424 + (_i) * 0x8)
-+#define   AR40XX_PORT_VLAN1_CORE_PORT		BIT(9)
-+#define   AR40XX_PORT_VLAN1_PORT_TLS_MODE	BIT(7)
- #define   AR40XX_PORT_VLAN1_PORT_VLAN_PROP	BIT(6)
- #define   AR40XX_PORT_VLAN1_OUT_MODE		BITS(12, 2)
- #define   AR40XX_PORT_VLAN1_OUT_MODE_S		12