netdev - Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using non-MediaTek DSA switches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aWZVIBh6AKiIaxdr@makrotopia.org>
Date: Tue, 13 Jan 2026 14:22:24 +0000
From: Daniel Golle <daniel@...rotopia.org>
To: Andrew Lunn <andrew@...n.ch>
Cc: Felix Fietkau <nbd@....name>, John Crispin <john@...ozen.org>,
	Sean Wang <sean.wang@...iatek.com>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Bc-bocun Chen <bc-bocun.chen@...iatek.com>,
	Rex Lu <rex.lu@...iatek.com>,
	Mason-cw Chang <Mason-cw.Chang@...iatek.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Vladimir Oltean <olteanv@...il.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH net-next RFC] net: ethernet: mtk_eth_soc: support using
 non-MediaTek DSA switches

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...