Re: [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list()

[Zhaoping Shu (舒召平)]

On Wed, 2026-07-29 at 14:57 -0400, Willem de Bruijn wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Zhaoping Shu (舒召平) wrote:
> > On Thu, 2026-07-23 at 16:02 +0200, Willem de Bruijn wrote:
> > > On Thu, Jul 23, 2026 at 3:39 PM Jakub Kicinski <kuba@xxxxxxxxxx>
> > > wrote:
> > > > 
> > > > On Thu, 23 Jul 2026 17:16:01 +0800 zhaoping.shu@xxxxxxxxxxxx
> > > > wrote:
> > > > > From: HW He <hw.he@xxxxxxxxxxxx>
> > > > > 
> > > > > On devices that support both NETIF_F_GRO_HW and
> > > > > NETIF_F_GRO_FRAGLIST,
> > > > 
> > > > "devices that support FRAGLIST"? Isn't it a software feature?
> > 
> > Sorry for not making it clear. The test environment is:
> > device supports GRO_HW, and enable NETIF_F_GRO_FRAGLIST in
> > driver.
> > 
> > > > 
> > > > > the hardware or driver may deliver packets that have already
> > > > > been
> > > > 
> > > > If you have a driver in mind please name it.
> > > > 
> > > > > aggregated into a TCP GSO skb with frags[]. GRO may then
> > > > > aggregate the skb again in skb_gro_receive_list().
> > > > > 
> > > > > This can create a nested GSO skb, which is not handled
> > > > > correctly
> > > > > by the
> > > > > later GSO segmentation paths. When the skb is segmented by
> > > > > skb_segment_list(), it not be fully restored to the original
> > > > > packets.
> > > > > 
> > > > > Avoid this by setting NAPI_GRO_CB(skb)->flush for GSO skbs
> > > > > before
> > > > > aggregation.
> > > > 
> > > > I don't think we can do this. For GRO_HW devices re-aggregating
> > > > in SW is quite helpful, HW often runs out of contexts or times
> > > > out too soon, generating skbs with 16kB..32kB of data, the SW
> > > > can help bring it up to full TSO.
> > 
> > I'll try to explain this issue below.
> > 
> > > 
> > > Also, after e751256486d0 ("net: gro: fix double aggregation of
> > > flush-marked skbs"), it's not clear an another bug remains.
> > > 
> > > If it is: as said "nested GRO" of hw + sw GRO is intentional,
> > > e.g.,
> > > for BIG-TCP.
> > > 
> > > But it may not be anticipated for skb_gro_receive_list. One
> > > option
> > > would be to skip the fraglist GSO optimization for such packets.
> > > 
> > > First I'd like to understand better what exact bug remains.
> > 
> > I agree that re-aggregation is useful for improving GRO efficiency.
> > 
> > However, as a general rule, a GSO skb must be segment back to the
> > exact original packets stream. In tethering test,
> > skb_segment_list()
> > cannot correctly segment a nested GSO skb produced by this path.
> 
> So the specific issue is a driver that builds a regular (HW) GSO
> packet followed by software GSO that uses fraglist?
> 
> Then I see three paths to fixing this
> 
> 1. decline to further apply SW GRO if skb is GSO and in fraglist mode
> 2. if in fraglist mode, further apply SW GRO, but do not use fraglist
> 3. in skb_segment_list detect this case and fall back onto
>    skb_segment
> 
> We already apply option 3 to various cases where skb_segment_list
> cannot handle complex use-cases of fraglist.
> 
> This patch chooses option 1, which is fine. Alternatively it could
> fall through to the regular skb_gro_receive path below.
> 

Thanks for the feedback.
 
Patch (option 1) is a minimal fix for the reported issue.
 
For the other options, my initial thought is to handle this in
tcp4_check_fraglist_gro()/tcp6_check_fraglist_gro(): if the netdev
has both NETIF_F_GRO_HW and NETIF_F_GRO_FRAGLIST enabled, do not set
NAPI_GRO_CB(skb)->is_flist, so that this tethering/forwarding case
can keep using SW GRO and fall through to the regular skb_gro_receive()
path instead of skb_gro_receive_list().
 
If that direction makes sense, I can work on it,
or send a follow-up patch to fix the reported issue with option 1.

> > This issue can reproduce in the following scenario:
> > 1.Driver submits a single TCP packet, P1. P1 is kept in the
> > gro_list as the first packet.
> > 
> > 2. The driver submits a TCP GSO skb, P2. P2 has already aggregated
> > multiple TCP packets by HW_GRO, and its non-linear data is stored
> > in
> > frags[].
> > 
> > 3. P1 and P2 match the GRO rules, and since there is no local
> > socket,
> > they are aggregated by skb_gro_receive_list(). The resulting skb,
> > P3, has a frag_list entry that still contains frags[]:
> > P3: [ Linear Data ] -> frag_list -> [ Linear Data ]
> >                                     [ frag[1] ]
> >                                     [ frag[2] ]
> >                                     ...
> > 
> > 4. Later, tcp4_gso_segment() or tcp6_gso_segment() calls
> > skb_segment_list() to segment P3. However, skb_segment_list() only
> > segments the entries in frag_list. It does not segment the frags[]
> > inside P2, so P3 is not restored to the original packets, which
> > leads
> > to IP fragmentation or packet drop in the following path.
> > 
> > The patch only prevents that nested case before
> > skb_gro_receive_list()
> > aggregation. It does not affect packets that are re-aggregated by
> > skb_gro_receive().
> 
> Thanks for the detailed explanation.
> 
>