[PATCH] net: fix segmentation of forwarding fraglist GRO

[PATCH] net: fix segmentation of forwarding fraglist GRO

[Jibin Zhang]

This patch enhances GSO segment checks by verifying the presence
of frag_list and protocol consistency, addressing low throughput
issues on IPv4 servers when used as hotspots

Specifically, it fixes a bug in GSO segmentation when forwarding
GRO packets with frag_list. The function skb_segment_list cannot
correctly process GRO skbs converted by XLAT, because XLAT only
converts the header of the head skb. As a result, skbs in the
frag_list may remain unconverted, leading to protocol
inconsistencies and reduced throughput.

To resolve this, the patch uses skb_segment to handle forwarded
packets converted by XLAT, ensuring that all fragments are
properly converted and segmented.

Signed-off-by: Jibin Zhang <jibin.zhang@mediatek.com>
---
 net/ipv4/tcp_offload.c | 3 ++-
 net/ipv4/udp_offload.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index fdda18b1abda..162a384a15bb 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -104,7 +104,8 @@ static struct sk_buff *tcp4_gso_segment(struct sk_buff *skb,
 	if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
 		return ERR_PTR(-EINVAL);
 
-	if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) {
+	if ((skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) && skb_has_frag_list(skb) &&
+	    (skb->protocol == skb_shinfo(skb)->frag_list->protocol)) {
 		struct tcphdr *th = tcp_hdr(skb);
 
 		if (skb_pagelen(skb) - th->doff * 4 == skb_shinfo(skb)->gso_size)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 19d0b5b09ffa..704fb32d10d7 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -512,7 +512,8 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 		return NULL;
 	}
 
-	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
+	if ((skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) && skb_has_frag_list(gso_skb) &&
+	    (gso_skb->protocol == skb_shinfo(gso_skb)->frag_list->protocol)) {
 		 /* Detect modified geometry and pass those to skb_segment. */
 		if (skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size)
 			return __udp_gso_segment_list(gso_skb, features, is_ipv6);
-- 
2.45.2

Re: [PATCH] net: fix segmentation of forwarding fraglist GRO

[Paolo Abeni]

On 12/17/25 4:55 AM, Jibin Zhang wrote:
> This patch enhances GSO segment checks by verifying the presence
> of frag_list and protocol consistency, addressing low throughput
> issues on IPv4 servers when used as hotspots
> 
> Specifically, it fixes a bug in GSO segmentation when forwarding
> GRO packets with frag_list. The function skb_segment_list cannot
> correctly process GRO skbs converted by XLAT, because XLAT only
> converts the header of the head skb. As a result, skbs in the
> frag_list may remain unconverted, leading to protocol
> inconsistencies and reduced throughput.
> 
> To resolve this, the patch uses skb_segment to handle forwarded
> packets converted by XLAT, ensuring that all fragments are
> properly converted and segmented.
> 
> Signed-off-by: Jibin Zhang <jibin.zhang@mediatek.com>

This looks like a fix, it should target the 'net' tree and include a
suitable Fixes tag.

> ---
>  net/ipv4/tcp_offload.c | 3 ++-
>  net/ipv4/udp_offload.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index fdda18b1abda..162a384a15bb 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -104,7 +104,8 @@ static struct sk_buff *tcp4_gso_segment(struct sk_buff *skb,
>  	if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
>  		return ERR_PTR(-EINVAL);
>  
> -	if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) {
> +	if ((skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) && skb_has_frag_list(skb) &&
> +	    (skb->protocol == skb_shinfo(skb)->frag_list->protocol)) {
>  		struct tcphdr *th = tcp_hdr(skb);
>  
>  		if (skb_pagelen(skb) - th->doff * 4 == skb_shinfo(skb)->gso_size)
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 19d0b5b09ffa..704fb32d10d7 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -512,7 +512,8 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>  		return NULL;
>  	}
>  
> -	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
> +	if ((skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) && skb_has_frag_list(gso_skb) &&
> +	    (gso_skb->protocol == skb_shinfo(gso_skb)->frag_list->protocol)) {
>  		 /* Detect modified geometry and pass those to skb_segment. */
>  		if (skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size)
>  			return __udp_gso_segment_list(gso_skb, features, is_ipv6);

I guess checks should be needed for ipv6.

Also it looks like this skips the CSUM_PARTIAL preparation, and possibly
break csum offload.

Additionally I don't like the ever increasing stack of hacks needed to
let GSO_FRAGLIST operate in the most diverse setups, the simpler fix
would be disabling such aggregation.

/P

Re: [PATCH] net: fix segmentation of forwarding fraglist GRO

[Willem de Bruijn]

Paolo Abeni wrote:
> On 12/17/25 4:55 AM, Jibin Zhang wrote:
> > This patch enhances GSO segment checks by verifying the presence
> > of frag_list and protocol consistency, addressing low throughput
> > issues on IPv4 servers when used as hotspots
> > 
> > Specifically, it fixes a bug in GSO segmentation when forwarding
> > GRO packets with frag_list. The function skb_segment_list cannot
> > correctly process GRO skbs converted by XLAT, because XLAT only
> > converts the header of the head skb. As a result, skbs in the
> > frag_list may remain unconverted, leading to protocol
> > inconsistencies and reduced throughput.
> > 
> > To resolve this, the patch uses skb_segment to handle forwarded
> > packets converted by XLAT, ensuring that all fragments are
> > properly converted and segmented.
> > 
> > Signed-off-by: Jibin Zhang <jibin.zhang@mediatek.com>
> 
> This looks like a fix, it should target the 'net' tree and include a
> suitable Fixes tag.
> 
> > ---
> >  net/ipv4/tcp_offload.c | 3 ++-
> >  net/ipv4/udp_offload.c | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> > index fdda18b1abda..162a384a15bb 100644
> > --- a/net/ipv4/tcp_offload.c
> > +++ b/net/ipv4/tcp_offload.c
> > @@ -104,7 +104,8 @@ static struct sk_buff *tcp4_gso_segment(struct sk_buff *skb,
> >  	if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
> >  		return ERR_PTR(-EINVAL);
> >  
> > -	if (skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) {
> > +	if ((skb_shinfo(skb)->gso_type & SKB_GSO_FRAGLIST) && skb_has_frag_list(skb) &&
> > +	    (skb->protocol == skb_shinfo(skb)->frag_list->protocol)) {
> >  		struct tcphdr *th = tcp_hdr(skb);

Using fraglist gso on a system that modifies packet headers is a known
bad idea. I guess this was not anticipated when the feature was added.
But we've seen plenty of examples with BPF already before.

This skb->protocol change is only one of a variety of ways that the
headers may end up mismatching.

It's not bad to bandaid it and fall back onto regular GSO.

But it seems like we'll continue to have to play whack-a-mole unless
we find a more fundamental solution. E.g., disabling fraglist GRO when
such a path is detected, or downgrade an skb to non-fraglist in paths
like this XLAT.


> >  
> >  		if (skb_pagelen(skb) - th->doff * 4 == skb_shinfo(skb)->gso_size)
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 19d0b5b09ffa..704fb32d10d7 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -512,7 +512,8 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> >  		return NULL;
> >  	}
> >  
> > -	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) {
> > +	if ((skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST) && skb_has_frag_list(gso_skb) &&
> > +	    (gso_skb->protocol == skb_shinfo(gso_skb)->frag_list->protocol)) {
> >  		 /* Detect modified geometry and pass those to skb_segment. */
> >  		if (skb_pagelen(gso_skb) - sizeof(*uh) == skb_shinfo(gso_skb)->gso_size)
> >  			return __udp_gso_segment_list(gso_skb, features, is_ipv6);
> 
> I guess checks should be needed for ipv6.
> 
> Also it looks like this skips the CSUM_PARTIAL preparation, and possibly
> break csum offload.
> 
> Additionally I don't like the ever increasing stack of hacks needed to
> let GSO_FRAGLIST operate in the most diverse setups, the simpler fix
> would be disabling such aggregation.
> 
> /P
>