Page MenuHomeFreeBSD

pf: Replace union pf_krule_ptr with struct pf_krule in in-kernel structs
ClosedPublic

Authored by vegeta_tuxpowered.net on Oct 2 2024, 8:54 AM.
Tags
None
Referenced Files
F102965235: D46868.diff
Tue, Nov 19, 7:05 AM
Unknown Object (File)
Sat, Nov 16, 4:02 PM
Unknown Object (File)
Sat, Nov 16, 8:24 AM
Unknown Object (File)
Fri, Nov 15, 2:14 PM
Unknown Object (File)
Fri, Nov 15, 2:01 PM
Unknown Object (File)
Sat, Nov 9, 7:16 PM
Unknown Object (File)
Fri, Nov 8, 3:54 PM
Unknown Object (File)
Mon, Oct 21, 4:13 PM

Details

Summary

There is no need for the union pf_krule_ptr for kernel-only structs like pf_kstate and pf_ksrc_node. The rules are always accessed by pointer. The rule numbers are a leftover from using the same structure for pfctl(8) and pf(4).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

vegeta_tuxpowered.net edited the summary of this revision. (Show Details)

I like this, but we should go all the way and remove the pf_krule_ptr entirely. It's strictly a kernel structure, so there's no reason to keep it:

diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index fc24961abb51..b5d56ab45ce7 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -761,18 +761,13 @@ struct pf_keth_rule {
        uint32_t                ridentifier;
 };

-union pf_krule_ptr {
-       struct pf_krule         *ptr;
-       u_int32_t                nr;
-};
-
 RB_HEAD(pf_krule_global, pf_krule);
 RB_PROTOTYPE(pf_krule_global, pf_krule, entry_global, pf_krule_compare);

 struct pf_krule {
        struct pf_rule_addr      src;
        struct pf_rule_addr      dst;
-       union pf_krule_ptr       skip[PF_SKIP_COUNT];
+       struct pf_krule         *skip[PF_SKIP_COUNT];
        char                     label[PF_RULE_MAX_LABEL_COUNT][PF_RULE_LABEL_SIZE];
        uint32_t                 ridentifier;
        char                     ifname[IFNAMSIZ];
@@ -889,7 +884,7 @@ struct pf_ksrc_node {
        struct pf_addr           addr;
        struct pf_addr           raddr;
        struct pf_krule_slist    match_rules;
-       union pf_krule_ptr       rule;
+       struct pf_krule         *rule;
        struct pfi_kkif         *rkif;
        counter_u64_t            bytes[2];
        counter_u64_t            packets[2];
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 3f56ad4255a0..b5a18c37a89b 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -976,7 +976,7 @@ pf_find_src_node(struct pf_addr *src, struct pf_krule *rule, sa_family_t af,
        *sh = &V_pf_srchash[pf_hashsrc(src, af)];
        PF_HASHROW_LOCK(*sh);
        LIST_FOREACH(n, &(*sh)->nodes, entry)
-               if (n->rule.ptr == rule && n->af == af &&
+               if (n->rule == rule && n->af == af &&
                    ((af == AF_INET && n->addr.v4.s_addr == src->v4.s_addr) ||
                    (af == AF_INET6 && bcmp(&n->addr, src, sizeof(*src)) == 0)))
                        break;
...

I'll update the patch to include my changes once the test run finishes.

Completely remove pf_krule_ptr

In D46868#1068815, @kp wrote:

Completely remove pf_krule_ptr

LGTM

This revision was not accepted when it landed; it landed in state Needs Review.Oct 2 2024, 5:54 PM
This revision was automatically updated to reflect the committed changes.