[PATCH] netns: Delete virtual interfaces during namespace cleanup

[PATCH] netns: Delete virtual interfaces during namespace cleanup

[Eric W. Biederman]

When physical devices are inside of network namespace and that network
namespace terminates we have to keep them and moving them to the
initial network namespace is the best we can do.

For virtual devices left in a network namespace that is exiting
we have no need to preserve them and we now have the infrastructure
that allows us to delete them.  So delete virtual devices when we
exit a network namespace.  Keeping the necessary user space clean up
after a network namespace exits much more tractable.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 net/core/dev.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index ea480a4..670abad 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4492,6 +4492,12 @@ static void __net_exit default_device_exit(struct net *net)
 		if (dev->features & NETIF_F_NETNS_LOCAL)
 			continue;
 
+		/* Delete virtual devices */
+		if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink) {
+			dev->rtnl_link_ops->dellink(dev);
+			continue;
+		}
+
 		/* Push remaing network devices to init_net */
 		snprintf(fb_name, IFNAMSIZ, "dev%d", dev->ifindex);
 		err = dev_change_net_namespace(dev, &init_net, fb_name);
-- 
1.5.3.rc6.17.g1911

[PATCH] netns: Delete virtual interfaces during namespace cleanup

[Eric W. Biederman]

When physical devices are inside of network namespace and that network
namespace terminates we have to keep them and moving them to the
initial network namespace is the best we can do.

For virtual devices left in a network namespace that is exiting
we have no need to preserve them and we now have the infrastructure
that allows us to delete them.  So delete virtual devices when we
exit a network namespace.  Keeping the necessary user space clean up
after a network namespace exits much more tractable.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 net/core/dev.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index ea480a4..670abad 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4492,6 +4492,12 @@ static void __net_exit default_device_exit(struct net *net)
 		if (dev->features & NETIF_F_NETNS_LOCAL)
 			continue;
 
+		/* Delete virtual devices */
+		if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink) {
+			dev->rtnl_link_ops->dellink(dev);
+			continue;
+		}
+
 		/* Push remaing network devices to init_net */
 		snprintf(fb_name, IFNAMSIZ, "dev%d", dev->ifindex);
 		err = dev_change_net_namespace(dev, &init_net, fb_name);
-- 
1.5.3.rc6.17.g1911

[PATCH] netns: Delete virtual interfaces during namespace cleanup

[Eric W. Biederman]

When physical devices are inside of network namespace and that
network namespace terminates we can not make them go away.  We
have to keep them and moving them to the initial network namespace
is the best we can do.

For virtual devices left in a network namespace that is exiting
we have no need to preserve them and we now have the infrastructure
that allows us to delete them.  So delete virtual devices when we
exit a network namespace.  Keeping the necessary user space clean up
after a network namespace exits much more tractable.

This patch removes much of the need for user space clean up code to
run after a network namespace exits.

Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
 net/core/dev.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 7091040..f3476d4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4844,6 +4844,12 @@ static void __net_exit default_device_exit(struct net *net)
 		if (dev->features & NETIF_F_NETNS_LOCAL)
 			continue;
 
+		/* Delete virtual devices */
+		if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink) {
+			dev->rtnl_link_ops->dellink(dev);
+			continue;
+		}
+
 		/* Push remaing network devices to init_net */
 		snprintf(fb_name, IFNAMSIZ, "dev%d", dev->ifindex);
 		err = dev_change_net_namespace(dev, &init_net, fb_name);
-- 
1.5.3.rc6.17.g1911

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Daniel Lezcano]

Eric W. Biederman wrote:
> When physical devices are inside of network namespace and that
> network namespace terminates we can not make them go away.  We
> have to keep them and moving them to the initial network namespace
> is the best we can do.
> 
> For virtual devices left in a network namespace that is exiting
> we have no need to preserve them and we now have the infrastructure
> that allows us to delete them.  So delete virtual devices when we
> exit a network namespace.  Keeping the necessary user space clean up
> after a network namespace exits much more tractable.
> 
> This patch removes much of the need for user space clean up code to
> run after a network namespace exits.

I agree that will make the life easier for user space developer :)

I have a few questions about this new behaviour.

After discussing with Benjamin, this patch means an user can no longer 
manage a pool of virtual devices because they will be automatically 
destroyed when the namespace exits. I don't think it is a big concern, 
but just in case I am asking :)

Another point, at present, the virtual devices go back to the initial 
network namespace when the namespace dies, and this behaviour is used to 
track the namespace life cycle. With this patch we have no way to know 
when the network namespace has exited. So we can have the last process 
exiting the network namespace, but the network namespace can stay alive 
(eg. some sockets still have buffer to send) ?

AFAIR, Pavel told us about a patch extending the "wait" semantic and 
pass namespace options to wait for. Is that right Pavel ? Shouldn't we 
apply this path before deleting the virtual network devices ?

Thanks.
   -- Daniel

> Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
> ---
>  net/core/dev.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7091040..f3476d4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4844,6 +4844,12 @@ static void __net_exit default_device_exit(struct net *net)
>  		if (dev->features & NETIF_F_NETNS_LOCAL)
>  			continue;
> 
> +		/* Delete virtual devices */
> +		if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink) {
> +			dev->rtnl_link_ops->dellink(dev);
> +			continue;
> +		}
> +
>  		/* Push remaing network devices to init_net */
>  		snprintf(fb_name, IFNAMSIZ, "dev%d", dev->ifindex);
>  		err = dev_change_net_namespace(dev, &init_net, fb_name);

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Eric W. Biederman]

Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org> writes:

> I agree that will make the life easier for user space developer :)
>
> I have a few questions about this new behaviour.
>
> After discussing with Benjamin, this patch means an user can no longer manage a
> pool of virtual devices because they will be automatically destroyed when the
> namespace exits. I don't think it is a big concern, but just in case I am asking
> :)
>
> Another point, at present, the virtual devices go back to the initial network
> namespace when the namespace dies, and this behaviour is used to track the
> namespace life cycle. With this patch we have no way to know when the network
> namespace has exited. So we can have the last process exiting the network
> namespace, but the network namespace can stay alive (eg. some sockets still have
> buffer to send) ?

Depending on the network interfaces going back to the initial network namespace
is problematic in the long term because is breaks recursive containers.  I can't
see any behavior that does that as anything other than a bug.  Especially
as I already have production uses for recursive containers.

> AFAIR, Pavel told us about a patch extending the "wait" semantic and pass
> namespace options to wait for. Is that right Pavel ? Shouldn't we apply this
> path before deleting the virtual network devices ?

Tell you what.  I will post in a bit my patchset that makes /proc/net
it's own filesystem, that magically mounts on /proc/self/net.  It is a
long stupid story why I haven't posted it publicly.  At which point we have
complete visibility into when a network namespace exits and if we need to
be able to wait for it we can just implement a poll method on the root
directory.

That is simple and will work quite nicely in the long term, and gives
us a lot more visibility than a simple this namespace has exited bit.

Eric

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Daniel Lezcano]

Eric W. Biederman wrote:
> Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org> writes:
> 
>> I agree that will make the life easier for user space developer :)
>>
>> I have a few questions about this new behaviour.
>>
>> After discussing with Benjamin, this patch means an user can no longer manage a
>> pool of virtual devices because they will be automatically destroyed when the
>> namespace exits. I don't think it is a big concern, but just in case I am asking
>> :)
>>
>> Another point, at present, the virtual devices go back to the initial network
>> namespace when the namespace dies, and this behaviour is used to track the
>> namespace life cycle. With this patch we have no way to know when the network
>> namespace has exited. So we can have the last process exiting the network
>> namespace, but the network namespace can stay alive (eg. some sockets still have
>> buffer to send) ?
> 
> Depending on the network interfaces going back to the initial network namespace
> is problematic in the long term because is breaks recursive containers.  I can't
> see any behavior that does that as anything other than a bug.  

Perhaps, I am misunderstanding your sentence :) But just in case, let me 
clarify my idea.

If you have a TCP connection with a send queue not empty (the kernel has 
buffered the data the application has sent), when your process exits 
because the last socket write was done in userspace, the TCP socket will 
be orphan but still there taking a ref count on your network namespace. 
Hence the process and the network namespace life cycles is not 
correlated. This is a correct behavior, it is not a bug.

> Especially
> as I already have production uses for recursive containers.
> 
>> AFAIR, Pavel told us about a patch extending the "wait" semantic and pass
>> namespace options to wait for. Is that right Pavel ? Shouldn't we apply this
>> path before deleting the virtual network devices ?
> 
> Tell you what.  I will post in a bit my patchset that makes /proc/net
> it's own filesystem, that magically mounts on /proc/self/net.  It is a
> long stupid story why I haven't posted it publicly.  At which point we have
> complete visibility into when a network namespace exits and if we need to
> be able to wait for it we can just implement a poll method on the root
> directory.

Can a process outside of the network namespace look at this directory 
even if there are no more processes running inside the namespace ?

> That is simple and will work quite nicely in the long term, and gives
> us a lot more visibility than a simple this namespace has exited bit.

This is nice :)

Shouldn't you send /proc/net filesystem patch before sending the virtual 
network devices autodestroy patch ?

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Eric W. Biederman]

Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org> writes:

> Perhaps, I am misunderstanding your sentence :) But just in case, let me clarify
> my idea.
>
> If you have a TCP connection with a send queue not empty (the kernel has
> buffered the data the application has sent), when your process exits because the
> last socket write was done in userspace, the TCP socket will be orphan but still
> there taking a ref count on your network namespace. Hence the process and the
> network namespace life cycles is not correlated. This is a correct behavior, it
> is not a bug.

Agreed, letting the network stack finish sending the last bits of data from
sockets is not a bug.

The problem I see is having a veth device that exists in a child container,
and the application waiting for the child device to return to the current network
namespace.  Since we always return network devices to the initial network namespace
waiting for a device in a child container does not work in a recursive container.
And thus is a bug.

> Can a process outside of the network namespace look at this directory even if
> there are no more processes running inside the namespace ?

If you mount it somewhere besides /proc/self/net yes.

Say: mount --bind /proc/self/net /tmp/net

>> That is simple and will work quite nicely in the long term, and gives
>> us a lot more visibility than a simple this namespace has exited bit.
>
> This is nice :)
>
> Shouldn't you send /proc/net filesystem patch before sending the virtual network
> devices autodestroy patch ?

Well there is the cheap trick with this patch of waiting until the local end
of veth dies.

Eric

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Daniel Lezcano]

Eric W. Biederman wrote:
> Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org> writes:
> 
>> Perhaps, I am misunderstanding your sentence :) But just in case, let me clarify
>> my idea.
>>
>> If you have a TCP connection with a send queue not empty (the kernel has
>> buffered the data the application has sent), when your process exits because the
>> last socket write was done in userspace, the TCP socket will be orphan but still
>> there taking a ref count on your network namespace. Hence the process and the
>> network namespace life cycles is not correlated. This is a correct behavior, it
>> is not a bug.
> 
> Agreed, letting the network stack finish sending the last bits of data from
> sockets is not a bug.
> 
> The problem I see is having a veth device that exists in a child container,
> and the application waiting for the child device to return to the current network
> namespace.  Since we always return network devices to the initial network namespace
> waiting for a device in a child container does not work in a recursive container.
> And thus is a bug.

I agree.

>> Can a process outside of the network namespace look at this directory even if
>> there are no more processes running inside the namespace ?
> 
> If you mount it somewhere besides /proc/self/net yes.
> 
> Say: mount --bind /proc/self/net /tmp/net

Ok, thanks.

>>> That is simple and will work quite nicely in the long term, and gives
>>> us a lot more visibility than a simple this namespace has exited bit.
>> This is nice :)
>>
>> Shouldn't you send /proc/net filesystem patch before sending the virtual network
>> devices autodestroy patch ?
> 
> Well there is the cheap trick with this patch of waiting until the local end
> of veth dies.

I actually use veth, macvlan, empty netns and physical. But if you are 
planning the send netns fs soon, I guess I can live with that a time.

Thanks.
   -- Daniel

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[David Miller]

From: Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
Date: Tue, 07 Oct 2008 14:07:00 +0200

> Eric W. Biederman wrote:
> > Well there is the cheap trick with this patch of waiting until the local end
> > of veth dies.
>
> I actually use veth, macvlan, empty netns and physical. But if you
> are planning the send netns fs soon, I guess I can live with that a
> time.

Are we anywhere near consensus with this patch?

Please resubmit once there is some agreement.

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Daniel Lezcano]

David Miller wrote:
> From: Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
> Date: Tue, 07 Oct 2008 14:07:00 +0200
> 
>> Eric W. Biederman wrote:
>>> Well there is the cheap trick with this patch of waiting until the local end
>>> of veth dies.
>> I actually use veth, macvlan, empty netns and physical. But if you
>> are planning the send netns fs soon, I guess I can live with that a
>> time.
> 
> Are we anywhere near consensus with this patch?
> 
> Please resubmit once there is some agreement.

I am fine with this patch.

Acked-by: Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Pavel Emelyanov]

Daniel Lezcano wrote:
> Eric W. Biederman wrote:
>> When physical devices are inside of network namespace and that
>> network namespace terminates we can not make them go away.  We
>> have to keep them and moving them to the initial network namespace
>> is the best we can do.
>>
>> For virtual devices left in a network namespace that is exiting
>> we have no need to preserve them and we now have the infrastructure
>> that allows us to delete them.  So delete virtual devices when we
>> exit a network namespace.  Keeping the necessary user space clean up
>> after a network namespace exits much more tractable.
>>
>> This patch removes much of the need for user space clean up code to
>> run after a network namespace exits.
> 
> I agree that will make the life easier for user space developer :)
> 
> I have a few questions about this new behaviour.
> 
> After discussing with Benjamin, this patch means an user can no longer 
> manage a pool of virtual devices because they will be automatically 
> destroyed when the namespace exits. I don't think it is a big concern, 
> but just in case I am asking :)
> 
> Another point, at present, the virtual devices go back to the initial 
> network namespace when the namespace dies, and this behaviour is used to 
> track the namespace life cycle. With this patch we have no way to know 
> when the network namespace has exited. So we can have the last process 
> exiting the network namespace, but the network namespace can stay alive 
> (eg. some sockets still have buffer to send) ?
> 
> AFAIR, Pavel told us about a patch extending the "wait" semantic and 
> pass namespace options to wait for. Is that right Pavel ? Shouldn't we 
> apply this path before deleting the virtual network devices ?

I remember that I promised to prepare the wait-extending patch. But I
haven't manage to find time for this, sorry :( In a month or two I will
finish one time-hungry task and hopefully be able to do it.

As far as this particular patch is concerned.

All the virtual devices we have now in namespaces (vlan and tunnels)
kill themselves *before* this code is called. But even if we try to
move this destruction from modules to here, we'll be in a tricky
situation, when the e.g. ipip module has already kfree-d the net_ipip
structure, while none of ipip devices are released yet.

If we try to look in the future - if we ever have a virtual device
driver, that will be able to create its devices in namespace, we'll
have to destroy all these devices *before* (or inside) this driver's
net->exit callback is called, but this patch dies the ->dellink call
at the very end, i.e. *after* any potential ->exit callback.

Eric - did you see any device, that was ->dellink-ed by this patch?

> Thanks.
>    -- Daniel
> 
>> Signed-off-by: Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
>> ---
>>  net/core/dev.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 7091040..f3476d4 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4844,6 +4844,12 @@ static void __net_exit default_device_exit(struct net *net)
>>  		if (dev->features & NETIF_F_NETNS_LOCAL)
>>  			continue;
>>
>> +		/* Delete virtual devices */
>> +		if (dev->rtnl_link_ops && dev->rtnl_link_ops->dellink) {
>> +			dev->rtnl_link_ops->dellink(dev);
>> +			continue;
>> +		}
>> +
>>  		/* Push remaing network devices to init_net */
>>  		snprintf(fb_name, IFNAMSIZ, "dev%d", dev->ifindex);
>>  		err = dev_change_net_namespace(dev, &init_net, fb_name);
> 

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Eric W. Biederman]

Pavel Emelyanov <xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> writes:

> I remember that I promised to prepare the wait-extending patch. But I
> haven't manage to find time for this, sorry :( In a month or two I will
> finish one time-hungry task and hopefully be able to do it.
>
> As far as this particular patch is concerned.
>
> All the virtual devices we have now in namespaces (vlan and tunnels)
> kill themselves *before* this code is called. But even if we try to
> move this destruction from modules to here, we'll be in a tricky
> situation, when the e.g. ipip module has already kfree-d the net_ipip
> structure, while none of ipip devices are released yet.
>
> If we try to look in the future - if we ever have a virtual device
> driver, that will be able to create its devices in namespace, we'll
> have to destroy all these devices *before* (or inside) this driver's
> net->exit callback is called, but this patch dies the ->dellink call
> at the very end, i.e. *after* any potential ->exit callback.
>
> Eric - did you see any device, that was ->dellink-ed by this patch?

macvlan, veth, and dummy interfaces.  Basically
everything I use this deletes the virtual interfaces, instead
of sending them back to the initial network namespace.

From the looks of the code vlans are also be handled.

Things like tun/tap and ipip are handled in the exit methods and I
have no problem with that, although I do wonder if we are handling
moving or refusing to move them between namespaces properly.

For devices that don't need a dedicated virtual interface in
every network namespace this certainly looks like the easiest
way to handle them, as the driver doesn't need to a thing
about network namespaces and the right thing just happens.

Eric

Re: [PATCH] netns: Delete virtual interfaces during namespace cleanup

[Pavel Emelyanov]

Eric W. Biederman wrote:
> Pavel Emelyanov <xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> writes:
> 
>> I remember that I promised to prepare the wait-extending patch. But I
>> haven't manage to find time for this, sorry :( In a month or two I will
>> finish one time-hungry task and hopefully be able to do it.
>>
>> As far as this particular patch is concerned.
>>
>> All the virtual devices we have now in namespaces (vlan and tunnels)
>> kill themselves *before* this code is called. But even if we try to
>> move this destruction from modules to here, we'll be in a tricky
>> situation, when the e.g. ipip module has already kfree-d the net_ipip
>> structure, while none of ipip devices are released yet.
>>
>> If we try to look in the future - if we ever have a virtual device
>> driver, that will be able to create its devices in namespace, we'll
>> have to destroy all these devices *before* (or inside) this driver's
>> net->exit callback is called, but this patch dies the ->dellink call
>> at the very end, i.e. *after* any potential ->exit callback.
>>
>> Eric - did you see any device, that was ->dellink-ed by this patch?
> 
> macvlan, veth, and dummy interfaces.  Basically
> everything I use this deletes the virtual interfaces, instead
> of sending them back to the initial network namespace.

OK, then.

Acked-by: Pavel Emelyanov <xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>

>>From the looks of the code vlans are also be handled.
> 
> Things like tun/tap and ipip are handled in the exit methods and I
> have no problem with that, although I do wonder if we are handling
> moving or refusing to move them between namespaces properly.
> 
> For devices that don't need a dedicated virtual interface in
> every network namespace this certainly looks like the easiest
> way to handle them, as the driver doesn't need to a thing
> about network namespaces and the right thing just happens.
> 
> Eric
>