git: c6f111635790 - main - pf: fix dummynet + route-to

The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=c6f1116357904d3c2e95430e27213e4d0948fc64

commit c6f1116357904d3c2e95430e27213e4d0948fc64
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-03-12 12:29:08 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-03-19 15:29:29 +0000

    pf: fix dummynet + route-to
    
    Ensure that we pick the correct dummynet pipe (i.e. forward vs. reverse
    direction) when applying route-to.
    
    We mark the processing as outbound so that dummynet will re-inject in
    the correct phase of processing after it's done with the packet, but
    that will cause us to pick the wrong pipe number. Reverse them so that
    the incorrect decision ends up picking the correct pipe.
    
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D44366
---
 sys/netpfil/pf/pf.c              | 26 ++++++++++++----
 tests/sys/netpfil/pf/route_to.sh | 65 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 5089b3ea2570..d7536e44623e 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -7240,6 +7240,7 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 	struct pf_ksrc_node	*sn = NULL;
 	int			 error = 0;
 	uint16_t		 ip_len, ip_off;
+	uint16_t		 tmp;
 	int			 r_rt, r_dir;
 
 	KASSERT(m && *m && r && oifp, ("%s: invalid parameters", __func__));
@@ -7381,11 +7382,26 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 		m0->m_pkthdr.csum_flags &= ~CSUM_SCTP;
 	}
 
-	/*
-	 * Make sure dummynet gets the correct direction, in case it needs to
-	 * re-inject later.
-	 */
-	pd->dir = PF_OUT;
+	if (pd->dir == PF_IN) {
+		/*
+		 * Make sure dummynet gets the correct direction, in case it needs to
+		 * re-inject later.
+		 */
+		pd->dir = PF_OUT;
+
+		/*
+		 * The following processing is actually the rest of the inbound processing, even
+		 * though we've marked it as outbound (so we don't look through dummynet) and it
+		 * happens after the outbound processing (pf_test(PF_OUT) above).
+		 * Swap the dummynet pipe numbers, because it's going to come to the wrong
+		 * conclusion about what direction it's processing, and we can't fix it or it
+		 * will re-inject incorrectly. Swapping the pipe numbers means that its incorrect
+		 * decision will pick the right pipe, and everything will mostly work as expected.
+		 */
+		tmp = pd->act.dnrpipe;
+		pd->act.dnrpipe = pd->act.dnpipe;
+		pd->act.dnpipe = tmp;
+	}
 
 	/*
 	 * If small enough for interface, or the interface will take
diff --git a/tests/sys/netpfil/pf/route_to.sh b/tests/sys/netpfil/pf/route_to.sh
index 4df9b790359a..44fe6786e896 100644
--- a/tests/sys/netpfil/pf/route_to.sh
+++ b/tests/sys/netpfil/pf/route_to.sh
@@ -345,7 +345,7 @@ dummynet_body()
 
 	# The ping request will pass, but take 1.2 seconds
 	# So this works:
-	atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+	atf_check -s exit:0 -o ignore ping -c 1 -t 2 192.0.2.1
 	# But this times out:
 	atf_check -s exit:2 -o ignore ping -c 1 -t 1 192.0.2.1
 
@@ -355,7 +355,7 @@ dummynet_body()
 
 	# The ping request will pass, but take 1.2 seconds
 	# So this works:
-	atf_check -s exit:0 -o ignore ping -c 1 192.0.2.1
+	atf_check -s exit:0 -o ignore ping -c 1 -t 2 192.0.2.1
 	# But this times out:
 	atf_check -s exit:2 -o ignore ping -c 1 -t 1 192.0.2.1
 }
@@ -365,6 +365,66 @@ dummynet_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "dummynet_in" "cleanup"
+dummynet_in_head()
+{
+	atf_set descr 'Thest that dummynet works as expected on pass in route-to packets'
+	atf_set require.user root
+}
+
+dummynet_in_body()
+{
+	dummynet_init
+
+	epair_srv=$(vnet_mkepair)
+	epair_gw=$(vnet_mkepair)
+
+	vnet_mkjail srv ${epair_srv}a
+	jexec srv ifconfig ${epair_srv}a 192.0.2.1/24 up
+	jexec srv route add default 192.0.2.2
+
+	vnet_mkjail gw ${epair_srv}b ${epair_gw}a
+	jexec gw ifconfig ${epair_srv}b 192.0.2.2/24 up
+	jexec gw ifconfig ${epair_gw}a 198.51.100.1/24 up
+	jexec gw sysctl net.inet.ip.forwarding=1
+
+	ifconfig ${epair_gw}b 198.51.100.2/24 up
+	route add -net 192.0.2.0/24 198.51.100.1
+
+	# Sanity check
+	atf_check -s exit:0 -o ignore ping -c 1 -t 1 192.0.2.1
+
+	jexec gw dnctl pipe 1 config delay 1200
+	pft_set_rules gw \
+		"pass in route-to (${epair_srv}b 192.0.2.1) to 192.0.2.1 dnpipe 1"
+	jexec gw pfctl -e
+
+	# The ping request will pass, but take 1.2 seconds
+	# So this works:
+	echo "Expect 1.2 s"
+	ping -c 1 192.0.2.1
+	atf_check -s exit:0 -o ignore ping -c 1 -t 2 192.0.2.1
+	# But this times out:
+	atf_check -s exit:2 -o ignore ping -c 1 -t 1 192.0.2.1
+
+	# return path dummynet
+	pft_set_rules gw \
+		"pass in route-to (${epair_srv}b 192.0.2.1) to 192.0.2.1 dnpipe (0, 1)"
+
+	# The ping request will pass, but take 1.2 seconds
+	# So this works:
+	echo "Expect 1.2 s"
+	ping -c 1 192.0.2.1
+	atf_check -s exit:0 -o ignore ping -c 1 -t 2 192.0.2.1
+	# But this times out:
+	atf_check -s exit:2 -o ignore ping -c 1 -t 1 192.0.2.1
+}
+
+dummynet_in_cleanup()
+{
+	pft_cleanup
+}
+
 atf_test_case "ifbound" "cleanup"
 ifbound_head()
 {
@@ -675,6 +735,7 @@ atf_init_test_cases()
 	atf_add_test_case "multiwanlocal"
 	atf_add_test_case "icmp_nat"
 	atf_add_test_case "dummynet"
+	atf_add_test_case "dummynet_in"
 	atf_add_test_case "ifbound"
 	atf_add_test_case "ifbound_v6"
 	atf_add_test_case "ifbound_reply_to"