[RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[Glenn Judd]

Software GRO fails to coalesce small IPv4/TCP segment that was
padded up to the 60-byte minimum Ethernet frame.

The selftest tools/testing/selftests/drivers/net/hw/gro.py subtest
sw_ipv4_data_lrg_1byte sends {100, 1} expecting to receive {101}.
In current code, it receives {100, 1} (no coalescing) instead.

Cause: inet_gro_receive() computes its flush term from
tot_len ^ skb_gro_len() before skb_gro_pull(), while skb_gro_len()
still includes trailing Ethernet padding. A small IPv4/TCP segment
padded up to the 60-byte minimum frame has tot_len != skb_gro_len(),
so flush is set and the runt never coalesces.

Assisted-by: Claude:claude-opus-4-8
Assisted-by: Codex:gpt-5.6
Assisted-by: Meta:internal-AI-tooling
Signed-off-by: Glenn Judd <gmj@meta.com>
---

Notes:
    RFC notes
    ---------
    Per Jakub Kicinski, the open question is fast-path cost: this adds two
    operations to the common IPv4 GRO path for every packet -- reading
    iph->tot_len and the skb_gro_len() comparison.  Everything expensive
    (linear check, trim, pointer refresh, csum recompute) is behind unlikely()
    on the slow path.  Is that per-packet cost worth the coalescing win for
    padded runts?
    
    Testing: netdevsim cannot reproduce this -- it never pads short frames to
    ETH_ZLEN -- so sw_ipv4_data_lrg_1byte passes trivially there.  Reproduced
    and fixed on a real NIC (cx7): baseline FAIL -> patched PASS.  Also
    validated locally under KASAN + CONFIG_FAIL_SKB_REALLOC (no UAF).

 net/ipv4/af_inet.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 32d006c1a8ee..998ff77fd7b9 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1470,6 +1470,7 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
 	const struct net_offload *ops;
 	struct sk_buff *pp = NULL;
 	const struct iphdr *iph;
+	unsigned int tot_len;
 	struct sk_buff *p;
 	unsigned int hlen;
 	unsigned int off;
@@ -1498,6 +1499,25 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
 		goto out;
 
 	NAPI_GRO_CB(skb)->proto = proto;
+
+	tot_len = ntohs(iph->tot_len);
+	if (unlikely(skb_gro_len(skb) > tot_len)) {
+		if (!skb_is_nonlinear(skb)) {
+			if (tot_len < sizeof(*iph) ||
+			    pskb_trim_rcsum(skb, off + tot_len))
+				goto out;
+
+			NAPI_GRO_CB(skb)->frag0 = skb->data;
+			NAPI_GRO_CB(skb)->frag0_len = skb->len;
+			iph = skb_gro_header(skb, hlen, off);
+			if (unlikely(!iph))
+				goto out;
+			if (skb->ip_summed == CHECKSUM_COMPLETE)
+				NAPI_GRO_CB(skb)->csum =
+					skb_checksum(skb, off, tot_len, 0);
+		}
+	}
+
 	flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (ntohl(*(__be32 *)&iph->id) & ~IP_DF));
 
 	list_for_each_entry(p, head, list) {

base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51
-- 
2.53.0-Meta

Re: [RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[Richard Gobert]

Glenn Judd wrote:
> Software GRO fails to coalesce small IPv4/TCP segment that was
> padded up to the 60-byte minimum Ethernet frame.
> 
> The selftest tools/testing/selftests/drivers/net/hw/gro.py subtest
> sw_ipv4_data_lrg_1byte sends {100, 1} expecting to receive {101}.
> In current code, it receives {100, 1} (no coalescing) instead.
> 
> Cause: inet_gro_receive() computes its flush term from
> tot_len ^ skb_gro_len() before skb_gro_pull(), while skb_gro_len()
> still includes trailing Ethernet padding. A small IPv4/TCP segment
> padded up to the 60-byte minimum frame has tot_len != skb_gro_len(),
> so flush is set and the runt never coalesces.
> 
> Assisted-by: Claude:claude-opus-4-8
> Assisted-by: Codex:gpt-5.6
> Assisted-by: Meta:internal-AI-tooling
> Signed-off-by: Glenn Judd <gmj@meta.com>
> ---
> 
> Notes:
>     RFC notes
>     ---------
>     Per Jakub Kicinski, the open question is fast-path cost: this adds two
>     operations to the common IPv4 GRO path for every packet -- reading
>     iph->tot_len and the skb_gro_len() comparison.  Everything expensive
>     (linear check, trim, pointer refresh, csum recompute) is behind unlikely()
>     on the slow path.  Is that per-packet cost worth the coalescing win for
>     padded runts?
>     
>     Testing: netdevsim cannot reproduce this -- it never pads short frames to
>     ETH_ZLEN -- so sw_ipv4_data_lrg_1byte passes trivially there.  Reproduced
>     and fixed on a real NIC (cx7): baseline FAIL -> patched PASS.  Also
>     validated locally under KASAN + CONFIG_FAIL_SKB_REALLOC (no UAF).
> 
>  net/ipv4/af_inet.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 32d006c1a8ee..998ff77fd7b9 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1470,6 +1470,7 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
>  	const struct net_offload *ops;
>  	struct sk_buff *pp = NULL;
>  	const struct iphdr *iph;
> +	unsigned int tot_len;
>  	struct sk_buff *p;
>  	unsigned int hlen;
>  	unsigned int off;
> @@ -1498,6 +1499,25 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
>  		goto out;
>  
>  	NAPI_GRO_CB(skb)->proto = proto;
> +
> +	tot_len = ntohs(iph->tot_len);
> +	if (unlikely(skb_gro_len(skb) > tot_len)) {
> +		if (!skb_is_nonlinear(skb)) {
> +			if (tot_len < sizeof(*iph) ||
> +			    pskb_trim_rcsum(skb, off + tot_len))
> +				goto out;
> +
> +			NAPI_GRO_CB(skb)->frag0 = skb->data;
> +			NAPI_GRO_CB(skb)->frag0_len = skb->len;
> +			iph = skb_gro_header(skb, hlen, off);
> +			if (unlikely(!iph))
> +				goto out;
> +			if (skb->ip_summed == CHECKSUM_COMPLETE)
> +				NAPI_GRO_CB(skb)->csum =
> +					skb_checksum(skb, off, tot_len, 0);
> +		}
> +	}
> +
>  	flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (ntohl(*(__be32 *)&iph->id) & ~IP_DF));
>  
>  	list_for_each_entry(p, head, list) {
> 
> base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51

To address Jakub's question on the fast-path cost: I benchmarked GRO
forwarding with two-minute long iperf sessions using 1, 2 and 4 TCP streams
(17 runs per configuration) and CPU frequency scaling disabled. I also
disabled RSS during the benchmarks because it caused a lot of noise - up to
20% variance in the deltas.

| streams | baseline (Gbit/s)  | patched (Gbit/s)  | delta  |
|---------|--------------------|-------------------|--------|
|    1    | 14.083 ± 1.01      | 14.065 ± 0.67     | −0.13% |
|    2    | 13.891 ± 0.75      | 13.926 ± 0.78     | +0.25% |
|    4    | 13.008 ± 1.26      | 13.029 ± 0.97     | +0.16% |

The two added fast-path operations (iph->tot_len read + the skb_gro_len()
comparison on every IPv4 GRO packet) produce no measurable throughput
change. The deltas are all well under the 95% confidence interval and
indistinguishable from noise.

Re: [RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[Glenn Judd]

On Mon, Aug 10, 2026 at 6:11 AM Richard Gobert <richardbgobert@gmail.com> wrote:
>
> To address Jakub's question on the fast-path cost: I benchmarked GRO
> forwarding with two-minute long iperf sessions using 1, 2 and 4 TCP streams
> (17 runs per configuration) and CPU frequency scaling disabled. I also
> disabled RSS during the benchmarks because it caused a lot of noise - up to
> 20% variance in the deltas.
>
> | streams | baseline (Gbit/s)  | patched (Gbit/s)  | delta  |
> |---------|--------------------|-------------------|--------|
> |    1    | 14.083 ± 1.01      | 14.065 ± 0.67     | −0.13% |
> |    2    | 13.891 ± 0.75      | 13.926 ± 0.78     | +0.25% |
> |    4    | 13.008 ± 1.26      | 13.029 ± 0.97     | +0.16% |
>
> The two added fast-path operations (iph->tot_len read + the skb_gro_len()
> comparison on every IPv4 GRO packet) produce no measurable throughput
> change. The deltas are all well under the 95% confidence interval and
> indistinguishable from noise.

Thanks for that analysis. It's in the ballpark of what I see on my
side, though I do see a small cost in my setup. I'll follow up with a
revised approach in v2.

Re: [RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[Eric Dumazet]

On Fri, Jul 31, 2026 at 8:54 PM Glenn Judd <gmj@meta.com> wrote:
>
> Software GRO fails to coalesce small IPv4/TCP segment that was
> padded up to the 60-byte minimum Ethernet frame.
>
> The selftest tools/testing/selftests/drivers/net/hw/gro.py subtest
> sw_ipv4_data_lrg_1byte sends {100, 1} expecting to receive {101}.
> In current code, it receives {100, 1} (no coalescing) instead.
>
> Cause: inet_gro_receive() computes its flush term from
> tot_len ^ skb_gro_len() before skb_gro_pull(), while skb_gro_len()
> still includes trailing Ethernet padding. A small IPv4/TCP segment
> padded up to the 60-byte minimum frame has tot_len != skb_gro_len(),
> so flush is set and the runt never coalesces.
>
> Assisted-by: Claude:claude-opus-4-8
> Assisted-by: Codex:gpt-5.6
> Assisted-by: Meta:internal-AI-tooling
> Signed-off-by: Glenn Judd <gmj@meta.com>
> ---
>
> Notes:
>     RFC notes
>     ---------
>     Per Jakub Kicinski, the open question is fast-path cost: this adds two
>     operations to the common IPv4 GRO path for every packet -- reading
>     iph->tot_len and the skb_gro_len() comparison.  Everything expensive
>     (linear check, trim, pointer refresh, csum recompute) is behind unlikely()
>     on the slow path.  Is that per-packet cost worth the coalescing win for
>     padded runts?
>
>     Testing: netdevsim cannot reproduce this -- it never pads short frames to
>     ETH_ZLEN -- so sw_ipv4_data_lrg_1byte passes trivially there.  Reproduced
>     and fixed on a real NIC (cx7): baseline FAIL -> patched PASS.  Also
>     validated locally under KASAN + CONFIG_FAIL_SKB_REALLOC (no UAF).
>
>  net/ipv4/af_inet.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 32d006c1a8ee..998ff77fd7b9 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1470,6 +1470,7 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
>         const struct net_offload *ops;
>         struct sk_buff *pp = NULL;
>         const struct iphdr *iph;
> +       unsigned int tot_len;
>         struct sk_buff *p;
>         unsigned int hlen;
>         unsigned int off;
> @@ -1498,6 +1499,25 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
>                 goto out;
>
>         NAPI_GRO_CB(skb)->proto = proto;
> +
> +       tot_len = ntohs(iph->tot_len);
> +       if (unlikely(skb_gro_len(skb) > tot_len)) {
> +               if (!skb_is_nonlinear(skb)) {
> +                       if (tot_len < sizeof(*iph) ||
> +                           pskb_trim_rcsum(skb, off + tot_len))
> +                               goto out;
> +
> +                       NAPI_GRO_CB(skb)->frag0 = skb->data;
> +                       NAPI_GRO_CB(skb)->frag0_len = skb->len;
> +                       iph = skb_gro_header(skb, hlen, off);
> +                       if (unlikely(!iph))
> +                               goto out;
> +                       if (skb->ip_summed == CHECKSUM_COMPLETE)
> +                               NAPI_GRO_CB(skb)->csum =
> +                                       skb_checksum(skb, off, tot_len, 0);

This seems potentially expensive in a malicious network environment.
An attacker could force the host to recompute checksums by adding one
extra byte to the frames.

We can infer what adjustment needs to be done on  NAPI_GRO_CB(skb)->csum
based on the skb->csum changes done by  pskb_trim_rcsum()

  __wsum csum = skb->csum;

  if (tot_len < sizeof(*iph) ||
      pskb_trim_rcsum(skb, off + tot_len))
          goto out;

  NAPI_GRO_CB(skb)->csum = csum_add(NAPI_GRO_CB(skb)->csum,
                                               csum_sub(skb->csum, csum));


> +               }
> +       }
> +
>         flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (ntohl(*(__be32 *)&iph->id) & ~IP_DF));
>
>         list_for_each_entry(p, head, list) {
>
> base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51
> --
> 2.53.0-Meta
>

Re: [RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[Eric Dumazet]

On Thu, Aug 13, 2026 at 9:54 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Jul 31, 2026 at 8:54 PM Glenn Judd <gmj@meta.com> wrote:
> >
> > Software GRO fails to coalesce small IPv4/TCP segment that was
> > padded up to the 60-byte minimum Ethernet frame.
> >
> > The selftest tools/testing/selftests/drivers/net/hw/gro.py subtest
> > sw_ipv4_data_lrg_1byte sends {100, 1} expecting to receive {101}.
> > In current code, it receives {100, 1} (no coalescing) instead.
> >
> > Cause: inet_gro_receive() computes its flush term from
> > tot_len ^ skb_gro_len() before skb_gro_pull(), while skb_gro_len()
> > still includes trailing Ethernet padding. A small IPv4/TCP segment
> > padded up to the 60-byte minimum frame has tot_len != skb_gro_len(),
> > so flush is set and the runt never coalesces.
> >
> > Assisted-by: Claude:claude-opus-4-8
> > Assisted-by: Codex:gpt-5.6
> > Assisted-by: Meta:internal-AI-tooling
> > Signed-off-by: Glenn Judd <gmj@meta.com>
> > ---
> >
> > Notes:
> >     RFC notes
> >     ---------
> >     Per Jakub Kicinski, the open question is fast-path cost: this adds two
> >     operations to the common IPv4 GRO path for every packet -- reading
> >     iph->tot_len and the skb_gro_len() comparison.  Everything expensive
> >     (linear check, trim, pointer refresh, csum recompute) is behind unlikely()
> >     on the slow path.  Is that per-packet cost worth the coalescing win for
> >     padded runts?
> >
> >     Testing: netdevsim cannot reproduce this -- it never pads short frames to
> >     ETH_ZLEN -- so sw_ipv4_data_lrg_1byte passes trivially there.  Reproduced
> >     and fixed on a real NIC (cx7): baseline FAIL -> patched PASS.  Also
> >     validated locally under KASAN + CONFIG_FAIL_SKB_REALLOC (no UAF).
> >
> >  net/ipv4/af_inet.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> > index 32d006c1a8ee..998ff77fd7b9 100644
> > --- a/net/ipv4/af_inet.c
> > +++ b/net/ipv4/af_inet.c
> > @@ -1470,6 +1470,7 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
> >         const struct net_offload *ops;
> >         struct sk_buff *pp = NULL;
> >         const struct iphdr *iph;
> > +       unsigned int tot_len;
> >         struct sk_buff *p;
> >         unsigned int hlen;
> >         unsigned int off;
> > @@ -1498,6 +1499,25 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
> >                 goto out;
> >
> >         NAPI_GRO_CB(skb)->proto = proto;
> > +
> > +       tot_len = ntohs(iph->tot_len);
> > +       if (unlikely(skb_gro_len(skb) > tot_len)) {
> > +               if (!skb_is_nonlinear(skb)) {
> > +                       if (tot_len < sizeof(*iph) ||
> > +                           pskb_trim_rcsum(skb, off + tot_len))
> > +                               goto out;
> > +
> > +                       NAPI_GRO_CB(skb)->frag0 = skb->data;
> > +                       NAPI_GRO_CB(skb)->frag0_len = skb->len;

Also do not change frag0 and frag0_len: In GRO, frag0 is strictly
reserved for page-fragmented skbs (napi_gro_frags(), where
!skb_headlen(skb)).
For linear skbs, frag0 must remain NULL.
Calling iph = skb_gro_header(skb, hlen, off) is sufficient to refresh
iph in case pskb_trim_rcsum() reallocated the head.

> > +                       iph = skb_gro_header(skb, hlen, off);
> > +                       if (unlikely(!iph))
> > +                               goto out;
> > +                       if (skb->ip_summed == CHECKSUM_COMPLETE)
> > +                               NAPI_GRO_CB(skb)->csum =
> > +                                       skb_checksum(skb, off, tot_len, 0);
>
> This seems potentially expensive in a malicious network environment.
> An attacker could force the host to recompute checksums by adding one
> extra byte to the frames.
>
> We can infer what adjustment needs to be done on  NAPI_GRO_CB(skb)->csum
> based on the skb->csum changes done by  pskb_trim_rcsum()
>
>   __wsum csum = skb->csum;
>
>   if (tot_len < sizeof(*iph) ||
>       pskb_trim_rcsum(skb, off + tot_len))
>           goto out;
>
>   NAPI_GRO_CB(skb)->csum = csum_add(NAPI_GRO_CB(skb)->csum,
>                                                csum_sub(skb->csum, csum));
>
>
> > +               }
> > +       }
> > +
> >         flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (ntohl(*(__be32 *)&iph->id) & ~IP_DF));
> >
> >         list_for_each_entry(p, head, list) {
> >
> > base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51
> > --
> > 2.53.0-Meta
> >

Re: [RFC PATCH net-next] net: gro: coalesce padded small IPv4 TCP segments

[Glenn Judd]

> Also do not change frag0 and frag0_len: In GRO, frag0 is strictly
> reserved for page-fragmented skbs (napi_gro_frags(), where
> !skb_headlen(skb)). For linear skbs, frag0 must remain NULL.

Thanks for all the feedback. I have incorporated the partial
checksum optimization into my prototype.

Before presenting a prototype that addresses all feedback and
provides quantitative evaluation, I think it's important to discuss
the frag0, frag0_len point that you raised above.

gro.h:20 agrees with that description:
"Virtual address of skb_shinfo(skb)->frags[0].page + offset"

As far as I can tell, however, this appears to have changed
in c7583e9f768e ("net: gro: enable fast path for more cases"):
  "We therefore can initialize frag0 to skb->data so that GRO fast path
can be used in the following additional cases:
  - Drivers using header split (populating skb->data with headers, and
    having payload in one or more page fragments).
  - Drivers not using any page frag (entire packet is in skb->data)"

skb_gro_reset_offset() was modified as follows:
         NAPI_GRO_CB(skb)->data_offset = 0;
-        NAPI_GRO_CB(skb)->frag0 = NULL;
-        NAPI_GRO_CB(skb)->frag0_len = 0;
+        headlen = skb_headlen(skb);
+        NAPI_GRO_CB(skb)->frag0 = skb->data;
+        NAPI_GRO_CB(skb)->frag0_len = headlen;
+        if (headlen)
+                return;

-        if (!skb_headlen(skb) && pinfo->nr_frags &&
+       if (pinfo->nr_frags && ...

Unless I'm misreading something here, not touching frag0 and
frag0_len appears to leave the trim patch stuck. Trimming
shortens the skb, so frag0_len goes stale, resulting in reads
past the trimmed length.

Updating frag0_len after trim solves one inconsistency, but
updating *only* frag0_len causes a new inconsistency.
skb_gro_header() computes frag0 + offset before testing
skb_gro_may_pull(), while pskb_trim_rcsum() calls
skb_might_realloc() first, so the head can move even for
a linear skb. Refreshing iph through skb_gro_header()
does not appear to be enough on its own.

I believe that both frag0_len and frag0 must be updated together.
This could be coded directly in af_inet.c, but gro.c/h owns updates to
these fields. If we want to cleanly update these after a trim, we need
a small helper in gro.h (other fields already have similar helpers):

static inline void skb_gro_reset_frag0(struct sk_buff *skb)
{
        NAPI_GRO_CB(skb)->frag0 = skb->data;
        NAPI_GRO_CB(skb)->frag0_len = skb_headlen(skb);
}

with skb_gro_reset_offset() using it for its linear path, and af_inet.c
calling it after a successful trim. The write then belongs to
GRO rather than IPv4.

Let me know what you think.

Thanks.