On Thu, 11 Jan 2024 15:58:51 +0100 Romain Gantois wrote:
Some DSA tagging protocols change the EtherType field in the MAC header e.g. DSA_TAG_PROTO_(DSA/EDSA/BRCM/MTK/RTL4C_A/SJA1105). On TX these tagged frames are ignored by the checksum offload engine and IP header checker of some stmmac cores.
On RX, the stmmac driver wrongly assumes that checksums have been computed for these tagged packets, and sets CHECKSUM_UNNECESSARY.
Add an additional check in the stmmac TX and RX hotpaths so that COE is deactivated for packets with ethertypes that will not trigger the COE and IP header checks.
Fixes: 6b2c6e4a938f ("net: stmmac: propagate feature flags to vlan") Cc: stable@vger.kernel.org
nit: double space
+/**
- stmmac_has_ip_ethertype() - Check if packet has IP ethertype
- @skb: socket buffer to check
- Check if a packet has an ethertype that will trigger the IP header checks
- and IP/TCP checksum engine of the stmmac core.
- Return: true if the ethertype can trigger the checksum engine, false otherwise
nit: please don't go over 80 chars unless there's a good reason. we are old school and stick to checkpatch --max-line-length=80 in netdev
if (csum_insertion &&
priv->plat->tx_queues_cfg[queue].coe_unsupported) {
(priv->plat->tx_queues_cfg[queue].coe_unsupported ||!stmmac_has_ip_ethertype(skb))) {
nit: minor misalignment here, the '!' should be under 'p'
if (unlikely(skb_checksum_help(skb))) goto dma_map_err; csum_insertion = !csum_insertion;@@ -4997,7 +5020,7 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue, stmmac_rx_vlan(priv->dev, skb); skb->protocol = eth_type_trans(skb, priv->dev);
- if (unlikely(!coe))
- if (unlikely(!coe) || !stmmac_has_ip_ethertype(skb))
The lack of Rx side COE checking in this driver is kinda crazy. Looking at enh_desc_coe_rdes0() it seems like RDES0_FRAME_TYPE may be the indication we need here?
We can dig into it as a follow up but I'm guessing that sending an IPv6 packet with extension headers will also make the device skip checksumming, or a UDP packet with csum of 0?