Re: [PATCH net v3] net: gro: Fix nesting of TCP GSO SKBs in skb_gro_receive_list()

On Wed, 2026-08-12 at 22:29 -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:

quoted

From: HW He <redacted>

A device supports GRO_HW, and the device driver enables the
NETIF_F_GRO_FRAGLIST feature. During a tethering test,
skb_gro_receive_list() reaggregates the GSO packet. However,
skb_segment_list() cannot segment this packet back into
the original packets, which leads to IP fragmentation or packet
drop.

Scenario (Tethering/Forwarding):
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.

When NETIF_F_GRO_HW is enabled, do not set NAPI_GRO_CB(skb)-

quoted

is_flist.

Fall through to the regular skb_gro_receive() path instead of
skb_gro_receive_list().

Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
Signed-off-by: HW He <redacted>
Signed-off-by: Zhaoping Shu <redacted>
---
[2]: 
https://urldefense.com/v3/__https://patchwork.kernel.org/patch/14706032__;!!CTRNKA9wMg0ARbw!kigb147-G9EgNqdWMhaQ43udYOvN90SG6bVjeM-GxTsPo3E07_tbysuoEieszr5CbCqT94AYpt1b_M7VsgNaZcjtBS_AnYo$
[1]: 
https://urldefense.com/v3/__https://patchwork.kernel.org/patch/14702209__;!!CTRNKA9wMg0ARbw!kigb147-G9EgNqdWMhaQ43udYOvN90SG6bVjeM-GxTsPo3E07_tbysuoEieszr5CbCqT94AYpt1b_M7VsgNaZcjtu0bQPNI$
---
 net/ipv4/tcp_offload.c   | 7 +++----
 net/ipv6/tcpv6_offload.c | 3 ++-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 3b1fdcd3cb29..641c47fb1ea2 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c

@@ -395,9 +395,6 @@ static void tcp4_check_fraglist_gro(struct

list_head *head, struct sk_buff *skb,
      struct net *net;
      int iif, sdif;

-     if (likely(!(skb->dev->features & NETIF_F_GRO_FRAGLIST)))
-             return;
-

Interesting that ipv4 and ipv6 diverge here. Nice to try to make them
more alike.

quoted

      p = tcp_gro_lookup(head, th);
      if (p) {
              NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)-

quoted

is_flist;

@@ -430,7 +427,9 @@ struct sk_buff *tcp4_gro_receive(struct

list_head *head, struct sk_buff *skb)
      if (!th)
              goto flush;

-     tcp4_check_fraglist_gro(head, skb, th);
+     if (unlikely((skb->dev->features & NETIF_F_GRO_FRAGLIST) &&
+                  !(skb->dev->features & NETIF_F_GRO_HW)))

Would it be better to test skb_is_gso(skb) and only skip fraglist GRO
for HW-GRO skbs, rather than disabling it for all skbs?

Yes, for tethering packets, HW-GRO skbs are aggregated by
skb_gro_receive(), while others go through fraglist GRO. However, care
must be taken to avoid packets arriving out of order. I will work on
it and submit V4.

If treating the features as mutually exclusive, another option would
be to replace these datapath checks with disabling one at
configuration
time, in netdev_fix_features.

If V4 work well, there will be no need to check netdev->features.
Linux kernel GRO will be more robust and able to handle scenarios
where both NETIF_F_GRO_HW and NETIF_F_GRO_FRAGLIST are enabled.