[PATCH] ipv6: addrconf: don't remove address state on ifdown if the address is being kept

[PATCH] ipv6: addrconf: don't remove address state on ifdown if the address is being kept

[Lorenzo Colitti]

Currently, addrconf_ifdown does not delete statically configured IPv6
addresses when the interface is brought down. The intent is that when
the interface comes back up the address will be usable again. However,
this doesn't actually work, because the system stops listening on the
corresponding solicited-node multicast address, so the address cannot
respond to neighbor solicitations and thus receive traffic. Also, the
code notifies the rest of the system that the address is being deleted
(e.g, RTM_DELADDR), even though it is not. Fix it so that none of this
state is updated if the address is being kept on the interface.

--- a/net/ipv6/addrconf.c	2010-10-20 13:30:22.000000000 -0700
+++ b/net/ipv6/addrconf.c	2010-10-27 21:04:39.000000000 -0700
@@ -2737,10 +2737,7 @@ static int addrconf_ifdown(struct net_de
 			/* Flag it for later restoration when link comes up */
 			ifa->flags |= IFA_F_TENTATIVE;
 			ifa->state = INET6_IFADDR_STATE_DAD;
-
-			write_unlock_bh(&idev->lock);
-
-			in6_ifa_hold(ifa);
+			continue;
 		} else {
 			list_del(&ifa->if_list);

Re: [PATCH] ipv6: addrconf: don't remove address state on ifdown if the address is being kept

[Lorenzo Colitti]

On Wed, Oct 27, 2010 at 9:16 PM, Lorenzo Colitti <lorenzo@google.com> wrote:
> Fix it so that none of this state is updated if the address is being kept on the interface.

Or, since the logic of the patched code is a little hard to follow,
here's an alternative patch against 2.6.36. I think it does exactly
the same as the previous patch, but it moves things around to make the
result more readable.

Tested: Added a statically configured IPv6 address to an interface,
started ping, brought link down, brought link up again. When link came
up ping kept on going and "ip -6 maddr" showed that the host was still
subscribed to there

Signed-off-by: Lorenzo Colitti <lorenzo@google.com>

--- 2.6.36-orig/net/ipv6/addrconf.c	2010-10-20 13:30:22.000000000 -0700
+++ linux-2.6.36/net/ipv6/addrconf.c	2010-10-27 23:26:57.000000000 -0700
@@ -2737,10 +2737,6 @@ static int addrconf_ifdown(struct net_de
 			/* Flag it for later restoration when link comes up */
 			ifa->flags |= IFA_F_TENTATIVE;
 			ifa->state = INET6_IFADDR_STATE_DAD;
-
-			write_unlock_bh(&idev->lock);
-
-			in6_ifa_hold(ifa);
 		} else {
 			list_del(&ifa->if_list);

@@ -2755,19 +2751,15 @@ static int addrconf_ifdown(struct net_de
 			ifa->state = INET6_IFADDR_STATE_DEAD;
 			spin_unlock_bh(&ifa->state_lock);

-			if (state == INET6_IFADDR_STATE_DEAD)
-				goto put_ifa;
+			if (state == INET6_IFADDR_STATE_DEAD) {
+				in6_ifa_put(ifa);
+			} else {
+				__ipv6_ifa_notify(RTM_DELADDR, ifa);
+				atomic_notifier_call_chain(&inet6addr_chain,
+							   NETDEV_DOWN, ifa);
+			}
+			write_lock_bh(&idev->lock);
 		}
-
-		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		if (ifa->state == INET6_IFADDR_STATE_DEAD)
-			atomic_notifier_call_chain(&inet6addr_chain,
-						   NETDEV_DOWN, ifa);
-
-put_ifa:
-		in6_ifa_put(ifa);
-
-		write_lock_bh(&idev->lock);
 	}

 	list_splice(&keep_list, &idev->addr_list);

Re: [PATCH] ipv6: addrconf: don't remove address state on ifdown if the address is being kept

[Lorenzo Colitti]

On Thu, Oct 28, 2010 at 2:45 PM, Lorenzo Colitti <lorenzo@google.com> wrote:
>> Fix it so that none of this state is updated if the address is being kept on the interface.

David, any feedback on this? The existing code clearly has the
intention of keeping the addresses around, except they don't actually
work, and this just fixes the bug.

Thanks,
Lorenzo

Re: [PATCH] ipv6: addrconf: don't remove address state on ifdown if the address is being kept

[David Miller]

From: Lorenzo Colitti <lorenzo@google.com>
Date: Fri, 5 Nov 2010 14:45:28 +0800

> On Thu, Oct 28, 2010 at 2:45 PM, Lorenzo Colitti <lorenzo@google.com> wrote:
>>> Fix it so that none of this state is updated if the address is being kept on the interface.
> 
> David, any feedback on this? The existing code clearly has the
> intention of keeping the addresses around, except they don't actually
> work, and this just fixes the bug.

I'm travelling and busy at Kernel Summit and Linux Plumbers Conference,
so I'm only processing the most easiest patches this week.

I'll look into this when I get back.

Re: [PATCH] ipv6: addrconf: don't remove address state on ifdown if the address is being kept

[David Miller]

From: Lorenzo Colitti <lorenzo@google.com>
Date: Wed, 27 Oct 2010 23:45:17 -0700

> On Wed, Oct 27, 2010 at 9:16 PM, Lorenzo Colitti <lorenzo@google.com> wrote:
>> Fix it so that none of this state is updated if the address is being kept on the interface.
> 
> Or, since the logic of the patched code is a little hard to follow,
> here's an alternative patch against 2.6.36. I think it does exactly
> the same as the previous patch, but it moves things around to make the
> result more readable.
> 
> Tested: Added a statically configured IPv6 address to an interface,
> started ping, brought link down, brought link up again. When link came
> up ping kept on going and "ip -6 maddr" showed that the host was still
> subscribed to there
> 
> Signed-off-by: Lorenzo Colitti <lorenzo@google.com>

Ok, this looks good, applied.

Thanks for your patience.