Re: pf: once for match rules?
On Tue, Aug 12, 2014 at 18:26 +0200, Mike Belopuhov wrote:
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?
here we go then. OK?
diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index c277b8d..61c2646 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -1488,12 +1488,18 @@ pfrule : action dir logquick interface af
proto fromto
if ($8.marker FOM_SETPRIO) {
r.set_prio[0] = $8.set_prio[0];
r.set_prio[1] = $8.set_prio[1];
r.scrub_flags |= PFSTATE_SETPRIO;
}
- if ($8.marker FOM_ONCE)
+ if ($8.marker FOM_ONCE) {
+ if (r.action == PF_MATCH) {
+ yyerror(can't specify once for
+ match rules);
+ YYERROR;
+ }
r.rule_flag |= PFRULE_ONCE;
+ }
if ($8.marker FOM_AFTO)
r.rule_flag |= PFRULE_AFTO;
r.af = $5;
if ($8.tag)
Re: pf: once for match rules?
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
};
pf: once for match rules?
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?
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 };
