On Tue, Jul 22, 2014 at 19:03 +0200, Mike Belopuhov wrote:
> Hi,
>
> Before I send a diff for pfctl to disable "once" on "match" rules,
> I've decided to try and see how much work is it to make it actually
> work. Turns out that I need to extend pf_rule_item by 3 pointers
> to track the match rule ruleset, anchor rule and the ruleset it
> belongs to.
>
> Here's what this means in practice. Consider a ruleset:
>
> block drop all
> match out log proto tcp to port 22 once
> anchor "foo" all {
> match out log proto tcp to port 22 once
> anchor "bar" all {
> match out log proto tcp to port 22 once
> pass out quick proto tcp to port 22 once
> }
> }
>
> Once we send a packet to port 22 the ruleset collapses to just:
>
> block drop all
>
> Thoughts?
Henning thinks it's a bit of an overkill. Any other opinions?
>
> diff --git sys/net/pf.c sys/net/pf.c
> index 9f0e2d6..5679a40 100644
> --- sys/net/pf.c
> +++ sys/net/pf.c
> @@ -3279,15 +3279,16 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule
> **rm, struct pf_state **sm,
> PR_NOWAIT)) == NULL) {
> REASON_SET(&reason, PFRES_MEMORY);
> goto cleanup;
> }
> ri->r = r;
> + ri->ar = a;
> + ri->rs = ruleset;
> + ri->ars = aruleset;
> /* order is irrelevant */
> SLIST_INSERT_HEAD(&rules, ri, entry);
> pf_rule_to_actions(r, &act);
> - if (r->rule_flag & PFRULE_AFTO)
> - pd->naf = r->naf;
> if (pf_get_transaddr(r, pd, sns, &nr) == -1) {
> REASON_SET(&reason, PFRES_TRANSLATE);
> goto cleanup;
> }
> if (r->log || act.log & PF_LOG_MATCHES) {
> @@ -3428,10 +3429,12 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule
> **rm, struct pf_state **sm,
> virtual_type, icmp_dir);
> }
> } else {
> while ((ri = SLIST_FIRST(&rules))) {
> SLIST_REMOVE_HEAD(&rules, entry);
> + if (ri->r->rule_flag & PFRULE_ONCE)
> + pf_purge_rule(ri->rs, ri->r, ri->ars, ri->ar);
> pool_put(&pf_rule_item_pl, ri);
> }
> }
>
> /* copy back packet headers if needed */
> @@ -3454,10 +3457,23 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule
> **rm, struct pf_state **sm,
> }
> #endif
>
> if (r->rule_flag & PFRULE_ONCE)
> pf_purge_rule(ruleset, r, aruleset, a);
> + if (*sm) {
> + SLIST_FOREACH(ri, &(*sm)->match_rules, entry) {
> + if (ri->r->rule_flag & PFRULE_ONCE)
> + /*
> + * We can be sure that pf_purge_rule won't
> + * pool_put the rule because when *sm != NULL
> + * STATE_INC_COUNTERS has increased states_cur.
> + * pf_rule_item's and rules will be g/c'ed by
> + * pf_free_state.
> + */
> + pf_purge_rule(ri->rs, ri->r, ri->ars, ri->ar);
> + }
> + }
>
> #if INET && INET6
> if (rewrite && skw->af != sks->af)
> return (PF_AFRT);
> #endif /* INET && INET6 */
> diff --git sys/net/pfvar.h sys/net/pfvar.h
> index a0d94f7..49af7b4 100644
> --- sys/net/pfvar.h
> +++ sys/net/pfvar.h
> @@ -691,10 +691,13 @@ struct pf_threshold {
> };
>
> struct pf_rule_item {
> SLIST_ENTRY(pf_rule_item) entry;
> struct pf_rule *r;
> + struct pf_rule *ar;
> + struct pf_ruleset *rs;
> + struct pf_ruleset *ars;
> };
>
> SLIST_HEAD(pf_rule_slist, pf_rule_item);
>
> enum pf_sn_types { PF_SN_NONE, PF_SN_NAT, PF_SN_RDR, PF_SN_ROUTE, PF_SN_MAX
> };
