Date: Mon, 7 Jul 2025 15:07:53 GMT From: Kristof Provost <kp@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 058422752727 - main - pfctl: Simplify lookups when killing entries Message-ID: <202507071507.567F7rPh016547@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=0584227527271b18fb3164522c0362a5df2f3732 commit 0584227527271b18fb3164522c0362a5df2f3732 Author: Kristof Provost <kp@FreeBSD.org> AuthorDate: 2025-07-02 09:41:58 +0000 Commit: Kristof Provost <kp@FreeBSD.org> CommitDate: 2025-07-07 15:06:49 +0000 pfctl: Simplify lookups when killing entries Killing source tracking or state entries by hostname or CIDR would pass given keys twice to getaddrinfo(3): once to resolve/parse and again to parse the numerical address in case a prefix was specified. Avoid this overhead by making pfctl_addrprefix() resolve, pass and mask in one go and return the list of IPs to the callers. This notably simplifies both logic and sanity checks around prefix length and address family. While here, also pass -N along such that -k and -K can be restricted to not use DNS. Discussed with procter sashan, OK sashan Obtained from: OpenBSD, kn <kn@openbsd.org>, 4a32e6d5fb Sponsored by: Rubicon Communications, LLC ("Netgate") --- sbin/pfctl/pfctl.c | 86 +++++++++++++++++++++--------------------------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index f133a106c69c..10183084ceec 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -77,7 +77,8 @@ void pfctl_flush_nat(int, int, char *); int pfctl_clear_altq(int, int); void pfctl_clear_src_nodes(int, int); void pfctl_clear_iface_states(int, const char *, int); -void pfctl_addrprefix(char *, struct pf_addr *); +struct addrinfo * + pfctl_addrprefix(char *, struct pf_addr *, int); void pfctl_kill_src_nodes(int, int); void pfctl_net_kill_states(int, const char *, int); void pfctl_gateway_kill_states(int, const char *, int); @@ -539,35 +540,36 @@ pfctl_clear_iface_states(int dev, const char *iface, int opts) fprintf(stderr, "%d states cleared\n", killed); } -void -pfctl_addrprefix(char *addr, struct pf_addr *mask) +struct addrinfo * +pfctl_addrprefix(char *addr, struct pf_addr *mask, int numeric) { char *p; const char *errstr; int prefix, ret_ga, q, r; struct addrinfo hints, *res; - if ((p = strchr(addr, '/')) == NULL) - return; - - *p++ = '\0'; - prefix = strtonum(p, 0, 128, &errstr); - if (errstr) - errx(1, "prefix is %s: %s", errstr, p); - bzero(&hints, sizeof(hints)); - /* prefix only with numeric addresses */ - hints.ai_flags |= AI_NUMERICHOST; + hints.ai_socktype = SOCK_DGRAM; /* dummy */ + if (numeric) + hints.ai_flags = AI_NUMERICHOST; + + if ((p = strchr(addr, '/')) != NULL) { + *p++ = '\0'; + /* prefix only with numeric addresses */ + hints.ai_flags |= AI_NUMERICHOST; + } if ((ret_ga = getaddrinfo(addr, NULL, &hints, &res))) { errx(1, "getaddrinfo: %s", gai_strerror(ret_ga)); /* NOTREACHED */ } - if (res->ai_family == AF_INET && prefix > 32) - errx(1, "prefix too long for AF_INET"); - else if (res->ai_family == AF_INET6 && prefix > 128) - errx(1, "prefix too long for AF_INET6"); + if (p == NULL) + return (res); + + prefix = strtonum(p, 0, res->ai_family == AF_INET6 ? 128 : 32, &errstr); + if (errstr) + errx(1, "prefix is %s: %s", errstr, p); q = prefix >> 3; r = prefix & 7; @@ -586,7 +588,8 @@ pfctl_addrprefix(char *addr, struct pf_addr *mask) (0xff00 >> r) & 0xff; break; } - freeaddrinfo(res); + + return (res); } void @@ -596,7 +599,6 @@ pfctl_kill_src_nodes(int dev, int opts) struct addrinfo *res[2], *resp[2]; struct sockaddr last_src, last_dst; int killed, sources, dests; - int ret_ga; killed = sources = dests = 0; @@ -606,12 +608,9 @@ pfctl_kill_src_nodes(int dev, int opts) memset(&last_src, 0xff, sizeof(last_src)); memset(&last_dst, 0xff, sizeof(last_dst)); - pfctl_addrprefix(src_node_kill[0], &psnk.psnk_src.addr.v.a.mask); + res[0] = pfctl_addrprefix(src_node_kill[0], + &psnk.psnk_src.addr.v.a.mask, (opts & PF_OPT_NODNS)); - if ((ret_ga = getaddrinfo(src_node_kill[0], NULL, NULL, &res[0]))) { - errx(1, "getaddrinfo: %s", gai_strerror(ret_ga)); - /* NOTREACHED */ - } for (resp[0] = res[0]; resp[0]; resp[0] = resp[0]->ai_next) { if (resp[0]->ai_addr == NULL) continue; @@ -638,14 +637,9 @@ pfctl_kill_src_nodes(int dev, int opts) memset(&psnk.psnk_dst.addr.v.a.mask, 0xff, sizeof(psnk.psnk_dst.addr.v.a.mask)); memset(&last_dst, 0xff, sizeof(last_dst)); - pfctl_addrprefix(src_node_kill[1], - &psnk.psnk_dst.addr.v.a.mask); - if ((ret_ga = getaddrinfo(src_node_kill[1], NULL, NULL, - &res[1]))) { - errx(1, "getaddrinfo: %s", - gai_strerror(ret_ga)); - /* NOTREACHED */ - } + res[1] = pfctl_addrprefix(src_node_kill[1], + &psnk.psnk_dst.addr.v.a.mask, + (opts & PF_OPT_NODNS)); for (resp[1] = res[1]; resp[1]; resp[1] = resp[1]->ai_next) { if (resp[1]->ai_addr == NULL) @@ -699,7 +693,7 @@ pfctl_net_kill_states(int dev, const char *iface, int opts) struct sockaddr last_src, last_dst; unsigned int newkilled; int killed, sources, dests; - int ret_ga, ret; + int ret; killed = sources = dests = 0; @@ -718,15 +712,12 @@ pfctl_net_kill_states(int dev, const char *iface, int opts) state_killers = 1; } - pfctl_addrprefix(state_kill[0], &kill.src.addr.v.a.mask); + res[0] = pfctl_addrprefix(state_kill[0], + &kill.src.addr.v.a.mask, (opts & PF_OPT_NODNS)); if (opts & PF_OPT_KILLMATCH) kill.kill_match = true; - if ((ret_ga = getaddrinfo(state_kill[0], NULL, NULL, &res[0]))) { - errx(1, "getaddrinfo: %s", gai_strerror(ret_ga)); - /* NOTREACHED */ - } for (resp[0] = res[0]; resp[0]; resp[0] = resp[0]->ai_next) { if (resp[0]->ai_addr == NULL) continue; @@ -753,14 +744,9 @@ pfctl_net_kill_states(int dev, const char *iface, int opts) memset(&kill.dst.addr.v.a.mask, 0xff, sizeof(kill.dst.addr.v.a.mask)); memset(&last_dst, 0xff, sizeof(last_dst)); - pfctl_addrprefix(state_kill[1], - &kill.dst.addr.v.a.mask); - if ((ret_ga = getaddrinfo(state_kill[1], NULL, NULL, - &res[1]))) { - errx(1, "getaddrinfo: %s", - gai_strerror(ret_ga)); - /* NOTREACHED */ - } + res[1] = pfctl_addrprefix(state_kill[1], + &kill.dst.addr.v.a.mask, + (opts & PF_OPT_NODNS)); for (resp[1] = res[1]; resp[1]; resp[1] = resp[1]->ai_next) { if (resp[1]->ai_addr == NULL) @@ -814,7 +800,6 @@ pfctl_gateway_kill_states(int dev, const char *iface, int opts) struct sockaddr last_src; unsigned int newkilled; int killed = 0; - int ret_ga; if (state_killers != 2 || (strlen(state_kill[1]) == 0)) { warnx("no gateway specified"); @@ -832,12 +817,9 @@ pfctl_gateway_kill_states(int dev, const char *iface, int opts) if (opts & PF_OPT_KILLMATCH) kill.kill_match = true; - pfctl_addrprefix(state_kill[1], &kill.rt_addr.addr.v.a.mask); + res = pfctl_addrprefix(state_kill[1], &kill.rt_addr.addr.v.a.mask, + (opts & PF_OPT_NODNS)); - if ((ret_ga = getaddrinfo(state_kill[1], NULL, NULL, &res))) { - errx(1, "getaddrinfo: %s", gai_strerror(ret_ga)); - /* NOTREACHED */ - } for (resp = res; resp; resp = resp->ai_next) { if (resp->ai_addr == NULL) continue;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202507071507.567F7rPh016547>