diff mbox

[1/2] IPv6: keep permanent addresses on admin down

Message ID 20100208214805.1f8f2c6d@nehalam
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

stephen hemminger Feb. 9, 2010, 5:48 a.m. UTC
Permanent IPV6 addresses should not be removed when the link is
set to admin down, only when device is removed. 

When link is lost permanent addresses should be marked as tentative
so that when link comes back they are subject to duplicate address
detection (if DAD was enabled for that address).

Other routing systems keep manually configured IPv6 addresses
when link is set down.

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

---
The logic here is getting long and twisted, doing some work
to use RCU here and refactor.

--
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

Comments

Brian Haley Feb. 9, 2010, 2:40 p.m. UTC | #1
Stephen Hemminger wrote:
> @@ -2686,18 +2687,30 @@ static int addrconf_ifdown(struct net_de
>  		write_lock_bh(&idev->lock);
>  	}
>  #endif
> -	while ((ifa = idev->addr_list) != NULL) {
> -		idev->addr_list = ifa->if_next;
> -		ifa->if_next = NULL;
> -		ifa->dead = 1;
> -		addrconf_del_timer(ifa);
> -		write_unlock_bh(&idev->lock);
> -
> -		__ipv6_ifa_notify(RTM_DELADDR, ifa);
> -		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
> -		in6_ifa_put(ifa);
> +	bifa = &idev->addr_list;
> +	while ((ifa = *bifa) != NULL) {
> +		if (how == 0 && (ifa->flags&IFA_F_PERMANENT)) {
> +			/* Retain permanent address on admin down */
> +			bifa = &ifa->if_next;
> +
> +			/* Restart DAD if needed when link comes back up */
> +			if ( !((dev->flags&(IFF_NOARP|IFF_LOOPBACK)) ||
> +			       idev->cnf.accept_dad <= 0 ||
> +			       (ifa->flags & IFA_F_NODAD)))
> +				ifa->flags |= IFA_F_TENTATIVE;
> +		} else {
> +			*bifa = ifa->if_next;
> +			ifa->if_next = NULL;
> +
> +			ifa->dead = 1;
> +			write_unlock_bh(&idev->lock);
> +
> +			__ipv6_ifa_notify(RTM_DELADDR, ifa);
> +			atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
> +			in6_ifa_put(ifa);
>  
> -		write_lock_bh(&idev->lock);
> +			write_lock_bh(&idev->lock);
> +		}

What about addrconf_del_timer()?  It didn't seem to make it to this else{} from
the original.

-Brian
--
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
stephen hemminger Feb. 9, 2010, 4:36 p.m. UTC | #2
On Tue, 09 Feb 2010 09:40:11 -0500
Brian Haley <brian.haley@hp.com> wrote:

> Stephen Hemminger wrote:
> > @@ -2686,18 +2687,30 @@ static int addrconf_ifdown(struct net_de
> >  		write_lock_bh(&idev->lock);
> >  	}
> >  #endif
> > -	while ((ifa = idev->addr_list) != NULL) {
> > -		idev->addr_list = ifa->if_next;
> > -		ifa->if_next = NULL;
> > -		ifa->dead = 1;
> > -		addrconf_del_timer(ifa);
> > -		write_unlock_bh(&idev->lock);
> > -
> > -		__ipv6_ifa_notify(RTM_DELADDR, ifa);
> > -		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
> > -		in6_ifa_put(ifa);
> > +	bifa = &idev->addr_list;
> > +	while ((ifa = *bifa) != NULL) {
> > +		if (how == 0 && (ifa->flags&IFA_F_PERMANENT)) {
> > +			/* Retain permanent address on admin down */
> > +			bifa = &ifa->if_next;
> > +
> > +			/* Restart DAD if needed when link comes back up */
> > +			if ( !((dev->flags&(IFF_NOARP|IFF_LOOPBACK)) ||
> > +			       idev->cnf.accept_dad <= 0 ||
> > +			       (ifa->flags & IFA_F_NODAD)))
> > +				ifa->flags |= IFA_F_TENTATIVE;
> > +		} else {
> > +			*bifa = ifa->if_next;
> > +			ifa->if_next = NULL;
> > +
> > +			ifa->dead = 1;
> > +			write_unlock_bh(&idev->lock);
> > +
> > +			__ipv6_ifa_notify(RTM_DELADDR, ifa);
> > +			atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
> > +			in6_ifa_put(ifa);
> >  
> > -		write_lock_bh(&idev->lock);
> > +			write_lock_bh(&idev->lock);
> > +		}
> 
> What about addrconf_del_timer()?  It didn't seem to make it to this else{} from
> the original.
> 
> -Brian

addrconf_del_timer is already being done earlier in same code
no need to do it twice.
David Miller Feb. 12, 2010, 8:30 p.m. UTC | #3
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 8 Feb 2010 21:48:05 -0800

> Permanent IPV6 addresses should not be removed when the link is
> set to admin down, only when device is removed. 
> 
> When link is lost permanent addresses should be marked as tentative
> so that when link comes back they are subject to duplicate address
> detection (if DAD was enabled for that address).
> 
> Other routing systems keep manually configured IPv6 addresses
> when link is set down.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied to net-next-2.6
--
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
diff mbox

Patch

--- a/net/ipv6/addrconf.c	2010-02-08 21:05:04.798785751 -0800
+++ b/net/ipv6/addrconf.c	2010-02-08 21:38:58.734340743 -0800
@@ -2646,7 +2646,8 @@  static int addrconf_ifdown(struct net_de
 
 		write_lock_bh(&addrconf_hash_lock);
 		while ((ifa = *bifa) != NULL) {
-			if (ifa->idev == idev) {
+			if (ifa->idev == idev &&
+			    (how || !(ifa->flags&IFA_F_PERMANENT))) {
 				*bifa = ifa->lst_next;
 				ifa->lst_next = NULL;
 				addrconf_del_timer(ifa);
@@ -2686,18 +2687,30 @@  static int addrconf_ifdown(struct net_de
 		write_lock_bh(&idev->lock);
 	}
 #endif
-	while ((ifa = idev->addr_list) != NULL) {
-		idev->addr_list = ifa->if_next;
-		ifa->if_next = NULL;
-		ifa->dead = 1;
-		addrconf_del_timer(ifa);
-		write_unlock_bh(&idev->lock);
-
-		__ipv6_ifa_notify(RTM_DELADDR, ifa);
-		atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
-		in6_ifa_put(ifa);
+	bifa = &idev->addr_list;
+	while ((ifa = *bifa) != NULL) {
+		if (how == 0 && (ifa->flags&IFA_F_PERMANENT)) {
+			/* Retain permanent address on admin down */
+			bifa = &ifa->if_next;
+
+			/* Restart DAD if needed when link comes back up */
+			if ( !((dev->flags&(IFF_NOARP|IFF_LOOPBACK)) ||
+			       idev->cnf.accept_dad <= 0 ||
+			       (ifa->flags & IFA_F_NODAD)))
+				ifa->flags |= IFA_F_TENTATIVE;
+		} else {
+			*bifa = ifa->if_next;
+			ifa->if_next = NULL;
+
+			ifa->dead = 1;
+			write_unlock_bh(&idev->lock);
+
+			__ipv6_ifa_notify(RTM_DELADDR, ifa);
+			atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+			in6_ifa_put(ifa);
 
-		write_lock_bh(&idev->lock);
+			write_lock_bh(&idev->lock);
+		}
 	}
 	write_unlock_bh(&idev->lock);