Re: [PATCH net v2] net: stmmac: keep gso_enabled_types in sync with netdev features

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 12 Aug 2026 06:38:13 +0200 Lorenzo Bianconi wrote:
> stmmac_set_gso_types() derives priv->gso_enabled_types all-or-nothing
> from NETIF_F_TSO, but stmmac_set_gso_features() advertises NETIF_F_TSO,
> NETIF_F_TSO6 and (on GMAC4) NETIF_F_GSO_UDP_L4 as independently
> toggleable features. Since netdev_fix_features() only ties TSO6 to the
> checksum features, disabling TSO (ethtool -K ethX tx-tcp-segmentation
> off) leaves TSO6 enabled in dev->features while gso_enabled_types
> becomes 0.
> 
> For a TCPv6 GSO skb the stack then keeps the frame unsegmented
> (NETIF_F_TSO6 is still set), stmmac_features_check() does not clear the
> GSO mask, and the stmmac_xmit() gate (gso_type & gso_enabled_types) is
> false, so the multi-MSS skb is transmitted through the ordinary
> descriptor path as a single oversized frame.
> 
> Derive each GSO type from its own feature bit instead, so the mask stays
> in sync with dev->features and TCPv6 (or UDP L4 on GMAC4) segmentation
> keeps working when only TSO is disabled.
> 
> Fixes: 9edfa7dab811 ("net: stmmac: enable TSO for IPv6")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
> ---
> Changes in v2:
> - Fix possible race between stmmac_set_gso_types() and stmmac_xmit().
> - Fix fixes tag.
> - Link to v1: https://lore.kernel.org/r/20260808-stmmac-fix-tso6-features-v1-1-f82b17595052@oss.qualcomm.com
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++++----------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a71f0df26378..df3549f55741 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4371,16 +4371,20 @@ static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue)
>  	stmmac_set_queue_tx_tail_ptr(priv, tx_q, queue, tx_q->cur_tx);
>  }
>  
> -static void stmmac_set_gso_types(struct stmmac_priv *priv, bool tso)
> +static void stmmac_set_gso_types(struct stmmac_priv *priv,
> +				 netdev_features_t features)
>  {
> -	if (!tso) {
> -		priv->gso_enabled_types = 0;
> -	} else {
> -		/* Manage oversized TCP frames for GMAC4 device */
> -		priv->gso_enabled_types = SKB_GSO_TCPV4 | SKB_GSO_TCPV6;
> -		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> -			priv->gso_enabled_types |= SKB_GSO_UDP_L4;
> -	}
> +	unsigned int gso_types = 0;
> +
> +	if (features & NETIF_F_TSO)
> +		gso_types |= SKB_GSO_TCPV4;
> +	if (features & NETIF_F_TSO6)
> +		gso_types |= SKB_GSO_TCPV6;
> +	/* Manage oversized UDP frames for GMAC4 devices */
> +	if (features & NETIF_F_GSO_UDP_L4)
> +		gso_types |= SKB_GSO_UDP_L4;
> +
> +	WRITE_ONCE(priv->gso_enabled_types, gso_types);
>  }
>  
>  static void stmmac_set_gso_features(struct net_device *ndev)
> @@ -4416,7 +4420,7 @@ static void stmmac_set_gso_features(struct net_device *ndev)
>  	if (priv->plat->core_type == DWMAC_CORE_GMAC4)
>  		ndev->hw_features |= NETIF_F_GSO_UDP_L4;
>  
> -	stmmac_set_gso_types(priv, true);
> +	stmmac_set_gso_types(priv, ndev->hw_features);
>  
>  	dev_info(priv->device, "TSO feature enabled\n");
>  }
> @@ -4766,7 +4770,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  		stmmac_stop_sw_lpi(priv);
>  
>  	if (skb_is_gso(skb) &&
> -	    skb_shinfo(skb)->gso_type & priv->gso_enabled_types)
> +	    (skb_shinfo(skb)->gso_type & READ_ONCE(priv->gso_enabled_types)))

Maybe I'm missing something but I don't see any device configuration
based on gso_enabled_types. Why does this mask exist in the first place?

Right now AFAICT packets which miss the mask check get chucked on the
wire as one giant frame, without segmentation.

Can we just delete gso_enabled_types completely? Stack should obey the
features for future frames, and avoiding races here may be hard.

>  		return stmmac_tso_xmit(skb, dev);
>  
>  	if (priv->est && priv->est->enable &&
> @@ -6198,7 +6202,7 @@ static int stmmac_set_features(struct net_device *netdev,
>  			stmmac_enable_sph(priv, priv->ioaddr, sph_en, chan);
>  	}
>  
> -	stmmac_set_gso_types(priv, features & NETIF_F_TSO);
> +	stmmac_set_gso_types(priv, features);
>  
>  	if (features & NETIF_F_HW_VLAN_CTAG_RX)
>  		priv->hw->hw_vlan_en = true;
-- 
pw-bot: cr