* [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() @ 2026-07-23 9:16 zhaoping.shu 2026-07-23 13:39 ` Jakub Kicinski 0 siblings, 1 reply; 5+ messages in thread From: zhaoping.shu @ 2026-07-23 9:16 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, horms, ncardwell, kuniyu, matthias.bgg, angelogioacchino.delregno, willemb, alice, sd, eilaimemedsnaimel, imv4bel, hw.he, netdev, linux-kernel, linux-arm-kernel, linux-mediatek Cc: haijun.liu, xiayu.zhang, lambert.wang, Iven.Yang, Zhaoping Shu From: HW He <hw.he@mediatek.com> On devices that support both NETIF_F_GRO_HW and NETIF_F_GRO_FRAGLIST, the hardware or driver may deliver packets that have already been 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. Signed-off-by: HW He <hw.he@mediatek.com> Signed-off-by: Zhaoping Shu <zhaoping.shu@mediatek.com> --- v2: Take Antoine Tenart's suggestion v1: https://lore.kernel.org/netdev/amB7wb1A2Oo2dv5d@kwain/ --- net/ipv4/tcp_offload.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index 3b1fdcd3cb29..c363434a5646 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; + NAPI_GRO_CB(skb)->flush |= skb_is_gso(skb); skb_set_network_header(skb, skb_gro_receive_network_offset(skb)); if (flush || skb_gro_receive_list(p, skb)) -- 2.17.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() 2026-07-23 9:16 [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() zhaoping.shu @ 2026-07-23 13:39 ` Jakub Kicinski 2026-07-23 14:02 ` Willem de Bruijn 0 siblings, 1 reply; 5+ messages in thread From: Jakub Kicinski @ 2026-07-23 13:39 UTC (permalink / raw) To: zhaoping.shu Cc: davem, edumazet, pabeni, horms, ncardwell, kuniyu, matthias.bgg, angelogioacchino.delregno, willemb, alice, sd, eilaimemedsnaimel, imv4bel, hw.he, netdev, linux-kernel, linux-arm-kernel, linux-mediatek, haijun.liu, xiayu.zhang, lambert.wang, Iven.Yang On Thu, 23 Jul 2026 17:16:01 +0800 zhaoping.shu@mediatek.com wrote: > From: HW He <hw.he@mediatek.com> > > On devices that support both NETIF_F_GRO_HW and NETIF_F_GRO_FRAGLIST, "devices that support FRAGLIST"? Isn't it a software feature? > 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. Also - you're missing a Fixes tag. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() 2026-07-23 13:39 ` Jakub Kicinski @ 2026-07-23 14:02 ` Willem de Bruijn 2026-07-29 7:39 ` Zhaoping Shu (舒召平) 0 siblings, 1 reply; 5+ messages in thread From: Willem de Bruijn @ 2026-07-23 14:02 UTC (permalink / raw) To: Jakub Kicinski Cc: zhaoping.shu, davem, edumazet, pabeni, horms, ncardwell, kuniyu, matthias.bgg, angelogioacchino.delregno, alice, sd, eilaimemedsnaimel, imv4bel, hw.he, netdev, linux-kernel, linux-arm-kernel, linux-mediatek, haijun.liu, xiayu.zhang, lambert.wang, Iven.Yang On Thu, Jul 23, 2026 at 3:39 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 23 Jul 2026 17:16:01 +0800 zhaoping.shu@mediatek.com wrote: > > From: HW He <hw.he@mediatek.com> > > > > On devices that support both NETIF_F_GRO_HW and NETIF_F_GRO_FRAGLIST, > > "devices that support FRAGLIST"? Isn't it a software feature? > > > 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. 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() 2026-07-23 14:02 ` Willem de Bruijn @ 2026-07-29 7:39 ` Zhaoping Shu (舒召平) 2026-07-29 18:57 ` Willem de Bruijn 0 siblings, 1 reply; 5+ messages in thread From: Zhaoping Shu (舒召平) @ 2026-07-29 7:39 UTC (permalink / raw) To: kuba@kernel.org, willemb@google.com Cc: sd@queasysnail.net, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, imv4bel@gmail.com, alice@isovalent.com, eilaimemedsnaimel@gmail.com, HW He (何伟), Haijun Liu (刘海军), horms@kernel.org, Xiayu Zhang (张夏宇), Iven Yang (阳光), pabeni@redhat.com, edumazet@google.com, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Lambert Wang (王伟), matthias.bgg@gmail.com, davem@davemloft.net, AngeloGioacchino Del Regno, kuniyu@google.com, ncardwell@google.com 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@kernel.org> > wrote: > > > > On Thu, 23 Jul 2026 17:16:01 +0800 zhaoping.shu@mediatek.com wrote: > > > From: HW He <hw.he@mediatek.com> > > > > > > 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. 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(). ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() 2026-07-29 7:39 ` Zhaoping Shu (舒召平) @ 2026-07-29 18:57 ` Willem de Bruijn 0 siblings, 0 replies; 5+ messages in thread From: Willem de Bruijn @ 2026-07-29 18:57 UTC (permalink / raw) To: Zhaoping Shu (舒召平), kuba@kernel.org, willemb@google.com Cc: sd@queasysnail.net, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, imv4bel@gmail.com, alice@isovalent.com, eilaimemedsnaimel@gmail.com, HW He (何伟), Haijun Liu (刘海军), horms@kernel.org, Xiayu Zhang (张夏宇), Iven Yang (阳光), pabeni@redhat.com, edumazet@google.com, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Lambert Wang (王伟), matthias.bgg@gmail.com, davem@davemloft.net, AngeloGioacchino Del Regno, kuniyu@google.com, ncardwell@google.com, steffen.klassert 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@kernel.org> > > wrote: > > > > > > On Thu, 23 Jul 2026 17:16:01 +0800 zhaoping.shu@mediatek.com wrote: > > > > From: HW He <hw.he@mediatek.com> > > > > > > > > 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. > 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-29 18:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-23 9:16 [PATCH net v2] net: gro: avoid nesting TCP GSO skbs in skb_gro_receive_list() zhaoping.shu 2026-07-23 13:39 ` Jakub Kicinski 2026-07-23 14:02 ` Willem de Bruijn 2026-07-29 7:39 ` Zhaoping Shu (舒召平) 2026-07-29 18:57 ` Willem de Bruijn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox