[Lxc-users] uevent when moving nic between network namespaces?

[Serge Hallyn]

Quoting Eric W. Biederman (ebiederm at xmission.com):
> Serge Hallyn <serge.hallyn at canonical.com> writes:
> 
> > Quoting Eric W. Biederman (ebiederm at xmission.com):
> >> Serge Hallyn <serge.hallyn at canonical.com> writes:
> >> 
> >> > Quoting Eric W. Biederman (ebiederm at xmission.com):
> >> >> I am not currently working on a patch for this, but I will be happy to
> >> >> review one. At a quick glance it looks like this could just be as
> >> >> simple as calling kobject_uevent at the proper time, but testing and
> >> >> reading through the relevant code paths is probably a good idea as there
> >> >> always seems to be gotchas in that code.
> >> >> 
> >> >> Eric
> >> >
> >> > This (the simple fix) works for me, actually.
> >> >
> >> > I do notice the ifdef shouldn't be needed, all the better.
> >> 
> >> Should we have a KOBJ_ADD in the new network namespace or is the
> >> KOBJ_MOVE sufficient?
> >
> > I was wondering about that...  the KOBJ_ADD is technically not sufficient
> > imo, since a MOVE (for a device which udev/upstart has never seen before)
> > doesn't necessarily mean "configure this."  So when I pass one end of a
> > veth into a running ubuntu container, there is no network-interface or
> > network-interface-security upstart job for it, whereas if I do a
> > ip link add type veth inside the container, those do get the jobs.
> >
> > Now, ISTM passing an endpoing into a container is mainly done at
> > startup, and upstart will end up configuring it anyway.  Nothing is
> > really breaking in any of the container usages I've seen because of this.
> > But it would definately be cleaner to pass a KOBJ_ADD before the KOBJ_MOVE.
> > Otherwise, udev has to guess what the MOVE meant.
> >
> > If there's no objection, I'll add that (and test it) and send to netdev
> > on monday.
> 
> Sounds good.  Right now I have the suspicion we might want our own
> variant on sysfs_move that sends these instead of the move...
> 
> But let's confirm things work better with add/remove before we go crazy
> on the best way to generate maintainable code.

Yup all still looks good with the following trivial patch.  And now when
I pass a netdev into a running container, it gets a network-interface
upstart job just as it does on a real host.

And no network-interface jobs stick around after the container shuts
down, meaning this solves the kernel part of bug 1065589
(https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/1065589).

(Pre-existing nics don't get a network-interface job - the fact that lxc
first passes in the netdevs and then execs init therefore still causes
some asymmetry wrt a real host, where netdevs always come up after init
starts.  AFAIK we don't care, but Stéphane might know of a reason why we
do  - in either case it's not the kernel's problem)

>From 01dc08273fa63a50f6dbb7377397ec52a7a337f8 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at canonical.com>
Date: Fri, 12 Oct 2012 21:42:05 +0100
Subject: [PATCH 1/1] dev_change_net_namespace: send a KOBJ_REMOVED to
 original netns

v2: also send KOBJ_ADD to new netns.  There will then be a
_MOVE event from the device_rename() call, but that should
be innocuous.

Signed-off-by: Serge Hallyn <serge.hallyn at canonical.com>
---
 net/core/dev.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index e2215ee..2c43aaf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6172,6 +6172,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
 	dev_uc_flush(dev);
 	dev_mc_flush(dev);
 
+	/* Send a netdev-removed uevent to the old namespace */
+	kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
+
 	/* Actually switch the network namespace */
 	dev_net_set(dev, net);
 
@@ -6183,6 +6186,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
 			dev->iflink = dev->ifindex;
 	}
 
+	/* Send a netdev-add uevent to the new namespace */
+	kobject_uevent(&dev->dev.kobj, KOBJ_ADD);
+
 	/* Fixup kobjects */
 	err = device_rename(&dev->dev, dev->name);
 	WARN_ON(err);
-- 
1.7.10.4