Re: Expand IN6_IFF_NOTREADY

2016-07-04 Thread Alexander Bluhm
On Mon, Jul 04, 2016 at 01:12:24PM +0200, Martin Pieuchot wrote:
> I find IPv6 address states (flags) really hard to follow.  This macro
> doesn't seem to help as it is not always used.  I'd like to get rid of
> it, any concern?

OK bluhm@

> 
> After applying this diff you'll see that some patterns appear and they
> make me wonder, are some IN6_IFF_DETACHED missing?
> 
> Index: netinet6/icmp6.c
> ===
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 icmp6.c
> --- netinet6/icmp6.c  29 Mar 2016 11:57:51 -  1.185
> +++ netinet6/icmp6.c  4 Jul 2016 11:06:29 -
> @@ -1229,13 +1229,14 @@ icmp6_reflect(struct mbuf *m, size_t off
>   /*
>* If the incoming packet was addressed directly to us (i.e. unicast),
>* use dst as the src for the reply.
> -  * The IN6_IFF_NOTREADY case would be VERY rare, but is possible
> -  * (for example) when we encounter an error while forwarding procedure
> -  * destined to a duplicated address of ours.
> +  * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be VERY rare,
> +  * but is possible (for example) when we encounter an error while
> +  * forwarding procedure destined to a duplicated address of ours.
>*/
>   TAILQ_FOREACH(ia6, _ifaddr, ia_list)
>   if (IN6_ARE_ADDR_EQUAL(, >ia_addr.sin6_addr) &&
> - (ia6->ia6_flags & (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY)) == 0) 
> {
> + (ia6->ia6_flags & (IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|
> + IN6_IFF_DUPLICATED)) == 0) {
>   src = 
>   break;
>   }
> @@ -1621,9 +1622,8 @@ icmp6_redirect_output(struct mbuf *m0, s
>   {
>   /* get ip6 linklocal address for ifp(my outgoing interface). */
>   struct in6_ifaddr *ia6;
> - if ((ia6 = in6ifa_ifpforlinklocal(ifp,
> -  IN6_IFF_NOTREADY|
> -  IN6_IFF_ANYCAST)) == NULL)
> + if ((ia6 = in6ifa_ifpforlinklocal(ifp, IN6_IFF_TENTATIVE|
> + IN6_IFF_DUPLICATED|IN6_IFF_ANYCAST)) == NULL)
>   goto fail;
>   ifp_ll6 = >ia_addr.sin6_addr;
>   }
> Index: netinet6/in6.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.187
> diff -u -p -r1.187 in6.c
> --- netinet6/in6.c13 Jun 2016 10:34:40 -  1.187
> +++ netinet6/in6.c4 Jul 2016 11:06:29 -
> @@ -1637,7 +1637,8 @@ in6_ifawithscope(struct ifnet *oifp, str
>* Don't use an address before completing DAD
>* nor a duplicated address.
>*/
> - if (ifatoia6(ifa)->ia6_flags & IN6_IFF_NOTREADY)
> + if (ifatoia6(ifa)->ia6_flags &
> + (IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED))
>   continue;
>  
>   /* XXX: is there any case to allow anycasts? */
> Index: netinet6/in6_pcb.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_pcb.c,v
> retrieving revision 1.92
> diff -u -p -r1.92 in6_pcb.c
> --- netinet6/in6_pcb.c11 Apr 2016 21:24:29 -  1.92
> +++ netinet6/in6_pcb.c4 Jul 2016 11:06:29 -
> @@ -208,9 +208,8 @@ in6_pcbaddrisavail(struct inpcb *inp, st
>* flag to control the bind(2) behavior against
>* deprecated addresses (default: forbid bind(2)).
>*/
> - if (ifa &&
> - ifatoia6(ifa)->ia6_flags &
> - (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED))
> + if (ifa && ifatoia6(ifa)->ia6_flags & (IN6_IFF_ANYCAST|
> + IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED|IN6_IFF_DETACHED))
>   return (EADDRNOTAVAIL);
>   }
>   if (lport) {
> Index: netinet6/in6_src.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 in6_src.c
> --- netinet6/in6_src.c30 Jun 2016 12:36:27 -  1.74
> +++ netinet6/in6_src.c4 Jul 2016 11:06:29 -
> @@ -130,8 +130,8 @@ in6_selectsrc(struct in6_addr **in6src, 
>   if_put(ifp); /* put reference from in6_selectif */
>  
>   ia6 = ifatoia6(ifa_ifwithaddr(sin6tosa(), rtableid));
> - if (ia6 == NULL ||
> - (ia6->ia6_flags & (IN6_IFF_ANYCAST | IN6_IFF_NOTREADY)))
> + if (ia6 == NULL || (ia6->ia6_flags &
> +  (IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)))
>   return (EADDRNOTAVAIL);
>  
>   pi->ipi6_addr = sa6.sin6_addr; /* XXX: this overrides pi */

Expand IN6_IFF_NOTREADY

2016-07-04 Thread Martin Pieuchot
I find IPv6 address states (flags) really hard to follow.  This macro
doesn't seem to help as it is not always used.  I'd like to get rid of
it, any concern?

After applying this diff you'll see that some patterns appear and they
make me wonder, are some IN6_IFF_DETACHED missing?

Index: netinet6/icmp6.c
===
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.185
diff -u -p -r1.185 icmp6.c
--- netinet6/icmp6.c29 Mar 2016 11:57:51 -  1.185
+++ netinet6/icmp6.c4 Jul 2016 11:06:29 -
@@ -1229,13 +1229,14 @@ icmp6_reflect(struct mbuf *m, size_t off
/*
 * If the incoming packet was addressed directly to us (i.e. unicast),
 * use dst as the src for the reply.
-* The IN6_IFF_NOTREADY case would be VERY rare, but is possible
-* (for example) when we encounter an error while forwarding procedure
-* destined to a duplicated address of ours.
+* The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be VERY rare,
+* but is possible (for example) when we encounter an error while
+* forwarding procedure destined to a duplicated address of ours.
 */
TAILQ_FOREACH(ia6, _ifaddr, ia_list)
if (IN6_ARE_ADDR_EQUAL(, >ia_addr.sin6_addr) &&
-   (ia6->ia6_flags & (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY)) == 0) 
{
+   (ia6->ia6_flags & (IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|
+   IN6_IFF_DUPLICATED)) == 0) {
src = 
break;
}
@@ -1621,9 +1622,8 @@ icmp6_redirect_output(struct mbuf *m0, s
{
/* get ip6 linklocal address for ifp(my outgoing interface). */
struct in6_ifaddr *ia6;
-   if ((ia6 = in6ifa_ifpforlinklocal(ifp,
-IN6_IFF_NOTREADY|
-IN6_IFF_ANYCAST)) == NULL)
+   if ((ia6 = in6ifa_ifpforlinklocal(ifp, IN6_IFF_TENTATIVE|
+   IN6_IFF_DUPLICATED|IN6_IFF_ANYCAST)) == NULL)
goto fail;
ifp_ll6 = >ia_addr.sin6_addr;
}
Index: netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.187
diff -u -p -r1.187 in6.c
--- netinet6/in6.c  13 Jun 2016 10:34:40 -  1.187
+++ netinet6/in6.c  4 Jul 2016 11:06:29 -
@@ -1637,7 +1637,8 @@ in6_ifawithscope(struct ifnet *oifp, str
 * Don't use an address before completing DAD
 * nor a duplicated address.
 */
-   if (ifatoia6(ifa)->ia6_flags & IN6_IFF_NOTREADY)
+   if (ifatoia6(ifa)->ia6_flags &
+   (IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED))
continue;
 
/* XXX: is there any case to allow anycasts? */
Index: netinet6/in6_pcb.c
===
RCS file: /cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.92
diff -u -p -r1.92 in6_pcb.c
--- netinet6/in6_pcb.c  11 Apr 2016 21:24:29 -  1.92
+++ netinet6/in6_pcb.c  4 Jul 2016 11:06:29 -
@@ -208,9 +208,8 @@ in6_pcbaddrisavail(struct inpcb *inp, st
 * flag to control the bind(2) behavior against
 * deprecated addresses (default: forbid bind(2)).
 */
-   if (ifa &&
-   ifatoia6(ifa)->ia6_flags &
-   (IN6_IFF_ANYCAST|IN6_IFF_NOTREADY|IN6_IFF_DETACHED))
+   if (ifa && ifatoia6(ifa)->ia6_flags & (IN6_IFF_ANYCAST|
+   IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED|IN6_IFF_DETACHED))
return (EADDRNOTAVAIL);
}
if (lport) {
Index: netinet6/in6_src.c
===
RCS file: /cvs/src/sys/netinet6/in6_src.c,v
retrieving revision 1.74
diff -u -p -r1.74 in6_src.c
--- netinet6/in6_src.c  30 Jun 2016 12:36:27 -  1.74
+++ netinet6/in6_src.c  4 Jul 2016 11:06:29 -
@@ -130,8 +130,8 @@ in6_selectsrc(struct in6_addr **in6src, 
if_put(ifp); /* put reference from in6_selectif */
 
ia6 = ifatoia6(ifa_ifwithaddr(sin6tosa(), rtableid));
-   if (ia6 == NULL ||
-   (ia6->ia6_flags & (IN6_IFF_ANYCAST | IN6_IFF_NOTREADY)))
+   if (ia6 == NULL || (ia6->ia6_flags &
+(IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)))
return (EADDRNOTAVAIL);
 
pi->ipi6_addr = sa6.sin6_addr; /* XXX: this overrides pi */
Index: netinet6/in6_var.h
===
RCS file: /cvs/src/sys/netinet6/in6_var.h,v
retrieving revision 1.63
diff -u -p -r1.63