Re: [PATCH net-next 06/10] net: stmmac: simplify GSO/TSO test in stmmac_xmit()

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Sat, Mar 28, 2026 at 09:37:07PM +0000, Russell King (Oracle) wrote:
> +static void stmmac_set_gso_types(struct stmmac_priv *priv, bool tso)
> +{
> +	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;

I've been wondering whether keying all three of these off NETIF_F_TSO
is correct. Shouldn't SKB_GSP_UDP_L4 be dependent on NETIF_F_GSO_UDP_L4?

(The above code doesn't change the current driver behaviour, so this
would be a separate fix.)

> +	}
> +}
> +
>  /**
>   *  stmmac_tso_xmit - Tx entry point of the driver for oversized frames (TSO)
>   *  @skb : the socket buffer
> @@ -4671,7 +4683,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	u32 queue = skb_get_queue_mapping(skb);
>  	int nfrags = skb_shinfo(skb)->nr_frags;
>  	unsigned int first_entry, tx_packets;
> -	int gso = skb_shinfo(skb)->gso_type;
>  	struct stmmac_txq_stats *txq_stats;
>  	struct dma_desc *desc, *first_desc;
>  	struct stmmac_tx_queue *tx_q;
> @@ -4683,14 +4694,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en)
>  		stmmac_stop_sw_lpi(priv);
>  
> -	/* Manage oversized TCP frames for GMAC4 device */
> -	if (skb_is_gso(skb) && priv->tso) {
> -		if (gso & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
> -			return stmmac_tso_xmit(skb, dev);
> -		if (priv->plat->core_type == DWMAC_CORE_GMAC4 &&
> -		    (gso & SKB_GSO_UDP_L4))
> -			return stmmac_tso_xmit(skb, dev);
> -	}
> +	if (skb_is_gso(skb) &&
> +	    skb_shinfo(skb)->gso_type & priv->gso_enabled_types)
> +		return stmmac_tso_xmit(skb, dev);

I'm also wondering whether we should check gso_type in our
.ndo_features_check() method rather than here - if we get a GSO skb at
this point for a type that we don't recognise, surely it is wrong to
pass it via the normal skb transmission flow.

Yet more worms in the stmmac can... :/

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!