On Tue 17 May 2022 at 13:20, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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?
>
>> config NETFILTER_XTABLES
>> tristate "Netfilter Xtables support (required for ip_tables)"
>> default m if NETFILTER_ADVANCED=n
>> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
>> index e2598f98017c..c86dd627ef42 100644
>> --- a/net/netfilter/nf_flow_table_core.c
>> +++ b/net/netfilter/nf_flow_table_core.c
>> @@ -662,17 +662,51 @@ void nf_flow_table_free(struct nf_flowtable *flow_table)
>> }
>> EXPORT_SYMBOL_GPL(nf_flow_table_free);
>>
>> +static int nf_flow_table_init_net(struct net *net)
>> +{
>> + net->ft.stat = alloc_percpu(struct nf_flow_table_stat);
>
> Missing check for NULL in case alloc_percpu() fails?
>
I might be missing something, but why isn't NULL check in following line
sufficient?
>> + return net->ft.stat ? 0 : -ENOMEM;
>> +}
