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

[Willem de Bruijn]

On Thu, Aug 13, 2026 at 4:40 AM Zhaoping Shu (舒召平)
<Zhaoping.Shu at mediatek.com> wrote:
>
> 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:
> > > From: HW He <hw.he at mediatek.com>
> > >
> > > 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)-
> > > >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 <hw.he at mediatek.com>
> > > Signed-off-by: Zhaoping Shu <zhaoping.shu at mediatek.com>
> > > ---
> > > [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.
> >
> > >       p = tcp_gro_lookup(head, th);
> > >       if (p) {
> > >               NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)-
> > > >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.

Oh right.

As long as the choice is only between fraglist GRO or not fraglist
(rather than GRO or bypass GRO), it should not introduce any new
reordering concerns.

But the decision cannot be made based on skb_is_gso(skb) of an
arriving skb. Because when such a HW-GRO skb arrives a SW GRO context
in fraglist mode may already have been opened, and it is too late to
convert that to non-fraglist.

So essentially HW-GRO and fraglist are mutually exclusive.

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

What is your plan for v4?