Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches
From: Daniel Golle <daniel@makrotopia.org>
Date: 2026-01-13 14:22:45
Also in:
linux-arm-kernel, linux-mediatek, lkml
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:quoted
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)...
quoted
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; +#endifI 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
quoted
@@ -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
quoted
@@ -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...