Hello,
diff below simplifies handling of 'once' rules in pf(4).
Currently matching packet marks 'once' rule as expired
and puts it to garbage collection list, where the rule
waits to be removed from its ruleset by timer.
diff below simplifies that. matching packet marks
once rule as expired and leaves that rule in ruleset.
The 'expired' flag prevents other packets to match
such rule. The expired rule will be kept in ruleset until
the ruleset itself is either destroyed or updated by DIOCXCOMMIT
operation.
OK ?
thanks and
regards
sashan
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 4c70b08571e..1c06e2f6eeb 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -317,9 +317,6 @@ RB_GENERATE(pf_state_tree, pf_state_key, entry,
pf_state_compare_key);
RB_GENERATE(pf_state_tree_id, pf_state,
entry_id, pf_state_compare_id);
-SLIST_HEAD(pf_rule_gcl, pf_rule) pf_rule_gcl =
- SLIST_HEAD_INITIALIZER(pf_rule_gcl);
-
__inline int
pf_addr_compare(struct pf_addr *a, struct pf_addr *b, sa_family_t af)
{
@@ -1263,23 +1260,6 @@ pf_state_export(struct pfsync_state *sp, struct pf_state
*st)
/* END state table stuff */
-void
-pf_purge_expired_rules(void)
-{
- struct pf_rule *r;
-
- PF_ASSERT_LOCKED();
-
- if (SLIST_EMPTY(&pf_rule_gcl))
- return;
-
- 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);
- }
-}
-
void
pf_purge_timeout(void *unused)
{
@@ -1307,10 +1287,8 @@ pf_purge(void *xnloops)
PF_LOCK();
/* purge other expired types every PFTM_INTERVAL seconds */
- if (++(*nloops) >= pf_default_rule.timeout[PFTM_INTERVAL]) {
+ if (++(*nloops) >= pf_default_rule.timeout[PFTM_INTERVAL])
pf_purge_expired_src_nodes();
- pf_purge_expired_rules();
- }
PF_UNLOCK();
/*
@@ -3625,8 +3603,11 @@ pf_match_rule(struct pf_test_ctx *ctx, struct pf_ruleset
*ruleset)
struct pf_rule *save_a;
struct pf_ruleset *save_aruleset;
+retry:
r = TAILQ_FIRST(ruleset->rules.active.ptr);
while (r != NULL) {
+ PF_TEST_ATTRIB(r->rule_flag & PFRULE_EXPIRED,
+ TAILQ_NEXT(r, entries));
r->evaluations++;
PF_TEST_ATTRIB(
(pfi_kif_match(r->kif, ctx->pd->kif) == r->ifnot),
@@ -3751,6 +3732,19 @@ pf_match_rule(struct pf_test_ctx *ctx, struct pf_ruleset
*ruleset)
if (r->tag)
ctx->tag = r->tag;
if (r->anchor == NULL) {
+
+ if (r->rule_flag & PFRULE_ONCE) {
+ u_int32_t rule_flag;
+
+ rule_flag = r->rule_flag;
+ if (((rule_flag & PFRULE_EXPIRED) == 0) &&
+ atomic_cas_uint(&r->rule_flag, rule_flag,
+ rule_flag | PFRULE_EXPIRED) == rule_flag)
+ r->exptime = gettime();
+ else
+ goto retry;
+ }
+
if (r->action == PF_MATCH) {
if ((ctx->ri = pool_get(&pf_rule_item_pl,
PR_NOWAIT)) == NULL) {
@@ -3962,13 +3956,6 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm,
struct pf_state **sm,
if (r->action == PF_DROP)
goto cleanup;
- /*
- * If an expired "once" rule has not been purged, drop any new matching
- * packets.
- */
- if (r->rule_flag & PFRULE_EXPIRED)
- goto cleanup;
-
pf_tag_packet(pd->m, ctx.tag, ctx.act.rtableid);
if (ctx.act.rtableid >= 0 &&
rtable_l2(ctx.act.rtableid) != pd->rdomain)
@@ -4039,22 +4026,6 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm,
struct pf_state **sm,
m_copyback(pd->m, pd->off, pd->hdrlen, &pd->hdr, M_NOWAIT);
}
- if (r->rule_flag & PFRULE_ONCE) {
- u_int32_t rule_flag;
-
- /*
- * Use atomic_cas() to determine a clear winner, which will
- * insert an expired rule to gcl.
- */
- rule_flag = r->rule_flag;
- if (((rule_flag & PFRULE_EXPIRED) == 0) &&
- atomic_cas_uint(&r->rule_flag, rule_flag,
- rule_flag | PFRULE_EXPIRED) == rule_flag) {
- r->exptime = gettime();
- SLIST_INSERT_HEAD(&pf_rule_gcl, r, gcle);
- }
- }
-
#if NPFSYNC > 0
if (*sm != NULL && !ISSET((*sm)->state_flags, PFSTATE_NOSYNC) &&
pd->dir == PF_OUT && pfsync_up()) {
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index ef4f18e730d..a109b38b0e7 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -348,27 +348,6 @@ pf_rm_rule(struct pf_rulequeue *rulequeue, struct pf_rule
*rule)
pool_put(&pf_rule_pl, rule);
}
-void
-pf_purge_rule(struct pf_rule *rule)
-{
- u_int32_t nr = 0;
- struct pf_ruleset *ruleset;
-
- KASSERT((rule != NULL) && (rule->ruleset != NULL));
- ruleset = rule->ruleset;
-
- pf_rm_rule(ruleset->rules.active.ptr, rule);
- ruleset->rules.active.rcount--;
- TAILQ_FOREACH(rule, ruleset->rules.active.ptr, entries)
- rule->nr = nr++;
- ruleset->rules.active.ticket++;
- pf_calc_skip_steps(ruleset->rules.active.ptr);
- pf_remove_if_empty_ruleset(ruleset);
-
- if (ruleset == &pf_main_ruleset)
- pf_calc_chksum(ruleset);
-}
-
u_int16_t
tagname2tag(struct pf_tags *head, char *tagname, int create)
{
@@ -837,9 +816,6 @@ pf_commit_rules(u_int32_t ticket, char *anchor)
struct pf_rulequeue *old_rules;
u_int32_t old_rcount;
- /* Make sure any expired rules get removed from active rules first. */
- pf_purge_expired_rules();
-
rs = pf_find_ruleset(anchor);
if (rs == NULL || !rs->rules.inactive.open ||
ticket != rs->rules.inactive.ticket)
@@ -1446,7 +1422,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
}
TAILQ_INSERT_TAIL(ruleset->rules.inactive.ptr,
rule, entries);
- rule->ruleset = ruleset;
ruleset->rules.inactive.rcount++;
PF_UNLOCK();
NET_UNLOCK();
@@ -1520,8 +1495,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
pr->rule.anchor = NULL;
pr->rule.overload_tbl = NULL;
pr->rule.pktrate.limit /= PF_THRESHOLD_MULT;
- memset(&pr->rule.gcle, 0, sizeof(pr->rule.gcle));
- pr->rule.ruleset = NULL;
if (pf_anchor_copyout(ruleset, rule, pr)) {
error = EBUSY;
PF_UNLOCK();
@@ -1712,7 +1685,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
ruleset->rules.active.ptr,
oldrule, newrule, entries);
ruleset->rules.active.rcount++;
- newrule->ruleset = ruleset;
}
nr = 0;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 8339863a94a..4b1df23f189 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -599,8 +599,6 @@ struct pf_rule {
u_int8_t type;
} divert;
- SLIST_ENTRY(pf_rule) gcle;
- struct pf_ruleset *ruleset;
time_t exptime;
};
