git: 6d7f0162bd5d - main - pf: take the rules read lock in pf_handle_getrule()

From: R. Christian McDonald <rcm_at_FreeBSD.org>
Date: Wed, 30 Sep 2026 23:35:42 UTC
The branch main has been updated by rcm:

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

commit 6d7f0162bd5d25919cc18119a6e5a371acb2cf78
Author:     R. Christian McDonald <rcm@FreeBSD.org>
AuthorDate: 2026-09-30 23:33:43 +0000
Commit:     R. Christian McDonald <rcm@FreeBSD.org>
CommitDate: 2026-09-30 23:33:43 +0000

    pf: take the rules read lock in pf_handle_getrule()
    
    pfctl -sr calls PFNL_CMD_GETRULE once per rule, and
    pf_handle_getrule() takes the rules write lock each time, so listing a
    ruleset of N rules stops packet processing N times. Only zeroing the
    counters (pfctl -z) needs the write lock. Take the read lock
    otherwise, as DIOCGETRULENV does.
    
    Reviewed by:            kp
    Approved by:            kp (mentor)
    Fixes:                  777a4702c591 ("pf: implement addrule via netlink")
    MFC after:              1 week
    Sponsored by:           Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D60161
---
 sys/netpfil/pf/pf_nl.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/sys/netpfil/pf/pf_nl.c b/sys/netpfil/pf/pf_nl.c
index 8d06798065ad..ad060ea3230a 100644
--- a/sys/netpfil/pf/pf_nl.c
+++ b/sys/netpfil/pf/pf_nl.c
@@ -1044,6 +1044,17 @@ pf_handle_getrule(struct nlmsghdr *hdr, struct nl_pstate *npt)
 	struct pf_krule			*rule;
 	int				 rs_num;
 	int				 error;
+	PF_RULES_RLOCK_TRACKER;
+
+/* The write lock to clear counters, the read lock to only read them. */
+#define	PF_GETRULE_LOCKOP(op) do {	\
+	if (attrs.clear)		\
+		PF_RULES_W##op();	\
+	else				\
+		PF_RULES_R##op();	\
+} while (0)
+#define	PF_GETRULE_LOCK()	PF_GETRULE_LOCKOP(LOCK)
+#define	PF_GETRULE_UNLOCK()	PF_GETRULE_LOCKOP(UNLOCK)
 
 	error = nl_parse_nlmsg(hdr, &getrule_parser, npt, &attrs);
 	if (error != 0)
@@ -1055,23 +1066,23 @@ pf_handle_getrule(struct nlmsghdr *hdr, struct nl_pstate *npt)
 	ghdr_new = nlmsg_reserve_object(nw, struct genlmsghdr);
 	ghdr_new->cmd = PFNL_CMD_GETRULE;
 
-	PF_RULES_WLOCK();
+	PF_GETRULE_LOCK();
 	ruleset = pf_find_kruleset(attrs.anchor);
 	if (ruleset == NULL) {
-		PF_RULES_WUNLOCK();
+		PF_GETRULE_UNLOCK();
 		error = ENOENT;
 		goto out;
 	}
 
 	rs_num = pf_get_ruleset_number(attrs.action);
 	if (rs_num >= PF_RULESET_MAX) {
-		PF_RULES_WUNLOCK();
+		PF_GETRULE_UNLOCK();
 		error = EINVAL;
 		goto out;
 	}
 
 	if (attrs.ticket != ruleset->rules[rs_num].active.ticket) {
-		PF_RULES_WUNLOCK();
+		PF_GETRULE_UNLOCK();
 		error = EBUSY;
 		goto out;
 	}
@@ -1080,7 +1091,7 @@ pf_handle_getrule(struct nlmsghdr *hdr, struct nl_pstate *npt)
 	while ((rule != NULL) && (rule->nr != attrs.nr))
 		rule = TAILQ_NEXT(rule, entries);
 	if (rule == NULL) {
-		PF_RULES_WUNLOCK();
+		PF_GETRULE_UNLOCK();
 		error = EBUSY;
 		goto out;
 	}
@@ -1095,7 +1106,10 @@ pf_handle_getrule(struct nlmsghdr *hdr, struct nl_pstate *npt)
 	if (attrs.clear)
 		pf_krule_clear_counters(rule);
 
-	PF_RULES_WUNLOCK();
+	PF_GETRULE_UNLOCK();
+#undef PF_GETRULE_LOCKOP
+#undef PF_GETRULE_LOCK
+#undef PF_GETRULE_UNLOCK
 
 	if (!nlmsg_end(nw)) {
 		error = ENOMEM;