IPv6 "badness" in recent releases

IPv6 "badness" in recent releases

[Jeff Garzik]

So, while ssh'ing _successfully_ to a DNS server I maintain, I noticed 
that my gateway would grind its disk whenever traffic flowed through the 
ssh connection.

I login (same machine that used to have the "BUG: dst underflow" 
problems) and see

[first one I can find in log]
Badness in dst_release at include/net/dst.h:149
  [<f8c0f169>] ip6_dst_check+0x64/0x6a [ipv6]
  [<f8c060ab>] ip6_dst_lookup+0x17b/0x195 [ipv6]
  [<f8c18366>] udpv6_sendmsg+0x2bb/0x934 [ipv6]
  [<c031a41f>] udp_recvmsg+0x60/0x2e9
  [<c032161d>] inet_sendmsg+0x4d/0x59
  [<c02d4588>] sock_sendmsg+0xe8/0x103
  [<c011eafd>] find_busiest_group+0xcf/0x2db
  [<c01eec8c>] copy_from_user+0x42/0x6e
  [<c0136c86>] autoremove_wake_function+0x0/0x57
  [<c02d5ea6>] sys_sendmsg+0x189/0x1e6
  [<c011f7ec>] __wake_up_common+0x3f/0x5e
  [<c011f84b>] __wake_up+0x40/0x56
  [<c01372f5>] wake_futex+0x37/0x62
  [<c0137394>] futex_wake+0x74/0xc4
  [<c01eec8c>] copy_from_user+0x42/0x6e
  [<c02d6330>] sys_socketcall+0x236/0x254
  [<c0105f11>] sysenter_past_esp+0x52/0x71

[current ones]
Badness in dst_release at include/net/dst.h:149
  [<f8c0dde5>] ipip6_tunnel_xmit+0x5e9/0x7c8 [ipv6]
  [<c011dd83>] try_to_wake_up+0x1f3/0x2c9
  [<c02ddaf7>] dev_queue_xmit+0xd1/0x272
  [<f8c04a24>] ip6_output2+0x18c/0x29e [ipv6]
  [<f8c05416>] ip6_forward+0x148/0x3e6 [ipv6]
  [<f8c07222>] ipv6_rcv+0x142/0x284 [ipv6]
  [<c02de147>] netif_receive_skb+0x12f/0x18f
  [<f8be9387>] e1000_clean_rx_irq+0x12e/0x447 [e1000]
  [<c02d82af>] __kfree_skb+0x83/0x107
  [<f8be8fc8>] e1000_clean+0x51/0xca [e1000]
  [<c02de329>] net_rx_action+0x77/0xf6
  [<c01281f3>] __do_softirq+0xb7/0xc6
  [<c012822f>] do_softirq+0x2d/0x2f
  [<c0108362>] do_IRQ+0x1e/0x24
  [<c010694a>] common_interrupt+0x1a/0x20
  [<c010401e>] default_idle+0x0/0x2c
  [<c0104047>] default_idle+0x29/0x2c
  [<c01040b0>] cpu_idle+0x33/0x3c

filling my logs.

This is i686 HyperThreaded (fake SMP) box running 2.6.10-rc1-bk16.

As I mentioned at the beginning of the message, IPv6 still seems to 
function.  It just fills my kernel ring buffer with these BUG messages.

	Jeff

Re: IPv6 "badness" in recent releases

[Jeff Garzik]

Two additional data points...

* I'm running -bk24 now, and trying to reproduce

* the problem in the previous email was generated by an ssh connection 
from host A, through router B, to host C.  router B is the Linux kernel 
that was emitting the "Badness in dst_release" messages.  No obvious 
error messages on host A (2.6.10-rc1-bk22) or host C (UML).

	Jeff

Re: IPv6 "badness" in recent releases

[Herbert Xu]

Jeff Garzik <jgarzik@pobox.com> wrote:
> 
> I login (same machine that used to have the "BUG: dst underflow" 
> problems) and see
> 
> [first one I can find in log]
> Badness in dst_release at include/net/dst.h:149

Thanks.  This is what "dst underflow" looks like in the new kernels.
Unfortunately whatever caused this has already happened so we can
only guess about where the bug is.

Could you please apply this patch and see if the warning triggers
before the underflows start coming in? We already know that if it
does then it can cause underflows but I want to make sure that
this is what's causing your problems.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
===== net/ipv6/addrconf.c 1.115 vs edited =====
--- 1.115/net/ipv6/addrconf.c	2004-10-26 14:11:35 +10:00
+++ edited/net/ipv6/addrconf.c	2004-11-09 22:44:35 +11:00
@@ -2989,6 +2989,7 @@
 		dst_hold(&ifp->rt->u.dst);
 		if (ip6_ins_rt(ifp->rt, NULL, NULL))
 			dst_release(&ifp->rt->u.dst);
+		WARN_ON(ifp->dead);
 		if (ifp->idev->cnf.forwarding)
 			addrconf_join_anycast(ifp);
 		break;

Fix ipv6_ifa_notify race (was: IPv6 "badness" in recent releases)

[Herbert Xu]

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

On Mon, Nov 15, 2004 at 07:35:17AM +1100, Herbert Xu wrote:
> 
> Could you please apply this patch and see if the warning triggers
> before the underflows start coming in? We already know that if it
> does then it can cause underflows but I want to make sure that
> this is what's causing your problems.

Well I think it's time to fix this bug so that we don't release
2.6.10 with it.

This is only a temporary hack that should be rolled back once
the locking in addrconf.c is fixed properly.  At the moment
the locking is just too twisted to fix properly for 2.6.10 :(

This patch doesn't even fix all the races in ipv6_ifa_notify.
For example, the anycast/multicast addresses may be left
standing should the race strike.  However, they should have
a much milder effect compared to a dead route in the routing
table :)

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

May I ask this question again: Is it possible to move some of
this complexity to user-space?

addrconf.c is by far the biggest file in net/ipv6.  This is
also not the first time we've had a serious bug in this file.
I recall fixing another timer-related bug in this file a year
ago that left dangling references on devices.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 890 bytes --]

===== net/ipv6/addrconf.c 1.120 vs edited =====
--- 1.120/net/ipv6/addrconf.c	2004-11-16 09:08:09 +11:00
+++ edited/net/ipv6/addrconf.c	2004-11-19 20:43:45 +11:00
@@ -2987,8 +2987,13 @@
 	switch (event) {
 	case RTM_NEWADDR:
 		dst_hold(&ifp->rt->u.dst);
-		if (ip6_ins_rt(ifp->rt, NULL, NULL))
+		spin_lock_bh(&ifp->lock);
+
+		if (ifp->dead || ip6_ins_rt(ifp->rt, NULL, NULL))
 			dst_release(&ifp->rt->u.dst);
+
+		spin_unlock_bh(&ifp->lock);
+
 		if (ifp->idev->cnf.forwarding)
 			addrconf_join_anycast(ifp);
 		break;
@@ -2996,11 +3001,16 @@
 		if (ifp->idev->cnf.forwarding)
 			addrconf_leave_anycast(ifp);
 		addrconf_leave_solict(ifp->idev, &ifp->addr);
+
 		dst_hold(&ifp->rt->u.dst);
+		spin_lock_bh(&ifp->lock);
+
 		if (ip6_del_rt(ifp->rt, NULL, NULL))
 			dst_free(&ifp->rt->u.dst);
 		else
 			dst_release(&ifp->rt->u.dst);
+
+		spin_unlock_bh(&ifp->lock);
 		break;
 	}
 }

Re: Fix ipv6_ifa_notify race

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20041119094924.GA3726@gondor.apana.org.au> (at Fri, 19 Nov 2004 20:49:24 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:

> May I ask this question again: Is it possible to move some of
> this complexity to user-space?

I strongly believe it should be implemented in kernel.

--yoshfuji

Re: Fix ipv6_ifa_notify race

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20041119094924.GA3726@gondor.apana.org.au> (at Fri, 19 Nov 2004 20:49:24 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:

> On Mon, Nov 15, 2004 at 07:35:17AM +1100, Herbert Xu wrote:
> > 
> > Could you please apply this patch and see if the warning triggers
> > before the underflows start coming in? We already know that if it
> > does then it can cause underflows but I want to make sure that
> > this is what's causing your problems.
> 
> Well I think it's time to fix this bug so that we don't release
> 2.6.10 with it.
> 
> This is only a temporary hack that should be rolled back once
> the locking in addrconf.c is fixed properly.  At the moment
> the locking is just too twisted to fix properly for 2.6.10 :(

This patch is wrong (dead is not protected appropriately).
I have another idea to fix this.
I'll send it tomorrow.

--yoshfuji

Re: Fix ipv6_ifa_notify race

[Herbert Xu]

On Fri, Nov 19, 2004 at 11:36:58AM -0500, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> 
> This patch is wrong (dead is not protected appropriately).

I don't understand your point.  ifp->dead is an int and it's read
and written atomically.

> I have another idea to fix this.

Great.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt