Hello,
sorry to come back after while...
On Tue, Jan 05, 2021 at 10:05:39PM +1000, David Gwynne wrote:
> 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.
I think this should go in. I was trying to test this change, but
I'm unable to create set up which would work on google cloud in the
way I need. I suspect failing to convince underlying vswitch
to carry packets between test hosts for me.
as soon as I start to do something more fancy, packets start to disappear
in underlying fabric...
If I understand the change right we really need to take care of
route-to action for inbound packet 'to convince PF' stack
the packet actually enters firewall on interface desired by
route-to action ordered by rule found by currently executed pf_test().
for outbound case the IP stack will be executing the pf_test().
so yes, I'm OK with this change.
regards
sashan