Remove ability for userland to toggle IPv6 tentative address flag

[Roy Marples <roy%marples.name@localhost>]

Hi List

Can anyone think of a use case for allowing userland to toggle the 
IN6_IFF_TENTATIVE flag for IPv6 addresses?

I can't.

The way it's presently implemented can cause a problem as well when the same 
address is added twice, because the logic is thus:

Add IPv6 address without tentative flag
Kernel checks if it's a new address, if so sets tentative flag and starts DAD 
after a random delay.
Add the same IPv6 address without tentative flag again while DAD is still 
running.
Kernel sees it's the same address, so blindly updates the flags which clears 
IN6_IFF_TENTATIVE. This then confuses the DAD process. I've seen it not notify 
userland of the flag being cleared whilst others have reported invalid 
duplicate addresses being reported.

So, can you think of a valid use case for allowing userland to toggle the 
tentative flag? Note that setting it doesn't actually start DAD, just makes 
the address unuseable.

Attached is the patch to remove the ability for userland to toggle 
IN6_IFF_TENTATIVE.

Comments as always are welcome

Roy

Index: sbin/ifconfig/af_inet6.c
===================================================================
RCS file: /cvsroot/src/sbin/ifconfig/af_inet6.c,v
retrieving revision 1.29
diff -u -p -r1.29 af_inet6.c
--- sbin/ifconfig/af_inet6.c	19 Oct 2013 15:50:26 -0000	1.29
+++ sbin/ifconfig/af_inet6.c	12 Oct 2014 20:28:32 -0000
@@ -78,7 +78,6 @@ static cmdloop_branch_t branch[2];
 
 static const struct kwinst ia6flagskw[] = {
 	  IFKW("anycast",	IN6_IFF_ANYCAST)
-	, IFKW("tentative",	IN6_IFF_TENTATIVE)
 	, IFKW("deprecated",	IN6_IFF_DEPRECATED)
 };
 
@@ -476,7 +475,7 @@ in6_usage(prop_dictionary_t env)
 {
 	fprintf(stderr,
 	    "\t[ anycast | -anycast ] [ deprecated | -deprecated ]\n"
-	    "\t[ tentative | -tentative ] [ pltime n ] [ vltime n ] "
+	    "\t[ pltime n ] [ vltime n ] "
 	    "[ eui64 ]\n");
 }
 
Index: sbin/ifconfig/ifconfig.8
===================================================================
RCS file: /cvsroot/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.108
diff -u -p -r1.108 ifconfig.8
--- sbin/ifconfig/ifconfig.8	15 Sep 2014 06:48:05 -0000	1.108
+++ sbin/ifconfig/ifconfig.8	12 Oct 2014 20:28:32 -0000
@@ -29,7 +29,7 @@
 .\"
 .\"     @(#)ifconfig.8	8.4 (Berkeley) 6/1/94
 .\"
-.Dd September 15, 2014
+.Dd October 12, 2014
 .Dt IFCONFIG 8
 .Os
 .Sh NAME
@@ -551,12 +551,6 @@ Set the IPv6 deprecated address bit.
 .It Fl deprecated
 .Pq inet6 only
 Clear the IPv6 deprecated address bit.
-.It Cm tentative
-.Pq inet6 only
-Set the IPv6 tentative address bit.
-.It Fl tentative
-.Pq inet6 only
-Clear the IPv6 tentative address bit.
 .It Cm eui64
 .Pq inet6 only
 Fill interface index
Index: sys/netinet6/in6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/in6.c,v
retrieving revision 1.176
diff -u -p -r1.176 in6.c
--- sys/netinet6/in6.c	9 Sep 2014 20:16:12 -0000	1.176
+++ sys/netinet6/in6.c	12 Oct 2014 20:28:33 -0000
@@ -649,6 +649,7 @@ in6_control1(struct socket *so, u_long c
 		/* reject read-only flags */
 		if ((ifra->ifra_flags & IN6_IFF_DUPLICATED) != 0 ||
 		    (ifra->ifra_flags & IN6_IFF_DETACHED) != 0 ||
+		    (ifra->ifra_flags & IN6_IFF_TENTATIVE) != 0 ||
 		    (ifra->ifra_flags & IN6_IFF_NODAD) != 0 ||
 		    (ifra->ifra_flags & IN6_IFF_AUTOCONF) != 0) {
 			return EINVAL;
@@ -834,7 +835,7 @@ in6_update_ifa1(struct ifnet *ifp, struc
 	struct in6_multi_mship *imm;
 	struct in6_multi *in6m_sol;
 	struct rtentry *rt;
-	int dad_delay;
+	int dad_delay, was_tentative;
 
 	in6m_sol = NULL;
 
@@ -1063,7 +1064,10 @@ in6_update_ifa1(struct ifnet *ifp, struc
 
 	/*
 	 * configure address flags.
+	 * We need to preserve tentative state so DAD works if
+	 * something adds the same address before DAD finishes.
 	 */
+	was_tentative = ia->ia6_flags & (IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED);
 	ia->ia6_flags = ifra->ifra_flags;
 
 	/*
@@ -1075,7 +1079,7 @@ in6_update_ifa1(struct ifnet *ifp, struc
 	if (ifp->if_link_state == LINK_STATE_DOWN) {
 		ia->ia6_flags |= IN6_IFF_DETACHED;
 		ia->ia6_flags &= ~IN6_IFF_TENTATIVE;
-	} else if (hostIsNew && in6if_do_dad(ifp))
+	} else if ((hostIsNew || was_tentative) && in6if_do_dad(ifp))
 		ia->ia6_flags |= IN6_IFF_TENTATIVE;
 
 	/*