ipv6 neighbour code keeps interfaces held

ipv6 neighbour code keeps interfaces held

[Johannes Berg]

Hi,

Every once a while when the stars align right, I'll run into this
message:

[22350.676116] unregister_netdevice: waiting for wlan0 to become free. Usage count = 3

when rmmod'ing b43 and mac80211.

In trying to debug it, I have created the patch you can find at the end
of this email. I'm not sure it's generally useful, but I now found it
useful to pinpoint what's going on and to confirm that mac80211 isn't at
fault :)

The patch will WARN_ON() when a netdev reference is released via
dev_put() after the message above has already appeared at least once.
For me, I got this dump out of it:

[22369.076490] Badness at include/linux/netdevice.h:1255
[22369.076493] NIP: c0301aac LR: c0306b6c CTR: c0306b28
[22369.076497] REGS: efff5e40 TRAP: 0700   Tainted: G        W  (2.6.27-rc5-wl-00855-gebb8706-dirty)
[22369.076500] MSR: 00029032 <EE,ME,IR,DR>  CR: 28428442  XER: 20000000
[22369.076509] TASK = ed1da3a0[17998] 'cc1' THREAD: ee68a000
[22369.076512] GPR00: 00000001 efff5ef0 ed1da3a0 efb9a000 bbbbbbbb 00000004 ef8f54bc ef8f54e4 
[22369.076522] GPR08: 0000005a edb174b0 ffffffff edb172a0 28428442 10756ca4 c064ee04 eefb5494 
[22369.076532] GPR16: 00000000 1074f028 105ebb0c 00200200 00000000 c0870ff8 c0870df8 c0870bf8 
[22369.076542] GPR24: efff4000 c08709f8 c064461c efff5f78 00000000 00000000 ef8f5400 efb9a000 
[22369.076559] NIP [c0301aac] in6_dev_finish_destroy+0x50/0xb0
[22369.076565] LR [c0306b6c] ip6_dst_destroy+0x44/0x54
[22369.076567] Call Trace:
[22369.076571] [efff5ef0] [c028f984] neigh_destroy+0x16c/0x1b8 (unreliable)
[22369.076577] [efff5f00] [c0306b6c] ip6_dst_destroy+0x44/0x54
[22369.076582] [efff5f10] [c028c304] dst_destroy+0x78/0x154
[22369.076586] [efff5f30] [c030a270] icmp6_dst_gc+0xa4/0xd4
[22369.076591] [efff5f50] [c030b084] fib6_run_gc+0x50/0x110
[22369.076596] [efff5f70] [c003b040] run_timer_softirq+0x170/0x22c
[22369.076602] [efff5fc0] [c0035e7c] __do_softirq+0x8c/0xfc
[22369.076607] [efff5ff0] [c00120c4] call_do_softirq+0x14/0x24
[22369.076612] [ee68bd00] [c0007000] do_softirq+0x78/0x88
[22369.076616] [ee68bd20] [c0035ae4] irq_exit+0x60/0x80
[22369.076621] [ee68bd30] [c000fde0] timer_interrupt+0x11c/0x16c
[22369.076626] [ee68bd50] [c0012e78] ret_from_except+0x0/0x14
[22369.076632] --- Exception: 901 at __flush_dcache_icache+0x24/0x40
[22369.076634]     LR = update_mmu_cache+0x124/0x128
[22369.076644] [ee68be10] [ed6a0000] 0xed6a0000 (unreliable)
[22369.076649] [ee68be30] [c0085378] handle_mm_fault+0x774/0xb10
[22369.076653] [ee68be90] [c0332ba4] do_page_fault+0x3a8/0x540
[22369.076658] [ee68bf40] [c0012bfc] handle_page_fault+0xc/0x80
[22369.076663] --- Exception: 301 at 0x10383868
[22369.076665]     LR = 0x10383858
[22369.076667] Instruction dump:
[22369.076670] 7c0a4910 0f000000 81230008 3149ffff 7c0a4910 0f000000 392b0210 7c004828 
[22369.076680] 3000ffff 7c00492d 40a2fff4 880b0430 <0f000000> 812300a0 2f890000 419e0040 


Having never looked at the code I'll spare you any speculation, but I
don't see why I should be getting this once a while, no less on an
interface that's not even seeing traffic.

johannes
---
 include/linux/netdevice.h |    7 +++++++
 net/Kconfig               |    6 ++++++
 net/core/dev.c            |    3 +++
 3 files changed, 16 insertions(+)

--- everything.orig/include/linux/netdevice.h	2008-09-10 11:06:31.000000000 +0200
+++ everything/include/linux/netdevice.h	2008-09-10 11:10:55.000000000 +0200
@@ -749,6 +749,10 @@ struct net_device
 	/* for setting kernel sock attribute on TCP connection setup */
 #define GSO_MAX_SIZE		65536
 	unsigned int		gso_max_size;
+
+#ifdef CONFIG_NET_HOLD_DBG
+	bool			held_warning;
+#endif
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
@@ -1247,6 +1251,9 @@ extern void netdev_run_todo(void);
 static inline void dev_put(struct net_device *dev)
 {
 	atomic_dec(&dev->refcnt);
+#ifdef CONFIG_NET_HOLD_DBG
+	WARN_ON(dev->held_warning);
+#endif
 }
 
 /**
--- everything.orig/net/core/dev.c	2008-09-10 11:06:42.000000000 +0200
+++ everything/net/core/dev.c	2008-09-10 11:11:31.000000000 +0200
@@ -4117,6 +4117,9 @@ static void netdev_wait_allrefs(struct n
 			       "count = %d\n",
 			       dev->name, atomic_read(&dev->refcnt));
 			warning_time = jiffies;
+#ifdef CONFIG_NET_HOLD_DBG
+			dev->held_warning = true;
+#endif
 		}
 	}
 }
--- everything.orig/net/Kconfig	2008-09-10 11:11:36.000000000 +0200
+++ everything/net/Kconfig	2008-09-10 11:13:29.000000000 +0200
@@ -24,6 +24,12 @@ if NET
 
 menu "Networking options"
 
+config NET_HOLD_DBG
+	bool "Network device refcount debugging"
+	default n
+	help
+	  Enable this option to have the kernel WARN too late dev_put().
+
 config NET_NS
 	bool "Network namespace support"
 	default n

Re: ipv6 neighbour code keeps interfaces held

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 10 Sep 2008 23:18:59 +0200

> [22369.076567] Call Trace:
> [22369.076571] [efff5ef0] [c028f984] neigh_destroy+0x16c/0x1b8 (unreliable)
> [22369.076577] [efff5f00] [c0306b6c] ip6_dst_destroy+0x44/0x54
> [22369.076582] [efff5f10] [c028c304] dst_destroy+0x78/0x154
> [22369.076586] [efff5f30] [c030a270] icmp6_dst_gc+0xa4/0xd4
> [22369.076591] [efff5f50] [c030b084] fib6_run_gc+0x50/0x110

This is yet another side effect of how ipv6 handles ICMP routes
seperately from "real" ipv6 routes.

And this seperation has been responsible for a large number of
bugs over the years.

Anyways, what happens is that when a device goes down a notifier
goes out, and in IPV6's case it has a notifier registered in
net/ipv6/addrconf.c that ends up calling rt6_ifdown() on
the device.

This is supposed to purge routes from the tree so that the device
references can be dropped (eventually).

This is done in ipv6 by fib6_clean_all() but that only handles
normal routes.  The seperate ICMP6 routes are not purged properly
in this situation.

Here is a quick patch that might resolve this specific situation.
Let me know how it works for you:

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9af6115..776871e 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1003,6 +1003,25 @@ int icmp6_dst_gc(void)
 	return more;
 }
 
+static void icmp6_clean_all(int (*func)(struct rt6_info *rt, void *arg),
+			    void *arg)
+{
+	struct dst_entry *dst, **pprev;
+
+	spin_lock_bh(&icmp6_dst_lock);
+	pprev = &icmp6_dst_gc_list;
+	while ((dst = *pprev) != NULL) {
+		struct rt6_info *rt = (struct rt6_info *) dst;
+		if (func(rt, arg)) {
+			*pprev = dst->next;
+			dst_free(dst);
+		} else {
+			pprev = &dst->next;
+		}
+	}
+	spin_unlock_bh(&icmp6_dst_lock);
+}
+
 static int ip6_dst_gc(struct dst_ops *ops)
 {
 	unsigned long now = jiffies;
@@ -1930,6 +1949,7 @@ void rt6_ifdown(struct net *net, struct net_device *dev)
 	};
 
 	fib6_clean_all(net, fib6_ifdown, 0, &adn);
+	icmp6_clean_all(fib6_ifdown, &adn);
 }
 
 struct rt6_mtu_change_arg

Re: ipv6 neighbour code keeps interfaces held

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Wed, 2008-09-10 at 16:40 -0700, David Miller wrote:

> This is yet another side effect of how ipv6 handles ICMP routes
> seperately from "real" ipv6 routes.

I have no idea :) I just had never seen this on a wired interface and
wanted to see if it was mac80211's fault although I couldn't see how it
could have been.

> Anyways, what happens is that when a device goes down a notifier
> goes out, and in IPV6's case it has a notifier registered in
> net/ipv6/addrconf.c that ends up calling rt6_ifdown() on
> the device.
> 
> This is supposed to purge routes from the tree so that the device
> references can be dropped (eventually).
> 
> This is done in ipv6 by fib6_clean_all() but that only handles
> normal routes.  The seperate ICMP6 routes are not purged properly
> in this situation.

And then they time out after 30 seconds? I see the message exactly three
times and then it gets removed.

> Here is a quick patch that might resolve this specific situation.
> Let me know how it works for you:

I'll add it to my tree, but I cannot reproduce the issue at will. It
just happens occasionally when I'm developing.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ipv6 neighbour code keeps interfaces held

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 11 Sep 2008 01:57:33 +0200

> And then they time out after 30 seconds? I see the message exactly three
> times and then it gets removed.

That's how often garbage collection runs on ICMP6 routes.

Re: ipv6 neighbour code keeps interfaces held

[Rami Rosen]

>That's how often garbage collection runs on ICMP6 routes.

And which, BTW, can be configured through:
 /proc/sys/net/ipv6/route/gc_interval

Regards,
Rami Rosen


On Thu, Sep 11, 2008 at 3:00 AM, David Miller <davem@davemloft.net> wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Thu, 11 Sep 2008 01:57:33 +0200
>
>> And then they time out after 30 seconds? I see the message exactly three
>> times and then it gets removed.
>
> That's how often garbage collection runs on ICMP6 routes.
> --
> 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
>

Re: ipv6 neighbour code keeps interfaces held

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 335 bytes --]

On Wed, 2008-09-10 at 16:40 -0700, David Miller wrote:

> Here is a quick patch that might resolve this specific situation.
> Let me know how it works for you:

I haven't seen it again so far, but it's not necessarily an indication
that it worked. But normally I think I probably would have seen it
already by now.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ipv6 neighbour code keeps interfaces held

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 11 Sep 2008 18:12:38 +0200

> On Wed, 2008-09-10 at 16:40 -0700, David Miller wrote:
> 
> > Here is a quick patch that might resolve this specific situation.
> > Let me know how it works for you:
> 
> I haven't seen it again so far, but it's not necessarily an indication
> that it worked. But normally I think I probably would have seen it
> already by now.

Thanks for the feedback.

I put this fix into net-next-2.6, and what I'll likely do is backport
this to net-2.6 and -stable if you still don't see if after a few more
days.