From patchwork Fri Sep 17 13:22:19 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Lamparter X-Patchwork-Id: 65075 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id ACFFCB70A7 for ; Fri, 17 Sep 2010 23:22:38 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753968Ab0IQNWd (ORCPT ); Fri, 17 Sep 2010 09:22:33 -0400 Received: from spaceboyz.net ([87.106.131.203]:35513 "EHLO spaceboyz.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751567Ab0IQNWc (ORCPT ); Fri, 17 Sep 2010 09:22:32 -0400 Received: from jupiter.n2.diac24.net ([2001:8d8:81:5c2:21b:fcff:fe4c:9e6f]) by spaceboyz.net with esmtps (TLSv1:AES256-SHA:256) (Exim 4.71) (envelope-from ) id 1Owata-004C6t-2t; Fri, 17 Sep 2010 15:22:30 +0200 Received: from equinox by jupiter.n2.diac24.net with local (Exim 4.72) (envelope-from ) id 1OwatP-00BUge-Fe; Fri, 17 Sep 2010 15:22:26 +0200 Date: Fri, 17 Sep 2010 15:22:19 +0200 From: David Lamparter To: David Miller Cc: netdev@vger.kernel.org, "Eric W. Biederman" , Daniel Lezcano , David Lamparter Subject: Re: [PATCH] netns: keep vlan slaves on master netns move Message-ID: <20100917132219.GA2363813@jupiter.n2.diac24.net> References: <20100824115056.GA235835@jupiter.n2.diac24.net> <20100913114315.GA859019@jupiter.n2.diac24.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Reviewed-by: "Eric W. Biederman" Acked-by: Daniel Lezcano Acked-by: Patrick McHardy --- Hi David, Q)ueue, F)eedback, A)pply? -David On Tue, Sep 14, 2010 at 07:10:01PM -0700, Eric W. Biederman wrote: > Reviewed-by: "Eric W. Biederman" > > My inclination is that the best way to handle this is to would be to > push the device deletion for vlan and macvlan devices into the device > core, where we could get the advantage of batched deletions. Right > now vlan and macvlan devices are the only devices I know of that cause > other devices to be removed during unregister, so removing that > specialness seems reasonable. > > However not being able to move the primary vlan to a different network > namespace is usability bug with no real alternatives. There are > not any other patches on the table right now, and your patch below > seems obviously correct. Let's get this merged before we forget about > this, bug fix. > > David Lamparter writes: > > On Tue, Aug 24, 2010 at 01:50:56PM +0200, David Lamparter wrote: > >> 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 would like to resurrect this. After the small excursion into > > NETREG_NETNS_MOVING i've reevaluated my personal opinion and think the > > behaviour is the desired one and this patch is the cleanest way to > > implement it. While it depends on somewhat "fine print" on the reg_state > > state machine, the probability of major screw-ups in this region is > > rather small and it also quite likely makes a nice large boom when it is > > unduly tampered with. drivers/net/macvlan.c | 4 ++++ net/8021q/vlan.c | 4 ++++ net/core/dev.c | 4 ++++ 3 files changed, 12 insertions(+), 0 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 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; + list_for_each_entry_safe(vlan, next, &port->vlans, list) vlan->dev->rtnl_link_ops->dellink(vlan->dev, NULL); break; diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c index 3c1c8c1..cdc37e8 100644 --- a/net/8021q/vlan.c +++ b/net/8021q/vlan.c @@ -516,6 +516,10 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event, break; case NETDEV_UNREGISTER: + /* twiddle thumbs on netns device moves */ + if (dev->reg_state != NETREG_UNREGISTERING) + break; + /* Delete all VLANs for this dev. */ grp->killall = 1; diff --git a/net/core/dev.c b/net/core/dev.c index 1f466e8..b1589bc 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5618,6 +5618,10 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char /* Notify protocols, that we are about to destroy this device. They should clean all the things. + + Note that dev->reg_state stays at NETREG_REGISTERED. + This is wanted because this way 8021q and macvlan know + the device is just moving and can keep their slaves up. */ call_netdevice_notifiers(NETDEV_UNREGISTER, dev); call_netdevice_notifiers(NETDEV_UNREGISTER_BATCH, dev);