tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: PATCH to only announce RTM_NEWADDR once IPv6 DAD completes



On 15/05/2013 18:06, Greg Troxel wrote:
Roy Marples <roy%marples.name@localhost> writes:

You mean what happens when a conflict is detected?
I'm not sure that generating DELADDR is the right thing to do as it
does exist within the kernel.
Maybe a new message, RTM_DUPADDR? That suffers from the same possible
crash for badly written programs though.

Not specifically. I just meant that there is a correctness notion that
goes something like

   an address is in the kernel and not tentative
     if and only if
   a NEWADDR without a matching DELADDR has been received

so that a watcher that does bookeeping is guaranteed to have the right
state.  Right now I think that rule works, except for the 'not
tentative'.

If it's always the case that it's a one-way trip from tentative to
non-tentative to removed, then there are probably no issues.

Well, with this new patch there should be no issues.
I have also extended the patch slightly - we now issue a RTM_NEWADDR on
each flag change except when we add a new address marked tentative
or the flag changes to tentative. In this case we report the result of
the DAD. This mirrors Linux behavior which seems to work quite well.

Comments welcome.

If someone can suggest text to route(4) that would be nice to as I'm
suffering from writers block!

Thanks

Roy
Index: sys/netinet6/in6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/in6.c,v
retrieving revision 1.161
diff -u -p -r1.161 in6.c
--- sys/netinet6/in6.c  23 Jun 2012 03:14:03 -0000      1.161
+++ sys/netinet6/in6.c  18 May 2013 22:54:32 -0000
@@ -191,13 +191,29 @@ in6_ifloop_request(int cmd, struct ifadd
                rt_replace_ifa(nrt, ifa);
 
        /*
-        * Report the addition/removal of the address to the routing socket.
+        * Report the addition/removal of the address to the routing socket
+        * unless the address is marked tentative, where it will be reported
+        * once DAD completes.
         * XXX: since we called rtinit for a p2p interface with a destination,
         *      we end up reporting twice in such a case.  Should we rather
         *      omit the second report?
         */
        if (nrt) {
-               rt_newaddrmsg(cmd, ifa, e, nrt);
+               if (cmd != RTM_ADD ||
+                   !(((struct in6_ifaddr *)ifa)->ia6_flags &IN6_IFF_TENTATIVE))
+               {
+#if 0
+                       struct in6_ifaddr *ia;
+
+                       ia = (struct in6_ifaddr *)ifa;
+                       log(LOG_DEBUG,
+                           "in6_ifloop_request: announced %s (%s %d)\n",
+                           ip6_sprintf(&ia->ia_addr.sin6_addr),
+                           cmd == RTM_ADD ? "RTM_ADD" : "RTM_DELETE",
+                           ia->ia6_flags);
+#endif
+                       rt_newaddrmsg(cmd, ifa, e, nrt);
+               }
                if (cmd == RTM_DELETE) {
                        if (nrt->rt_refcnt <= 0) {
                                /* XXX: we should free the entry ourselves. */
@@ -1058,10 +1074,6 @@ in6_update_ifa1(struct ifnet *ifp, struc
        } else
                ia->ia6_lifetime.ia6t_preferred = 0;
 
-       /* reset the interface and routing table appropriately. */
-       if ((error = in6_ifinit(ifp, ia, &ifra->ifra_addr, hostIsNew)) != 0)
-               goto unlink;
-
        /*
         * configure address flags.
         */
@@ -1084,6 +1096,10 @@ in6_update_ifa1(struct ifnet *ifp, struc
        if (hostIsNew && in6if_do_dad(ifp)) 
                ia->ia6_flags |= IN6_IFF_TENTATIVE;
 
+       /* reset the interface and routing table appropriately. */
+       if ((error = in6_ifinit(ifp, ia, &ifra->ifra_addr, hostIsNew)) != 0)
+               goto unlink;
+
        /*
         * We are done if we have simply modified an existing address.
         */
Index: sys/netinet6/nd6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6.c,v
retrieving revision 1.144
diff -u -p -r1.144 nd6.c
--- sys/netinet6/nd6.c  24 Jan 2013 14:23:09 -0000      1.144
+++ sys/netinet6/nd6.c  18 May 2013 22:54:33 -0000
@@ -582,7 +582,10 @@ nd6_timer(void *ignored_arg)
                } else if (IFA6_IS_DEPRECATED(ia6)) {
                        int oldflags = ia6->ia6_flags;
 
-                       ia6->ia6_flags |= IN6_IFF_DEPRECATED;
+                       if ((oldflags & IN6_IFF_DEPRECATED) == 0) {
+                               ia6->ia6_flags |= IN6_IFF_DEPRECATED;
+                               nd6_newaddrmsg((struct ifaddr *)ia6);
+                       }
 
                        /*
                         * If a temporary address has just become deprecated,
@@ -613,7 +616,10 @@ nd6_timer(void *ignored_arg)
                         * A new RA might have made a deprecated address
                         * preferred.
                         */
-                       ia6->ia6_flags &= ~IN6_IFF_DEPRECATED;
+                       if (ia6->ia6_flags & IN6_IFF_DEPRECATED) {
+                               ia6->ia6_flags &= ~IN6_IFF_DEPRECATED;
+                               nd6_newaddrmsg((struct ifaddr *)ia6);
+                       }
                }
        }
 
Index: sys/netinet6/nd6.h
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6.h,v
retrieving revision 1.57
diff -u -p -r1.57 nd6.h
--- sys/netinet6/nd6.h  23 Jun 2012 03:14:04 -0000      1.57
+++ sys/netinet6/nd6.h  18 May 2013 22:54:33 -0000
@@ -435,6 +435,7 @@ void nd6_ns_input(struct mbuf *, int, in
 void nd6_ns_output(struct ifnet *, const struct in6_addr *,
        const struct in6_addr *, struct llinfo_nd6 *, int);
 const void *nd6_ifptomac(const struct ifnet *);
+void nd6_newaddrmsg(struct ifaddr *);
 void nd6_dad_start(struct ifaddr *, int);
 void nd6_dad_stop(struct ifaddr *);
 void nd6_dad_duplicated(struct ifaddr *);
Index: sys/netinet6/nd6_nbr.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.96
diff -u -p -r1.96 nd6_nbr.c
--- sys/netinet6/nd6_nbr.c      22 Mar 2012 20:34:41 -0000      1.96
+++ sys/netinet6/nd6_nbr.c      18 May 2013 22:54:33 -0000
@@ -1056,6 +1056,39 @@ nd6_dad_stoptimer(struct dadq *dp)
 }
 
 /*
+ * Routine to report address flag changes to the routing socket
+ */
+void
+nd6_newaddrmsg(struct ifaddr *ifa)
+{
+       struct sockaddr_in6 all1_sa;
+       struct rtentry *nrt = NULL;
+       int e;
+
+       sockaddr_in6_init(&all1_sa, &in6mask128, 0, 0, 0);
+
+       e = rtrequest(RTM_GET, ifa->ifa_addr, ifa->ifa_addr,
+           (struct sockaddr *)&all1_sa, RTF_UP|RTF_HOST|RTF_LLINFO, &nrt);
+       if (e != 0) {
+               log(LOG_ERR, "nd6_newaddrmsg: "
+                   "RTM_GET operation failed for %s (errno=%d)\n",
+                   ip6_sprintf(&((struct in6_ifaddr *)ifa)->ia_addr.sin6_addr),
+                   e);
+       }
+
+       if (nrt) {
+               rt_newaddrmsg(RTM_ADD, ifa, e, nrt);
+//#if 0
+               log(LOG_DEBUG, "nd6_newaddrmsg: announced %s\n",
+                   ip6_sprintf(&((struct in6_ifaddr *)ifa)->ia_addr.sin6_addr)
+               );
+//#endif
+               nrt->rt_refcnt--;
+       }
+}
+
+
+/*
  * Start Duplicate Address Detection (DAD) for specified interface address.
  *
  * xtick: minimum delay ticks for IFF_UP event
@@ -1085,12 +1118,9 @@ nd6_dad_start(struct ifaddr *ifa, int xt
                        ifa->ifa_ifp ? if_name(ifa->ifa_ifp) : "???");
                return;
        }
-       if (ia->ia6_flags & IN6_IFF_ANYCAST) {
-               ia->ia6_flags &= ~IN6_IFF_TENTATIVE;
-               return;
-       }
-       if (!ip6_dad_count) {
+       if (ia->ia6_flags & IN6_IFF_ANYCAST || !ip6_dad_count) {
                ia->ia6_flags &= ~IN6_IFF_TENTATIVE;
+               nd6_newaddrmsg(ifa);
                return;
        }
        if (ifa->ifa_ifp == NULL)
@@ -1246,6 +1276,7 @@ nd6_dad_timer(struct ifaddr *ifa)
                         * No duplicate address found.
                         */
                        ia->ia6_flags &= ~IN6_IFF_TENTATIVE;
+                       nd6_newaddrmsg(ifa);
 
                        nd6log((LOG_DEBUG,
                            "%s: DAD complete for %s - no duplicates found\n",
@@ -1294,6 +1325,9 @@ nd6_dad_duplicated(struct ifaddr *ifa)
        log(LOG_ERR, "%s: manual intervention required\n",
            if_name(ifp));
 
+       /* Inform the routing socket that DAD has completed */
+       nd6_newaddrmsg(ifa);
+
        /*
         * If the address is a link-local address formed from an interface
         * identifier based on the hardware address which is supposed to be
Index: sys/netinet6/nd6_rtr.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.86
diff -u -p -r1.86 nd6_rtr.c
--- sys/netinet6/nd6_rtr.c      18 Feb 2013 16:45:50 -0000      1.86
+++ sys/netinet6/nd6_rtr.c      18 May 2013 22:54:33 -0000
@@ -1580,10 +1580,14 @@ pfxlist_onlink_check(void)
                                        ifa->ia6_flags |= IN6_IFF_TENTATIVE;
                                        nd6_dad_start((struct ifaddr *)ifa,
                                            0);
+                                       /* We will notify the routing socket
+                                        * of the DAD result, so no need to
+                                        * here */
                                }
                        } else {
                                if ((ifa->ia6_flags & IN6_IFF_DETACHED) == 0) {
                                        ifa->ia6_flags |= IN6_IFF_DETACHED;
+                                       nd6_newaddrmsg((struct ifaddr *)ifa);
                                }
                        }
                }


Home | Main Index | Thread Index | Old Index