Netdev List
 help / color / mirror / Atom feed
* [PATCH V2 5/7] sctp: use limited socket backlog
From: Zhu Yi @ 2010-03-03  8:36 UTC (permalink / raw)
  To: netdev; +Cc: Zhu Yi, Vlad Yasevich, Sridhar Samudrala
In-Reply-To: <1267605389-7369-4-git-send-email-yi.zhu@intel.com>

Make sctp adapt to the limited socket backlog change.

Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
Cc: Sridhar Samudrala <sri@us.ibm.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/sctp/input.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index c0c973e..20e69c3 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -75,7 +75,7 @@ static struct sctp_association *__sctp_lookup_association(
 					const union sctp_addr *peer,
 					struct sctp_transport **pt);
 
-static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
+static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
 
 
 /* Calculate the SCTP checksum of an SCTP packet.  */
@@ -265,8 +265,12 @@ int sctp_rcv(struct sk_buff *skb)
 	}
 
 	if (sock_owned_by_user(sk)) {
+		if (sctp_add_backlog(sk, skb)) {
+			sctp_bh_unlock_sock(sk);
+			sctp_chunk_free(chunk);
+			goto discard_release;
+		}
 		SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_BACKLOG);
-		sctp_add_backlog(sk, skb);
 	} else {
 		SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_SOFTIRQ);
 		sctp_inq_push(&chunk->rcvr->inqueue, chunk);
@@ -362,7 +366,7 @@ done:
 	return 0;
 }
 
-static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
+static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
 {
 	struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
 	struct sctp_ep_common *rcvr = chunk->rcvr;
@@ -377,7 +381,7 @@ static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
 	else
 		BUG();
 
-	sk_add_backlog(sk, skb);
+	return sk_add_backlog_limited(sk, skb);
 }
 
 /* Handle icmp frag needed error. */
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH V2 3/7] udp: use limited socket backlog
From: Zhu Yi @ 2010-03-03  8:36 UTC (permalink / raw)
  To: netdev
  Cc: Zhu Yi, David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	Patrick McHardy
In-Reply-To: <1267605389-7369-2-git-send-email-yi.zhu@intel.com>

Make udp adapt to the limited socket backlog change.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/ipv4/udp.c |    6 ++++--
 net/ipv6/udp.c |   28 ++++++++++++++++++----------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 608a544..e7eb47f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1371,8 +1371,10 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	bh_lock_sock(sk);
 	if (!sock_owned_by_user(sk))
 		rc = __udp_queue_rcv_skb(sk, skb);
-	else
-		sk_add_backlog(sk, skb);
+	else if (sk_add_backlog_limited(sk, skb)) {
+		bh_unlock_sock(sk);
+		goto drop;
+	}
 	bh_unlock_sock(sk);
 
 	return rc;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 52b8347..6480491 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -583,16 +583,20 @@ static void flush_stack(struct sock **stack, unsigned int count,
 			bh_lock_sock(sk);
 			if (!sock_owned_by_user(sk))
 				udpv6_queue_rcv_skb(sk, skb1);
-			else
-				sk_add_backlog(sk, skb1);
+			else if (sk_add_backlog_limited(sk, skb1)) {
+				kfree_skb(skb1);
+				bh_unlock_sock(sk);
+				goto drop;
+			}
 			bh_unlock_sock(sk);
-		} else {
-			atomic_inc(&sk->sk_drops);
-			UDP6_INC_STATS_BH(sock_net(sk),
-					UDP_MIB_RCVBUFERRORS, IS_UDPLITE(sk));
-			UDP6_INC_STATS_BH(sock_net(sk),
-					UDP_MIB_INERRORS, IS_UDPLITE(sk));
+			continue;
 		}
+drop:
+		atomic_inc(&sk->sk_drops);
+		UDP6_INC_STATS_BH(sock_net(sk),
+				UDP_MIB_RCVBUFERRORS, IS_UDPLITE(sk));
+		UDP6_INC_STATS_BH(sock_net(sk),
+				UDP_MIB_INERRORS, IS_UDPLITE(sk));
 	}
 }
 /*
@@ -754,8 +758,12 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	bh_lock_sock(sk);
 	if (!sock_owned_by_user(sk))
 		udpv6_queue_rcv_skb(sk, skb);
-	else
-		sk_add_backlog(sk, skb);
+	else if (sk_add_backlog_limited(sk, skb)) {
+		atomic_inc(&sk->sk_drops);
+		bh_unlock_sock(sk);
+		sock_put(sk);
+		goto discard;
+	}
 	bh_unlock_sock(sk);
 	sock_put(sk);
 	return 0;
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH V2 2/7] tcp: use limited socket backlog
From: Zhu Yi @ 2010-03-03  8:36 UTC (permalink / raw)
  To: netdev
  Cc: Zhu Yi, David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	Patrick McHardy
In-Reply-To: <1267605389-7369-1-git-send-email-yi.zhu@intel.com>

Make tcp adapt to the limited socket backlog change.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/ipv4/tcp_ipv4.c |    6 ++++--
 net/ipv6/tcp_ipv6.c |    6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c3588b4..4baf194 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1682,8 +1682,10 @@ process:
 			if (!tcp_prequeue(sk, skb))
 				ret = tcp_v4_do_rcv(sk, skb);
 		}
-	} else
-		sk_add_backlog(sk, skb);
+	} else if (sk_add_backlog_limited(sk, skb)) {
+		bh_unlock_sock(sk);
+		goto discard_and_relse;
+	}
 	bh_unlock_sock(sk);
 
 	sock_put(sk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6963a6b..c4ea9d5 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1740,8 +1740,10 @@ process:
 			if (!tcp_prequeue(sk, skb))
 				ret = tcp_v6_do_rcv(sk, skb);
 		}
-	} else
-		sk_add_backlog(sk, skb);
+	} else if (sk_add_backlog_limited(sk, skb)) {
+		bh_unlock_sock(sk);
+		goto discard_and_relse;
+	}
 	bh_unlock_sock(sk);
 
 	sock_put(sk);
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH V2 1/7] net: add limit for socket backlog
From: Zhu Yi @ 2010-03-03  8:36 UTC (permalink / raw)
  To: netdev
  Cc: Zhu Yi, David Miller, Arnaldo Carvalho de Melo,
	Pekka Savola (ipv6), Patrick McHardy, Vlad Yasevich,
	Sridhar Samudrala, Jon Maloy, Allan Stephens, Andrew Hendry,
	Eric Dumazet

We got system OOM while running some UDP netperf testing on the loopback
device. The case is multiple senders sent stream UDP packets to a single
receiver via loopback on local host. Of course, the receiver is not able
to handle all the packets in time. But we surprisingly found that these
packets were not discarded due to the receiver's sk->sk_rcvbuf limit.
Instead, they are kept queuing to sk->sk_backlog and finally ate up all
the memory. We believe this is a secure hole that a none privileged user
can crash the system.

The root cause for this problem is, when the receiver is doing
__release_sock() (i.e. after userspace recv, kernel udp_recvmsg ->
skb_free_datagram_locked -> release_sock), it moves skbs from backlog to
sk_receive_queue with the softirq enabled. In the above case, multiple
busy senders will almost make it an endless loop. The skbs in the
backlog end up eat all the system memory.

The issue is not only for UDP. Any protocols using socket backlog is
potentially affected. The patch adds limit for socket backlog so that
the backlog size cannot be expanded endlessly.

Reported-by: Alex Shi <alex.shi@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
Cc: Sridhar Samudrala <sri@us.ibm.com>
Cc: Jon Maloy <jon.maloy@ericsson.com>
Cc: Allan Stephens <allan.stephens@windriver.com>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/sock.h |   15 ++++++++++++++-
 net/core/sock.c    |   16 ++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 6cb1676..2516d76 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -253,6 +253,8 @@ struct sock {
 	struct {
 		struct sk_buff *head;
 		struct sk_buff *tail;
+		int len;
+		int limit;
 	} sk_backlog;
 	wait_queue_head_t	*sk_sleep;
 	struct dst_entry	*sk_dst_cache;
@@ -589,7 +591,7 @@ static inline int sk_stream_memory_free(struct sock *sk)
 	return sk->sk_wmem_queued < sk->sk_sndbuf;
 }
 
-/* The per-socket spinlock must be held here. */
+/* OOB backlog add */
 static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
 {
 	if (!sk->sk_backlog.tail) {
@@ -601,6 +603,17 @@ static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
 	skb->next = NULL;
 }
 
+/* The per-socket spinlock must be held here. */
+static inline int sk_add_backlog_limited(struct sock *sk, struct sk_buff *skb)
+{
+	if (sk->sk_backlog.len >= max(sk->sk_backlog.limit, sk->sk_rcvbuf << 1))
+		return -ENOBUFS;
+
+	sk_add_backlog(sk, skb);
+	sk->sk_backlog.len += skb->truesize;
+	return 0;
+}
+
 static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	return sk->sk_backlog_rcv(sk, skb);
diff --git a/net/core/sock.c b/net/core/sock.c
index fcd397a..6e22dc9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -340,8 +340,12 @@ int sk_receive_skb(struct sock *sk, struct sk_buff *skb, const int nested)
 		rc = sk_backlog_rcv(sk, skb);
 
 		mutex_release(&sk->sk_lock.dep_map, 1, _RET_IP_);
-	} else
-		sk_add_backlog(sk, skb);
+	} else if (sk_add_backlog_limited(sk, skb)) {
+		bh_unlock_sock(sk);
+		atomic_inc(&sk->sk_drops);
+		goto discard_and_relse;
+	}
+
 	bh_unlock_sock(sk);
 out:
 	sock_put(sk);
@@ -1139,6 +1143,7 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
 		sock_lock_init(newsk);
 		bh_lock_sock(newsk);
 		newsk->sk_backlog.head	= newsk->sk_backlog.tail = NULL;
+		newsk->sk_backlog.len = 0;
 
 		atomic_set(&newsk->sk_rmem_alloc, 0);
 		/*
@@ -1542,6 +1547,12 @@ static void __release_sock(struct sock *sk)
 
 		bh_lock_sock(sk);
 	} while ((skb = sk->sk_backlog.head) != NULL);
+
+	/*
+	 * Doing the zeroing here guarantee we can not loop forever
+	 * while a wild producer attempts to flood us.
+	 */
+	sk->sk_backlog.len = 0;
 }
 
 /**
@@ -1874,6 +1885,7 @@ void sock_init_data(struct socket *sock, struct sock *sk)
 	sk->sk_allocation	=	GFP_KERNEL;
 	sk->sk_rcvbuf		=	sysctl_rmem_default;
 	sk->sk_sndbuf		=	sysctl_wmem_default;
+	sk->sk_backlog.limit	=	sk->sk_rcvbuf << 1;
 	sk->sk_state		=	TCP_CLOSE;
 	sk_set_socket(sk, sock);
 
-- 
1.6.3.3


^ permalink raw reply related

* Re: KS8695: possible NAPI issue
From: Yegor Yefremov @ 2010-03-03  8:28 UTC (permalink / raw)
  To: Dick Hollenbeck; +Cc: figo zhang, netdev, zealcook
In-Reply-To: <4B8DED29.9000605@softplc.com>

>>>> I'm using 2.6.33 kernel and I noticed such a strange behavior:
>>>>
>>>> after system start I transfer one file via netcat from my development
>>>> host, after this transfer the network is not functioning i.e. no pings
>>>> possible etc.
>>>>
>>>> To narrow down the problem I checked out this commit
>>>>
>>>>
>>>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=451f14439847db302e5104c44458b2dbb4b1829d.
>>>> Till here the network driver is functioning as intended, but after
>>>> NAPI introduction I have this issue. With latest git-pull of "Linus'
>>>> kernel tree" I can't even ping right after the systems start.
>>>>
>>>> Any Ideas? What am I missing?
>>>>
>>>>
>>>
>>> No idea, although I am using the same ARM chip, my kernel is at 2.6.30.5,
>>> and except for this occasional loss of connection I get, the ethernet
>>> driver
>>> seems to work better than what you are reporting.
>>>
>>
>> from linux-2.6.32, this ethernet  driver have add NAPI support, would
>> you like to
>> try using this version?
>>
>
> Seems like a lot of work for me, given that my 2.6.30.5 kernel is mostly
> working.   Yegor is having the problems on the newer kernels, newer than
> mine.  Sounds like a warning flag to me, not an incentive.
>
> What is NAPI and why do I care?

Here is Wikipedia article: http://en.wikipedia.org/wiki/NAPI

The problem also is that I really need a kernel version not less than
2.6.33 because of the support of other platforms.

I sniffed a little with wireshark and the symptoms are the following:

after netcat transfer the system sends only ARP requests and no other packets.

I just git-pulled the latest Linus tree and the network still has the
above described issues.

Figo, could you test the latest kernel with netcat transfers? So you
could reproduce this behavior?

Yegor

^ permalink raw reply

* [patch] cassini: fix off by one
From: Dan Carpenter @ 2010-03-03  8:22 UTC (permalink / raw)
  To: netdev
  Cc: Alexey Dobriyan, Andrew Morton, Yang Hongyang, Patrick McHardy,
	David S. Miller, linux-kernel, kernel-janitors

There are only 6 link_modes.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
index 7cbcfb0..9bd155e 100644
--- a/drivers/net/cassini.c
+++ b/drivers/net/cassini.c
@@ -5072,7 +5072,7 @@ static int __devinit cas_init_one(struct pci_dev *pdev,
 	INIT_WORK(&cp->reset_task, cas_reset_task);
 
 	/* Default link parameters */
-	if (link_mode >= 0 && link_mode <= 6)
+	if (link_mode >= 0 && link_mode < 6)
 		cp->link_cntl = link_modes[link_mode];
 	else
 		cp->link_cntl = BMCR_ANENABLE;

^ permalink raw reply related

* TCP Appropriate Byte Counting per default off
From: Alexander Zimmermann @ 2010-03-03  8:21 UTC (permalink / raw)
  To: David Miller, ilpo.jarvinen@helsinki.fi J?rvinen; +Cc: netdev

Hi David, hi Ilpo,

I wonder why ABC is off per default? What are the reasons?
Should we turn it on per default?

>From algorithmic point of view I see no flaws just
benefits.

With RFC5681 ABC is recommended.
OSX uses ABC per default.

Alex

//
// Dipl.-Inform. Alexander Zimmermann
// Department of Computer Science, Informatik 4
// RWTH Aachen University
// Ahornstr. 55, 52056 Aachen, Germany
// phone: (49-241) 80-21422, fax: (49-241) 80-22220
// email: zimmermann@cs.rwth-aachen.de
// web: http://www.umic-mesh.net
//


^ permalink raw reply

* Re: [PATCH 1/8] net: add limit for socket backlog
From: Zhu Yi @ 2010-03-03  8:14 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, David Miller, Arnaldo Carvalho de Melo,
	Pekka Savola (ipv6), Patrick McHardy, Vlad Yasevich,
	Sridhar Samudrala, Per Liden, Jon Maloy, Allan Stephens,
	Andrew Hendry
In-Reply-To: <1267603357.2839.105.camel@edumazet-laptop>

On Wed, 2010-03-03 at 16:02 +0800, Eric Dumazet wrote:
> > sk->sk_rcvbuf can be adjusted by setsockopt from user at run time.
> The
> > minimum allowed value is 256. I'm afraid this will break most
> protocols.
> 
> I see, then maybe use some offset in your test, to allow some extra
> space.
> 
> #define MINBACKLOG 2048
> 
> if (sk->sk_backlog.len >= (sk->sk_rcvbuf << 1) + MINBACKLOG)
>         return -ENOBUFS;

I want to provide a method for protocols to set its sock backlog limit.
For example, the sctp_rmem is 373500 (sysctl_sctp_rmem[1]) which is
larger than (sysctl_rmem_default << 1). But I'm not very sure about the
actual required size. So I didn't make it in the sctp patch. What do you
think?

Thanks,
-yi


^ permalink raw reply

* Re: [PATCH 1/8] net: add limit for socket backlog
From: Eric Dumazet @ 2010-03-03  8:02 UTC (permalink / raw)
  To: Zhu Yi
  Cc: netdev@vger.kernel.org, David Miller, Arnaldo Carvalho de Melo,
	Pekka Savola (ipv6), Patrick McHardy, Vlad Yasevich,
	Sridhar Samudrala, Per Liden, Jon Maloy, Allan Stephens,
	Andrew Hendry
In-Reply-To: <1267601728.2370.47.camel@debian>

Le mercredi 03 mars 2010 à 15:35 +0800, Zhu Yi a écrit :
> On Wed, 2010-03-03 at 14:54 +0800, Eric Dumazet wrote:
> > > diff --git a/include/net/sock.h b/include/net/sock.h
> > > index 6cb1676..847119a 100644
> > > --- a/include/net/sock.h
> > > +++ b/include/net/sock.h
> > > @@ -253,6 +253,8 @@ struct sock {
> > >  	struct {
> > >  		struct sk_buff *head;
> > >  		struct sk_buff *tail;
> > > +		int len;
> > > +		int limit;
> > 
> > This new limit field is really not needed
> 
> sk->sk_rcvbuf can be adjusted by setsockopt from user at run time. The
> minimum allowed value is 256. I'm afraid this will break most protocols.

I see, then maybe use some offset in your test, to allow some extra
space.

#define MINBACKLOG 2048

if (sk->sk_backlog.len >= (sk->sk_rcvbuf << 1) + MINBACKLOG)
	return -ENOBUFS;




^ permalink raw reply

* Re: [RFC][ PATCH 0/3] vhost-net: Add mergeable RX buffer support to vhost-net
From: Michael S. Tsirkin @ 2010-03-03  7:54 UTC (permalink / raw)
  To: David Stevens; +Cc: rusty, netdev, kvm, virtualization
In-Reply-To: <OF8B8E9FD7.1F227ED3-ON882576DA.008312B5-882576DB.0001D5AB@us.ibm.com>

On Tue, Mar 02, 2010 at 04:20:03PM -0800, David Stevens wrote:
> These patches add support for mergeable receive buffers to
> vhost-net, allowing it to use multiple virtio buffer heads for a single
> receive packet.
>                                         +-DLS
> 
> 
> Signed-off-by: David L Stevens <dlstevens@us.ibm.com>

Do you have performance numbers (both with and without mergeable buffers
in guest)?

-- 
MST

^ permalink raw reply

* Re: [PATCH 2/8] dccp: use limited socket backlog
From: Zhu Yi @ 2010-03-03  7:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev@vger.kernel.org, Arnaldo Carvalho de Melo
In-Reply-To: <1267599387.2839.89.camel@edumazet-laptop>

On Wed, 2010-03-03 at 14:56 +0800, Eric Dumazet wrote:
> Le mercredi 03 mars 2010 à 14:35 +0800, Zhu Yi a écrit :
> > Make dccp adapt to the limited socket backlog change.
> > 
> > Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> > Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> > ---
> >  net/dccp/minisocks.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
> > index af226a0..0d508c3 100644
> > --- a/net/dccp/minisocks.c
> > +++ b/net/dccp/minisocks.c
> > @@ -254,7 +254,7 @@ int dccp_child_process(struct sock *parent, struct sock *child,
> >  		 * in main socket hash table and lock on listening
> >  		 * socket does not protect us more.
> >  		 */
> > -		sk_add_backlog(child, skb);
> > +		__sk_add_backlog(child, skb);
> >  	}
> >  
> >  	bh_unlock_sock(child);
> 
> I dont understand this patch.
> 
> You make dccp vulnerable to memory exhaustion, I thought you wanted to
> solve this problem.
> 
> It should therefore be named "dccp: use unlimited socket backlog"
> 
> (And this sounds not so sexy :) )

dccp uses sk_receive_skb() which calls sk_add_backlog() that I fixed in
the first patch. This patch here handles a different case, when the
parent sk is in the LISTEN state and we want the child to process the
packet. Should the backlog limit be applied here? I'm waiting for
comments.

Thanks,
-yi



^ permalink raw reply

* Re: [net-2.6 PATCH] af_packet: move strict addr_len check right before dev_[mc/unicast]_[add/del]
From: Jiri Pirko @ 2010-03-03  7:36 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: netdev, davem
In-Reply-To: <20100303020110.hhu4cdecg08kkck0-cebfxv@webmail.spamcop.net>

Wed, Mar 03, 2010 at 08:01:10AM CET, proski@gnu.org wrote:
>Quoting Jiri Pirko <jpirko@redhat.com>:
>
>>@@ -1734,7 +1738,7 @@ static int packet_mc_add(struct sock *sk,
>>struct packet_mreq_max *mreq)
>> 		goto done;
>>
>> 	err = -EINVAL;
>>-	if (mreq->mr_alen != dev->addr_len)
>>+	if (mreq->mr_alen > dev->addr_len)
>> 		goto done;
>>
>> 	err = -ENOBUFS;
>
>The patch looks good, but did you mean to include this change?  It's
>not described.

Sure - this is revert of the bit from my original patch. I think it's clear from
description.

Jirka

>
>-- 
>Regards,
>Pavel Roskin

^ permalink raw reply

* Re: [PATCH 1/8] net: add limit for socket backlog
From: Zhu Yi @ 2010-03-03  7:35 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, David Miller, Arnaldo Carvalho de Melo,
	Pekka Savola (ipv6), Patrick McHardy, Vlad Yasevich,
	Sridhar Samudrala, Per Liden, Jon Maloy, Allan Stephens,
	Andrew Hendry
In-Reply-To: <1267599269.2839.84.camel@edumazet-laptop>

On Wed, 2010-03-03 at 14:54 +0800, Eric Dumazet wrote:
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 6cb1676..847119a 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -253,6 +253,8 @@ struct sock {
> >  	struct {
> >  		struct sk_buff *head;
> >  		struct sk_buff *tail;
> > +		int len;
> > +		int limit;
> 
> This new limit field is really not needed

sk->sk_rcvbuf can be adjusted by setsockopt from user at run time. The
minimum allowed value is 256. I'm afraid this will break most protocols.

<...>

> Ouch, this patch breaks bisection, since all protocols currently ignore
> -ENOBUFS value and dont free skb
> 
> If you split your work on several patches, you still have to make
> resulting kernels usable.

OK. Will fix.

<...>

> > +	sk->sk_backlog.limit	=	sk->sk_rcvbuf >> 1;

Sorry, typo.

Thanks,
-yi


^ permalink raw reply

* Re: [PATCH 8/8] x25: use limited socket backlog
From: Eric Dumazet @ 2010-03-03  7:08 UTC (permalink / raw)
  To: Zhu Yi; +Cc: netdev, Andrew Hendry
In-Reply-To: <1267598111-12503-8-git-send-email-yi.zhu@intel.com>

Le mercredi 03 mars 2010 à 14:35 +0800, Zhu Yi a écrit :
> Make x25 adapt to the limited socket backlog change.
> 
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
>  net/x25/x25_dev.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
> index 3e1efe5..5688123 100644
> --- a/net/x25/x25_dev.c
> +++ b/net/x25/x25_dev.c
> @@ -53,7 +53,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
>  		if (!sock_owned_by_user(sk)) {
>  			queued = x25_process_rx_frame(sk, skb);
>  		} else {
> -			sk_add_backlog(sk, skb);
> +			__sk_add_backlog(sk, skb);
>  		}
>  		bh_unlock_sock(sk);
>  		sock_put(sk);

Please respin your patch the other way 

Ie: let sk_add_backlog(sk, skb) do its previous job (not leaking skbs,
and returning a void status)

Add a new function able to no limit backlog, and returns an error code,
so that caller can free skb and increment SNMP counters accordingly.

Callers MUST test return value, or use another helper that can free the
skb for them.

Name it sk_move_backlog() for example

This will permit you to split the work as you tried.

sk_add_backlog() could be redefined as the helper :

void sk_add_backlog(sk, skb)
{
	if (sk_move_backlog(sk, skb)) {
		kfree_skb(skb);
		atomic_inc(&sk->sk_drops);
	}
	
}

Thanks



^ permalink raw reply

* [patch] davinci_emac: off by one
From: Dan Carpenter @ 2010-03-03  7:07 UTC (permalink / raw)
  To: netdev
  Cc: Chaithrika U S, Sriramakrishnan, David S. Miller, Kevin Hilman,
	Anant Gole, kernel-janitors

This off by one error was found by smatch.

drivers/net/davinci_emac.c +2390 emac_dev_open(13) error: buffer overflow 'priv->mac_addr' 6 <= 6

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
I don't have the hardware to test this.  If would be cool if someone
would verify that the hardware still works after the patch is applied.

diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index 1605bc2..eab07cd 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -2386,7 +2386,7 @@ static int emac_dev_open(struct net_device *ndev)
 	struct emac_priv *priv = netdev_priv(ndev);
 
 	netif_carrier_off(ndev);
-	for (cnt = 0; cnt <= ETH_ALEN; cnt++)
+	for (cnt = 0; cnt < ETH_ALEN; cnt++)
 		ndev->dev_addr[cnt] = priv->mac_addr[cnt];
 
 	/* Configuration items */

^ permalink raw reply related

* Re: [net-2.6 PATCH] af_packet: move strict addr_len check right before dev_[mc/unicast]_[add/del]
From: Pavel Roskin @ 2010-03-03  7:01 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem
In-Reply-To: <20100303064001.GB2648@psychotron.redhat.com>

Quoting Jiri Pirko <jpirko@redhat.com>:

> @@ -1734,7 +1738,7 @@ static int packet_mc_add(struct sock *sk,   
> struct packet_mreq_max *mreq)
>  		goto done;
>
>  	err = -EINVAL;
> -	if (mreq->mr_alen != dev->addr_len)
> +	if (mreq->mr_alen > dev->addr_len)
>  		goto done;
>
>  	err = -ENOBUFS;

The patch looks good, but did you mean to include this change?  It's  
not described.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: [net-2.6 PATCH] af_packet: move strict addr_len check right before dev_[mc/unicast]_[add/del]
From: Eric Dumazet @ 2010-03-03  6:57 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	tcpdump-workers-YE1jQ5a0g24KACXWX4p+q9i2O/JbrIOy,
	proski-mXXj517/zsQ
In-Reply-To: <20100303064001.GB2648-YzwxZg+R7et1/kRsl7OVgNvLeJWuRmrY@public.gmane.org>

Le mercredi 03 mars 2010 à 07:40 +0100, Jiri Pirko a écrit :
> Subject: [net-2.6 PATCH] af_packet: move strict addr_len check right before dev_[mc/unicast]_[add/del]
> 
> My previous patch 914c8ad2d18b62ad1420f518c0cab0b0b90ab308 incorrectly changed
> the length check in packet_mc_add to be more strict. The problem is that
> userspace is not filling this field (and it stays zeroed) in case of setting
> PACKET_MR_PROMISC or PACKET_MR_ALLMULTI. So move the strict check to the point
> in path where the addr_len must be set correctly.
> 
> Signed-off-by: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 

I am not sure it solves Pavel Roskin concern, but some credit should be
given to him :)

Reported-by: Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org>

Thanks


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Current wireless-testing breaks libpcap: mr_alen should be set
From: Pavel Roskin @ 2010-03-03  6:57 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev
In-Reply-To: <20100303062359.GA2648@psychotron.redhat.com>

Quoting Jiri Pirko <jpirko@redhat.com>:

> Sorry about this. Corrected patch will follow.

I guess the address length should not be checked if the address is not  
supplied, as in this case.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: [PATCH 2/8] dccp: use limited socket backlog
From: Eric Dumazet @ 2010-03-03  6:56 UTC (permalink / raw)
  To: Zhu Yi; +Cc: netdev, Arnaldo Carvalho de Melo
In-Reply-To: <1267598111-12503-2-git-send-email-yi.zhu@intel.com>

Le mercredi 03 mars 2010 à 14:35 +0800, Zhu Yi a écrit :
> Make dccp adapt to the limited socket backlog change.
> 
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
>  net/dccp/minisocks.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
> index af226a0..0d508c3 100644
> --- a/net/dccp/minisocks.c
> +++ b/net/dccp/minisocks.c
> @@ -254,7 +254,7 @@ int dccp_child_process(struct sock *parent, struct sock *child,
>  		 * in main socket hash table and lock on listening
>  		 * socket does not protect us more.
>  		 */
> -		sk_add_backlog(child, skb);
> +		__sk_add_backlog(child, skb);
>  	}
>  
>  	bh_unlock_sock(child);

I dont understand this patch.

You make dccp vulnerable to memory exhaustion, I thought you wanted to
solve this problem.

It should therefore be named "dccp: use unlimited socket backlog"

(And this sounds not so sexy :) )



^ permalink raw reply

* Re: [PATCH 1/8] net: add limit for socket backlog
From: Eric Dumazet @ 2010-03-03  6:54 UTC (permalink / raw)
  To: Zhu Yi
  Cc: netdev, David Miller, Arnaldo Carvalho de Melo,
	Pekka Savola (ipv6), Patrick McHardy, Vlad Yasevich,
	Sridhar Samudrala, Per Liden, Jon Maloy, Allan Stephens,
	Andrew Hendry
In-Reply-To: <1267598111-12503-1-git-send-email-yi.zhu@intel.com>

Le mercredi 03 mars 2010 à 14:35 +0800, Zhu Yi a écrit :
> We got system OOM while running some UDP netperf testing on the loopback
> device. The case is multiple senders sent stream UDP packets to a single
> receiver via loopback on local host. Of course, the receiver is not able
> to handle all the packets in time. But we surprisingly found that these
> packets were not discarded due to the receiver's sk->sk_rcvbuf limit.
> Instead, they are kept queuing to sk->sk_backlog and finally ate up all
> the memory. We believe this is a secure hole that a none privileged user
> can crash the system.
> 
> The root cause for this problem is, when the receiver is doing
> __release_sock() (i.e. after userspace recv, kernel udp_recvmsg ->
> skb_free_datagram_locked -> release_sock), it moves skbs from backlog to
> sk_receive_queue with the softirq enabled. In the above case, multiple
> busy senders will almost make it an endless loop. The skbs in the
> backlog end up eat all the system memory.
> 
> The issue is not only for UDP. Any protocols using socket backlog is
> potentially affected. The patch adds limit for socket backlog so that
> the backlog size cannot be expanded endlessly.
> 
> Reported-by: Alex Shi <alex.shi@intel.com>
> Cc: David Miller <davem@davemloft.net>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru
> Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
> Cc: Sridhar Samudrala <sri@us.ibm.com>
> Cc: Per Liden <per.liden@ericsson.com>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Allan Stephens <allan.stephens@windriver.com>
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>

Your SOB should be before mine, you are the main author of this patch, I
am a contributor

> ---
>  include/net/sock.h |   17 +++++++++++++++--
>  net/core/sock.c    |   15 +++++++++++++--
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 6cb1676..847119a 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -253,6 +253,8 @@ struct sock {
>  	struct {
>  		struct sk_buff *head;
>  		struct sk_buff *tail;
> +		int len;
> +		int limit;

This new limit field is really not needed

>  	} sk_backlog;
>  	wait_queue_head_t	*sk_sleep;
>  	struct dst_entry	*sk_dst_cache;
> @@ -589,8 +591,8 @@ static inline int sk_stream_memory_free(struct sock *sk)
>  	return sk->sk_wmem_queued < sk->sk_sndbuf;
>  }
>  
> -/* The per-socket spinlock must be held here. */
> -static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
> +/* OOB backlog add */
> +static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
>  {
>  	if (!sk->sk_backlog.tail) {
>  		sk->sk_backlog.head = sk->sk_backlog.tail = skb;
> @@ -601,6 +603,17 @@ static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
>  	skb->next = NULL;
>  }
>  
> +/* The per-socket spinlock must be held here. */
> +static inline int sk_add_backlog(struct sock *sk, struct sk_buff *skb)
> +{
> +	if (sk->sk_backlog.len >= max(sk->sk_backlog.limit, sk->sk_rcvbuf >> 1))
> +		return -ENOBUFS;
> +
> +	__sk_add_backlog(sk, skb);
> +	sk->sk_backlog.len += skb->truesize;
> +	return 0;
> +}
> +

Ouch, this patch breaks bisection, since all protocols currently ignore
-ENOBUFS value and dont free skb

If you split your work on several patches, you still have to make
resulting kernels usable.

>  static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
>  {
>  	return sk->sk_backlog_rcv(sk, skb);
> diff --git a/net/core/sock.c b/net/core/sock.c
> index fcd397a..fa042bc 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -340,8 +340,12 @@ int sk_receive_skb(struct sock *sk, struct sk_buff *skb, const int nested)
>  		rc = sk_backlog_rcv(sk, skb);
>  
>  		mutex_release(&sk->sk_lock.dep_map, 1, _RET_IP_);
> -	} else
> -		sk_add_backlog(sk, skb);
> +	} else if (sk_add_backlog(sk, skb)) {
> +		bh_unlock_sock(sk);
> +		atomic_inc(&sk->sk_drops);
> +		goto discard_and_relse;
> +	}
> +
>  	bh_unlock_sock(sk);
>  out:
>  	sock_put(sk);
> @@ -1139,6 +1142,7 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
>  		sock_lock_init(newsk);
>  		bh_lock_sock(newsk);
>  		newsk->sk_backlog.head	= newsk->sk_backlog.tail = NULL;
> +		newsk->sk_backlog.len = 0;
>  
>  		atomic_set(&newsk->sk_rmem_alloc, 0);
>  		/*
> @@ -1542,6 +1546,12 @@ static void __release_sock(struct sock *sk)
>  
>  		bh_lock_sock(sk);
>  	} while ((skb = sk->sk_backlog.head) != NULL);
> +
> +	/*
> +	 * Doing the zeroing here guarantee we can not loop forever
> +	 * while a wild producer attempts to flood us.
> +	 */
> +	sk->sk_backlog.len = 0;
>  }
>  
>  /**
> @@ -1874,6 +1884,7 @@ void sock_init_data(struct socket *sock, struct sock *sk)
>  	sk->sk_allocation	=	GFP_KERNEL;
>  	sk->sk_rcvbuf		=	sysctl_rmem_default;
>  	sk->sk_sndbuf		=	sysctl_wmem_default;
> +	sk->sk_backlog.limit	=	sk->sk_rcvbuf >> 1;

Didnt we agreed to use sk_>rcvbuf << 1  in previous round ?

>  	sk->sk_state		=	TCP_CLOSE;
>  	sk_set_socket(sk, sock);
>  




^ permalink raw reply

* [net-2.6 PATCH] af_packet: move strict addr_len check right before dev_[mc/unicast]_[add/del]
From: Jiri Pirko @ 2010-03-03  6:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-wireless, netdev, tcpdump-workers, proski
In-Reply-To: <1267578048.14049.11.camel@mj>

Subject: [net-2.6 PATCH] af_packet: move strict addr_len check right before dev_[mc/unicast]_[add/del]

My previous patch 914c8ad2d18b62ad1420f518c0cab0b0b90ab308 incorrectly changed
the length check in packet_mc_add to be more strict. The problem is that
userspace is not filling this field (and it stays zeroed) in case of setting
PACKET_MR_PROMISC or PACKET_MR_ALLMULTI. So move the strict check to the point
in path where the addr_len must be set correctly.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 031a5e6..1612d41 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1688,6 +1688,8 @@ static int packet_dev_mc(struct net_device *dev, struct packet_mclist *i,
 {
 	switch (i->type) {
 	case PACKET_MR_MULTICAST:
+		if (i->alen != dev->addr_len)
+			return -EINVAL;
 		if (what > 0)
 			return dev_mc_add(dev, i->addr, i->alen, 0);
 		else
@@ -1700,6 +1702,8 @@ static int packet_dev_mc(struct net_device *dev, struct packet_mclist *i,
 		return dev_set_allmulti(dev, what);
 		break;
 	case PACKET_MR_UNICAST:
+		if (i->alen != dev->addr_len)
+			return -EINVAL;
 		if (what > 0)
 			return dev_unicast_add(dev, i->addr);
 		else
@@ -1734,7 +1738,7 @@ static int packet_mc_add(struct sock *sk, struct packet_mreq_max *mreq)
 		goto done;
 
 	err = -EINVAL;
-	if (mreq->mr_alen != dev->addr_len)
+	if (mreq->mr_alen > dev->addr_len)
 		goto done;
 
 	err = -ENOBUFS;

^ permalink raw reply related

* [PATCH 7/8] tipc: use limited socket backlog
From: Zhu Yi @ 2010-03-03  6:35 UTC (permalink / raw)
  To: netdev; +Cc: Zhu Yi, Per Liden, Jon Maloy, Allan Stephens
In-Reply-To: <1267598111-12503-6-git-send-email-yi.zhu@intel.com>

Make tipc adapt to the limited socket backlog change.

Cc: Per Liden <per.liden@ericsson.com>
Cc: Jon Maloy <jon.maloy@ericsson.com>
Cc: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/tipc/socket.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 1ea64f0..4b235fc 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1322,8 +1322,10 @@ static u32 dispatch(struct tipc_port *tport, struct sk_buff *buf)
 	if (!sock_owned_by_user(sk)) {
 		res = filter_rcv(sk, buf);
 	} else {
-		sk_add_backlog(sk, buf);
-		res = TIPC_OK;
+		if (sk_add_backlog(sk, buf))
+			res = TIPC_ERR_OVERLOAD;
+		else
+			res = TIPC_OK;
 	}
 	bh_unlock_sock(sk);
 
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 8/8] x25: use limited socket backlog
From: Zhu Yi @ 2010-03-03  6:35 UTC (permalink / raw)
  To: netdev; +Cc: Zhu Yi, Andrew Hendry
In-Reply-To: <1267598111-12503-7-git-send-email-yi.zhu@intel.com>

Make x25 adapt to the limited socket backlog change.

Cc: Andrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/x25/x25_dev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index 3e1efe5..5688123 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -53,7 +53,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
 		if (!sock_owned_by_user(sk)) {
 			queued = x25_process_rx_frame(sk, skb);
 		} else {
-			sk_add_backlog(sk, skb);
+			__sk_add_backlog(sk, skb);
 		}
 		bh_unlock_sock(sk);
 		sock_put(sk);
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 6/8] sctp: use limited socket backlog
From: Zhu Yi @ 2010-03-03  6:35 UTC (permalink / raw)
  To: netdev; +Cc: Zhu Yi, Vlad Yasevich, Sridhar Samudrala
In-Reply-To: <1267598111-12503-5-git-send-email-yi.zhu@intel.com>

Make sctp adapt to the limited socket backlog change.

Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
Cc: Sridhar Samudrala <sri@us.ibm.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/sctp/input.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index c0c973e..ca42181 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -75,7 +75,7 @@ static struct sctp_association *__sctp_lookup_association(
 					const union sctp_addr *peer,
 					struct sctp_transport **pt);
 
-static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
+static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
 
 
 /* Calculate the SCTP checksum of an SCTP packet.  */
@@ -265,8 +265,12 @@ int sctp_rcv(struct sk_buff *skb)
 	}
 
 	if (sock_owned_by_user(sk)) {
+		if (sctp_add_backlog(sk, skb)) {
+			sctp_bh_unlock_sock(sk);
+			sctp_chunk_free(chunk);
+			goto discard_release;
+		}
 		SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_BACKLOG);
-		sctp_add_backlog(sk, skb);
 	} else {
 		SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_SOFTIRQ);
 		sctp_inq_push(&chunk->rcvr->inqueue, chunk);
@@ -336,7 +340,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 		sctp_bh_lock_sock(sk);
 
 		if (sock_owned_by_user(sk)) {
-			sk_add_backlog(sk, skb);
+			__sk_add_backlog(sk, skb);
 			backloged = 1;
 		} else
 			sctp_inq_push(inqueue, chunk);
@@ -362,7 +366,7 @@ done:
 	return 0;
 }
 
-static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
+static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
 {
 	struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
 	struct sctp_ep_common *rcvr = chunk->rcvr;
@@ -377,7 +381,7 @@ static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
 	else
 		BUG();
 
-	sk_add_backlog(sk, skb);
+	return sk_add_backlog(sk, skb);
 }
 
 /* Handle icmp frag needed error. */
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 5/8] llc: use limited socket backlog
From: Zhu Yi @ 2010-03-03  6:35 UTC (permalink / raw)
  To: netdev; +Cc: Zhu Yi, Arnaldo Carvalho de Melo
In-Reply-To: <1267598111-12503-4-git-send-email-yi.zhu@intel.com>

Make llc adapt to the limited socket backlog change.

Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
 net/llc/llc_c_ac.c |    2 +-
 net/llc/llc_conn.c |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
index 019c780..86d6985 100644
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -1437,7 +1437,7 @@ static void llc_process_tmr_ev(struct sock *sk, struct sk_buff *skb)
 			llc_conn_state_process(sk, skb);
 		else {
 			llc_set_backlog_type(skb, LLC_EVENT);
-			sk_add_backlog(sk, skb);
+			__sk_add_backlog(sk, skb);
 		}
 	}
 }
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index a8dde9b..a12144d 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -827,7 +827,8 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
 	else {
 		dprintk("%s: adding to backlog...\n", __func__);
 		llc_set_backlog_type(skb, LLC_PACKET);
-		sk_add_backlog(sk, skb);
+		if (sk_add_backlog(sk, skb))
+			goto drop_unlock;
 	}
 out:
 	bh_unlock_sock(sk);
-- 
1.6.3.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox