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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <252d6877-d966-4d19-a38c-cc83ba908494@lunn.ch>
Date: Tue, 13 Jan 2026 15:00:18 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Daniel Golle <daniel@...rotopia.org>
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: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