From nobody Fri Dec 19 14:13:19 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4dXqJH2WtRz6LlLJ for ; Fri, 19 Dec 2025 14:13:19 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4dXqJH1jTKz3LJR for ; Fri, 19 Dec 2025 14:13:19 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1766153599; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Mq3d611jbMDt8b/9DL4ur2Fv1KPqdK99bjSJTQv57Vk=; b=w7W3IBoHPn2JXSquzdiM3mbW/1d28hnE5VFF2lj5HADHB0w7h1q+pw/GnuIPQCFtDwRhfg //09HZ0PLlc0UrBoXkekPJ+ZxujHksyO0w0yM1EGMk1+B4nyCOUjtzgI0wSlNCP7Sqrj3F E5NTpWx0ntlg8+6f0EX9KwsrXDIemXfFc+lLohVXq1pT7l9s4G9vIv9zI7g5+6kyvMGaO4 37tCzKsVTMSGYmMHRohikcvENrI1pmzx0JD8O1I/3W4kuQPnq3TF1Q1m6rz9Tzw545HLIT kBglMYN0IRiWPb6Vpun5nSD7fRq62Oj3FCmaKSa5lcKwNaIJ+O0prf4H5QwoCA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1766153599; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=Mq3d611jbMDt8b/9DL4ur2Fv1KPqdK99bjSJTQv57Vk=; b=ypzqfCKPE1+UY1dhGEAWR0F4vdZoDJOOjbQyQiiG3x19V4IP7SlW3DIolmq8lDvu81LuoZ SmjrqoQeF9prYbvsxyYChm4EAmwIZsScnNdLtN5s0aE6vQEHN8x99n6C8gP4/9ESPxHQrv YA1tvTrlAThk8ybwXYS/E+j0xRENT43Ua2DKd2AEF1nNYCaPgk0s0QuCQuV6RsP1xzro/+ 0oW4zwH+rROE9MTdG38ybPwQ3+8qavsZ4QuJyHjhm4Sa8krM7QDfFhLFhBkA3GHfg+z6L2 X1FwsxATRpGJjll0j82W3+iBan/PwO/DEsmWmq6lERyMjs5L74R5g4n49eLBuQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1766153599; a=rsa-sha256; cv=none; b=ab4Dj77kQvsAOvPzqODCH3hq+vBp7gGoXc8QyriKbmWAjR6rfJzBHqaR4SMLTVOgAtoTYL +Ioz6SvX4BEDHet04vAA5L6TkB9s7j9EczQDpnvDkjagaX+hY6yF0aN+3ECcBftKE1qtYs syZOMpFyhIEhm5wZ263rdKEd8RTPGwEhjpiISugbYjpDevDlRD6Kzq/RxRXQVHXgDZMNg9 MGNR8VJ3GQep3g6qfbVhhRdSKtUnLqWj3fgiKxoyiGuiRVAtmGCrNqMQBlHokncz+FAJa3 lyxNIBtJBCC1IMsLvnAJNwSOX3wlEossRaZpcLnKrxLhGhxQjgFZ+6zg6CrAMA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) by mxrelay.nyi.freebsd.org (Postfix) with ESMTP id 4dXqJH1GXfz1Qh0 for ; Fri, 19 Dec 2025 14:13:19 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from git (uid 1279) (envelope-from git@FreeBSD.org) id 3ca92 by gitrepo.freebsd.org (DragonFly Mail Agent v0.13+ on gitrepo.freebsd.org); Fri, 19 Dec 2025 14:13:19 +0000 To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: ae96ff302f8a - main - pf: Avoid taking the pf rules write lock in a couple of ioctls List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: ae96ff302f8ae50903a96d3a1857f9acf243f3c4 Auto-Submitted: auto-generated Date: Fri, 19 Dec 2025 14:13:19 +0000 Message-Id: <69455d7f.3ca92.3cf68097@gitrepo.freebsd.org> The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=ae96ff302f8ae50903a96d3a1857f9acf243f3c4 commit ae96ff302f8ae50903a96d3a1857f9acf243f3c4 Author: Mark Johnston AuthorDate: 2025-12-19 14:07:26 +0000 Commit: Mark Johnston CommitDate: 2025-12-19 14:07:26 +0000 pf: Avoid taking the pf rules write lock in a couple of ioctls The DIOCGETRULES ioctl handlers has taken the write lock ever since fine-grained locking was merged to pf, but I believe it's unneeded. Use the read lock instead. DIOCGETRULENV takes the write lock as well but I believe this is only required when clearing rule counters. Acquire the read lock if that is not the case. Reviewed by: kp, allanjude MFC after: 2 weeks Sponsored by: OPNsense Sponsored by: Klara, Inc. Differential Revision: https://reviews.freebsd.org/D54292 --- sys/netpfil/pf/pf_ioctl.c | 91 ++++++++++++++++++++++------------------------- sys/netpfil/pf/pf_nv.c | 2 +- sys/netpfil/pf/pf_nv.h | 2 +- 3 files changed, 45 insertions(+), 50 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 9856842c72b2..ca1815984797 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -2087,19 +2087,20 @@ pf_rule_to_krule(const struct pf_rule *rule, struct pf_krule *krule) int pf_ioctl_getrules(struct pfioc_rule *pr) { + PF_RULES_RLOCK_TRACKER; struct pf_kruleset *ruleset; struct pf_krule *tail; int rs_num; - PF_RULES_WLOCK(); + PF_RULES_RLOCK(); ruleset = pf_find_kruleset(pr->anchor); if (ruleset == NULL) { - PF_RULES_WUNLOCK(); + PF_RULES_RUNLOCK(); return (EINVAL); } rs_num = pf_get_ruleset_number(pr->rule.action); if (rs_num >= PF_RULESET_MAX) { - PF_RULES_WUNLOCK(); + PF_RULES_RUNLOCK(); return (EINVAL); } tail = TAILQ_LAST(ruleset->rules[rs_num].active.ptr, @@ -2109,7 +2110,7 @@ pf_ioctl_getrules(struct pfioc_rule *pr) else pr->nr = 0; pr->ticket = ruleset->rules[rs_num].active.ticket; - PF_RULES_WUNLOCK(); + PF_RULES_RUNLOCK(); return (0); } @@ -3692,6 +3693,7 @@ DIOCADDRULENV_error: } case DIOCGETRULENV: { + PF_RULES_RLOCK_TRACKER; struct pfioc_nv *nv = (struct pfioc_nv *)addr; nvlist_t *nvrule = NULL; nvlist_t *nvl = NULL; @@ -3702,6 +3704,13 @@ DIOCADDRULENV_error: bool clear_counter = false; #define ERROUT(x) ERROUT_IOCTL(DIOCGETRULENV_error, x) +#define ERROUT_LOCKED(x) do { \ + if (clear_counter) \ + PF_RULES_WUNLOCK(); \ + else \ + PF_RULES_RUNLOCK(); \ + ERROUT(x); \ +} while (0) if (nv->len > pf_ioctl_maxcount) ERROUT(ENOMEM); @@ -3733,78 +3742,64 @@ DIOCADDRULENV_error: nr = nvlist_get_number(nvl, "nr"); - PF_RULES_WLOCK(); + if (clear_counter) + PF_RULES_WLOCK(); + else + PF_RULES_RLOCK(); ruleset = pf_find_kruleset(nvlist_get_string(nvl, "anchor")); - if (ruleset == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(ENOENT); - } + if (ruleset == NULL) + ERROUT_LOCKED(ENOENT); rs_num = pf_get_ruleset_number(nvlist_get_number(nvl, "ruleset")); - if (rs_num >= PF_RULESET_MAX) { - PF_RULES_WUNLOCK(); - ERROUT(EINVAL); - } + if (rs_num >= PF_RULESET_MAX) + ERROUT_LOCKED(EINVAL); if (nvlist_get_number(nvl, "ticket") != - ruleset->rules[rs_num].active.ticket) { - PF_RULES_WUNLOCK(); - ERROUT(EBUSY); - } + ruleset->rules[rs_num].active.ticket) + ERROUT_LOCKED(EBUSY); - if ((error = nvlist_error(nvl))) { - PF_RULES_WUNLOCK(); - ERROUT(error); - } + if ((error = nvlist_error(nvl))) + ERROUT_LOCKED(error); rule = TAILQ_FIRST(ruleset->rules[rs_num].active.ptr); while ((rule != NULL) && (rule->nr != nr)) rule = TAILQ_NEXT(rule, entries); - if (rule == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(EBUSY); - } + if (rule == NULL) + ERROUT_LOCKED(EBUSY); nvrule = pf_krule_to_nvrule(rule); nvlist_destroy(nvl); nvl = nvlist_create(0); - if (nvl == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(ENOMEM); - } + if (nvl == NULL) + ERROUT_LOCKED(ENOMEM); nvlist_add_number(nvl, "nr", nr); nvlist_add_nvlist(nvl, "rule", nvrule); nvlist_destroy(nvrule); nvrule = NULL; - if (pf_kanchor_nvcopyout(ruleset, rule, nvl)) { - PF_RULES_WUNLOCK(); - ERROUT(EBUSY); - } + if (pf_kanchor_nvcopyout(ruleset, rule, nvl)) + ERROUT_LOCKED(EBUSY); free(nvlpacked, M_NVLIST); nvlpacked = nvlist_pack(nvl, &nv->len); - if (nvlpacked == NULL) { - PF_RULES_WUNLOCK(); - ERROUT(ENOMEM); - } + if (nvlpacked == NULL) + ERROUT_LOCKED(ENOMEM); - if (nv->size == 0) { - PF_RULES_WUNLOCK(); - ERROUT(0); - } - else if (nv->size < nv->len) { - PF_RULES_WUNLOCK(); - ERROUT(ENOSPC); - } + if (nv->size == 0) + ERROUT_LOCKED(0); + else if (nv->size < nv->len) + ERROUT_LOCKED(ENOSPC); - if (clear_counter) + if (clear_counter) { pf_krule_clear_counters(rule); - - PF_RULES_WUNLOCK(); + PF_RULES_WUNLOCK(); + } else { + PF_RULES_RUNLOCK(); + } error = copyout(nvlpacked, nv->data, nv->len); +#undef ERROUT_LOCKED #undef ERROUT DIOCGETRULENV_error: free(nvlpacked, M_NVLIST); diff --git a/sys/netpfil/pf/pf_nv.c b/sys/netpfil/pf/pf_nv.c index 2f484e2dabc6..3e741dd39974 100644 --- a/sys/netpfil/pf/pf_nv.c +++ b/sys/netpfil/pf/pf_nv.c @@ -684,7 +684,7 @@ error: } nvlist_t * -pf_krule_to_nvrule(struct pf_krule *rule) +pf_krule_to_nvrule(const struct pf_krule *rule) { nvlist_t *nvl, *tmp; u_int64_t src_nodes_total = 0; diff --git a/sys/netpfil/pf/pf_nv.h b/sys/netpfil/pf/pf_nv.h index cf9fbf8bcf5b..9e43ff1e642a 100644 --- a/sys/netpfil/pf/pf_nv.h +++ b/sys/netpfil/pf/pf_nv.h @@ -78,7 +78,7 @@ int pf_nvstring(const nvlist_t *, const char *, char *, size_t); int pf_check_rule_addr(const struct pf_rule_addr *); -nvlist_t *pf_krule_to_nvrule(struct pf_krule *); +nvlist_t *pf_krule_to_nvrule(const struct pf_krule *); int pf_nvrule_to_krule(const nvlist_t *, struct pf_krule *); int pf_nvstate_kill_to_kstate_kill(const nvlist_t *, struct pf_kstate_kill *);