On Tue, Apr 14, 2026 at 06:55:51PM +0200, Alexandr Nedvedicky wrote:
> it feels like the change is bringing nothing.
>
> if the thread owns the reference to anchor (ruleset) where
> rules belongs to why we need the reference for rule?
>
> or is there some follow up work planned?

You're right that the diff on its own brings no current benefit.
It's pure scaffolding; no caller takes an extra reference yet, so
every refcnt_rele() sees count 1 and the existing pool_put() runs
unchanged. I should have said this more plainly in the cover.

The follow-up I had in mind is refcnt_take(&r->ref) at rule
selection in pf_test_rule() and refcnt_rele() on the pf_test()
exit path, so a packet thread holds its rule reference explicitly
across evaluation. The reason I didn't think anchor refcount alone
was sufficient is pf_rm_rule(): it TAILQ_REMOVEs individual rules
from a ruleset and pool_puts them, while the anchor stays alive.
Anchor refcount protects the ruleset as a container, but not an
individual rule pointer a thread has already picked up from it.

With the current NET_LOCK() + PF_LOCK() regime this is moot. Both
pf_test_rule() and the pf_rm_rule() callers run serialised, so
there is no race today. The rule-level refcount only becomes
load-bearing once the rule-tree walk moves off PF_LOCK() exclusive
(SMR or shared mode), at which point anchor-level alone doesn't
close the gap between "rule removed from TAILQ" and "last packet
thread using it drops its reference".

Is that the direction you want? If you'd rather unlock the read
side via SMR on the rule tree — with pf_rm_rule() deferring
pool_put() to a grace period instead of per-rule refcounting — I
am happy to park this diff and start there instead. I don't want
to execute the comment at the end of pf_test() in a shape that's
no longer the one you have in mind.

thanks
pierre