Skip to content

Commit

Permalink
udp: fix busy polling
Browse files Browse the repository at this point in the history
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.

v3: - make sk_busy_loop_end() nicer (Willem)

v2: - add a READ_ONCE(sk->sk_family) in sk_is_inet() to avoid KCSAN splats.
    - add a sk_is_inet() check in sk_is_udp() (Willem feedback)
    - add a sk_is_inet() check in sk_is_tcp().

Fixes: 2276f58 ("udp: use a separate rx queue for packet reception")
Signed-off-by: Eric Dumazet <[email protected]>
Reviewed-by: Paolo Abeni <[email protected]>
Reviewed-by: Willem de Bruijn <[email protected]>
Reviewed-by: Kuniyuki Iwashima <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
Eric Dumazet authored and davem330 committed Jan 21, 2024
1 parent e3f9bed commit a54d51f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 14 deletions.
6 changes: 0 additions & 6 deletions include/linux/skmsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions include/net/inet_sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,6 @@ static inline unsigned long inet_cmsg_flags(const struct inet_sock *inet)
#define inet_assign_bit(nr, sk, val) \
assign_bit(INET_FLAGS_##nr, &inet_sk(sk)->inet_flags, val)

static inline bool sk_is_inet(struct sock *sk)
{
return sk->sk_family == AF_INET || sk->sk_family == AF_INET6;
}

/**
* sk_to_full_sk - Access to a full socket
* @sk: pointer to a socket
Expand Down
18 changes: 17 additions & 1 deletion include/net/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -2765,9 +2765,25 @@ static inline void skb_setup_tx_timestamp(struct sk_buff *skb, __u16 tsflags)
&skb_shinfo(skb)->tskey);
}

static inline bool sk_is_inet(const struct sock *sk)
{
int family = READ_ONCE(sk->sk_family);

return family == AF_INET || family == AF_INET6;
}

static inline bool sk_is_tcp(const struct sock *sk)
{
return sk->sk_type == SOCK_STREAM && sk->sk_protocol == IPPROTO_TCP;
return sk_is_inet(sk) &&
sk->sk_type == SOCK_STREAM &&
sk->sk_protocol == IPPROTO_TCP;
}

static inline bool sk_is_udp(const struct sock *sk)
{
return sk_is_inet(sk) &&
sk->sk_type == SOCK_DGRAM &&
sk->sk_protocol == IPPROTO_UDP;
}

static inline bool sk_is_stream_unix(const struct sock *sk)
Expand Down
11 changes: 9 additions & 2 deletions net/core/sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -4144,8 +4145,14 @@ bool sk_busy_loop_end(void *p, unsigned long start_time)
{
struct sock *sk = p;

return !skb_queue_empty_lockless(&sk->sk_receive_queue) ||
sk_busy_loop_timeout(sk, start_time);
if (!skb_queue_empty_lockless(&sk->sk_receive_queue))
return true;

if (sk_is_udp(sk) &&
!skb_queue_empty_lockless(&udp_sk(sk)->reader_queue))
return true;

return sk_busy_loop_timeout(sk, start_time);
}
EXPORT_SYMBOL(sk_busy_loop_end);
#endif /* CONFIG_NET_RX_BUSY_POLL */
Expand Down

0 comments on commit a54d51f

Please sign in to comment.