On Mon, Jan 04, 2021 at 11:21:50PM +1000, David Gwynne wrote: > this chunk pops out as a standalone change. > > having pf_find_state() return PF_PASS here means the callers short > circuit and let the packet go through without running it through the > a lot of the state handling, which includes things like protocol state > updates, nat, scrubbing, some pflog handling, and most importantly, > later calls to pf_route().
pf_route() calls pf_test() again with a different interface. The idea of this code is, that the interface which is passed to pf_test() from ip_output() is wrong. The call to pf_set_rt_ifp() changes it in the state. In the pf_test() call from ip_output() we skip the tests. We know they will happen in pf_test() called from pf_route(). Without this chunk we would do state handling twice with different interfaces. Is that analysis correct? bluhm > Index: pf.c > =================================================================== > RCS file: /cvs/src/sys/net/pf.c,v > retrieving revision 1.1097 > diff -u -p -r1.1097 pf.c > --- pf.c 4 Jan 2021 12:48:27 -0000 1.1097 > +++ pf.c 4 Jan 2021 13:08:26 -0000 > @@ -1122,12 +1122,6 @@ pf_find_state(struct pf_pdesc *pd, struc > } > > *state = s; > - if (pd->dir == PF_OUT && s->rt_kif != NULL && s->rt_kif != pd->kif && > - ((s->rule.ptr->rt == PF_ROUTETO && > - s->rule.ptr->direction == PF_OUT) || > - (s->rule.ptr->rt == PF_REPLYTO && > - s->rule.ptr->direction == PF_IN))) > - return (PF_PASS); > > return (PF_MATCH); > }