Hi,
this removes the pf_consistency_lock and protects the users with NET_LOCK()
I ran into a crash when using pfctl -k, and mpi suggested this change (and
likes the diff).
pf hackers: ok?
diff --git sys/net/pf.c sys/net/pf.c
index 2ae434e405d..a8ade533484 100644
--- sys/net/pf.c
+++ sys/net/pf.c
@@ -1154,26 +1154,20 @@ pf_state_export(struct pfsync_state *sp, struct
pf_state *st)
/* END state table stuff */
void
-pf_purge_expired_rules(int locked)
+pf_purge_expired_rules(void)
{
struct pf_rule *r;
+ NET_ASSERT_LOCKED();
+
if (SLIST_EMPTY(&pf_rule_gcl))
return;
- if (!locked)
- rw_enter_write(&pf_consistency_lock);
- else
- rw_assert_wrlock(&pf_consistency_lock);
-
while ((r = SLIST_FIRST(&pf_rule_gcl)) != NULL) {
SLIST_REMOVE(&pf_rule_gcl, r, pf_rule, gcle);
KASSERT(r->rule_flag & PFRULE_EXPIRED);
pf_purge_rule(r);
}
-
- if (!locked)
- rw_exit_write(&pf_consistency_lock);
}
void
@@ -1194,7 +1188,7 @@ pf_purge_thread(void *v)
if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
pf_purge_expired_fragments();
pf_purge_expired_src_nodes(0);
- pf_purge_expired_rules(0);
+ pf_purge_expired_rules();
nloops = 0;
}
@@ -1241,27 +1235,21 @@ pf_state_expires(const struct pf_state *state)
}
void
-pf_purge_expired_src_nodes(int waslocked)
+pf_purge_expired_src_nodes(void)
{
struct pf_src_node *cur, *next;
- int locked = waslocked;
+
+ NET_ASSERT_LOCKED();
for (cur = RB_MIN(pf_src_tree, &tree_src_tracking); cur; cur = next) {
next = RB_NEXT(pf_src_tree, &tree_src_tracking, cur);
if (cur->states == 0 && cur->expire <= time_uptime) {
- if (! locked) {
- rw_enter_write(&pf_consistency_lock);
- next = RB_NEXT(pf_src_tree,
- &tree_src_tracking, cur);
- locked = 1;
- }
+ next = RB_NEXT(pf_src_tree,
+ &tree_src_tracking, cur);
pf_remove_src_node(cur);
}
}
-
- if (locked && !waslocked)
- rw_exit_write(&pf_consistency_lock);
}
void
@@ -1334,13 +1322,12 @@ pf_remove_divert_state(struct pf_state_key *sk)
}
}
-/* callers should hold the write_lock on pf_consistency_lock */
void
pf_free_state(struct pf_state *cur)
{
struct pf_rule_item *ri;
- splsoftassert(IPL_SOFTNET);
+ NET_ASSERT_LOCKED();
#if NPFSYNC > 0
if (pfsync_state_in_use(cur))
@@ -1375,7 +1362,8 @@ pf_purge_expired_states(u_int32_t maxcheck)
{
static struct pf_state *cur = NULL;
struct pf_state *next;
- int locked = 0;
+
+ NET_ASSERT_LOCKED();
while (maxcheck--) {
/* wrap to start of list when we hit the end */
@@ -1390,25 +1378,14 @@ pf_purge_expired_states(u_int32_t maxcheck)
if (cur->timeout == PFTM_UNLINKED) {
/* free removed state */
- if (! locked) {
- rw_enter_write(&pf_consistency_lock);
- locked = 1;
- }
pf_free_state(cur);
} else if (pf_state_expires(cur) <= time_uptime) {
/* remove and free expired state */
pf_remove_state(cur);
- if (! locked) {
- rw_enter_write(&pf_consistency_lock);
- locked = 1;
- }
pf_free_state(cur);
}
cur = next;
}
-
- if (locked)
- rw_exit_write(&pf_consistency_lock);
}
int
diff --git sys/net/pf_ioctl.c sys/net/pf_ioctl.c
index 9b278c907f5..444425c8bd1 100644
--- sys/net/pf_ioctl.c
+++ sys/net/pf_ioctl.c
@@ -111,7 +111,6 @@ void pf_qid2qname(u_int16_t, char
*);
void pf_qid_unref(u_int16_t);
struct pf_rule pf_default_rule, pf_default_rule_new;
-struct rwlock pf_consistency_lock = RWLOCK_INITIALIZER("pfcnslk");
struct {
char statusif[IFNAMSIZ];
@@ -987,12 +986,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
return (EACCES);
}
- if (flags & FWRITE)
- rw_enter_write(&pf_consistency_lock);
- else
- rw_enter_read(&pf_consistency_lock);
-
- s = splsoftnet();
+ NET_LOCK(s);
switch (cmd) {
case DIOCSTART:
@@ -2388,11 +2382,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
break;
}
fail:
- splx(s);
- if (flags & FWRITE)
- rw_exit_write(&pf_consistency_lock);
- else
- rw_exit_read(&pf_consistency_lock);
+ NET_UNLOCK(s);
return (error);
}
diff --git sys/net/pf_norm.c sys/net/pf_norm.c
index 6bf9681e37f..06c8643b5e4 100644
--- sys/net/pf_norm.c
+++ sys/net/pf_norm.c
@@ -176,6 +176,8 @@ pf_purge_expired_fragments(void)
struct pf_fragment *frag;
int32_t expire;
+ NET_ASSERT_LOCKED();
+
expire = time_uptime - pf_default_rule.timeout[PFTM_FRAG];
while ((frag = TAILQ_LAST(&pf_fragqueue, pf_fragqueue)) != NULL) {
if (frag->fr_timeout > expire)
diff --git sys/net/pfvar.h sys/net/pfvar.h
index 9896bf82eca..85ce5074657 100644
--- sys/net/pfvar.h
+++ sys/net/pfvar.h
@@ -1614,9 +1614,9 @@ extern void
pf_tbladdr_remove(struct pf_addr_wrap *);
extern void pf_tbladdr_copyout(struct pf_addr_wrap *);
extern void pf_calc_skip_steps(struct pf_rulequeue *);
extern void pf_purge_thread(void *);
-extern void pf_purge_expired_src_nodes(int);
+extern void pf_purge_expired_src_nodes();
extern void pf_purge_expired_states(u_int32_t);
-extern void pf_purge_expired_rules(int);
+extern void pf_purge_expired_rules();
extern void pf_remove_state(struct pf_state *);
extern void pf_remove_divert_state(struct pf_state_key *);
extern void pf_free_state(struct pf_state *);
@@ -1790,7 +1790,6 @@ int pf_addr_compare(struct pf_addr *,
struct pf_addr *,
extern struct pf_status pf_status;
extern struct pool pf_frent_pl, pf_frag_pl;
-extern struct rwlock pf_consistency_lock;
struct pf_pool_limit {
void *pp;
