Let purge thread to remove once rules, not packets.

Thanks mikeb@ for idea to add expire time.

OK mpi@, OK mikeb@

5 files changed, 79 insertions, 24 deletions

diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 933ac31d94b..8a7beb31dc5 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl.c,v 1.334 2016/01/14 12:05:51 henning Exp $ */
+/*	$OpenBSD: pfctl.c,v 1.335 2016/09/03 17:11:40 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -701,6 +701,10 @@ pfctl_print_rule_counters(struct pf_rule *rule, int opts)
 
 		printf("  [ queue: qname=%s qid=%u pqname=%s pqid=%u ]\n",
 		    rule->qname, rule->qid, rule->pqname, rule->pqid);
+
+		if (rule->rule_flag & PFRULE_EXPIRED)
+			printf("  [ Expired: %lld secs ago ]\n",
+			    (long long)(time(NULL) - rule->exptime));
 	}
 	if (opts & PF_OPT_VERBOSE) {
 		printf("  [ Evaluations: %-8llu  Packets: %-8llu  "
@@ -848,7 +852,13 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format,
 				INDENT(depth, !(opts & PF_OPT_VERBOSE));
 				printf("}\n");
 			} else {
-				printf("\n");
+				/*
+				 * Do not print newline, when we have not
+				 * printed expired rule.
+				 */
+				if (!(pr.rule.rule_flag & PFRULE_EXPIRED) ||
+				    (opts & (PF_OPT_VERBOSE2|PF_OPT_DEBUG)))
+					printf("\n");
 				pfctl_print_rule_counters(&pr.rule, opts);
 			}
 			break;
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index 31e82ef5a51..743a7908d59 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfctl_parser.c,v 1.307 2016/08/26 06:06:58 guenther Exp $ */
+/*	$OpenBSD: pfctl_parser.c,v 1.308 2016/09/03 17:11:40 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -701,8 +701,12 @@ print_rule(struct pf_rule *r, const char *anchor_call, int opts)
 	int	verbose = opts & (PF_OPT_VERBOSE2 | PF_OPT_DEBUG);
 	char	*p;
 
+	if ((r->rule_flag & PFRULE_EXPIRED) && (!verbose))
+		return;
+
 	if (verbose)
 		printf("@%d ", r->nr);
+
 	if (r->action > PF_MATCH)
 		printf("action(%d)", r->action);
 	else if (anchor_call[0]) {
diff --git a/sys/net/pf.c b/sys/net/pf.c
index b105a420c82..e89b38e245a 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.982 2016/08/20 08:34:30 procter Exp $ */
+/*	$OpenBSD: pf.c,v 1.983 2016/09/03 17:11:40 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -311,6 +311,9 @@ 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)
 {
@@ -1174,6 +1177,29 @@ pf_state_export(struct pfsync_state *sp, struct pf_state *st)
 /* END state table stuff */
 
 void
+pf_purge_expired_rules(int locked)
+{
+	struct pf_rule	*r;
+
+	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
 pf_purge_thread(void *v)
 {
 	int nloops = 0, s;
@@ -1191,6 +1217,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);
 			nloops = 0;
 		}
 
@@ -3491,6 +3518,10 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm,
 	ruleset = &pf_main_ruleset;
 	r = TAILQ_FIRST(pf_main_ruleset.rules.active.ptr);
 	while (r != NULL) {
+		if (r->rule_flag & PFRULE_EXPIRED) {
+			r = TAILQ_NEXT(r, entries);
+			goto nextrule;
+		}
 		r->evaluations++;
 		PF_TEST_ATTRIB((pfi_kif_match(r->kif, pd->kif) == r->ifnot),
 			r->skip[PF_SKIP_IFP].ptr);
@@ -3796,8 +3827,17 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, struct pf_state **sm,
 	}
 #endif	/* NPFSYNC > 0 */
 
-	if (r->rule_flag & PFRULE_ONCE)
-		pf_purge_rule(ruleset, r, aruleset, a);
+	if (r->rule_flag & PFRULE_ONCE) {
+		if ((a != NULL) && TAILQ_EMPTY(a->ruleset->rules.active.ptr)) {
+			a->rule_flag |= PFRULE_EXPIRED;
+			a->exptime = time_second;
+			SLIST_INSERT_HEAD(&pf_rule_gcl, a, gcle);
+		}
+
+		r->rule_flag |= PFRULE_EXPIRED;
+		r->exptime = time_second;
+		SLIST_INSERT_HEAD(&pf_rule_gcl, r, gcle);
+	}
 
 	return (action);
 
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 32359ac56c6..7b360b0db05 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf_ioctl.c,v 1.298 2016/09/02 10:19:49 dlg Exp $ */
+/*	$OpenBSD: pf_ioctl.c,v 1.299 2016/09/03 17:11:40 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -309,12 +309,13 @@ pf_rm_rule(struct pf_rulequeue *rulequeue, struct pf_rule *rule)
 }
 
 void
-pf_purge_rule(struct pf_ruleset *ruleset, struct pf_rule *rule,
-    struct pf_ruleset *aruleset, struct pf_rule *arule)
+pf_purge_rule(struct pf_rule *rule)
 {
 	u_int32_t		 nr = 0;
+	struct pf_ruleset	*ruleset;
 
-	KASSERT(ruleset != NULL && rule != NULL);
+	KASSERT((rule != NULL) && (rule->ruleset != NULL));
+	ruleset = rule->ruleset;
 
 	pf_rm_rule(ruleset->rules.active.ptr, rule);
 	ruleset->rules.active.rcount--;
@@ -322,16 +323,6 @@ pf_purge_rule(struct pf_ruleset *ruleset, struct pf_rule *rule,
 		rule->nr = nr++;
 	ruleset->rules.active.ticket++;
 	pf_calc_skip_steps(ruleset->rules.active.ptr);
-
-	/* remove the parent anchor rule */
-	if (nr == 0 && arule && aruleset) {
-		pf_rm_rule(aruleset->rules.active.ptr, arule);
-		aruleset->rules.active.rcount--;
-		TAILQ_FOREACH(rule, aruleset->rules.active.ptr, entries)
-			rule->nr = nr++;
-		aruleset->rules.active.ticket++;
-		pf_calc_skip_steps(aruleset->rules.active.ptr);
-	}
 }
 
 u_int16_t
@@ -783,6 +774,9 @@ pf_commit_rules(u_int32_t ticket, char *anchor)
 	int			 s, error;
 	u_int32_t		 old_rcount;
 
+	/* Make sure any expired rules get removed from active rules first. */
+	pf_purge_expired_rules(1);
+
 	rs = pf_find_ruleset(anchor);
 	if (rs == NULL || !rs->rules.inactive.open ||
 	    ticket != rs->rules.inactive.ticket)
@@ -1217,6 +1211,7 @@ 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++;
 		break;
 	}
@@ -1273,6 +1268,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
 		pr->rule.rcv_kif = NULL;
 		pr->rule.anchor = NULL;
 		pr->rule.overload_tbl = NULL;
+		bzero(&pr->rule.gcle, sizeof(pr->rule.gcle));
+		pr->rule.ruleset = NULL;
 		if (pf_anchor_copyout(ruleset, rule, pr)) {
 			error = EBUSY;
 			break;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index eba38b0a858..855d39228bc 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.437 2016/09/03 14:34:13 bluhm Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.438 2016/09/03 17:11:40 sashan Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -571,6 +571,10 @@ struct pf_rule {
 		struct pf_addr		addr;
 		u_int16_t		port;
 	}			divert, divert_packet;
+
+	SLIST_ENTRY(pf_rule)	 gcle;
+	struct pf_ruleset	*ruleset;
+	time_t			 exptime;
 };
 
 /* rule flags */
@@ -589,6 +593,7 @@ struct pf_rule {
 #define PFRULE_PFLOW		0x00040000
 #define PFRULE_ONCE		0x00100000	/* one shot rule */
 #define PFRULE_AFTO		0x00200000	/* af-to rule */
+#define	PFRULE_EXPIRED		0x00400000	/* one shot rule hit by pkt */
 
 #define PFSTATE_HIWAT		10000	/* default state table size */
 #define PFSTATE_ADAPT_START	6000	/* default adaptive timeout start */
@@ -1666,6 +1671,7 @@ 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_states(u_int32_t);
+extern void			 pf_purge_expired_rules(int);
 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 *);
@@ -1695,9 +1701,7 @@ extern void			 pf_addrcpy(struct pf_addr *, struct pf_addr *,
 				    sa_family_t);
 void				 pf_rm_rule(struct pf_rulequeue *,
 				    struct pf_rule *);
-void				 pf_purge_rule(struct pf_ruleset *,
-				    struct pf_rule *, struct pf_ruleset *,
-				    struct pf_rule *);
+void				 pf_purge_rule(struct pf_rule *);
 struct pf_divert		*pf_find_divert(struct mbuf *);
 int				 pf_setup_pdesc(struct pf_pdesc *, void *,
 				    sa_family_t, int, struct pfi_kif *,