From: sashan <sashan@openbsd.org>
Date: Wed, 9 Nov 2022 23:00:00 +0000 (+0000)
Subject: simplify expiration of 'once' rules.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a21b78cad07af7560419baae8ed65d4a38a32eac;p=openbsd

simplify expiration of 'once' rules.
let packet to mark 'once' rule as expired. The rule
will be removed by pfctl(8) when rules are updated.

OK kn@
---

diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index 6f39ad72384..07ed75ceaea 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_parser.c,v 1.346 2021/02/01 00:31:04 dlg Exp $ */
+/*	$OpenBSD: pfctl_parser.c,v 1.347 2022/11/09 23:00:00 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1148,6 +1148,9 @@ print_rule(struct pf_rule *r, const char *anchor_call, int opts)
 		printf(" ");
 		print_pool(&r->route, 0, 0, r->af, PF_POOL_ROUTE, verbose);
 	}
+
+	if (r->rule_flag & PFRULE_EXPIRED)
+		printf(" # expired");
 }
 
 void
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index 3e5a17acb95..ce52d79d927 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: pf.conf.5,v 1.597 2022/07/24 12:22:12 jmc Exp $
+.\"	$OpenBSD: pf.conf.5,v 1.598 2022/11/09 23:00:00 sashan Exp $
 .\"
 .\" Copyright (c) 2002, Daniel Hartmeier
 .\" Copyright (c) 2003 - 2013 Henning Brauer <henning@openbsd.org>
@@ -28,7 +28,7 @@
 .\" ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: July 24 2022 $
+.Dd $Mdocdate: November 9 2022 $
 .Dt PF.CONF 5
 .Os
 .Sh NAME
@@ -661,10 +661,14 @@ When the rate is exceeded, all ICMP is blocked until the rate falls below
 100 per 10 seconds again.
 .Pp
 .It Cm once
-Creates a one shot rule that will remove itself from an active ruleset after
-the first match.
-In case this is the only rule in the anchor, the anchor will be destroyed
-automatically after the rule is matched.
+Creates a one shot rule. The first matching packet marks rule as expired.
+The expired rule is never evaluated then.
+.Xr pfctl 8
+does not report expired rules unless run in verbose mode ('-vv'). In verbose
+mode
+.Xr pfctl 8
+appends  '# expired' to note the once rule which got hit by packet other
+already.
 .Pp
 .It Cm probability Ar number Ns %
 A probability attribute can be attached to a rule,
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 2c13c99baac..b0147c1974b 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.1145 2022/11/08 16:20:26 sashan Exp $ */
+/*	$OpenBSD: pf.c,v 1.1146 2022/11/09 23:00:00 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -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)
 {
@@ -1482,23 +1479,6 @@ pf_state_import(const struct pfsync_state *sp, int flags)
 
 /* 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)
 {
@@ -1526,10 +1506,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();
 
 	/*
@@ -3844,8 +3822,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),
@@ -3970,6 +3951,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) {
@@ -4181,13 +4175,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)
@@ -4258,22 +4245,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 4ab367fcc28..e3ea4177e6a 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.389 2022/11/07 16:35:12 dlg Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.390 2022/11/09 23:00:00 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -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 cc6c9f3dedc..ced0e95b607 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.514 2022/11/07 16:35:12 dlg Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.515 2022/11/09 23:00:00 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -599,8 +599,6 @@ struct pf_rule {
 		u_int8_t		type;
 	}			divert;
 
-	SLIST_ENTRY(pf_rule)	 gcle;
-	struct pf_ruleset	*ruleset;
 	time_t			 exptime;
 };