On Tue, Jan 05, 2021 at 10:05:39PM +1000, David Gwynne wrote:
> 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?

I found the original commit that introduced this strange check.

----------------------------
revision 1.294
date: 2003/01/02 01:56:56;  author: dhartmei;  state: Exp;  lines: +27 -49;
When route-to/reply-to is used in combination with address translation,
pf_test() may be called twice for the same packet. In this case, make
sure the translation is only applied in the second call. This solves
the problem with state insert failures where the second pf_test() call
tried to insert another state entry after the first call's translation.
ok henning@, mcbride@, thanks to Joe Nall for additional testing.
----------------------------

I have tested your diffs in my setup, they all pass.  I have not
tested the scenario mentioned in the commit message.  Note that the
address translation implementation in 2003 was different from what
we have now.  And sasha@'s analysis shows that the current code is
wrong in other use cases.

The check in pf_find_state() seems to be unrelated to the call to
pf_test() in pf_route().  I have to rethink it separately.

How can we figure out what happens when we remove the check?  It
may harm some cases and benefit others or make no sense at all.  My
regression test, which tests each feature individually, is not
affected.

The only way to find out is to commit it.  It reduces comlexity that
noone understands.

OK bluhm@ to remove the check

Please leave the "if (pd->kif->pfik_ifp != ifp)" around pf_test()
in pf_route() as it is for now.

Reply via email to