Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 7 Sep 2021 15:25:58 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 353783964c60 - main - ip6mrouter: Make the expiration callout MPSAFE
Message-ID:  <202109071525.187FPw7X055612@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=353783964c605e9cd71f359444c1230226d5782f

commit 353783964c605e9cd71f359444c1230226d5782f
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-09-07 15:15:49 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-09-07 15:19:29 +0000

    ip6mrouter: Make the expiration callout MPSAFE
    
    - Protect the `expire_upcalls` callout with the MFC6 mutex.  The callout
      handler needs this mutex anyway.
    - Convert the MROUTER6 mutex to a sleepable sx lock.  It is only used
      when configuring the global v6 multicast routing socket, so is only
      used in system call paths where sleeping is safe.  This lets us drain
      the callout without having to drop the lock.
    - For all locking macros in the file, convert to using a _LOCKPTR macro.
    
    Reported by:    mav
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31836
---
 sys/netinet6/ip6_mroute.c | 56 +++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/sys/netinet6/ip6_mroute.c b/sys/netinet6/ip6_mroute.c
index 503c83490940..087e0c5059fd 100644
--- a/sys/netinet6/ip6_mroute.c
+++ b/sys/netinet6/ip6_mroute.c
@@ -167,16 +167,13 @@ SYSCTL_STRUCT(_net_inet6_ip6, OID_AUTO, mrt6stat, CTLFLAG_RW,
 #define NO_RTE_FOUND	0x1
 #define RTE_FOUND	0x2
 
-static struct mtx mrouter6_mtx;
-#define	MROUTER6_LOCK()		mtx_lock(&mrouter6_mtx)
-#define	MROUTER6_UNLOCK()	mtx_unlock(&mrouter6_mtx)
-#define	MROUTER6_LOCK_ASSERT()	do {					\
-	mtx_assert(&mrouter6_mtx, MA_OWNED);				\
-	NET_ASSERT_GIANT();						\
-} while (0)
-#define	MROUTER6_LOCK_INIT()	\
-	mtx_init(&mrouter6_mtx, "IPv6 multicast forwarding", NULL, MTX_DEF)
-#define	MROUTER6_LOCK_DESTROY()	mtx_destroy(&mrouter6_mtx)
+static struct sx mrouter6_mtx;
+#define	MROUTER6_LOCKPTR()	(&mrouter6_mtx)
+#define	MROUTER6_LOCK()		sx_xlock(MROUTER6_LOCKPTR())
+#define	MROUTER6_UNLOCK()	sx_xunlock(MROUTER6_LOCKPTR())
+#define	MROUTER6_LOCK_ASSERT()	sx_assert(MROUTER6_LOCKPTR(), SA_XLOCKED
+#define	MROUTER6_LOCK_INIT()	sx_init(MROUTER6_LOCKPTR(), "mrouter6")
+#define	MROUTER6_LOCK_DESTROY()	sx_destroy(MROUTER6_LOCKPTR())
 
 static struct mf6c *mf6ctable[MF6CTBLSIZ];
 SYSCTL_OPAQUE(_net_inet6_ip6, OID_AUTO, mf6ctable, CTLFLAG_RD,
@@ -185,15 +182,14 @@ SYSCTL_OPAQUE(_net_inet6_ip6, OID_AUTO, mf6ctable, CTLFLAG_RD,
     "netinet6/ip6_mroute.h)");
 
 static struct mtx mfc6_mtx;
-#define	MFC6_LOCK()		mtx_lock(&mfc6_mtx)
-#define	MFC6_UNLOCK()		mtx_unlock(&mfc6_mtx)
-#define	MFC6_LOCK_ASSERT()	do {					\
-	mtx_assert(&mfc6_mtx, MA_OWNED);				\
-	NET_ASSERT_GIANT();						\
-} while (0)
-#define	MFC6_LOCK_INIT()		\
-	mtx_init(&mfc6_mtx, "IPv6 multicast forwarding cache", NULL, MTX_DEF)
-#define	MFC6_LOCK_DESTROY()	mtx_destroy(&mfc6_mtx)
+#define	MFC6_LOCKPTR()		(&mfc6_mtx)
+#define	MFC6_LOCK()		mtx_lock(MFC6_LOCKPTR())
+#define	MFC6_UNLOCK()		mtx_unlock(MFC6_LOCKPTR())
+#define	MFC6_LOCK_ASSERT()	mtx_assert(MFC6_LOCKPTR(), MA_OWNED)
+#define	MFC6_LOCK_INIT()	mtx_init(MFC6_LOCKPTR(),		\
+				    "IPv6 multicast forwarding cache",	\
+				    NULL, MTX_DEF)
+#define	MFC6_LOCK_DESTROY()	mtx_destroy(MFC6_LOCKPTR())
 
 static u_char n6expire[MF6CTBLSIZ];
 
@@ -230,12 +226,13 @@ SYSCTL_PROC(_net_inet6_ip6, OID_AUTO, mif6table,
     "netinet6/ip6_mroute.h)");
 
 static struct mtx mif6_mtx;
-#define	MIF6_LOCK()		mtx_lock(&mif6_mtx)
-#define	MIF6_UNLOCK()		mtx_unlock(&mif6_mtx)
-#define	MIF6_LOCK_ASSERT()	mtx_assert(&mif6_mtx, MA_OWNED)
+#define	MIF6_LOCKPTR()		(&mif6_mtx)
+#define	MIF6_LOCK()		mtx_lock(MIF6_LOCKPTR())
+#define	MIF6_UNLOCK()		mtx_unlock(MIF6_LOCKPTR())
+#define	MIF6_LOCK_ASSERT()	mtx_assert(MIF6_LOCKPTR(), MA_OWNED)
 #define	MIF6_LOCK_INIT()	\
-	mtx_init(&mif6_mtx, "IPv6 multicast interfaces", NULL, MTX_DEF)
-#define	MIF6_LOCK_DESTROY()	mtx_destroy(&mif6_mtx)
+	mtx_init(MIF6_LOCKPTR(), "IPv6 multicast interfaces", NULL, MTX_DEF)
+#define	MIF6_LOCK_DESTROY()	mtx_destroy(MIF6_LOCKPTR())
 
 #ifdef MRT6DEBUG
 VNET_DEFINE_STATIC(u_int, mrt6debug) = 0;	/* debug level */
@@ -583,11 +580,12 @@ ip6_mrouter_init(struct socket *so, int v, int cmd)
 
 	V_pim6 = 0;/* used for stubbing out/in pim stuff */
 
-	callout_init(&expire_upcalls_ch, 0);
+	callout_init_mtx(&expire_upcalls_ch, MFC6_LOCKPTR(), 0);
 	callout_reset(&expire_upcalls_ch, EXPIRE_TIMEOUT,
 	    expire_upcalls, NULL);
 
 	MROUTER6_UNLOCK();
+
 	MRT6_DLOG(DEBUG_ANY, "finished");
 
 	return (0);
@@ -626,8 +624,6 @@ X_ip6_mrouter_done(void)
 
 	V_pim6 = 0; /* used to stub out/in pim specific code */
 
-	callout_stop(&expire_upcalls_ch);
-
 	/*
 	 * Free all multicast forwarding cache entries.
 	 */
@@ -652,6 +648,8 @@ X_ip6_mrouter_done(void)
 	bzero((caddr_t)mf6ctable, sizeof(mf6ctable));
 	MFC6_UNLOCK();
 
+	callout_drain(&expire_upcalls_ch);
+
 	/*
 	 * Reset register interface
 	 */
@@ -1315,7 +1313,8 @@ expire_upcalls(void *unused)
 	struct mf6c *mfc, **nptr;
 	u_long i;
 
-	MFC6_LOCK();
+	MFC6_LOCK_ASSERT();
+
 	for (i = 0; i < MF6CTBLSIZ; i++) {
 		if (n6expire[i] == 0)
 			continue;
@@ -1353,7 +1352,6 @@ expire_upcalls(void *unused)
 			}
 		}
 	}
-	MFC6_UNLOCK();
 	callout_reset(&expire_upcalls_ch, EXPIRE_TIMEOUT,
 	    expire_upcalls, NULL);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202109071525.187FPw7X055612>