Re: [External] : Re: Please backport: netfilter: nft_counter: Use u64_stats_t for statistic.

On 10/4/24 03:39, Sebastian Andrzej Siewior wrote:
> On 2024-09-27 15:01:00 [-0400], Joseph Salisbury wrote:
>> Is it needed in all stable release patch sets, including v5.15?
> Yes. I would appreciate backporting it all the way where the code is
> available. The dependencies
> 	1eacdd71b3436 ("netfilter: nft_counter: Disable BH in nft_counter_offload_stats().")
> 	a0b39e2dc7017 ("netfilter: nft_counter: Synchronize nft_counter_reset() against reader.")
>
> were already routed via stable.
> The problem is that the seqcount has no lock associated so a reader
> could preempt a writer and then lockup spinning.
>
> Sebastian
Re-send to stable-rt mailing list after turning off HTML:



Thanks for the feedback, Sebastian!

I confirmed that v5.15-rt received those commits via stable.

I attempted a backport of commit 4a1d3acd6ea8 to v5.15-rt.  Commit 
4a1d3acd6ea8 removes nft_counter_seq() from most places, but in v5.15 it 
is still in: nft_counter_module_init():

...
...
...
static int __init nft_counter_module_init(void)
{
         int cpu, err;

         for_each_possible_cpu(cpu)
                 seqcount_init(per_cpu_ptr(*&nft_counter_seq*, cpu));
...
...
...


This then leads to a build failure:
#####################
In file included from ./include/linux/mm_types.h:18,
                  from ./include/linux/buildid.h:5,
                  from ./include/linux/module.h:14,
                  from net/netfilter/nft_counter.c:10:
net/netfilter/nft_counter.c: In function ‘nft_counter_module_init’:
net/netfilter/nft_counter.c:321:44: error: ‘nft_counter_seq’ undeclared 
(first use in this function); did you mean ‘nft_counter_ops’?
   321 | seqcount_init(per_cpu_ptr(&nft_counter_seq, cpu));
       |                                            ^~~~~~~~~~~~~~~
./include/linux/seqlock.h:110:43: note: in definition of macro 
‘seqcount_init’
   110 | # define seqcount_init(s) __seqcount_init(s, NULL, NULL)
       |                                           ^
./include/linux/percpu-defs.h:235:9: note: in expansion of macro 
‘__verify_pcpu_ptr’
   235 | __verify_pcpu_ptr(ptr);                                         \
       |         ^~~~~~~~~~~~~~~~~
net/netfilter/nft_counter.c:321:31: note: in expansion of macro 
‘per_cpu_ptr’
   321 | seqcount_init(per_cpu_ptr(&nft_counter_seq, cpu));
       |                               ^~~~~~~~~~~
net/netfilter/nft_counter.c:321:44: note: each undeclared identifier is 
reported only once for each function it appears in
   321 | seqcount_init(per_cpu_ptr(&nft_counter_seq, cpu));
       |                                            ^~~~~~~~~~~~~~~
./include/linux/seqlock.h:110:43: note: in definition of macro 
‘seqcount_init’
   110 | # define seqcount_init(s) __seqcount_init(s, NULL, NULL)
       |                                           ^
./include/linux/percpu-defs.h:235:9: note: in expansion of macro 
‘__verify_pcpu_ptr’
   235 | __verify_pcpu_ptr(ptr);                                         \
       |         ^~~~~~~~~~~~~~~~~
net/netfilter/nft_counter.c:321:31: note: in expansion of macro 
‘per_cpu_ptr’
   321 | seqcount_init(per_cpu_ptr(&nft_counter_seq, cpu));
       |                               ^~~~~~~~~~~
make[4]: *** [scripts/Makefile.build:289: net/netfilter/nft_counter.o] 
Error 1
make[3]: *** [scripts/Makefile.build:552: net/netfilter] Error 2
#####################



It looks like your patch handles this in nft_counter_init_seqcount().  I 
believe I can just backport the same into in nft_counter_module_init() 
for v5.15, but I wanted to run it by you.

Thanks,

Joe