From: Daniel Lezcano <daniel.lezcano@free•fr>
To: David Lamparter <equinox@diac24•net>
Cc: netdev@vger•kernel.org, Patrick McHardy <kaber@trash•net>,
"Eric W. Biederman" <ebiederm@xmission•com>
Subject: Re: [PATCH RFC] netns: keep vlan slaves on master netns move
Date: Tue, 24 Aug 2010 19:00:40 +0200 [thread overview]
Message-ID: <4C73FAB8.1090402@free.fr> (raw)
In-Reply-To: <20100824115056.GA235835@jupiter.n2.diac24.net>
On 08/24/2010 01:50 PM, David Lamparter wrote:
> Hi everyone,
>
> attached patch makes it possible to keep macvlan / 802.1q slave devices
> on moving their master to a different namespace. as the opposite works
> without problems (moving the slaves into a different namespace), this
> shouldn't cause problems either.
>
> i've tested this with 802.1q on real ethernet devs and bridges and,
> well, it works without crashing, but that obviously doesn't mean it is
> correct :) - therefore input very welcome.
>
> RFC,
>
> David
>
> P.S.: who do i Cc: on netns related stuff?
Definitively the netns is the brain child of:
Eric W. Biederman <ebiederm@xmission•com>
As I contributed and I am interested by following the changes, that will
be nice if you can Cc me too.
Daniel Lezcano <daniel.lezcano@free•fr>
> --
> previously, if a vlan master device was moved from one network namespace
> to another, all 802.1q and macvlan slaves were deleted.
>
> we can use dev->reg_state to figure out whether dev_change_net_namespace
> is happening, since that won't set dev->reg_state NETREG_UNREGISTERING.
> so, this changes 8021q and macvlan to ignore NETDEV_UNREGISTER when
> reg_state is not NETREG_UNREGISTERING.
>
> Signed-off-by: David Lamparter<equinox@diac24•net>
> ---
> drivers/net/macvlan.c | 4 ++++
> net/8021q/vlan.c | 4 ++++
> net/core/dev.c | 4 ++++
> 3 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index f15fe2c..f43f8e4 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -754,6 +754,10 @@ static int macvlan_device_event(struct notifier_block *unused,
> }
> break;
> case NETDEV_UNREGISTER:
> + /* twiddle thumbs on netns device moves */
> + if (dev->reg_state != NETREG_UNREGISTERING)
> + break;
> +
Hmm, I don't feel comfortable with this change. It is like we are trying
to catch a transient state, not clearly defined (you need a comment) and
that may lead to some confusion later.
IMHO, it should be convenient to add a NETREG_NETNS_MOVING and set this
state in the dev_change_net_namespace.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5c6f519..0214b77 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -924,6 +924,7 @@ struct net_device {
/* register/unregister state machine */
enum { NETREG_UNINITIALIZED=0,
+ NETREG_NETNS_MOVING, /* changing netns */
NETREG_REGISTERED, /* completed register_netdevice */
NETREG_UNREGISTERING, /* called unregister_netdevice */
NETREG_UNREGISTERED, /* completed unregister todo */
diff --git a/net/core/dev.c b/net/core/dev.c
index e233933..9154123 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5752,6 +5752,8 @@ int dev_change_net_namespace(struct net_device
*dev, struct net *net, const char
err = -ENODEV;
unlist_netdevice(dev);
+ dev->reg_state = NETREG_NETNS_MOVING;
+
synchronize_net();
/* Shutdown queueing discipline. */
@@ -5786,6 +5788,8 @@ int dev_change_net_namespace(struct net_device
*dev, struct net *net, const char
err = netdev_register_kobject(dev);
WARN_ON(err);
+ dev->reg_state = NETREG_REGISTERED;
+
/* Add the device back in the hashes */
list_netdevice(dev);
Then you can check in macvlan_device_event
...
if (dev->reg_state != NETREG_NETNS_MOVING)
break;
...
Does it make sense ?
-- Daniel
next prev parent reply other threads:[~2010-08-24 17:00 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-24 11:50 [PATCH RFC] netns: keep vlan slaves on master netns move David Lamparter
2010-08-24 17:00 ` Daniel Lezcano [this message]
2010-08-24 19:14 ` Eric W. Biederman
2010-08-25 11:52 ` [PATCH RFC] netns: introduce NETREG_NETNS_MOVING reg_state David Lamparter
2010-08-25 13:03 ` Daniel Lezcano
2010-08-25 13:52 ` David Lamparter
2010-08-25 17:39 ` Eric W. Biederman
2010-08-26 8:21 ` David Lamparter
2010-09-13 11:43 ` [PATCH RFC] netns: keep vlan slaves on master netns move David Lamparter
2010-09-15 2:10 ` Eric W. Biederman
2010-09-17 12:49 ` Patrick McHardy
2010-09-27 16:23 ` Eric W. Biederman
2010-09-17 13:22 ` [PATCH] " David Lamparter
2010-09-17 17:56 ` David Miller
2010-09-15 6:47 ` [PATCH RFC] " Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C73FAB8.1090402@free.fr \
--to=daniel.lezcano@free$(echo .)fr \
--cc=ebiederm@xmission$(echo .)com \
--cc=equinox@diac24$(echo .)net \
--cc=kaber@trash$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox