git: daea703963f5 - main - pf: Use pf_map_addr() only once when choosing source port and address

From: Kajetan Staszkiewicz <ks_at_FreeBSD.org>
Date: Thu, 06 Feb 2025 13:19:32 UTC
The branch main has been updated by ks:

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

commit daea703963f53b5530c2957f72bed57b10151810
Author:     Kajetan Staszkiewicz <ks@FreeBSD.org>
AuthorDate: 2025-02-03 16:41:03 +0000
Commit:     Kajetan Staszkiewicz <ks@FreeBSD.org>
CommitDate: 2025-02-06 13:14:33 +0000

    pf: Use pf_map_addr() only once when choosing source port and address
    
    When choosing source port and address for NAT operations the proper order of
    operations is:
    1. Try to get them from udp_mapping if rule has PF_POOL_ENDPI. This
       might be enough to return.
    2. Get IP address from pf_map_addr_sn()
    3. Look for free ports for the IP address
    4. Get another IP address from pf_map_addr() if no ports are free
    
    Calling pf_map_addr_sn() before checking udp_mappings is not necessary,
    remove the first call. Since now a rule can have multiple pools, don't
    hardcode pools anymore, always use the pool given in pf_get_sport() call.
    
    Reviewed by:            kp
    Approved by:            kp (mentor)
    Sponsored by:           InnoGames GmbH
    Differential Revision:  https://reviews.freebsd.org/D48821
---
 sys/netpfil/pf/pf_lb.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c
index cb92c724a541..6251ddee7d19 100644
--- a/sys/netpfil/pf/pf_lb.c
+++ b/sys/netpfil/pf/pf_lb.c
@@ -238,11 +238,6 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 
 	bzero(&init_addr, sizeof(init_addr));
 
-	if (! TAILQ_EMPTY(&r->nat.list) &&
-	    pf_map_addr_sn(pd->naf, r, &pd->nsaddr, naddr, NULL, &init_addr,
-	    sn, sh, &r->nat))
-		return (1);
-
 	if (udp_mapping) {
 		MPASS(*udp_mapping == NULL);
 	}
@@ -252,7 +247,7 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 	 * from the mapping. In this case we have to look up the src_node as
 	 * pf_map_addr would.
 	 */
-	if (pd->proto == IPPROTO_UDP && (r->rdr.opts & PF_POOL_ENDPI)) {
+	if (pd->proto == IPPROTO_UDP && (rpool->opts & PF_POOL_ENDPI)) {
 		struct pf_udp_endpoint_cmp udp_source;
 
 		bzero(&udp_source, sizeof(udp_source));
@@ -265,8 +260,8 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 				PF_ACPY(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 && r->rdr.opts & PF_POOL_STICKYADDR &&
-				    (r->rdr.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
+				if (*sn == NULL && rpool->opts & PF_POOL_STICKYADDR &&
+				    (rpool->opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
 					*sn = pf_find_src_node(&pd->nsaddr, r, pd->af, sh, false);
 				if (*sn != NULL)
 					PF_SRC_NODE_UNLOCK(*sn);
@@ -280,7 +275,7 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 		}
 	}
 
-	if (pf_map_addr_sn(pd->af, r, &pd->nsaddr, naddr, NULL, &init_addr,
+	if (pf_map_addr_sn(pd->naf, r, &pd->nsaddr, naddr, NULL, &init_addr,
 	    sn, sh, rpool))
 		goto failed;
 
@@ -379,7 +374,7 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 			tmp = cut;
 			for (tmp -= 1; tmp >= low && tmp <= 0xffff; --tmp) {
 				if (pd->proto == IPPROTO_UDP &&
-				    (r->rdr.opts & PF_POOL_ENDPI &&
+				    (rpool->opts & PF_POOL_ENDPI &&
 				    udp_mapping != NULL)) {
 					(*udp_mapping)->endpoints[1].port = htons(tmp);
 					if (pf_udp_mapping_insert(*udp_mapping) == 0) {
@@ -396,7 +391,7 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 			}
 		}
 
-		switch (r->rdr.opts & PF_POOL_TYPEMASK) {
+		switch (rpool->opts & PF_POOL_TYPEMASK) {
 		case PF_POOL_RANDOM:
 		case PF_POOL_ROUNDROBIN:
 			/*
@@ -404,8 +399,8 @@ pf_get_sport(struct pf_pdesc *pd, struct pf_krule *r,
 			 * of free port choices for the current one.
 			 */
 			(*sn) = NULL;
-			if (pf_map_addr_sn(pd->af, r, &pd->nsaddr, naddr, NULL,
-			    &init_addr, sn, sh, &r->rdr))
+			if (pf_map_addr_sn(pd->naf, r, &pd->nsaddr, naddr, NULL,
+			    &init_addr, sn, sh, rpool))
 				return (1);
 			break;
 		case PF_POOL_NONE:
@@ -658,8 +653,8 @@ pf_map_addr_sn(sa_family_t af, struct pf_krule *r, struct pf_addr *saddr,
 	 * Request the sh to be unlocked if sn was not found, as we never
 	 * insert a new sn when parsing the ruleset.
 	 */
-	if (r->rdr.opts & PF_POOL_STICKYADDR &&
-	    (r->rdr.opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
+	if (rpool->opts & PF_POOL_STICKYADDR &&
+	    (rpool->opts & PF_POOL_TYPEMASK) != PF_POOL_NONE)
 		*sn = pf_find_src_node(saddr, r, af, sh, false);
 
 	if (*sn != NULL) {