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@xxxxxxxxxx> wrote:
>
> On Fri, Jul 31, 2026 at 8:54 PM Glenn Judd <gmj@xxxxxxxx> 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@xxxxxxxx>
> > ---
> >
> > 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
> >