On Mon, Jan 04, 2021 at 06:37:54PM +0100, Alexander Bluhm wrote:
> 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?
I think so, but I didn't get as much time to poke at this today as I was
hoping.
If the idea is to avoid running most of pf_test again if route-to is
applied during ip_output, I think this tweaked diff is simpler. Is there
a valid use case for running some of pf_test again after route-to is
applied?
The pf_set_rt_ifp() stuff could be cleaned up if we can get away with
this.
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 5 Jan 2021 11:18:14 -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);
}
@@ -6049,7 +6043,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
if (ifp == NULL)
goto bad;
- if (pd->kif->pfik_ifp != ifp) {
+ if (pd->dir == PF_IN) {
if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
goto bad;
else if (m0 == NULL)
@@ -6204,7 +6198,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
if (ifp == NULL)
goto bad;
- if (pd->kif->pfik_ifp != ifp) {
+ if (pd->dir == PF_IN) {
if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
goto bad;
else if (m0 == NULL)
