From 2f60aac277af6a5bab2625fcd1033448790e60b6 Mon Sep 17 00:00:00 2001 From: mlaier Date: Fri, 10 Jun 2005 17:23:49 +0000 Subject: 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 MFC after: 3 days X-MFC after: Marking callouts MPSAFE + 1 week --- sys/contrib/pf/net/if_pfsync.c | 33 ++++++++++++++++++++++++++------- sys/contrib/pf/net/if_pfsync.h | 4 ++++ 2 files changed, 30 insertions(+), 7 deletions(-) (limited to 'sys/contrib/pf') 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; -- cgit v1.1