Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 23 Jul 2019 12:52:36 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r350240 - head/sys/netpfil/ipfw
Message-ID:  <201907231252.x6NCqaS9018882@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: ae
Date: Tue Jul 23 12:52:36 2019
New Revision: 350240
URL: https://svnweb.freebsd.org/changeset/base/350240

Log:
  Eliminate rmlock from ipfw's BPF code.
  
  After r343631 pfil hooks are invoked in net_epoch_preempt section,
  this allows to avoid extra locking. Add NET_EPOCH_ASSER() assertion
  to each ipfw_bpf_*tap*() call to require to be called from inside
  epoch section.
  
  Use NET_EPOCH_WAIT() in ipfw_clone_destroy() to wait until it becomes
  safe to free() ifnet. And use on-stack ifnet pointer in each
  ipfw_bpf_*tap*() call to avoid NULL pointer dereference in case when
  V_*log_if global variable will become NULL during ipfw_bpf_*tap*() call.
  
  Sponsored by:	Yandex LLC

Modified:
  head/sys/netpfil/ipfw/ip_fw_bpf.c

Modified: head/sys/netpfil/ipfw/ip_fw_bpf.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw_bpf.c	Tue Jul 23 09:39:27 2019	(r350239)
+++ head/sys/netpfil/ipfw/ip_fw_bpf.c	Tue Jul 23 12:52:36 2019	(r350240)
@@ -32,7 +32,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/mbuf.h>
 #include <sys/kernel.h>
 #include <sys/lock.h>
-#include <sys/rmlock.h>
 #include <sys/socket.h>
 #include <net/ethernet.h>
 #include <net/if.h>
@@ -57,15 +56,6 @@ VNET_DEFINE_STATIC(struct if_clone *, ipfwlog_cloner);
 #define	V_log_if		VNET(log_if)
 #define	V_pflog_if		VNET(pflog_if)
 
-static struct rmlock log_if_lock;
-#define	LOGIF_LOCK_INIT(x)	rm_init(&log_if_lock, "ipfw log_if lock")
-#define	LOGIF_LOCK_DESTROY(x)	rm_destroy(&log_if_lock)
-#define	LOGIF_RLOCK_TRACKER	struct rm_priotracker _log_tracker
-#define	LOGIF_RLOCK(x)		rm_rlock(&log_if_lock, &_log_tracker)
-#define	LOGIF_RUNLOCK(x)	rm_runlock(&log_if_lock, &_log_tracker)
-#define	LOGIF_WLOCK(x)		rm_wlock(&log_if_lock)
-#define	LOGIF_WUNLOCK(x)	rm_wunlock(&log_if_lock)
-
 static const char ipfwname[] = "ipfw";
 static const char ipfwlogname[] = "ipfwlog";
 
@@ -90,13 +80,12 @@ static void
 ipfw_clone_destroy(struct ifnet *ifp)
 {
 
-	LOGIF_WLOCK();
 	if (ifp->if_hdrlen == ETHER_HDR_LEN)
 		V_log_if = NULL;
 	else
 		V_pflog_if = NULL;
-	LOGIF_WUNLOCK();
 
+	NET_EPOCH_WAIT();
 	bpfdetach(ifp);
 	if_detach(ifp);
 	if_free(ifp);
@@ -118,16 +107,13 @@ ipfw_clone_create(struct if_clone *ifc, int unit, cadd
 	ifp->if_hdrlen = ETHER_HDR_LEN;
 	if_attach(ifp);
 	bpfattach(ifp, DLT_EN10MB, ETHER_HDR_LEN);
-	LOGIF_WLOCK();
 	if (V_log_if != NULL) {
-		LOGIF_WUNLOCK();
 		bpfdetach(ifp);
 		if_detach(ifp);
 		if_free(ifp);
 		return (EEXIST);
 	}
 	V_log_if = ifp;
-	LOGIF_WUNLOCK();
 	return (0);
 }
 
@@ -147,48 +133,42 @@ ipfwlog_clone_create(struct if_clone *ifc, int unit, c
 	ifp->if_hdrlen = PFLOG_HDRLEN;
 	if_attach(ifp);
 	bpfattach(ifp, DLT_PFLOG, PFLOG_HDRLEN);
-	LOGIF_WLOCK();
 	if (V_pflog_if != NULL) {
-		LOGIF_WUNLOCK();
 		bpfdetach(ifp);
 		if_detach(ifp);
 		if_free(ifp);
 		return (EEXIST);
 	}
 	V_pflog_if = ifp;
-	LOGIF_WUNLOCK();
 	return (0);
 }
 
 void
 ipfw_bpf_tap(u_char *pkt, u_int pktlen)
 {
-	LOGIF_RLOCK_TRACKER;
+	struct ifnet *ifp = V_log_if;
 
-	LOGIF_RLOCK();
-	if (V_log_if != NULL)
-		BPF_TAP(V_log_if, pkt, pktlen);
-	LOGIF_RUNLOCK();
+	NET_EPOCH_ASSERT();
+	if (ifp != NULL)
+		BPF_TAP(ifp, pkt, pktlen);
 }
 
 void
 ipfw_bpf_mtap(struct mbuf *m)
 {
-	LOGIF_RLOCK_TRACKER;
+	struct ifnet *ifp = V_log_if;
 
-	LOGIF_RLOCK();
-	if (V_log_if != NULL)
-		BPF_MTAP(V_log_if, m);
-	LOGIF_RUNLOCK();
+	NET_EPOCH_ASSERT();
+	if (ifp != NULL)
+		BPF_MTAP(ifp, m);
 }
 
 void
 ipfw_bpf_mtap2(void *data, u_int dlen, struct mbuf *m)
 {
 	struct ifnet *logif;
-	LOGIF_RLOCK_TRACKER;
 
-	LOGIF_RLOCK();
+	NET_EPOCH_ASSERT();
 	switch (dlen) {
 	case (ETHER_HDR_LEN):
 		logif = V_log_if;
@@ -205,19 +185,14 @@ ipfw_bpf_mtap2(void *data, u_int dlen, struct mbuf *m)
 
 	if (logif != NULL)
 		BPF_MTAP2(logif, data, dlen, m);
-
-	LOGIF_RUNLOCK();
 }
 
 void
-ipfw_bpf_init(int first)
+ipfw_bpf_init(int first __unused)
 {
 
-	if (first) {
-		LOGIF_LOCK_INIT();
-		V_log_if = NULL;
-		V_pflog_if = NULL;
-	}
+	V_log_if = NULL;
+	V_pflog_if = NULL;
 	V_ipfw_cloner = if_clone_simple(ipfwname, ipfw_clone_create,
 	    ipfw_clone_destroy, 0);
 	V_ipfwlog_cloner = if_clone_simple(ipfwlogname, ipfwlog_clone_create,
@@ -225,12 +200,10 @@ ipfw_bpf_init(int first)
 }
 
 void
-ipfw_bpf_uninit(int last)
+ipfw_bpf_uninit(int last __unused)
 {
 
 	if_clone_detach(V_ipfw_cloner);
 	if_clone_detach(V_ipfwlog_cloner);
-	if (last)
-		LOGIF_LOCK_DESTROY();
 }

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