Re: [PATCH net-next v2 2/4] 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.
> 
> skb_segment() only groups several MSS into one output skb when the device
> advertises NETIF_F_GSO_PARTIAL, or when the skb has 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 that grouping
> regardless, so the frag_list check is skipped when max_segs is set. Every
> other caller keeps it, and the bounded path is only used for skbs which do
> not carry a 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. Clear the value
> when each output skb copies the input header so the temporary limit is not
> propagated to the next GSO call.
> 
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
> 
> ---
> v2:
> - wrap the tap.c declaration and skb_gso_cb comment to 80 columns
> v1: https://lore.kernel.org/20260917063854.2011613-3-wang.zhan@smartx.com/
> ---
>  drivers/net/tap.c          |  3 ++-
>  include/net/gso.h          |  6 ++++--
>  include/net/udp.h          |  2 +-
>  net/core/gso.c             |  5 ++++-
>  net/core/skbuff.c          | 14 ++++++++++++--
>  net/ipv4/tcp_offload.c     |  3 ++-
>  net/openvswitch/datapath.c |  2 +-
>  7 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index ff67d99deb39e..bc111495ebbce 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -278,9 +278,10 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb)
>  	if (q->flags & IFF_VNET_HDR)
>  		features |= tap->tap_features;
>  	if (netif_needs_gso(skb, features)) {
> -		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
> +		struct sk_buff *segs;
>  		struct sk_buff *next;
>  
> +		segs = __skb_gso_segment(skb, features, false, 0);

irrelevant?

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index dbbe10277d51d..9c0d140236bc6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4793,6 +4793,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	struct sk_buff *segs = NULL;
>  	struct sk_buff *tail = NULL;
>  	struct sk_buff *list_skb = skb_shinfo(head_skb)->frag_list;
> +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;

could this be computed inside skb_segment, rather than having to be
passed through SKB_GSO_CB. I haven't checked, but it would simplify.

>  	unsigned int mss = skb_shinfo(head_skb)->gso_size;
>  	bool gso_by_frags = mss == GSO_BY_FRAGS;
>  	unsigned int doffset = head_skb->data - skb_mac_header(head_skb);