Skip to content

Commit

Permalink
sock: deduplicate errqueue dequeue
Browse files Browse the repository at this point in the history 
sk->sk_error_queue is dequeued in four locations. All share the
exact same logic. Deduplicate.

Also collapse the two critical sections for dequeue (at the top of
the recv handler) and signal (at the bottom).

This moves signal generation for the next packet forward, which should
be harmless.

It also changes the behavior if the recv handler exits early with an
error. Previously, a signal for follow-up packets on the errqueue
would then not be scheduled. The new behavior, to always signal, is
arguably a bug fix.

For rxrpc, the change causes the same function to be called repeatedly
for each queued packet (because the recv handler == sk_error_report).
It is likely that all packets will fail for the same reason (e.g.,
memory exhaustion).

This code runs without sk_lock held, so it is not safe to trust that
sk->sk_err is immutable inbetween releasing q->lock and the subsequent
test. Introduce int err just to avoid this potential race.

Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
@wdebruij @davem330
wdebruij authored and davem330 committed Sep 2, 2014
1 parent 8fe2f76 commit 364a9e9
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 51 deletions.
1 change: 1 addition & 0 deletions include/net/sock.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,7 @@ void sk_stop_timer(struct sock *sk, struct timer_list *timer);
int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);

int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb);
struct sk_buff *sock_dequeue_err_skb(struct sock *sk);

/*
* Recover an error report and clear atomically
Expand Down
20 changes: 20 additions & 0 deletions net/core/skbuff.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3491,6 +3491,26 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL(sock_queue_err_skb);

struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
{
struct sk_buff_head *q = &sk->sk_error_queue;
struct sk_buff *skb, *skb_next;
int err = 0;

spin_lock_bh(&q->lock);
skb = __skb_dequeue(q);
if (skb && (skb_next = skb_peek(q)))
err = SKB_EXT_ERR(skb_next)->ee.ee_errno;
spin_unlock_bh(&q->lock);

sk->sk_err = err;
if (err)
sk->sk_error_report(sk);

return skb;
}
EXPORT_SYMBOL(sock_dequeue_err_skb);

void __skb_tstamp_tx(struct sk_buff *orig_skb,
struct skb_shared_hwtstamps *hwtstamps,
struct sock *sk, int tstype)
Expand Down
14 changes: 2 additions & 12 deletions net/core/sock.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2488,11 +2488,11 @@ int sock_recv_errqueue(struct sock *sk, struct msghdr *msg, int len,
int level, int type)
{
struct sock_exterr_skb *serr;
struct sk_buff *skb, *skb2;
struct sk_buff *skb;
int copied, err;

err = -EAGAIN;
skb = skb_dequeue(&sk->sk_error_queue);
skb = sock_dequeue_err_skb(sk);
if (skb == NULL)
goto out;

Expand All @@ -2513,16 +2513,6 @@ int sock_recv_errqueue(struct sock *sk, struct msghdr *msg, int len,
msg->msg_flags |= MSG_ERRQUEUE;
err = copied;

/* Reset and regenerate socket error */
spin_lock_bh(&sk->sk_error_queue.lock);
sk->sk_err = 0;
if ((skb2 = skb_peek(&sk->sk_error_queue)) != NULL) {
sk->sk_err = SKB_EXT_ERR(skb2)->ee.ee_errno;
spin_unlock_bh(&sk->sk_error_queue.lock);
sk->sk_error_report(sk);
} else
spin_unlock_bh(&sk->sk_error_queue.lock);

out_free_skb:
kfree_skb(skb);
out:
Expand Down
15 changes: 2 additions & 13 deletions net/ipv4/ip_sockglue.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 port, u32 inf
int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
{
struct sock_exterr_skb *serr;
struct sk_buff *skb, *skb2;
struct sk_buff *skb;
DECLARE_SOCKADDR(struct sockaddr_in *, sin, msg->msg_name);
struct {
struct sock_extended_err ee;
Expand All @@ -415,7 +415,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
int copied;

err = -EAGAIN;
skb = skb_dequeue(&sk->sk_error_queue);
skb = sock_dequeue_err_skb(sk);
if (skb == NULL)
goto out;

Expand Down Expand Up @@ -462,17 +462,6 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
msg->msg_flags |= MSG_ERRQUEUE;
err = copied;

/* Reset and regenerate socket error */
spin_lock_bh(&sk->sk_error_queue.lock);
sk->sk_err = 0;
skb2 = skb_peek(&sk->sk_error_queue);
if (skb2 != NULL) {
sk->sk_err = SKB_EXT_ERR(skb2)->ee.ee_errno;
spin_unlock_bh(&sk->sk_error_queue.lock);
sk->sk_error_report(sk);
} else
spin_unlock_bh(&sk->sk_error_queue.lock);

out_free_skb:
kfree_skb(skb);
out:
Expand Down
15 changes: 2 additions & 13 deletions net/ipv6/datagram.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
{
struct ipv6_pinfo *np = inet6_sk(sk);
struct sock_exterr_skb *serr;
struct sk_buff *skb, *skb2;
struct sk_buff *skb;
DECLARE_SOCKADDR(struct sockaddr_in6 *, sin, msg->msg_name);
struct {
struct sock_extended_err ee;
Expand All @@ -342,7 +342,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
int copied;

err = -EAGAIN;
skb = skb_dequeue(&sk->sk_error_queue);
skb = sock_dequeue_err_skb(sk);
if (skb == NULL)
goto out;

Expand Down Expand Up @@ -415,17 +415,6 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
msg->msg_flags |= MSG_ERRQUEUE;
err = copied;

/* Reset and regenerate socket error */
spin_lock_bh(&sk->sk_error_queue.lock);
sk->sk_err = 0;
if ((skb2 = skb_peek(&sk->sk_error_queue)) != NULL) {
sk->sk_err = SKB_EXT_ERR(skb2)->ee.ee_errno;
spin_unlock_bh(&sk->sk_error_queue.lock);
sk->sk_error_report(sk);
} else {
spin_unlock_bh(&sk->sk_error_queue.lock);
}

out_free_skb:
kfree_skb(skb);
out:
Expand Down
14 changes: 1 addition & 13 deletions net/rxrpc/ar-error.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void rxrpc_UDP_error_report(struct sock *sk)

_enter("%p{%d}", sk, local->debug_id);

skb = skb_dequeue(&sk->sk_error_queue);
skb = sock_dequeue_err_skb(sk);
if (!skb) {
_leave("UDP socket errqueue empty");
return;
Expand Down Expand Up @@ -111,18 +111,6 @@ void rxrpc_UDP_error_report(struct sock *sk)
skb_queue_tail(&trans->error_queue, skb);
rxrpc_queue_work(&trans->error_handler);

/* reset and regenerate socket error */
spin_lock_bh(&sk->sk_error_queue.lock);
sk->sk_err = 0;
skb = skb_peek(&sk->sk_error_queue);
if (skb) {
sk->sk_err = SKB_EXT_ERR(skb)->ee.ee_errno;
spin_unlock_bh(&sk->sk_error_queue.lock);
sk->sk_error_report(sk);
} else {
spin_unlock_bh(&sk->sk_error_queue.lock);
}

_leave("");
}

Expand Down

0 comments on commit 364a9e9

Please sign in to comment.