[PATCH] ipv6 addrconf: Ignore requests to set NODAD flag when changing addresses

[PATCH] ipv6 addrconf: Ignore requests to set NODAD flag when changing addresses

[Thomas Graf]

The decision to run DAD has already been made at the time the address
was added. Changing the flag later on does not have any effect and
is only confusing.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

Index: net-2.6/net/ipv6/addrconf.c
===================================================================
--- net-2.6.orig/net/ipv6/addrconf.c	2008-06-04 22:57:41.000000000 +0200
+++ net-2.6/net/ipv6/addrconf.c	2008-06-04 22:59:11.000000000 +0200
@@ -3200,7 +3200,7 @@
 	}
 
 	spin_lock_bh(&ifp->lock);
-	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
+	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_HOMEADDRESS)) | ifa_flags;
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = valid_lft;
 	ifp->prefered_lft = prefered_lft;
@@ -3267,6 +3267,9 @@
 				      preferred_lft, valid_lft);
 	}
 
+	/* It's too late to disable DOD now, ignore the flag */
+	ifa_flags &= ~IFA_F_NODAD;
+
 	if (nlh->nlmsg_flags & NLM_F_EXCL ||
 	    !(nlh->nlmsg_flags & NLM_F_REPLACE))
 		err = -EEXIST;

Re: [PATCH] ipv6 addrconf: Ignore requests to set NODAD flag when changing addresses

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20080604234751.GH20815@postel.suug.ch> (at Thu, 5 Jun 2008 01:47:51 +0200), Thomas Graf <tgraf@suug.ch> says:

> The decision to run DAD has already been made at the time the address
> was added. Changing the flag later on does not have any effect and
> is only confusing.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

I need let MIP team check this but anyway....

If the NODAD flag change is confusing, just clear
that flag in the caller.

And well, it does not take effect so far, but 
DAD after link failure logic is planned in long term.
So current code is okay to me.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3a83557..05dcfd8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3191,7 +3191,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u8 ifa_flags,
 		prefered_lft = 0x7FFFFFFF/HZ;
 
 	spin_lock_bh(&ifp->lock);
-	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
+	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_HOMEADDRESS)) | ifa_flags;
 	ifp->tstamp = jiffies;
 	ifp->valid_lft = valid_lft;
 	ifp->prefered_lft = prefered_lft;
@@ -3245,7 +3245,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 		return -ENODEV;
 
 	/* We ignore other flags so far. */
-	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
+	ifa_flags = ifm->ifa_flags & IFA_F_HOMEADDRESS;
 
 	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
 	if (ifa == NULL) {

--yoshfuji

Re: [PATCH] ipv6 addrconf: Ignore requests to set NODAD flag when changing addresses

[Thomas Graf]

* YOSHIFUJI Hideaki / ?$B5HF#1QL@ <yoshfuji@linux-ipv6.org> 2008-06-05 11:53
> And well, it does not take effect so far, but 
> DAD after link failure logic is planned in long term.
> So current code is okay to me.

Alright, that makes sense if that code ever is written.

I'm not sure I understand your new patch though, why do you want to
disable setting the nodad flag for new addresses nowW?

> 
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 3a83557..05dcfd8 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3191,7 +3191,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u8 ifa_flags,
>  		prefered_lft = 0x7FFFFFFF/HZ;
>  
>  	spin_lock_bh(&ifp->lock);
> -	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
> +	ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_HOMEADDRESS)) | ifa_flags;
>  	ifp->tstamp = jiffies;
>  	ifp->valid_lft = valid_lft;
>  	ifp->prefered_lft = prefered_lft;
> @@ -3245,7 +3245,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
>  		return -ENODEV;
>  
>  	/* We ignore other flags so far. */
> -	ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
> +	ifa_flags = ifm->ifa_flags & IFA_F_HOMEADDRESS;
>  
>  	ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
>  	if (ifa == NULL) {
> 
> --yoshfuji
> 

Re: [PATCH] ipv6 addrconf: Ignore requests to set NODAD flag when changing addresses

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20080605090447.GL20815@postel.suug.ch> (at Thu, 5 Jun 2008 11:04:47 +0200), Thomas Graf <tgraf@suug.ch> says:

> * YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> 2008-06-05 11:53
> > And well, it does not take effect so far, but 
> > DAD after link failure logic is planned in long term.
> > So current code is okay to me.
> 
> Alright, that makes sense if that code ever is written.
> 
> I'm not sure I understand your new patch though, why do you want to
> disable setting the nodad flag for new addresses nowW?

If we should really need to avoid the confusion, we have an option
to disallow modification of NODAD flag so far with the patch I've posted.
But anyway, I'm going to leave it as-is if you are okay.

--yoshfuji

Re: [PATCH] ipv6 addrconf: Ignore requests to set NODAD flag when changing addresses

[Thomas Graf]

* YOSHIFUJI Hideaki / ?$B5HF#1QL@ <yoshfuji@linux-ipv6.org> 2008-06-05 23:32
> In article <20080605090447.GL20815@postel.suug.ch> (at Thu, 5 Jun 2008 11:04:47 +0200), Thomas Graf <tgraf@suug.ch> says:
> 
> > * YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org> 2008-06-05 11:53
> > > And well, it does not take effect so far, but 
> > > DAD after link failure logic is planned in long term.
> > > So current code is okay to me.
> > 
> > Alright, that makes sense if that code ever is written.
> > 
> > I'm not sure I understand your new patch though, why do you want to
> > disable setting the nodad flag for new addresses nowW?
> 
> If we should really need to avoid the confusion, we have an option
> to disallow modification of NODAD flag so far with the patch I've posted.
> But anyway, I'm going to leave it as-is if you are okay.

Your patch would have disabled the ability to even add new addresses
with the NODAD flag set, that's what confused me. I'm fine with
leaving everything as-is since you explained that DAD will be done
after link failures in the future.