Source-Changes-HG archive

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

[src/trunk]: src/sys Make DAD of ARP/NDP MP-safe with coarse-grained locks



details:   https://anonhg.NetBSD.org/src/rev/89f17be8d99b
branches:  trunk
changeset: 346643:89f17be8d99b
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Mon Jul 25 04:21:19 2016 +0000

description:
Make DAD of ARP/NDP MP-safe with coarse-grained locks

The change also prevents arp_dad_timer/nd6_dad_timer from running if
arp_dad_stop/nd6_dad_stop is called, which makes sure that callout_reset
won't be called during callout_halt.

diffstat:

 sys/netinet/if_arp.c   |  40 +++++++++++++++++++++++++++++++---------
 sys/netinet6/nd6_nbr.c |  46 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 68 insertions(+), 18 deletions(-)

diffs (truncated from 353 to 300 lines):

diff -r a777f0eb5b67 -r 89f17be8d99b sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c      Mon Jul 25 01:52:21 2016 +0000
+++ b/sys/netinet/if_arp.c      Mon Jul 25 04:21:19 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arp.c,v 1.218 2016/07/25 01:52:21 ozaki-r Exp $     */
+/*     $NetBSD: if_arp.c,v 1.219 2016/07/25 04:21:19 ozaki-r Exp $     */
 
 /*-
  * Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.218 2016/07/25 01:52:21 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.219 2016/07/25 04:21:19 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -1477,12 +1477,15 @@
 static struct dadq_head dadq;
 static int dad_init = 0;
 static int dad_maxtry = 15;     /* max # of *tries* to transmit DAD packet */
+static kmutex_t arp_dad_lock;
 
 static struct dadq *
 arp_dad_find(struct ifaddr *ifa)
 {
        struct dadq *dp;
 
+       KASSERT(mutex_owned(&arp_dad_lock));
+
        TAILQ_FOREACH(dp, &dadq, dad_list) {
                if (dp->dad_ifa == ifa)
                        return dp;
@@ -1537,6 +1540,7 @@
 
        if (!dad_init) {
                TAILQ_INIT(&dadq);
+               mutex_init(&arp_dad_lock, MUTEX_DEFAULT, IPL_NONE);
                dad_init++;
        }
 
@@ -1563,13 +1567,17 @@
        KASSERT(ifa->ifa_ifp != NULL);
        if (!(ifa->ifa_ifp->if_flags & IFF_UP))
                return;
+
+       mutex_enter(&arp_dad_lock);
        if (arp_dad_find(ifa) != NULL) {
+               mutex_exit(&arp_dad_lock);
                /* DAD already in progress */
                return;
        }
 
        dp = malloc(sizeof(*dp), M_IPARP, M_NOWAIT);
        if (dp == NULL) {
+               mutex_exit(&arp_dad_lock);
                log(LOG_ERR, "%s: memory allocation failed for %s(%s)\n",
                    __func__, in_fmtaddr(ia->ia_addr.sin_addr),
                    ifa->ifa_ifp ? if_name(ifa->ifa_ifp) : "???");
@@ -1577,10 +1585,6 @@
        }
        memset(dp, 0, sizeof(*dp));
        callout_init(&dp->dad_timer_ch, CALLOUT_MPSAFE);
-       TAILQ_INSERT_TAIL(&dadq, (struct dadq *)dp, dad_list);
-
-       arplog((LOG_DEBUG, "%s: starting DAD for %s\n", if_name(ifa->ifa_ifp),
-           in_fmtaddr(ia->ia_addr.sin_addr)));
 
        /*
         * Send ARP packet for DAD, ip_dad_count times.
@@ -1591,8 +1595,14 @@
        dp->dad_count = ip_dad_count;
        dp->dad_arp_announce = 0; /* Will be set when starting to announce */
        dp->dad_arp_acount = dp->dad_arp_ocount = dp->dad_arp_tcount = 0;
+       TAILQ_INSERT_TAIL(&dadq, (struct dadq *)dp, dad_list);
+
+       arplog((LOG_DEBUG, "%s: starting DAD for %s\n", if_name(ifa->ifa_ifp),
+           in_fmtaddr(ia->ia_addr.sin_addr)));
 
        arp_dad_starttimer(dp, cprng_fast32() % (PROBE_WAIT * hz));
+
+       mutex_exit(&arp_dad_lock);
 }
 
 /*
@@ -1605,15 +1615,21 @@
 
        if (!dad_init)
                return;
+
+       mutex_enter(&arp_dad_lock);
        dp = arp_dad_find(ifa);
        if (dp == NULL) {
+               mutex_exit(&arp_dad_lock);
                /* DAD wasn't started yet */
                return;
        }
 
+       /* Prevent the timer from running anymore. */
+       TAILQ_REMOVE(&dadq, dp, dad_list);
+       mutex_exit(&arp_dad_lock);
+
        arp_dad_stoptimer(dp);
 
-       TAILQ_REMOVE(&dadq, dp, dad_list);
        free(dp, M_IPARP);
        dp = NULL;
        ifafree(ifa);
@@ -1628,6 +1644,7 @@
 
        mutex_enter(softnet_lock);
        KERNEL_LOCK(1, NULL);
+       mutex_enter(&arp_dad_lock);
 
        /* Sanity check */
        if (ia == NULL) {
@@ -1636,7 +1653,7 @@
        }
        dp = arp_dad_find(ifa);
        if (dp == NULL) {
-               log(LOG_ERR, "%s: DAD structure not found\n", __func__);
+               /* DAD seems to be stopping, so do nothing. */
                goto done;
        }
        if (ia->ia4_flags & IN_IFF_DUPLICATED) {
@@ -1719,6 +1736,7 @@
        ifafree(ifa);
 
 done:
+       mutex_exit(&arp_dad_lock);
        KERNEL_UNLOCK_ONE(NULL);
        mutex_exit(softnet_lock);
 }
@@ -1730,9 +1748,11 @@
        struct ifnet *ifp;
        struct dadq *dp;
 
+       mutex_enter(&arp_dad_lock);
        dp = arp_dad_find(ifa);
        if (dp == NULL) {
-               log(LOG_ERR, "%s: DAD structure not found\n", __func__);
+               mutex_exit(&arp_dad_lock);
+               /* DAD seems to be stopping, so do nothing. */
                return;
        }
 
@@ -1752,6 +1772,8 @@
        rt_newaddrmsg(RTM_NEWADDR, ifa, 0, NULL);
 
        TAILQ_REMOVE(&dadq, dp, dad_list);
+       mutex_exit(&arp_dad_lock);
+
        free(dp, M_IPARP);
        dp = NULL;
        ifafree(ifa);
diff -r a777f0eb5b67 -r 89f17be8d99b sys/netinet6/nd6_nbr.c
--- a/sys/netinet6/nd6_nbr.c    Mon Jul 25 01:52:21 2016 +0000
+++ b/sys/netinet6/nd6_nbr.c    Mon Jul 25 04:21:19 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nd6_nbr.c,v 1.124 2016/07/25 01:52:21 ozaki-r Exp $    */
+/*     $NetBSD: nd6_nbr.c,v 1.125 2016/07/25 04:21:20 ozaki-r Exp $    */
 /*     $KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $        */
 
 /*
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.124 2016/07/25 01:52:21 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.125 2016/07/25 04:21:20 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1049,12 +1049,15 @@
 
 static struct dadq_head dadq;
 static int dad_init = 0;
+static kmutex_t nd6_dad_lock;
 
 static struct dadq *
 nd6_dad_find(struct ifaddr *ifa)
 {
        struct dadq *dp;
 
+       KASSERT(mutex_owned(&nd6_dad_lock));
+
        TAILQ_FOREACH(dp, &dadq, dad_list) {
                if (dp->dad_ifa == ifa)
                        return dp;
@@ -1092,6 +1095,7 @@
 
        if (!dad_init) {
                TAILQ_INIT(&dadq);
+               mutex_init(&nd6_dad_lock, MUTEX_DEFAULT, IPL_NONE);
                dad_init++;
        }
 
@@ -1117,13 +1121,17 @@
        KASSERT(ifa->ifa_ifp != NULL);
        if (!(ifa->ifa_ifp->if_flags & IFF_UP))
                return;
+
+       mutex_enter(&nd6_dad_lock);
        if (nd6_dad_find(ifa) != NULL) {
+               mutex_exit(&nd6_dad_lock);
                /* DAD already in progress */
                return;
        }
 
        dp = malloc(sizeof(*dp), M_IP6NDP, M_NOWAIT);
        if (dp == NULL) {
+               mutex_exit(&nd6_dad_lock);
                log(LOG_ERR, "nd6_dad_start: memory allocation failed for "
                        "%s(%s)\n",
                        ip6_sprintf(&ia->ia_addr.sin6_addr),
@@ -1132,10 +1140,6 @@
        }
        memset(dp, 0, sizeof(*dp));
        callout_init(&dp->dad_timer_ch, CALLOUT_MPSAFE);
-       TAILQ_INSERT_TAIL(&dadq, (struct dadq *)dp, dad_list);
-
-       nd6log(LOG_DEBUG, "%s: starting DAD for %s\n", if_name(ifa->ifa_ifp),
-           ip6_sprintf(&ia->ia_addr.sin6_addr));
 
        /*
         * Send NS packet for DAD, ip6_dad_count times.
@@ -1148,12 +1152,18 @@
        dp->dad_count = ip6_dad_count;
        dp->dad_ns_icount = dp->dad_na_icount = 0;
        dp->dad_ns_ocount = dp->dad_ns_tcount = 0;
+       TAILQ_INSERT_TAIL(&dadq, (struct dadq *)dp, dad_list);
+
+       nd6log(LOG_DEBUG, "%s: starting DAD for %s\n", if_name(ifa->ifa_ifp),
+           ip6_sprintf(&ia->ia_addr.sin6_addr));
+
        if (xtick == 0) {
                nd6_dad_ns_output(dp, ifa);
                nd6_dad_starttimer(dp,
                    (long)ND_IFINFO(ifa->ifa_ifp)->retrans * hz / 1000);
        } else
                nd6_dad_starttimer(dp, xtick);
+       mutex_exit(&nd6_dad_lock);
 }
 
 /*
@@ -1166,15 +1176,21 @@
 
        if (!dad_init)
                return;
+
+       mutex_enter(&nd6_dad_lock);
        dp = nd6_dad_find(ifa);
        if (dp == NULL) {
+               mutex_exit(&nd6_dad_lock);
                /* DAD wasn't started yet */
                return;
        }
 
+       /* Prevent the timer from running anymore. */
+       TAILQ_REMOVE(&dadq, dp, dad_list);
+       mutex_exit(&nd6_dad_lock);
+
        nd6_dad_stoptimer(dp);
 
-       TAILQ_REMOVE(&dadq, dp, dad_list);
        free(dp, M_IP6NDP);
        dp = NULL;
        ifafree(ifa);
@@ -1188,6 +1204,7 @@
 
        mutex_enter(softnet_lock);
        KERNEL_LOCK(1, NULL);
+       mutex_enter(&nd6_dad_lock);
 
        /* Sanity check */
        if (ia == NULL) {
@@ -1196,7 +1213,7 @@
        }
        dp = nd6_dad_find(ifa);
        if (dp == NULL) {
-               log(LOG_ERR, "nd6_dad_timer: DAD structure not found\n");
+               /* DAD seems to be stopping, so do nothing. */
                goto done;
        }
        if (ia->ia6_flags & IN6_IFF_DUPLICATED) {
@@ -1281,6 +1298,7 @@
        }
 
 done:
+       mutex_exit(&nd6_dad_lock);
        KERNEL_UNLOCK_ONE(NULL);
        mutex_exit(softnet_lock);
 }
@@ -1292,9 +1310,11 @@
        struct ifnet *ifp;
        struct dadq *dp;
 
+       mutex_enter(&nd6_dad_lock);
        dp = nd6_dad_find(ifa);



Home | Main Index | Thread Index | Old Index