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

On Fri, Jul 31, 2026 at 8:54 PM Glenn Judd <[email protected]> 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 <[email protected]>
> ---
>
> 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
>