net: sock_queue_err_skb() dont mess with sk_forward_alloc

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 29030374
(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>

diff --git a/include/net/sock.h b/include/net/sock.h
index ca241ea148758a555d123343e2fe8639f7dd5bd3..731150d52799ecc5492b3590c3b7b5b34de09072 100644 (file)
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -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

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4e7ac09c281a4859874851459a2472ad30a955ab..9f07e749d7b15ce9451a8a9b8ac7ed5318568ff3 100644 (file)
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -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)
 {
@@ -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);

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 50678f9a27632dec479288a394f37b5f77a49ac5..eec4ff456e332e0b086d5451478fa5874213e0d1 100644 (file)
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -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:

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 3048f906c0422a08a385514a6702e1ccf04f324d..87be58673b55fffc75e34ec0d6cc1dceafd7de8e 100644 (file)
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -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: