netdev - Re: [PATCH net-next] tcp: avoid atomic operations on sk->sk_rmem

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: <20250324223243.76632-1-kuniyu@amazon.com>
Date: Mon, 24 Mar 2025 15:32:15 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <edumazet@...gle.com>
CC: <davem@...emloft.net>, <eric.dumazet@...il.com>, <horms@...nel.org>,
	<kuba@...nel.org>, <kuniyu@...zon.com>, <ncardwell@...gle.com>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>
Subject: Re: [PATCH net-next] tcp: avoid atomic operations on sk->sk_rmem_alloc

From: Eric Dumazet <edumazet@...gle.com>
Date: Thu, 20 Mar 2025 12:16:04 +0000
> TCP uses generic skb_set_owner_r() and sock_rfree()
> for received packets, with socket lock being owned.
> 
> Switch to private versions, avoiding two atomic operations
> per packet.
> 
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
>  include/net/tcp.h       | 15 +++++++++++++++
>  net/ipv4/tcp.c          | 18 ++++++++++++++++--
>  net/ipv4/tcp_fastopen.c |  2 +-
>  net/ipv4/tcp_input.c    |  6 +++---
>  4 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d08fbf90495de69b157d3c87c50e82d781a365df..dd6d63a6f42b99774e9461b69d3e7932cf629082 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -779,6 +779,7 @@ static inline int tcp_bound_to_half_wnd(struct tcp_sock *tp, int pktsize)
>  
>  /* tcp.c */
>  void tcp_get_info(struct sock *, struct tcp_info *);
> +void tcp_sock_rfree(struct sk_buff *skb);
>  
>  /* Read 'sendfile()'-style from a TCP socket */
>  int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
> @@ -2898,4 +2899,18 @@ enum skb_drop_reason tcp_inbound_hash(struct sock *sk,
>  		const void *saddr, const void *daddr,
>  		int family, int dif, int sdif);
>  
> +/* version of skb_set_owner_r() avoiding one atomic_add() */
> +static inline void tcp_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
> +{
> +	skb_orphan(skb);
> +	skb->sk = sk;
> +	skb->destructor = tcp_sock_rfree;
> +
> +	sock_owned_by_me(sk);
> +	atomic_set(&sk->sk_rmem_alloc,
> +		   atomic_read(&sk->sk_rmem_alloc) + skb->truesize);

It's nice to learn that atomic_set() and _read() are just __WRITE_ONCE()
and __READ_ONCE() and have no LOCK_PREFIX :)

Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>

Thanks!