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

Zhaoping Shu (舒召平) wrote:

On Wed, 2026-07-29 at 14:57 -0400, Willem de Bruijn wrote:

quoted

External email : Please do not click links or open attachments until
you have verified the sender or the content.


Zhaoping Shu (舒召平) wrote:

quoted

On Thu, 2026-07-23 at 16:02 +0200, Willem de Bruijn wrote:

quoted

On Thu, Jul 23, 2026 at 3:39 PM Jakub Kicinski [off-list ref]
wrote:

quoted

On Thu, 23 Jul 2026 17:16:01 +0800 zhaoping.shu@mediatek.com
wrote:

quoted

From: HW He <redacted>

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.

quoted

quoted

quoted

the hardware or driver may deliver packets that have already
been

If you have a driver in mind please name it.

quoted

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.

quoted

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.

And that would be sufficient.
  

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().

Yes, that sounds good. And then the above option 1 is not needed.

No HW-GRO implementation generates fraglist GRO packets, so the two
are fundamentally at odds anyway.

Ignoring the fraglist hint for HW-GRO skbs sounds good to me, thanks.
  

If that direction makes sense, I can work on it,
or send a follow-up patch to fix the reported issue with option 1.

quoted

quoted

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.