Patchwork wireless extensions: play with netns

login
register
mail settings
Submitter Johannes Berg
Date June 17, 2009, 11:41 p.m.
Message ID <1245282099.31588.69.camel@johannes.local>
Download mbox | patch
Permalink /patch/28823/
State RFC
Delegated to: David Miller
Headers show

Comments

Johannes Berg - June 17, 2009, 11:41 p.m.
On Wed, 2009-06-17 at 16:24 -0700, Eric W. Biederman wrote:

> > So it looks like I can also use rcu_read_lock(), but there's no
> > for_each_net_rcu(), should there be?
> 
> I'm not using rcu safe list manipulation.  What makes it look like
> rcu_read_lock() is safe?

Indeed. I was looking at rcu_barrier() only. How about the patch below?

johannes

Subject: net: make namespace iteration possible under RCU

We already call rcu_barrier(), so all we need to take
care of is using proper RCU list add/del primitives.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 include/net/net_namespace.h |    3 +++
 net/core/net_namespace.c    |    7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman - June 17, 2009, 11:54 p.m.
Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2009-06-17 at 16:24 -0700, Eric W. Biederman wrote:
>
>> > So it looks like I can also use rcu_read_lock(), but there's no
>> > for_each_net_rcu(), should there be?
>> 
>> I'm not using rcu safe list manipulation.  What makes it look like
>> rcu_read_lock() is safe?
>
> Indeed. I was looking at rcu_barrier() only. How about the patch below?
>
> johannes
>
> Subject: net: make namespace iteration possible under RCU
>
> We already call rcu_barrier(), so all we need to take
> care of is using proper RCU list add/del primitives.

This of course gives you a network namespace that can be found by for_each_net rcu
while the per net exit functions are running.  I think that opens up to races
that I don't want to think about.

I still haven't tracked down how I am occasionally getting time wait sockets
with an invalid network namespace.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg - June 18, 2009, 12:02 a.m.
On Wed, 2009-06-17 at 16:54 -0700, Eric W. Biederman wrote:

> > Subject: net: make namespace iteration possible under RCU
> >
> > We already call rcu_barrier(), so all we need to take
> > care of is using proper RCU list add/del primitives.
> 
> This of course gives you a network namespace that can be found by for_each_net rcu
> while the per net exit functions are running.  I think that opens up to races
> that I don't want to think about.

Indeed. Can we move the rcu_barrier() up? Or we could insert a
synchronize_rcu() (which is sufficient for rcu_read_lock) before the
exit functions are run?

> I still haven't tracked down how I am occasionally getting time wait sockets
> with an invalid network namespace.

Ouch.

johannes
Eric W. Biederman - June 18, 2009, 12:30 a.m.
Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2009-06-17 at 16:54 -0700, Eric W. Biederman wrote:
>
>> > Subject: net: make namespace iteration possible under RCU
>> >
>> > We already call rcu_barrier(), so all we need to take
>> > care of is using proper RCU list add/del primitives.
>> 
>> This of course gives you a network namespace that can be found by for_each_net rcu
>> while the per net exit functions are running.  I think that opens up to races
>> that I don't want to think about.
>
> Indeed. Can we move the rcu_barrier() up? Or we could insert a
> synchronize_rcu() (which is sufficient for rcu_read_lock) before the
> exit functions are run?

I don't think we can move the barrier.   But adding an extra synchronize_rcu
should be fine.  We are talking about the slowest of the slow paths here.
It is so slow it even gets it's own kernel thread.

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

Patch

--- wireless-testing.orig/include/net/net_namespace.h	2009-06-18 01:36:26.000000000 +0200
+++ wireless-testing/include/net/net_namespace.h	2009-06-18 01:39:35.000000000 +0200
@@ -211,6 +211,9 @@  static inline struct net *read_pnet(stru
 #define for_each_net(VAR)				\
 	list_for_each_entry(VAR, &net_namespace_list, list)
 
+#define for_each_net_rcu(VAR)				\
+	list_for_each_entry_rcu(VAR, &net_namespace_list, list)
+
 #ifdef CONFIG_NET_NS
 #define __net_init
 #define __net_exit
--- wireless-testing.orig/net/core/net_namespace.c	2009-06-18 01:36:39.000000000 +0200
+++ wireless-testing/net/core/net_namespace.c	2009-06-18 01:38:36.000000000 +0200
@@ -6,6 +6,7 @@ 
 #include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/idr.h>
+#include <linux/rculist.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 
@@ -134,7 +135,7 @@  struct net *copy_net_ns(unsigned long fl
 	err = setup_net(new_net);
 	if (!err) {
 		rtnl_lock();
-		list_add_tail(&new_net->list, &net_namespace_list);
+		list_add_tail_rcu(&new_net->list, &net_namespace_list);
 		rtnl_unlock();
 	}
 	mutex_unlock(&net_mutex);
@@ -163,7 +164,7 @@  static void cleanup_net(struct work_stru
 
 	/* Don't let anyone else find us. */
 	rtnl_lock();
-	list_del(&net->list);
+	list_del_rcu(&net->list);
 	rtnl_unlock();
 
 	/* Run all of the network namespace exit methods */
@@ -227,7 +228,7 @@  static int __init net_ns_init(void)
 	err = setup_net(&init_net);
 
 	rtnl_lock();
-	list_add_tail(&init_net.list, &net_namespace_list);
+	list_add_tail_rcu(&init_net.list, &net_namespace_list);
 	rtnl_unlock();
 
 	mutex_unlock(&net_mutex);