Netdev List
 help / color / mirror / Atom feed
* Re: 2.6.24-rc4-mm1
From: Ilpo Järvinen @ 2007-12-07 13:16 UTC (permalink / raw)
  To: reuben-linuxkernel; +Cc: akpm, LKML, Netdev, auke-jan.h.kok, David Miller
In-Reply-To: <20071205.230925.99250129.davem@davemloft.net>

On Wed, 5 Dec 2007, David Miller wrote:

> From: Reuben Farrelly <reuben-linuxkernel@reub.net>
> Date: Thu, 06 Dec 2007 17:59:37 +1100
> 
> > On 5/12/2007 4:17 PM, Andrew Morton wrote:
> > > - Lots of device IDs have been removed from the e1000 driver and moved over
> > >   to e1000e.  So if your e1000 stops working, you forgot to set CONFIG_E1000E.
> > 
> > This non fatal oops which I have just noticed may be related to this change then 
> > - certainly looks networking related.
> > 
> > WARNING: at net/ipv4/tcp_input.c:2518 tcp_fastretrans_alert()
> > Pid: 0, comm: swapper Not tainted 2.6.24-rc4-mm1 #1
> > 
> > Call Trace:
> >   <IRQ>  [<ffffffff8046e038>] tcp_fastretrans_alert+0x229/0xe63
> >   [<ffffffff80470975>] tcp_ack+0xa3f/0x127d
> >   [<ffffffff804747b7>] tcp_rcv_established+0x55f/0x7f8
> >   [<ffffffff8047b1aa>] tcp_v4_do_rcv+0xdb/0x3a7
> >   [<ffffffff881148a8>] :nf_conntrack:nf_ct_deliver_cached_events+0x75/0x99
> 
> No, it's from TCP assertions and changes added by Ilpo to the
> net-2.6.25 tree recently.

Yeah, this (very likely) due to the new SACK processing (in net-2.6.25). 
I'll look what could go wrong with fack_count calculations, most likely 
it's the reason (I've found earlier one out-of-place retransmission 
segment in one of my test case which already indicated that there's 
something incorrect with them but didn't have time to debug it yet).

Thanks for report. Some info about how easily you can reproduce & 
couple of sentences about the test case might be useful later on when 
evaluating the fix.

-- 
 i.

^ permalink raw reply

* [PATCH net-2.6.25 3/3]sysctl: make sysctl_somaxconn per-namespace
From: Pavel Emelyanov @ 2007-12-07 13:12 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, Linux Containers, devel

Just move the variable on the struct net and adjust
its usage.

Others sysctls from sys.net.core table are more
difficult to virtualize (i.e. make them per-namespace),
but I'll look at them as well a bit later.

Signed-off-by: Pavel Emelyanov <xemul@oenvz.org>

---

diff --git a/include/linux/socket.h b/include/linux/socket.h
index eb5bdd5..bd2b30a 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -24,7 +24,6 @@ struct __kernel_sockaddr_storage {
 #include <linux/types.h>		/* pid_t			*/
 #include <linux/compiler.h>		/* __user			*/
 
-extern int sysctl_somaxconn;
 #ifdef CONFIG_PROC_FS
 struct seq_file;
 extern void socket_seq_show(struct seq_file *seq);
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index d593611..b62e31f 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -39,6 +39,7 @@ struct net {
 
 	/* core sysctls */
 	struct ctl_table_header	*sysctl_core_hdr;
+	int			sysctl_somaxconn;
 
 	/* List of all packet sockets. */
 	rwlock_t		packet_sklist_lock;
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index dc4cf7d..130338f 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -127,7 +127,7 @@ static struct ctl_table net_core_table[] = {
 	{
 		.ctl_name	= NET_CORE_SOMAXCONN,
 		.procname	= "somaxconn",
-		.data		= &sysctl_somaxconn,
+		.data		= &init_net.sysctl_somaxconn,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec
@@ -161,6 +161,8 @@ static __net_init int sysctl_core_net_init(struct net *net)
 {
 	struct ctl_table *tbl, *tmp;
 
+	net->sysctl_somaxconn = SOMAXCONN;
+
 	tbl = net_core_table;
 	if (net != &init_net) {
 		tbl = kmemdup(tbl, sizeof(net_core_table), GFP_KERNEL);
diff --git a/net/socket.c b/net/socket.c
index 9ebca5c..7651de0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1365,17 +1365,17 @@ asmlinkage long sys_bind(int fd, struct sockaddr __user *umyaddr, int addrlen)
  *	ready for listening.
  */
 
-int sysctl_somaxconn __read_mostly = SOMAXCONN;
-
 asmlinkage long sys_listen(int fd, int backlog)
 {
 	struct socket *sock;
 	int err, fput_needed;
+	int somaxconn;
 
 	sock = sockfd_lookup_light(fd, &err, &fput_needed);
 	if (sock) {
-		if ((unsigned)backlog > sysctl_somaxconn)
-			backlog = sysctl_somaxconn;
+		somaxconn = sock->sk->sk_net->sysctl_somaxconn;
+		if ((unsigned)backlog > somaxconn)
+			backlog = somaxconn;
 
 		err = security_socket_listen(sock, backlog);
 		if (!err)
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.25 2/3]sysctl: prepare core tables to point to netns variables
From: Pavel Emelyanov @ 2007-12-07 13:10 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, Linux Containers, devel

Some of ctl variables are going to be on the struct 
net. Here's the way to adjust the ->data pointer on the
ctl_table-s to point on the right variable.

Since some pointers still point on the global variables,
I keep turning the write bits off on such tables.

This looks to become a common procedure for net sysctls,
so later parts of this code may migrate to some more
generic place.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 57a7ead..dc4cf7d 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -167,8 +167,13 @@ static __net_init int sysctl_core_net_init(struct net *net)
 		if (tbl == NULL)
 			goto err_dup;
 
-		for (tmp = tbl; tmp->procname; tmp++)
-			tmp->mode &= ~0222;
+		for (tmp = tbl; tmp->procname; tmp++) {
+			if (tmp->data >= (void *)&init_net &&
+					tmp->data < (void *)(&init_net + 1))
+				tmp->data += (char *)net - (char *)&init_net;
+			else
+				tmp->mode &= ~0222;
+		}
 	}
 
 	net->sysctl_core_hdr = register_net_sysctl_table(net,
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.25 1/3]sysctl: make the sys.net.core sysctls per-namespace
From: Pavel Emelyanov @ 2007-12-07 13:07 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, Linux Containers, devel

Making them per-namespace is required for the following 
two reasons:

 First, some ctl values have a per-namespace meaning.
 Second, making them writable from the sub-namespace
 is an isolation hole.

So I introduce the pernet operations to create these
tables. For init_net I use the existing statically
declared tables, for sub-namespace they are duplicated
and the write bits are removed from the mode.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index f97b2a4..d593611 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -37,6 +37,9 @@ struct net {
 
 	struct sock 		*rtnl;			/* rtnetlink socket */
 
+	/* core sysctls */
+	struct ctl_table_header	*sysctl_core_hdr;
+
 	/* List of all packet sockets. */
 	rwlock_t		packet_sklist_lock;
 	struct hlist_head	packet_sklist;
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index e322713..57a7ead 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -151,18 +151,58 @@ static struct ctl_table net_core_table[] = {
 	{ .ctl_name = 0 }
 };
 
-static __initdata struct ctl_path net_core_path[] = {
+static __net_initdata struct ctl_path net_core_path[] = {
 	{ .procname = "net", .ctl_name = CTL_NET, },
 	{ .procname = "core", .ctl_name = NET_CORE, },
 	{ },
 };
 
-static __init int sysctl_core_init(void)
+static __net_init int sysctl_core_net_init(struct net *net)
 {
-	struct ctl_table_header *hdr;
+	struct ctl_table *tbl, *tmp;
+
+	tbl = net_core_table;
+	if (net != &init_net) {
+		tbl = kmemdup(tbl, sizeof(net_core_table), GFP_KERNEL);
+		if (tbl == NULL)
+			goto err_dup;
+
+		for (tmp = tbl; tmp->procname; tmp++)
+			tmp->mode &= ~0222;
+	}
+
+	net->sysctl_core_hdr = register_net_sysctl_table(net,
+			net_core_path, tbl);
+	if (net->sysctl_core_hdr == NULL)
+		goto err_reg;
 
-	hdr = register_sysctl_paths(net_core_path, net_core_table);
-	return hdr == NULL ? -ENOMEM : 0;
+	return 0;
+
+err_reg:
+	if (tbl != net_core_table)
+		kfree(tbl);
+err_dup:
+	return -ENOMEM;
+}
+
+static __net_exit void sysctl_core_net_exit(struct net *net)
+{
+	struct ctl_table *tbl;
+
+	tbl = net->sysctl_core_hdr->ctl_table_arg;
+	unregister_net_sysctl_table(net->sysctl_core_hdr);
+	BUG_ON(tbl == net_core_table);
+	kfree(tbl);
+}
+
+static __net_initdata struct pernet_operations sysctl_core_ops = {
+	.init = sysctl_core_net_init,
+	.exit = sysctl_core_net_exit,
+};
+
+static __init int sysctl_core_init(void)
+{
+	return register_pernet_subsys(&sysctl_core_ops);
 }
 
 __initcall(sysctl_core_init);
-- 
1.5.3.4



^ permalink raw reply related

* Re: [RFC] TCP illinois max rtt aging
From: Ilpo Järvinen @ 2007-12-07 13:05 UTC (permalink / raw)
  To: David Miller; +Cc: lachlan.andrew, Netdev, quetchen
In-Reply-To: <20071207.044150.02925935.davem@davemloft.net>

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

On Fri, 7 Dec 2007, David Miller wrote:

> From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Fri, 7 Dec 2007 13:05:46 +0200 (EET)
> 
> > I guess if you get a large cumulative ACK, the amount of processing is 
> > still overwhelming (added DaveM if he has some idea how to combat it).
> > 
> > Even a simple scenario (this isn't anything fancy at all, will occur all 
> > the time): Just one loss => rest skbs grow one by one into a single 
> > very large SACK block (and we do that efficiently for sure) => then the 
> > fast retransmit gets delivered and a cumulative ACK for whole orig_window 
> > arrives => clean_rtx_queue has to do a lot of processing. In this case we 
> > could optimize RB-tree cleanup away (by just blanking it all) but still 
> > getting rid of all those skbs is going to take a larger moment than I'd 
> > like to see.
> > 
> > That tree blanking could be extended to cover anything which ACK more than 
> > half of the tree by just replacing the root (and dealing with potential 
> > recolorization of the root).
> 
> Yes, it's the classic problem.  But it ought to be at least
> partially masked when TSO is in use, because we'll only process
> a handful of SKBs.  The more effectively TSO batches, the
> less work clean_rtx_queue() will do.

No, that's not what is going to happen, TSO won't help at all
because one-by-one SACKs will fragment every single one of them
(see tcp_match_skb_to_sack) :-(. ...So we're back in non-TSO
case, or am I missing something?

> Web100 just provides statistics and other kinds of connection data
> to userspace, all the actual algorithm etc. modifications have been
> merged upstream and yanked out of the web100 patch.  I was looking
> at it the other night and it's frankly totally uninteresting these
> days :-)

...Thanks, I'll keep that in my mind when looking... :-)


-- 
 i.

^ permalink raw reply

* Re: [RFC] TCP illinois max rtt aging
From: David Miller @ 2007-12-07 12:41 UTC (permalink / raw)
  To: ilpo.jarvinen; +Cc: lachlan.andrew, netdev, quetchen
In-Reply-To: <Pine.LNX.4.64.0712071254070.18529@kivilampi-30.cs.helsinki.fi>

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Fri, 7 Dec 2007 13:05:46 +0200 (EET)

> I guess if you get a large cumulative ACK, the amount of processing is 
> still overwhelming (added DaveM if he has some idea how to combat it).
> 
> Even a simple scenario (this isn't anything fancy at all, will occur all 
> the time): Just one loss => rest skbs grow one by one into a single 
> very large SACK block (and we do that efficiently for sure) => then the 
> fast retransmit gets delivered and a cumulative ACK for whole orig_window 
> arrives => clean_rtx_queue has to do a lot of processing. In this case we 
> could optimize RB-tree cleanup away (by just blanking it all) but still 
> getting rid of all those skbs is going to take a larger moment than I'd 
> like to see.
> 
> That tree blanking could be extended to cover anything which ACK more than 
> half of the tree by just replacing the root (and dealing with potential 
> recolorization of the root).

Yes, it's the classic problem.  But it ought to be at least
partially masked when TSO is in use, because we'll only process
a handful of SKBs.  The more effectively TSO batches, the
less work clean_rtx_queue() will do.

When not doing TSO the behavior is super-stupid, we bump reference
counts on the same page multiple times while running over the SKBs
since consequetive SKBs cover data in different spans of the same
page.

The core issue is that we have a poorly behaving data container,
and therefore that's obviously what we need to change.

Conceptually what we probably need to do is seperate the data
maintainence from the SKB objects themselves.  There is a blob
that maintains the paged data state for everything in the
retransmit queue.  SKBs are built and get the page pointers
but don't actually grab references to the pages, the blob
does that and it keeps track of how many SKB references to each
page there are, non-atomically.

The hardest part is dealing with the page lifetime issues.
Unfortunately, when we trim the rtx queue, references to the clones
can still exist in the driver output path.  It's a difficult problem
to overcome in fact, so in the end my suggestion above might not
even be workable.

> No idea about what it could do, haven't yet looked web100, I was planning 
> at some point of time...

Web100 just provides statistics and other kinds of connection data
to userspace, all the actual algorithm etc. modifications have been
merged upstream and yanked out of the web100 patch.  I was looking
at it the other night and it's frankly totally uninteresting these
days :-)

^ permalink raw reply

* Re: [PATCH] AF_RXRPC: Add a missing goto
From: David Miller @ 2007-12-07 12:31 UTC (permalink / raw)
  To: dhowells; +Cc: netdev
In-Reply-To: <20071207112355.12996.56012.stgit@warthog.procyon.org.uk>

From: David Howells <dhowells@redhat.com>
Date: Fri, 07 Dec 2007 11:23:55 +0000

> Add a missing goto to error handling in the RXKAD security module for AF_RXRPC.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Applied, thanks David.

^ permalink raw reply

* [PATCH] AF_RXRPC: Add a missing goto
From: David Howells @ 2007-12-07 11:23 UTC (permalink / raw)
  To: netdev; +Cc: dhowells

Add a missing goto to error handling in the RXKAD security module for AF_RXRPC.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/rxkad.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index e09a95a..8e69d69 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -1021,6 +1021,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 
 	abort_code = RXKADINCONSISTENCY;
 	if (version != RXKAD_VERSION)
+		goto protocol_error;
 
 	abort_code = RXKADTICKETLEN;
 	if (ticket_len < 4 || ticket_len > MAXKRB5TICKETLEN)


^ permalink raw reply related

* Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
From: Denis V. Lunev @ 2007-12-07 11:11 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Woodhouse, Eric W. Biederman, Linus Torvalds,
	Rafael J. Wysocki, Pavel Machek, kernel list, netdev,
	Pavel Emelyanov, Denis V. Lunev
In-Reply-To: <20071207022342.806ee4ee.akpm@linux-foundation.org>

Andrew Morton wrote:
> On Fri, 07 Dec 2007 04:51:37 +0000 David Woodhouse <dwmw2@infradead.org> wrote:
> 
>> On Mon, 2007-11-26 at 15:17 -0700, Eric W. Biederman wrote:
>>> Well I clearly goofed when I added the initial network namespace support
>>> for /proc/net.  Currently things work but there are odd details visible
>>> to user space, even when we have a single network namespace.
>>>
>>> Since we do not cache proc_dir_entry dentries at the moment we can
>>> just modify ->lookup to return a different directory inode depending
>>> on the network namespace of the process looking at /proc/net, replacing
>>> the current technique of using a magic and fragile follow_link method.
>>>
>>> To accomplish that this patch:
>>> - introduces a shadow_proc method to allow different dentries to
>>>   be returned from proc_lookup.
>>> - Removes the old /proc/net follow_link magic
>>> - Fixes a weakness in our not caching of proc generic dentries.
>>>
>>> As shadow_proc uses a task struct to decided which dentry to return we
>>> can go back later and fix the proc generic caching without modifying any code that
>>> uses the shadow_proc method.
>>>
>>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>>> ---
>>>  fs/proc/generic.c       |   12 ++++++-
>>>  fs/proc/proc_net.c      |   86 +++--------------------------------------------
>>>  include/linux/proc_fs.h |    3 ++
>>>  3 files changed, 19 insertions(+), 82 deletions(-)
>> (commit 2b1e300a9dfc3196ccddf6f1d74b91b7af55e416)
>>
>> This seems to have broken the use of /proc/bus/usb as a mountpoint. It
>> always appears empty now, whatever's supposed to be mounted there.
>>
> 
> Yes.  Denis and Eric are tossing around competing patches but afaik nobody
> is happy with any of them.  Guys, could we get this sorted soonish please?
> 

Andrew, I become too relaxed after receiving
"Tested-by: Giacomo Catenazzi <cate@debian.org>"

Eric, I believe that reverting an original behavior is better than your
new one as
- you introduce search into the depth by calling have_submounts(dentry)
during revalidation for all(!) /proc dentries
- your shadowing behavior will be broken if you'll mount something in
the depth of shadowed tree (this can be done as a DoS attempt)

As a last minute call, may be it will be better to pin network namespace
like a pid namespace during mount to avoid this crap at all?

Regards,
	Den

^ permalink raw reply

* Re: [RFC] TCP illinois max rtt aging
From: Ilpo Järvinen @ 2007-12-07 11:05 UTC (permalink / raw)
  To: Lachlan Andrew, David Miller; +Cc: Netdev, Tom Quetchenbach
In-Reply-To: <aa7d2c6d0712061927g2e6f1679o8b05f76d723c0c76@mail.gmail.com>

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

On Thu, 6 Dec 2007, Lachlan Andrew wrote:
> On 04/12/2007, Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> wrote:
> > On Mon, 3 Dec 2007, Lachlan Andrew wrote:
> > >
> > > When SACK is active, the per-packet processing becomes more involved,
> > > tracking the list of lost/SACKed packets.  This causes a CPU spike
> > > just after a loss, which increases the RTTs, at least in my
> > > experience.
> >
> > I suspect that as long as old code was able to use hint, it wasn't doing
> > that bad. But it was seriously lacking ability to take advantage of sack
> > processing hint when e.g., a new hole appeared, or cumulative ACK arrived.
> >
> > ...Code available in net-2.6.25 might cure those.
> 
> We had been using one of your earlier patches, and still had the
> problem.  I think you've cured the problem with SACK itself, but there
> still seems to be something taking a lot of CPU while recovering from
> the loss. 

I guess if you get a large cumulative ACK, the amount of processing is 
still overwhelming (added DaveM if he has some idea how to combat it).

Even a simple scenario (this isn't anything fancy at all, will occur all 
the time): Just one loss => rest skbs grow one by one into a single 
very large SACK block (and we do that efficiently for sure) => then the 
fast retransmit gets delivered and a cumulative ACK for whole orig_window 
arrives => clean_rtx_queue has to do a lot of processing. In this case we 
could optimize RB-tree cleanup away (by just blanking it all) but still 
getting rid of all those skbs is going to take a larger moment than I'd 
like to see.

That tree blanking could be extended to cover anything which ACK more than 
half of the tree by just replacing the root (and dealing with potential 
recolorization of the root).

> It is possible that it was to do with  web100  which we
> have also been running, but I cut out most of the statistics from that
> and still had problems.

No idea about what it could do, haven't yet looked web100, I was planning 
at some point of time...

-- 
 i.

^ permalink raw reply

* Re: [PATCH][VLAN] Merge tree equal tails in vlan_skb_recv
From: Patrick McHardy @ 2007-12-07 10:25 UTC (permalink / raw)
  To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List, devel
In-Reply-To: <47591ECF.2020707@openvz.org>

Pavel Emelyanov wrote:
> There are tree paths in it, that set the skb->proto and then
> perform common receive manipulations (basically call netif_rx()).
> 
> I think, that we can make this code flow easier to understand
> by introducing the vlan_set_encap_proto() function (I hope the 
> name is good) to setup the skb proto and merge the paths calling 
> netif_rx() together.
> 
> Surprisingly, but gcc detects this thing and merges these paths
> by itself, so this patch doesn't make the vlan module smaller.


I already have something similar queued, but your patch is a nice
cleanup on top. I'll merge it into my tree and send it out after
some testing, hopefully today.

^ permalink raw reply

* Re: [PATCH 2.6.24-rc3] Fix /proc/net breakage
From: Andrew Morton @ 2007-12-07 10:23 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Eric W. Biederman, Linus Torvalds, Rafael J. Wysocki,
	Pavel Machek, kernel list, netdev, Pavel Emelyanov,
	Denis V. Lunev
In-Reply-To: <1197003097.13978.557.camel@pmac.infradead.org>

On Fri, 07 Dec 2007 04:51:37 +0000 David Woodhouse <dwmw2@infradead.org> wrote:

> On Mon, 2007-11-26 at 15:17 -0700, Eric W. Biederman wrote:
> > Well I clearly goofed when I added the initial network namespace support
> > for /proc/net.  Currently things work but there are odd details visible
> > to user space, even when we have a single network namespace.
> > 
> > Since we do not cache proc_dir_entry dentries at the moment we can
> > just modify ->lookup to return a different directory inode depending
> > on the network namespace of the process looking at /proc/net, replacing
> > the current technique of using a magic and fragile follow_link method.
> > 
> > To accomplish that this patch:
> > - introduces a shadow_proc method to allow different dentries to
> >   be returned from proc_lookup.
> > - Removes the old /proc/net follow_link magic
> > - Fixes a weakness in our not caching of proc generic dentries.
> > 
> > As shadow_proc uses a task struct to decided which dentry to return we
> > can go back later and fix the proc generic caching without modifying any code that
> > uses the shadow_proc method.
> > 
> > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> > ---
> >  fs/proc/generic.c       |   12 ++++++-
> >  fs/proc/proc_net.c      |   86 +++--------------------------------------------
> >  include/linux/proc_fs.h |    3 ++
> >  3 files changed, 19 insertions(+), 82 deletions(-)
> 
> (commit 2b1e300a9dfc3196ccddf6f1d74b91b7af55e416)
> 
> This seems to have broken the use of /proc/bus/usb as a mountpoint. It
> always appears empty now, whatever's supposed to be mounted there.
> 

Yes.  Denis and Eric are tossing around competing patches but afaik nobody
is happy with any of them.  Guys, could we get this sorted soonish please?

^ permalink raw reply

* [PATCH][VLAN] Merge tree equal tails in vlan_skb_recv
From: Pavel Emelyanov @ 2007-12-07 10:22 UTC (permalink / raw)
  To: David Miller; +Cc: Patrick McHardy, Linux Netdev List, devel

There are tree paths in it, that set the skb->proto and then
perform common receive manipulations (basically call netif_rx()).

I think, that we can make this code flow easier to understand
by introducing the vlan_set_encap_proto() function (I hope the 
name is good) to setup the skb proto and merge the paths calling 
netif_rx() together.

Surprisingly, but gcc detects this thing and merges these paths
by itself, so this patch doesn't make the vlan module smaller.

Fits both net-2.6 and net-2.6.25.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 4f99bb8..11198c1 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -90,6 +90,40 @@ static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
 	return skb;
 }
 
+static inline void vlan_set_encap_proto(struct sk_buff *skb,
+		struct vlan_hdr *vhdr)
+{
+	__be16 proto;
+	unsigned char *rawp;
+
+	/*
+	 * Was a VLAN packet, grab the encapsulated protocol, which the layer
+	 * three protocols care about.
+	 */
+
+	proto = vhdr->h_vlan_encapsulated_proto;
+	if (ntohs(proto) >= 1536) {
+		skb->protocol = proto;
+		return;
+	}
+
+	rawp = skb->data;
+	if (*(unsigned short *)rawp == 0xFFFF)
+		/*
+		 * This is a magic hack to spot IPX packets. Older Novell
+		 * breaks the protocol design and runs IPX over 802.3 without
+		 * an 802.2 LLC layer. We look for FFFF which isn't a used
+		 * 802.2 SSAP/DSAP. This won't work for fault tolerant netware
+		 * but does for the rest.
+		 */
+		skb->protocol = htons(ETH_P_802_3);
+	else
+		/*
+		 * Real 802.2 LLC
+		 */
+		skb->protocol = htons(ETH_P_802_2);
+}
+
 /*
  *	Determine the packet's protocol ID. The rule here is that we
  *	assume 802.3 if the type field is short enough to be a length.
@@ -115,12 +149,10 @@ static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
 int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		  struct packet_type* ptype, struct net_device *orig_dev)
 {
-	unsigned char *rawp = NULL;
 	struct vlan_hdr *vhdr;
 	unsigned short vid;
 	struct net_device_stats *stats;
 	unsigned short vlan_TCI;
-	__be16 proto;
 
 	if (dev->nd_net != &init_net) {
 		kfree_skb(skb);
@@ -236,70 +268,11 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		break;
 	}
 
-	/*  Was a VLAN packet, grab the encapsulated protocol, which the layer
-	 * three protocols care about.
-	 */
-	/* proto = get_unaligned(&vhdr->h_vlan_encapsulated_proto); */
-	proto = vhdr->h_vlan_encapsulated_proto;
-
-	skb->protocol = proto;
-	if (ntohs(proto) >= 1536) {
-		/* place it back on the queue to be handled by
-		 * true layer 3 protocols.
-		 */
-
-		/* See if we are configured to re-write the VLAN header
-		 * to make it look like ethernet...
-		 */
-		skb = vlan_check_reorder_header(skb);
-
-		/* Can be null if skb-clone fails when re-ordering */
-		if (skb) {
-			netif_rx(skb);
-		} else {
-			/* TODO:  Add a more specific counter here. */
-			stats->rx_errors++;
-		}
-		rcu_read_unlock();
-		return 0;
-	}
-
-	rawp = skb->data;
-
-	/*
-	 * This is a magic hack to spot IPX packets. Older Novell breaks
-	 * the protocol design and runs IPX over 802.3 without an 802.2 LLC
-	 * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
-	 * won't work for fault tolerant netware but does for the rest.
-	 */
-	if (*(unsigned short *)rawp == 0xFFFF) {
-		skb->protocol = htons(ETH_P_802_3);
-		/* place it back on the queue to be handled by true layer 3 protocols.
-		 */
-
-		/* See if we are configured to re-write the VLAN header
-		 * to make it look like ethernet...
-		 */
-		skb = vlan_check_reorder_header(skb);
-
-		/* Can be null if skb-clone fails when re-ordering */
-		if (skb) {
-			netif_rx(skb);
-		} else {
-			/* TODO:  Add a more specific counter here. */
-			stats->rx_errors++;
-		}
-		rcu_read_unlock();
-		return 0;
-	}
-
-	/*
-	 *	Real 802.2 LLC
-	 */
-	skb->protocol = htons(ETH_P_802_2);
 	/* place it back on the queue to be handled by upper layer protocols.
 	 */
 
+	vlan_set_encap_proto(skb, vhdr);
+
 	/* See if we are configured to re-write the VLAN header
 	 * to make it look like ethernet...
 	 */
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.25] Remove unused devconf macros
From: Pavel Emelyanov @ 2007-12-07 10:16 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel, Herbert Xu

The SNMP_INC_STATS_OFFSET_BH is used only by ICMP6_INC_STATS_OFFSET_BH.
The ICMP6_INC_STATS_OFFSET_BH is unused.

Can we drop them?

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index a84f3f6..38df94b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -143,14 +143,6 @@ DECLARE_SNMP_STAT(struct icmpv6msg_mib, icmpv6msg_statistics);
 #define ICMP6_INC_STATS_BH(idev, field)	_DEVINC(icmpv6, _BH, idev, field)
 #define ICMP6_INC_STATS_USER(idev, field) _DEVINC(icmpv6, _USER, idev, field)
 
-#define ICMP6_INC_STATS_OFFSET_BH(idev, field, offset)	({			\
-	struct inet6_dev *_idev = idev;						\
-	__typeof__(offset) _offset = (offset);					\
-	if (likely(_idev != NULL))						\
-		SNMP_INC_STATS_OFFSET_BH(_idev->stats.icmpv6, field, _offset);	\
-	SNMP_INC_STATS_OFFSET_BH(icmpv6_statistics, field, _offset);    	\
-})
-
 #define ICMP6MSGOUT_INC_STATS(idev, field) \
 	_DEVINC(icmpv6msg, , idev, field +256)
 #define ICMP6MSGOUT_INC_STATS_BH(idev, field) \
diff --git a/include/net/snmp.h b/include/net/snmp.h
index ea206bf..9c5793d 100644
--- a/include/net/snmp.h
+++ b/include/net/snmp.h
@@ -134,8 +134,6 @@ struct linux_mib {
 
 #define SNMP_INC_STATS_BH(mib, field) 	\
 	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field]++)
-#define SNMP_INC_STATS_OFFSET_BH(mib, field, offset)	\
-	(per_cpu_ptr(mib[0], raw_smp_processor_id())->mibs[field + (offset)]++)
 #define SNMP_INC_STATS_USER(mib, field) \
 	(per_cpu_ptr(mib[1], raw_smp_processor_id())->mibs[field]++)
 #define SNMP_INC_STATS(mib, field) 	\
-- 
1.5.3.4


^ permalink raw reply related

* Re: sockets affected by IPsec always block (2.6.23)
From: Stefan Rompf @ 2007-12-07  9:29 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, simon, linux-kernel, netdev
In-Reply-To: <20071206.192030.170597084.davem@davemloft.net>

Am Freitag, 7. Dezember 2007 04:20 schrieb David Miller:

> If IPSEC takes a long time to resolve, and we don't block, the
> connect() can hard fail (we will just keep dropping the outgoing SYN
> packet send attempts, eventually hitting the retry limit) in cases
> where if we did block it would not fail (because we wouldn't send
> the first SYN until IPSEC resolved).

David - I'm aware of this, the discussion is which behaviour is ok. Let's go 
back to a real life example. I've already researched that the squid web proxy 
has a poll() based main loop doing nonblocking connects, may be with multiple 
threads.

Situation: One user wants to access a web page that needs IPSEC. The SA takes 
30 seconds to come up.

a) Non-blocking connect is respected: SYN packets during the first 30 seconds 
will be dropped as you said. Connection can be completed on the next SYN 
retry (timeout in linux: 3 minutes). During this time, the 500 other users 
can continue to browse using the proxy.

b) Non-blocking connect is ignored during IPSEC resolving as you advocate it: 
Connection for the one user can be completed immediatly after IPSEC comes up. 
That's the pro. However, until then, the other 500 proxy user CANNOT ACCESS 
THE WEB because squid's threads are stuck in connect()s on sockets they 
configured not to block. If the IPSEC SA never resolves due to some network 
outage, squid will sleep forever or until an admin configures it that it 
doesn't try to connect the adress in question and restarts it.

Don't you realize how broken this behaviour is? Can you give me ONE example of 
an application that works better with b) and why this outweights the problems 
it creates for everybody else?

Even the DNS example you posted in  
<20071204.231200.117152338.davem@davemloft.net> is wrong because the second 
server will never queried if the kernel puts the process into coma while the 
IPSEC SA to the first server cannot be resolved.

Stefan

^ permalink raw reply

* [PATCH][IPV4] Swap the ifa allocation with the"ipv4_devconf_setall" call
From: Pavel Emelyanov @ 2007-12-07  9:22 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Linux Netdev List, devel

According to Herbert, the ipv4_devconf_setall should be called
only when the ifa is added to the device. However, failed 
ifa allocation may bring things into inconsistent state.

Move the call to ipv4_devconf_setall after the ifa allocation.

Fits both net-2.6 (with offsets) and net-2.6.25 (cleanly).

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 0b5f042..1c3e20c 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -519,8 +519,6 @@ static struct in_ifaddr *rtm_to_ifaddr(struct nlmsghdr *nlh)
 		goto errout;
 	}
 
-	ipv4_devconf_setall(in_dev);
-
 	ifa = inet_alloc_ifa();
 	if (ifa == NULL) {
 		/*
@@ -531,6 +529,7 @@ static struct in_ifaddr *rtm_to_ifaddr(struct nlmsghdr *nlh)
 		goto errout;
 	}
 
+	ipv4_devconf_setall(in_dev);
 	in_dev_hold(in_dev);
 
 	if (tb[IFA_ADDRESS] == NULL)

^ permalink raw reply related

* Re: [PATCH 14/20] net/ipv4/cipso_ipv4.c: use LIST_HEAD instead of LIST_HEAD_INIT
From: David Miller @ 2007-12-07  8:49 UTC (permalink / raw)
  To: crquan; +Cc: netdev, akpm, linux-kernel
In-Reply-To: <1196957076-5706-1-git-send-email-crquan@gmail.com>

From: Denis Cheng <crquan@gmail.com>
Date: Fri,  7 Dec 2007 00:04:36 +0800

> single list_head variable initialized with LIST_HEAD_INIT could almost
> always can be replaced with LIST_HEAD declaration, this shrinks the code
> and looks better.
> 
> Signed-off-by: Denis Cheng <crquan@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH 13/20] net/core/dev.c: use LIST_HEAD instead of LIST_HEAD_INIT
From: David Miller @ 2007-12-07  8:49 UTC (permalink / raw)
  To: crquan; +Cc: netdev, akpm, linux-kernel
In-Reply-To: <1196956886-5681-1-git-send-email-crquan@gmail.com>

From: Denis Cheng <crquan@gmail.com>
Date: Fri,  7 Dec 2007 00:01:26 +0800

> single list_head variable initialized with LIST_HEAD_INIT could almost
> always can be replaced with LIST_HEAD declaration, this shrinks the code
> and looks better.
> 
> Signed-off-by: Denis Cheng <crquan@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2.6.25] net: move trie_local and trie_main into the proc iterator
From: David Miller @ 2007-12-07  8:48 UTC (permalink / raw)
  To: den; +Cc: containers, devel, netdev, ebiederm
In-Reply-To: <20071206150012.GA22475@iris.sw.ru>

From: "Denis V. Lunev" <den@openvz.org>
Date: Thu, 6 Dec 2007 18:00:12 +0300

> From: Eric W. Biederman <ebiederm@xmission.com>
> 
> We only use these variables when displaying the trie in proc so
> place them into the iterator to make this explicit.  We should
> probably do something smarter to handle the CONFIG_IP_MULTIPLE_TABLES
> case but at least this makes it clear that the silliness is limited
> to the display in /proc.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Applied.

^ permalink raw reply

* Re: [PATCH 2.6.25] Remove ip_fib_local_table and ip_fib_main_table defines
From: David Miller @ 2007-12-07  8:46 UTC (permalink / raw)
  To: den; +Cc: containers, devel, netdev, ebiederm
In-Reply-To: <20071206145825.GA22435@iris.sw.ru>

From: "Denis V. Lunev" <den@openvz.org>
Date: Thu, 6 Dec 2007 17:58:25 +0300

> From: Eric W. Biederman <ebiederm@xmission.com>
> 
> There are only 2 users and it doesn't hurt to call fib_get_table
> instead, and it makes it easier to make the fib network namespace
> aware.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Applied, thanks.

^ permalink raw reply

* Re: [patch 6/6] ipv6 - route6/fib6 : dont panic a kmem_cache_create
From: David Miller @ 2007-12-07  8:45 UTC (permalink / raw)
  To: dlezcano; +Cc: yoshfuji, pekkas, netdev, benjamin.thery
In-Reply-To: <20071206140324.805833763@ICON-9-164-138-215.megacenter.de.ibm.com>

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:35 +0100

> If the kmem_cache_creation fails, the kernel will panic. It is acceptable
> if the system is booting, but if the ipv6 protocol is compiled as a module
> and it is loaded after the system has booted, do we want to panic instead
> of just failing to initialize the protocol ?
> 
> The init function is now returning an error and this one is checked for
> protocol initialization. So the ipv6 protocol will safely fails.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Also applied.

^ permalink raw reply

* Re: [patch 5/6] ipv6 - make af_inet6 to check ip6_route_init return value
From: David Miller @ 2007-12-07  8:44 UTC (permalink / raw)
  To: dlezcano; +Cc: yoshfuji, pekkas, netdev, benjamin.thery
In-Reply-To: <20071206140307.686276824@ICON-9-164-138-215.megacenter.de.ibm.com>

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:34 +0100

> The af_inet6 initialization function does not check the return code
> of the route initilization, so if something goes wrong, the protocol
> initialization will continue anyway.
> This patch takes into account the modification made in the different
> route's initialization subroutines to check the return value and to 
> make the protocol initialization to fail.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks!

^ permalink raw reply

* Re: [patch 4/6] ipv6 - make ip6_route_init to return an error code
From: David Miller @ 2007-12-07  8:43 UTC (permalink / raw)
  To: dlezcano; +Cc: yoshfuji, pekkas, netdev, benjamin.thery
In-Reply-To: <20071206140303.281847428@ICON-9-164-138-215.megacenter.de.ibm.com>

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:33 +0100

> The route initialization function does not return any value to notify if
> the initialization is successful or not. This patch checks all calls made
> for the initilization in order to return a value for the caller.
> 
> Unfortunatly, proc_net_fops_create will return a NULL pointer if CONFIG_PROC_FS
> is off, so we can not check the return code without an ifdef CONFIG_PROC_FS 
> block in the ip6_route_init function.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks.

^ permalink raw reply

* Re: [patch 3/6] ipv6 - make fib6_rules_init to return an error code
From: David Miller @ 2007-12-07  8:43 UTC (permalink / raw)
  To: dlezcano; +Cc: yoshfuji, pekkas, netdev, benjamin.thery
In-Reply-To: <20071206140258.610073641@ICON-9-164-138-215.megacenter.de.ibm.com>

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:32 +0100

> When the fib_rules initialization finished, no return code is provided
> so there is no way to know, for the caller, if the initialization has
> been successful or has failed. This patch fix that.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks.

^ permalink raw reply

* Re: [patch 2/6] ipv6 - make xfrm6_init to return an error code
From: David Miller @ 2007-12-07  8:42 UTC (permalink / raw)
  To: dlezcano; +Cc: yoshfuji, pekkas, netdev, benjamin.thery
In-Reply-To: <20071206140254.504102655@ICON-9-164-138-215.megacenter.de.ibm.com>

From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Thu, 06 Dec 2007 14:53:31 +0100

> The xfrm initialization function does not return any error code, so
> if there is an error, the caller can not be advise of that.
> This patch checks the return code of the different called functions
> in order to return a successful or failed initialization.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Acked-by: Benjamin Thery <benjamin.thery@bull.net>

Applied, thanks.

^ 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