this is part of a high level discussion about when pf runs against a
packet. the options are:

1. pf runs when a packet goes over an interface
or
2. pf runs when a packet enters or leaves the network stack.

for normal packet handling there isn't a difference between these
options. in the routing case a packet comes in on an interface, pf tests
it, then the stack processes it and decides to send it out another
interface, pf tests it again on the way out, the packet goes on the
wire. for packets handled by the local system, a packet comes in on an
interface, pf tests it, the stack processes it locally, something
generates a reply, the stack decides to route that out an interface, pf
tests it on the way out, the reply packet ends up on the wire.

in both situations, you get the same sequence of events if you think
that pf runs when a packet goes over an interface or wether pf runs when
a packet enters or leaves the stack.

however, there is a difference if route-to gets involved. if route-to is
applied on an outbound rule/state, it could change which interface the
packet should be going over.

currently the code implements option 1. this means that if route-to
changes the interface, it reruns pf test for the packet going over the
new interface. i would like to change it to option 2.

the main reason i want to change it is that option 1 creates confusion
for the state table. by default, pf states are floating, meaning that
packets are matched to states regardless of which interface they're
going over. if a packet leaving on em0 is rerouted out em1, both
traversals will end up using the same state, which at best will make the
accounting look weird, or at worst fail some checks in the state and get
dropped.

another reason i want to change this is to make it consistent with
other changes that are made to packet. eg, when nat is applied to
a packet, we don't run pf_test again with the new addresses.

the downside to this change is that the pf_test rerun may have been used
to do things like push a packet out another interface with the first run
through pf, and pick up a broad "nat all packets leaving this interface"
rule on the second one.

however, like most things relating to route-to/reply-to/dup-to, im
pretty sure at this point it's not used a lot, so the impact is minimal.
a lot of changes in this space have already been made, so adding another
simplification is justifiable. if this does remove functionality that
people need, i believe sashan@ has agreed to help me implement route-to
on match rules to give more flexibility and composability of rules.

i've canvassed a few people, and their responses have varied from "i
don't care, route-to is the worst" to "i thought we did option 2
anyway". anyone else want to chime in?

this keeps the behaviour where route-to on a packet coming into the
stack is pushed past it and immediately forwarded to the output
interface. the condition for that is greatly simplified now though.

ok?

Index: pf.c
===================================================================
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.1106
diff -u -p -r1.1106 pf.c
--- pf.c        1 Feb 2021 00:31:05 -0000       1.1106
+++ pf.c        2 Feb 2021 03:44:51 -0000
@@ -6033,7 +6033,7 @@ pf_route(struct pf_pdesc *pd, struct pf_
            (ifp->if_flags & IFF_LOOPBACK) == 0)
                ip->ip_src = ifatoia(rt->rt_ifa)->ia_addr.sin_addr;
 
-       if (s->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
+       if (s->rt != PF_DUPTO && pd->dir == PF_IN) {
                if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
                        goto bad;
                else if (m0 == NULL)
@@ -6178,7 +6178,7 @@ pf_route6(struct pf_pdesc *pd, struct pf
            (ifp->if_flags & IFF_LOOPBACK) == 0)
                ip6->ip6_src = ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
 
-       if (s->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
+       if (s->rt != PF_DUPTO && pd->dir == PF_IN) {
                if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
                        goto bad;
                else if (m0 == NULL)


Reply via email to