git: 8e3d252901e8 - main - pf: Split pf_map_addr()

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Mon, 30 Sep 2024 10:57:14 UTC
The branch main has been updated by kp:

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

commit 8e3d252901e8e99cbaa2e09e7219e96c8daaa3fe
Author:     Kajetan Staszkiewicz <vegeta@tuxpowered.net>
AuthorDate: 2024-09-30 09:15:40 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-09-30 09:16:24 +0000

    pf: Split pf_map_addr()
    
    Split pf_map_addr() into 2 functions:
    - pf_map_addr() handles just the address mapping algorithms, it is used
      in pf_route() and pf_route6() in case of stateless route-to, where
      source nodes would never be created
    - pf_map_addr_sn() handles source nodes and calls pf_map_addr() for
      address mapping algorightms, it is used everywhere else, like NAT
      rules, which are always stateful
    
    Reviewed by:    kp
    Differential Revision:  https://reviews.freebsd.org/D46776
---
 sys/net/pfvar.h        |   3 ++
 sys/netpfil/pf/pf.c    |   8 ++--
 sys/netpfil/pf/pf_lb.c | 109 ++++++++++++++++++++++++++++++-------------------
 3 files changed, 72 insertions(+), 48 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 3afbabf30d4d..dfc42c16547f 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2618,6 +2618,9 @@ int			 pf_step_out_of_keth_anchor(struct pf_keth_anchor_stackframe *,
 			    int *);
 
 u_short			 pf_map_addr(u_int8_t, struct pf_krule *,
+			    struct pf_addr *, struct pf_addr *,
+			    struct pfi_kkif **nkif, struct pf_addr *);
+u_short			 pf_map_addr_sn(u_int8_t, struct pf_krule *,
 			    struct pf_addr *, struct pf_addr *,
 			    struct pfi_kkif **nkif, struct pf_addr *,
 			    struct pf_ksrc_node **);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 1634c856f51f..19e0014ce4eb 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -5451,7 +5451,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a,
 
 	if (r->rt) {
 		/* pf_map_addr increases the reason counters */
-		if ((reason = pf_map_addr(pd->af, r, pd->src, &s->rt_addr,
+		if ((reason = pf_map_addr_sn(pd->af, r, pd->src, &s->rt_addr,
 		    &s->rt_kif, NULL, &sn)) != 0)
 			goto csfailed;
 		s->rt = r->rt;
@@ -7801,7 +7801,6 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 	struct pfi_kkif		*nkif = NULL;
 	struct ifnet		*ifp = NULL;
 	struct pf_addr		 naddr;
-	struct pf_ksrc_node	*sn = NULL;
 	int			 error = 0;
 	uint16_t		 ip_len, ip_off;
 	uint16_t		 tmp;
@@ -7884,7 +7883,7 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			goto bad_locked;
 		}
 		pf_map_addr(AF_INET, r, (struct pf_addr *)&ip->ip_src,
-		    &naddr, &nkif, NULL, &sn);
+		    &naddr, &nkif, NULL);
 		if (!PF_AZERO(&naddr, AF_INET))
 			dst.sin_addr.s_addr = naddr.v4.s_addr;
 		ifp = nkif ? nkif->pfik_ifp : NULL;
@@ -8053,7 +8052,6 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 	struct pfi_kkif		*nkif = NULL;
 	struct ifnet		*ifp = NULL;
 	struct pf_addr		 naddr;
-	struct pf_ksrc_node	*sn = NULL;
 	int			 r_rt, r_dir;
 
 	KASSERT(m && *m && r && oifp, ("%s: invalid parameters", __func__));
@@ -8133,7 +8131,7 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			goto bad_locked;
 		}
 		pf_map_addr(AF_INET6, r, (struct pf_addr *)&ip6->ip6_src,
-		    &naddr, &nkif, NULL, &sn);
+		    &naddr, &nkif, NULL);
 		if (!PF_AZERO(&naddr, AF_INET6))
 			PF_ACPY((struct pf_addr *)&dst.sin6_addr,
 			    &naddr, AF_INET6);
diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index b322bd65cfd3..007e8606ee8a 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -264,7 +264,7 @@ pf_get_sport(sa_family_t af, u_int8_t proto, struct pf_krule *r,
 		}
 	}
 
-	if (pf_map_addr(af, r, saddr, naddr, NULL, &init_addr, sn))
+	if (pf_map_addr_sn(af, r, saddr, naddr, NULL, &init_addr, sn))
 		goto failed;
 
 	if (proto == IPPROTO_ICMP) {
@@ -385,7 +385,7 @@ pf_get_sport(sa_family_t af, u_int8_t proto, struct pf_krule *r,
 			 * pick a different source address since we're out
 			 * of free port choices for the current one.
 			 */
-			if (pf_map_addr(af, r, saddr, naddr, NULL, &init_addr, sn))
+			if (pf_map_addr_sn(af, r, saddr, naddr, NULL, &init_addr, sn))
 				return (1);
 			break;
 		case PF_POOL_NONE:
@@ -448,47 +448,11 @@ pf_get_mape_sport(sa_family_t af, u_int8_t proto, struct pf_krule *r,
 
 u_short
 pf_map_addr(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
-    struct pf_addr *naddr, struct pfi_kkif **nkif, struct pf_addr *init_addr,
-    struct pf_ksrc_node **sn)
+    struct pf_addr *naddr, struct pfi_kkif **nkif, struct pf_addr *init_addr)
 {
 	u_short			 reason = PFRES_MATCH;
 	struct pf_kpool		*rpool = &r->rpool;
 	struct pf_addr		*raddr = NULL, *rmask = NULL;
-	struct pf_srchash	*sh = NULL;
-
-	/* Try to find a src_node if none was given and this
-	   is a sticky-address rule. */
-	if (*sn == NULL && r->rpool.opts & PF_POOL_STICKYADDR &&
-	    (r->rpool.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
-		*sn = pf_find_src_node(saddr, r, af, &sh, false);
-
-	/* If a src_node was found or explicitly given and it has a non-zero
-	   route address, use this address. A zeroed address is found if the
-	   src node was created just a moment ago in pf_create_state and it
-	   needs to be filled in with routing decision calculated here. */
-	if (*sn != NULL && !PF_AZERO(&(*sn)->raddr, af)) {
-		/* If the supplied address is the same as the current one we've
-		 * been asked before, so tell the caller that there's no other
-		 * address to be had. */
-		if (PF_AEQ(naddr, &(*sn)->raddr, af)) {
-			reason = PFRES_MAPFAILED;
-			goto done;
-		}
-
-		PF_ACPY(naddr, &(*sn)->raddr, af);
-		if (nkif)
-			*nkif = (*sn)->rkif;
-		if (V_pf_status.debug >= PF_DEBUG_NOISY) {
-			printf("pf_map_addr: src tracking maps ");
-			pf_print_host(saddr, 0, af);
-			printf(" to ");
-			pf_print_host(naddr, 0, af);
-			if (nkif)
-				printf("@%s", (*nkif)->pfik_name);
-			printf("\n");
-		}
-		goto done;
-	}
 
 	mtx_lock(&rpool->mtx);
 	/* Find the route using chosen algorithm. Store the found route
@@ -646,6 +610,68 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
 	if (nkif)
 		*nkif = rpool->cur->kif;
 
+done_pool_mtx:
+	mtx_unlock(&rpool->mtx);
+
+	if (reason) {
+		counter_u64_add(V_pf_status.counters[reason], 1);
+	}
+
+	return (reason);
+}
+
+u_short
+pf_map_addr_sn(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
+    struct pf_addr *naddr, struct pfi_kkif **nkif, struct pf_addr *init_addr,
+    struct pf_ksrc_node **sn)
+{
+	u_short			 reason = 0;
+	struct pf_kpool		*rpool = &r->rpool;
+	struct pf_srchash	*sh = NULL;
+
+	/* Try to find a src_node if none was given and this
+	   is a sticky-address rule. */
+	if (*sn == NULL && r->rpool.opts & PF_POOL_STICKYADDR &&
+	    (r->rpool.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
+		*sn = pf_find_src_node(saddr, r, af, &sh, false);
+
+	/* If a src_node was found or explicitly given and it has a non-zero
+	   route address, use this address. A zeroed address is found if the
+	   src node was created just a moment ago in pf_create_state and it
+	   needs to be filled in with routing decision calculated here. */
+	if (*sn != NULL && !PF_AZERO(&(*sn)->raddr, af)) {
+		/* If the supplied address is the same as the current one we've
+		 * been asked before, so tell the caller that there's no other
+		 * address to be had. */
+		if (PF_AEQ(naddr, &(*sn)->raddr, af)) {
+			reason = PFRES_MAPFAILED;
+			goto done;
+		}
+
+		PF_ACPY(naddr, &(*sn)->raddr, af);
+		if (nkif)
+			*nkif = (*sn)->rkif;
+		if (V_pf_status.debug >= PF_DEBUG_NOISY) {
+			printf("pf_map_addr: src tracking maps ");
+			pf_print_host(saddr, 0, af);
+			printf(" to ");
+			pf_print_host(naddr, 0, af);
+			if (nkif)
+				printf("@%s", (*nkif)->pfik_name);
+			printf("\n");
+		}
+		goto done;
+	}
+
+	/*
+	 * Source node has not been found. Find a new address and store it
+	 * in variables given by the caller.
+	 */
+	if (pf_map_addr(af, r, saddr, naddr, nkif, init_addr) != 0) {
+		/* pf_map_addr() sets reason counters on its own */
+		goto done;
+	}
+
 	if (*sn != NULL) {
 		PF_ACPY(&(*sn)->raddr, naddr, af);
 		if (nkif)
@@ -661,9 +687,6 @@ pf_map_addr(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
 		printf("\n");
 	}
 
-done_pool_mtx:
-	mtx_unlock(&rpool->mtx);
-
 done:
 	if (reason) {
 		counter_u64_add(V_pf_status.counters[reason], 1);
@@ -841,7 +864,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off,
 		int tries;
 		uint16_t cut, low, high, nport;
 
-		reason = pf_map_addr(pd->af, r, saddr, naddr, NULL, NULL, sn);
+		reason = pf_map_addr_sn(pd->af, r, saddr, naddr, NULL, NULL, sn);
 		if (reason != 0)
 			goto notrans;
 		if ((r->rpool.opts & PF_POOL_TYPEMASK) == PF_POOL_BITMASK)