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 *,
