[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65a9407bd77fc_1caa3f29452@willemb.c.googlers.com.notmuch>
Date: Thu, 18 Jan 2024 10:15:07 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Eric Dumazet <edumazet@...gle.com>,
"David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: Willem de Bruijn <willemb@...gle.com>,
David Ahern <dsahern@...nel.org>,
netdev@...r.kernel.org,
eric.dumazet@...il.com,
Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net] udp: fix busy polling
Eric Dumazet wrote:
> Generic sk_busy_loop_end() only looks at sk->sk_receive_queue
> for presence of packets.
>
> Problem is that for UDP sockets after blamed commit, some packets
> could be present in another queue: udp_sk(sk)->reader_queue
>
> In some cases, a busy poller could spin until timeout expiration,
> even if some packets are available in udp_sk(sk)->reader_queue.
>
> Fixes: 2276f58ac589 ("udp: use a separate rx queue for packet reception")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
> include/linux/skmsg.h | 6 ------
> include/net/sock.h | 5 +++++
> net/core/sock.c | 10 +++++++++-
> 3 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> index 888a4b217829fd4d6baf52f784ce35e9ad6bd0ed..e65ec3fd27998a5b82fc2c4597c575125e653056 100644
> --- a/include/linux/skmsg.h
> +++ b/include/linux/skmsg.h
> @@ -505,12 +505,6 @@ static inline bool sk_psock_strp_enabled(struct sk_psock *psock)
> return !!psock->saved_data_ready;
> }
>
> -static inline bool sk_is_udp(const struct sock *sk)
> -{
> - return sk->sk_type == SOCK_DGRAM &&
> - sk->sk_protocol == IPPROTO_UDP;
> -}
> -
> #if IS_ENABLED(CONFIG_NET_SOCK_MSG)
>
> #define BPF_F_STRPARSER (1UL << 1)
> diff --git a/include/net/sock.h b/include/net/sock.h
> index a7f815c7cfdfdf1296be2967fd100efdb10cdd63..b1ceba8e179aa5cc4c90e98d353551b3a3e1ab86 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2770,6 +2770,11 @@ static inline bool sk_is_tcp(const struct sock *sk)
> return sk->sk_type == SOCK_STREAM && sk->sk_protocol == IPPROTO_TCP;
> }
>
> +static inline bool sk_is_udp(const struct sock *sk)
> +{
> + return sk->sk_type == SOCK_DGRAM && sk->sk_protocol == IPPROTO_UDP;
> +}
> +
Since busy polling code is protocol (family) independent, is it safe
to assume sk->sk_family == PF_INET or PF_INET6 here?
> static inline bool sk_is_stream_unix(const struct sock *sk)
> {
> return sk->sk_family == AF_UNIX && sk->sk_type == SOCK_STREAM;
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 158dbdebce6a3693deb63e557e856d9cdd7500ae..e7e2435ed28681772bf3637b96ddd9334e6a639e 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -107,6 +107,7 @@
> #include <linux/interrupt.h>
> #include <linux/poll.h>
> #include <linux/tcp.h>
> +#include <linux/udp.h>
> #include <linux/init.h>
> #include <linux/highmem.h>
> #include <linux/user_namespace.h>
> @@ -4143,8 +4144,15 @@ subsys_initcall(proto_init);
> bool sk_busy_loop_end(void *p, unsigned long start_time)
> {
> struct sock *sk = p;
> + bool packet_ready;
>
> - return !skb_queue_empty_lockless(&sk->sk_receive_queue) ||
> + packet_ready = !skb_queue_empty_lockless(&sk->sk_receive_queue);
> + if (!packet_ready && sk_is_udp(sk)) {
> + struct sk_buff_head *reader_queue = &udp_sk(sk)->reader_queue;
> +
> + packet_ready = !skb_queue_empty_lockless(reader_queue);
> + }
> + return packet_ready ||
> sk_busy_loop_timeout(sk, start_time);
> }
> EXPORT_SYMBOL(sk_busy_loop_end);
> --
> 2.43.0.381.gb435a96ce8-goog
>
Powered by blists - more mailing lists