Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 3/17 net-2.6.26] [NETFILTER]: Consolidate masq_inet_event and masq_device_event.
From: Patrick McHardy @ 2008-02-19 14:10 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: davem, netdev, containers, devel
In-Reply-To: <1203406297-32725-3-git-send-email-den@openvz.org>

Denis V. Lunev wrote:
> They do exactly the same job.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  net/ipv4/netfilter/ipt_MASQUERADE.c |   14 ++------------
>  1 files changed, 2 insertions(+), 12 deletions(-)

Looks fine.

^ permalink raw reply

* Re: AW: AW: Problem receiving multicast with kernel.2.6.24 #3
From: Patrick McHardy @ 2008-02-19 14:02 UTC (permalink / raw)
  To: Reither Robert; +Cc: David Stevens, netdev
In-Reply-To: <BA41E8A07B73A041948C2E3A4EF6239D062F1A@av-digital-exch.avdigital.at>

[-- Attachment #1: Type: text/plain, Size: 565 bytes --]

Reither Robert wrote:
> Visit AVD on prolight+sound in Frankfurt from 12.-15. March 2008 - Hall 8.0, Stand G16
> ________________________________________________________________________
> 
> 
> 
> OK, found one solution (but i think, this should not the normal way)
> 
> MC joining first the VLAN device (eth0.3) and than the physical (eth0) does the trick, 100% success ...
> 
> Doing it the opposite way, gives the same problem as before ...
> 
> So somehow joining the VLAN interface disrupts the join to the physical one ...
> 
> Ideas ?

Does this patch help?

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1058 bytes --]

commit 6548b91f39381b2c5f02f99c14734546354bff89
Author: Jorge Boncompte [DTI2] <jorge@dti2.net>
Date:   Mon Feb 18 16:02:01 2008 +0100

    [NET]: Messed multicast lists after dev_mc_sync/unsync
    
    Commit a0a400d79e3dd7843e7e81baa3ef2957bdc292d0 from you
    introduced a new field "da_synced" to struct dev_addr_list that is
    not properly initialized to 0. So when any of the current users (8021q,
    macvlan, mac80211) calls dev_mc_sync/unsync they mess the address
    list for both devices.
    
    The attached patch fixed it for me and avoid future problems.
    
    Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/core/dev.c b/net/core/dev.c
index 6cfc123..9516105 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2900,7 +2900,7 @@ int __dev_addr_add(struct dev_addr_list **list, int *count,
 		}
 	}
 
-	da = kmalloc(sizeof(*da), GFP_ATOMIC);
+	da = kzalloc(sizeof(*da), GFP_ATOMIC);
 	if (da == NULL)
 		return -ENOMEM;
 	memcpy(da->da_addr, addr, alen);

^ permalink raw reply related

* Re: [NETFILTER]: Introduce nf_inet_address
From: Patrick McHardy @ 2008-02-19 14:01 UTC (permalink / raw)
  To: David Woodhouse; +Cc: netdev, Jan Engelhardt, David S. Miller, varevoka
In-Reply-To: <1203428949.3223.25.camel@shinybook.infradead.org>

[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]

David Woodhouse wrote:
>> +union nf_inet_addr {
>> +	u_int32_t	all[4];
>> +	__be32		ip;
>> +	__be32		ip6[4];
>> +};
>> +
>>  #ifdef __KERNEL__
>>  #ifdef CONFIG_NETFILTER
> 
> This breaks the busybox build:
> 
> CC      ipsvd/tcpudp.o
> In file included from /usr/include/linux/netfilter_ipv4.h:8,
>                  from ipsvd/tcpudp.c:33:
> /usr/include/linux/netfilter.h:40: error: expected specifier-qualifier-list before 'u_int32_t'
> 
> What is this 'u_int32_t' nonsense anyway?
> 
> If a user-visible header is likely to be included by libc directly from
> a 'standard' header, it may not require <stdint.h>. Therefore it should
> use the system-specific types such as '__u32'.

Right, I queued this patch to fix it.

> If it isn't likely to be included by libc, which is the case for
> netfilter, then it might as well just use the proper C types. Those who
> are stuck on C89 or earlier might still prefer to use '__u32' even when
> there's no need for it, but 'u_int32_t' is just silly. I suspect we
> should eradicate it.

Yes, some more consitency would be nice. So far the consensus was
to not use it in new code, but keep using it in subsystems like
netfilter that (almost) consistently use it everywhere.

> I couldn't make busybox work with it --
> __BIT_TYPES_DEFINED__ is defined in <sys/types.h> and prevents the
> definitions of u_int32_t et al from appearing in <linux/types.h>. And if
> I include <linux/types.h> first, other things break.
> 
> A later commit adds struct in_addr and struct in6_addr to this union
> too, which breaks busybox even harder.

Thats odd, the iptables headers have always used struct in_addr and
struct in6_addr in struct ipt_ip/struct ip6t_ip6, which are also
used by userspace. What is "ipsvd/tcpudp.c"? I couldn't find it in
the Debian busybox source.

> How is this supposed to be used in userspace? Or is it even supposed to
> be exposed?

Yes, its meant to replace many self-made "AF-independant" address
representations.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 709 bytes --]

commit 6f2e68f81457d72bdb14a7ead305a1da06e78893
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Feb 19 14:54:36 2008 +0100

    [NETFILTER]: Use __u32 in struct nf_inet_addr
    
    As reported by David Woodhouse <dwmw2@infradead.org>, using u_int32_t in
    struct nf_inet_addr breaks the busybox build. Fix by using __u32.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index d74e79b..b74b615 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -51,7 +51,7 @@ enum nf_inet_hooks {
 };
 
 union nf_inet_addr {
-	u_int32_t	all[4];
+	__u32		all[4];
 	__be32		ip;
 	__be32		ip6[4];
 	struct in_addr	in;

^ permalink raw reply related

* AW: AW: Problem receiving multicast with kernel.2.6.24 #3
From: Reither Robert @ 2008-02-19 13:57 UTC (permalink / raw)
  To: David Stevens; +Cc: netdev

Visit AVD on prolight+sound in Frankfurt from 12.-15. March 2008 - Hall 8.0, Stand G16
________________________________________________________________________



OK, found one solution (but i think, this should not the normal way)

MC joining first the VLAN device (eth0.3) and than the physical (eth0) does the trick, 100% success ...

Doing it the opposite way, gives the same problem as before ...

So somehow joining the VLAN interface disrupts the join to the physical one ...

Ideas ?

Robert

^ permalink raw reply

* Re: [NETFILTER]: Introduce nf_inet_address
From: David Woodhouse @ 2008-02-19 13:49 UTC (permalink / raw)
  To: netdev; +Cc: Jan Engelhardt, Patrick McHardy, David S. Miller, varevoka
In-Reply-To: <200801291316.m0TDGivY024953@hera.kernel.org>


On Tue, 2008-01-29 at 13:16 +0000, Linux Kernel Mailing List wrote:
> Commit:     643a2c15a407faf08101a20e1a3461160711899d
>
>     [NETFILTER]: Introduce nf_inet_address
>     
>     A few netfilter modules provide their own union of IPv4 and IPv6
>     address storage. Will unify that in this patch series.
>     
>     (1/4): Rename union nf_conntrack_address to union nf_inet_addr and
>     move it to x_tables.h.
>     
>     Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
>     Signed-off-by: Patrick McHardy <kaber@trash.net>
>     Signed-off-by: David S. Miller <davem@davemloft.net>

 ...

> --- a/include/linux/netfilter.h
> +++ b/include/linux/netfilter.h
> @@ -48,6 +48,12 @@ enum nf_inet_hooks {
>  	NF_INET_NUMHOOKS
>  };
>  
> +union nf_inet_addr {
> +	u_int32_t	all[4];
> +	__be32		ip;
> +	__be32		ip6[4];
> +};
> +
>  #ifdef __KERNEL__
>  #ifdef CONFIG_NETFILTER

This breaks the busybox build:

CC      ipsvd/tcpudp.o
In file included from /usr/include/linux/netfilter_ipv4.h:8,
                 from ipsvd/tcpudp.c:33:
/usr/include/linux/netfilter.h:40: error: expected specifier-qualifier-list before 'u_int32_t'

What is this 'u_int32_t' nonsense anyway?

If a user-visible header is likely to be included by libc directly from
a 'standard' header, it may not require <stdint.h>. Therefore it should
use the system-specific types such as '__u32'.

If it isn't likely to be included by libc, which is the case for
netfilter, then it might as well just use the proper C types. Those who
are stuck on C89 or earlier might still prefer to use '__u32' even when
there's no need for it, but 'u_int32_t' is just silly. I suspect we
should eradicate it. I couldn't make busybox work with it --
__BIT_TYPES_DEFINED__ is defined in <sys/types.h> and prevents the
definitions of u_int32_t et al from appearing in <linux/types.h>. And if
I include <linux/types.h> first, other things break.

A later commit adds struct in_addr and struct in6_addr to this union
too, which breaks busybox even harder.

How is this supposed to be used in userspace? Or is it even supposed to
be exposed?

-- 
dwmw2


^ permalink raw reply

* Re: "protocol 0300 is buggy" spam in dmesg when injecting&capturing on same interface
From: Pekka Pietikainen @ 2008-02-19 13:39 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20080218153903.GA30253@ee.oulu.fi>

On Mon, Feb 18, 2008 at 05:39:03PM +0200, Pekka Pietikainen wrote:
> When playing with some L2 level fuzzing I started getting lots of
> "protocol 0300 is buggy, dev eth3" spew in dmesg. That interface is also
> capturing the traffic that's being sent, that's probably why the
> dev_queue_xmit_nit codepath is getting called in the first place.
> 
> Any ideas? Add a "If it came from AF_PACKET, don't print out anything" to
> that if-statement?
I'm probably just plastering over a bug in af_packet.c with this one,
but the following patch should make it shut up. The printk definately
needs a ntohs for skb2->protocol, took me a while to figure out where
the 0300 even came from :-) 

Signed-off-by: Pekka Pietikainen <pp@ee.oulu.fi>

diff -up linux-2.6.24.i686/net/core/dev.c.orig linux-2.6.24.i686/net/core/dev.c
--- linux-2.6.24.i686/net/core/dev.c.orig	2008-02-19 15:22:12.000000000 +0200
+++ linux-2.6.24.i686/net/core/dev.c	2008-02-19 15:29:37.000000000 +0200
@@ -1262,10 +1262,11 @@ static void dev_queue_xmit_nit(struct sk
 
 			if (skb_network_header(skb2) < skb2->data ||
 			    skb2->network_header > skb2->tail) {
-				if (net_ratelimit())
-					printk(KERN_CRIT "protocol %04x is "
-					       "buggy, dev %s\n",
-					       skb2->protocol, dev->name);
+				if (skb2->protocol != htons(ETH_P_ALL) &&
+				    net_ratelimit())
+					pr_crit("protocol %04x is buggy, dev %s\n",
+						ntohs(skb2->protocol),
+						dev->name);
 				skb_reset_network_header(skb2);
 			}
 


-- 
Pekka Pietikainen

^ permalink raw reply

* [PATCH] [NETNS]: Namespace leak in pneigh_lookup.
From: Denis V. Lunev @ 2008-02-19 13:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, dlezcano, Denis V. Lunev

release_net is missed on the error path in pneigh_lookup.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/core/neighbour.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 7bb6a9a..174e29e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -507,6 +507,7 @@ struct pneigh_entry * pneigh_lookup(struct neigh_table *tbl,
 	if (tbl->pconstructor && tbl->pconstructor(n)) {
 		if (dev)
 			dev_put(dev);
+		release_net(net);
 		kfree(n);
 		n = NULL;
 		goto out;
-- 
1.5.3.rc5


^ permalink raw reply related

* On the Top of the World~『Textile Industry』
From: 『Taiwan News Express』 @ 2008-02-19 11:48 UTC (permalink / raw)
  To: netdev



 <http://www.taiwannews.com.tw/static/express/080219-01.pdf>
<http://www.taiwannews.com.tw/static/express/080219-02.pdf>
<http://www.taiwannews.com.tw/static/express/080219-03.pdf>
<http://www.taiwannews.com.tw/static/express/080219-04.pdf>



 


--
Powered by PHPlist, www.phplist.com --




^ permalink raw reply

* [PATCH net-2.6.26 5/5][SYSCTL]: Move some net.core sysctls to RO root.
From: Pavel Emelyanov @ 2008-02-19 12:05 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <47BAC38F.10100@openvz.org>

There are many tables in net/core/sysctl_net_core.c that are
to be read-only. Current implementation duplicates this array
for each namespace just to clear the "write" bits in the
permissions mask.

Keep the writable tables to per-net ctl root and move the others
to the read-only one. This saves some memory in run time and
removes the... ugly code, that prepared the tables.

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

---
 net/core/sysctl_net_core.c |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 130338f..4e530ce 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -125,14 +125,6 @@ static struct ctl_table net_core_table[] = {
 #endif /* CONFIG_XFRM */
 #endif /* CONFIG_NET */
 	{
-		.ctl_name	= NET_CORE_SOMAXCONN,
-		.procname	= "somaxconn",
-		.data		= &init_net.sysctl_somaxconn,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= &proc_dointvec
-	},
-	{
 		.ctl_name	= NET_CORE_BUDGET,
 		.procname	= "netdev_budget",
 		.data		= &netdev_budget,
@@ -151,6 +143,18 @@ static struct ctl_table net_core_table[] = {
 	{ .ctl_name = 0 }
 };
 
+static struct ctl_table netns_core_table[] = {
+	{
+		.ctl_name	= NET_CORE_SOMAXCONN,
+		.procname	= "somaxconn",
+		.data		= &init_net.sysctl_somaxconn,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec
+	},
+	{ .ctl_name = 0 }
+};
+
 static __net_initdata struct ctl_path net_core_path[] = {
 	{ .procname = "net", .ctl_name = CTL_NET, },
 	{ .procname = "core", .ctl_name = NET_CORE, },
@@ -159,23 +163,17 @@ static __net_initdata struct ctl_path net_core_path[] = {
 
 static __net_init int sysctl_core_net_init(struct net *net)
 {
-	struct ctl_table *tbl, *tmp;
+	struct ctl_table *tbl;
 
 	net->sysctl_somaxconn = SOMAXCONN;
 
-	tbl = net_core_table;
+	tbl = netns_core_table;
 	if (net != &init_net) {
-		tbl = kmemdup(tbl, sizeof(net_core_table), GFP_KERNEL);
+		tbl = kmemdup(tbl, sizeof(netns_core_table), GFP_KERNEL);
 		if (tbl == NULL)
 			goto err_dup;
 
-		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;
-		}
+		tbl[0].data = &net->sysctl_somaxconn;
 	}
 
 	net->sysctl_core_hdr = register_net_sysctl_table(net,
@@ -209,6 +207,7 @@ static __net_initdata struct pernet_operations sysctl_core_ops = {
 
 static __init int sysctl_core_init(void)
 {
+	register_init_net_ctl_table(net_core_path, net_core_table);
 	return register_pernet_subsys(&sysctl_core_ops);
 }
 
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.26 4/5][SYSCTL]: Create the net sysctl root for RO tables.
From: Pavel Emelyanov @ 2008-02-19 12:02 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <47BAC38F.10100@openvz.org>

This root keeps ctl tables in one global list, but doesn't allow
for non-init namespaces to write into tables, stored in it.

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

---
 include/net/net_namespace.h |    2 ++
 net/sysctl_net.c            |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 28738b7..2930ae3 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -173,6 +173,8 @@ struct ctl_table;
 struct ctl_table_header;
 extern struct ctl_table_header *register_net_sysctl_table(struct net *net,
 	const struct ctl_path *path, struct ctl_table *table);
+extern struct ctl_table_header *register_init_net_ctl_table(
+		struct ctl_path *path, struct ctl_table *table);
 extern void unregister_net_sysctl_table(struct ctl_table_header *header);
 
 #endif /* __NET_NET_NAMESPACE_H */
diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index 665e856..42c99e6 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -40,6 +40,30 @@ static struct ctl_table_root net_sysctl_root = {
 	.lookup = net_ctl_header_lookup,
 };
 
+static LIST_HEAD(net_ro_headers);
+
+static struct list_head *net_ctl_ro_header_lookup(struct ctl_table_root *root,
+		struct nsproxy *namespaces)
+{
+	return &net_ro_headers;
+}
+
+static int net_ctl_ro_permissions(struct ctl_table_root *root,
+		struct nsproxy *ns, struct ctl_table *table)
+{
+	int mode;
+
+	mode = table->mode;
+	if (ns->net_ns != &init_net)
+		mode &= ~0222;
+	return mode;
+}
+
+static struct ctl_table_root net_sysctl_ro_root = {
+	.lookup = net_ctl_ro_header_lookup,
+	.permissions = net_ctl_ro_permissions,
+};
+
 static int sysctl_net_init(struct net *net)
 {
 	INIT_LIST_HEAD(&net->sysctl_table_headers);
@@ -64,6 +88,7 @@ static __init int sysctl_init(void)
 	if (ret)
 		goto out;
 	register_sysctl_root(&net_sysctl_root);
+	register_sysctl_root(&net_sysctl_ro_root);
 out:
 	return ret;
 }
@@ -80,6 +105,14 @@ struct ctl_table_header *register_net_sysctl_table(struct net *net,
 }
 EXPORT_SYMBOL_GPL(register_net_sysctl_table);
 
+struct ctl_table_header *register_init_net_ctl_table(struct ctl_path *path,
+		struct ctl_table *table)
+{
+	return __register_sysctl_paths(&net_sysctl_ro_root,
+			&init_nsproxy, path, table);
+}
+EXPORT_SYMBOL_GPL(register_net_ro_ctl_table);
+
 void unregister_net_sysctl_table(struct ctl_table_header *header)
 {
 	return unregister_sysctl_table(header);
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.26 3/5][SYSCTL]: Add the ->permissions callback on the ctl_table_root.
From: Pavel Emelyanov @ 2008-02-19 12:00 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <47BAC38F.10100@openvz.org>

When the table came from some other root, this root may affect
the table's permissions, depending on who is working with the
table.

The core hunk is at the bottom of this patch. All the rest is
just pushing the ctl_table_root argument up to the sysctl_perm
function.

This will be mostly (only?) used in the net sysctls.

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

---
 fs/proc/proc_sysctl.c  |    4 ++--
 include/linux/sysctl.h |    7 ++++++-
 kernel/sysctl.c        |   25 ++++++++++++++++++-------
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 5e31585..5acc001 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -190,7 +190,7 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
 	 * and won't be until we finish.
 	 */
 	error = -EPERM;
-	if (sysctl_perm(table, write ? MAY_WRITE : MAY_READ))
+	if (sysctl_perm(head->root, table, write ? MAY_WRITE : MAY_READ))
 		goto out;
 
 	/* careful: calling conventions are nasty here */
@@ -388,7 +388,7 @@ static int proc_sys_permission(struct inode *inode, int mask, struct nameidata *
 		goto out;
 
 	/* Use the permissions on the sysctl table entry */
-	error = sysctl_perm(table, mask);
+	error = sysctl_perm(head->root, table, mask);
 out:
 	sysctl_head_finish(head);
 	return error;
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 8e50196..3239561 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -945,11 +945,14 @@ enum
 /* For the /proc/sys support */
 struct ctl_table;
 struct nsproxy;
+struct ctl_table_root;
+
 extern struct ctl_table_header *sysctl_head_next(struct ctl_table_header *prev);
 extern struct ctl_table_header *__sysctl_head_next(struct nsproxy *namespaces,
 						struct ctl_table_header *prev);
 extern void sysctl_head_finish(struct ctl_table_header *prev);
-extern int sysctl_perm(struct ctl_table *table, int op);
+extern int sysctl_perm(struct ctl_table_root *root,
+		struct ctl_table *table, int op);
 
 typedef struct ctl_table ctl_table;
 
@@ -1049,6 +1052,8 @@ struct ctl_table_root {
 	struct list_head header_list;
 	struct list_head *(*lookup)(struct ctl_table_root *root,
 					   struct nsproxy *namespaces);
+	int (*permissions)(struct ctl_table_root *root,
+			struct nsproxy *namespaces, struct ctl_table *table);
 };
 
 /* struct ctl_table_header is used to maintain dynamic lists of
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index c224cc5..8b8b582 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1454,7 +1454,8 @@ void register_sysctl_root(struct ctl_table_root *root)
 
 #ifdef CONFIG_SYSCTL_SYSCALL
 /* Perform the actual read/write of a sysctl table entry. */
-static int do_sysctl_strategy (struct ctl_table *table,
+static int do_sysctl_strategy (struct ctl_table_root *root,
+			struct ctl_table *table,
 			int __user *name, int nlen,
 			void __user *oldval, size_t __user *oldlenp,
 			void __user *newval, size_t newlen)
@@ -1465,7 +1466,7 @@ static int do_sysctl_strategy (struct ctl_table *table,
 		op |= 004;
 	if (newval) 
 		op |= 002;
-	if (sysctl_perm(table, op))
+	if (sysctl_perm(root, table, op))
 		return -EPERM;
 
 	if (table->strategy) {
@@ -1491,6 +1492,7 @@ static int do_sysctl_strategy (struct ctl_table *table,
 static int parse_table(int __user *name, int nlen,
 		       void __user *oldval, size_t __user *oldlenp,
 		       void __user *newval, size_t newlen,
+		       struct ctl_table_root *root,
 		       struct ctl_table *table)
 {
 	int n;
@@ -1505,14 +1507,14 @@ repeat:
 		if (n == table->ctl_name) {
 			int error;
 			if (table->child) {
-				if (sysctl_perm(table, 001))
+				if (sysctl_perm(root, table, 001))
 					return -EPERM;
 				name++;
 				nlen--;
 				table = table->child;
 				goto repeat;
 			}
-			error = do_sysctl_strategy(table, name, nlen,
+			error = do_sysctl_strategy(root, table, name, nlen,
 						   oldval, oldlenp,
 						   newval, newlen);
 			return error;
@@ -1538,7 +1540,8 @@ int do_sysctl(int __user *name, int nlen, void __user *oldval, size_t __user *ol
 	for (head = sysctl_head_next(NULL); head;
 			head = sysctl_head_next(head)) {
 		error = parse_table(name, nlen, oldval, oldlenp, 
-					newval, newlen, head->ctl_table);
+					newval, newlen,
+					head->root, head->ctl_table);
 		if (error != -ENOTDIR) {
 			sysctl_head_finish(head);
 			break;
@@ -1584,13 +1587,21 @@ static int test_perm(int mode, int op)
 	return -EACCES;
 }
 
-int sysctl_perm(struct ctl_table *table, int op)
+int sysctl_perm(struct ctl_table_root *root, struct ctl_table *table, int op)
 {
 	int error;
+	int mode;
+
 	error = security_sysctl(table, op);
 	if (error)
 		return error;
-	return test_perm(table->mode, op);
+
+	if (root->permissions)
+		mode = root->permissions(root, current->nsproxy, table);
+	else
+		mode = table->mode;
+
+	return test_perm(mode, op);
 }
 
 static void sysctl_set_parent(struct ctl_table *parent, struct ctl_table *table)
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.26 2/5][SYSCTL]: Clean sysctls from unneeded extern and forward declarations.
From: Pavel Emelyanov @ 2008-02-19 11:58 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <47BAC38F.10100@openvz.org>

The do_sysctl_strategy can be static since it's used in 
kernel/sysctl.c only.

Besides, move it and parse_table above their callers and 
drop the forward declarations.

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

---
 include/linux/sysctl.h |    5 --
 kernel/sysctl.c        |  144 +++++++++++++++++++++++-------------------------
 2 files changed, 68 insertions(+), 81 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 571f01d..8e50196 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -981,11 +981,6 @@ extern int do_sysctl (int __user *name, int nlen,
 		      void __user *oldval, size_t __user *oldlenp,
 		      void __user *newval, size_t newlen);
 
-extern int do_sysctl_strategy (struct ctl_table *table,
-			       int __user *name, int nlen,
-			       void __user *oldval, size_t __user *oldlenp,
-			       void __user *newval, size_t newlen);
-
 extern ctl_handler sysctl_data;
 extern ctl_handler sysctl_string;
 extern ctl_handler sysctl_intvec;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 740e144..c224cc5 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -145,12 +145,6 @@ extern int no_unaligned_warning;
 extern int max_lock_depth;
 #endif
 
-#ifdef CONFIG_SYSCTL_SYSCALL
-static int parse_table(int __user *, int, void __user *, size_t __user *,
-		void __user *, size_t, struct ctl_table *);
-#endif
-
-
 #ifdef CONFIG_PROC_SYSCTL
 static int proc_do_cad_pid(struct ctl_table *table, int write, struct file *filp,
 		  void __user *buffer, size_t *lenp, loff_t *ppos);
@@ -1459,6 +1453,74 @@ void register_sysctl_root(struct ctl_table_root *root)
 }
 
 #ifdef CONFIG_SYSCTL_SYSCALL
+/* Perform the actual read/write of a sysctl table entry. */
+static int do_sysctl_strategy (struct ctl_table *table,
+			int __user *name, int nlen,
+			void __user *oldval, size_t __user *oldlenp,
+			void __user *newval, size_t newlen)
+{
+	int op = 0, rc;
+
+	if (oldval)
+		op |= 004;
+	if (newval) 
+		op |= 002;
+	if (sysctl_perm(table, op))
+		return -EPERM;
+
+	if (table->strategy) {
+		rc = table->strategy(table, name, nlen, oldval, oldlenp,
+				     newval, newlen);
+		if (rc < 0)
+			return rc;
+		if (rc > 0)
+			return 0;
+	}
+
+	/* If there is no strategy routine, or if the strategy returns
+	 * zero, proceed with automatic r/w */
+	if (table->data && table->maxlen) {
+		rc = sysctl_data(table, name, nlen, oldval, oldlenp,
+				 newval, newlen);
+		if (rc < 0)
+			return rc;
+	}
+	return 0;
+}
+
+static int parse_table(int __user *name, int nlen,
+		       void __user *oldval, size_t __user *oldlenp,
+		       void __user *newval, size_t newlen,
+		       struct ctl_table *table)
+{
+	int n;
+repeat:
+	if (!nlen)
+		return -ENOTDIR;
+	if (get_user(n, name))
+		return -EFAULT;
+	for ( ; table->ctl_name || table->procname; table++) {
+		if (!table->ctl_name)
+			continue;
+		if (n == table->ctl_name) {
+			int error;
+			if (table->child) {
+				if (sysctl_perm(table, 001))
+					return -EPERM;
+				name++;
+				nlen--;
+				table = table->child;
+				goto repeat;
+			}
+			error = do_sysctl_strategy(table, name, nlen,
+						   oldval, oldlenp,
+						   newval, newlen);
+			return error;
+		}
+	}
+	return -ENOTDIR;
+}
+
 int do_sysctl(int __user *name, int nlen, void __user *oldval, size_t __user *oldlenp,
 	       void __user *newval, size_t newlen)
 {
@@ -1531,76 +1593,6 @@ int sysctl_perm(struct ctl_table *table, int op)
 	return test_perm(table->mode, op);
 }
 
-#ifdef CONFIG_SYSCTL_SYSCALL
-static int parse_table(int __user *name, int nlen,
-		       void __user *oldval, size_t __user *oldlenp,
-		       void __user *newval, size_t newlen,
-		       struct ctl_table *table)
-{
-	int n;
-repeat:
-	if (!nlen)
-		return -ENOTDIR;
-	if (get_user(n, name))
-		return -EFAULT;
-	for ( ; table->ctl_name || table->procname; table++) {
-		if (!table->ctl_name)
-			continue;
-		if (n == table->ctl_name) {
-			int error;
-			if (table->child) {
-				if (sysctl_perm(table, 001))
-					return -EPERM;
-				name++;
-				nlen--;
-				table = table->child;
-				goto repeat;
-			}
-			error = do_sysctl_strategy(table, name, nlen,
-						   oldval, oldlenp,
-						   newval, newlen);
-			return error;
-		}
-	}
-	return -ENOTDIR;
-}
-
-/* Perform the actual read/write of a sysctl table entry. */
-int do_sysctl_strategy (struct ctl_table *table,
-			int __user *name, int nlen,
-			void __user *oldval, size_t __user *oldlenp,
-			void __user *newval, size_t newlen)
-{
-	int op = 0, rc;
-
-	if (oldval)
-		op |= 004;
-	if (newval) 
-		op |= 002;
-	if (sysctl_perm(table, op))
-		return -EPERM;
-
-	if (table->strategy) {
-		rc = table->strategy(table, name, nlen, oldval, oldlenp,
-				     newval, newlen);
-		if (rc < 0)
-			return rc;
-		if (rc > 0)
-			return 0;
-	}
-
-	/* If there is no strategy routine, or if the strategy returns
-	 * zero, proceed with automatic r/w */
-	if (table->data && table->maxlen) {
-		rc = sysctl_data(table, name, nlen, oldval, oldlenp,
-				 newval, newlen);
-		if (rc < 0)
-			return rc;
-	}
-	return 0;
-}
-#endif /* CONFIG_SYSCTL_SYSCALL */
-
 static void sysctl_set_parent(struct ctl_table *parent, struct ctl_table *table)
 {
 	for (; table->ctl_name || table->procname; table++) {
-- 
1.5.3.4


^ permalink raw reply related

* [PATCH net-2.6.26 1/5][SYSCTL]: Merge equal code in sysctl proc handlers.
From: Pavel Emelyanov @ 2008-02-19 11:56 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <47BAC38F.10100@openvz.org>

The ->read and ->write callbacks act in a very similar way, so
merge these paths to reduce the number of places to patch later.

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

---
 fs/proc/proc_sysctl.c |   50 ++++++++++--------------------------------------
 1 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 614c34b..5e31585 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -165,8 +165,8 @@ out:
 	return err;
 }
 
-static ssize_t proc_sys_read(struct file *filp, char __user *buf,
-				size_t count, loff_t *ppos)
+static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
+		size_t count, loff_t *ppos, int write)
 {
 	struct dentry *dentry = filp->f_dentry;
 	struct ctl_table_header *head;
@@ -190,12 +190,12 @@ static ssize_t proc_sys_read(struct file *filp, char __user *buf,
 	 * and won't be until we finish.
 	 */
 	error = -EPERM;
-	if (sysctl_perm(table, MAY_READ))
+	if (sysctl_perm(table, write ? MAY_WRITE : MAY_READ))
 		goto out;
 
 	/* careful: calling conventions are nasty here */
 	res = count;
-	error = table->proc_handler(table, 0, filp, buf, &res, ppos);
+	error = table->proc_handler(table, write, filp, buf, &res, ppos);
 	if (!error)
 		error = res;
 out:
@@ -204,44 +204,16 @@ out:
 	return error;
 }
 
-static ssize_t proc_sys_write(struct file *filp, const char __user *buf,
+static ssize_t proc_sys_read(struct file *filp, char __user *buf,
 				size_t count, loff_t *ppos)
 {
-	struct dentry *dentry = filp->f_dentry;
-	struct ctl_table_header *head;
-	struct ctl_table *table;
-	ssize_t error;
-	size_t res;
-
-	table = do_proc_sys_lookup(dentry->d_parent, &dentry->d_name, &head);
-	/* Has the sysctl entry disappeared on us? */
-	error = -ENOENT;
-	if (!table)
-		goto out;
-
-	/* Has the sysctl entry been replaced by a directory? */
-	error = -EISDIR;
-	if (!table->proc_handler)
-		goto out;
-
-	/*
-	 * At this point we know that the sysctl was not unregistered
-	 * and won't be until we finish.
-	 */
-	error = -EPERM;
-	if (sysctl_perm(table, MAY_WRITE))
-		goto out;
-
-	/* careful: calling conventions are nasty here */
-	res = count;
-	error = table->proc_handler(table, 1, filp, (char __user *)buf,
-				    &res, ppos);
-	if (!error)
-		error = res;
-out:
-	sysctl_head_finish(head);
+	return proc_sys_call_handler(filp, (void __user *)buf, count, ppos, 0);
+}
 
-	return error;
+static ssize_t proc_sys_write(struct file *filp, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	return proc_sys_call_handler(filp, (void __user *)buf, count, ppos, 1);
 }
 
 
-- 
1.5.3.4


^ permalink raw reply related

* Re: [Bugme-new] [Bug 9920] New: kernel panic when using ebtables redirect target
From: Patrick McHardy @ 2008-02-19 11:56 UTC (permalink / raw)
  To: David Miller
  Cc: joonwpark81, akpm, netfilter-devel, netdev, bugme-daemon,
	mingching.tiew
In-Reply-To: <20080218.205317.62981417.davem@davemloft.net>

David Miller wrote:
> From: Joonwoo Park <joonwpark81@gmail.com>
> Date: Tue, 19 Feb 2008 11:53:24 +0900
> 
>> [PATCH] netfilter: fix incorrect use of skb_make_writable
>>
>> http://bugzilla.kernel.org/show_bug.cgi?id=9920
>> The function skb_make_writable returns true or false.
>>
>> Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
> 
> I'll let Patrick pull this in, thanks!


Applied, thanks.

^ permalink raw reply

* [PATCH net-2.6.26 0/5][SYSCTL]: Make some sysctl RO in net namespaces.
From: Pavel Emelyanov @ 2008-02-19 11:54 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List

Hi, David.

Some time ago, when I made the net.core.somaxconn ctl per-namespace,
you told that the approach I used to make some ctl tables read-only
in namespace was not very good and said to improve it. After looking
at other code, I decided, that many ctl variables will have to be 
read-only in namespace, so we need some generic way to do this.

So, here's the patchset, that allows to create ctl tables, that are
read-only in some namespace in general (and in some net namespace in 
particular). I tried to make it work the way not to consume extra
memory at run time.

This patchset is related to net namespaces only, but on the other hand
it affects the core sysctl engine. What is your opinion about this set:
should I send these patches (or some of them) to Andrew instead and wait 
till it appears in mainline (and sequentially in net tree) or will you
accept this one in net-2.6.26?

Thanks,
Pavel


^ permalink raw reply

* Re: [PATCH] cls_u32 u32_classify()
From: jamal @ 2008-02-19 11:54 UTC (permalink / raw)
  To: David Miller; +Cc: mahatma, mahatma, netdev
In-Reply-To: <20080218.214600.94322068.davem@davemloft.net>

On Mon, 2008-18-02 at 21:46 -0800, David Miller wrote:


> Can some u32 expert review this?

http://marc.info/?l=linux-netdev&m=120178638323045&w=2

cheers,
jamal


^ permalink raw reply

* Re: [PATCH] net/8021q/vlan_dev.c - Use print_mac
From: Patrick McHardy @ 2008-02-19 11:48 UTC (permalink / raw)
  To: David Miller; +Cc: joe, bruno, netdev, jgarzik, linux-wireless, linville
In-Reply-To: <20080218.165036.218650084.davem@davemloft.net>

David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 18 Feb 2008 16:43:05 -0800 (PST)
> 
>> I think we can fix this easily by using __attribute_const_
>> on the print_mac() declaration.  Let me play with that.
> 
> Actually it seems the 'pure' attribute is more important
> here.  Although it's not semantically a perfect match,
> what we need to tell the compiler is basically that:
> 
> 1) the return value depends upon the inputs
> 2) if the input is not used, it's safe to avoid the call
> 
> and 'pure' accomplishes that without any unwanted side-effects.
> 
> I think this will not result in any unwanted over-optimization.
> Because if the inputs change in any way GCC has to emit the
> call.
> 
> Any objections?


This seems fine to me, thanks Dave.

^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: Jarek Poplawski @ 2008-02-19 10:36 UTC (permalink / raw)
  To: James Chapman; +Cc: David Miller, netdev
In-Reply-To: <20080219103047.GA3898@ff.dom.local>

On Tue, Feb 19, 2008 at 10:30:47AM +0000, Jarek Poplawski wrote:
...
> IMHO, just like I wrote earlier, the main problem is in ppp_generic(),

...or maybe ppp_generic.c? Whatever...

Jarek P.

^ permalink raw reply

* Re: [PATCH 6/17 net-2.6.26] [NETNS]: Default arp parameters lookup.
From: Daniel Lezcano @ 2008-02-19 10:22 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: davem, netdev, containers, devel
In-Reply-To: <1203415549.27296.16.camel@iris.sw.ru>

Denis V. Lunev wrote:
> On Tue, 2008-02-19 at 10:51 +0100, Daniel Lezcano wrote:
>> Denis V. Lunev wrote:
>>> On Tue, 2008-02-19 at 10:14 +0100, Daniel Lezcano wrote:
>>>> Denis V. Lunev wrote:
>>>>> Default ARP parameters should be findable regardless of the context.
>>>>> Required to make inetdev_event working.
>>>>>
>>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>>> ---
>>>>>  net/core/neighbour.c |    4 +---
>>>>>  1 files changed, 1 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>>>>> index c895ad4..45ed620 100644
>>>>> --- a/net/core/neighbour.c
>>>>> +++ b/net/core/neighbour.c
>>>>> @@ -1275,9 +1275,7 @@ static inline struct neigh_parms *lookup_neigh_params(struct neigh_table *tbl,
>>>>>  	struct neigh_parms *p;
>>>>>
>>>>>  	for (p = &tbl->parms; p; p = p->next) {
>>>>> -		if (p->net != net)
>>>>> -			continue;
>>>>> -		if ((p->dev && p->dev->ifindex == ifindex) ||
>>>>> +		if ((p->dev && p->dev->ifindex == ifindex && p->net == net) ||
>>>>>  		    (!p->dev && !ifindex))
>>>>>  			return p;
>>>>>  	}
>>>> If the values are:
>>>> 	p->dev == NULL
>>>> 	ifindex == 0
>>>> 	p->net != net
>>>>
>>>> The parms should not be taken into account and the looping must 
>>>> continue. But with this modification it is not the case, if we specify 
>>>> parms ifindex == 0, the first parms with the dev field set to NULL will 
>>>> be taken belonging or not to the right net.
>>> They should be taken. In the other case inetdev_event will fail for sure
>>> in the middle. You could check.
>>>
>>> These are ARP defaults and I do not see a problem for now to get them.
>> Because there is a parms default per namespace. So several instances of 
>> them per nd table. That was the initial approach with Eric's patchset.
>>
> 
> These changes are not in mainstream and I do not want to touch ARP as
> this is not a simple thing. In reality ARP will be needed only when
> we'll have a real device inside a namespace.
> 
> Right now I prefer to have minimal set of working changes to finish IP
> and upper layers.

core/neighbour.c is a common part between several protocols, especially 
ipv4 and ipv6. If you modify this function just to fit your need in the 
arp that will block me for ipv6 until you make parms default per 
namespace. So please, find another way to do that, perhaps just add a 
helper function.

I suggest you do parms default per namespace first, it is quite small 
and easy :)

Just let me the time to send the copy-parms-default function.

Is it ok ?

   -- Daniel



  -- Daniel

^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: Jarek Poplawski @ 2008-02-19 10:30 UTC (permalink / raw)
  To: James Chapman; +Cc: David Miller, netdev
In-Reply-To: <47BA9B50.8040404@katalix.com>

On Tue, Feb 19, 2008 at 09:03:12AM +0000, James Chapman wrote:
> David Miller wrote:
>> From: James Chapman <jchapman@katalix.com>
>> Date: Mon, 18 Feb 2008 22:09:24 +0000
>>
>>> Here's a new version of the patch. The patch avoids disabling irqs
>>> and fixes the sk_dst_get() usage that DaveM mentioned. But even with
>>> this patch, lockdep still complains if hundreds of ppp sessions are
>>> inserted into a tunnel as rapidly as possible (lockdep trace is
>>> below). I can stop these errors by wrapping the call to ppp_input()
>>> in pppol2tp_recv_dequeue_skb() with local_irq_save/restore. What is
>>> a better fix?
>>
>> Firstly, let's fix one thing at a time.  Leave the sk_dst_get()
>> thing alone until we can prove that it's part of the lockdep
>> traces.
>
> In reproducing the problem, I obtained several lockdep traces that  
> implicated sk_dst_get().

As a matter of fact I missed just that kind information on previous
lockdep report, so if you could send them too this should be still
helpful.

...
> I agree. I'm seeking advice on what the underlying cause is of this new  
> trace.

IMHO, just like I wrote earlier, the main problem is in ppp_generic(),
especially ppp_connect_channel(), where main tx & rx locks are used.
I didn't know enough about this sk_dst_lock traces yet. I hope I could
help with this, but after these changes I need some time to figure
this out again.

Jarek P.

^ permalink raw reply

* Re: [PATCH 6/17 net-2.6.26] [NETNS]: Default arp parameters lookup.
From: Denis V. Lunev @ 2008-02-19 10:05 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: davem, netdev, containers, devel
In-Reply-To: <47BAA690.2010504@fr.ibm.com>

On Tue, 2008-02-19 at 10:51 +0100, Daniel Lezcano wrote:
> Denis V. Lunev wrote:
> > On Tue, 2008-02-19 at 10:14 +0100, Daniel Lezcano wrote:
> >> Denis V. Lunev wrote:
> >>> Default ARP parameters should be findable regardless of the context.
> >>> Required to make inetdev_event working.
> >>>
> >>> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >>> ---
> >>>  net/core/neighbour.c |    4 +---
> >>>  1 files changed, 1 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> >>> index c895ad4..45ed620 100644
> >>> --- a/net/core/neighbour.c
> >>> +++ b/net/core/neighbour.c
> >>> @@ -1275,9 +1275,7 @@ static inline struct neigh_parms *lookup_neigh_params(struct neigh_table *tbl,
> >>>  	struct neigh_parms *p;
> >>>
> >>>  	for (p = &tbl->parms; p; p = p->next) {
> >>> -		if (p->net != net)
> >>> -			continue;
> >>> -		if ((p->dev && p->dev->ifindex == ifindex) ||
> >>> +		if ((p->dev && p->dev->ifindex == ifindex && p->net == net) ||
> >>>  		    (!p->dev && !ifindex))
> >>>  			return p;
> >>>  	}
> >> If the values are:
> >> 	p->dev == NULL
> >> 	ifindex == 0
> >> 	p->net != net
> >>
> >> The parms should not be taken into account and the looping must 
> >> continue. But with this modification it is not the case, if we specify 
> >> parms ifindex == 0, the first parms with the dev field set to NULL will 
> >> be taken belonging or not to the right net.
> > 
> > They should be taken. In the other case inetdev_event will fail for sure
> > in the middle. You could check.
> > 
> > These are ARP defaults and I do not see a problem for now to get them.
> 
> Because there is a parms default per namespace. So several instances of 
> them per nd table. That was the initial approach with Eric's patchset.
> 

These changes are not in mainstream and I do not want to touch ARP as
this is not a simple thing. In reality ARP will be needed only when
we'll have a real device inside a namespace.

Right now I prefer to have minimal set of working changes to finish IP
and upper layers.

Regards,
	Den


^ permalink raw reply

* Re: [PATCH 6/17 net-2.6.26] [NETNS]: Default arp parameters lookup.
From: Daniel Lezcano @ 2008-02-19  9:51 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: davem, netdev, containers, devel
In-Reply-To: <1203413980.27296.9.camel@iris.sw.ru>

Denis V. Lunev wrote:
> On Tue, 2008-02-19 at 10:14 +0100, Daniel Lezcano wrote:
>> Denis V. Lunev wrote:
>>> Default ARP parameters should be findable regardless of the context.
>>> Required to make inetdev_event working.
>>>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> ---
>>>  net/core/neighbour.c |    4 +---
>>>  1 files changed, 1 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
>>> index c895ad4..45ed620 100644
>>> --- a/net/core/neighbour.c
>>> +++ b/net/core/neighbour.c
>>> @@ -1275,9 +1275,7 @@ static inline struct neigh_parms *lookup_neigh_params(struct neigh_table *tbl,
>>>  	struct neigh_parms *p;
>>>
>>>  	for (p = &tbl->parms; p; p = p->next) {
>>> -		if (p->net != net)
>>> -			continue;
>>> -		if ((p->dev && p->dev->ifindex == ifindex) ||
>>> +		if ((p->dev && p->dev->ifindex == ifindex && p->net == net) ||
>>>  		    (!p->dev && !ifindex))
>>>  			return p;
>>>  	}
>> If the values are:
>> 	p->dev == NULL
>> 	ifindex == 0
>> 	p->net != net
>>
>> The parms should not be taken into account and the looping must 
>> continue. But with this modification it is not the case, if we specify 
>> parms ifindex == 0, the first parms with the dev field set to NULL will 
>> be taken belonging or not to the right net.
> 
> They should be taken. In the other case inetdev_event will fail for sure
> in the middle. You could check.
> 
> These are ARP defaults and I do not see a problem for now to get them.

Because there is a parms default per namespace. So several instances of 
them per nd table. That was the initial approach with Eric's patchset.


^ permalink raw reply

* Re: [PATCH 6/17 net-2.6.26] [NETNS]: Default arp parameters lookup.
From: Denis V. Lunev @ 2008-02-19  9:39 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: davem, netdev, containers, devel
In-Reply-To: <47BA9E0B.6080209@fr.ibm.com>

On Tue, 2008-02-19 at 10:14 +0100, Daniel Lezcano wrote:
> Denis V. Lunev wrote:
> > Default ARP parameters should be findable regardless of the context.
> > Required to make inetdev_event working.
> > 
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > ---
> >  net/core/neighbour.c |    4 +---
> >  1 files changed, 1 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index c895ad4..45ed620 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -1275,9 +1275,7 @@ static inline struct neigh_parms *lookup_neigh_params(struct neigh_table *tbl,
> >  	struct neigh_parms *p;
> > 
> >  	for (p = &tbl->parms; p; p = p->next) {
> > -		if (p->net != net)
> > -			continue;
> > -		if ((p->dev && p->dev->ifindex == ifindex) ||
> > +		if ((p->dev && p->dev->ifindex == ifindex && p->net == net) ||
> >  		    (!p->dev && !ifindex))
> >  			return p;
> >  	}
> 
> If the values are:
> 	p->dev == NULL
> 	ifindex == 0
> 	p->net != net
> 
> The parms should not be taken into account and the looping must 
> continue. But with this modification it is not the case, if we specify 
> parms ifindex == 0, the first parms with the dev field set to NULL will 
> be taken belonging or not to the right net.

They should be taken. In the other case inetdev_event will fail for sure
in the middle. You could check.

These are ARP defaults and I do not see a problem for now to get them.


^ permalink raw reply

* Re: [PATCH 6/17 net-2.6.26] [NETNS]: Default arp parameters lookup.
From: Daniel Lezcano @ 2008-02-19  9:14 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: davem, netdev, containers, devel
In-Reply-To: <1203406297-32725-6-git-send-email-den@openvz.org>

Denis V. Lunev wrote:
> Default ARP parameters should be findable regardless of the context.
> Required to make inetdev_event working.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  net/core/neighbour.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index c895ad4..45ed620 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1275,9 +1275,7 @@ static inline struct neigh_parms *lookup_neigh_params(struct neigh_table *tbl,
>  	struct neigh_parms *p;
> 
>  	for (p = &tbl->parms; p; p = p->next) {
> -		if (p->net != net)
> -			continue;
> -		if ((p->dev && p->dev->ifindex == ifindex) ||
> +		if ((p->dev && p->dev->ifindex == ifindex && p->net == net) ||
>  		    (!p->dev && !ifindex))
>  			return p;
>  	}

If the values are:
	p->dev == NULL
	ifindex == 0
	p->net != net

The parms should not be taken into account and the looping must 
continue. But with this modification it is not the case, if we specify 
parms ifindex == 0, the first parms with the dev field set to NULL will 
be taken belonging or not to the right net.

IMO the right test is:

if (p->net == net && ((p->dev && p->dev->ifindex == ifindex) || !p->dev 
&& !ifindex)))

I definitively prefer the first notation :)

 > -		if (p->net != net)
 > -			continue;


^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: James Chapman @ 2008-02-19  9:09 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: David Miller, netdev
In-Reply-To: <20080218230154.GB6373@ami.dom.local>

Jarek Poplawski wrote:
> On Mon, Feb 18, 2008 at 10:09:24PM +0000, James Chapman wrote:
>> Jarek Poplawski wrote:
>>> Hi,
>>>
>>> It seems, this nice report is still uncomplete: could you check if
>>> there could have been something more yet?
>> Unfortunately the ISP's syslog stops. But I've been able to borrow two
>> Quad Xeon boxes and have reproduced the problem.
>>
>> Here's a new version of the patch. The patch avoids disabling irqs and
>> fixes the sk_dst_get() usage that DaveM mentioned. But even with this
>> patch, lockdep still complains if hundreds of ppp sessions are inserted
>> into a tunnel as rapidly as possible (lockdep trace is below). I can
>> stop these errors by wrapping the call to ppp_input() in
>> pppol2tp_recv_dequeue_skb() with local_irq_save/restore. What is a
>> better fix?
> 
> Hmm... This is a really long report and quite a bit different from
> the previous one. I need some time for this. BTW: you sent before a
> lockdep report with hlist_lock problem. I think this could be fixed
> in some independent patch to make this all more readable. Are all
> the other changes in this current patch only because of this or
> previous lockdep report or for some other reasons (or reports) yet?

As I mentioned in my reply to davem, modifying the pppol2tp driver as 
described in the patch I sent made the original lockdep problems go away.


-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development


^ 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