BUG: net-next netns teardown bridge bug "waiting for lo to become free."

BUG: net-next netns teardown bridge bug "waiting for lo to become free."

[David Lamparter]

(no idea who to Cc on this...)

Hi,


i've recently been playing with network namespaces under net-next
feb8f478... and have been able to reproducibly cause:

netconsole: network logging stopped, interface lo unregistered
netconsole: network logging stopped, interface lo unregistered
unregister_netdevice: waiting for lo to become free. Usage count = 4
netconsole: network logging stopped, interface lo unregistered
(repeat.)

i can trigger this with

unshare -n -- /bin/bash -c 'ip link add link lo veth0 type veth peer
name veth3 netns 1; brctl addbr br0; ip l s br0 up; ip a l; sleep 8'

(It happens when the netns is going down.)

Without the "ip l s br0 up" it does not happen; the veth I have added
for "notification" when the namespace goes down, it is probably not
related.

I'm looking into this, but i'm not versed in Linux kernel code, so my
luck in fixing this might be limited. Some reference taken in bridge
"up"? Should the bridge be set "down" on netns teardown?


-David

Re: BUG: net-next netns teardown bridge bug "waiting for lo to become free."

[Stephen Hemminger]

On Tue, 27 Jul 2010 17:25:14 +0200
David Lamparter <equinox@diac24.net> wrote:

> (no idea who to Cc on this...)
> 
> Hi,
> 
> 
> i've recently been playing with network namespaces under net-next
> feb8f478... and have been able to reproducibly cause:
> 
> netconsole: network logging stopped, interface lo unregistered
> netconsole: network logging stopped, interface lo unregistered
> unregister_netdevice: waiting for lo to become free. Usage count = 4
> netconsole: network logging stopped, interface lo unregistered
> (repeat.)
> 
> i can trigger this with
> 
> unshare -n -- /bin/bash -c 'ip link add link lo veth0 type veth peer
> name veth3 netns 1; brctl addbr br0; ip l s br0 up; ip a l; sleep 8'
> 
> (It happens when the netns is going down.)
> 
> Without the "ip l s br0 up" it does not happen; the veth I have added
> for "notification" when the namespace goes down, it is probably not
> related.
> 
> I'm looking into this, but i'm not versed in Linux kernel code, so my
> luck in fixing this might be limited. Some reference taken in bridge
> "up"? Should the bridge be set "down" on netns teardown?

Loopback device is special and is referenced by routes etc.
You need to manually remove bridge, and all routes, neighbors and cache
before removing namespace.



-- 

Re: BUG: net-next netns teardown bridge bug "waiting for lo to become free."

[David Lamparter]

On Tue, Jul 27, 2010 at 10:02:49AM -0700, Stephen Hemminger wrote:
> David Lamparter <equinox@diac24.net> wrote:
> > unregister_netdevice: waiting for lo to become free. Usage count = 4
> > 
> > unshare -n -- /bin/bash -c 'ip link add link lo veth0 type veth peer
> > name veth3 netns 1; brctl addbr br0; ip l s br0 up; ip a l; sleep 8'
> > 
> > (It happens when the netns is going down.)
> > 
> > Without the "ip l s br0 up" it does not happen;
[...]
> > I'm looking into this, but i'm not versed in Linux kernel code, so my
> > luck in fixing this might be limited. Some reference taken in bridge
> > "up"? Should the bridge be set "down" on netns teardown?
> 
> Loopback device is special and is referenced by routes etc.
> You need to manually remove bridge, and all routes, neighbors and cache
> before removing namespace.

That doesn't do. I have namespaces which are only network namespaces and
they have only one or two processes like openvpn. openvpn can have bugs,
so it might just terminate. If it is the last process in the namespace,
the namespace needs to go, and correctly. Even if I had some "master
daemon" to tear down the namespace at the "end", who tells me that that
will work perfectly and not die? It shouldn't be possible to break the
network stack by just killing the wrong process...

I'll try reading the code today, maybe I can come up with the right
dereferences in the right places.


-David

[BUG] bridge leaks 3 references on lo per up&down

[David Lamparter]

On Wed, Jul 28, 2010 at 09:44:28AM +0200, David Lamparter wrote:
> On Tue, Jul 27, 2010 at 10:02:49AM -0700, Stephen Hemminger wrote:
> > David Lamparter <equinox@diac24.net> wrote:
> > > unregister_netdevice: waiting for lo to become free. Usage count = 4
> > > 
> > > unshare -n -- /bin/bash -c 'ip link add link lo veth0 type veth peer
> > > name veth3 netns 1; brctl addbr br0; ip l s br0 up; ip a l; sleep 8'

When I do

unshare -n -- /bin/bash -c 'ip link add link lo veth0 type veth peer
	name veth3 netns 1; brctl addbr br0; \
	ip l s br0 up; sleep 8; ip l s br0 down; sleep 1;
	ip l s br0 up; sleep 8;'

I get "usage count = 7" on lo; when I add another up & down, I get
"usage count = 10". 4 times makes it a merry 13 refs...


-David

Re: [BUG] bridge leaks 3 references on lo per up&down

[David Lamparter]

On Wed, Jul 28, 2010 at 11:58:25AM +0200, David Lamparter wrote:
> unshare -n -- /bin/bash -c 'ip link add link lo veth0 type veth peer
>       name veth3 netns 1; brctl addbr br0; \
>       ip l s br0 up; sleep 8; ip l s br0 down; sleep 1;
>       ip l s br0 up; sleep 8;'
>
> I get "usage count = 7" on lo; when I add another up & down, I get
> "usage count = 10". 4 times makes it a merry 13 refs...

It only happens with BRIDGE_IGMP_SNOOPING. As we don't need that, our
"fix" is just to run with it disabled... works pretty fine.

Herbert, you implemented bridge multicast support, maybe you can figure
out where these references are taken?


-David

Re: [BUG] bridge leaks 3 references on lo per up&down

[Herbert Xu]

On Wed, Jul 28, 2010 at 02:03:36PM +0200, David Lamparter wrote:
> 
> Herbert, you implemented bridge multicast support, maybe you can figure
> out where these references are taken?

OK this patch should fix it.

However, it also means that the IPv6 membership report parsing is
broken and we still need look into it.

bridge: Fix skb leak when multicast parsing fails on TX

On the bridge TX path we're leaking an skb when br_multicast_rcv
returns an error.

Reported-by: David Lamparter <equinox@diac24.net>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 753fc42..f49f20b 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -47,8 +47,10 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	skb_pull(skb, ETH_HLEN);
 
 	if (is_multicast_ether_addr(dest)) {
-		if (br_multicast_rcv(br, NULL, skb))
+		if (br_multicast_rcv(br, NULL, skb)) {
+			kfree_skb(skb);
 			goto out;
+		}
 
 		mdst = br_mdb_get(br, skb);
 		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb))

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [BUG] bridge leaks 3 references on lo per up&down

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 29 Jul 2010 19:12:31 +0800

> bridge: Fix skb leak when multicast parsing fails on TX
> 
> On the bridge TX path we're leaking an skb when br_multicast_rcv
> returns an error.
> 
> Reported-by: David Lamparter <equinox@diac24.net>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied to net-2.6, thanks!