Re: [PATCH net-next v3 3/5] net: gso: support bounded TCP segmentation

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Wang Zhan wrote:
> The bounded resegmentation added by the next patch splits an oversized TCP
> GSO skb into several GSO skbs which fit the device limits.  That needs the
> GSO engine to group several MSS segments into one output skb, so let
> callers bound the number of MSS segments each output skb carries and pass
> the bound through the existing __skb_gso_segment() entry point.  Ordinary
> callers use zero for no limit.
> 
> Without a bound, skb_segment() groups several MSS into one output skb only
> for a device which advertises NETIF_F_GSO_PARTIAL or for a skb with a
> frag_list which can be split into uniform pieces, and falls back to one
> segment per skb otherwise.  A caller which passes a bound asks for the
> grouping regardless, so the block which makes that decision is skipped
> when max_segs is set.  Callers which pass no bound keep it, and the bounded
> path only runs for skbs which carry no frag_list.
> 
> The output stays a GSO skb: gso_size is the original MSS and gso_segs is
> the number of MSS it holds, so a downstream device can still perform
> ordinary TSO.  Store the bound in the existing skb_gso_cb scratch context,
> alongside the call-local data_offset and mac_offset fields, so that the
> segmentation methods keep their signature.  A zero max_segs value means
> that no bound is active; it is not a persistent skb flag.
> 
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
> 
> ---
> v3:
> - drop the tcp_gso_segment() exception: the caller keeps the features
> - cap the bound with GSO_MAX_SEGS instead of U16_MAX
> - drop the reset of the field in the output skbs: every caller initializes it
> - note in the kernel-doc that a bound is for TCP GSO skbs only
> - note at the frag_list gate that a bound must not go with a frag_list skb
> v2: https://lore.kernel.org/20260918084651.3022878-3-wang.zhan@smartx.com/
> v1: https://lore.kernel.org/20260917063854.2011613-3-wang.zhan@smartx.com/

> @@ -4839,7 +4840,13 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	csum = !!can_checksum_protocol(features, proto);
>  
>  	if (sg && csum && !gso_by_frags)  {
> -		if (!(features & NETIF_F_GSO_PARTIAL)) {
> +		/*
> +		 * A call with max_segs set groups the MSS segments below,
> +		 * and the frag_list split in this block does not carry the
> +		 * bound, so a bound must not be passed for a skb with a
> +		 * frag_list.
> +		 */

To me this comment confuses rather than helps.

For one, it refers to "the bound" without any context.

Probably just drop.

> +		if (!max_segs && !(features & NETIF_F_GSO_PARTIAL)) {
>  			struct sk_buff *iter;
>  			unsigned int frag_len;
>