Hello, mikeb has just pointed out the patch fell under the desk asking me to resend it.
> henning@ and mikeb@ showed some interest to change handling of once rules to > the same way as PF has it on Solaris. Just to refresh the audience on once > option offered by PF: > > 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. > -- pf.conf(5) > > Currently the once rules are removed by matching packet. Patch makes life for > packets, which match once rules bit easier. Packets instead of removing rule > from ruleset just mark rule as expired and put it to garbage colloector list. > The list is processed by pf_purge_thread(), which just removes and deletes > those expired rules. To get there we need to simplify pf_purge_rule() image, > which currently looks as follows: > > void > pf_purge_rule(struct pf_ruleset *ruleset, struct pf_rule *rule, > struct pf_ruleset *aruleset, struct pf_rule *arule) > > - ruleset is the ruleset, where once rule is being removed from > > - rule is a once rule to remove > > - aruleset holds an anchor rule with once-rule we remove > > - arule an anchor which holds a once rule > > To make pf_purge_rule() suitable for pf_purge_thread() it has to be changed > to: > > void > pf_purge_rule(struct pf_rule *once_rule) > > To get there the ruleset and arule has to be carried by once_rule itself. > Therefore patch adds those members to pf_rule: > struct pf_ruleset *myruleset > struct pf_rule *myarule > SLIST_ENTRY(pf_rule) gcle > (the gcle is garbage colleter list link). > > Patch sets myruleset as soon as rule gets inserted to ruleset in SIOCADDRULE > ioctl. The myarule is set in pf_test_rule(), when once rule is marked as > expired. > > Don't forget to recompile all user-land bits (pfctl, proxies et. al.) when > you'll be testing the patch, since pf_rule structure gets changed. > > regards > sasha > updated version is below. comments? O.K.? --------8<---------------8<---------------8<------------------8<-------- diff -r ca96e396772c src/sbin/pfctl/pfctl_parser.c --- a/src/sbin/pfctl/pfctl_parser.c Mon Aug 29 23:23:38 2016 +0200 +++ b/src/sbin/pfctl/pfctl_parser.c Tue Aug 30 00:41:03 2016 +0200 @@ -701,8 +701,12 @@ 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]) { @@ -1120,6 +1124,9 @@ printf(" "); print_pool(&r->route, 0, 0, r->af, PF_POOL_ROUTE, verbose); } + + if (r->rule_flag & PFRULE_EXPIRED) + printf("[ rule expired ]"); } void diff -r ca96e396772c src/sys/net/pf.c --- a/src/sys/net/pf.c Mon Aug 29 23:23:38 2016 +0200 +++ b/src/sys/net/pf.c Tue Aug 30 00:41:03 2016 +0200 @@ -311,6 +311,9 @@ 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,27 @@ /* 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); + + 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 +1215,7 @@ 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 +3516,10 @@ 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 +3825,15 @@ } #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; + SLIST_INSERT_HEAD(&pf_rule_gcl, a, gcle); + } + + r->rule_flag |= PFRULE_EXPIRED; + SLIST_INSERT_HEAD(&pf_rule_gcl, r, gcle); + } return (action); diff -r ca96e396772c src/sys/net/pf_ioctl.c --- a/src/sys/net/pf_ioctl.c Mon Aug 29 23:23:38 2016 +0200 +++ b/src/sys/net/pf_ioctl.c Tue Aug 30 00:41:03 2016 +0200 @@ -301,12 +301,13 @@ } 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--; @@ -314,16 +315,6 @@ 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 @@ -775,6 +766,9 @@ 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) @@ -1209,6 +1203,7 @@ } TAILQ_INSERT_TAIL(ruleset->rules.inactive.ptr, rule, entries); + rule->ruleset = ruleset; ruleset->rules.inactive.rcount++; break; } diff -r ca96e396772c src/sys/net/pfvar.h --- a/src/sys/net/pfvar.h Mon Aug 29 23:23:38 2016 +0200 +++ b/src/sys/net/pfvar.h Tue Aug 30 00:41:03 2016 +0200 @@ -571,6 +571,9 @@ struct pf_addr addr; u_int16_t port; } divert, divert_packet; + + SLIST_ENTRY(pf_rule) gcle; + struct pf_ruleset *ruleset; }; /* rule flags */ @@ -589,6 +592,7 @@ #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 +1670,7 @@ 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 +1700,7 @@ 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 *,