Re: [PATCH net-next v3 4/5] net: core: resegment oversized TCP GSO skbs

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

Wang Zhan wrote:
> On Mon, 28 Sep 2026 19:47:43 -0400 Willem de Bruijn wrote:
> > > +	/*
> > > +	 * The TCP frag-list path segments through skb_segment_list(), which
> > > +	 * does not carry max_segs, so bounded calls skip those skbs.
> > > +	 */
> >
> > This comment answers only one of six conditions. And one that is
> > pretty straightforward. I'd drop.
> >
> > In general, drop all too-obvious comments. AI has a habit of adding
> > a lot more, and more low information, comments than is customary in
> > kernel code (where we also have commit messages). Generally, repeating
> > what the code does is of little value.
> 
> Dropped in v4.  I will check all the comments in the series.
> 
> > > +	if (!skb_is_gso(skb) || !skb_is_gso_tcp(skb) ||
> > > +	    skb->encapsulation || skb_has_frag_list(skb) ||
> > > +	    !skb_mac_header_was_set(skb) ||
> > > +	    !skb_transport_header_was_set(skb))
> >
> > Conversely, they last two conditions are less obvious. Are they not
> > always true for a TSO packet?
> 
> The transport header can be missing.  qdisc_pkt_len_segs_init() does the
> same check on this path (net/core/dev.c:4245, a0dce8752193e).
> 
> The mac header is always set.  It can be dropped in v4.
> 
> > > +	gso_max_size = netif_get_gso_max_size(dev, vlan_get_protocol(skb));
> >
> > Third time this is now called in validate_xmit_skb. Not sure if that can
> > easily be avoided.
> 
> Maybe we can pass the oversize and gso_max_size flags out of
> __netif_skb_features, but it would be a bit ugly.  I think the current
> cost is acceptable.

Agreed. I was hoping otherwise, but don't see an easy fix (and didn't
explore more deeply to be fair).