[PATCH net v4] net: gro: Fix nesting of TCP GSO SKBs in skb_gro_receive_list()
Willem de Bruijn
willemdebruijn.kernel at gmail.com
Wed Aug 19 16:00:46 PDT 2026
On Wed, Aug 19, 2026 at 5:37 AM <zhaoping.shu at mediatek.com> wrote:
>
> From: HW He <hw.he at mediatek.com>
>
Perhaps a brief one sentence summary of the problem, before
a detailed repro. For instance:
Fraglist GRO plus hardware GRO can create an fraglist of
HW-GRO packets. This cannot be segmented back into
the original form.
Avoid constructing such a GSO packet, by flushing an already
built fraglist GRO packet if a hardware GRO packet arrives.
> 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.
>
> Check skb_is_gso(skb) and current GRO method, make sure fraglist GRO
> applies to consecutive non-GSO skb, others adopt regular GRO path.
>
> Fixes: 8d95dc474f85 ("net: add code for TCP fraglist GRO")
> Signed-off-by: Zhaoping Shu <zhaoping.shu at mediatek.com>
> Signed-off-by: HW He <hw.he at mediatek.com>
>
> ---
> [3]: https://patchwork.kernel.org/patch/14747095
> [2]: https://patchwork.kernel.org/patch/14706032
> [1]: https://patchwork.kernel.org/patch/14702209
> ---
> net/ipv4/tcp_offload.c | 22 ++++++++++++++++------
> net/ipv6/tcpv6_offload.c | 15 +++++++++++++--
> 2 files changed, 29 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index 3b1fdcd3cb29..e74d99ca9fac 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -332,6 +332,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb,
> flush |= skb->ip_summed != p->ip_summed;
> flush |= skb->csum_level != p->csum_level;
> flush |= NAPI_GRO_CB(p)->count >= 64;
> + flush |= NAPI_GRO_CB(p)->is_flist != NAPI_GRO_CB(skb)->is_flist;
Technically, is this one-line sufficient?
> skb_set_network_header(skb, skb_gro_receive_network_offset(skb));
>
> if (flush || skb_gro_receive_list(p, skb))
> @@ -395,12 +396,20 @@ 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;
> -
> p = tcp_gro_lookup(head, th);
> if (p) {
> - NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist;
> + /* flist GRO applies to consecutive non-GSO skbs */
> + if (!skb_is_gso(skb) || !NAPI_GRO_CB(p)->is_flist) {
> + NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist;
> + return;
> + }
> +
> + /* Fall back to the regular GRO path */
> + if (NAPI_GRO_CB(p)->count == 1)
> + NAPI_GRO_CB(p)->is_flist = 0;
> +
> + NAPI_GRO_CB(skb)->is_flist = 0;
> +
This reduces the chance to have to flush, by downgrading fraglist GRO
packets if a hardware GRO packet is detected at the stage (count == 1)
where fraglist is not enabled in practice.
This is quite a bit of complexity. And if hardware GRO is enabled, what
are the odds in practice that the device does not produce any hardware
GRO packets, yet the software GRO stack is capable of assembling
signficant GSO packets. I suspect low.
The solution is technically correct. But would simply disabling fraglist
GRO when hardware GRO is enabled not be simpler? Or only keeping
the flush and avoiding this micro-optimization.
> return;
> }
>
> @@ -410,7 +419,7 @@ static void tcp4_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
> sk = __inet_lookup_established(net, iph->saddr, th->source,
> iph->daddr, ntohs(th->dest),
> iif, sdif);
> - NAPI_GRO_CB(skb)->is_flist = !sk;
> + NAPI_GRO_CB(skb)->is_flist = !sk && !skb_is_gso(skb);
> if (sk)
> sock_gen_put(sk);
> }
> @@ -430,7 +439,8 @@ 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))
> + tcp4_check_fraglist_gro(head, skb, th);
>
> return tcp_gro_receive(head, skb, th);
>
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index f2a659cd6183..eec3778855eb 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -26,7 +26,18 @@ static void tcp6_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
>
> p = tcp_gro_lookup(head, th);
> if (p) {
> - NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist;
> + /* flist GRO applies to consecutive non-GSO skbs */
> + if (!skb_is_gso(skb) || !NAPI_GRO_CB(p)->is_flist) {
> + NAPI_GRO_CB(skb)->is_flist = NAPI_GRO_CB(p)->is_flist;
> + return;
> + }
> +
> + /* Fall back to the regular GRO path */
> + if (NAPI_GRO_CB(p)->count == 1)
> + NAPI_GRO_CB(p)->is_flist = 0;
> +
> + NAPI_GRO_CB(skb)->is_flist = 0;
> +
> return;
> }
>
> @@ -36,7 +47,7 @@ static void tcp6_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
> sk = __inet6_lookup_established(net, &hdr->saddr, th->source,
> &hdr->daddr, ntohs(th->dest),
> iif, sdif);
> - NAPI_GRO_CB(skb)->is_flist = !sk;
> + NAPI_GRO_CB(skb)->is_flist = !sk && !skb_is_gso(skb);
> if (sk)
> sock_gen_put(sk);
> #endif /* IS_ENABLED(CONFIG_IPV6) */
> --
> 2.17.0
>
>
More information about the Linux-mediatek
mailing list