Defer ip_output of pfsync updates to an independent callout thread instead

of just dropping the lock around the ip_output call.  This used to cause
corrupted state tree walks for some call-paths.

In a second stage all callouts will be marked MPSAFE according to the
setting of mpsafenet.

Reported and tested by:	Matthew Grooms <mgrooms at seton dot org>
MFC after:		3 days
X-MFC after:		Marking callouts MPSAFE + 1 week

2 files changed, 30 insertions, 7 deletions

diff --git a/sys/contrib/pf/net/if_pfsync.c b/sys/contrib/pf/net/if_pfsync.c
index 746a6ea..315342c 100644
--- a/sys/contrib/pf/net/if_pfsync.c
+++ b/sys/contrib/pf/net/if_pfsync.c
@@ -132,6 +132,7 @@ struct pfsyncstats	pfsyncstats;
 
 static void	pfsync_clone_destroy(struct ifnet *);
 static int	pfsync_clone_create(struct if_clone *, int);
+static void	pfsync_senddef(void *);
 #else
 void	pfsyncattach(int);
 #endif
@@ -174,6 +175,8 @@ pfsync_clone_destroy(struct ifnet *ifp)
 	callout_stop(&sc->sc_bulk_tmo);
 	callout_stop(&sc->sc_bulkfail_tmo);
 
+	callout_stop(&sc->sc_send_tmo);
+
 #if NBPFILTER > 0
         bpfdetach(ifp);
 #endif
@@ -225,6 +228,9 @@ pfsync_clone_create(struct if_clone *ifc, int unit)
 	callout_init(&sc->sc_tmo, 0);
 	callout_init(&sc->sc_bulk_tmo, 0);
 	callout_init(&sc->sc_bulkfail_tmo, 0);
+	callout_init(&sc->sc_send_tmo, 0);
+	mtx_init(&sc->sc_ifq.ifq_mtx, ifp->if_xname, "pfsync send queue",
+	    MTX_DEF);
 	if_attach(ifp);
 
 	LIST_INSERT_HEAD(&pfsync_list, sc, sc_next);
@@ -1038,6 +1044,7 @@ pfsyncioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 		if (pfsyncr.pfsyncr_maxupdates > 255)
 			return (EINVAL);
 #ifdef __FreeBSD__
+		callout_drain(&sc->sc_send_tmo);
 		PF_LOCK();
 #endif
 		sc->sc_maxupdates = pfsyncr.pfsyncr_maxupdates;
@@ -1794,15 +1801,13 @@ pfsync_sendout(sc)
 #endif
 
 		pfsyncstats.pfsyncs_opackets++;
-
 #ifdef __FreeBSD__
-		PF_UNLOCK();
-#endif
+		if (IF_HANDOFF(&sc->sc_ifq, m, NULL))
+			pfsyncstats.pfsyncs_oerrors++;
+		callout_reset(&sc->sc_send_tmo, 1, pfsync_senddef, sc);
+#else
 		if (ip_output(m, NULL, NULL, IP_RAWOUTPUT, &sc->sc_imo, NULL))
 			pfsyncstats.pfsyncs_oerrors++;
-
-#ifdef __FreeBSD__
-		PF_LOCK();
 #endif
 	} else
 		m_freem(m);
@@ -1810,8 +1815,22 @@ pfsync_sendout(sc)
 	return (0);
 }
 
-
 #ifdef __FreeBSD__
+static void
+pfsync_senddef(void *arg)
+{
+	struct pfsync_softc *sc = (struct pfsync_softc *)arg;
+	struct mbuf *m;
+
+	for(;;) {
+		IF_DEQUEUE(&sc->sc_ifq, m);
+		if (m == NULL)
+			break;
+		if (ip_output(m, NULL, NULL, IP_RAWOUTPUT, &sc->sc_imo, NULL))
+			pfsyncstats.pfsyncs_oerrors++;
+	}
+}
+
 static int
 pfsync_modevent(module_t mod, int type, void *data)
 {
diff --git a/sys/contrib/pf/net/if_pfsync.h b/sys/contrib/pf/net/if_pfsync.h
index 4a52195..f9df354 100644
--- a/sys/contrib/pf/net/if_pfsync.h
+++ b/sys/contrib/pf/net/if_pfsync.h
@@ -168,6 +168,10 @@ struct pfsync_softc {
 	struct in_addr		 sc_sendaddr;
 	struct mbuf		*sc_mbuf;	/* current cumulative mbuf */
 	struct mbuf		*sc_mbuf_net;	/* current cumulative mbuf */
+#ifdef __FreeBSD__
+	struct ifqueue		 sc_ifq;
+	struct callout		 sc_send_tmo;
+#endif
 	union sc_statep		 sc_statep;
 	union sc_statep		 sc_statep_net;
 	u_int32_t		 sc_ureq_received;