[pfil loop prevention experiment 5/5] pf: Simplify af-to, route-to and dummynet reinjection

[vegeta_tuxpowered.net]

Summary

Split pf_route() into pf_route_to(), pf_route_tag_to(), pf_route_tag_afto(), and pf_forward[46](). Make dummynet use pf_forward[46]() when packets are tagged for forwarding. Make pf_send_tcp() / pf_intr() use pf_forward[46]().

What we gain:

• split the logic of route-to and af-to from actually sending the packets out, pf_route_to() becomes a very small function with a clear role, the code should be easier to understand and review
• syncookies work with route-to
• synproxy works with route-to
• reinjection from dummynet skips already taken pf_test()
• dummynet pipes can be used separately on inbound and outbound path
• sending to dummynet occurs only in one place in pf.c and works for normal routing, route-to and af-to

[vegeta_tuxpowered.net]

sys/netpfil/pf/pf.c

673

Or maybe we could even further deduplicate the code by sending the mbuf to pf_translate_compat()

9166

I'm wondering if pf_send_icmp() had an option to not really send the packet but just give us the mbuf back, could we just send it back through pf_test() to un-NAT its contents?

9348

For better performance we should probably stick to using pointer to ifp and only covert it to if_index when sending packet to dummynet, but this code is a proof of concept, this can be addressed later.

9376

To be honest I have not checked how this works currently, are duplicated packets sent to dummynet?

11117

I've also tried another way by using netisr_dispatch() to pretend that there is post-af-to packet inbound on the original inbound interface (pfil hooks would still be skipped) but that won't work: if there is no IP address of new address family on the interface, the af-translated packet was dropped.

[kp]

First impressions are that this makes sense, and removes a lot of 'route-to' special casing from code that ideally shouldn't have to know about route-to.

We do need to get feedback from others (mostly Gleb) about the related netpfil changes.

sys/netpfil/pf/pf.c

9067

Why are we only looking for the mtag in the INVARIANTS case?
We later assert that we have one, so that looks unintentional.

9348

ifnet_byindexgen() is basically just a table lookup, so I wouldn't really worry about performance for that.

9376

I don't think the interaction between dummy net and dup-to is particularly well defined right now, so "whatever makes sense" seems like the best option to take here.
I'm inclined to agree that the intended use case for dup-to is to be port-mirror-like, and in that case skipping dummy net makes sense to me.