[PATCH 0/4] IPv6 addrconf related fixes

[PATCH 0/4] IPv6 addrconf related fixes

[Stephen Hemminger]

These apply to net-next, the problems do not exist in earlier kernels.
The problems started when I added changes to retain IPv6 addresses
when link goes down.

-- 

[PATCH 1/4] IPv6: keep route for tentative address

[Stephen Hemminger]

[-- Attachment #1: ipv6-addrconf1.patch --]
[-- Type: text/plain, Size: 689 bytes --]

Recent changes preserve IPv6 address when link goes down (good).
But would cause address to point to dead dst entry (bad).
The simplest fix is to just not delete route if address is
being held for later use.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/net/ipv6/addrconf.c	2010-04-11 12:19:37.938082190 -0700
+++ b/net/ipv6/addrconf.c	2010-04-11 12:25:05.349309074 -0700
@@ -4046,7 +4046,8 @@ static void __ipv6_ifa_notify(int event,
 			addrconf_leave_anycast(ifp);
 		addrconf_leave_solict(ifp->idev, &ifp->addr);
 		dst_hold(&ifp->rt->u.dst);
-		if (ip6_del_rt(ifp->rt))
+
+		if (ifp->dead && ip6_del_rt(ifp->rt))
 			dst_free(&ifp->rt->u.dst);
 		break;
 	}

-- 

RE: [PATCH 1/4] IPv6: keep route for tentative address

[Tantilov, Emil S]

Stephen Hemminger wrote:
> Recent changes preserve IPv6 address when link goes down (good).
> But would cause address to point to dead dst entry (bad).
> The simplest fix is to just not delete route if address is
> being held for later use.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> 
> --- a/net/ipv6/addrconf.c	2010-04-11 12:19:37.938082190 -0700
> +++ b/net/ipv6/addrconf.c	2010-04-11 12:25:05.349309074 -0700
> @@ -4046,7 +4046,8 @@ static void __ipv6_ifa_notify(int event,
>  			addrconf_leave_anycast(ifp);
>  		addrconf_leave_solict(ifp->idev, &ifp->addr);
>  		dst_hold(&ifp->rt->u.dst);
> -		if (ip6_del_rt(ifp->rt))
> +
> +		if (ifp->dead && ip6_del_rt(ifp->rt))
>  			dst_free(&ifp->rt->u.dst);
>  		break;
>  	}

Stephen,

With these series of patches (1-4) applied I am no longer seeing the
hangs and warnings associated with ipv6. I ran few rounds of
tests with resetting the interface and loading/unloading modules.

Thanks a lot for looking into this!
Emil

Re: [PATCH 1/4] IPv6: keep route for tentative address

[Emil S Tantilov]

On Mon, Apr 12, 2010 at 1:17 PM, Tantilov, Emil S
<emil.s.tantilov@intel.com> wrote:
> Stephen Hemminger wrote:
>> Recent changes preserve IPv6 address when link goes down (good).
>> But would cause address to point to dead dst entry (bad).
>> The simplest fix is to just not delete route if address is
>> being held for later use.
>>
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>>
>>
>> --- a/net/ipv6/addrconf.c     2010-04-11 12:19:37.938082190 -0700
>> +++ b/net/ipv6/addrconf.c     2010-04-11 12:25:05.349309074 -0700
>> @@ -4046,7 +4046,8 @@ static void __ipv6_ifa_notify(int event,
>>                       addrconf_leave_anycast(ifp);
>>               addrconf_leave_solict(ifp->idev, &ifp->addr);
>>               dst_hold(&ifp->rt->u.dst);
>> -             if (ip6_del_rt(ifp->rt))
>> +
>> +             if (ifp->dead && ip6_del_rt(ifp->rt))
>>                       dst_free(&ifp->rt->u.dst);
>>               break;
>>       }
>
> Stephen,
>
> With these series of patches (1-4) applied I am no longer seeing the
> hangs and warnings associated with ipv6. I ran few rounds of
> tests with resetting the interface and loading/unloading modules.

Looks like these patches did not make it into net-2.6 and the issue can be seen
in the latest stable 2.6.34.

Any chance that the patches can be pushed to 2.6.34-stable?

Thanks,
Emil

[PATCH 1/2] IPv6: keep route for tentative address

[Stephen Hemminger]

Recent changes preserve IPv6 address when link goes down (good).
But would cause address to point to dead dst entry (bad).
The simplest fix is to just not delete route if address is
being held for later use.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b)
---
Patch is for 2.6.34-stable (problem doesn't exist in earlier kernel)

 net/ipv6/addrconf.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 413054f..12e9558 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4047,7 +4047,8 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
 			addrconf_leave_anycast(ifp);
 		addrconf_leave_solict(ifp->idev, &ifp->addr);
 		dst_hold(&ifp->rt->u.dst);
-		if (ip6_del_rt(ifp->rt))
+
+		if (ifp->dead && ip6_del_rt(ifp->rt))
 			dst_free(&ifp->rt->u.dst);
 		break;
 	}
-- 
1.7.0.4

[PATCH 2/2] IPv6: only notify protocols if address is completely gone

[Stephen Hemminger]

The notifier for address down should only be called if address is completely
gone, not just being marked as tentative on link transition. The code
in net-next would case bonding/sctp/s390 to see address disappear on link
down, but they would never see it reappear on link up.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 8595805aafc8b077e01804c9a3668e9aa3510e89)
---
Patch for 2.6.34-stable, not needed on earlier or later kernels

 net/ipv6/addrconf.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 12e9558..844ffc5 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2729,7 +2729,9 @@ static int addrconf_ifdown(struct net_device *dev, int how)
 		write_unlock_bh(&idev->lock);
 
 		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+		if (ifa->dead)
+			atomic_notifier_call_chain(&inet6addr_chain,
+						   NETDEV_DOWN, ifa);
 		in6_ifa_put(ifa);
 
 		write_lock_bh(&idev->lock);
-- 
1.7.0.4

Patch "IPv6: only notify protocols if address is completely gone" has been added to the 2.6.34-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    IPv6: only notify protocols if address is completely gone

to the 2.6.34-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
    ipv6-only-notify-protocols-if-address-is-completely-gone.patch
and it can be found in the queue-2.6.34 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@kernel.org> know about it.


>From shemminger@vyatta.com  Tue Jul 27 16:57:29 2010
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 24 May 2010 11:33:00 -0700
Subject: IPv6: only notify protocols if address is completely gone
To: Emil S Tantilov <emils.tantilov@gmail.com>, "David S. Miller" <davem@davemloft.net>, Greg KH <greg@kroah.com>
Cc: NetDev <netdev@vger.kernel.org>, "Tantilov, Emil S" <emil.s.tantilov@intel.com>, stable@kernel.org
Message-ID: <20100524113300.2ef38e12@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>

(cherry picked from commit 8595805aafc8b077e01804c9a3668e9aa3510e89)

The notifier for address down should only be called if address is completely
gone, not just being marked as tentative on link transition. The code
in net-next would case bonding/sctp/s390 to see address disappear on link
down, but they would never see it reappear on link up.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 net/ipv6/addrconf.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2729,7 +2729,9 @@ static int addrconf_ifdown(struct net_de
 		write_unlock_bh(&idev->lock);
 
 		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+		if (ifa->dead)
+			atomic_notifier_call_chain(&inet6addr_chain,
+						   NETDEV_DOWN, ifa);
 		in6_ifa_put(ifa);
 
 		write_lock_bh(&idev->lock);


Patches currently in stable-queue which might be from shemminger@vyatta.com are

queue-2.6.34/sky2-restore-multicast-after-restart.patch
queue-2.6.34/ipv6-keep-route-for-tentative-address.patch
queue-2.6.34/ipv6-only-notify-protocols-if-address-is-completely-gone.patch
queue-2.6.34/ipv6-fix-null-reference-in-proxy-neighbor-discovery.patch
queue-2.6.34/bridge-fdb-cleanup-runs-too-often.patch

Re: [PATCH 1/2] IPv6: keep route for tentative address

[Greg KH]

On Mon, May 24, 2010 at 11:31:18AM -0700, Stephen Hemminger wrote:
> Recent changes preserve IPv6 address when link goes down (good).
> But would cause address to point to dead dst entry (bad).
> The simplest fix is to just not delete route if address is
> being held for later use.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b)
> ---
> Patch is for 2.6.34-stable (problem doesn't exist in earlier kernel)

Normally I wait for David to send networking patches for the stable
trees, so I would like to get David's ack on these two before I am
willing to apply them.

thanks,

greg k-h

Re: [PATCH 1/2] IPv6: keep route for tentative address

[David Miller]

From: Greg KH <greg@kroah.com>
Date: Mon, 24 May 2010 11:47:13 -0700

> On Mon, May 24, 2010 at 11:31:18AM -0700, Stephen Hemminger wrote:
>> Recent changes preserve IPv6 address when link goes down (good).
>> But would cause address to point to dead dst entry (bad).
>> The simplest fix is to just not delete route if address is
>> being held for later use.
>> 
>> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> (cherry picked from commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b)
>> ---
>> Patch is for 2.6.34-stable (problem doesn't exist in earlier kernel)
> 
> Normally I wait for David to send networking patches for the stable
> trees, so I would like to get David's ack on these two before I am
> willing to apply them.

These two are fine to go to -stable, Greg please integrate.

Patch "IPv6: keep route for tentative address" has been added to the 2.6.34-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    IPv6: keep route for tentative address

to the 2.6.34-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
    ipv6-keep-route-for-tentative-address.patch
and it can be found in the queue-2.6.34 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@kernel.org> know about it.


>From shemminger@vyatta.com  Tue Jul 27 16:56:59 2010
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 24 May 2010 11:31:18 -0700
Subject: IPv6: keep route for tentative address
To: Emil S Tantilov <emils.tantilov@gmail.com>
Cc: NetDev <netdev@vger.kernel.org>, stable@kernel.org, Greg KH <greg@kroah.com>, "David S. Miller" <davem@davemloft.net>, "Tantilov, Emil S" <emil.s.tantilov@intel.com>
Message-ID: <20100524113118.47cc9852@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>

(cherry picked from commit 93fa159abe50d3c55c7f83622d3f5c09b6e06f4b)

Recent changes preserve IPv6 address when link goes down (good).
But would cause address to point to dead dst entry (bad).
The simplest fix is to just not delete route if address is
being held for later use.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 net/ipv6/addrconf.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4047,7 +4047,8 @@ static void __ipv6_ifa_notify(int event,
 			addrconf_leave_anycast(ifp);
 		addrconf_leave_solict(ifp->idev, &ifp->addr);
 		dst_hold(&ifp->rt->u.dst);
-		if (ip6_del_rt(ifp->rt))
+
+		if (ifp->dead && ip6_del_rt(ifp->rt))
 			dst_free(&ifp->rt->u.dst);
 		break;
 	}


Patches currently in stable-queue which might be from shemminger@vyatta.com are

queue-2.6.34/sky2-restore-multicast-after-restart.patch
queue-2.6.34/ipv6-keep-route-for-tentative-address.patch
queue-2.6.34/ipv6-only-notify-protocols-if-address-is-completely-gone.patch
queue-2.6.34/ipv6-fix-null-reference-in-proxy-neighbor-discovery.patch
queue-2.6.34/bridge-fdb-cleanup-runs-too-often.patch

[PATCH 2/4] IPv6: keep tentative addresses in hash table

[Stephen Hemminger]

[-- Attachment #1: ipv6-addrconf2.patch --]
[-- Type: text/plain, Size: 1105 bytes --]

When link goes down, want address to be preserved but in a tentative
state, therefore it has to stay in hash list.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/ipv6/addrconf.c	2010-04-11 12:25:05.349309074 -0700
+++ b/net/ipv6/addrconf.c	2010-04-11 12:25:10.408996382 -0700
@@ -2703,17 +2703,18 @@ static int addrconf_ifdown(struct net_de
 			/* Flag it for later restoration when link comes up */
 			ifa->flags |= IFA_F_TENTATIVE;
 			in6_ifa_hold(ifa);
+			write_unlock_bh(&idev->lock);
 		} else {
 			list_del(&ifa->if_list);
 			ifa->dead = 1;
-		}
-		write_unlock_bh(&idev->lock);
+			write_unlock_bh(&idev->lock);
 
-		/* clear hash table */
-		spin_lock_bh(&addrconf_hash_lock);
-		hlist_del_init_rcu(&ifa->addr_lst);
-		__in6_ifa_put(ifa);
-		spin_unlock_bh(&addrconf_hash_lock);
+			/* clear hash table */
+			spin_lock_bh(&addrconf_hash_lock);
+			hlist_del_init_rcu(&ifa->addr_lst);
+			__in6_ifa_put(ifa);
+			spin_unlock_bh(&addrconf_hash_lock);
+		}
 
 		__ipv6_ifa_notify(RTM_DELADDR, ifa);
 		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);

-- 

[PATCH 3/4] ipv6: additional ref count for hash list unnecessary

[Stephen Hemminger]

[-- Attachment #1: ipv6-addrconf3.patch --]
[-- Type: text/plain, Size: 1002 bytes --]

Since an address in hash list has to already have a ref count,
no additional ref count is needed. 

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/net/ipv6/addrconf.c	2010-04-11 12:25:32.609002374 -0700
+++ b/net/ipv6/addrconf.c	2010-04-11 12:26:52.715246164 -0700
@@ -675,7 +675,6 @@ ipv6_add_addr(struct inet6_dev *idev, co
 	hash = ipv6_addr_hash(addr);
 
 	hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
-	in6_ifa_hold(ifa);
 	spin_unlock(&addrconf_hash_lock);
 
 	write_lock(&idev->lock);
@@ -723,7 +722,6 @@ static void ipv6_del_addr(struct inet6_i
 
 	spin_lock_bh(&addrconf_hash_lock);
 	hlist_del_init_rcu(&ifp->addr_lst);
-	__in6_ifa_put(ifp);
 	spin_unlock_bh(&addrconf_hash_lock);
 
 	write_lock_bh(&idev->lock);
@@ -2712,7 +2710,6 @@ static int addrconf_ifdown(struct net_de
 			/* clear hash table */
 			spin_lock_bh(&addrconf_hash_lock);
 			hlist_del_init_rcu(&ifa->addr_lst);
-			__in6_ifa_put(ifa);
 			spin_unlock_bh(&addrconf_hash_lock);
 		}
 

-- 

[PATCH 4/4] IPv6: only notify protocols if address is compeletely gone

[Stephen Hemminger]

[-- Attachment #1: ipv6-addrconf4.patch --]
[-- Type: text/plain, Size: 793 bytes --]

The notifier for address down should only be called if address is completely
gone, not just being marked as tentative on link transistion. The code
in net-next would case bonding/sctp/s390 to see address disappear on link
down, but they would never see it reappear on link up.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/ipv6/addrconf.c	2010-04-11 14:34:36.919767724 -0700
+++ b/net/ipv6/addrconf.c	2010-04-11 14:35:00.533967946 -0700
@@ -2714,7 +2714,9 @@ static int addrconf_ifdown(struct net_de
 		}
 
 		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+		if (ifa->dead)
+			atomic_notifier_call_chain(&inet6addr_chain,
+						   NETDEV_DOWN, ifa);
 		in6_ifa_put(ifa);
 
 		write_lock_bh(&idev->lock);

-- 

Re: [PATCH 0/4] IPv6 addrconf related fixes

[David Miller]

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 12 Apr 2010 08:41:30 -0700

> These apply to net-next, the problems do not exist in earlier kernels.
> The problems started when I added changes to retain IPv6 addresses
> when link goes down.

All aplied, thanks for fixing these bugs Stephen.