On Wed, Nov 27, 2019 at 08:04:47PM +0100, Klemens Nanni wrote:
> If an anchor/ruleset contains no rules, there is no point in creating
> a temporary copy, optimizing and replacing it.
>
> Regress passes on amd64.
>
> Feedback? OK?
Anyone?
All optimizations work on actual rules; if there are none, we don't
need to look further, especially not in "profile" mode where existing
rules are read from the kernel as feedback: an empty ruleset will stay
empty after optimization is done.
This also does not affect `set' or `table' lines in any way, e.g.
# echo 'table <t>' | pfctl -o basic -d -nf-
still is an empty ruleset.
I came across when debugging anchors, but with -DOPT_DEBUG as well this
time where `-d' output for multiple anchors wouldn't really be helpful:
$ pfctl -dnf test.pf
pfctl_optimize_ruleset: optimizing ruleset
pfctl_optimize_ruleset: optimizing ruleset
pfctl_optimize_ruleset: optimizing ruleset
So below is an updated diff that also prints the anchor path, letting
developers know which anchor is being optimized in wha order:
pfctl_optimize_ruleset: optimizing ruleset ""
pfctl_optimize_ruleset: optimizing ruleset "a1"
pfctl_optimize_ruleset: optimizing ruleset "_1/a2"
Yes, the main anchor prints as "" but all that is behind compile time
-DOPT_DEBUG so regular users won't deal with it anyway, so keep the code
simple instead of adding logging around `rs->anchor->path'.
OK?
Index: pfctl_optimize.c
===================================================================
RCS file: /cvs/src/sbin/pfctl/pfctl_optimize.c,v
retrieving revision 1.42
diff -u -p -r1.42 pfctl_optimize.c
--- pfctl_optimize.c 28 Jun 2019 13:32:45 -0000 1.42
+++ pfctl_optimize.c 12 Dec 2019 20:06:15 -0000
@@ -270,7 +270,10 @@ pfctl_optimize_ruleset(struct pfctl *pf,
struct pf_rule *r;
struct pf_rulequeue *old_rules;
- DEBUG("optimizing ruleset");
+ if (TAILQ_EMPTY(rs->rules.active.ptr))
+ return (0);
+
+ DEBUG("optimizing ruleset \"%s\"", rs->anchor->path);
memset(&table_buffer, 0, sizeof(table_buffer));
skip_init();
TAILQ_INIT(&opt_queue);
