public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches
@ 2026-01-13  3:11 Daniel Golle
  2026-01-13 14:00 ` Andrew Lunn
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Golle @ 2026-01-13  3:11 UTC (permalink / raw)
  To: Felix Fietkau, John Crispin, Sean Wang, Lorenzo Bianconi,
	Bc-bocun Chen, Rex Lu, Mason-cw Chang, Andrew Lunn,
	David S. Miller, Andrew Lunn, Vladimir Oltean, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno, netdev, linux-kernel,
	linux-arm-kernel, linux-mediatek

MediaTek's Ethernet Frame Engine is tailored for use with their
switches. This broke checksum and VLAN offloading when attaching a
DSA switch which does not use MediaTek special tag format.

As a work-around, make sure checksum offloading is disabled and
make sure bits instructing the FE to parse or modify MTK special
tags aren't set if the attached DSA switch isn't using the
MediaTek special tag format.

Note that this currently also disables checksum and VLAN offloading
when using DSA switches using 802.1Q-based tags, though that those
do work fine and can benefit from the offloading features of the
Ethernet Frame Engine. This is why more feedback from DSA maintainers
regarding this issue would be very welcome.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 44 +++++++++++++++------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index e68997a29191b..654b707ee27a1 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1459,6 +1459,26 @@ static void setup_tx_buf(struct mtk_eth *eth, struct mtk_tx_buf *tx_buf,
 	}
 }
 
+static bool mtk_uses_dsa(struct net_device *dev)
+{
+#if IS_ENABLED(CONFIG_NET_DSA)
+	return netdev_uses_dsa(dev) &&
+	       dev->dsa_ptr->tag_ops->proto == DSA_TAG_PROTO_MTK;
+#else
+	return false;
+#endif
+}
+
+static bool non_mtk_uses_dsa(struct net_device *dev)
+{
+#if IS_ENABLED(CONFIG_NET_DSA)
+	return netdev_uses_dsa(dev) &&
+	       dev->dsa_ptr->tag_ops->proto != DSA_TAG_PROTO_MTK;
+#else
+	return false;
+#endif
+}
+
 static void mtk_tx_set_dma_desc_v1(struct net_device *dev, void *txd,
 				   struct mtk_tx_dma_desc_info *info)
 {
@@ -1531,7 +1551,7 @@ static void mtk_tx_set_dma_desc_v2(struct net_device *dev, void *txd,
 		/* tx checksum offload */
 		if (info->csum)
 			data |= TX_DMA_CHKSUM_V2;
-		if (mtk_is_netsys_v3_or_greater(eth) && netdev_uses_dsa(dev))
+		if (mtk_is_netsys_v3_or_greater(eth) && mtk_uses_dsa(dev))
 			data |= TX_DMA_SPTAG_V3;
 	}
 	WRITE_ONCE(desc->txd5, data);
@@ -2350,7 +2370,7 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
 		 * hardware treats the MTK special tag as a VLAN and untags it.
 		 */
 		if (mtk_is_netsys_v1(eth) && (trxd.rxd2 & RX_DMA_VTAG) &&
-		    netdev_uses_dsa(netdev)) {
+		    mtk_uses_dsa(netdev)) {
 			unsigned int port = RX_DMA_VPID(trxd.rxd3) & GENMASK(2, 0);
 
 			if (port < ARRAY_SIZE(eth->dsa_meta) &&
@@ -3192,6 +3212,14 @@ static netdev_features_t mtk_fix_features(struct net_device *dev,
 		}
 	}
 
+	if ((features & NETIF_F_IP_CSUM) &&
+	    non_mtk_uses_dsa(dev))
+		features &= ~NETIF_F_IP_CSUM;
+
+	if ((features & NETIF_F_IPV6_CSUM) &&
+	    non_mtk_uses_dsa(dev))
+		features &= ~NETIF_F_IPV6_CSUM;
+
 	return features;
 }
 
@@ -3508,23 +3536,13 @@ static void mtk_gdm_config(struct mtk_eth *eth, u32 id, u32 config)
 
 	val |= config;
 
-	if (eth->netdev[id] && netdev_uses_dsa(eth->netdev[id]))
+	if (eth->netdev[id] && mtk_uses_dsa(eth->netdev[id]))
 		val |= MTK_GDMA_SPECIAL_TAG;
 
 	mtk_w32(eth, val, MTK_GDMA_FWD_CFG(id));
 }
 
 
-static bool mtk_uses_dsa(struct net_device *dev)
-{
-#if IS_ENABLED(CONFIG_NET_DSA)
-	return netdev_uses_dsa(dev) &&
-	       dev->dsa_ptr->tag_ops->proto == DSA_TAG_PROTO_MTK;
-#else
-	return false;
-#endif
-}
-
 static int mtk_device_event(struct notifier_block *n, unsigned long event, void *ptr)
 {
 	struct mtk_mac *mac = container_of(n, struct mtk_mac, device_notifier);
-- 
2.52.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches
  2026-01-13  3:11 [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches Daniel Golle
@ 2026-01-13 14:00 ` Andrew Lunn
  2026-01-13 14:22   ` Daniel Golle
  2026-01-14 22:17   ` Vladimir Oltean
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Lunn @ 2026-01-13 14:00 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Felix Fietkau, John Crispin, Sean Wang, Lorenzo Bianconi,
	Bc-bocun Chen, Rex Lu, Mason-cw Chang, Andrew Lunn,
	David S. Miller, Vladimir Oltean, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Tue, Jan 13, 2026 at 03:11:54AM +0000, Daniel Golle wrote:
> MediaTek's Ethernet Frame Engine is tailored for use with their
> switches. This broke checksum and VLAN offloading when attaching a
> DSA switch which does not use MediaTek special tag format.

This has been seen before. The Freescale FEC has similar problems when
combined with a Marvell switch, it cannot find the IP header, and so
checksum offloading does not work.

I thought we solved this be modifying the ndev->feature of the conduit
interface to disable such offloads. But i don't see such code. So i
must be remembering wrongly.

This is assuming the frame engine respects these flags:

/usr/sbin/ethtool -k enp2s0
Features for enp2s0:
rx-checksumming: on
tx-checksumming: on
	tx-checksum-ipv4: on
	tx-checksum-ip-generic: off [fixed]
	tx-checksum-ipv6: on
	tx-checksum-fcoe-crc: off [fixed]
	tx-checksum-sctp: off [fixed]

When you combine a Marvell Ethernet interface with a Marvell switch
offloading works of course. So it probably does require some logic in
the MAC driver to determine if the switch is of the same vendor or
not.

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index e68997a29191b..654b707ee27a1 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -1459,6 +1459,26 @@ static void setup_tx_buf(struct mtk_eth *eth, struct mtk_tx_buf *tx_buf,
>  	}
>  }
>  
> +static bool mtk_uses_dsa(struct net_device *dev)
> +{
> +#if IS_ENABLED(CONFIG_NET_DSA)
> +	return netdev_uses_dsa(dev) &&
> +	       dev->dsa_ptr->tag_ops->proto == DSA_TAG_PROTO_MTK;
> +#else
> +	return false;
> +#endif

I think the concept of determining if the switch is using a specific
tag in order to enable/disable acceleration should be generic. So i
would try to make this an helper in include/next/dsa.h. Any MAC driver
can then use it.

> @@ -1531,7 +1551,7 @@ static void mtk_tx_set_dma_desc_v2(struct net_device *dev, void *txd,
>  		/* tx checksum offload */
>  		if (info->csum)
>  			data |= TX_DMA_CHKSUM_V2;
> -		if (mtk_is_netsys_v3_or_greater(eth) && netdev_uses_dsa(dev))
> +		if (mtk_is_netsys_v3_or_greater(eth) && mtk_uses_dsa(dev))
>  			data |= TX_DMA_SPTAG_V3;

This looks to be in the hot path. Do you really want to do this
evaluation on every frame? You can change the tag protocol via sysfs,
however, dsa_tree_change_tag_proto() will only allow you to change the
tag while the conduit interface is down. So it should be safe to look
at the tag protocol once during open, and cache the result somewhere
local, struct mtk_eth? That should avoid a few cache misses.

> @@ -3192,6 +3212,14 @@ static netdev_features_t mtk_fix_features(struct net_device *dev,
>  		}
>  	}
>  
> +	if ((features & NETIF_F_IP_CSUM) &&
> +	    non_mtk_uses_dsa(dev))
> +		features &= ~NETIF_F_IP_CSUM;
> +
> +	if ((features & NETIF_F_IPV6_CSUM) &&
> +	    non_mtk_uses_dsa(dev))
> +		features &= ~NETIF_F_IPV6_CSUM;
> +


When is mtk_fix_features() actually called? I don't know without
looking at the core. You will want it when open is called, when the
tagging protocol is fixed.

	Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches
  2026-01-13 14:00 ` Andrew Lunn
@ 2026-01-13 14:22   ` Daniel Golle
  2026-01-13 22:38     ` Luiz Angelo Daros de Luca
  2026-01-14 22:17   ` Vladimir Oltean
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Golle @ 2026-01-13 14:22 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Felix Fietkau, John Crispin, Sean Wang, Lorenzo Bianconi,
	Bc-bocun Chen, Rex Lu, Mason-cw Chang, Andrew Lunn,
	David S. Miller, Vladimir Oltean, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Tue, Jan 13, 2026 at 03:00:18PM +0100, Andrew Lunn wrote:
> On Tue, Jan 13, 2026 at 03:11:54AM +0000, Daniel Golle wrote:
> > MediaTek's Ethernet Frame Engine is tailored for use with their
> > switches. This broke checksum and VLAN offloading when attaching a
> > DSA switch which does not use MediaTek special tag format.
> 
> This has been seen before. The Freescale FEC has similar problems when
> combined with a Marvell switch, it cannot find the IP header, and so
> checksum offloading does not work.
> 
> I thought we solved this be modifying the ndev->feature of the conduit
> interface to disable such offloads. But i don't see such code. So i
> must be remembering wrongly.
> 
> This is assuming the frame engine respects these flags:
> 
> /usr/sbin/ethtool -k enp2s0
> Features for enp2s0:
> rx-checksumming: on
> tx-checksumming: on
> 	tx-checksum-ipv4: on
> 	tx-checksum-ip-generic: off [fixed]
> 	tx-checksum-ipv6: on
> 	tx-checksum-fcoe-crc: off [fixed]
> 	tx-checksum-sctp: off [fixed]
> 
> When you combine a Marvell Ethernet interface with a Marvell switch
> offloading works of course. So it probably does require some logic in
> the MAC driver to determine if the switch is of the same vendor or
> not.

MediaTek folks also got back to me in a private message, confirming
the issue and also clarifying that the length of the tag is the
limiting factor. Every 4-byte tag can work, sizes other than 4 bytes
cannot. As MediaTek's tag format includes the 802.1Q VLAN as part of
the tag itself I suspect VLAN offloading will still need some extra
care to work on non-MTK 4-byte tags (like RealTek 4B, for example)...

> 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index e68997a29191b..654b707ee27a1 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -1459,6 +1459,26 @@ static void setup_tx_buf(struct mtk_eth *eth, struct mtk_tx_buf *tx_buf,
> >  	}
> >  }
> >  
> > +static bool mtk_uses_dsa(struct net_device *dev)
> > +{
> > +#if IS_ENABLED(CONFIG_NET_DSA)
> > +	return netdev_uses_dsa(dev) &&
> > +	       dev->dsa_ptr->tag_ops->proto == DSA_TAG_PROTO_MTK;
> > +#else
> > +	return false;
> > +#endif
> 
> I think the concept of determining if the switch is using a specific
> tag in order to enable/disable acceleration should be generic. So i
> would try to make this an helper in include/next/dsa.h. Any MAC driver
> can then use it.

Now that I know that the Ethernet driver should have 4 modes:
 - no DSA at all
 - DSA with MediaTek special tag
 - DSA with non-MediaTek but still 4 byte special tag
   -> VLAN offloading needs to be figured out
 - DSA with special tag size not equal to 4 bytes
   -> no checksum and no VLAN offloading

> 
> > @@ -1531,7 +1551,7 @@ static void mtk_tx_set_dma_desc_v2(struct net_device *dev, void *txd,
> >  		/* tx checksum offload */
> >  		if (info->csum)
> >  			data |= TX_DMA_CHKSUM_V2;
> > -		if (mtk_is_netsys_v3_or_greater(eth) && netdev_uses_dsa(dev))
> > +		if (mtk_is_netsys_v3_or_greater(eth) && mtk_uses_dsa(dev))
> >  			data |= TX_DMA_SPTAG_V3;
> 
> This looks to be in the hot path. Do you really want to do this
> evaluation on every frame? You can change the tag protocol via sysfs,
> however, dsa_tree_change_tag_proto() will only allow you to change the
> tag while the conduit interface is down. So it should be safe to look
> at the tag protocol once during open, and cache the result somewhere
> local, struct mtk_eth? That should avoid a few cache misses.

+1

> 
> > @@ -3192,6 +3212,14 @@ static netdev_features_t mtk_fix_features(struct net_device *dev,
> >  		}
> >  	}
> >  
> > +	if ((features & NETIF_F_IP_CSUM) &&
> > +	    non_mtk_uses_dsa(dev))
> > +		features &= ~NETIF_F_IP_CSUM;
> > +
> > +	if ((features & NETIF_F_IPV6_CSUM) &&
> > +	    non_mtk_uses_dsa(dev))
> > +		features &= ~NETIF_F_IPV6_CSUM;
> > +
> 
> 
> When is mtk_fix_features() actually called? I don't know without
> looking at the core. You will want it when open is called, when the
> tagging protocol is fixed.

It's used as .ndo_fix_features operation, which is called by
__netdev_update_features() at various occasions, and I'm not sure if it
would be called as well when changing the tag protocol...

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches
  2026-01-13 14:22   ` Daniel Golle
@ 2026-01-13 22:38     ` Luiz Angelo Daros de Luca
  0 siblings, 0 replies; 5+ messages in thread
From: Luiz Angelo Daros de Luca @ 2026-01-13 22:38 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Andrew Lunn, Felix Fietkau, John Crispin, Sean Wang,
	Lorenzo Bianconi, Bc-bocun Chen, Rex Lu, Mason-cw Chang,
	Andrew Lunn, David S. Miller, Vladimir Oltean, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Matthias Brugger,
	AngeloGioacchino Del Regno, netdev, linux-kernel,
	linux-arm-kernel, linux-mediatek

> MediaTek folks also got back to me in a private message, confirming
> the issue and also clarifying that the length of the tag is the
> limiting factor. Every 4-byte tag can work, sizes other than 4 bytes
> cannot. As MediaTek's tag format includes the 802.1Q VLAN as part of
> the tag itself I suspect VLAN offloading will still need some extra
> care to work on non-MTK 4-byte tags (like RealTek 4B, for example)...

My suggestion is to enable it only when sure (mediatek tag) and drop
it otherwise. Something like this:

https://github.com/openwrt/openwrt/commit/2603d6d81d1a088c6ad271fe20a63fa6e3a75124

It is not worth it to risk enabling offload for unknown cases.

As Realtek was mentioned, the 4-byte tag (rtl4_a) is for old SoCs,
hardly paired with a mediatek SoC. The newer tag, rtl8_4, is already
too big (8 bytes).

Regards,

Luiz

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches
  2026-01-13 14:00 ` Andrew Lunn
  2026-01-13 14:22   ` Daniel Golle
@ 2026-01-14 22:17   ` Vladimir Oltean
  1 sibling, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2026-01-14 22:17 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Daniel Golle, Felix Fietkau, John Crispin, Sean Wang,
	Lorenzo Bianconi, Bc-bocun Chen, Rex Lu, Mason-cw Chang,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Matthias Brugger, AngeloGioacchino Del Regno, netdev,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Tue, Jan 13, 2026 at 03:00:18PM +0100, Andrew Lunn wrote:
> On Tue, Jan 13, 2026 at 03:11:54AM +0000, Daniel Golle wrote:
> > MediaTek's Ethernet Frame Engine is tailored for use with their
> > switches. This broke checksum and VLAN offloading when attaching a
> > DSA switch which does not use MediaTek special tag format.
> 
> This has been seen before. The Freescale FEC has similar problems when
> combined with a Marvell switch, it cannot find the IP header, and so
> checksum offloading does not work.
> 
> I thought we solved this be modifying the ndev->feature of the conduit
> interface to disable such offloads. But i don't see such code. So i
> must be remembering wrongly.
> 
> This is assuming the frame engine respects these flags:
> 
> /usr/sbin/ethtool -k enp2s0
> Features for enp2s0:
> rx-checksumming: on
> tx-checksumming: on
> 	tx-checksum-ipv4: on
> 	tx-checksum-ip-generic: off [fixed]
> 	tx-checksum-ipv6: on
> 	tx-checksum-fcoe-crc: off [fixed]
> 	tx-checksum-sctp: off [fixed]
> 
> When you combine a Marvell Ethernet interface with a Marvell switch
> offloading works of course. So it probably does require some logic in
> the MAC driver to determine if the switch is of the same vendor or
> not.

I don't know about FEC, but we did end up documenting this:

If the DSA conduit driver still uses the legacy NETIF_F_IP_CSUM
or NETIF_F_IPV6_CSUM in vlan_features, the offload might only work if the
offload hardware already expects that specific tag (perhaps due to matching
vendors). DSA user ports inherit those flags from the conduit, and it is up to
the driver to correctly fall back to software checksum when the IP header is not
where the hardware expects.

I would suggest searching for skb_checksum_help() in xmit() implementations.
For example c2945c435c99 ("net: stmmac: Prevent DSA tags from breaking COE").

This is just at a first glance having read the commit message and the
discussion, not necessarily the code.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-01-14 22:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-13  3:11 [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches Daniel Golle
2026-01-13 14:00 ` Andrew Lunn
2026-01-13 14:22   ` Daniel Golle
2026-01-13 22:38     ` Luiz Angelo Daros de Luca
2026-01-14 22:17   ` Vladimir Oltean

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox