Netdev List
 help / color / mirror / Atom feed
* Re: SFQ: backport some features from ESFQ (try 4)
From: Patrick McHardy @ 2007-09-29 15:38 UTC (permalink / raw)
  To: Corey Hickey; +Cc: netdev
In-Reply-To: <1191019977201-git-send-email-bugfood-ml@fatooh.org>

Corey Hickey wrote:
> Patchset try 2 addresses the review by Michael Buesch.
> Patchset try 3 addresses the review by Patrick McHardy.
> Patchset try 4 has a few cosmetic improvements.
> 
> Nobody reviewed my last set of patches, and I wasn't pushy about asking.
> Since it's been a while, I ported the kernel and userspace patchsets to
> current git of net-2.6 and iproute2, respectively.


Thanks for posting these patches, I'll have a closer look tommorrow.

^ permalink raw reply

* Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Patrick McHardy @ 2007-09-29 15:44 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <m1hcle9td8.fsf@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
>  void rtnl_unlock(void)
>  {
> -	mutex_unlock(&rtnl_mutex);
> -	if (rtnl && rtnl->sk_receive_queue.qlen)
> +	struct net *net;
> +
> +	/*
> +	 * Loop through all of the rtnl sockets until none of them (in
> +	 * a live network namespace) have queue packets.
> +	 *
> +	 * We have to be careful with the locking here as
> +	 * sk_data_ready aka rtnetlink_rcv takes the rtnl_mutex.
> +	 *
> +	 * To ensure the network namespace does not exit while
> +	 * we are processing packets on it's rtnl socket we
> +	 * grab a reference to the network namespace, ignoring
> +	 * it if the network namespace has already exited.
> +	 */
> +retry:
> +	for_each_net(net) {
> +		struct sock *rtnl = net->rtnl;
> +
> +		if (!rtnl || !rtnl->sk_receive_queue.qlen)
> +			continue;
> +
> +		if (!maybe_get_net(net))
> +			continue;
> +
> +		mutex_unlock(&rtnl_mutex);
>  		rtnl->sk_data_ready(rtnl, 0);
> +		mutex_lock(&rtnl_mutex);
> +		put_net(net);
> +		goto retry;
> +	}
> +	mutex_unlock(&rtnl_mutex);
> +
>  	netdev_run_todo();
>  }


I'm wondering why this receive queue processing on unlock is still
necessary today, we don't do trylock in rtnetlink_rcv anymore, so
all senders will simply wait until the lock is released and then
process the queue.

^ permalink raw reply

* Re: [PATCH 5/5] net: Make AF_UNIX per network namespace safe.
From: Patrick McHardy @ 2007-09-29 15:47 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <m11wci9t3e.fsf_-_@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
> Because of the global nature of garbage collection, and because of the
> cost of per namespace hash tables unix_socket_table has been kept
> global.  With a filter added on lookups so we don't see sockets from
> the wrong namespace.
> 
> Currently I don't fold the namesapce into the hash so multiple
> namespaces using the same socket name will be guaranteed a hash
> collision.


That doesn't sound like a good thing :) Is there a reason for
not avoiding the collisions?

^ permalink raw reply

* Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Eric W. Biederman @ 2007-09-29 16:51 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <46FE72E3.3000402@trash.net>

Patrick McHardy <kaber@trash.net> writes:

> I'm wondering why this receive queue processing on unlock is still
> necessary today, we don't do trylock in rtnetlink_rcv anymore, so
> all senders will simply wait until the lock is released and then
> process the queue.

Good question, I should probably look.  I was lazy and didn't go back
and audit why we were doing this.  I just coded a routine that I was
certain would work.  It does appear that we are processing the queue
with sk_data_read when we add a message, so this may be completely
unnecessary.  I will go back and look.  If we can remove this bit
things should be simpler.

Eric


^ permalink raw reply

* Re: [PATCH 5/5] net: Make AF_UNIX per network namespace safe.
From: Eric W. Biederman @ 2007-09-29 17:03 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <46FE73AC.40807@trash.net>

Patrick McHardy <kaber@trash.net> writes:

> Eric W. Biederman wrote:
>> Because of the global nature of garbage collection, and because of the
>> cost of per namespace hash tables unix_socket_table has been kept
>> global.  With a filter added on lookups so we don't see sockets from
>> the wrong namespace.
>> 
>> Currently I don't fold the namesapce into the hash so multiple
>> namespaces using the same socket name will be guaranteed a hash
>> collision.
>
>
> That doesn't sound like a good thing :) Is there a reason for
> not avoiding the collisions?

Two reasons.  Minimizing the size of the changes to make review
easier, and I don't know if hash collisions are likely in practice
or if they matter.  I don't believe we can't physically collide and
have the same inode because we make a node in the filesystem.  The
abstract domain is local to linux and so people don't use it as much.

All of which boils down to.  I don't see it matter a heck of a lot
especially initially.  So I did the traditional unix thing and started
with a simple and stupid implementation.  But it didn't quite feel
right to me either so I documented it.

Whipping up a patch to take the namespace into account in mkname
doesn't look to hard though.

Eric

^ permalink raw reply

* Re: [PATCH] rtnl: Simplify ASSERT_RTNL
From: Eric W. Biederman @ 2007-09-29 17:18 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <E1IbTz0-0000FK-00@gondolin.me.apana.org.au>

Herbert Xu <herbert@gondor.apana.org.au> writes:

> Eric W. Biederman <ebiederm@xmission.com> wrote:
>> 
>> Currently we have the call path:
>> macvlan_open -> dev_unicast_add -> __dev_set_rx_mode ->
>>        __dev_set_promiscuity -> ASSERT_RTNL -> mutex_trylock
>> 
>> When mutex debugging is on taking a mutex complains if we are not
>> allowed to sleep.  At that point we have called netif_tx_lock_bh
>> so we are clearly not allowed to sleep.  Arguably this is not a
>> problem for mutex_trylock.
>
> Actually holding the TX lock here is a bug.  We're going to
> call down into the hardware with __dev_set_promiscuity, which
> may sleep (think USB NICs), so we definitely shouldn't be holding
> any spin locks.

Regardless of the correctness of where we have ASSERT_RTNL.
I think not actually taking the mutex on the assertion failure path
(just so we can release it), is still a good deal regardless.

For this particular call site clearly we need to look at what
is happening a little more.  The obvious thing would be to add
an explicit might_sleep if we are calling code that can sleep.

Eric

^ permalink raw reply

* Re: SFQ: backport some features from ESFQ (try 4)
From: Corey Hickey @ 2007-09-29 17:36 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev
In-Reply-To: <46FE7171.70501@trash.net>

Patrick McHardy wrote:
> Corey Hickey wrote:
>> Patchset try 2 addresses the review by Michael Buesch.
>> Patchset try 3 addresses the review by Patrick McHardy.
>> Patchset try 4 has a few cosmetic improvements.
>>
>> Nobody reviewed my last set of patches, and I wasn't pushy about asking.
>> Since it's been a while, I ported the kernel and userspace patchsets to
>> current git of net-2.6 and iproute2, respectively.
> 
> Thanks for posting these patches, I'll have a closer look tommorrow.

Many thanks for reviewing.

-Corey

^ permalink raw reply

* Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Patrick McHardy @ 2007-09-29 17:48 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <m1tzpd8lfh.fsf@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
> 
> 
>>I'm wondering why this receive queue processing on unlock is still
>>necessary today, we don't do trylock in rtnetlink_rcv anymore, so
>>all senders will simply wait until the lock is released and then
>>process the queue.
> 
> 
> Good question, I should probably look.  I was lazy and didn't go back
> and audit why we were doing this.  I just coded a routine that I was
> certain would work.  It does appear that we are processing the queue
> with sk_data_read when we add a message, so this may be completely
> unnecessary.  I will go back and look.  If we can remove this bit
> things should be simpler.


Maybe I can save you some time: we used to do down_trylock()
for the rtnl mutex, so senders would simply return if someone
else was already processing the queue *or* the rtnl was locked
for some other reason. In the first case the process already
processing the queue would also process the new messages, but
if it the rtnl was locked for some other reason (for example
during module registration) the message would sit in the
queue until the next rtnetlink sendmsg call, which is why
rtnl_unlock does queue processing. Commit 6756ae4b changed
the down_trylock to mutex_lock, so senders will now simply wait
until the mutex is released and then call netlink_run_queue
themselves. This means its not needed anymore.


^ permalink raw reply

* Re: [PATCH 5/5] net: Make AF_UNIX per network namespace safe.
From: Patrick McHardy @ 2007-09-29 17:50 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <m1abr58kvq.fsf@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
> 
>>>Currently I don't fold the namesapce into the hash so multiple
>>>namespaces using the same socket name will be guaranteed a hash
>>>collision.
>>
>>
>>That doesn't sound like a good thing :) Is there a reason for
>>not avoiding the collisions?
> 
> 
> Two reasons.  Minimizing the size of the changes to make review
> easier, and I don't know if hash collisions are likely in practice
> or if they matter.  I don't believe we can't physically collide and
> have the same inode because we make a node in the filesystem.  The
> abstract domain is local to linux and so people don't use it as much.
> 
> All of which boils down to.  I don't see it matter a heck of a lot
> especially initially.  So I did the traditional unix thing and started
> with a simple and stupid implementation.  But it didn't quite feel
> right to me either so I documented it.
> 
> Whipping up a patch to take the namespace into account in mkname
> doesn't look to hard though.


It doesn't look like it would increase patch size significantly
(about 4 more changed lines), but it could of course be done in
a follow-up patch.


^ permalink raw reply

* Re: [PATCH] rtnl: Simplify ASSERT_RTNL
From: Patrick McHardy @ 2007-09-29 17:51 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Herbert Xu, davem, netdev
In-Reply-To: <m13awx8k6d.fsf@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
> Regardless of the correctness of where we have ASSERT_RTNL.
> I think not actually taking the mutex on the assertion failure path
> (just so we can release it), is still a good deal regardless.


Agreed. I actually have an identical patch somewhere that
I wanted to submit soon.



^ permalink raw reply

* [RFC][IPv6] Export userland ND options through netlink (RDNSS support)
From: Pierre Ynard @ 2007-09-29 17:47 UTC (permalink / raw)
  To: netdev

As discussed before, this patch provides userland with a way to access
relevant options in Router Advertisements, after they are processed and
validated by the kernel. Extra options are processed in a generic way;
this patch only exports RDNSS options described in RFC5006, but support
to control which options are exported could be easily added.

Options are exported using a new message type of rtnetlink. I am totally
new to netlink, so help would be greatly appreciated to design a right
interface. For now each message only contains a single ND option, in its
raw format. Should they contains several options when possible?

Also, depending on the option, userland might need context information
contained in the rest of the RA packet. For example, RFC5006 states:

"Note:  An RDNSS address MUST be used only as long as both the RA
router lifetime and the RDNSS option lifetime have not expired."

which implies that a userland daemon processing RDNSS options needs a
way to associate the option to the router that sent it, and fetch its
lifetime. This kind of information could be included in a header in the
rtnetlink message (in this version of the patch there is none).

Regards,

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index dff3192..f69d415 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -97,6 +97,9 @@ enum {
 	RTM_SETNEIGHTBL,
 #define RTM_SETNEIGHTBL	RTM_SETNEIGHTBL
 
+	RTM_NEWNDUSEROPT = 68,
+#define RTM_NEWNDUSEROPT RTM_NEWNDUSEROPT
+
 	__RTM_MAX,
 #define RTM_MAX		(((__RTM_MAX + 3) & ~3) - 1)
 };
@@ -542,6 +545,8 @@ enum rtnetlink_groups {
 #define RTNLGRP_IPV6_PREFIX	RTNLGRP_IPV6_PREFIX
 	RTNLGRP_IPV6_RULE,
 #define RTNLGRP_IPV6_RULE	RTNLGRP_IPV6_RULE
+	RTNLGRP_ND_USEROPT,
+#define RTNLGRP_ND_USEROPT	RTNLGRP_ND_USEROPT
 	__RTNLGRP_MAX
 };
 #define RTNLGRP_MAX	(__RTNLGRP_MAX - 1)
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 475b10c..6684f7e 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -24,6 +24,7 @@ enum {
 	ND_OPT_MTU = 5,			/* RFC2461 */
 	__ND_OPT_ARRAY_MAX,
 	ND_OPT_ROUTE_INFO = 24,		/* RFC4191 */
+	ND_OPT_RDNSS = 25,		/* RFC5006 */
 	__ND_OPT_MAX
 };
 
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 74c4d8d..646cd4a 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -15,9 +15,10 @@
 /*
  *	Changes:
  *
+ *	Pierre Ynard			:	export userland ND options
+ *						through netlink (RDNSS support)
  *	Lars Fenneberg			:	fixed MTU setting on receipt
  *						of an RA.
- *
  *	Janos Farkas			:	kmalloc failure checks
  *	Alexey Kuznetsov		:	state machine reworked
  *						and moved to net/core.
@@ -78,6 +79,9 @@
 #include <net/addrconf.h>
 #include <net/icmp.h>
 
+#include <net/netlink.h>
+#include <linux/rtnetlink.h>
+
 #include <net/flow.h>
 #include <net/ip6_checksum.h>
 #include <linux/proc_fs.h>
@@ -161,6 +165,8 @@ struct ndisc_options {
 	struct nd_opt_hdr *nd_opts_ri;
 	struct nd_opt_hdr *nd_opts_ri_end;
 #endif
+	struct nd_opt_hdr *nd_useropts;
+	struct nd_opt_hdr *nd_useropts_end;
 };
 
 #define nd_opts_src_lladdr	nd_opt_array[ND_OPT_SOURCE_LL_ADDR]
@@ -225,6 +231,22 @@ static struct nd_opt_hdr *ndisc_next_option(struct nd_opt_hdr *cur,
 	return (cur <= end && cur->nd_opt_type == type ? cur : NULL);
 }
 
+static inline int ndisc_is_useropt(struct nd_opt_hdr *opt)
+{
+	return (opt->nd_opt_type == ND_OPT_RDNSS);
+}
+
+static struct nd_opt_hdr *ndisc_next_useropt(struct nd_opt_hdr *cur,
+					     struct nd_opt_hdr *end)
+{
+	if (!cur || !end || cur >= end)
+		return NULL;
+	do {
+		cur = ((void *)cur) + (cur->nd_opt_len << 3);
+	} while(cur < end && !ndisc_is_useropt(cur));
+	return (cur <= end && ndisc_is_useropt(cur) ? cur : NULL);
+}
+
 static struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len,
 						 struct ndisc_options *ndopts)
 {
@@ -267,14 +289,21 @@ static struct ndisc_options *ndisc_parse_options(u8 *opt, int opt_len,
 			break;
 #endif
 		default:
-			/*
-			 * Unknown options must be silently ignored,
-			 * to accommodate future extension to the protocol.
-			 */
-			ND_PRINTK2(KERN_NOTICE
-				   "%s(): ignored unsupported option; type=%d, len=%d\n",
-				   __FUNCTION__,
-				   nd_opt->nd_opt_type, nd_opt->nd_opt_len);
+			if (ndisc_is_useropt(nd_opt)) {
+				ndopts->nd_useropts_end = nd_opt;
+				if (!ndopts->nd_useropts)
+					ndopts->nd_useropts = nd_opt;
+			} else {
+				/*
+				 * Unknown options must be silently ignored,
+				 * to accommodate future extension to the
+				 * protocol.
+				 */
+				ND_PRINTK2(KERN_NOTICE
+					   "%s(): ignored unsupported option; type=%d, len=%d\n",
+					   __FUNCTION__,
+					   nd_opt->nd_opt_type, nd_opt->nd_opt_len);
+			}
 		}
 		opt_len -= l;
 		nd_opt = ((void *)nd_opt) + l;
@@ -984,6 +1013,33 @@ out:
 	in6_dev_put(idev);
 }
 
+static void ndisc_useropt_notify(struct nd_opt_hdr *opt)
+{
+	struct sk_buff *skb;
+	struct nlmsghdr *nlh;
+	int err = -ENOBUFS;
+
+	skb = nlmsg_new(opt->nd_opt_len << 3, GFP_ATOMIC);
+	if (skb == NULL)
+		goto errout;
+
+	nlh = nlmsg_put(skb, 0, 0, RTM_NEWNDUSEROPT, opt->nd_opt_len << 3, 0);
+	if (nlh == NULL) {
+		nlmsg_free(skb);
+		goto errout;
+	}
+
+	memcpy(nlmsg_data(nlh), opt, opt->nd_opt_len << 3);
+	nlmsg_end(skb, nlh);
+
+	err = rtnl_notify(skb, 0, RTNLGRP_ND_USEROPT, NULL, GFP_ATOMIC);
+
+	if (err < 0) {
+errout:
+		rtnl_set_sk_err(RTNLGRP_ND_USEROPT, err);
+	}
+}
+
 static void ndisc_router_discovery(struct sk_buff *skb)
 {
 	struct ra_msg *ra_msg = (struct ra_msg *)skb_transport_header(skb);
@@ -1216,6 +1272,15 @@ skip_defrtr:
 		}
 	}
 
+	if (ndopts.nd_useropts) {
+		struct nd_opt_hdr *opt;
+		for (opt = ndopts.nd_useropts;
+		     opt;
+		     opt = ndisc_next_useropt(opt, ndopts.nd_useropts_end)) {
+				ndisc_useropt_notify(opt);
+		}
+	}
+
 	if (ndopts.nd_opts_tgt_lladdr || ndopts.nd_opts_rh) {
 		ND_PRINTK2(KERN_WARNING
 			   "ICMPv6 RA: invalid RA options");

-- 
Pierre Ynard
For hire in Quebec City, Canada, during Fall 2007
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."

^ permalink raw reply related

* Re: 2.6.23-rc8-mm2 - tcp_fastretrans_alert() WARNING
From: Ilpo Järvinen @ 2007-09-29 20:49 UTC (permalink / raw)
  To: Cedric Le Goater; +Cc: Andrew Morton, LKML, Netdev, David Miller
In-Reply-To: <46FE6751.3050205@free.fr>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2047 bytes --]

On Sat, 29 Sep 2007, Cedric Le Goater wrote:

> Ilpo Järvinen wrote:
> > On Fri, 28 Sep 2007, Ilpo Järvinen wrote:
> >> On Fri, 28 Sep 2007, Cedric Le Goater wrote:
> >>
> >>> I just found that warning in my logs. It seems that it's been 
> >>> happening since rc7-mm1 at least. 
> >>>
> >>> WARNING: at /home/legoater/linux/2.6.23-rc8-mm2/net/ipv4/tcp_input.c:2314 tcp_fastretrans_alert()
> >>>
> >>> Call Trace:
> >>>  <IRQ>  [<ffffffff8040fdc3>] tcp_ack+0xcd6/0x1894
> >>> ...snip...
> >> ...Thanks for the report, I'll have look what could still break 
> >> fackets_out...
> > 
> > I think this one is now clear to me, tcp_fragment/collapse adjusts 
> > fackets_out (incorrectly) also for reno flow when there were some dupACKs 
> > that made sacked_out != 0. Could you please try if patch below proves all 
> > them to be of non-SACK origin... In case that's true, it's rather 
> > harmless, I'll send a fix on Monday or so (this would anyway be needed)... 
> > If you find out that them occur with SACK enabled flow, that would be
> > more interesting and requires more digging...
> 
> I'm trying now to reproduce this WARNING. 
> 
> It seems that the n/w behaves differently during the week ends. Probably
> taking a break. 

Thanks.

Of course there are other means too to determine if TCP flows do negotiate 
SACK enabled or not. Depending on your test case (which is fully unknown 
to me) they may or may not be usable... At least the value of tcp_sack 
sysctl on both systems or tcpdump catching SYN packets should give that 
detail. ...If you know to which hosts TCP could be connected (and active) 
to, while the WARNING triggers, it's really easy to test what is being 
negotiated as it's unlikely to change at short notice and any TCP flow to 
that host will get us the same information though the WARNING would not be 
triggered with it at this time. Obviously if at least one of the remotes 
is not known or the set ends up being mixture of reno and SACK flows, then 
we'll just have to wait and see which fish we get...


-- 
 i.

^ permalink raw reply

* Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Eric W. Biederman @ 2007-09-29 21:00 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <46FE8FD3.2030308@trash.net>

Patrick McHardy <kaber@trash.net> writes:

> Maybe I can save you some time: we used to do down_trylock()
> for the rtnl mutex, so senders would simply return if someone
> else was already processing the queue *or* the rtnl was locked
> for some other reason. In the first case the process already
> processing the queue would also process the new messages, but
> if it the rtnl was locked for some other reason (for example
> during module registration) the message would sit in the
> queue until the next rtnetlink sendmsg call, which is why
> rtnl_unlock does queue processing. Commit 6756ae4b changed
> the down_trylock to mutex_lock, so senders will now simply wait
> until the mutex is released and then call netlink_run_queue
> themselves. This means its not needed anymore.

Sounds reasonable.

I started looking through the code paths and I currently cannot
see anything that would leave a message on a kernel rtnl socket.

However I did a quick test adding a WARN_ON if there were any messages
found in the queue during rtnl_unlock and I found this code path
getting invoked from linkwatch_event.  So there is clearly something I
don't understand, and it sounds at odds just a bit from your
description.

If we can remove the extra queue processing that would be great,
as it looks like a nice way to simplify the locking and the special
cases in the code.

Eric

^ permalink raw reply

* Re: [PKT_SCHED]: Add stateless NAT
From: Herbert Xu @ 2007-09-29 23:36 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: davem, hadi, netdev, kuznet
In-Reply-To: <46FE6D73.4000002@trash.net>

On Sat, Sep 29, 2007 at 05:21:23PM +0200, Patrick McHardy wrote:
>
> Unfortunately gcc doesn't perform tail call optimization when the
> address of a parameter or local variable is passed to an extern
> function before the tail call, which NF_HOOK does for the skb.

You mean the struct sk_buff **pskb bit? I've always wondered
why netfilter had to modify the skb.  Could we perhaps make it
so that it doesn't need to do so?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PKT_SCHED]: Add stateless NAT
From: David Miller @ 2007-09-30  0:13 UTC (permalink / raw)
  To: kaber; +Cc: herbert, hadi, netdev, kuznet
In-Reply-To: <46FE6D73.4000002@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 29 Sep 2007 17:21:23 +0200

> Unfortunately gcc doesn't perform tail call optimization when the
> address of a parameter or local variable is passed to an extern
> function before the tail call, which NF_HOOK does for the skb.

To which Herbert later asked why we need the **pskb thing.

The reason is that if we have to COW or resize the SKB it
could result in a realloc of the sk_buff struct which is
why we need the **pskb thing.

But strangely I can't find any skbuff.h interfaces that
require that any longer :-)))  So this might be some artifact
that we can in fact kill off.

^ permalink raw reply

* Re: [PATCH] rtnl: Simplify ASSERT_RTNL
From: Herbert Xu @ 2007-09-30  0:24 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev, oliver, Eric W. Biederman, linux-usb-devel, davem
In-Reply-To: <46FE7019.4030705@trash.net>

On Sat, Sep 29, 2007 at 05:32:41PM +0200, Patrick McHardy wrote:
>
> For unicast addresses its not strictly necessary since they may
> only be changed under the RTNL anyway. The reason why it takes
> the tx_lock is for consistency with multicast address handling,
> which can't rely on the RTNL since IPv6 changes them from
> BH context. The idea was that the ->set_rx_mode function should
> handle both secondary unicast and multicast addresses for
> simplicity.

OK, the documentation (which predates USB NIC drivers) disagrees
with me and dev->set_multicast will always operate with the xmit
lock.

This isn't very nice for USB drivers since they have to leave
the change request hanging after dev->set_multicast returns.
I suppose the one thing that guarantees this will work is the
S in USB :)

However, at least one USB driver is buggy (kaweth) in not
allowing another set_multicast until the previous one is
done, which can lead to silent losses of set_multicast calls
since it's a void.

In any case, coming back to the original question, the RTNL
assertion is simply wrong in this case because if we're being
called from IPv6 then the RTNL won't even be held.

So I think we need to

1) Move the assert into dev_set_promiscuity.
2) Take the TX lock in dev_set_promiscuity.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

^ permalink raw reply

* Re: [PKT_SCHED]: Add stateless NAT
From: Herbert Xu @ 2007-09-30  0:26 UTC (permalink / raw)
  To: David Miller; +Cc: kaber, hadi, netdev, kuznet
In-Reply-To: <20070929.171339.59681016.davem@davemloft.net>

On Sat, Sep 29, 2007 at 05:13:39PM -0700, David Miller wrote:
> 
> The reason is that if we have to COW or resize the SKB it
> could result in a realloc of the sk_buff struct which is
> why we need the **pskb thing.
> 
> But strangely I can't find any skbuff.h interfaces that
> require that any longer :-)))  So this might be some artifact
> that we can in fact kill off.

Indeed.  The only other case I can think of is defragmentation.
But even there we should be able to squeeze it into the original
skb :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] rtnl: Simplify ASSERT_RTNL
From: Herbert Xu @ 2007-09-30  0:28 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: davem, netdev
In-Reply-To: <m13awx8k6d.fsf@ebiederm.dsl.xmission.com>

On Sat, Sep 29, 2007 at 11:18:18AM -0600, Eric W. Biederman wrote:
>
> Regardless of the correctness of where we have ASSERT_RTNL.
> I think not actually taking the mutex on the assertion failure path
> (just so we can release it), is still a good deal regardless.

Provided that you add a might_sleep call in there so that if
somebody does this under locks it'll complain then I agree.

Checking RTNL under spin locks is almost certainly the sign
of a bug.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PKT_SCHED]: Add stateless NAT
From: Herbert Xu @ 2007-09-30  0:43 UTC (permalink / raw)
  To: David Miller; +Cc: kaber, hadi, netdev, kuznet
In-Reply-To: <20070930002601.GB7502@gondor.apana.org.au>

On Sun, Sep 30, 2007 at 08:26:01AM +0800, Herbert Xu wrote:
>
> Indeed.  The only other case I can think of is defragmentation.
> But even there we should be able to squeeze it into the original
> skb :)

OK it won't be pretty but it's definitely doable.  We simply
swap the contents of that skb with the head skb that would've
otherwise been returned.

If this is the only place in netfilter where we need to modify
*pskb then I think it's worthwhile.

So the question is are there any other places that we haven't
covered which also needs to change *pskb?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: 2.6.23-rc8 network problem.  Mem leak?  ip1000a?
From: linux @ 2007-09-30  7:59 UTC (permalink / raw)
  To: akpm, linux; +Cc: linux-kernel, netdev
In-Reply-To: <20070928022051.a6f27b1e.akpm@linux-foundation.org>

> ntpd.  Sounds like pps leaking to me.

That's what I'd think, except that pps does no allocation in the normal
running state, so there's nothing to leak.  The interrupt path just
records the time in some preallocated, static buffers and wakes up
blocked readers.  The read path copies the latest data out of those
static buffers.  There's allocation when the PPS device is created,
and more when it's opened.

>> Can anyone offer some diagnosis advice?

> CONFIG_DEBUG_SLAB_LEAK?

Ah, thanks you; I've been using SLUB which doesn't support this option.
Here's what I've extracted.  I've only presented the top few
slab_allocators and a small subset of the oom-killer messages, but I
have full copies if desired.  Unfortunately, I've discovered that the
machine doesn't live in this unhappy state forever.  Indeed, I'm not
sure if killing ntpd "fixes" anything; my previous observations
may have been optimistic ignorance.

(For my own personal reference looking for more oom-kill, I nuked ntpd
at 06:46:56.  And the oom-kills are continuing, with the latest at
07:43:52.)

Anyway, I have a bunch of information from the slab_allocators file, but
I'm not quire sure how to make sense of it.


With a machine in the unhappy state and firing the OOM killer, the top
20 slab_allocators are:
$ sort -rnk2 /proc/slab_allocators | head -20
skbuff_head_cache: 1712746 __alloc_skb+0x31/0x121
size-512: 1706572 tcp_send_ack+0x23/0x102
skbuff_fclone_cache: 149113 __alloc_skb+0x31/0x121
size-2048: 148500 tcp_sendmsg+0x1b5/0xae1
sysfs_dir_cache: 5289 sysfs_new_dirent+0x4b/0xec
size-512: 2613 sock_alloc_send_skb+0x93/0x1dd
Acpi-Operand: 2014 acpi_ut_allocate_object_desc_dbg+0x34/0x6e
size-32: 1995 sysfs_new_dirent+0x29/0xec
vm_area_struct: 1679 mmap_region+0x18f/0x421
size-512: 1618 tcp_xmit_probe_skb+0x1f/0xcd
size-512: 1571 arp_create+0x4e/0x1cd
vm_area_struct: 1544 copy_process+0x9f1/0x1108
anon_vma: 1448 anon_vma_prepare+0x29/0x74
filp: 1201 get_empty_filp+0x44/0xcd
UDP: 1173 sk_alloc+0x25/0xaf
size-128: 1048 r1bio_pool_alloc+0x23/0x3b
size-128: 1024 nfsd_cache_init+0x2d/0xcf
Acpi-Namespace: 973 acpi_ns_create_node+0x2c/0x45
vm_area_struct: 717 split_vma+0x33/0xe5
dentry: 594 d_alloc+0x24/0x177

I'm not sure quite what "normal" numbers are, but I do wonder why there
are 1.7 million TCP acks buffered in the system.  Shouldn't they be
transmitted and deallocated pretty quickly?

This machine receives more data than it sends, so I'd expect acks to
outnumber "real" packets.  Could the ip1000a driver's transmit path be
leaking skbs somehow?  that would also explain the "flailing" of the
oom-killer; it can't associate the allocations with a process.

Here's /proc/meminfo:
MemTotal:      1035756 kB
MemFree:         43508 kB
Buffers:         72920 kB
Cached:         224056 kB
SwapCached:     344916 kB
Active:         664976 kB
Inactive:       267656 kB
SwapTotal:     4950368 kB
SwapFree:      3729384 kB
Dirty:            6460 kB
Writeback:           0 kB
AnonPages:      491708 kB
Mapped:          79232 kB
Slab:            41324 kB
SReclaimable:    25008 kB
SUnreclaim:      16316 kB
PageTables:       8132 kB
NFS_Unstable:        0 kB
Bounce:              0 kB
CommitLimit:   5468244 kB
Committed_AS:  1946008 kB
VmallocTotal:   253900 kB
VmallocUsed:      2672 kB
VmallocChunk:   251228 kB

I have a lot of oom-killer messages, that I have saved but am not
posting for size reasons, but here are some example backtraces.
They're not very helpful to me; do they enlighten anyone else?

02:50:20: apcupsd invoked oom-killer: gfp_mask=0xd0, order=1, oomkilladj=0
02:50:22: 
02:50:22: Call Trace:
02:50:22:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
02:50:22:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
02:50:22:  [<ffffffff8025cbd6>] cache_alloc_refill+0x2f4/0x60a
02:50:22:  [<ffffffff8040602c>] hiddev_ioctl+0x579/0x919
02:50:22:  [<ffffffff8025d0fc>] kmem_cache_alloc+0x57/0x95
02:50:22:  [<ffffffff8040602c>] hiddev_ioctl+0x579/0x919
02:50:22:  [<ffffffff80262511>] cp_new_stat+0xe5/0xfd
02:50:22:  [<ffffffff804058ff>] hiddev_read+0x199/0x1f6
02:50:22:  [<ffffffff80222fa0>] default_wake_function+0x0/0xe
02:50:22:  [<ffffffff80269bb5>] do_ioctl+0x45/0x50
02:50:22:  [<ffffffff80269db9>] vfs_ioctl+0x1f9/0x20b
02:50:22:  [<ffffffff80269e07>] sys_ioctl+0x3c/0x5d
02:50:22:  [<ffffffff8020b43e>] system_call+0x7e/0x83

02:52:18: postgres invoked oom-killer: gfp_mask=0xd0, order=1, oomkilladj=0
02:52:18: 
02:52:18: Call Trace:
02:52:18:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
02:52:18:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
02:52:18:  [<ffffffff8025be8a>] poison_obj+0x26/0x2f
02:52:18:  [<ffffffff8024761f>] __get_free_pages+0x40/0x79
02:52:18:  [<ffffffff80224d66>] copy_process+0xb0/0x1108
02:52:18:  [<ffffffff80233388>] alloc_pid+0x1f/0x27d
02:52:18:  [<ffffffff80225ed6>] do_fork+0xb1/0x1a7
02:52:18:  [<ffffffff802f0627>] copy_user_generic_string+0x17/0x40
02:52:18:  [<ffffffff8020b43e>] system_call+0x7e/0x83
02:52:18:  [<ffffffff8020b757>] ptregscall_common+0x67/0xb0

02:52:18: kthreadd invoked oom-killer: gfp_mask=0xd0, order=1, oomkilladj=0
02:52:18: 
02:52:18: Call Trace:
02:52:18:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
02:52:18:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
02:52:18:  [<ffffffff8024761f>] __get_free_pages+0x40/0x79
02:52:18:  [<ffffffff80224d66>] copy_process+0xb0/0x1108
02:52:18:  [<ffffffff80233388>] alloc_pid+0x1f/0x27d
02:52:18:  [<ffffffff80225ed6>] do_fork+0xb1/0x1a7
02:52:18:  [<ffffffff80222bb8>] update_curr+0xe6/0x10b
02:52:18:  [<ffffffff8022334d>] dequeue_entity+0x73/0x97
02:52:18:  [<ffffffff8020bd21>] kernel_thread+0x81/0xde
02:52:18:  [<ffffffff802e9c81>] cfq_may_queue+0x0/0xd2
02:52:18:  [<ffffffff802355db>] kthread+0x0/0x75
02:52:18:  [<ffffffff8020bd7e>] child_rip+0x0/0x12
02:52:18:  [<ffffffff802354ad>] kthreadd+0xb4/0xf5
02:52:18:  [<ffffffff8020bd88>] child_rip+0xa/0x12
02:52:18:  [<ffffffff802353f9>] kthreadd+0x0/0xf5
02:52:18:  [<ffffffff8020bd7e>] child_rip+0x0/0x12

02:54:53: apache2 invoked oom-killer: gfp_mask=0xd0, order=1, oomkilladj=0
02:54:53: 
02:54:53: Call Trace:
02:54:53:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
02:54:53:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
02:54:53:  [<ffffffff8025be8a>] poison_obj+0x26/0x2f
02:54:53:  [<ffffffff8024761f>] __get_free_pages+0x40/0x79
02:54:53:  [<ffffffff80224d66>] copy_process+0xb0/0x1108
02:54:53:  [<ffffffff80233388>] alloc_pid+0x1f/0x27d
02:54:53:  [<ffffffff80225ed6>] do_fork+0xb1/0x1a7
02:54:53:  [<ffffffff8020b43e>] system_call+0x7e/0x83
02:54:53:  [<ffffffff8020b757>] ptregscall_common+0x67/0xb0

02:55:45: ssh invoked oom-killer: gfp_mask=0x4d0, order=2, oomkilladj=0
02:55:45: 
02:55:45: Call Trace:
02:55:45:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
02:55:45:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
02:55:45:  [<ffffffff8025cbd6>] cache_alloc_refill+0x2f4/0x60a
02:55:45:  [<ffffffff8025be8a>] poison_obj+0x26/0x2f
02:55:45:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
02:55:45:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
02:55:45:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
02:55:45:  [<ffffffff8025d067>] __kmalloc_track_caller+0x9d/0xdb
02:55:45:  [<ffffffff8040e109>] __alloc_skb+0x5b/0x121
02:55:45:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
02:55:45:  [<ffffffff802f0627>] copy_user_generic_string+0x17/0x40
02:55:45:  [<ffffffff8048155c>] unix_stream_sendmsg+0x151/0x2ea
02:55:45:  [<ffffffff80408349>] sock_aio_write+0xe5/0xf0
02:55:45:  [<ffffffff802437d3>] find_get_page+0xe/0x36
02:55:45:  [<ffffffff8025f9b4>] do_sync_write+0xd1/0x118
02:55:45:  [<ffffffff802357f5>] autoremove_wake_function+0x0/0x2e
02:55:45:  [<ffffffff80222bb8>] update_curr+0xe6/0x10b
02:55:45:  [<ffffffff80260118>] vfs_write+0xc0/0x136
02:55:45:  [<ffffffff802605c2>] sys_write+0x45/0x6e
02:55:45:  [<ffffffff8020b43e>] system_call+0x7e/0x83

03:01:34: smbclient invoked oom-killer: gfp_mask=0x4d0, order=2, oomkilladj=0
03:01:34: 
03:01:34: Call Trace:
03:01:34:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
03:01:34:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
03:01:34:  [<ffffffff8025cbd6>] cache_alloc_refill+0x2f4/0x60a
03:01:34:  [<ffffffff8025be8a>] poison_obj+0x26/0x2f
03:01:34:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
03:01:34:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
03:01:34:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
03:01:34:  [<ffffffff8025d067>] __kmalloc_track_caller+0x9d/0xdb
03:01:34:  [<ffffffff8040e109>] __alloc_skb+0x5b/0x121
03:01:34:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
03:01:34:  [<ffffffff8040a8ab>] release_sock+0xe/0x7f
03:01:34:  [<ffffffff8048155c>] unix_stream_sendmsg+0x151/0x2ea
03:01:34:  [<ffffffff80408349>] sock_aio_write+0xe5/0xf0
03:01:34:  [<ffffffff8025f9b4>] do_sync_write+0xd1/0x118
03:01:34:  [<ffffffff802357f5>] autoremove_wake_function+0x0/0x2e
03:01:34:  [<ffffffff80222bb8>] update_curr+0xe6/0x10b
03:01:34:  [<ffffffff80260118>] vfs_write+0xc0/0x136
03:01:34:  [<ffffffff802605c2>] sys_write+0x45/0x6e
03:01:34:  [<ffffffff8020b43e>] system_call+0x7e/0x83

05:48:04: scp invoked oom-killer: gfp_mask=0x4d0, order=1, oomkilladj=0
05:48:04: 
05:48:04: Call Trace:
05:48:04:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
05:48:04:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
05:48:04:  [<ffffffff8025cbd6>] cache_alloc_refill+0x2f4/0x60a
05:48:04:  [<ffffffff8025be8a>] poison_obj+0x26/0x2f
05:48:04:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
05:48:04:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
05:48:04:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
05:48:04:  [<ffffffff8025d067>] __kmalloc_track_caller+0x9d/0xdb
05:48:04:  [<ffffffff8040e109>] __alloc_skb+0x5b/0x121
05:48:04:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
05:48:04:  [<ffffffff8048155c>] unix_stream_sendmsg+0x151/0x2ea
05:48:04:  [<ffffffff80243645>] file_read_actor+0x0/0x118
05:48:04:  [<ffffffff80408349>] sock_aio_write+0xe5/0xf0
05:48:04:  [<ffffffff8025f9b4>] do_sync_write+0xd1/0x118
05:48:04:  [<ffffffff802357f5>] autoremove_wake_function+0x0/0x2e
05:48:04:  [<ffffffff80260118>] vfs_write+0xc0/0x136
05:48:04:  [<ffffffff802605c2>] sys_write+0x45/0x6e
05:48:04:  [<ffffffff8020b43e>] system_call+0x7e/0x83

And here's the latest one, in full:

05:48:11: smbclient invoked oom-killer: gfp_mask=0x4d0, order=2, oomkilladj=0
05:48:12: 
05:48:12: Call Trace:
05:48:12:  [<ffffffff80246053>] out_of_memory+0x71/0x1ba
05:48:12:  [<ffffffff8024755d>] __alloc_pages+0x255/0x2d7
05:48:12:  [<ffffffff8025cbd6>] cache_alloc_refill+0x2f4/0x60a
05:48:12:  [<ffffffff8025be8a>] poison_obj+0x26/0x2f
05:48:12:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
05:48:12:  [<ffffffff8040e0df>] __alloc_skb+0x31/0x121
05:48:12:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
05:48:12:  [<ffffffff8025d067>] __kmalloc_track_caller+0x9d/0xdb
05:48:12:  [<ffffffff8040e109>] __alloc_skb+0x5b/0x121
05:48:12:  [<ffffffff8040ab8b>] sock_alloc_send_skb+0x93/0x1dd
05:48:12:  [<ffffffff802f0627>] copy_user_generic_string+0x17/0x40
05:48:12:  [<ffffffff8048155c>] unix_stream_sendmsg+0x151/0x2ea
05:48:12:  [<ffffffff80408349>] sock_aio_write+0xe5/0xf0
05:48:12:  [<ffffffff8025f9b4>] do_sync_write+0xd1/0x118
05:48:12:  [<ffffffff802357f5>] autoremove_wake_function+0x0/0x2e
05:48:12:  [<ffffffff80222bb8>] update_curr+0xe6/0x10b
05:48:12:  [<ffffffff80260118>] vfs_write+0xc0/0x136
05:48:12:  [<ffffffff802605c2>] sys_write+0x45/0x6e
05:48:12:  [<ffffffff8020b43e>] system_call+0x7e/0x83
05:48:12: 
05:48:12: Mem-info:
05:48:12: DMA per-cpu:
05:48:12: CPU    0: Hot: hi:    0, btch:   1 usd:   0   Cold: hi:    0, btch:   1 usd:   0
05:48:12: DMA32 per-cpu:
05:48:12: CPU    0: Hot: hi:  186, btch:  31 usd:  10   Cold: hi:   62, btch:  15 usd:  58
05:48:12: Active:67 inactive:1197 dirty:0 writeback:779 unstable:0
05:48:12:  free:39163 slab:464538 mapped:518 pagetables:1800 bounce:0
05:48:12: DMA free:8040kB min:28kB low:32kB high:40kB active:0kB inactive:0kB present:11132kB pages_scanned:0 all_unreclaimable? yes
05:48:12: lowmem_reserve[]: 0 2003 2003 2003
05:48:12: DMA32 free:148612kB min:5712kB low:7140kB high:8568kB active:268kB inactive:4788kB present:2051184kB pages_scanned:7841 all_unreclaimable? yes
05:48:12: lowmem_reserve[]: 0 0 0 0
05:48:12: DMA: 56*4kB 1*8kB 0*16kB 0*32kB 0*64kB 1*128kB 0*256kB 1*512kB 1*1024kB 1*2048kB 1*4096kB = 8040kB
05:48:12: DMA32: 36459*4kB 55*8kB 2*16kB 0*32kB 0*64kB 0*128kB 1*256kB 0*512kB 0*1024kB 1*2048kB 0*4096kB = 148612kB
05:48:12: Swap cache: add 6853288, delete 6852498, find 1146430/2641691, race 0+0
05:48:12: Free swap  = 5213316kB
05:48:12: Total swap = 5855208kB
05:48:12: Free swap:       5213316kB
05:48:12: 524000 pages of RAM
05:48:12: 11264 reserved pages
05:48:12: 241 pages shared
05:48:12: 790 pages swap cached
05:48:12: Out of memory: kill process 9156 (apache2) score 83536 or a child
05:48:12: Killed process 9156 (apache2)

Oh, FWIW, a later snapshot of /proc/alab_allocators:

skbuff_head_cache: 1746940 __alloc_skb+0x31/0x121
size-512: 1740532 tcp_send_ack+0x23/0x102
skbuff_fclone_cache: 152230 __alloc_skb+0x31/0x121
size-2048: 151603 tcp_sendmsg+0x1b5/0xae1
sysfs_dir_cache: 5279 sysfs_new_dirent+0x4b/0xec
size-512: 2837 sock_alloc_send_skb+0x93/0x1dd
Acpi-Operand: 2014 acpi_ut_allocate_object_desc_dbg+0x34/0x6e
size-32: 1989 sysfs_new_dirent+0x29/0xec
size-512: 1678 arp_create+0x4e/0x1cd
size-512: 1619 tcp_xmit_probe_skb+0x1f/0xcd
UDP: 1217 sk_alloc+0x25/0xaf
size-128: 1024 r1bio_pool_alloc+0x23/0x3b
size-128: 1024 nfsd_cache_init+0x2d/0xcf
Acpi-Namespace: 973 acpi_ns_create_node+0x2c/0x45
vm_area_struct: 804 copy_process+0x9f1/0x1108
dentry: 488 d_alloc+0x24/0x177
size-2048: 480 tcp_fragment+0xdf/0x4aa
anon_vma: 463 anon_vma_prepare+0x29/0x74
filp: 442 get_empty_filp+0x44/0xcd
ip_dst_cache: 421 dst_alloc+0x29/0x76


I'm backing out the ip1000a driver and seeing what happens.

^ permalink raw reply

* Re: 2.6.23-rc8 network problem.  Mem leak?  ip1000a?
From: Andrew Morton @ 2007-09-30  9:23 UTC (permalink / raw)
  To: linux; +Cc: linux-kernel, netdev, Francois Romieu
In-Reply-To: <20070930075956.11935.qmail@science.horizon.com>

On 30 Sep 2007 03:59:56 -0400 linux@horizon.com wrote:

> > ntpd.  Sounds like pps leaking to me.
> 
> That's what I'd think, except that pps does no allocation in the normal
> running state, so there's nothing to leak.  The interrupt path just
> records the time in some preallocated, static buffers and wakes up
> blocked readers.  The read path copies the latest data out of those
> static buffers.  There's allocation when the PPS device is created,
> and more when it's opened.

OK.  Did you try to reproduce it without the pps patch applied?

> >> Can anyone offer some diagnosis advice?
> 
> > CONFIG_DEBUG_SLAB_LEAK?
> 
> Ah, thanks you; I've been using SLUB which doesn't support this option.
> Here's what I've extracted.  I've only presented the top few
> slab_allocators and a small subset of the oom-killer messages, but I
> have full copies if desired.  Unfortunately, I've discovered that the
> machine doesn't live in this unhappy state forever.  Indeed, I'm not
> sure if killing ntpd "fixes" anything; my previous observations
> may have been optimistic ignorance.
> 
> (For my own personal reference looking for more oom-kill, I nuked ntpd
> at 06:46:56.  And the oom-kills are continuing, with the latest at
> 07:43:52.)
> 
> Anyway, I have a bunch of information from the slab_allocators file, but
> I'm not quire sure how to make sense of it.
> 
> 
> With a machine in the unhappy state and firing the OOM killer, the top
> 20 slab_allocators are:
> $ sort -rnk2 /proc/slab_allocators | head -20
> skbuff_head_cache: 1712746 __alloc_skb+0x31/0x121
> size-512: 1706572 tcp_send_ack+0x23/0x102
> skbuff_fclone_cache: 149113 __alloc_skb+0x31/0x121
> size-2048: 148500 tcp_sendmsg+0x1b5/0xae1
> sysfs_dir_cache: 5289 sysfs_new_dirent+0x4b/0xec
> size-512: 2613 sock_alloc_send_skb+0x93/0x1dd
> Acpi-Operand: 2014 acpi_ut_allocate_object_desc_dbg+0x34/0x6e
> size-32: 1995 sysfs_new_dirent+0x29/0xec
> vm_area_struct: 1679 mmap_region+0x18f/0x421
> size-512: 1618 tcp_xmit_probe_skb+0x1f/0xcd
> size-512: 1571 arp_create+0x4e/0x1cd
> vm_area_struct: 1544 copy_process+0x9f1/0x1108
> anon_vma: 1448 anon_vma_prepare+0x29/0x74
> filp: 1201 get_empty_filp+0x44/0xcd
> UDP: 1173 sk_alloc+0x25/0xaf
> size-128: 1048 r1bio_pool_alloc+0x23/0x3b
> size-128: 1024 nfsd_cache_init+0x2d/0xcf
> Acpi-Namespace: 973 acpi_ns_create_node+0x2c/0x45
> vm_area_struct: 717 split_vma+0x33/0xe5
> dentry: 594 d_alloc+0x24/0x177
> 
> I'm not sure quite what "normal" numbers are, but I do wonder why there
> are 1.7 million TCP acks buffered in the system.  Shouldn't they be
> transmitted and deallocated pretty quickly?

Yeah, that's an skbuff leak.

> This machine receives more data than it sends, so I'd expect acks to
> outnumber "real" packets.  Could the ip1000a driver's transmit path be
> leaking skbs somehow?

Absolutely.  Normally a driver's transmit completion interrupt handler will
run dev_kfree_skb_irq() against the skbs which have been fully sent.

However it'd be darned odd if the driver was leaking only tcp acks.

I can find no occurrence of "dev_kfree_skb" in drivers/net/ipg.c, which is
suspicious.

Where did you get your ipg.c from, btw?  davem's tree?  rc8-mm1? rc8-mm2??

>  that would also explain the "flailing" of the
> oom-killer; it can't associate the allocations with a process.
> 
> Here's /proc/meminfo:
> MemTotal:      1035756 kB
> MemFree:         43508 kB
> Buffers:         72920 kB
> Cached:         224056 kB
> SwapCached:     344916 kB
> Active:         664976 kB
> Inactive:       267656 kB
> SwapTotal:     4950368 kB
> SwapFree:      3729384 kB
> Dirty:            6460 kB
> Writeback:           0 kB
> AnonPages:      491708 kB
> Mapped:          79232 kB
> Slab:            41324 kB
> SReclaimable:    25008 kB
> SUnreclaim:      16316 kB
> PageTables:       8132 kB
> NFS_Unstable:        0 kB
> Bounce:              0 kB
> CommitLimit:   5468244 kB
> Committed_AS:  1946008 kB
> VmallocTotal:   253900 kB
> VmallocUsed:      2672 kB
> VmallocChunk:   251228 kB

I assume that meminfo was not captured when the system was ooming?  There
isn't much slab there.


^ permalink raw reply

* Re: 2.6.23-rc8 network problem.  Mem leak?  ip1000a?
From: linux @ 2007-09-30 11:40 UTC (permalink / raw)
  To: akpm, linux; +Cc: jesse, linux-kernel, netdev, romieu, s.l-h
In-Reply-To: <20070930022347.37514be3.akpm@linux-foundation.org>

> OK.  Did you try to reproduce it without the pps patch applied?

No.  But I've yanked the ip1000a driver (using old crufy vendor-supplied
out-of-kernel module) and the problems are GONE.

>> This machine receives more data than it sends, so I'd expect acks to
>> outnumber "real" packets.  Could the ip1000a driver's transmit path be
>> leaking skbs somehow?

> Absolutely.  Normally a driver's transmit completion interrupt handler will
> run dev_kfree_skb_irq() against the skbs which have been fully sent.
>
> However it'd be darned odd if the driver was leaking only tcp acks.

It's leaking lots of things... you can see ARP packets in there and
all sorts of stuff.  But the big traffic hog is BackupPC doing inbound
rsyncs all night long, which generates a lot of acks.  Those are the
packets it sends, so those are the packets that get leaked.

> I can find no occurrence of "dev_kfree_skb" in drivers/net/ipg.c, which is
> suspicious.

Look for "IPG_DEV_KFREE_SKB", which is a wrapper macro.  (Or just add
"-i" to your grep.)  It should probably be deleted (it just expands to
dev_kfree_skb), but was presumably useful to someone during development.

> Where did you get your ipg.c from, btw?  davem's tree?  rc8-mm1? rc8-mm2??

As I wrote originally, I got it from
http://marc.info/?l=linux-netdev&m=118980588419882
which was a reuqest for mainline submission.

If there are other patches floating around, I'm happy to try them.
Now that I know what to look for, it's easy to spot the leak before OOM.

> I assume that meminfo was not captured when the system was ooming?  There
> isn't much slab there.

Oops, sorry.  I captured slabinfo but not meminfo.


Thank you very much!  Sorry to jump the gun and post a lot before I had
all the data, but if it WAS a problem in -rc8, I wanted to mention it
before -final.

Now, the rush is to get the ip1000a driver fixed before the merge
window opens.  I've added all the ip1000a developers to the Cc: list in
an attempt to speed that up.

^ permalink raw reply

* Re: [Devel] Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Denis V. Lunev @ 2007-09-30 13:13 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Patrick McHardy, Linux Containers, netdev, David Miller
In-Reply-To: <m1myv56vb3.fsf@ebiederm.dsl.xmission.com>

Hmm, so it looks like we do not need this queue processing at all...

Regards,
	Den

Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
> 
>> Maybe I can save you some time: we used to do down_trylock()
>> for the rtnl mutex, so senders would simply return if someone
>> else was already processing the queue *or* the rtnl was locked
>> for some other reason. In the first case the process already
>> processing the queue would also process the new messages, but
>> if it the rtnl was locked for some other reason (for example
>> during module registration) the message would sit in the
>> queue until the next rtnetlink sendmsg call, which is why
>> rtnl_unlock does queue processing. Commit 6756ae4b changed
>> the down_trylock to mutex_lock, so senders will now simply wait
>> until the mutex is released and then call netlink_run_queue
>> themselves. This means its not needed anymore.
> 
> Sounds reasonable.
> 
> I started looking through the code paths and I currently cannot
> see anything that would leave a message on a kernel rtnl socket.
> 
> However I did a quick test adding a WARN_ON if there were any messages
> found in the queue during rtnl_unlock and I found this code path
> getting invoked from linkwatch_event.  So there is clearly something I
> don't understand, and it sounds at odds just a bit from your
> description.
> 
> If we can remove the extra queue processing that would be great,
> as it looks like a nice way to simplify the locking and the special
> cases in the code.
> 
> Eric


^ permalink raw reply

* Re: [PKT_SCHED]: Add stateless NAT
From: Patrick McHardy @ 2007-09-30 15:38 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Miller, hadi, netdev, kuznet
In-Reply-To: <20070930004341.GA7769@gondor.apana.org.au>

Herbert Xu wrote:
> On Sun, Sep 30, 2007 at 08:26:01AM +0800, Herbert Xu wrote:
> 
>>Indeed.  The only other case I can think of is defragmentation.
>>But even there we should be able to squeeze it into the original
>>skb :)
> 
> 
> OK it won't be pretty but it's definitely doable.  We simply
> swap the contents of that skb with the head skb that would've
> otherwise been returned.
> 
> If this is the only place in netfilter where we need to modify
> *pskb then I think it's worthwhile.
> 
> So the question is are there any other places that we haven't
> covered which also needs to change *pskb?


>From a quick look it seems at least IPv4 and IPv6 don't need to
change the skb anywhere else. It seems to be necessary for
bridging though to unshare the skb. OTOH br_netfilter.c already
unshares the skb for IPv4 and IPv6, so we could simply do this
unconditionally before calling the hooks.


^ permalink raw reply

* Re: [PATCH 2/5] net: Make rtnetlink infrastructure network namespace aware
From: Patrick McHardy @ 2007-09-30 15:39 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, netdev, Linux Containers
In-Reply-To: <m1myv56vb3.fsf@ebiederm.dsl.xmission.com>

Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
> 
> 
>>Maybe I can save you some time: we used to do down_trylock()
>>for the rtnl mutex, so senders would simply return if someone
>>else was already processing the queue *or* the rtnl was locked
>>for some other reason. In the first case the process already
>>processing the queue would also process the new messages, but
>>if it the rtnl was locked for some other reason (for example
>>during module registration) the message would sit in the
>>queue until the next rtnetlink sendmsg call, which is why
>>rtnl_unlock does queue processing. Commit 6756ae4b changed
>>the down_trylock to mutex_lock, so senders will now simply wait
>>until the mutex is released and then call netlink_run_queue
>>themselves. This means its not needed anymore.
> 
> 
> Sounds reasonable.
> 
> I started looking through the code paths and I currently cannot
> see anything that would leave a message on a kernel rtnl socket.
> 
> However I did a quick test adding a WARN_ON if there were any messages
> found in the queue during rtnl_unlock and I found this code path
> getting invoked from linkwatch_event.  So there is clearly something I
> don't understand, and it sounds at odds just a bit from your
> description.


That sounds like a bug. Did you place the WARN_ON before or after
the mutex_unlock()?

^ permalink raw reply


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