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