Skip to content

Commit

Permalink
net: sock_queue_err_skb() dont mess with sk_forward_alloc
Browse files Browse the repository at this point in the history
Correct sk_forward_alloc handling for error_queue would need to use a
backlog of frames that softirq handler could not deliver because socket
is owned by user thread. Or extend backlog processing to be able to
process normal and error packets.

Another possibility is to not use mem charge for error queue, this is
what I implemented in this patch.

Note: this reverts commit 2903037
(net: fix sk_forward_alloc corruptions), since we dont need to lock
socket anymore.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Eric Dumazet authored and David S. Miller committed Jun 1, 2010
1 parent bc284f9 commit b1faf56
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 24 deletions.
15 changes: 1 addition & 14 deletions include/net/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -1524,20 +1524,7 @@ extern void sk_stop_timer(struct sock *sk, struct timer_list* timer);

extern int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);

static inline int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
{
/* Cast skb->rcvbuf to unsigned... It's pointless, but reduces
number of warnings when compiling with -W --ANK
*/
if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
(unsigned)sk->sk_rcvbuf)
return -ENOMEM;
skb_set_owner_r(skb, sk);
skb_queue_tail(&sk->sk_error_queue, skb);
if (!sock_flag(sk, SOCK_DEAD))
sk->sk_data_ready(sk, skb->len);
return 0;
}
extern int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb);

/*
* Recover an error report and clear atomically
Expand Down
30 changes: 28 additions & 2 deletions net/core/skbuff.c
Original file line number Diff line number Diff line change
Expand Up @@ -2965,6 +2965,34 @@ int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
}
EXPORT_SYMBOL_GPL(skb_cow_data);

static void sock_rmem_free(struct sk_buff *skb)
{
struct sock *sk = skb->sk;

atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
}

/*
* Note: We dont mem charge error packets (no sk_forward_alloc changes)
*/
int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
{
if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
(unsigned)sk->sk_rcvbuf)
return -ENOMEM;

skb_orphan(skb);
skb->sk = sk;
skb->destructor = sock_rmem_free;
atomic_add(skb->truesize, &sk->sk_rmem_alloc);

skb_queue_tail(&sk->sk_error_queue, skb);
if (!sock_flag(sk, SOCK_DEAD))
sk->sk_data_ready(sk, skb->len);
return 0;
}
EXPORT_SYMBOL(sock_queue_err_skb);

void skb_tstamp_tx(struct sk_buff *orig_skb,
struct skb_shared_hwtstamps *hwtstamps)
{
Expand Down Expand Up @@ -2997,9 +3025,7 @@ void skb_tstamp_tx(struct sk_buff *orig_skb,
serr->ee.ee_errno = ENOMSG;
serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;

bh_lock_sock(sk);
err = sock_queue_err_skb(sk, skb);
bh_unlock_sock(sk);

if (err)
kfree_skb(skb);
Expand Down
6 changes: 2 additions & 4 deletions net/ipv4/udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,11 +633,9 @@ void __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)
if (!inet->recverr) {
if (!harderr || sk->sk_state != TCP_ESTABLISHED)
goto out;
} else {
bh_lock_sock(sk);
} else
ip_icmp_error(sk, skb, err, uh->dest, info, (u8 *)(uh+1));
bh_unlock_sock(sk);
}

sk->sk_err = err;
sk->sk_error_report(sk);
out:
Expand Down
6 changes: 2 additions & 4 deletions net/ipv6/udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -466,11 +466,9 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
if (sk->sk_state != TCP_ESTABLISHED && !np->recverr)
goto out;

if (np->recverr) {
bh_lock_sock(sk);
if (np->recverr)
ipv6_icmp_error(sk, skb, err, uh->dest, ntohl(info), (u8 *)(uh+1));
bh_unlock_sock(sk);
}

sk->sk_err = err;
sk->sk_error_report(sk);
out:
Expand Down

0 comments on commit b1faf56

Please sign in to comment.