Re: [PATCH net-next v2 3/3] netfilter: nf_flow_table: count pending offload workqueue tasks

[Vlad Buslov <vladbu@nvidia.com>]

On Tue 17 May 2022 at 14:26, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, May 17, 2022 at 02:16:04PM +0300, Vlad Buslov wrote:
>> 
>> On Tue 17 May 2022 at 13:20, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > On Mon, May 16, 2022 at 10:10:32PM +0300, Vlad Buslov wrote:
>> > [...]
>> >> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
>> >> index ddc54b6d18ee..c8fc5c7ef04a 100644
>> >> --- a/net/netfilter/Kconfig
>> >> +++ b/net/netfilter/Kconfig
>> >> @@ -734,6 +734,14 @@ config NF_FLOW_TABLE
>> >>  
>> >>  	  To compile it as a module, choose M here.
>> >>  
>> >> +config NF_FLOW_TABLE_PROCFS
>> >> +	bool "Supply flow table statistics in procfs"
>> >> +	default y
>> >> +	depends on PROC_FS
>> >> +	help
>> >> +	  This option enables for the flow table offload statistics
>> >> +	  to be shown in procfs under net/netfilter/nf_flowtable.
>> >
>> > This belongs to patch 2/3.
>> >
>> > Then, use NF_FLOW_TABLE_PROCFS to conditionally add it to
>> > nf_flow_table if this is enabled in .config? To honor this new Kconfig
>> > toggle.
>> >
>> > I mean instead of:
>> >
>> > obj-$(CONFIG_NF_FLOW_TABLE)    += nf_flow_table.o
>> >  nf_flow_table-objs             := nf_flow_table_core.o nf_flow_table_ip.o \
>> > -                                  nf_flow_table_offload.o
>> > +                                  nf_flow_table_offload.o \
>> > +                                  nf_flow_table_sysctl.o
>> >
>> > this?
>> >
>> > nf_flow_table-$(CONFIG_NF_FLOW_TABLE_SYSCTL)    += nf_flow_table_sysctl.o
>> 
>> In V2 I have both sysctl and procfs implementations in single file.
>> As I replied for previous patch in series: Should I split those in two
>> separate files (nf_flow_table_sysctl.c and nf_flow_table_procfs.c) that
>> both could be conditionally compiled depending on their respective
>> configs?
>
> Same file is fine.
>
> Probably instead ?
>
> nf_flow_table-$(CONFIG_SYSCTL)    += nf_flow_table_sysctl.o
>
> so the #ifdef CONFIG_SYSCTL in nf_flow_table_sysctl.c can go away.
>
> you would need to move:
>
>         unsigned int nf_ft_hw_max __read_mostly;
>
> to nf_flow_table_offload.c
>
> Make sense?

Yep. Will send the V3 soon.

Thanks,
Vlad


[...]