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

login
register
mail settings
Submitter stephen hemminger
Date Feb. 9, 2010, 5:48 a.m.
Message ID <20100208214805.1f8f2c6d@nehalam>
Download mbox | patch
Permalink /patch/44878/
State Accepted
Delegated to: David Miller
Headers show

Comments

stephen hemminger - Feb. 9, 2010, 5:48 a.m.
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
Brian Haley - Feb. 9, 2010, 2:40 p.m.
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.
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.
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

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