The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=fbb290df083678ceefec4da6133e55dbf99f9794
commit fbb290df083678ceefec4da6133e55dbf99f9794
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-07-07 12:53:49 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-07-15 07:55:28 +0000
pf: Remove ptr_array from struct pf_kruleset
Each ruleset's rules are stored in a TAILQ called "ptr" with "rcount"
representing the number of rules in the ruleset; "ptr_array" points to an
array of the same length.
"ptr" is backed by pool_get(9) and may change in size as "expired" rules
get removed from the ruleset - see "once" in pf.conf(5).
"ptr_array" is allocated momentarily through mallocarray(9) and gets filled
with the TAILQ entries, so that the sole user pfsync(4) can access the list
of rules by index to pick the n-th rule during state insertion.
Remove "ptr_array" and make pfsync iterate over the TAILQ instead to get the
matching rule's index. This simplifies both code and data structures and
avoids duplicate memory management.
OK sashan
Obtained from: OpenBSD, kn <k...@openbsd.org>, d13e571b26
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sys/net/pfvar.h | 1 -
sys/netpfil/pf/if_pfsync.c | 11 +++++++----
sys/netpfil/pf/pf_ioctl.c | 30 +++++-------------------------
3 files changed, 12 insertions(+), 30 deletions(-)
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 1f2011634695..452a8eb4024b 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1370,7 +1370,6 @@ struct pf_kruleset {
struct pf_krulequeue queues[2];
struct {
struct pf_krulequeue *ptr;
- struct pf_krule **ptr_array;
u_int32_t rcount;
u_int32_t ticket;
int open;
diff --git a/sys/netpfil/pf/if_pfsync.c b/sys/netpfil/pf/if_pfsync.c
index 2391edaf1a5a..4e03584b8f85 100644
--- a/sys/netpfil/pf/if_pfsync.c
+++ b/sys/netpfil/pf/if_pfsync.c
@@ -532,6 +532,7 @@ pfsync_state_import(union pfsync_state_union *sp, int
flags, int msg_version)
struct pf_kpooladdr *rpool_first;
int error;
uint8_t rt = 0;
+ int n = 0;
PF_RULES_RASSERT();
@@ -557,10 +558,12 @@ pfsync_state_import(union pfsync_state_union *sp, int
flags, int msg_version)
*/
if (sp->pfs_1301.rule != htonl(-1) && sp->pfs_1301.anchor == htonl(-1)
&&
(flags & (PFSYNC_SI_IOCTL | PFSYNC_SI_CKSUM)) &&
ntohl(sp->pfs_1301.rule) <
- pf_main_ruleset.rules[PF_RULESET_FILTER].active.rcount)
- r = pf_main_ruleset.rules[
-
PF_RULESET_FILTER].active.ptr_array[ntohl(sp->pfs_1301.rule)];
- else
+ pf_main_ruleset.rules[PF_RULESET_FILTER].active.rcount) {
+ TAILQ_FOREACH(r, pf_main_ruleset.rules[
+ PF_RULESET_FILTER].active.ptr, entries)
+ if (ntohl(sp->pfs_1301.rule) == n++)
+ break;
+ } else
r = &V_pf_default_rule;
/*
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index c14211edf10f..016bb1fedef0 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -1359,7 +1359,7 @@ static int
pf_commit_rules(u_int32_t ticket, int rs_num, char *anchor)
{
struct pf_kruleset *rs;
- struct pf_krule *rule, **old_array, *old_rule;
+ struct pf_krule *rule, *old_rule;
struct pf_krulequeue *old_rules;
struct pf_krule_global *old_tree;
int error;
@@ -1384,13 +1384,10 @@ pf_commit_rules(u_int32_t ticket, int rs_num, char
*anchor)
/* Swap rules, keep the old. */
old_rules = rs->rules[rs_num].active.ptr;
old_rcount = rs->rules[rs_num].active.rcount;
- old_array = rs->rules[rs_num].active.ptr_array;
old_tree = rs->rules[rs_num].active.tree;
rs->rules[rs_num].active.ptr =
rs->rules[rs_num].inactive.ptr;
- rs->rules[rs_num].active.ptr_array =
- rs->rules[rs_num].inactive.ptr_array;
rs->rules[rs_num].active.tree =
rs->rules[rs_num].inactive.tree;
rs->rules[rs_num].active.rcount =
@@ -1420,7 +1417,6 @@ pf_commit_rules(u_int32_t ticket, int rs_num, char
*anchor)
}
rs->rules[rs_num].inactive.ptr = old_rules;
- rs->rules[rs_num].inactive.ptr_array = old_array;
rs->rules[rs_num].inactive.tree = NULL; /* important for
pf_ioctl_addrule */
rs->rules[rs_num].inactive.rcount = old_rcount;
@@ -1433,9 +1429,6 @@ pf_commit_rules(u_int32_t ticket, int rs_num, char
*anchor)
while ((rule = TAILQ_FIRST(old_rules)) != NULL)
pf_unlink_rule_locked(old_rules, rule);
PF_UNLNKDRULES_UNLOCK();
- if (rs->rules[rs_num].inactive.ptr_array)
- free(rs->rules[rs_num].inactive.ptr_array, M_TEMP);
- rs->rules[rs_num].inactive.ptr_array = NULL;
rs->rules[rs_num].inactive.rcount = 0;
rs->rules[rs_num].inactive.open = 0;
pf_remove_if_empty_kruleset(rs);
@@ -1458,24 +1451,11 @@ pf_setup_pfsync_matching(struct pf_kruleset *rs)
if (rs_cnt == PF_RULESET_SCRUB)
continue;
- if (rs->rules[rs_cnt].inactive.ptr_array)
- free(rs->rules[rs_cnt].inactive.ptr_array, M_TEMP);
- rs->rules[rs_cnt].inactive.ptr_array = NULL;
-
if (rs->rules[rs_cnt].inactive.rcount) {
- rs->rules[rs_cnt].inactive.ptr_array =
- mallocarray(rs->rules[rs_cnt].inactive.rcount,
- sizeof(struct pf_rule **),
- M_TEMP, M_NOWAIT);
-
- if (!rs->rules[rs_cnt].inactive.ptr_array)
- return (ENOMEM);
- }
-
- TAILQ_FOREACH(rule, rs->rules[rs_cnt].inactive.ptr,
- entries) {
- pf_hash_rule_rolling(&ctx, rule);
- (rs->rules[rs_cnt].inactive.ptr_array)[rule->nr] = rule;
+ TAILQ_FOREACH(rule, rs->rules[rs_cnt].inactive.ptr,
+ entries) {
+ pf_hash_rule_rolling(&ctx, rule);
+ }
}
}
