I tested the diff and while im not qualified to judge the code, it now behaves
like i would expect.
Thanks for working on this,
Regards.
> Klemens Nanni <k...@openbsd.org> hat am 16. Oktober 2018 um 13:34 geschrieben:
>
>
> On Tue, Oct 16, 2018 at 10:03:01AM +0200, Alexandr Nedvedicky wrote:
> > I also wonder if we should apply same change to wildcard anchors here:
> They need fixing as well, but I have yet to look into it.
>
> > diff --git a/sys/net/pf.c b/sys/net/pf.c
> > index 0bdf90a8d13..5a5c739773a 100644
> > --- a/sys/net/pf.c
> > +++ b/sys/net/pf.c
> > @@ -3129,10 +3129,32 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct
> > pf_rule *r)
> > } else {
> > rv = pf_match_rule(ctx, &r->anchor->ruleset);
> This comment is way too much, imho. How about this:
>
> Unless errors occured, stop iff any rule matched
> within quick anchors.
>
> It's dense but we're inside pf_step_into_anchor() so context is given
> while the condition speaks for itself as well. I'd like to leave
> detailed explanations to the manual.
>
> > - if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK)
> > + if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK &&
> > + *ctx->am == r)
> > rv = PF_TEST_QUICK;
> > }
> That makes sense and works as expected with *all* examples and real use
> cases I could come up with.
>
> Index: net/pf.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.1076
> diff -u -p -r1.1076 pf.c
> --- net/pf.c 4 Oct 2018 20:25:59 -0000 1.1076
> +++ net/pf.c 16 Oct 2018 11:32:10 -0000
> @@ -3129,10 +3129,11 @@ pf_step_into_anchor(struct pf_test_ctx *
> } else {
> rv = pf_match_rule(ctx, &r->anchor->ruleset);
> /*
> - * Unless there was an error inside the anchor,
> - * retain its quick state.
> + * Unless errors occured, stop iff any rule matched
> + * within quick anchors.
> */
> - if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK)
> + if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK &&
> + *ctx->am == r)
> rv = PF_TEST_QUICK;
> }
>
>
