Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 1 Apr 2019 14:19:10 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r345764 - stable/12/sys/net
Message-ID:  <201904011419.x31EJAwd040790@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: markj
Date: Mon Apr  1 14:19:09 2019
New Revision: 345764
URL: https://svnweb.freebsd.org/changeset/base/345764

Log:
  Fix if_(m)addr_rlock().
  
  The use of a per-ifnet epoch context meant that these KPIs were not
  reentrant.  This was fixed in head in r340413, but the change cannot
  be MFCed because it breaks the KBI by modifying struct thread.  This
  is a direct commit to stable/12 which uses a per-CPU mutex to fix
  the problem without changing the KBI.
  
  PR:		236846
  Submitted by:	hselasky
  Reported and tested by:	Viktor Dukhovni <ietf-dane@dukhovni.org>
  Reviewed by:	hselasky (previous version)
  Differential Revision:	https://reviews.freebsd.org/D19764

Modified:
  stable/12/sys/net/if.c
  stable/12/sys/net/if_var.h

Modified: stable/12/sys/net/if.c
==============================================================================
--- stable/12/sys/net/if.c	Mon Apr  1 12:14:45 2019	(r345763)
+++ stable/12/sys/net/if.c	Mon Apr  1 14:19:09 2019	(r345764)
@@ -62,6 +62,8 @@
 #include <sys/domain.h>
 #include <sys/jail.h>
 #include <sys/priv.h>
+#include <sys/sched.h>
+#include <sys/smp.h>
 
 #include <machine/stdarg.h>
 #include <vm/uma.h>
@@ -1754,6 +1756,30 @@ if_data_copy(struct ifnet *ifp, struct if_data *ifd)
 	ifd->ifi_noproto = ifp->if_get_counter(ifp, IFCOUNTER_NOPROTO);
 }
 
+struct ifnet_read_lock {
+	struct mtx mtx;	/* lock protecting tracker below */
+	struct epoch_tracker et;
+};
+
+DPCPU_DEFINE_STATIC(struct ifnet_read_lock, ifnet_addr_read_lock);
+DPCPU_DEFINE_STATIC(struct ifnet_read_lock, ifnet_maddr_read_lock);
+
+static void
+ifnet_read_lock_init(void __unused *arg)
+{
+	struct ifnet_read_lock *pifrl;
+	int cpu;
+
+	CPU_FOREACH(cpu) {
+		pifrl = DPCPU_ID_PTR(cpu, ifnet_addr_read_lock);
+		mtx_init(&pifrl->mtx, "ifnet_addr_read_lock", NULL, MTX_DEF);
+
+		pifrl = DPCPU_ID_PTR(cpu, ifnet_maddr_read_lock);
+		mtx_init(&pifrl->mtx, "ifnet_maddr_read_lock", NULL, MTX_DEF);
+	}
+}
+SYSINIT(ifnet_read_lock_init, SI_SUB_CPU + 1, SI_ORDER_FIRST, &ifnet_read_lock_init, NULL);
+
 /*
  * Wrapper functions for struct ifnet address list locking macros.  These are
  * used by kernel modules to avoid encoding programming interface or binary
@@ -1763,35 +1789,47 @@ if_data_copy(struct ifnet *ifp, struct if_data *ifd)
 void
 if_addr_rlock(struct ifnet *ifp)
 {
-	MPASS(*(uint64_t *)&ifp->if_addr_et == 0);
-	epoch_enter_preempt(net_epoch_preempt, &ifp->if_addr_et);
+	struct ifnet_read_lock *pifrl;
+
+	sched_pin();
+	pifrl = DPCPU_PTR(ifnet_addr_read_lock);
+	mtx_lock(&pifrl->mtx);
+	epoch_enter_preempt(net_epoch_preempt, &pifrl->et);
 }
 
 void
 if_addr_runlock(struct ifnet *ifp)
 {
-	epoch_exit_preempt(net_epoch_preempt, &ifp->if_addr_et);
-#ifdef INVARIANTS
-	bzero(&ifp->if_addr_et, sizeof(struct epoch_tracker));
-#endif
+	struct ifnet_read_lock *pifrl;
+
+	pifrl = DPCPU_PTR(ifnet_addr_read_lock);
+
+	epoch_exit_preempt(net_epoch_preempt, &pifrl->et);
+	mtx_unlock(&pifrl->mtx);
+	sched_unpin();
 }
 
 void
 if_maddr_rlock(if_t ifp)
 {
+	struct ifnet_read_lock *pifrl;
 
-	MPASS(*(uint64_t *)&ifp->if_maddr_et == 0);
-	epoch_enter_preempt(net_epoch_preempt, &ifp->if_maddr_et);
+	sched_pin();
+	pifrl = DPCPU_PTR(ifnet_maddr_read_lock);
+	mtx_lock(&pifrl->mtx);
+	epoch_enter_preempt(net_epoch_preempt, &pifrl->et);
 }
 
 void
 if_maddr_runlock(if_t ifp)
 {
+	struct ifnet_read_lock *pifrl;
 
-	epoch_exit_preempt(net_epoch_preempt, &ifp->if_maddr_et);
-#ifdef INVARIANTS
-	bzero(&ifp->if_maddr_et, sizeof(struct epoch_tracker));
-#endif
+	pifrl = DPCPU_PTR(ifnet_maddr_read_lock);
+
+	epoch_exit_preempt(net_epoch_preempt, &pifrl->et);
+	mtx_unlock(&pifrl->mtx);
+	sched_unpin();
 }
 
 /*

Modified: stable/12/sys/net/if_var.h
==============================================================================
--- stable/12/sys/net/if_var.h	Mon Apr  1 12:14:45 2019	(r345763)
+++ stable/12/sys/net/if_var.h	Mon Apr  1 14:19:09 2019	(r345764)
@@ -381,8 +381,7 @@ struct ifnet {
 	 */
 	struct netdump_methods *if_netdump_methods;
 	struct epoch_context	if_epoch_ctx;
-	struct epoch_tracker	if_addr_et;
-	struct epoch_tracker	if_maddr_et;
+	void 		       *if_unused[4];
 
 	/*
 	 * Spare fields to be added before branching a stable branch, so

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