Module Name: src
Committed By: ozaki-r
Date: Wed Dec 6 09:03:13 UTC 2017
Modified Files:
src/sys/dev/pci: if_wm.c
src/sys/net: if.c if.h
Log Message:
Make if_timer MP-safe if IFEF_MPSAFE
if_timer, a counter used by if_watchdog (if_slowtimo), can be modified in
if_watchdog and if_start and/or interrupt handlers of some device drivers. All
such accesses were serialized by KERNEL_LOCK. If IFEF_MPSAFE is enabled,
KERNEL_LOCK of if_start (and perhaps interrupt handlers) is omitted and if_timer
becomes racy.
Fix the race condition by protecting if_timer by a spin mutex. if_watchdog_reset
and if_watchdog_stop are introduced to ensure to take the mutex on accessing
if_timer. Interface with IFEF_MPSAFE enabled must use the functions.
In addition, if_watchdog callout is now set CALLOUT_MPSAFE if IFEF_MPSAFE. It
means that if_watchdog implemented by a driver must be MP-safe if the driver is
set IFEF_MPSAFE.
Currenlty interfaces with IFEF_MPSAFE implementing if_watchdog and accessing
if_timer in if_start and interrupt handlers are only wm(4). wm is changed to
use the functions. (Its watchdog handler (wm_watchdog) is already MP-safe.
These contracts will be written somewhere in a further commit.
Note that the spin mutex is now ifp->if_snd.ifq_lock to avoid adding another
spin mutex to each interface. For now reusing it isn't problematic (see the
comment to know why) thought if that does matter in the future, feel free to
replace it with a new spin mutex. It's easy to do.
To generate a diff of this commit:
cvs rdiff -u -r1.546 -r1.547 src/sys/dev/pci/if_wm.c
cvs rdiff -u -r1.404 -r1.405 src/sys/net/if.c
cvs rdiff -u -r1.248 -r1.249 src/sys/net/if.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/pci/if_wm.c
diff -u src/sys/dev/pci/if_wm.c:1.546 src/sys/dev/pci/if_wm.c:1.547
--- src/sys/dev/pci/if_wm.c:1.546 Thu Nov 30 09:24:18 2017
+++ src/sys/dev/pci/if_wm.c Wed Dec 6 09:03:12 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: if_wm.c,v 1.546 2017/11/30 09:24:18 msaitoh Exp $ */
+/* $NetBSD: if_wm.c,v 1.547 2017/12/06 09:03:12 ozaki-r Exp $ */
/*
* Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -83,7 +83,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.546 2017/11/30 09:24:18 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.547 2017/12/06 09:03:12 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_net_mpsafe.h"
@@ -5932,7 +5932,7 @@ wm_stop_locked(struct ifnet *ifp, int di
/* Mark the interface as down and cancel the watchdog timer. */
ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
- ifp->if_timer = 0;
+ if_watchdog_stop(ifp);
if (disable) {
for (i = 0; i < sc->sc_nqueues; i++) {
@@ -7368,7 +7368,7 @@ wm_send_common_locked(struct ifnet *ifp,
if (txq->txq_free != ofree) {
/* Set a watchdog timer in case the chip flakes out. */
- ifp->if_timer = 5;
+ if_watchdog_reset(ifp, 5);
}
}
@@ -7940,7 +7940,7 @@ wm_nq_send_common_locked(struct ifnet *i
if (sent) {
/* Set a watchdog timer in case the chip flakes out. */
- ifp->if_timer = 5;
+ if_watchdog_reset(ifp, 5);
}
}
@@ -8077,7 +8077,7 @@ wm_txeof(struct wm_softc *sc, struct wm_
* timer.
*/
if (txq->txq_sfree == WM_TXQUEUELEN(txq))
- ifp->if_timer = 0;
+ if_watchdog_stop(ifp);
return processed;
}
Index: src/sys/net/if.c
diff -u src/sys/net/if.c:1.404 src/sys/net/if.c:1.405
--- src/sys/net/if.c:1.404 Wed Dec 6 08:23:17 2017
+++ src/sys/net/if.c Wed Dec 6 09:03:12 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: if.c,v 1.404 2017/12/06 08:23:17 knakahara Exp $ */
+/* $NetBSD: if.c,v 1.405 2017/12/06 09:03:12 ozaki-r Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.404 2017/12/06 08:23:17 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.405 2017/12/06 09:03:12 ozaki-r Exp $");
#if defined(_KERNEL_OPT)
#include "opt_inet.h"
@@ -768,9 +768,11 @@ if_register(ifnet_t *ifp)
rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
if (ifp->if_slowtimo != NULL) {
+ int flags = ISSET(ifp->if_extflags, IFEF_MPSAFE) ?
+ CALLOUT_MPSAFE : 0;
ifp->if_slowtimo_ch =
kmem_zalloc(sizeof(*ifp->if_slowtimo_ch), KM_SLEEP);
- callout_init(ifp->if_slowtimo_ch, 0);
+ callout_init(ifp->if_slowtimo_ch, flags);
callout_setfunc(ifp->if_slowtimo_ch, if_slowtimo, ifp);
if_slowtimo(ifp);
}
@@ -2503,6 +2505,18 @@ if_up_locked(struct ifnet *ifp)
}
/*
+ * XXX reusing (ifp)->if_snd->ifq_lock rather than having another spin mutex
+ * for each ifnet. It doesn't matter because:
+ * - if IFEF_MPSAFE is enabled, if_snd isn't used and lock contention on
+ * ifq_lock don't happen
+ * - if IFEF_MPSAFE is disabled, there is no lock contention on ifq_lock
+ * because if_snd and if_watchdog_reset is used with KERNEL_LOCK on packet
+ * transmissions and if_slowtimo is also called with KERNEL_LOCK
+ */
+#define IF_WATCHDOG_LOCK(ifp) mutex_enter((ifp)->if_snd.ifq_lock)
+#define IF_WATCHDOG_UNLOCK(ifp) mutex_exit((ifp)->if_snd.ifq_lock)
+
+/*
* Handle interface slowtimo timer routine. Called
* from softclock, we decrement timer (if set) and
* call the appropriate interface routine on expiration.
@@ -2513,21 +2527,40 @@ if_slowtimo(void *arg)
void (*slowtimo)(struct ifnet *);
struct ifnet *ifp = arg;
int s;
+ bool fire;
slowtimo = ifp->if_slowtimo;
if (__predict_false(slowtimo == NULL))
return;
s = splnet();
- if (ifp->if_timer != 0 && --ifp->if_timer == 0)
+ IF_WATCHDOG_LOCK(ifp);
+ fire = (ifp->if_timer != 0 && --ifp->if_timer == 0);
+ IF_WATCHDOG_UNLOCK(ifp);
+ if (fire)
(*slowtimo)(ifp);
-
splx(s);
if (__predict_true(ifp->if_slowtimo != NULL))
callout_schedule(ifp->if_slowtimo_ch, hz / IFNET_SLOWHZ);
}
+void
+if_watchdog_reset(struct ifnet *ifp, short v)
+{
+
+ IF_WATCHDOG_LOCK(ifp);
+ ifp->if_timer = v;
+ IF_WATCHDOG_UNLOCK(ifp);
+}
+
+void
+if_watchdog_stop(struct ifnet *ifp)
+{
+
+ if_watchdog_reset(ifp, 0);
+}
+
/*
* Mark an interface up and notify protocols of
* the transition.
Index: src/sys/net/if.h
diff -u src/sys/net/if.h:1.248 src/sys/net/if.h:1.249
--- src/sys/net/if.h:1.248 Wed Dec 6 08:23:17 2017
+++ src/sys/net/if.h Wed Dec 6 09:03:12 2017
@@ -1,4 +1,4 @@
-/* $NetBSD: if.h,v 1.248 2017/12/06 08:23:17 knakahara Exp $ */
+/* $NetBSD: if.h,v 1.249 2017/12/06 09:03:12 ozaki-r Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -1047,6 +1047,9 @@ bool if_held(struct ifnet *);
void if_input(struct ifnet *, struct mbuf *);
+void if_watchdog_reset(struct ifnet *, short);
+void if_watchdog_stop(struct ifnet *);
+
struct if_percpuq *
if_percpuq_create(struct ifnet *);
void if_percpuq_destroy(struct if_percpuq *);
