git: dedb4d3597e5 - main - pf: Don't return src node and hash from pf_map_addr_sn

From: Kajetan Staszkiewicz <ks_at_FreeBSD.org>
Date: Sat, 12 Jul 2025 14:19:41 UTC
The branch main has been updated by ks:

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

commit dedb4d3597e548bf72c380c850b061a8f5bec729
Author:     Kajetan Staszkiewicz <ks@FreeBSD.org>
AuthorDate: 2025-05-08 08:28:22 +0000
Commit:     Kajetan Staszkiewicz <ks@FreeBSD.org>
CommitDate: 2025-07-12 14:15:10 +0000

    pf: Don't return src node and hash from pf_map_addr_sn
    
    The function pf_map_addr_sn() already returns naddr and nkif, there is
    no need to return the source node too, it is redundant.
---
 sys/net/pfvar.h        |  1 -
 sys/netpfil/pf/pf.c    |  4 +--
 sys/netpfil/pf/pf_lb.c | 86 ++++++++++++++++++++++----------------------------
 3 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index f915f6d0f8fa..8ee4d00daaff 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2714,7 +2714,6 @@ u_short			 pf_map_addr(u_int8_t, struct pf_krule *,
 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 **, struct pf_srchash **,
 			    struct pf_kpool *, pf_sn_types_t);
 int			 pf_get_transaddr_af(struct pf_krule *,
 			    struct pf_pdesc *);
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index d5f01e5c4956..41658a29014e 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -5901,8 +5901,6 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 		M_SETFIB(pd->m, pd->act.rtableid);
 
 	if (r->rt) {
-		struct pf_ksrc_node	*sn = NULL;
-		struct pf_srchash	*snh = NULL;
 		/*
 		 * Set act.rt here instead of in pf_rule_to_actions() because
 		 * it is applied only from the last pass rule.
@@ -5910,7 +5908,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm,
 		pd->act.rt = r->rt;
 		/* Don't use REASON_SET, pf_map_addr increases the reason counters */
 		ctx.reason = pf_map_addr_sn(pd->af, r, pd->src, &pd->act.rt_addr,
-		    &pd->act.rt_kif, NULL, &sn, &snh, &(r->route), PF_SN_ROUTE);
+		    &pd->act.rt_kif, NULL, &(r->route), PF_SN_ROUTE);
 		if (ctx.reason != 0)
 			goto cleanup;
 	}
diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index 97e69c9d1986..d4728f61dce8 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -80,7 +80,6 @@ static enum pf_test_status pf_step_into_translation_anchor(int, struct pf_test_c
 			    struct pf_krule *);
 static int		 pf_get_sport(struct pf_pdesc *, struct pf_krule *,
 			    struct pf_addr *, uint16_t *, uint16_t, uint16_t,
-			    struct pf_ksrc_node **, struct pf_srchash **,
 			    struct pf_kpool *, struct pf_udp_mapping **,
 			    pf_sn_types_t);
 static bool		 pf_islinklocal(const sa_family_t, const struct pf_addr *);
@@ -291,10 +290,8 @@ pf_match_translation(int rs_num, struct pf_test_ctx *ctx)
 }
 
 static int
-pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
-    struct pf_addr *naddr, uint16_t *nport, uint16_t low,
-    uint16_t high, struct pf_ksrc_node **sn,
-    struct pf_srchash **sh, struct pf_kpool *rpool,
+pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r, struct pf_addr *naddr,
+    uint16_t *nport, uint16_t low, uint16_t high, struct pf_kpool *rpool,
     struct pf_udp_mapping **udp_mapping, pf_sn_types_t sn_type)
 {
 	struct pf_state_key_cmp	key;
@@ -322,19 +319,24 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 		pf_addrcpy(&udp_source.addr, &pd->nsaddr, pd->af);
 		udp_source.port = pd->nsport;
 		if (udp_mapping) {
+			struct pf_ksrc_node	*sn = NULL;
+			struct pf_srchash	*sh = NULL;
 			*udp_mapping = pf_udp_mapping_find(&udp_source);
 			if (*udp_mapping) {
 				pf_addrcpy(naddr,
 				    &(*udp_mapping)->endpoints[1].addr,
 				    pd->af);
 				*nport = (*udp_mapping)->endpoints[1].port;
-				/* Try to find a src_node as per pf_map_addr(). */
-				if (*sn == NULL && rpool->opts & PF_POOL_STICKYADDR &&
+				/*
+				 * Try to find a src_node as per pf_map_addr().
+				 * XXX: Why? This code seems to do nothing.
+				 */
+				if (rpool->opts & PF_POOL_STICKYADDR &&
 				    (rpool->opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
-					*sn = pf_find_src_node(&pd->nsaddr, r,
-					    pd->af, sh, sn_type, false);
-				if (*sn != NULL)
-					PF_SRC_NODE_UNLOCK(*sn);
+					sn = pf_find_src_node(&pd->nsaddr, r,
+					    pd->af, &sh, sn_type, false);
+				if (sn != NULL)
+					PF_SRC_NODE_UNLOCK(sn);
 				return (0);
 			} else {
 				*udp_mapping = pf_udp_mapping_create(pd->af, &pd->nsaddr,
@@ -346,7 +348,7 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 	}
 
 	if (pf_map_addr_sn(pd->naf, r, &pd->nsaddr, naddr, NULL, &init_addr,
-	    sn, sh, rpool, sn_type))
+	    rpool, sn_type))
 		goto failed;
 
 	if (pd->proto == IPPROTO_ICMP) {
@@ -470,9 +472,8 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 			 * pick a different source address since we're out
 			 * of free port choices for the current one.
 			 */
-			(*sn) = NULL;
 			if (pf_map_addr_sn(pd->naf, r, &pd->nsaddr, naddr, NULL,
-			    &init_addr, sn, sh, rpool, sn_type))
+			    &init_addr, rpool, sn_type))
 				return (1);
 			break;
 		case PF_POOL_NONE:
@@ -503,7 +504,6 @@ pf_islinklocal(const sa_family_t af, const struct pf_addr *addr)
 static int
 pf_get_mape_sport(struct pf_pdesc *pd, struct pf_krule *r,
     struct pf_addr *naddr, uint16_t *nport,
-    struct pf_ksrc_node **sn, struct pf_srchash **sh,
     struct pf_udp_mapping **udp_mapping, struct pf_kpool *rpool)
 {
 	uint16_t psmask, low, highmask;
@@ -523,16 +523,14 @@ pf_get_mape_sport(struct pf_pdesc *pd, struct pf_krule *r,
 
 	for (i = cut; i <= ahigh; i++) {
 		low = (i << ashift) | psmask;
-		if (!pf_get_sport(pd, r,
-		    naddr, nport, low, low | highmask, sn, sh, rpool,
-		    udp_mapping, PF_SN_NAT))
+		if (!pf_get_sport(pd, r, naddr, nport, low, low | highmask,
+		    rpool, udp_mapping, PF_SN_NAT))
 			return (0);
 	}
 	for (i = cut - 1; i > 0; i--) {
 		low = (i << ashift) | psmask;
-		if (!pf_get_sport(pd, r,
-		    naddr, nport, low, low | highmask, sn, sh, rpool,
-		    udp_mapping, PF_SN_NAT))
+		if (!pf_get_sport(pd, r, naddr, nport, low, low | highmask,
+		    rpool, udp_mapping, PF_SN_NAT))
 			return (0);
 	}
 	return (1);
@@ -767,36 +765,33 @@ done_pool_mtx:
 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, struct pf_srchash **sh, struct pf_kpool *rpool,
-    pf_sn_types_t sn_type)
+    struct pf_kpool *rpool, pf_sn_types_t sn_type)
 {
+	struct pf_ksrc_node	*sn = NULL;
+	struct pf_srchash	*sh = NULL;
 	u_short			 reason = 0;
 
-	KASSERT(*sn == NULL, ("*sn not NULL"));
-
 	/*
 	 * If this is a sticky-address rule, try to find an existing src_node.
-	 * Request the sh to be unlocked if sn was not found, as we never
-	 * insert a new sn when parsing the ruleset.
 	 */
 	if (rpool->opts & PF_POOL_STICKYADDR &&
 	    (rpool->opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
-		*sn = pf_find_src_node(saddr, r, af, sh, sn_type, false);
+		sn = pf_find_src_node(saddr, r, af, &sh, sn_type, false);
 
-	if (*sn != NULL) {
-		PF_SRC_NODE_LOCK_ASSERT(*sn);
+	if (sn != NULL) {
+		PF_SRC_NODE_LOCK_ASSERT(sn);
 
 		/* 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)) {
+		if (PF_AEQ(naddr, &(sn->raddr), af)) {
 			reason = PFRES_MAPFAILED;
 			goto done;
 		}
 
-		pf_addrcpy(naddr, &(*sn)->raddr, af);
+		pf_addrcpy(naddr, &(sn->raddr), af);
 		if (nkif)
-			*nkif = (*sn)->rkif;
+			*nkif = sn->rkif;
 		if (V_pf_status.debug >= PF_DEBUG_NOISY) {
 			printf("pf_map_addr: src tracking maps ");
 			pf_print_host(saddr, 0, af);
@@ -828,8 +823,8 @@ pf_map_addr_sn(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
 	}
 
 done:
-	if ((*sn) != NULL)
-		PF_SRC_NODE_UNLOCK(*sn);
+	if (sn != NULL)
+		PF_SRC_NODE_UNLOCK(sn);
 
 	if (reason) {
 		counter_u64_add(V_pf_status.counters[reason], 1);
@@ -883,8 +878,6 @@ pf_get_transaddr(struct pf_test_ctx *ctx, struct pf_krule *r,
 {
 	struct pf_pdesc	*pd = ctx->pd;
 	struct pf_addr	*naddr;
-	struct pf_ksrc_node	*sn = NULL;
-	struct pf_srchash	*sh = NULL;
 	uint16_t	*nportp;
 	uint16_t	 low, high;
 	u_short		 reason;
@@ -912,8 +905,8 @@ pf_get_transaddr(struct pf_test_ctx *ctx, struct pf_krule *r,
 			high = rpool->proxy_port[1];
 		}
 		if (rpool->mape.offset > 0) {
-			if (pf_get_mape_sport(pd, r, naddr, nportp, &sn,
-			    &sh, &ctx->udp_mapping, rpool)) {
+			if (pf_get_mape_sport(pd, r, naddr, nportp,
+			    &ctx->udp_mapping, rpool)) {
 				DPFPRINTF(PF_DEBUG_MISC,
 				    ("pf: MAP-E port allocation (%u/%u/%u)"
 				    " failed\n",
@@ -923,8 +916,8 @@ pf_get_transaddr(struct pf_test_ctx *ctx, struct pf_krule *r,
 				reason = PFRES_MAPFAILED;
 				goto notrans;
 			}
-		} else if (pf_get_sport(pd, r, naddr, nportp, low, high, &sn,
-		    &sh, rpool, &ctx->udp_mapping, PF_SN_NAT)) {
+		} else if (pf_get_sport(pd, r, naddr, nportp, low, high,
+		    rpool, &ctx->udp_mapping, PF_SN_NAT)) {
 			DPFPRINTF(PF_DEBUG_MISC,
 			    ("pf: NAT proxy port allocation (%u-%u) failed\n",
 			    rpool->proxy_port[0], rpool->proxy_port[1]));
@@ -1010,7 +1003,7 @@ pf_get_transaddr(struct pf_test_ctx *ctx, struct pf_krule *r,
 		uint16_t cut, low, high, nport;
 
 		reason = pf_map_addr_sn(pd->af, r, &pd->nsaddr, naddr, NULL,
-		    NULL, &sn, &sh, rpool, PF_SN_NAT);
+		    NULL, rpool, PF_SN_NAT);
 		if (reason != 0)
 			goto notrans;
 		if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_BITMASK)
@@ -1127,8 +1120,6 @@ pf_get_transaddr_af(struct pf_krule *r, struct pf_pdesc *pd)
 	struct pf_addr	 ndaddr, nsaddr, naddr;
 	u_int16_t	 nport = 0;
 	int		 prefixlen = 96;
-	struct pf_srchash	*sh = NULL;
-	struct pf_ksrc_node	*sns = NULL;
 
 	bzero(&nsaddr, sizeof(nsaddr));
 	bzero(&ndaddr, sizeof(ndaddr));
@@ -1147,9 +1138,8 @@ pf_get_transaddr_af(struct pf_krule *r, struct pf_pdesc *pd)
 		panic("pf_get_transaddr_af: no nat pool for source address");
 
 	/* get source address and port */
-	if (pf_get_sport(pd, r, &nsaddr, &nport,
-	    r->nat.proxy_port[0], r->nat.proxy_port[1], &sns, &sh, &r->nat,
-	    NULL, PF_SN_NAT)) {
+	if (pf_get_sport(pd, r, &nsaddr, &nport, r->nat.proxy_port[0],
+	    r->nat.proxy_port[1], &r->nat, NULL, PF_SN_NAT)) {
 		DPFPRINTF(PF_DEBUG_MISC,
 		    ("pf: af-to NAT proxy port allocation (%u-%u) failed",
 		    r->nat.proxy_port[0], r->nat.proxy_port[1]));
@@ -1175,7 +1165,7 @@ pf_get_transaddr_af(struct pf_krule *r, struct pf_pdesc *pd)
 	/* get the destination address and port */
 	if (! TAILQ_EMPTY(&r->rdr.list)) {
 		if (pf_map_addr_sn(pd->naf, r, &nsaddr, &naddr, NULL, NULL,
-		    &sns, NULL, &r->rdr, PF_SN_NAT))
+		    &r->rdr, PF_SN_NAT))
 			return (-1);
 		if (r->rdr.proxy_port[0])
 			pd->ndport = htons(r->rdr.proxy_port[0]);