diff mbox series

[net,v2] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging

Message ID 20230426172153.8352-1-linux@fw-web.de (mailing list archive)
State Accepted
Commit c6d96df9fa2c1d19525239d4262889cce594ce6c
Delegated to: Netdev Maintainers
Headers show
Series [net,v2] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/cc_maintainers success CCed 15 of 15 maintainers
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Frank Wunderlich April 26, 2023, 5:21 p.m. UTC 
From: Felix Fietkau <nbd@nbd.name>

Through testing I found out that hardware vlan rx offload support seems to
have some hardware issues. At least when using multiple MACs and when
receiving tagged packets on the secondary MAC, the hardware can sometimes
start to emit wrong tags on the first MAC as well.

In order to avoid such issues, drop the feature configuration and use
the offload feature only for DSA hardware untagging on MT7621/MT7622
devices where this feature works properly.

Fixes: 08666cbb7dd5 ("net: ethernet: mtk_eth_soc: add support for configuring vlan rx offload")
Tested-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
v2:
- changed commit message to drop "only one MAC used" phrase based on
  Arincs comments
- fixed too long line in commit description and add empty line after
  declaration
- add fixes tag

used felix Patch as base and ported up to 6.3-rc6

it basicly reverts changes from vladimirs patch

1a3245fe0cf8 net: ethernet: mtk_eth_soc: fix DSA TX tag hwaccel for switch port 0

tested this on bananapi-r3 on non-dsa gmac1 and dsa eth0 (wan).
on both vlan is working, but maybe it breaks HW-vlan-untagging
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 106 ++++++++------------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |   1 -
 2 files changed, 40 insertions(+), 67 deletions(-)

Comments

Frank Wunderlich April 26, 2023, 5:31 p.m. UTC | #1 
> Gesendet: Mittwoch, 26. April 2023 um 19:25 Uhr
> Von: "Arınç ÜNAL" <arinc.unal@arinc9.com>
> On 26/04/2023 20:21, Frank Wunderlich wrote:
> > From: Felix Fietkau <nbd@nbd.name>
> > 
> > Through testing I found out that hardware vlan rx offload support seems to
> > have some hardware issues. At least when using multiple MACs and when
> > receiving tagged packets on the secondary MAC, the hardware can sometimes
> > start to emit wrong tags on the first MAC as well.
> > 
> > In order to avoid such issues, drop the feature configuration and use
> > the offload feature only for DSA hardware untagging on MT7621/MT7622
> > devices where this feature works properly.
> > 
> > Fixes: 08666cbb7dd5 ("net: ethernet: mtk_eth_soc: add support for configuring vlan rx offload")
> > Tested-by: Frank Wunderlich <frank-w@public-files.de>
> > Signed-off-by: Felix Fietkau <nbd@nbd.name>
> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> I'm confused by this. What is HW-vlan-untagging, and which SoCs do you 
> think this patch would break this feature? How can I utilise this 
> feature on Linux so I can confirm whether it works or not?

the feature itself breaks vlan on mac of bpi-r3

1 mac is connected to switch and uses dsa tags, the other mac is directly accessible and vlan-tag
there is stripped by this feature.

with this patch i can use vlans on the "standalone" mac again (see tagged packets incoming).

regards Frank
Arınç ÜNAL April 26, 2023, 5:51 p.m. UTC | #2 
On 26/04/2023 20:31, Frank Wunderlich wrote:
> 
>> Gesendet: Mittwoch, 26. April 2023 um 19:25 Uhr
>> Von: "Arınç ÜNAL" <arinc.unal@arinc9.com>
>> On 26/04/2023 20:21, Frank Wunderlich wrote:
>>> From: Felix Fietkau <nbd@nbd.name>
>>>
>>> Through testing I found out that hardware vlan rx offload support seems to
>>> have some hardware issues. At least when using multiple MACs and when
>>> receiving tagged packets on the secondary MAC, the hardware can sometimes
>>> start to emit wrong tags on the first MAC as well.
>>>
>>> In order to avoid such issues, drop the feature configuration and use
>>> the offload feature only for DSA hardware untagging on MT7621/MT7622
>>> devices where this feature works properly.
>>>
>>> Fixes: 08666cbb7dd5 ("net: ethernet: mtk_eth_soc: add support for configuring vlan rx offload")
>>> Tested-by: Frank Wunderlich <frank-w@public-files.de>
>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
>>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> I'm confused by this. What is HW-vlan-untagging, and which SoCs do you
>> think this patch would break this feature? How can I utilise this
>> feature on Linux so I can confirm whether it works or not?
> 
> the feature itself breaks vlan on mac of bpi-r3
> 
> 1 mac is connected to switch and uses dsa tags, the other mac is directly accessible and vlan-tag
> there is stripped by this feature.
> 
> with this patch i can use vlans on the "standalone" mac again (see tagged packets incoming).

Ok, since this patch is disabling the feature, the patch cannot possibly 
break the feature. That's why I was confused as to why you mentioned 
this in a way that gives the impression that this patch may break the 
said feature.

Anyhow:

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Arınç
Frank Wunderlich April 26, 2023, 5:52 p.m. UTC | #3 
Hi
> Gesendet: Mittwoch, 26. April 2023 um 19:25 Uhr
> Von: "Arınç ÜNAL" <arinc.unal@arinc9.com>

> > tested this on bananapi-r3 on non-dsa gmac1 and dsa eth0 (wan).
> > on both vlan is working, but maybe it breaks HW-vlan-untagging
> 
> I'm confused by this. What is HW-vlan-untagging, and which SoCs do you 
> think this patch would break this feature? How can I utilise this 
> feature on Linux so I can confirm whether it works or not?

oh, you mean my wording about "hw-vlan-untagging"...i mean the hw vlan offload feature which may
not be working on non-mt7621/7622 devices as i have no idea how to check this. i hope felix can
answer this. at least the feature activeated on mt7986 breaks sw-vlan on the gmac1 (without
switch).

regards Frank
Arınç ÜNAL April 26, 2023, 5:58 p.m. UTC | #4 
On 26/04/2023 20:52, Frank Wunderlich wrote:
> Hi
>> Gesendet: Mittwoch, 26. April 2023 um 19:25 Uhr
>> Von: "Arınç ÜNAL" <arinc.unal@arinc9.com>
> 
>>> tested this on bananapi-r3 on non-dsa gmac1 and dsa eth0 (wan).
>>> on both vlan is working, but maybe it breaks HW-vlan-untagging
>>
>> I'm confused by this. What is HW-vlan-untagging, and which SoCs do you
>> think this patch would break this feature? How can I utilise this
>> feature on Linux so I can confirm whether it works or not?
> 
> oh, you mean my wording about "hw-vlan-untagging"...i mean the hw vlan offload feature which may
> not be working on non-mt7621/7622 devices as i have no idea how to check this. i hope felix can
> answer this. at least the feature activeated on mt7986 breaks sw-vlan on the gmac1 (without
> switch).

The "hw vlan offload" feature being broken on non-mt7621/7622 devices is 
already established, hence this patch disabling it for the 
non-mt7621/7622 devices. I still don't understand why you're mentioning 
it in this way, unless you're trying to say something else?

Could you also not trim the relevant parts of the mail on your response. 
It's getting hard to keep track of the conversation.

Arınç
patchwork-bot+netdevbpf@kernel.org May 3, 2023, 3:30 a.m. UTC | #5 
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 26 Apr 2023 19:21:53 +0200 you wrote:
> From: Felix Fietkau <nbd@nbd.name>
> 
> Through testing I found out that hardware vlan rx offload support seems to
> have some hardware issues. At least when using multiple MACs and when
> receiving tagged packets on the secondary MAC, the hardware can sometimes
> start to emit wrong tags on the first MAC as well.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: ethernet: mtk_eth_soc: drop generic vlan rx offload, only use DSA untagging
    https://git.kernel.org/netdev/net/c/c6d96df9fa2c

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 9e948d091a69..a75fd072082c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1918,9 +1918,7 @@  static int mtk_poll_rx(struct napi_struct *napi, int budget,
 
 	while (done < budget) {
 		unsigned int pktlen, *rxdcsum;
-		bool has_hwaccel_tag = false;
 		struct net_device *netdev;
-		u16 vlan_proto, vlan_tci;
 		dma_addr_t dma_addr;
 		u32 hash, reason;
 		int mac = 0;
@@ -2055,31 +2053,16 @@  static int mtk_poll_rx(struct napi_struct *napi, int budget,
 			skb_checksum_none_assert(skb);
 		skb->protocol = eth_type_trans(skb, netdev);
 
-		if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX) {
-			if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
-				if (trxd.rxd3 & RX_DMA_VTAG_V2) {
-					vlan_proto = RX_DMA_VPID(trxd.rxd4);
-					vlan_tci = RX_DMA_VID(trxd.rxd4);
-					has_hwaccel_tag = true;
-				}
-			} else if (trxd.rxd2 & RX_DMA_VTAG) {
-				vlan_proto = RX_DMA_VPID(trxd.rxd3);
-				vlan_tci = RX_DMA_VID(trxd.rxd3);
-				has_hwaccel_tag = true;
-			}
-		}
-
 		/* When using VLAN untagging in combination with DSA, the
 		 * hardware treats the MTK special tag as a VLAN and untags it.
 		 */
-		if (has_hwaccel_tag && netdev_uses_dsa(netdev)) {
-			unsigned int port = vlan_proto & GENMASK(2, 0);
+		if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2) &&
+		    (trxd.rxd2 & RX_DMA_VTAG) && netdev_uses_dsa(netdev)) {
+			unsigned int port = RX_DMA_VPID(trxd.rxd3) & GENMASK(2, 0);
 
 			if (port < ARRAY_SIZE(eth->dsa_meta) &&
 			    eth->dsa_meta[port])
 				skb_dst_set_noref(skb, &eth->dsa_meta[port]->dst);
-		} else if (has_hwaccel_tag) {
-			__vlan_hwaccel_put_tag(skb, htons(vlan_proto), vlan_tci);
 		}
 
 		if (reason == MTK_PPE_CPU_REASON_HIT_UNBIND_RATE_REACHED)
@@ -2907,29 +2890,11 @@  static netdev_features_t mtk_fix_features(struct net_device *dev,
 
 static int mtk_set_features(struct net_device *dev, netdev_features_t features)
 {
-	struct mtk_mac *mac = netdev_priv(dev);
-	struct mtk_eth *eth = mac->hw;
 	netdev_features_t diff = dev->features ^ features;
-	int i;
 
 	if ((diff & NETIF_F_LRO) && !(features & NETIF_F_LRO))
 		mtk_hwlro_netdev_disable(dev);
 
-	/* Set RX VLAN offloading */
-	if (!(diff & NETIF_F_HW_VLAN_CTAG_RX))
-		return 0;
-
-	mtk_w32(eth, !!(features & NETIF_F_HW_VLAN_CTAG_RX),
-		MTK_CDMP_EG_CTRL);
-
-	/* sync features with other MAC */
-	for (i = 0; i < MTK_MAC_COUNT; i++) {
-		if (!eth->netdev[i] || eth->netdev[i] == dev)
-			continue;
-		eth->netdev[i]->features &= ~NETIF_F_HW_VLAN_CTAG_RX;
-		eth->netdev[i]->features |= features & NETIF_F_HW_VLAN_CTAG_RX;
-	}
-
 	return 0;
 }
 
@@ -3247,30 +3212,6 @@  static int mtk_open(struct net_device *dev)
 	struct mtk_eth *eth = mac->hw;
 	int i, err;
 
-	if (mtk_uses_dsa(dev) && !eth->prog) {
-		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
-			struct metadata_dst *md_dst = eth->dsa_meta[i];
-
-			if (md_dst)
-				continue;
-
-			md_dst = metadata_dst_alloc(0, METADATA_HW_PORT_MUX,
-						    GFP_KERNEL);
-			if (!md_dst)
-				return -ENOMEM;
-
-			md_dst->u.port_info.port_id = i;
-			eth->dsa_meta[i] = md_dst;
-		}
-	} else {
-		/* Hardware special tag parsing needs to be disabled if at least
-		 * one MAC does not use DSA.
-		 */
-		u32 val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
-		val &= ~MTK_CDMP_STAG_EN;
-		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
-	}
-
 	err = phylink_of_phy_connect(mac->phylink, mac->of_node, 0);
 	if (err) {
 		netdev_err(dev, "%s: could not attach PHY: %d\n", __func__,
@@ -3309,6 +3250,40 @@  static int mtk_open(struct net_device *dev)
 	phylink_start(mac->phylink);
 	netif_tx_start_all_queues(dev);
 
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
+		return 0;
+
+	if (mtk_uses_dsa(dev) && !eth->prog) {
+		for (i = 0; i < ARRAY_SIZE(eth->dsa_meta); i++) {
+			struct metadata_dst *md_dst = eth->dsa_meta[i];
+
+			if (md_dst)
+				continue;
+
+			md_dst = metadata_dst_alloc(0, METADATA_HW_PORT_MUX,
+						    GFP_KERNEL);
+			if (!md_dst)
+				return -ENOMEM;
+
+			md_dst->u.port_info.port_id = i;
+			eth->dsa_meta[i] = md_dst;
+		}
+	} else {
+		/* Hardware special tag parsing needs to be disabled if at least
+		 * one MAC does not use DSA.
+		 */
+		u32 val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
+
+		val &= ~MTK_CDMP_STAG_EN;
+		mtk_w32(eth, val, MTK_CDMP_IG_CTRL);
+
+		val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
+		val &= ~MTK_CDMQ_STAG_EN;
+		mtk_w32(eth, val, MTK_CDMQ_IG_CTRL);
+
+		mtk_w32(eth, 0, MTK_CDMP_EG_CTRL);
+	}
+
 	return 0;
 }
 
@@ -3793,10 +3768,9 @@  static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 	if (!MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
 		val = mtk_r32(eth, MTK_CDMP_IG_CTRL);
 		mtk_w32(eth, val | MTK_CDMP_STAG_EN, MTK_CDMP_IG_CTRL);
-	}
 
-	/* Enable RX VLan Offloading */
-	mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);
+		mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);
+	}
 
 	/* set interrupt delays based on current Net DIM sample */
 	mtk_dim_rx(&eth->rx_dim.work);
@@ -4453,7 +4427,7 @@  static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 		eth->netdev[id]->hw_features |= NETIF_F_LRO;
 
 	eth->netdev[id]->vlan_features = eth->soc->hw_features &
-		~(NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX);
+		~NETIF_F_HW_VLAN_CTAG_TX;
 	eth->netdev[id]->features |= eth->soc->hw_features;
 	eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index cdcf8534283e..707445f6bcb1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -48,7 +48,6 @@ 
 #define MTK_HW_FEATURES		(NETIF_F_IP_CSUM | \
 				 NETIF_F_RXCSUM | \
 				 NETIF_F_HW_VLAN_CTAG_TX | \
-				 NETIF_F_HW_VLAN_CTAG_RX | \
 				 NETIF_F_SG | NETIF_F_TSO | \
 				 NETIF_F_TSO6 | \
 				 NETIF_F_IPV6_CSUM |\