Netdev List
 help / color / mirror / Atom feed
* [PATCH 10/17 net-2.6.26] [NETNS]: Process ip_rt_redirect in the correct namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/ipv4/route.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 525787b..44708ab 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1132,10 +1132,12 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 	__be32  skeys[2] = { saddr, 0 };
 	int  ikeys[2] = { dev->ifindex, 0 };
 	struct netevent_redirect netevent;
+	struct net *net;
 
 	if (!in_dev)
 		return;
 
+	net = dev->nd_net;
 	if (new_gw == old_gw || !IN_DEV_RX_REDIRECTS(in_dev)
 	    || ipv4_is_multicast(new_gw) || ipv4_is_lbcast(new_gw)
 	    || ipv4_is_zeronet(new_gw))
@@ -1147,7 +1149,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 		if (IN_DEV_SEC_REDIRECTS(in_dev) && ip_fib_check_default(new_gw, dev))
 			goto reject_redirect;
 	} else {
-		if (inet_addr_type(&init_net, new_gw) != RTN_UNICAST)
+		if (inet_addr_type(net, new_gw) != RTN_UNICAST)
 			goto reject_redirect;
 	}
 
@@ -1165,7 +1167,8 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 				    rth->fl.fl4_src != skeys[i] ||
 				    rth->fl.oif != ikeys[k] ||
 				    rth->fl.iif != 0 ||
-				    rth->rt_genid != atomic_read(&rt_genid)) {
+				    rth->rt_genid != atomic_read(&rt_genid) ||
+				    rth->u.dst.dev->nd_net != net) {
 					rthp = &rth->u.dst.rt_next;
 					continue;
 				}
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 12/17 net-2.6.26] [NETNS]: Process /proc/net/rt_cache inside a namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

Show routing cache for a particular namespace only.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/ipv4/route.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 67df872..c11e6bf 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -273,6 +273,7 @@ static unsigned int rt_hash_code(u32 daddr, u32 saddr)
 
 #ifdef CONFIG_PROC_FS
 struct rt_cache_iter_state {
+	struct seq_net_private p;
 	int bucket;
 	int genid;
 };
@@ -285,7 +286,8 @@ static struct rtable *rt_cache_get_first(struct rt_cache_iter_state *st)
 		rcu_read_lock_bh();
 		r = rcu_dereference(rt_hash_table[st->bucket].chain);
 		while (r) {
-			if (r->rt_genid == st->genid)
+			if (r->u.dst.dev->nd_net == st->p.net &&
+			    r->rt_genid == st->genid)
 				return r;
 			r = rcu_dereference(r->u.dst.rt_next);
 		}
@@ -312,6 +314,8 @@ static struct rtable *rt_cache_get_next(struct rt_cache_iter_state *st,
 					struct rtable *r)
 {
 	while ((r = __rt_cache_get_next(st, r)) != NULL) {
+		if (r->u.dst.dev->nd_net != st->p.net)
+			continue;
 		if (r->rt_genid == st->genid)
 			break;
 	}
@@ -398,7 +402,7 @@ static const struct seq_operations rt_cache_seq_ops = {
 
 static int rt_cache_seq_open(struct inode *inode, struct file *file)
 {
-	return seq_open_private(file, &rt_cache_seq_ops,
+	return seq_open_net(inode, file, &rt_cache_seq_ops,
 			sizeof(struct rt_cache_iter_state));
 }
 
@@ -407,7 +411,7 @@ static const struct file_operations rt_cache_seq_fops = {
 	.open	 = rt_cache_seq_open,
 	.read	 = seq_read,
 	.llseek	 = seq_lseek,
-	.release = seq_release_private,
+	.release = seq_release_net,
 };
 
 
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 13/17 net-2.6.26] [NETNS]: Register /proc/net/rt_cache for each namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/ipv4/route.c |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index c11e6bf..5f67eba 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -545,7 +545,7 @@ static int ip_rt_acct_read(char *buffer, char **start, off_t offset,
 }
 #endif
 
-static __init int ip_rt_proc_init(struct net *net)
+static int __net_init ip_rt_do_proc_init(struct net *net)
 {
 	struct proc_dir_entry *pde;
 
@@ -577,8 +577,26 @@ err2:
 err1:
 	return -ENOMEM;
 }
+
+static void __net_exit ip_rt_do_proc_exit(struct net *net)
+{
+	remove_proc_entry("rt_cache", net->proc_net_stat);
+	remove_proc_entry("rt_cache", net->proc_net);
+	remove_proc_entry("rt_acct", net->proc_net);
+}
+
+static struct pernet_operations ip_rt_proc_ops __net_initdata =  {
+	.init = ip_rt_do_proc_init,
+	.exit = ip_rt_do_proc_exit,
+};
+
+static int __init ip_rt_proc_init(void)
+{
+	return register_pernet_subsys(&ip_rt_proc_ops);
+}
+
 #else
-static inline int ip_rt_proc_init(struct net *net)
+static inline int ip_rt_proc_init(void)
 {
 	return 0;
 }
@@ -3056,7 +3074,7 @@ int __init ip_rt_init(void)
 		ip_rt_secret_interval;
 	add_timer(&rt_secret_timer);
 
-	if (ip_rt_proc_init(&init_net))
+	if (ip_rt_proc_init())
 		printk(KERN_ERR "Unable to create route proc files\n");
 #ifdef CONFIG_XFRM
 	xfrm_init();
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 9/17 net-2.6.26] [NETNS]: DST cleanup routines should be called inside namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

Device inside the namespace can be started and downed. So, active routing
cache should be cleaned up on device stop.

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

diff --git a/net/core/dst.c b/net/core/dst.c
index 7deef48..3a01a81 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -295,9 +295,6 @@ static int dst_dev_event(struct notifier_block *this, unsigned long event, void
 	struct net_device *dev = ptr;
 	struct dst_entry *dst, *last = NULL;
 
-	if (dev->nd_net != &init_net)
-		return NOTIFY_DONE;
-
 	switch (event) {
 	case NETDEV_UNREGISTER:
 	case NETDEV_DOWN:
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 11/17 net-2.6.26] [IPV4]: rt_cache_get_next should take rt_genid into account.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

In the other case /proc/net/rt_cache will look inconsistent in respect to
genid.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Acked-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
---
 net/ipv4/route.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 44708ab..67df872 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -294,7 +294,8 @@ static struct rtable *rt_cache_get_first(struct rt_cache_iter_state *st)
 	return r;
 }
 
-static struct rtable *rt_cache_get_next(struct rt_cache_iter_state *st, struct rtable *r)
+static struct rtable *__rt_cache_get_next(struct rt_cache_iter_state *st,
+					  struct rtable *r)
 {
 	r = r->u.dst.rt_next;
 	while (!r) {
@@ -307,16 +308,23 @@ static struct rtable *rt_cache_get_next(struct rt_cache_iter_state *st, struct r
 	return rcu_dereference(r);
 }
 
+static struct rtable *rt_cache_get_next(struct rt_cache_iter_state *st,
+					struct rtable *r)
+{
+	while ((r = __rt_cache_get_next(st, r)) != NULL) {
+		if (r->rt_genid == st->genid)
+			break;
+	}
+	return r;
+}
+
 static struct rtable *rt_cache_get_idx(struct rt_cache_iter_state *st, loff_t pos)
 {
 	struct rtable *r = rt_cache_get_first(st);
 
 	if (r)
-		while (pos && (r = rt_cache_get_next(st, r))) {
-			if (r->rt_genid != st->genid)
-				continue;
+		while (pos && (r = rt_cache_get_next(st, r)))
 			--pos;
-		}
 	return pos ? NULL : r;
 }
 
-- 
1.5.3.rc5


^ permalink raw reply related

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

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;
 	}
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 7/17 net-2.6.26] [NETNS]: Disable multicaststing configuration inside non-initial namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

Do not calls hooks from device notifiers and disallow configuration from
ioctl/netlink layer.

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

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 732cd07..d3f34a7 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1198,6 +1198,9 @@ void ip_mc_inc_group(struct in_device *in_dev, __be32 addr)
 
 	ASSERT_RTNL();
 
+	if (in_dev->dev->nd_net != &init_net)
+		return;
+
 	for (im=in_dev->mc_list; im; im=im->next) {
 		if (im->multiaddr == addr) {
 			im->users++;
@@ -1277,6 +1280,9 @@ void ip_mc_dec_group(struct in_device *in_dev, __be32 addr)
 
 	ASSERT_RTNL();
 
+	if (in_dev->dev->nd_net != &init_net)
+		return;
+
 	for (ip=&in_dev->mc_list; (i=*ip)!=NULL; ip=&i->next) {
 		if (i->multiaddr==addr) {
 			if (--i->users == 0) {
@@ -1304,6 +1310,9 @@ void ip_mc_down(struct in_device *in_dev)
 
 	ASSERT_RTNL();
 
+	if (in_dev->dev->nd_net != &init_net)
+		return;
+
 	for (i=in_dev->mc_list; i; i=i->next)
 		igmp_group_dropped(i);
 
@@ -1324,6 +1333,9 @@ void ip_mc_init_dev(struct in_device *in_dev)
 {
 	ASSERT_RTNL();
 
+	if (in_dev->dev->nd_net != &init_net)
+		return;
+
 	in_dev->mc_tomb = NULL;
 #ifdef CONFIG_IP_MULTICAST
 	in_dev->mr_gq_running = 0;
@@ -1347,6 +1359,9 @@ void ip_mc_up(struct in_device *in_dev)
 
 	ASSERT_RTNL();
 
+	if (in_dev->dev->nd_net != &init_net)
+		return;
+
 	ip_mc_inc_group(in_dev, IGMP_ALL_HOSTS);
 
 	for (i=in_dev->mc_list; i; i=i->next)
@@ -1363,6 +1378,9 @@ void ip_mc_destroy_dev(struct in_device *in_dev)
 
 	ASSERT_RTNL();
 
+	if (in_dev->dev->nd_net != &init_net)
+		return;
+
 	/* Deactivate timers */
 	ip_mc_down(in_dev);
 
@@ -1744,6 +1762,9 @@ int ip_mc_join_group(struct sock *sk , struct ip_mreqn *imr)
 	if (!ipv4_is_multicast(addr))
 		return -EINVAL;
 
+	if (sk->sk_net != &init_net)
+		return -EPROTONOSUPPORT;
+
 	rtnl_lock();
 
 	in_dev = ip_mc_find_dev(imr);
@@ -1812,6 +1833,9 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
 	u32 ifindex;
 	int ret = -EADDRNOTAVAIL;
 
+	if (sk->sk_net != &init_net)
+		return -EPROTONOSUPPORT;
+
 	rtnl_lock();
 	in_dev = ip_mc_find_dev(imr);
 	ifindex = imr->imr_ifindex;
@@ -1857,6 +1881,9 @@ int ip_mc_source(int add, int omode, struct sock *sk, struct
 	if (!ipv4_is_multicast(addr))
 		return -EINVAL;
 
+	if (sk->sk_net != &init_net)
+		return -EPROTONOSUPPORT;
+
 	rtnl_lock();
 
 	imr.imr_multiaddr.s_addr = mreqs->imr_multiaddr;
@@ -1990,6 +2017,9 @@ int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf, int ifindex)
 	    msf->imsf_fmode != MCAST_EXCLUDE)
 		return -EINVAL;
 
+	if (sk->sk_net != &init_net)
+		return -EPROTONOSUPPORT;
+
 	rtnl_lock();
 
 	imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
@@ -2070,6 +2100,9 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 	if (!ipv4_is_multicast(addr))
 		return -EINVAL;
 
+	if (sk->sk_net != &init_net)
+		return -EPROTONOSUPPORT;
+
 	rtnl_lock();
 
 	imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
@@ -2132,6 +2165,9 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 	if (!ipv4_is_multicast(addr))
 		return -EINVAL;
 
+	if (sk->sk_net != &init_net)
+		return -EPROTONOSUPPORT;
+
 	rtnl_lock();
 
 	err = -EADDRNOTAVAIL;
@@ -2216,6 +2252,9 @@ void ip_mc_drop_socket(struct sock *sk)
 	if (inet->mc_list == NULL)
 		return;
 
+	if (sk->sk_net != &init_net)
+		return;
+
 	rtnl_lock();
 	while ((iml = inet->mc_list) != NULL) {
 		struct in_device *in_dev;
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 14/17 net-2.6.26] [NETNS]: Process devinet ioctl in the correct namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

Add namespace parameter to devinet_ioctl and locate device inside it for
state changes.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 include/linux/inetdevice.h |    2 +-
 net/ipv4/af_inet.c         |    7 ++++---
 net/ipv4/devinet.c         |    6 +++---
 net/ipv4/ipconfig.c        |    2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index fc4e3db..da05ab4 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -129,7 +129,7 @@ extern int unregister_inetaddr_notifier(struct notifier_block *nb);
 
 extern struct net_device *ip_dev_find(struct net *net, __be32 addr);
 extern int		inet_addr_onlink(struct in_device *in_dev, __be32 a, __be32 b);
-extern int		devinet_ioctl(unsigned int cmd, void __user *);
+extern int		devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
 extern void		devinet_init(void);
 extern struct in_device	*inetdev_by_index(struct net *, int);
 extern __be32		inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 09ca529..c270080 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -784,6 +784,7 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 {
 	struct sock *sk = sock->sk;
 	int err = 0;
+	struct net *net = sk->sk_net;
 
 	switch (cmd) {
 		case SIOCGSTAMP:
@@ -795,12 +796,12 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		case SIOCADDRT:
 		case SIOCDELRT:
 		case SIOCRTMSG:
-			err = ip_rt_ioctl(sk->sk_net, cmd, (void __user *)arg);
+			err = ip_rt_ioctl(net, cmd, (void __user *)arg);
 			break;
 		case SIOCDARP:
 		case SIOCGARP:
 		case SIOCSARP:
-			err = arp_ioctl(sk->sk_net, cmd, (void __user *)arg);
+			err = arp_ioctl(net, cmd, (void __user *)arg);
 			break;
 		case SIOCGIFADDR:
 		case SIOCSIFADDR:
@@ -813,7 +814,7 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		case SIOCSIFPFLAGS:
 		case SIOCGIFPFLAGS:
 		case SIOCSIFFLAGS:
-			err = devinet_ioctl(cmd, (void __user *)arg);
+			err = devinet_ioctl(net, cmd, (void __user *)arg);
 			break;
 		default:
 			if (sk->sk_prot->ioctl)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 963e711..f7e78b7 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -595,7 +595,7 @@ static __inline__ int inet_abc_len(__be32 addr)
 }
 
 
-int devinet_ioctl(unsigned int cmd, void __user *arg)
+int devinet_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 {
 	struct ifreq ifr;
 	struct sockaddr_in sin_orig;
@@ -624,7 +624,7 @@ int devinet_ioctl(unsigned int cmd, void __user *arg)
 		*colon = 0;
 
 #ifdef CONFIG_KMOD
-	dev_load(&init_net, ifr.ifr_name);
+	dev_load(net, ifr.ifr_name);
 #endif
 
 	switch (cmd) {
@@ -665,7 +665,7 @@ int devinet_ioctl(unsigned int cmd, void __user *arg)
 	rtnl_lock();
 
 	ret = -ENODEV;
-	if ((dev = __dev_get_by_name(&init_net, ifr.ifr_name)) == NULL)
+	if ((dev = __dev_get_by_name(net, ifr.ifr_name)) == NULL)
 		goto done;
 
 	if (colon)
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index a52b585..009d78f 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -291,7 +291,7 @@ static int __init ic_dev_ioctl(unsigned int cmd, struct ifreq *arg)
 
 	mm_segment_t oldfs = get_fs();
 	set_fs(get_ds());
-	res = devinet_ioctl(cmd, (struct ifreq __user *) arg);
+	res = devinet_ioctl(&init_net, cmd, (struct ifreq __user *) arg);
 	set_fs(oldfs);
 	return res;
 }
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 4/17 net-2.6.26] [NETNS]: Disable inetaddr notifiers in namespaces other than initial.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

ip_fib_init is kept enabled. It is already namespace-aware.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/net/bonding/bond_main.c |    3 +++
 drivers/net/via-velocity.c      |    3 +++
 drivers/s390/net/qeth_main.c    |    3 +++
 net/sctp/protocol.c             |    3 +++
 4 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0942d82..9666434 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3511,6 +3511,9 @@ static int bond_inetaddr_event(struct notifier_block *this, unsigned long event,
 	struct bonding *bond, *bond_next;
 	struct vlan_entry *vlan, *vlan_next;
 
+	if (ifa->ifa_dev->dev->nd_net != &init_net)
+		return NOTIFY_DONE;
+
 	list_for_each_entry_safe(bond, bond_next, &bond_dev_list, bond_list) {
 		if (bond->dev == event_dev) {
 			switch (event) {
diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index c50fdee..1525e8a 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -3464,6 +3464,9 @@ static int velocity_netdev_event(struct notifier_block *nb, unsigned long notifi
 	struct velocity_info *vptr;
 	unsigned long flags;
 
+	if (dev->nd_net != &init_net)
+		return NOTIFY_DONE;
+
 	spin_lock_irqsave(&velocity_dev_list_lock, flags);
 	list_for_each_entry(vptr, &velocity_dev_list, list) {
 		if (vptr->dev == dev) {
diff --git a/drivers/s390/net/qeth_main.c b/drivers/s390/net/qeth_main.c
index 62606ce..d063e9e 100644
--- a/drivers/s390/net/qeth_main.c
+++ b/drivers/s390/net/qeth_main.c
@@ -8622,6 +8622,9 @@ qeth_ip_event(struct notifier_block *this,
 	struct qeth_ipaddr *addr;
 	struct qeth_card *card;
 
+	if (dev->nd_net != &init_net)
+		return NOTIFY_DONE;
+
 	QETH_DBF_TEXT(trace,3,"ipevent");
 	card = qeth_get_card_from_dev(dev);
 	if (!card)
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 22a1657..4475f7e 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -629,6 +629,9 @@ static int sctp_inetaddr_event(struct notifier_block *this, unsigned long ev,
 	struct sctp_sockaddr_entry *addr = NULL;
 	struct sctp_sockaddr_entry *temp;
 
+	if (ifa->ifa_dev->dev->nd_net != &init_net)
+		return NOTIFY_DONE;
+
 	switch (ev) {
 	case NETDEV_UP:
 		addr = kmalloc(sizeof(struct sctp_sockaddr_entry), GFP_ATOMIC);
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 8/17 net-2.6.26] [NETNS]: Enable inetdev_event notifier.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

After all these preparations it is time to enable main IPv4 device
initialization routine inside namespace. It is safe do this now.

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

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index f282b26..963e711 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1044,9 +1044,6 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 	struct net_device *dev = ptr;
 	struct in_device *in_dev = __in_dev_get_rtnl(dev);
 
-	if (dev->nd_net != &init_net)
-		return NOTIFY_DONE;
-
 	ASSERT_RTNL();
 
 	if (!in_dev) {
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 15/17 net-2.6.26] [NETNS]: Enable all routing manipulation via netlink inside namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

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

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 5f67eba..79e2e8a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2702,9 +2702,6 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 	int err;
 	struct sk_buff *skb;
 
-	if (net != &init_net)
-		return -EINVAL;
-
 	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy);
 	if (err < 0)
 		goto errout;
@@ -2734,7 +2731,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 	if (iif) {
 		struct net_device *dev;
 
-		dev = __dev_get_by_index(&init_net, iif);
+		dev = __dev_get_by_index(net, iif);
 		if (dev == NULL) {
 			err = -ENODEV;
 			goto errout_free;
@@ -2760,7 +2757,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 			},
 			.oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0,
 		};
-		err = ip_route_output_key(&init_net, &rt, &fl);
+		err = ip_route_output_key(net, &rt, &fl);
 	}
 
 	if (err)
@@ -2771,11 +2768,11 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr* nlh, void
 		rt->rt_flags |= RTCF_NOTIFY;
 
 	err = rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq,
-				RTM_NEWROUTE, 0, 0);
+			   RTM_NEWROUTE, 0, 0);
 	if (err <= 0)
 		goto errout_free;
 
-	err = rtnl_unicast(skb, &init_net, NETLINK_CB(in_skb).pid);
+	err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).pid);
 errout:
 	return err;
 
@@ -2789,6 +2786,9 @@ int ip_rt_dump(struct sk_buff *skb,  struct netlink_callback *cb)
 	struct rtable *rt;
 	int h, s_h;
 	int idx, s_idx;
+	struct net *net;
+
+	net = skb->sk->sk_net;
 
 	s_h = cb->args[0];
 	if (s_h < 0)
@@ -2798,7 +2798,7 @@ int ip_rt_dump(struct sk_buff *skb,  struct netlink_callback *cb)
 		rcu_read_lock_bh();
 		for (rt = rcu_dereference(rt_hash_table[h].chain), idx = 0; rt;
 		     rt = rcu_dereference(rt->u.dst.rt_next), idx++) {
-			if (idx < s_idx)
+			if (rt->u.dst.dev->nd_net != net || idx < s_idx)
 				continue;
 			if (rt->rt_genid != atomic_read(&rt_genid))
 				continue;
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 5/17 net-2.6.26] [NETNS]: Register neighbour table parameters in the correct namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

neigh_sysctl_register should register sysctl entries inside correct namespace
to avoid naming conflict. Typical example is a loopback. Entries for it
present in all namespaces.

Required to make inetdev_event working.

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

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 7bb6a9a..c895ad4 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2732,7 +2732,8 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
 	neigh_path[NEIGH_CTL_PATH_PROTO].procname = p_name;
 	neigh_path[NEIGH_CTL_PATH_PROTO].ctl_name = p_id;
 
-	t->sysctl_header = register_sysctl_paths(neigh_path, t->neigh_vars);
+	t->sysctl_header =
+		register_net_sysctl_table(p->net, neigh_path, t->neigh_vars);
 	if (!t->sysctl_header)
 		goto free_procname;
 
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 17/17 net-2.6.26] [NETNS]: Process inet_select_addr inside a namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

The context is available from a network device passed in.

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

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index aa23d10..033670d 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -871,6 +871,7 @@ __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope)
 {
 	__be32 addr = 0;
 	struct in_device *in_dev;
+	struct net *net = dev->nd_net;
 
 	rcu_read_lock();
 	in_dev = __in_dev_get_rcu(dev);
@@ -899,7 +900,7 @@ no_in_dev:
 	 */
 	read_lock(&dev_base_lock);
 	rcu_read_lock();
-	for_each_netdev(&init_net, dev) {
+	for_each_netdev(net, dev) {
 		if ((in_dev = __in_dev_get_rcu(dev)) == NULL)
 			continue;
 
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 16/17 net-2.6.26] [NETNS]: Enable IPv4 address manipulations inside namespace.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

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

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index f7e78b7..aa23d10 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -446,9 +446,6 @@ static int inet_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg
 
 	ASSERT_RTNL();
 
-	if (net != &init_net)
-		return -EINVAL;
-
 	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_ipv4_policy);
 	if (err < 0)
 		goto errout;
@@ -560,9 +557,6 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg
 
 	ASSERT_RTNL();
 
-	if (net != &init_net)
-		return -EINVAL;
-
 	ifa = rtm_to_ifaddr(net, nlh);
 	if (IS_ERR(ifa))
 		return PTR_ERR(ifa);
@@ -1169,9 +1163,6 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	struct in_ifaddr *ifa;
 	int s_ip_idx, s_idx = cb->args[0];
 
-	if (net != &init_net)
-		return 0;
-
 	s_ip_idx = ip_idx = cb->args[1];
 	idx = 0;
 	for_each_netdev(net, dev) {
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 1/17 net-2.6.26] [IPV4]: Remove ifa != NULL check.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

This is a callback registered to inet address notifier chain.
The check is useless as:
- ifa is always != NULL
- similar checks are abscent in all other notifiers.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/net/via-velocity.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index cc0addb..c50fdee 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -3460,21 +3460,19 @@ static int velocity_resume(struct pci_dev *pdev)
 static int velocity_netdev_event(struct notifier_block *nb, unsigned long notification, void *ptr)
 {
 	struct in_ifaddr *ifa = (struct in_ifaddr *) ptr;
+	struct net_device *dev = ifa->ifa_dev->dev;
+	struct velocity_info *vptr;
+	unsigned long flags;
 
-	if (ifa) {
-		struct net_device *dev = ifa->ifa_dev->dev;
-		struct velocity_info *vptr;
-		unsigned long flags;
-
-		spin_lock_irqsave(&velocity_dev_list_lock, flags);
-		list_for_each_entry(vptr, &velocity_dev_list, list) {
-			if (vptr->dev == dev) {
-				velocity_get_ip(vptr);
-				break;
-			}
+	spin_lock_irqsave(&velocity_dev_list_lock, flags);
+	list_for_each_entry(vptr, &velocity_dev_list, list) {
+		if (vptr->dev == dev) {
+			velocity_get_ip(vptr);
+			break;
 		}
-		spin_unlock_irqrestore(&velocity_dev_list_lock, flags);
 	}
+	spin_unlock_irqrestore(&velocity_dev_list_lock, flags);
+
 	return NOTIFY_DONE;
 }
 
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 3/17 net-2.6.26] [NETFILTER]: Consolidate masq_inet_event and masq_device_event.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

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

diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c
index d80fee8..313b3fc 100644
--- a/net/ipv4/netfilter/ipt_MASQUERADE.c
+++ b/net/ipv4/netfilter/ipt_MASQUERADE.c
@@ -139,18 +139,8 @@ static int masq_inet_event(struct notifier_block *this,
 			   unsigned long event,
 			   void *ptr)
 {
-	const struct net_device *dev = ((struct in_ifaddr *)ptr)->ifa_dev->dev;
-
-	if (event == NETDEV_DOWN) {
-		/* IP address was deleted.  Search entire table for
-		   conntracks which were associated with that device,
-		   and forget them. */
-		NF_CT_ASSERT(dev->ifindex != 0);
-
-		nf_ct_iterate_cleanup(device_cmp, (void *)(long)dev->ifindex);
-	}
-
-	return NOTIFY_DONE;
+	struct net_device *dev = ((struct in_ifaddr *)ptr)->ifa_dev->dev;
+	return masq_device_event(this, event, dev);
 }
 
 static struct notifier_block masq_dev_notifier = {
-- 
1.5.3.rc5


^ permalink raw reply related

* [PATCH 2/17 net-2.6.26] [IPV4]: Remove check for ifa->ifa_dev != NULL.
From: Denis V. Lunev @ 2008-02-19  7:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, containers, devel, Denis V. Lunev
In-Reply-To: <1203406116.27296.1.camel@iris.sw.ru>

This is a callback registered to inet address notifier chain.
The check is useless as:
- ifa->ifa_dev is always != NULL
- similar checks are abscent in all other notifiers.

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

diff --git a/net/atm/clip.c b/net/atm/clip.c
index 86b885e..dd96440 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -648,10 +648,6 @@ static int clip_inet_event(struct notifier_block *this, unsigned long event,
 	struct in_device *in_dev;
 
 	in_dev = ((struct in_ifaddr *)ifa)->ifa_dev;
-	if (!in_dev || !in_dev->dev) {
-		printk(KERN_WARNING "clip_inet_event: no device\n");
-		return NOTIFY_DONE;
-	}
 	/*
 	 * Transitions are of the down-change-up type, so it's sufficient to
 	 * handle the change on up.
-- 
1.5.3.rc5


^ permalink raw reply related

* Re: tbench regression in 2.6.25-rc1
From: Eric Dumazet @ 2008-02-19  7:35 UTC (permalink / raw)
  To: Zhang, Yanmin; +Cc: David Miller, herbert, linux-kernel, netdev
In-Reply-To: <1203389095.3248.6.camel@ymzhang>

Zhang, Yanmin a écrit :
> On Mon, 2008-02-18 at 11:11 +0100, Eric Dumazet wrote:
>> On Mon, 18 Feb 2008 16:12:38 +0800
>> "Zhang, Yanmin" <yanmin_zhang@linux.intel.com> wrote:
>>
>>> On Fri, 2008-02-15 at 15:22 -0800, David Miller wrote:
>>>> From: Eric Dumazet <dada1@cosmosbay.com>
>>>> Date: Fri, 15 Feb 2008 15:21:48 +0100
>>>>
>>>>> On linux-2.6.25-rc1 x86_64 :
>>>>>
>>>>> offsetof(struct dst_entry, lastuse)=0xb0
>>>>> offsetof(struct dst_entry, __refcnt)=0xb8
>>>>> offsetof(struct dst_entry, __use)=0xbc
>>>>> offsetof(struct dst_entry, next)=0xc0
>>>>>
>>>>> So it should be optimal... I dont know why tbench prefers __refcnt being 
>>>>> on 0xc0, since in this case lastuse will be on a different cache line...
>>>>>
>>>>> Each incoming IP packet will need to change lastuse, __refcnt and __use, 
>>>>> so keeping them in the same cache line is a win.
>>>>>
>>>>> I suspect then that even this patch could help tbench, since it avoids 
>>>>> writing lastuse...
>>>> I think your suspicions are right, and even moreso
>>>> it helps to keep __refcnt out of the same cache line
>>>> as input/output/ops which are read-almost-entirely :-
>>> I think you are right. The issue is these three variables sharing the same cache line
>>> with input/output/ops.
>>>
>>>> )
>>>>
>>>> I haven't done an exhaustive analysis, but it seems that
>>>> the write traffic to lastuse and __refcnt are about the
>>>> same.  However if we find that __refcnt gets hit more
>>>> than lastuse in this workload, it explains the regression.
>>> I also think __refcnt is the key. I did a new testing by adding 2 unsigned long
>>> pading before lastuse, so the 3 members are moved to next cache line. The performance is
>>> recovered.
>>>
>>> How about below patch? Almost all performance is recovered with the new patch.
>>>
>>> Signed-off-by: Zhang Yanmin <yanmin.zhang@intel.com>
>>>
>>> ---
>>>
>>> --- linux-2.6.25-rc1/include/net/dst.h	2008-02-21 14:33:43.000000000 +0800
>>> +++ linux-2.6.25-rc1_work/include/net/dst.h	2008-02-21 14:36:22.000000000 +0800
>>> @@ -52,11 +52,10 @@ struct dst_entry
>>>  	unsigned short		header_len;	/* more space at head required */
>>>  	unsigned short		trailer_len;	/* space to reserve at tail */
>>>  
>>> -	u32			metrics[RTAX_MAX];
>>> -	struct dst_entry	*path;
>>> -
>>> -	unsigned long		rate_last;	/* rate limiting for ICMP */
>>>  	unsigned int		rate_tokens;
>>> +	unsigned long		rate_last;	/* rate limiting for ICMP */
>>> +
>>> +	struct dst_entry	*path;
>>>  
>>>  #ifdef CONFIG_NET_CLS_ROUTE
>>>  	__u32			tclassid;
>>> @@ -70,10 +69,12 @@ struct dst_entry
>>>  	int			(*output)(struct sk_buff*);
>>>  
>>>  	struct  dst_ops	        *ops;
>>> -		
>>> -	unsigned long		lastuse;
>>> +
>>> +	u32			metrics[RTAX_MAX];
>>> +
>>>  	atomic_t		__refcnt;	/* client references	*/
>>>  	int			__use;
>>> +	unsigned long		lastuse;
>>>  	union {
>>>  		struct dst_entry *next;
>>>  		struct rtable    *rt_next;
>>>
>>>
>> Well, after this patch, we grow dst_entry by 8 bytes :
> With my .config, it doesn't grow. Perhaps because of CONFIG_NET_CLS_ROUTE, I don't
> enable it. I will move tclassid under ops.
> 
>> sizeof(struct dst_entry)=0xd0
>> offsetof(struct dst_entry, input)=0x68
>> offsetof(struct dst_entry, output)=0x70
>> offsetof(struct dst_entry, __refcnt)=0xb4
>> offsetof(struct dst_entry, lastuse)=0xc0
>> offsetof(struct dst_entry, __use)=0xb8
>> sizeof(struct rtable)=0x140
>>
>>
>> So we dirty two cache lines instead of one, unless your cpu have 128 bytes cache lines ?
>>
>> I am quite suprised that my patch to not change lastuse if already set to jiffies changes nothing...
>>
>> If you have some time, could you also test this (unrelated) patch ?
>>
>> We can avoid dirty all the time a cache line of loopback device.
>>
>> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
>> index f2a6e71..0a4186a 100644
>> --- a/drivers/net/loopback.c
>> +++ b/drivers/net/loopback.c
>> @@ -150,7 +150,10 @@ static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
>>                 return 0;
>>         }
>>  #endif
>> -       dev->last_rx = jiffies;
>> +#ifdef CONFIG_SMP
>> +       if (dev->last_rx != jiffies)
>> +#endif
>> +               dev->last_rx = jiffies;
>>  
>>         /* it's OK to use per_cpu_ptr() because BHs are off */
>>         pcpu_lstats = netdev_priv(dev);
>>
> Although I didn't test it, I don't think it's ok. The key is __refcnt shares the same
> cache line with ops/input/output.
> 

Note it was unrelated to struct dst, but dirtying of one cache line of 
'loopback netdevice'

I tested it, and tbench result was better with this patch : 890 MB/s instead 
of 870 MB/s on a bi dual core machine.


I was curious of the potential gain on your 16 cores (4x4) machine.

^ permalink raw reply

* Re: tbench regression in 2.6.25-rc1
From: Eric Dumazet @ 2008-02-19  7:40 UTC (permalink / raw)
  To: Zhang, Yanmin
  Cc: Valdis.Kletnieks, David Miller, herbert, linux-kernel, netdev
In-Reply-To: <1203403903.3248.29.camel@ymzhang>

Zhang, Yanmin a écrit :
> On Mon, 2008-02-18 at 12:33 -0500, Valdis.Kletnieks@vt.edu wrote: 
>> On Mon, 18 Feb 2008 16:12:38 +0800, "Zhang, Yanmin" said:
>>
>>> I also think __refcnt is the key. I did a new testing by adding 2 unsigned long
>>> pading before lastuse, so the 3 members are moved to next cache line. The performance is
>>> recovered.
>>>
>>> How about below patch? Almost all performance is recovered with the new patch.
>>>
>>> Signed-off-by: Zhang Yanmin <yanmin.zhang@intel.com>
>> Could you add a comment someplace that says "refcnt wants to be on a different
>> cache line from input/output/ops or performance tanks badly", to warn some
>> future kernel hacker who starts adding new fields to the structure?
> Ok. Below is the new patch.
> 
> 1) Move tclassid under ops in case CONFIG_NET_CLS_ROUTE=y. So sizeof(dst_entry)=200
> no matter if CONFIG_NET_CLS_ROUTE=y/n. I tested many patches on my 16-core tigerton by
> moving tclassid to different place. It looks like tclassid could also have impact on
> performance.
> If moving tclassid before metrics, or just don't move tclassid, the performance isn't
> good. So I move it behind metrics.
> 
> 2) Add comments before __refcnt.
> 
> If CONFIG_NET_CLS_ROUTE=y, the result with below patch is about 18% better than
> the one without the patch.
> 
> If CONFIG_NET_CLS_ROUTE=n, the result with below patch is about 30% better than
> the one without the patch.
> 
> Signed-off-by: Zhang Yanmin <yanmin.zhang@intel.com>
> 
> ---
> 
> --- linux-2.6.25-rc1/include/net/dst.h	2008-02-21 14:33:43.000000000 +0800
> +++ linux-2.6.25-rc1_work/include/net/dst.h	2008-02-22 12:52:19.000000000 +0800
> @@ -52,15 +52,10 @@ struct dst_entry
>  	unsigned short		header_len;	/* more space at head required */
>  	unsigned short		trailer_len;	/* space to reserve at tail */
>  
> -	u32			metrics[RTAX_MAX];
> -	struct dst_entry	*path;
> -
> -	unsigned long		rate_last;	/* rate limiting for ICMP */
>  	unsigned int		rate_tokens;
> +	unsigned long		rate_last;	/* rate limiting for ICMP */
>  
> -#ifdef CONFIG_NET_CLS_ROUTE
> -	__u32			tclassid;
> -#endif
> +	struct dst_entry	*path;
>  
>  	struct neighbour	*neighbour;
>  	struct hh_cache		*hh;
> @@ -70,10 +65,20 @@ struct dst_entry
>  	int			(*output)(struct sk_buff*);
>  
>  	struct  dst_ops	        *ops;
> -		
> -	unsigned long		lastuse;
> +
> +	u32			metrics[RTAX_MAX];
> +
> +#ifdef CONFIG_NET_CLS_ROUTE
> +	__u32			tclassid;
> +#endif
> +
> +	/*
> +	 * __refcnt wants to be on a different cache line from
> +	 * input/output/ops or performance tanks badly
> +	 */
>  	atomic_t		__refcnt;	/* client references	*/
>  	int			__use;
> +	unsigned long		lastuse;
>  	union {
>  		struct dst_entry *next;
>  		struct rtable    *rt_next;
> 
> 
> 

I prefer this patch, but unfortunatly your perf numbers are for 64 bits kernels.

Could you please test now with 32 bits one ?

Thank you

^ permalink raw reply

* Re: [patch] pegasus.c
From: Petko Manolov @ 2008-02-19  8:11 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev
In-Reply-To: <47B06BF4.7010009@garzik.org>

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

This is another attempt on sending the pegasus patch with Alpine.  I know 
it's even worse, but i've attached gzip-ed version just in case.

Here goes a brief description of what's changed:

---
This patch is fixing a driver bug triggered when malformed string is 
passed to the 'devid' module parameter.  The expected format is:

 	"device_name:vendor_id:device_id:flags"

but it turned out people often type:

 	"somename::0"

instead of:

 	"somename:::0"

which typically ends up dereferencing null pointer.


Signed-off-by: Petko Manolov <petkan@nucleusys.com>
---


cheers,
Petko


On Mon, 11 Feb 2008, Jeff Garzik wrote:

> Petko Manolov wrote:
>>     Hi Jeff,
>> 
>> Attached you'll find a patch that is fixing a driver bug triggered when 
>> malformed string is passed to the 'devid' module parameter.  The expected 
>> format is:
>>
>>     "device_name:vendor_id:device_id:flags"
>> 
>> but it turned out people often type:
>>
>>     "somename::0"
>> 
>> instead of:
>>
>>     "somename:::0"
>
> ACK but two process problems preventing application:
>
> * patch is base64-encoded
>
> * no signed-off-by included
>
>
>

[-- Attachment #2: Type: APPLICATION/OCTET-STREAM, Size: 580 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: TEXT/x-diff; name=pegasus.c.patch, Size: 1373 bytes --]

--- drivers/net/usb/pegasus.c.orig	2008-01-09 12:16:52.000000000 +0200
+++ drivers/net/usb/pegasus.c	2008-01-09 12:16:58.000000000 +0200
@@ -1461,12 +1461,24 @@ static void parse_id(char *id)
 
 	if ((token = strsep(&id, ":")) != NULL)
 		name = token;
+	else
+		goto err;
 	/* name now points to a null terminated string*/
 	if ((token = strsep(&id, ":")) != NULL)
 		vendor_id = simple_strtoul(token, NULL, 16);
+	else
+		goto err;
+
 	if ((token = strsep(&id, ":")) != NULL)
 		device_id = simple_strtoul(token, NULL, 16);
-	flags = simple_strtoul(id, NULL, 16);
+	else
+		goto err;
+
+	if (id != NULL)
+		flags = simple_strtoul(id, NULL, 16);
+	else
+		goto err;
+
 	pr_info("%s: new device %s, vendor ID 0x%04x, device ID 0x%04x, flags: 0x%x\n",
 	        driver_name, name, vendor_id, device_id, flags);
 
@@ -1476,6 +1488,7 @@ static void parse_id(char *id)
 		return;
 
 	for (i=0; usb_dev_id[i].name; i++);
+
 	usb_dev_id[i].name = name;
 	usb_dev_id[i].vendor = vendor_id;
 	usb_dev_id[i].device = device_id;
@@ -1483,6 +1496,11 @@ static void parse_id(char *id)
 	pegasus_ids[i].match_flags = USB_DEVICE_ID_MATCH_DEVICE;
 	pegasus_ids[i].idVendor = vendor_id;
 	pegasus_ids[i].idProduct = device_id;
+
+	return;
+
+err:
+	pr_info("malformed 'devid' module parameter\n");
 }
 
 static int __init pegasus_init(void)

^ permalink raw reply

* AW: Problem receiving multicast/promiscuous-mode with kernel.2.6.24
From: Reither Robert @ 2008-02-19  8:40 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
________________________________________________________________________



Hi,

ok, i managed to shrink down my code to show the behaviour in small size ;-)

It shows the same effect as my app, sometimes i get all the packets after a mc_join, sometimes i get only the timeouts.
I find no predictive behaviour, i'm really desperate ...
Hope you can see the effect too .. Do u need an app sending the multicast VLAN packets ?

Robert

-----------------------------------------

#include <stdint.h>
#include <sys/select.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <time.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <arpa/inet.h>
#include <net/if.h>
#include <errno.h>
#include <netdb.h>

//#include <linux/if.h>
#include <linux/sockios.h>
#include <sys/ioctl.h>

//#define JOIN_SINGLE_IF
#undef JOIN_SINGLE_IF


#define PORT 10500

/*
** Try to receive VLAN-tagged UDP multicast packets 
** VLAN-ID:3, VLAN_PRI=6, Addr: 224.1.9.0, dstnport 10500
**
** Kernel used: 2.6.24
**
** HW: VIA Epia 5000, VIA Epia LT (VIA Rhine II and VIA VT6107)
**
** Additional network settings:
** vconfig add eth0 3
** vconfig set_egress_map eth0.3 6 6
** ifconfig eth0.3 10.0.0.1
** route add -net 224.0.0.0 netmask 240.0.0.0 dev eth0.3
**
*/

#define MAX_PAYLOAD 1000

/* 
 * Network representation of the rtp header.
 */

struct rtp_header {
#ifdef WORDS_BIGENDIAN
        uint16_t version:2;
        uint16_t padbit:1;
        uint16_t extbit:1;
        uint16_t cc:4;
        uint16_t markbit:1;
        uint16_t paytype:7;
#else
        uint16_t cc:4;
        uint16_t extbit:1;
        uint16_t padbit:1;
        uint16_t version:2;
        uint16_t paytype:7;
        uint16_t markbit:1;
#endif
        uint16_t seq_number;
        uint32_t timestamp;
        uint32_t ssrc;

	/* In fact we rely on rtp_header being not larger than 
	 * the minimum header length. So, there's no contributing
	 * sync source field here. */
};

struct rtp_packet {
	struct rtp_header header;
	unsigned char payload[MAX_PAYLOAD];
};


#ifdef WORDS_BIGENDIAN
#error Sorry this is not supported on bigendian targets.
#endif

#define BYTE0(x) ((x) & 0xff)
#define BYTE1(x) (((x) >> 8)  & 0xff)
#define BYTE2(x) (((x) >> 16)  & 0xff)
#define BYTE3(x) (((x) >> 24)  & 0xff)

static char *my_inet_ntoa (char *buf, size_t n, struct in_addr in)
{
	if (snprintf (buf, n, "%u.%u.%u.%u", BYTE0(in.s_addr), BYTE1(in.s_addr), BYTE2(in.s_addr), BYTE3(in.s_addr))  >= n) {
		return NULL;
	}
	return buf;
}

static int set_non_blocking (int fd)
{
	int flags;

	if ((flags = fcntl (fd, F_GETFL)) < 0) {
		perror ("fcntl getflags ()");
		return -1;
	}
	if (fcntl (fd, F_SETFL, flags | O_NONBLOCK) < 0) {
		perror ("fcntl setflags ()");
		return -1;
	}

	return fd;
}

int main()
{
  int fd;
  fd_set read_fds;
  struct timespec timeout;
  
  while(1)
  {
    int i, ret;

  	if ((fd = subscribe_udp (PORT)) < 0) {
  		printf("Could not register UDP port.\n");
   		return 1;
    }
    	
  
  	if (join_multicast_group (fd, "224.1.9.0") < 0)
  	{
  	  printf("Error joining multicast !\n");
  	  return 1;  
  	}

    for(i=0;i<10;i++)
    {

      FD_ZERO (&read_fds);
    	FD_SET (fd, &read_fds);
    	timeout.tv_sec = 1;
    	timeout.tv_nsec = 0;
  
  
  		ret = pselect (fd+1, &read_fds, NULL, NULL, &timeout, NULL);
  
  
  		if (ret < 0) {
  			if (errno == EINTR) {
  				continue;
  			} else {
  				printf("select failed.\n");
  				return 1;
  			}
  		}

      if (FD_ISSET(fd, &read_fds))
      {
        ssize_t len;
      	struct rtp_packet p; 
      	struct sockaddr_in from;
      	socklen_t fromlen;
      	char from_ip[20];
        
    	  len = recvfrom (fd, &p, sizeof (p), 0, (struct sockaddr*) &from, &fromlen);
      	my_inet_ntoa (from_ip, sizeof (from_ip), from.sin_addr);
    
        printf("Received data packet from %s, length %u\n", from_ip, len);
      }
      else
      {
        printf("Got timeout receiving Multicast packets !\n");  
      }
    } /* loop 10 */
  
    if (leave_multicast_group(fd, "224.1.9.0") < 0)
  	{
  	  printf("Error leaving multicast !\n");
  	  return 1;  
  	}
    close(fd);	
  }
}

int subscribe_udp (int port)
{
	int s; 

	s = create_listening_socket (port, 1);
	return s;
}

int create_listening_socket (int listen_port, int udp) 
{
	struct sockaddr_in a;
	int s;
	int yes;
	if ((s = socket (AF_INET, udp ? SOCK_DGRAM : SOCK_STREAM, 0)) < 0) {
		perror ("socket");
		return -1;
	}
	yes = 1;
	if (setsockopt (s, SOL_SOCKET, SO_REUSEADDR,
		 (char *) &yes, sizeof (yes)) < 0) {
		perror ("setsockopt reuseaddr");
		close (s);
		return -1;
	}

	memset (&a, 0, sizeof (a));

	a.sin_port = htons (listen_port);
	a.sin_family = AF_INET;
	if (bind (s, (struct sockaddr *) &a, sizeof (a)) < 0) {
		perror ("bind");
		close (s);
		return -1;
	}
	if (set_non_blocking (s) < 0) {
		close (s);
		return -1;
	}

	if (!udp) {
		listen (s, 10);
	}
	return s;
}

int join_multicast_group (int fd, const char *mc_address)
{
	struct ip_mreqn imr;

  printf("In join_mc_group: %s\n",mc_address);


  if (inet_pton(AF_INET, mc_address, &imr.imr_multiaddr) != 1) {
//old	if (inet_aton (mc_address, &imr.imr_multiaddr) == 0) {
		printf ( "join_mc:Bad IP address format: %s\n", mc_address);
		return -1;
	}

	imr.imr_address.s_addr = INADDR_ANY;

#ifdef JOIN_SINGLE_IF
	imr.imr_ifindex = if_nametoindex("eth0.3"); // 0;
//	imr.imr_ifindex = if_nametoindex("eth0"); // Testing
  if (imr.imr_ifindex == 0)
  {
		printf ("join_mc:Got no interface-number from name !\n");
		return -1;    
  }
#else
	imr.imr_ifindex = 0;
#endif

  // Check if already member
	if (setsockopt (fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &imr, sizeof (imr)) < 0)
	{
	  if (errno == EADDRINUSE) // Address already joined for IP_ADD_MEMBERSHIP
	    printf("Already joined IP_ADD_MEMBERSHIP for %s.\n", mc_address);
    else
    {
  		perror ("getsockopt ip_add_membership");
  		return -1;
	  }
	}
	return 0;
}

int leave_multicast_group(int fd, const char *mc_address)
{
	struct ip_mreqn imr;

  printf("In leave_mc_group: %s\n",mc_address);

  if (inet_pton(AF_INET, mc_address, &imr.imr_multiaddr) != 1) {
//	if (inet_aton (mc_address, &imr.imr_multiaddr) == 0) {
		printf ("leave_mc:Bad IP address format: %s\n", mc_address);
		return -1;
	}

	imr.imr_address.s_addr = INADDR_ANY;

#ifdef JOIN_SINGLE_IF
	imr.imr_ifindex = if_nametoindex("eth0.3"); // 0 VLAN_ID ist aber vernderbar !!!!!
//	imr.imr_ifindex = if_nametoindex("eth0"); // 0
  if (imr.imr_ifindex == 0)
  {
		printf ("leave_mc:Got no interface-namber from name !\n");
		return -1;    
  }
#else
	imr.imr_ifindex = 0;
#endif

	if (setsockopt (fd, IPPROTO_IP, IP_DROP_MEMBERSHIP, &imr, sizeof (imr)) < 0) {
		perror ("setsockopt ip_drop_membership");
		return -1;
	}
  return 0;
}

^ permalink raw reply

* Re: tbench regression in 2.6.25-rc1
From: Zhang, Yanmin @ 2008-02-19  8:40 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, herbert, linux-kernel, netdev
In-Reply-To: <47BA86C8.4050207@cosmosbay.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 5384 bytes --]

On Tue, 2008-02-19 at 08:35 +0100, Eric Dumazet wrote:
> Zhang, Yanmin a écrit :
> > On Mon, 2008-02-18 at 11:11 +0100, Eric Dumazet wrote:
> >> On Mon, 18 Feb 2008 16:12:38 +0800
> >> "Zhang, Yanmin" <yanmin_zhang@linux.intel.com> wrote:
> >>
> >>> On Fri, 2008-02-15 at 15:22 -0800, David Miller wrote:
> >>>> From: Eric Dumazet <dada1@cosmosbay.com>
> >>>> Date: Fri, 15 Feb 2008 15:21:48 +0100
> >>>>
> >>>>> On linux-2.6.25-rc1 x86_64 :
> >>>>>
> >>>>> offsetof(struct dst_entry, lastuse)=0xb0
> >>>>> offsetof(struct dst_entry, __refcnt)=0xb8
> >>>>> offsetof(struct dst_entry, __use)=0xbc
> >>>>> offsetof(struct dst_entry, next)=0xc0
> >>>>>
> >>>>> So it should be optimal... I dont know why tbench prefers __refcnt being 
> >>>>> on 0xc0, since in this case lastuse will be on a different cache line...
> >>>>>
> >>>>> Each incoming IP packet will need to change lastuse, __refcnt and __use, 
> >>>>> so keeping them in the same cache line is a win.
> >>>>>
> >>>>> I suspect then that even this patch could help tbench, since it avoids 
> >>>>> writing lastuse...
> >>>> I think your suspicions are right, and even moreso
> >>>> it helps to keep __refcnt out of the same cache line
> >>>> as input/output/ops which are read-almost-entirely :-
> >>> I think you are right. The issue is these three variables sharing the same cache line
> >>> with input/output/ops.
> >>>
> >>>> )
> >>>>
> >>>> I haven't done an exhaustive analysis, but it seems that
> >>>> the write traffic to lastuse and __refcnt are about the
> >>>> same.  However if we find that __refcnt gets hit more
> >>>> than lastuse in this workload, it explains the regression.
> >>> I also think __refcnt is the key. I did a new testing by adding 2 unsigned long
> >>> pading before lastuse, so the 3 members are moved to next cache line. The performance is
> >>> recovered.
> >>>
> >>> How about below patch? Almost all performance is recovered with the new patch.
> >>>
> >>> Signed-off-by: Zhang Yanmin <yanmin.zhang@intel.com>
> >>>
> >>> ---
> >>>
> >>> --- linux-2.6.25-rc1/include/net/dst.h	2008-02-21 14:33:43.000000000 +0800
> >>> +++ linux-2.6.25-rc1_work/include/net/dst.h	2008-02-21 14:36:22.000000000 +0800
> >>> @@ -52,11 +52,10 @@ struct dst_entry
> >>>  	unsigned short		header_len;	/* more space at head required */
> >>>  	unsigned short		trailer_len;	/* space to reserve at tail */
> >>>  
> >>> -	u32			metrics[RTAX_MAX];
> >>> -	struct dst_entry	*path;
> >>> -
> >>> -	unsigned long		rate_last;	/* rate limiting for ICMP */
> >>>  	unsigned int		rate_tokens;
> >>> +	unsigned long		rate_last;	/* rate limiting for ICMP */
> >>> +
> >>> +	struct dst_entry	*path;
> >>>  
> >>>  #ifdef CONFIG_NET_CLS_ROUTE
> >>>  	__u32			tclassid;
> >>> @@ -70,10 +69,12 @@ struct dst_entry
> >>>  	int			(*output)(struct sk_buff*);
> >>>  
> >>>  	struct  dst_ops	        *ops;
> >>> -		
> >>> -	unsigned long		lastuse;
> >>> +
> >>> +	u32			metrics[RTAX_MAX];
> >>> +
> >>>  	atomic_t		__refcnt;	/* client references	*/
> >>>  	int			__use;
> >>> +	unsigned long		lastuse;
> >>>  	union {
> >>>  		struct dst_entry *next;
> >>>  		struct rtable    *rt_next;
> >>>
> >>>
> >> Well, after this patch, we grow dst_entry by 8 bytes :
> > With my .config, it doesn't grow. Perhaps because of CONFIG_NET_CLS_ROUTE, I don't
> > enable it. I will move tclassid under ops.
> > 
> >> sizeof(struct dst_entry)=0xd0
> >> offsetof(struct dst_entry, input)=0x68
> >> offsetof(struct dst_entry, output)=0x70
> >> offsetof(struct dst_entry, __refcnt)=0xb4
> >> offsetof(struct dst_entry, lastuse)=0xc0
> >> offsetof(struct dst_entry, __use)=0xb8
> >> sizeof(struct rtable)=0x140
> >>
> >>
> >> So we dirty two cache lines instead of one, unless your cpu have 128 bytes cache lines ?
> >>
> >> I am quite suprised that my patch to not change lastuse if already set to jiffies changes nothing...
> >>
> >> If you have some time, could you also test this (unrelated) patch ?
> >>
> >> We can avoid dirty all the time a cache line of loopback device.
> >>
> >> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
> >> index f2a6e71..0a4186a 100644
> >> --- a/drivers/net/loopback.c
> >> +++ b/drivers/net/loopback.c
> >> @@ -150,7 +150,10 @@ static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
> >>                 return 0;
> >>         }
> >>  #endif
> >> -       dev->last_rx = jiffies;
> >> +#ifdef CONFIG_SMP
> >> +       if (dev->last_rx != jiffies)
> >> +#endif
> >> +               dev->last_rx = jiffies;
> >>  
> >>         /* it's OK to use per_cpu_ptr() because BHs are off */
> >>         pcpu_lstats = netdev_priv(dev);
> >>
> > Although I didn't test it, I don't think it's ok. The key is __refcnt shares the same
> > cache line with ops/input/output.
> > 
> 
> Note it was unrelated to struct dst, but dirtying of one cache line of 
> 'loopback netdevice'
> 
> I tested it, and tbench result was better with this patch : 890 MB/s instead 
> of 870 MB/s on a bi dual core machine.
I tested your new patch and it doesn't help tbench.

On my 8-core stoakley machine, the regression is only 5%, but it's 30% on 16-core tigerton.
It looks like the scalability is poor.

> 
> 
> I was curious of the potential gain on your 16 cores (4x4) machine.



^ permalink raw reply

* Re: Linux 2.6.24.1 - kernel does not boot; IRQ trouble?
From: Kay Sievers @ 2008-02-19  8:47 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Chris Rankin, Andrew Morton, linux-acpi, linux-kernel, netdev
In-Reply-To: <20080218120636.00451d46@extreme>

On Feb 18, 2008 9:06 PM, Stephen Hemminger
<shemminger@linux-foundation.org> wrote:
> On Mon, 18 Feb 2008 19:42:25 +0000 (GMT)
> Chris Rankin <rankincj@yahoo.com> wrote:
>
> > --- Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > > > > sysfs: duplicate filename 'bridge' can not be created
> > > > > WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> > > > > Pid: 1, comm: swapper Not tainted 2.6.24.1 #1
> > > > >  [<c0105020>] show_trace_log_lvl+0x1a/0x2f
> > > > >  [<c0105990>] show_trace+0x12/0x14
> > > > >  [<c010613d>] dump_stack+0x6c/0x72
> > > > >  [<c01991bf>] sysfs_add_one+0x57/0xbc
> > > > >  [<c0199e41>] sysfs_create_link+0xc2/0x10d
> > > > >  [<c01bae9a>] pci_bus_add_devices+0xbd/0x103
> > > > >  [<c034016c>] pci_legacy_init+0x56/0xe3
> > > > >  [<c03274e1>] kernel_init+0x157/0x2c3
> > > > >  [<c0104c83>] kernel_thread_helper+0x7/0x10
> > > > >  =======================
> > > > > pci 0000:00:01.0: Error creating sysfs bridge symlink, continuing...
> > > > > sysfs: duplicate filename 'bridge' can not be created
> > > > > WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> > > > > Pid: 1, comm: swapper Not tainted 2.6.24.1 #1
> > > > >  [<c0105020>] show_trace_log_lvl+0x1a/0x2f
> > > > >  [<c0105990>] show_trace+0x12/0x14
> > > > >  [<c010613d>] dump_stack+0x6c/0x72
> > > > >  [<c01991bf>] sysfs_add_one+0x57/0xbc
> > > > >  [<c0199e41>] sysfs_create_link+0xc2/0x10d
> > > > >  [<c01bae9a>] pci_bus_add_devices+0xbd/0x103
> > > > >  [<c01bae82>] pci_bus_add_devices+0xa5/0x103
> > > > >  [<c034016c>] pci_legacy_init+0x56/0xe3
> > > > >  [<c03274e1>] kernel_init+0x157/0x2c3
> > > > >  [<c0104c83>] kernel_thread_helper+0x7/0x10
> > > > >  =======================
> > > >
> > > > I have a vague feeling that this was fixed, perhaps in 2.6.24.x?
> > >
> > > Never heard of this, what is the initialization script that causes this?
> > > Also do you have the SYSFS_DEPRECATED option configured? that caused issues
> > > with regular network drivers.
> >
> > Yes, SYSFS_DEPRECATED is enabled. And the init scripts are from Fedora 8.
>
> There was a bug (fixed in 2.6.24) that had to do with sysfs_create_link
> and SYSFS_DEPRECATED probably there is a similar problem with directories.

Chris, could you enable CONFIG_DEBUG_KOBJECT=y, it might show what
objects try to claim the same name.

Thanks,
Kay

^ permalink raw reply

* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: James Chapman @ 2008-02-19  9:03 UTC (permalink / raw)
  To: David Miller; +Cc: jarkao2, netdev
In-Reply-To: <20080218.202934.79548477.davem@davemloft.net>

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(). I changed the code to use __sk_dst_check() as 
you suggested and they went away. At that point, I was hopeful the 
locking issues were fixed. But after several minutes of 
creating/deleting hundreds of ppp sessions, lockdep dumped another 
error. It is that error that I posted yesterday.

> Next, I can't see why ppp_input() needs to be invoked with
> interrupts disabled.  There are many other things that invoke
> that in software interrupt context, such as pppoe.

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

> Please provide the lockdep traces without the ppp_input() IRQ
> disabling so this can be properly analyzed.

The trace _was_ without ppp_input IRQ disabling. The trace doesn't occur 
if I disable IRQs around the ppp_input() call. The patch I sent showed 
the changes I made before running the tests that created the new lockdep 
trace. I'm sorry this wasn't clear.


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


^ 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