netdev - [PATCH net-next 2/2] udp: track the forward memory release threshold in an hot cacheline

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20221019163306.70984-1-kuniyu@amazon.com>
Date:   Wed, 19 Oct 2022 09:33:06 -0700
From:   Kuniyuki Iwashima <kuniyu@...zon.com>
To:     <pabeni@...hat.com>
CC:     <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
        <kuba@...nel.org>, <mathew.j.martineau@...ux.intel.com>,
        <matthieu.baerts@...sares.net>, <mptcp@...ts.linux.dev>,
        <netdev@...r.kernel.org>
Subject: [PATCH net-next 2/2] udp: track the forward memory release threshold in an hot cacheline

From:   Paolo Abeni <pabeni@...hat.com>
Date:   Wed, 19 Oct 2022 12:02:01 +0200
> When the receiver process and the BH runs on different cores,
> udp_rmem_release() experience a cache miss while accessing sk_rcvbuf,
> as the latter shares the same cacheline with sk_forward_alloc, written
> by the BH.
> 
> With this patch, UDP tracks the rcvbuf value and its update via custom
> SOL_SOCKET socket options, and copies the forward memory threshold value
> used by udp_rmem_release() in a different cacheline, already accessed by
> the above function and uncontended.
> 
> Overall the above give a 10% peek throughput increase under UDP flood.
> 
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
>  include/linux/udp.h |  3 +++
>  net/ipv4/udp.c      | 22 +++++++++++++++++++---
>  net/ipv6/udp.c      |  8 ++++++--
>  3 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/udp.h b/include/linux/udp.h
> index e96da4157d04..5cdba00a904a 100644
> --- a/include/linux/udp.h
> +++ b/include/linux/udp.h
> @@ -87,6 +87,9 @@ struct udp_sock {
>  
>  	/* This field is dirtied by udp_recvmsg() */
>  	int		forward_deficit;
> +
> +	/* This fields follows rcvbuf value, and is touched by udp_recvmsg */
> +	int		forward_threshold;
>  };
>  
>  #define UDP_MAX_SEGMENTS	(1 << 6UL)
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 8126f67d18b3..915f573587fa 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1448,7 +1448,7 @@ static void udp_rmem_release(struct sock *sk, int size, int partial,
>  	if (likely(partial)) {
>  		up->forward_deficit += size;
>  		size = up->forward_deficit;
> -		if (size < (sk->sk_rcvbuf >> 2) &&
> +		if (size < READ_ONCE(up->forward_threshold) &&
>  		    !skb_queue_empty(&up->reader_queue))
>  			return;
>  	} else {
> @@ -1622,8 +1622,12 @@ static void udp_destruct_sock(struct sock *sk)
>  
>  int udp_init_sock(struct sock *sk)
>  {
> -	skb_queue_head_init(&udp_sk(sk)->reader_queue);
> +	struct udp_sock *up = udp_sk(sk);
> +
> +	skb_queue_head_init(&up->reader_queue);
> +	up->forward_threshold = sk->sk_rcvbuf >> 2;
>  	sk->sk_destruct = udp_destruct_sock;
> +	set_bit(SOCK_CUSTOM_SOCKOPT, &sk->sk_socket->flags);
>  	return 0;
>  }
>  
> @@ -2671,6 +2675,18 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
>  	int err = 0;
>  	int is_udplite = IS_UDPLITE(sk);
>  
> +	if (level == SOL_SOCKET) {
> +		err = sk_setsockopt(sk, level, optname, optval, optlen);
> +
> +		if (optname == SO_RCVBUF || optname == SO_RCVBUFFORCE) {
> +			sockopt_lock_sock(sk);

Can we drop this lock by adding READ_ONCE() to sk->sk_rcvbuf below ?


> +			/* paired with READ_ONCE in udp_rmem_release() */
> +			WRITE_ONCE(up->forward_threshold, sk->sk_rcvbuf >> 2);
> +			sockopt_release_sock(sk);
> +		}
> +		return err;
> +	}
> +
>  	if (optlen < sizeof(int))
>  		return -EINVAL;
>  
> @@ -2784,7 +2800,7 @@ EXPORT_SYMBOL(udp_lib_setsockopt);
>  int udp_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
>  		   unsigned int optlen)
>  {
> -	if (level == SOL_UDP  ||  level == SOL_UDPLITE)
> +	if (level == SOL_UDP  ||  level == SOL_UDPLITE || level == SOL_SOCKET)
>  		return udp_lib_setsockopt(sk, level, optname,
>  					  optval, optlen,
>  					  udp_push_pending_frames);
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 8d09f0ea5b8c..1ed20bcfd7a0 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -64,8 +64,12 @@ static void udpv6_destruct_sock(struct sock *sk)
>  
>  int udpv6_init_sock(struct sock *sk)
>  {
> -	skb_queue_head_init(&udp_sk(sk)->reader_queue);
> +	struct udp_sock *up = udp_sk(sk);
> +
> +	skb_queue_head_init(&up->reader_queue);
> +	up->forward_threshold = sk->sk_rcvbuf >> 2;
>  	sk->sk_destruct = udpv6_destruct_sock;
> +	set_bit(SOCK_CUSTOM_SOCKOPT, &sk->sk_socket->flags);
>  	return 0;
>  }

It's time to factorise this part like udp_destruct_common() ?


>  
> @@ -1671,7 +1675,7 @@ void udpv6_destroy_sock(struct sock *sk)
>  int udpv6_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval,
>  		     unsigned int optlen)
>  {
> -	if (level == SOL_UDP  ||  level == SOL_UDPLITE)
> +	if (level == SOL_UDP  ||  level == SOL_UDPLITE || level == SOL_SOCKET)
>  		return udp_lib_setsockopt(sk, level, optname,
>  					  optval, optlen,
>  					  udp_v6_push_pending_frames);
> -- 
> 2.37.3