On 18-10-15 13:38:32, Alexandr Nedvedicky wrote:
> Hello,
>
> I just got back on-line after a week. Thank you all for detailed analysis.
> This particular bug, which Klemens tries to fix is caused by my commit 1.1024:
>
> - percpu anchor stacks
> we actually don't need to pre-allocate per_anchor_stack[], if we use
> a 'natural' recursion, when doing anchor tree traversal.
>
> Diff below fixes the case reported by Fabian:
>
> > snap# uname -a
> > OpenBSD snap.my.domain 6.4 GENERIC#333 amd64
> > snap# pfctl -sr
> >
> > pass all flags S/SA
> > anchor quick all {
> > }
> > block drop all
> >
>
> The idea is to override 'anchor quick' for empty rulesets. If the particular
> anchor, which pf_step_into_anchor() is about to enter, is empty, then the
> pf_step_into_anchor() bails out immediately with PF_TEST_OK result to keep
> PF processing the rules. I think this was the missing piece in mosaic.
That is still different from the original (2006) behaviour which
would terminate ruleset-evaluation IFF any rules inside the anchor
actually match.
>
> OK ?
>
> sorry for inconveniences
> regards
> sashan
>
> --------8<---------------8<---------------8<------------------8<--------
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index 0bdf90a8d13..72841c9b8f0 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -3126,7 +3126,7 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct
> pf_rule *r)
> break;
> }
> }
> - } else {
> + } else if (!TAILQ_EMPTY(r->anchor->ruleset.rules.active.ptr)) {
> rv = pf_match_rule(ctx, &r->anchor->ruleset);
> /*
> * Unless there was an error inside the anchor,
> @@ -3134,7 +3134,8 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct
> pf_rule *r)
> */
> if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK)
> rv = PF_TEST_QUICK;
> - }
> + } else
> + rv = PF_TEST_OK;
>
> ctx->depth--;
>
