summaryrefslogtreecommitdiff
path: root/sys
diff options
context:
space:
mode:
authorDavid Gwynne <dlg@cvs.openbsd.org>2021-06-15 05:06:25 +0000
committerDavid Gwynne <dlg@cvs.openbsd.org>2021-06-15 05:06:25 +0000
commit70188becb3a93b844f40fb2d95e062781a608104 (patch)
treeefffed7fe23e4c30add1796f87689e1b73f4df73 /sys
parent09e3644893539f125a313b98a8a8ee7179eed18b (diff)
rework pfsync deferal timeout handling.
instead of having a timeout per deferred packet structure, use a single timeout in pfsync that pulls items off the list of deferred packets. this avoids confusion about whether a timeout is handling the defer or another context owns it. this way round, the context that removes a defer from the list owns it and is responsible for completing it. this should fix a panic we hit on the firewalls at work. there's still another one that needs a fix, but sashan@ has been looking at it. this might make it simpler to deal with though. ok sashan@ jmatthew@
Diffstat (limited to 'sys')
-rw-r--r--sys/net/if_pfsync.c83
1 files changed, 60 insertions, 23 deletions
diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index d82f31a8878..4f062f1c02e 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: if_pfsync.c,v 1.289 2021/06/02 21:49:31 sashan Exp $ */
+/* $OpenBSD: if_pfsync.c,v 1.290 2021/06/15 05:06:24 dlg Exp $ */
/*
* Copyright (c) 2002 Michael Shalayeff
@@ -187,7 +187,7 @@ struct pfsync_deferral {
TAILQ_ENTRY(pfsync_deferral) pd_entry;
struct pf_state *pd_st;
struct mbuf *pd_m;
- struct timeout pd_tmo;
+ struct timeval pd_deadline;
};
TAILQ_HEAD(pfsync_deferrals, pfsync_deferral);
@@ -223,6 +223,7 @@ struct pfsync_softc {
struct pfsync_deferrals sc_deferrals;
u_int sc_deferred;
struct mutex sc_deferrals_mtx;
+ struct timeout sc_deferrals_tmo;
void *sc_plus;
size_t sc_pluslen;
@@ -273,7 +274,7 @@ void pfsync_ifdetach(void *);
void pfsync_deferred(struct pf_state *, int);
void pfsync_undefer(struct pfsync_deferral *, int);
-void pfsync_defer_tmo(void *);
+void pfsync_deferrals_tmo(void *);
void pfsync_cancel_full_update(struct pfsync_softc *);
void pfsync_request_full_update(struct pfsync_softc *);
@@ -346,6 +347,7 @@ pfsync_clone_create(struct if_clone *ifc, int unit)
mtx_init(&sc->sc_upd_req_mtx, IPL_SOFTNET);
TAILQ_INIT(&sc->sc_deferrals);
mtx_init(&sc->sc_deferrals_mtx, IPL_SOFTNET);
+ timeout_set_proc(&sc->sc_deferrals_tmo, pfsync_deferrals_tmo, sc);
task_set(&sc->sc_ltask, pfsync_syncdev_state, sc);
task_set(&sc->sc_dtask, pfsync_ifdetach, sc);
sc->sc_deferred = 0;
@@ -1931,6 +1933,9 @@ pfsync_defer(struct pf_state *st, struct mbuf *m)
{
struct pfsync_softc *sc = pfsyncif;
struct pfsync_deferral *pd;
+ struct timeval now;
+ unsigned int sched;
+ static const struct timeval defer = { 0, 20000 };
NET_ASSERT_LOCKED();
@@ -1942,10 +1947,12 @@ pfsync_defer(struct pf_state *st, struct mbuf *m)
if (sc->sc_deferred >= 128) {
mtx_enter(&sc->sc_deferrals_mtx);
pd = TAILQ_FIRST(&sc->sc_deferrals);
- TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
- sc->sc_deferred--;
+ if (pd != NULL) {
+ TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
+ sc->sc_deferred--;
+ }
mtx_leave(&sc->sc_deferrals_mtx);
- if (timeout_del(&pd->pd_tmo))
+ if (pd != NULL)
pfsync_undefer(pd, 0);
}
@@ -1959,13 +1966,18 @@ pfsync_defer(struct pf_state *st, struct mbuf *m)
pd->pd_st = pf_state_ref(st);
pd->pd_m = m;
+ getmicrouptime(&now);
+ timeradd(&now, &defer, &pd->pd_deadline);
+
mtx_enter(&sc->sc_deferrals_mtx);
- sc->sc_deferred++;
+ sched = TAILQ_EMPTY(&sc->sc_deferrals);
+
TAILQ_INSERT_TAIL(&sc->sc_deferrals, pd, pd_entry);
+ sc->sc_deferred++;
mtx_leave(&sc->sc_deferrals_mtx);
- timeout_set_proc(&pd->pd_tmo, pfsync_defer_tmo, pd);
- timeout_add_msec(&pd->pd_tmo, 20);
+ if (sched)
+ timeout_add_tv(&sc->sc_deferrals_tmo, &defer);
schednetisr(NETISR_PFSYNC);
@@ -2047,17 +2059,44 @@ pfsync_undefer(struct pfsync_deferral *pd, int drop)
}
void
-pfsync_defer_tmo(void *arg)
+pfsync_deferrals_tmo(void *arg)
{
- struct pfsync_softc *sc = pfsyncif;
- struct pfsync_deferral *pd = arg;
+ struct pfsync_softc *sc = arg;
+ struct pfsync_deferral *pd;
+ struct timeval now, tv;
+ struct pfsync_deferrals pds = TAILQ_HEAD_INITIALIZER(pds);
mtx_enter(&sc->sc_deferrals_mtx);
- TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
- sc->sc_deferred--;
+ for (;;) {
+ pd = TAILQ_FIRST(&sc->sc_deferrals);
+ if (pd == NULL)
+ break;
+
+ if (timercmp(&now, &pd->pd_deadline, <)) {
+ timersub(&now, &pd->pd_deadline, &tv);
+ break;
+ }
+
+ TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
+ sc->sc_deferred--;
+ TAILQ_INSERT_TAIL(&pds, pd, pd_entry);
+ }
mtx_leave(&sc->sc_deferrals_mtx);
+
+ if (pd != NULL) {
+ /* we were looking at a pd, but it wasn't old enough */
+ timeout_add_tv(&sc->sc_deferrals_tmo, &tv);
+ }
+
+ if (TAILQ_EMPTY(&pds))
+ return;
+
NET_LOCK();
- pfsync_undefer(pd, 0);
+ while ((pd = TAILQ_FIRST(&pds)) != NULL) {
+ TAILQ_REMOVE(&pds, pd, pd_entry);
+
+ pfsync_undefer(pd, 0);
+ }
NET_UNLOCK();
}
@@ -2072,17 +2111,15 @@ pfsync_deferred(struct pf_state *st, int drop)
mtx_enter(&sc->sc_deferrals_mtx);
TAILQ_FOREACH(pd, &sc->sc_deferrals, pd_entry) {
if (pd->pd_st == st) {
- if (timeout_del(&pd->pd_tmo)) {
- TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
- sc->sc_deferred--;
- mtx_leave(&sc->sc_deferrals_mtx);
- pfsync_undefer(pd, drop);
- } else
- mtx_leave(&sc->sc_deferrals_mtx);
- return;
+ TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
+ sc->sc_deferred--;
+ break;
}
}
mtx_leave(&sc->sc_deferrals_mtx);
+
+ if (pd != NULL)
+ pfsync_undefer(pd, drop);
}
void