netdev - Re: [RFC PATCH 1/2] net: gro: avoid relaying on skb->transport

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iL3hp4Of_U+Yc34OrwVnTwn5j4j=WTq-yckGVcpptxcUg@mail.gmail.com>
Date: Fri, 5 Dec 2025 06:37:59 -0800
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, 
	Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>, Neal Cardwell <ncardwell@...gle.com>, 
	Kuniyuki Iwashima <kuniyu@...gle.com>, David Ahern <dsahern@...nel.org>
Subject: Re: [RFC PATCH 1/2] net: gro: avoid relaying on skb->transport_header
 at receive time

On Fri, Dec 5, 2025 at 6:04 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> Currently {tcp,udp}_gro_receive relay on the gro network stage setting

rely :)

> the correct transport header offset for all the skbs held by the GRO
> engine.
>
> Such assumption is not necessary, as the code can instead leverage the
> offset already available for the currently processed skb. Add a couple
> of helpers to for readabilty' sake.
>
> As skb->transport_header lays on a different cacheline wrt skb->data,
> this should save a cacheline access for each packet aggregation.
> Additionally this will make the next patch possible.
>
> Note that the compiler (gcc 15.2.1) does inline the tcp_gro_lookup()
> call in tcp_gro_receive(), so the additional argument is only relevant
> for the fraglist case.
>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
>  include/net/gro.h        | 26 ++++++++++++++++++++++++++
>  include/net/tcp.h        |  3 ++-
>  net/ipv4/tcp_offload.c   | 15 ++++++++-------
>  net/ipv4/udp_offload.c   |  4 ++--
>  net/ipv6/tcpv6_offload.c |  2 +-
>  5 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/gro.h b/include/net/gro.h
> index b65f631c521d..fdb9285ab117 100644
> --- a/include/net/gro.h
> +++ b/include/net/gro.h
> @@ -420,6 +420,18 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>                                 struct udphdr *uh, struct sock *sk);
>  int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
>
> +/* Return the skb hdr corresponding to the specified skb2 hdr.
> + * skb2 is held in the gro engine, i.e. its headers are in the linear part.
> + */
> +static inline const void *
> +skb_gro_header_from(const struct sk_buff *skb, const struct sk_buff *skb2,
> +                   const void *hdr2)
> +{
> +       size_t offset = (unsigned char *)hdr2 - skb2->data;
> +
> +       return skb->data + offset;
> +}

I would rather switch gro to pass an @offset instead of a header pointer ?

Rebuilding one header pointer from offset is fast : skb->data + offset
( offset : network header, transport header, ...)

As a matter of fact, some GRO state variables could be onstack, instead
of being stored in NAPI_GRO_CB()

This would avoid some stalls because skb->cb[] has been cleared with
memset() with long words,
while GRO is using smaller fields.