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] [<thread-prev] [day] [month] [year] [list]

Message-ID: <willemdebruijn.kernel.29a0666ba5355@gmail.com>
Date: Sat, 06 Dec 2025 16:26:45 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Paolo Abeni <pabeni@...hat.com>, 
 netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 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

Paolo Abeni wrote:
> Currently {tcp,udp}_gro_receive relay on the gro network stage setting
> 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.

I thought "being held in the gro engine" intended to mean behing held
on the gro_list, i.e., p.

But this is used inverse, where skb2 is the currently arriving packet
and skb == p. Is this intentional and am I just misunderstanding the
intent of this comment? Or is the comment intended to say

"skb is held on the gro list, therefore [..]"

> + */
> +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;
> +}