The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=d00f66feaa174de17f7df3647028d80c018a2fcc
commit d00f66feaa174de17f7df3647028d80c018a2fcc
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-07-13 14:04:19 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-07-16 11:33:51 +0000
pf: delay taking the rules lock in pf_test()
We don't need the rules lock to protect the mbuf, or even the kif. If an
interface is removed (which is the only way for a kif to go away) we're not
going to receive traffic on it.
We can't delay taking the lock more, because pf_setup_pdesc() calls the
normalisation code, which iterates the scrub rules. If we ever get rid of
those
(as OpenBSD has) it should be possible to delay taking the rules lock until
we
actually need to iterate of the rules. That is, we might be able to avoid
taking
it at all if we match an existing state.
Reviewed by: glebius
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D51329
---
sys/netpfil/pf/pf.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 264830fcf534..ad42f1cccd33 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -10064,6 +10064,8 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc
*pd, struct mbuf **m0,
pd->didx = (dir == PF_IN) ? 1 : 0;
pd->af = pd->naf = af;
+ PF_RULES_ASSERT();
+
TAILQ_INIT(&pd->sctp_multihome_jobs);
if (default_actions != NULL)
memcpy(&pd->act, default_actions, sizeof(pd->act));
@@ -10477,35 +10479,30 @@ pf_test(sa_family_t af, int dir, int pflags, struct
ifnet *ifp, struct mbuf **m0
PF_RULES_RLOCK_TRACKER;
KASSERT(dir == PF_IN || dir == PF_OUT, ("%s: bad direction %d\n",
__func__, dir));
M_ASSERTPKTHDR(*m0);
+ NET_EPOCH_ASSERT();
if (!V_pf_status.running)
return (PF_PASS);
- PF_RULES_RLOCK();
-
kif = (struct pfi_kkif *)ifp->if_pf_kif;
if (__predict_false(kif == NULL)) {
DPFPRINTF(PF_DEBUG_URGENT,
("%s: kif == NULL, if_xname %s\n",
__func__, ifp->if_xname));
- PF_RULES_RUNLOCK();
return (PF_DROP);
}
if (kif->pfik_flags & PFI_IFLAG_SKIP) {
- PF_RULES_RUNLOCK();
return (PF_PASS);
}
if ((*m0)->m_flags & M_SKIP_FIREWALL) {
- PF_RULES_RUNLOCK();
return (PF_PASS);
}
if (__predict_false(! M_WRITABLE(*m0))) {
*m0 = m_unshare(*m0, M_NOWAIT);
if (*m0 == NULL) {
- PF_RULES_RUNLOCK();
return (PF_DROP);
}
}
@@ -10518,12 +10515,10 @@ pf_test(sa_family_t af, int dir, int pflags, struct
ifnet *ifp, struct mbuf **m0
ifp = ifnet_byindexgen(pd.pf_mtag->if_index,
pd.pf_mtag->if_idxgen);
if (ifp == NULL || ifp->if_flags & IFF_DYING) {
- PF_RULES_RUNLOCK();
m_freem(*m0);
*m0 = NULL;
return (PF_PASS);
}
- PF_RULES_RUNLOCK();
(ifp->if_output)(ifp, *m0, sintosa(&pd.pf_mtag->dst), NULL);
*m0 = NULL;
return (PF_PASS);
@@ -10538,11 +10533,12 @@ pf_test(sa_family_t af, int dir, int pflags, struct
ifnet *ifp, struct mbuf **m0
/* But only once. We may see the packet multiple times (e.g.
* PFIL_IN/PFIL_OUT). */
pf_dummynet_flag_remove(pd.m, pd.pf_mtag);
- PF_RULES_RUNLOCK();
return (PF_PASS);
}
+ PF_RULES_RLOCK();
+
if (pf_setup_pdesc(af, dir, &pd, m0, &action, &reason,
kif, default_actions) == -1) {
if (action != PF_PASS)
