Date: Tue, 10 Sep 2024 14:59:30 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 339a1977c324 - main - pf: Add a sysctl to limit work done for rdr source port rewriting Message-ID: <202409101459.48AExUiI080539@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=339a1977c32414f3d23733504955245ca6f3802d commit 339a1977c32414f3d23733504955245ca6f3802d Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2024-09-10 14:33:47 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2024-09-10 14:58:19 +0000 pf: Add a sysctl to limit work done for rdr source port rewriting It was pointed out that the current approach of exhaustively searching for a free source port might be very time consuming. Limit the amount of work that we might do before giving up. Reviewed by: kp Reported by: Eirik Øverby <ltning-freebsd@anduin.net> MFC after: 3 months Sponsored by: Klara, Inc. Sponsored by: Modirum Differential Revision: https://reviews.freebsd.org/D46495 --- share/man/man4/pf.4 | 7 ++++++- share/man/man5/pf.conf.5 | 6 +++++- sys/netpfil/pf/pf_lb.c | 18 ++++++++++++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/share/man/man4/pf.4 b/share/man/man4/pf.4 index 3855d07faead..92e1a6fdf87b 100644 --- a/share/man/man4/pf.4 +++ b/share/man/man4/pf.4 @@ -102,8 +102,13 @@ This tells .Nm to also filter on the loopback output hook. This is typically used to allow redirect rules to adjust the source address. -.It net.pf.request_maxcount +.It Va net.pf.request_maxcount The maximum number of items in a single ioctl call. +.It Va net.pf.rdr_srcport_rewrite_tries +The maximum number of times to try and find a free source port when handling +redirects. +Such rules are typically applied to external traffic, so an exhaustive search +may be too expensive. .El .Pp Read only diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5 index 5aa936d509ed..733b00cc6732 100644 --- a/share/man/man5/pf.conf.5 +++ b/share/man/man5/pf.conf.5 @@ -1407,7 +1407,11 @@ A .Ar rdr rule may cause the source port to be modified if doing so avoids a conflict with an existing connection. -A random source port in the range 50001-65535 is chosen in this case. +A random source port in the range 50001-65535 is chosen in this case; to +avoid excessive CPU consumption, the number of searches for a free port is +limited by the +.Va net.pf.rdr_srcport_rewrite_tries +sysctl. Port numbers are never translated with a .Ar binat rule. diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c index cdd68aaf5dab..dbd85d530bb7 100644 --- a/sys/netpfil/pf/pf_lb.c +++ b/sys/netpfil/pf/pf_lb.c @@ -52,6 +52,13 @@ #include <net/pfvar.h> #include <net/if_pflog.h> +/* + * Limit the amount of work we do to find a free source port for redirects that + * introduce a state conflict. + */ +#define V_pf_rdr_srcport_rewrite_tries VNET(pf_rdr_srcport_rewrite_tries) +VNET_DEFINE_STATIC(int, pf_rdr_srcport_rewrite_tries) = 16; + #define DPFPRINTF(n, x) if (V_pf_status.debug >= (n)) printf x static void pf_hash(struct pf_addr *, struct pf_addr *, @@ -822,6 +829,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, break; case PF_RDR: { struct pf_state_key_cmp key; + int tries; uint16_t cut, low, high, nport; reason = pf_map_addr(pd->af, r, saddr, naddr, NULL, NULL, sn); @@ -873,11 +881,15 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, if (!pf_find_state_all_exists(&key, PF_OUT)) break; + tries = 0; + low = 50001; /* XXX-MJ PF_NAT_PROXY_PORT_LOW/HIGH */ high = 65535; cut = arc4random() % (1 + high - low) + low; for (uint32_t tmp = cut; - tmp <= high && tmp <= UINT16_MAX; tmp++) { + tmp <= high && tmp <= UINT16_MAX && + tries < V_pf_rdr_srcport_rewrite_tries; + tmp++, tries++) { key.port[0] = htons(tmp); if (!pf_find_state_all_exists(&key, PF_OUT)) { /* Update the source port. */ @@ -885,7 +897,9 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, goto out; } } - for (uint32_t tmp = cut - 1; tmp >= low; tmp--) { + for (uint32_t tmp = cut - 1; + tmp >= low && tries < V_pf_rdr_srcport_rewrite_tries; + tmp--, tries++) { key.port[0] = htons(tmp); if (!pf_find_state_all_exists(&key, PF_OUT)) { /* Update the source port. */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202409101459.48AExUiI080539>