Network namespace and bonding WARNING at fs/proc/generic.c remove_proc_entry

Hi,

I'd discussed the following with Serge Hallyn.

=> Environment based on 3.2.18 / x86_64 kernel.
=> WARNING: at fs/proc/generic.c:808 remove_proc_entry+0xdb/0x21f() => WARNING: at fs/proc/generic.c:849 remove_proc_entry+0x208/0x21f()

network namespace and bonding
-----------------------------

* Migrate two phy nics from host to netns (netns0).
- ip link set ethX netns netns0

* In host environment:
- load bonding module, /sbin/modprobe -v bonding mode=1 miimon=100
- /sys/class/net/bond0 exists.
- /proc/net/bonding/bond0 exists.
- /sys/class/net/bonding_masters has bond0.

* Migrate bond0 to netns (netns0):
- ip link set bond0 netns netns0.

* Within netns (netns0):
- /sys/class/net/bonding_masters is empty.
- /sys/class/net/bond0 exist.
- configure bond0 and ifenslave with two phy nics.
- /proc/net/bonding/bond0 does not exist within netns0, but does exist in the host environment.
- /sys/class/net/bonding_masters is empty.

- ping to remote end of bond0 works.

* Within netns (netns0), flushing ethX and bondY:
- down bond0 and its phy nic interfaces:
- ip link set ... down
- ip addr flush dev [bond0 | eth#]
- deleting bond0, /sbin/ip link del dev bond0

Quoted 7 lines · Dilip Daya

Hi,

I'd discussed the following with Serge Hallyn.

=> Environment based on 3.2.18 / x86_64 kernel.
=> WARNING: at fs/proc/generic.c:808 remove_proc_entry+0xdb/0x21f() => WARNING: at fs/proc/generic.c:849 remove_proc_entry+0x208/0x21f()

Hi,

thanks much for sending this. I'm still getting this error on 3.5.0-2-generic (today's ubuntu quantal kernel)

Quoted 15 lines

network namespace and bonding
-----------------------------

* Migrate two phy nics from host to netns (netns0).
- ip link set ethX netns netns0

* In host environment:
- load bonding module, /sbin/modprobe -v bonding mode=1 miimon=100
- /sys/class/net/bond0 exists.
- /proc/net/bonding/bond0 exists.
- /sys/class/net/bonding_masters has bond0.

* Migrate bond0 to netns (netns0):
- ip link set bond0 netns netns0.

Quoted 7 lines

* Within netns (netns0):
- /sys/class/net/bonding_masters is empty.
- /sys/class/net/bond0 exist.
- configure bond0 and ifenslave with two phy nics.
- /proc/net/bonding/bond0 does not exist within netns0, but does exist in the host environment.
- /sys/class/net/bonding_masters is empty.

mine is not empty, fwiw. However

Quoted 2 lines

- ping to remote end of bond0 works.

Quoted 5 lines

* Within netns (netns0), flushing ethX and bondY:
- down bond0 and its phy nic interfaces:
- ip link set ... down
- ip addr flush dev [bond0 | eth#]
- deleting bond0, /sbin/ip link del dev bond0

Yup I still get a remove_proc_entry WARNING at fs/proc/generic.c:808, which is the warning when (!de)

From the warning it looks like the proc files are being added/removed

Quoted 7 lines · Dilip Daya

Hi,

I'd discussed the following with Serge Hallyn.

=> Environment based on 3.2.18 / x86_64 kernel.
=> WARNING: at fs/proc/generic.c:808 remove_proc_entry+0xdb/0x21f() => WARNING: at fs/proc/generic.c:849 remove_proc_entry+0x208/0x21f()

Hi,

thanks much for sending this. I'm still getting this error on 3.5.0-2-generic (today's ubuntu quantal kernel)

Quoted 15 lines

network namespace and bonding
-----------------------------

* Migrate two phy nics from host to netns (netns0).
- ip link set ethX netns netns0

* In host environment:
- load bonding module, /sbin/modprobe -v bonding mode=1 miimon=100
- /sys/class/net/bond0 exists.
- /proc/net/bonding/bond0 exists.
- /sys/class/net/bonding_masters has bond0.

* Migrate bond0 to netns (netns0):
- ip link set bond0 netns netns0.

Quoted 7 lines

* Within netns (netns0):
- /sys/class/net/bonding_masters is empty.
- /sys/class/net/bond0 exist.
- configure bond0 and ifenslave with two phy nics.
- /proc/net/bonding/bond0 does not exist within netns0, but does exist in the host environment.
- /sys/class/net/bonding_masters is empty.

mine is not empty, fwiw. However

Quoted 2 lines

- ping to remote end of bond0 works.

Quoted 5 lines

* Within netns (netns0), flushing ethX and bondY:
- down bond0 and its phy nic interfaces:
- ip link set ... down
- ip addr flush dev [bond0 | eth#]
- deleting bond0, /sbin/ip link del dev bond0

Yup I still get a remove_proc_entry WARNING at fs/proc/generic.c:808, which is the warning when (!de)

Quoted 41 lines · Serge E. Hallyn

Quoted 7 lines · Dilip Daya

Hi,

I'd discussed the following with Serge Hallyn.

=> Environment based on 3.2.18 / x86_64 kernel.
=> WARNING: at fs/proc/generic.c:808 remove_proc_entry+0xdb/0x21f() => WARNING: at fs/proc/generic.c:849 remove_proc_entry+0x208/0x21f()

Hi,

thanks much for sending this. I'm still getting this error on 3.5.0-2-generic (today's ubuntu quantal kernel)

Quoted 15 lines

network namespace and bonding
-----------------------------

* Migrate two phy nics from host to netns (netns0).
- ip link set ethX netns netns0

* In host environment:
- load bonding module, /sbin/modprobe -v bonding mode=1 miimon=100
- /sys/class/net/bond0 exists.
- /proc/net/bonding/bond0 exists.
- /sys/class/net/bonding_masters has bond0.

* Migrate bond0 to netns (netns0):
- ip link set bond0 netns netns0.

Quoted 7 lines

* Within netns (netns0):
- /sys/class/net/bonding_masters is empty.
- /sys/class/net/bond0 exist.
- configure bond0 and ifenslave with two phy nics.
- /proc/net/bonding/bond0 does not exist within netns0, but does exist in the host environment.
- /sys/class/net/bonding_masters is empty.

mine is not empty, fwiw. However

Quoted 2 lines

- ping to remote end of bond0 works.

Quoted 8 lines

Quoted 5 lines

* Within netns (netns0), flushing ethX and bondY:
- down bond0 and its phy nic interfaces:
- ip link set ... down
- ip addr flush dev [bond0 | eth#]
- deleting bond0, /sbin/ip link del dev bond0

Yup I still get a remove_proc_entry WARNING at fs/proc/generic.c:808, which is the warning when (!de)

It looks like Dilip is running an old kernel. There should have been some version of /sys/class/net/bonding_masters in every network namespace since sometime in 2009.

Quoted 1 line

From the warning it looks like the proc files are being added/removed

to the wrong network namespace. So in one namespace we get an error when we delete the moved device and in the other network namespace we get an error when we remove the /proc/directory.

An old kernel without proper network namespace support is the only reason I can imagine someone would be moving an existing bond device between network namespaces.

If there are other reasons for wanting to move a bonding device between network namespaces it is possible to catch the NETDEV_UNREGISTER and NETDEV_REGISTER events to remove/add the per device proc files at the appropriate time.

However since moving bonding devices appears to be an unneded operation let's just do things simply and forbid moving bonding devices between network namespaces. Serge, Dilip can you two test the patch below and see if it fixes the warnings.

Eric

drivers/net/bonding/bond_main.c +0 -0

index 2ee8cf9..818ed64 100644

drivers/net/bonding/bond_main.c +1 -0

+
        /* At first, we block adding VLANs. That's the only way to
         * prevent problems that occur when adding VLANs over an
         * empty bond. The block will be removed once non-challenged

Quoted 41 lines · Serge E. Hallyn

Quoted 7 lines · Dilip Daya

Hi,

I'd discussed the following with Serge Hallyn.

=> Environment based on 3.2.18 / x86_64 kernel.
=> WARNING: at fs/proc/generic.c:808 remove_proc_entry+0xdb/0x21f() => WARNING: at fs/proc/generic.c:849 remove_proc_entry+0x208/0x21f()

Hi,

thanks much for sending this. I'm still getting this error on 3.5.0-2-generic (today's ubuntu quantal kernel)

Quoted 15 lines

network namespace and bonding
-----------------------------

* Migrate two phy nics from host to netns (netns0).
- ip link set ethX netns netns0

* In host environment:
- load bonding module, /sbin/modprobe -v bonding mode=1 miimon=100
- /sys/class/net/bond0 exists.
- /proc/net/bonding/bond0 exists.
- /sys/class/net/bonding_masters has bond0.

* Migrate bond0 to netns (netns0):
- ip link set bond0 netns netns0.

Quoted 7 lines

* Within netns (netns0):
- /sys/class/net/bonding_masters is empty.
- /sys/class/net/bond0 exist.
- configure bond0 and ifenslave with two phy nics.
- /proc/net/bonding/bond0 does not exist within netns0, but does exist in the host environment.
- /sys/class/net/bonding_masters is empty.

mine is not empty, fwiw. However

Quoted 2 lines

- ping to remote end of bond0 works.

Quoted 8 lines

Quoted 5 lines

* Within netns (netns0), flushing ethX and bondY:
- down bond0 and its phy nic interfaces:
- ip link set ... down
- ip addr flush dev [bond0 | eth#]
- deleting bond0, /sbin/ip link del dev bond0

Yup I still get a remove_proc_entry WARNING at fs/proc/generic.c:808, which is the warning when (!de)

It looks like Dilip is running an old kernel. There should have been some version of /sys/class/net/bonding_masters in every network namespace since sometime in 2009.

Quoted 1 line

From the warning it looks like the proc files are being added/removed

to the wrong network namespace. So in one namespace we get an error when we delete the moved device and in the other network namespace we get an error when we remove the /proc/directory.

An old kernel without proper network namespace support is the only reason I can imagine someone would be moving an existing bond device between network namespaces.

If there are other reasons for wanting to move a bonding device between network namespaces it is possible to catch the NETDEV_UNREGISTER and NETDEV_REGISTER events to remove/add the per device proc files at the appropriate time.

However since moving bonding devices appears to be an unneded operation let's just do things simply and forbid moving bonding devices between network namespaces. Serge, Dilip can you two test the patch below and see if it fixes the warnings.

Eric

drivers/net/bonding/bond_main.c +3 -0

index 2ee8cf9..818ed64 100644
@@ -4345,6 +4345,9 @@ static void bond_setup(struct net_device *bond_dev)
        bond_dev->priv_flags |= IFF_BONDING;
        bond_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 
+       /* Don't allow bond devices to change network namespaces. */
+       bond_dev->features |= NETIF_F_LOCAL;
+
        /* At first, we block adding VLANs. That's the only way to
         * prevent problems that occur when adding VLANs over an
         * empty bond. The block will be removed once non-challenged

Quoted 86 lines · Eric W. Biederman

Quoted 41 lines · Serge E. Hallyn

Quoted 7 lines · Dilip Daya

Hi,

I'd discussed the following with Serge Hallyn.

=> Environment based on 3.2.18 / x86_64 kernel.
=> WARNING: at fs/proc/generic.c:808 remove_proc_entry+0xdb/0x21f() => WARNING: at fs/proc/generic.c:849 remove_proc_entry+0x208/0x21f()

Hi,

thanks much for sending this. I'm still getting this error on 3.5.0-2-generic (today's ubuntu quantal kernel)

Quoted 15 lines

network namespace and bonding
-----------------------------

* Migrate two phy nics from host to netns (netns0).
- ip link set ethX netns netns0

* In host environment:
- load bonding module, /sbin/modprobe -v bonding mode=1 miimon=100
- /sys/class/net/bond0 exists.
- /proc/net/bonding/bond0 exists.
- /sys/class/net/bonding_masters has bond0.

* Migrate bond0 to netns (netns0):
- ip link set bond0 netns netns0.

Quoted 7 lines

* Within netns (netns0):
- /sys/class/net/bonding_masters is empty.
- /sys/class/net/bond0 exist.
- configure bond0 and ifenslave with two phy nics.
- /proc/net/bonding/bond0 does not exist within netns0, but does exist in the host environment.
- /sys/class/net/bonding_masters is empty.

mine is not empty, fwiw. However

Quoted 2 lines

- ping to remote end of bond0 works.

Quoted 8 lines

Quoted 5 lines

* Within netns (netns0), flushing ethX and bondY:
- down bond0 and its phy nic interfaces:
- ip link set ... down
- ip addr flush dev [bond0 | eth#]
- deleting bond0, /sbin/ip link del dev bond0

Yup I still get a remove_proc_entry WARNING at fs/proc/generic.c:808, which is the warning when (!de)

It looks like Dilip is running an old kernel. There should have been some version of /sys/class/net/bonding_masters in every network namespace since sometime in 2009.

Quoted 1 line

From the warning it looks like the proc files are being added/removed

to the wrong network namespace. So in one namespace we get an error when we delete the moved device and in the other network namespace we get an error when we remove the /proc/directory.

An old kernel without proper network namespace support is the only reason I can imagine someone would be moving an existing bond device between network namespaces.

If there are other reasons for wanting to move a bonding device between network namespaces it is possible to catch the NETDEV_UNREGISTER and NETDEV_REGISTER events to remove/add the per device proc files at the appropriate time.

However since moving bonding devices appears to be an unneded operation let's just do things simply and forbid moving bonding devices between network namespaces. Serge, Dilip can you two test the patch below and see if it fixes the warnings.

Eric

drivers/net/bonding/bond_main.c +0 -0

index 2ee8cf9..818ed64 100644
@@ -4345,6 +4345,9 @@ static void bond_setup(struct net_device *bond_dev)
        bond_dev->priv_flags |= IFF_BONDING;
        bond_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 

drivers/net/bonding/bond_main.c static void bond_setup(struct net_device *bond_dev) · line 4349

Quoted 2 lines

drivers/net/bonding/bond_main.c +2 -0

+       /* Don't allow bond devices to change network namespaces. */
+       bond_dev->features |= NETIF_F_LOCAL;

I believe this needs to be NETIF_F_NETNS_LOCAL. Test build still going with that change.

Hi Eric,

We do need to move bonds between namespaces - because we require physical interfaces in each namespace -- we don't want the overheads of virtual interfaces, don't have the management infrastructure, and don't want to manufacture fake mac addresses that would be required for
macvlan interfaces. Since the bonds are implicitly created in the host
namespace, the only way we know to get bonds directly into the namespaces is to move them.

Would "NETDEV_UNREGISTER and NETDEV_REGISTER events to remove/add the per device proc files at the appropriate time." help in the case?

Hi Eric,

We do need to move bonds between namespaces - because we require physical interfaces in each namespace -- we don't want the overheads of virtual interfaces, don't have the management infrastructure, and don't want to manufacture fake mac addresses that would be required for
macvlan interfaces. Since the bonds are implicitly created in the host
namespace, the only way we know to get bonds directly into the namespaces is to move them.

Would "NETDEV_UNREGISTER and NETDEV_REGISTER events to remove/add the per device proc files at the appropriate time." help in the case?

Quoted 8 lines

drivers/net/bonding/bond_main.c +0 -0

index 2ee8cf9..818ed64 100644
@@ -4345,6 +4345,9 @@ static void bond_setup(struct net_device *bond_dev)
        bond_dev->priv_flags |= IFF_BONDING;
        bond_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 

drivers/net/bonding/bond_main.c static void bond_setup(struct net_device *bond_dev) · line 4349

Quoted 2 lines

drivers/net/bonding/bond_main.c +2 -0

+       /* Don't allow bond devices to change network namespaces. */
+       bond_dev->features |= NETIF_F_LOCAL;

I believe this needs to be NETIF_F_NETNS_LOCAL. Test build still going with that change.

Quoted 8 lines · Serge E. Hallyn

Quoted 8 lines

drivers/net/bonding/bond_main.c +0 -0

index 2ee8cf9..818ed64 100644
@@ -4345,6 +4345,9 @@ static void bond_setup(struct net_device *bond_dev)
        bond_dev->priv_flags |= IFF_BONDING;
        bond_dev->priv_flags &= ~(IFF_XMIT_DST_RELEASE | IFF_TX_SKB_SHARING);
 

Quoted 5 lines

drivers/net/bonding/bond_main.c static void bond_setup(struct net_device *bond_dev) · line 4349

Quoted 2 lines

drivers/net/bonding/bond_main.c +2 -0

+       /* Don't allow bond devices to change network namespaces. */
+       bond_dev->features |= NETIF_F_LOCAL;

I believe this needs to be NETIF_F_NETNS_LOCAL. Test build still going with that change.

Yes that is what I mean.

It was recently reported that moving a bonding device between network namespaces causes warnings from /proc. It turns out after the move we were trying to add and to remove the /proc/net/bonding entries from the wrong network namespace.

Move the bonding /proc registration code into the NETDEV_REGISTER and NETDEV_UNREGISTER events where the proc registration and unregistration will always happen at the right time.

Signed-off-by "Eric W. Biederman" <[email protected]>

It was recently reported that moving a bonding device between network namespaces causes warnings from /proc. It turns out after the move we were trying to add and to remove the /proc/net/bonding entries from the wrong network namespace.

Move the bonding /proc registration code into the NETDEV_REGISTER and NETDEV_UNREGISTER events where the proc registration and unregistration will always happen at the right time.

Signed-off-by "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

The bonding debugfs support has been broken in the presence of network namespaces since it has been added. The debugfs support does not handle multiple bonding devices with the same name in different network namespaces.

I haven't had any bug reports, and I'm not interested in getting any. Disable the debugfs support when network namespaces are enabled.

Signed-off-by "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

From: [email protected] (Eric W. Biederman)
Date: Mon, 09 Jul 2012 13:52:43 -0700

Quoted 6 lines

The bonding debugfs support has been broken in the presence of network namespaces since it has been added. The debugfs support does not handle multiple bonding devices with the same name in different network namespaces.

Quoted 4 lines

I haven't had any bug reports, and I'm not interested in getting any. Disable the debugfs support when network namespaces are enabled.

Signed-off-by "Eric W. Biederman" <[email protected]>

Applied.

From: ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected] (Eric W. Biederman)
Date: Mon, 09 Jul 2012 13:52:43 -0700

Quoted 6 lines

The bonding debugfs support has been broken in the presence of network namespaces since it has been added. The debugfs support does not handle multiple bonding devices with the same name in different network namespaces.

Quoted 4 lines

I haven't had any bug reports, and I'm not interested in getting any. Disable the debugfs support when network namespaces are enabled.

Signed-off-by "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>

Applied.

Quoted 15 lines · Jay Vosburgh

Quoted 5 lines · David Miller

From: [email protected] (Eric W. Biederman)
Date: Mon, 09 Jul 2012 13:52:43 -0700

Quoted 2 lines

The bonding debugfs support has been broken in the presence of

network

Quoted 1 line

Quoted 1 line

namespaces since it has been added. The debugfs support does not

handle

Quoted 4 lines

Quoted 4 lines

multiple bonding devices with the same name in different network namespaces.

I haven't had any bug reports, and I'm not interested in getting

any.

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Disable the debugfs support when network namespaces are enabled.

Signed-off-by "Eric W. Biederman" <[email protected]>

Applied.

Since distro kernels appear to set CONFIG_NET_NS, doesn't this
effectively disable debugfs for bonding on most distros?

Yes.

Quoted 3 lines

Do the other network device drivers that support debugfs have a
similar problem? E.g., if each of two namespaces have an skge device with the same name, will there be a debugfs conflict there as well?

I haven't run across any of those network devices, but if they create a debugfs entry that embeds the device name it will be a problem.

Last I looked any custom user space interface from network devices was rare and bonding using debugfs is the first instance of using debugfs from networking devices I have seen.

I think the problem will be a little less severe for physical network devices as they all start in the initial network namespace and so start with distinct names.

With bonding I can do "ip link add type bond" in any network namespace and get another bond0. So name conflicts are very much expeted with all virtual networking devices.

But if you know of any other networking devices using debugsfs that code should probably get the same treatment as the bonding debugfs code.

Quoted 2 lines · Eric W. Biederman

I haven't run across any of those network devices, but if they create a debugfs entry that embeds the device name it will be a problem.

A quick grep suggests that cxgb4, skge, sky2, stmmac, ipoib and
half a dozen of the wireless drivers all create files in debugfs. I did not check exhaustively, but at least some of them include the device name.

Quoted 8 lines

Last I looked any custom user space interface from network devices was rare and bonding using debugfs is the first instance of using debugfs from networking devices I have seen.

I think the problem will be a little less severe for physical network devices as they all start in the initial network namespace and so start with distinct names.

Quoted 3 lines

With bonding I can do "ip link add type bond" in any network namespace and get another bond0. So name conflicts are very much expeted with all virtual networking devices.

Fair enough, although it is trivial to rename any network device
such that a conflict would occur.

Quoted 2 lines

But if you know of any other networking devices using debugsfs that code should probably get the same treatment as the bonding debugfs code.

Is there no alternative than simply disabling debugfs whenever
network namespaces are enabled? The information bonding displays via debugfs is useful, and having it unavailable on all distro kernels seems a bit harsh.

Why is the logic already in the driver not sufficient? If the
attempt to create the debugfs directory with the interface name fails, then it merely prints a warning and continues without the debugfs for that interface.

Quoted 9 lines · Jay Vosburgh

Quoted 2 lines · Eric W. Biederman

I haven't run across any of those network devices, but if they create a debugfs entry that embeds the device name it will be a problem.

A quick grep suggests that cxgb4, skge, sky2, stmmac, ipoib and
half a dozen of the wireless drivers all create files in debugfs. I did not check exhaustively, but at least some of them include the device name.

Yep. It looks like imperfect habits are common.

Quoted 12 lines

Quoted 11 lines

Last I looked any custom user space interface from network devices was rare and bonding using debugfs is the first instance of using debugfs from networking devices I have seen.

I think the problem will be a little less severe for physical network devices as they all start in the initial network namespace and so start with distinct names.

With bonding I can do "ip link add type bond" in any network namespace and get another bond0. So name conflicts are very much expeted with all virtual networking devices.

Quoted 2 lines

Fair enough, although it is trivial to rename any network device
such that a conflict would occur.

Actually for userspace and administrative reasons frequently it isn't trivial to rename devices.

Quoted 7 lines

Quoted 2 lines

But if you know of any other networking devices using debugsfs that code should probably get the same treatment as the bonding debugfs code.

Is there no alternative than simply disabling debugfs whenever
network namespaces are enabled? The information bonding displays via debugfs is useful, and having it unavailable on all distro kernels seems a bit harsh.

I took a good hard look at debugfs while writing this reply and debufs scares me. It is the kind of code that just about wants to me to throw in the towel seeing no hope of a good solid kernel.

I can definitely open a /sys/kernel/debug/bonding/bond0/rlb_hash_table and delete the bond and then read the file. On a bad day that will oops the kernel, as there is nothing holding a reference to the network device. I think only the BOND_MODE_ALB check makes keeps the kernel from oopsing in my quick tests.

The fact that debugfs is enabled in distro kernels is actually apalling to me. debugfs makes it easy to oops the kernel.

There are lots of alternatives to debugfs on where to put information and the bonding driver already uses most of them.

Quoted 4 lines

Why is the logic already in the driver not sufficient? If the
attempt to create the debugfs directory with the interface name fails, then it merely prints a warning and continues without the debugfs for that interface.

All I know for certain is the existing logic will eventually cause someone doing something reasonable to send me a bug report.

I can see where you are coming from in that the bonding driver debugfs code really was built to gracefully fail and ignore problems of instead of just hapharzardly and sloppily ignore problems. At the same time I can oops the kernel if I try with your debugfs in the bonding driver.

But it causes the code to fail and issue a warning. So if I don't disable the code now, I expect I will get a bug report, and who knows how many sill files in bonding will have in debugfs by then. what silly things bonding may be doing in debugfs by then.