Re: [PATCH nf-next 2/6] netfilter: nf_tables: wrap tracing with a static key

[Florian Westphal <fw@strlen.de>]

Patrick McHardy <kaber@trash.net> wrote:
> On 24.11, Florian Westphal wrote:
> > diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
> > index 29a6ca9..dabf5ed 100644
> > --- a/net/netfilter/nf_tables_core.c
> > +++ b/net/netfilter/nf_tables_core.c
> > @@ -138,7 +144,8 @@ next_rule:
> >  		if (unlikely(rule->genmask & (1 << gencursor)))
> >  			continue;
> >  
> > -		rulenum++;
> > +		if (static_key_false(&nft_trace_enabled))
> > +			rulenum++;
> 
> This API is deprecated, see Documentation/static-keys.txt.

Oh, thats great.  static_branch_unlikely() is a much better name.

I'll update this locally and send a v2 later (I want to give others
time to provide feedback as well).

> I'm also wondering if this introduces a race condition on architectures
> that don't support jump labels. static_key_slow_inc() simply increases
> the ->enabled counter without further synchronization, so this might
> happen while we're executing this function and some of the increments
> might be skipped.

Hmm, adding synchronization for this is braindead.
The new trace infrastructure doesn't need this rulenum, I only kept it
since thats what Pablo asked me to do.

So we have 3 choices:
- ignore this
- don't use static key here
- kill the old nf_log_packet trace part after all

I find it sad that we have to keep this rule counting around :-/

If there are no further comments I'll remove the static key and do
the rule counting unconditinally.

Thanks Patrick.