On Tue, Feb 22, 2022 at 05:09:57PM +0200, Vlad Buslov wrote:
> To improve hardware offload debuggability and allow capping total amount of
> offloaded entries in following patch extend struct netns_nftables with
> 'count_hw' counter and expose it to userspace as 'nf_flowtable_count_hw'
> sysctl entry. Increment the counter together with setting NF_FLOW_HW flag
> when scheduling offload add task on workqueue and decrement it after
> successfully scheduling offload del task.
>
> Signed-off-by: Vlad Buslov <vladbu@xxxxxxxxxx>
> Signed-off-by: Oz Shlomo <ozsh@xxxxxxxxxx>
> Reviewed-by: Paul Blakey <paulb@xxxxxxxxxx>
> ---
> include/net/netns/nftables.h | 1 +
> net/netfilter/nf_conntrack_standalone.c | 12 ++++++++++++
> net/netfilter/nf_flow_table_core.c | 12 ++++++++++--
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/netns/nftables.h b/include/net/netns/nftables.h
> index 8c77832d0240..262b8b3213cb 100644
> --- a/include/net/netns/nftables.h
> +++ b/include/net/netns/nftables.h
> @@ -6,6 +6,7 @@
>
> struct netns_nftables {
> u8 gencursor;
> + atomic_t count_hw;
> };
>
> #endif
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 3e1afd10a9b6..8cd55d502ffd 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -618,6 +618,9 @@ enum nf_ct_sysctl_index {
> #ifdef CONFIG_LWTUNNEL
> NF_SYSCTL_CT_LWTUNNEL,
> #endif
> +#if IS_ENABLED(CONFIG_NF_FLOW_TABLE)
> + NF_SYSCTL_CT_COUNT_HW,
> +#endif
>
> __NF_SYSCTL_CT_LAST_SYSCTL,
> };
> @@ -973,6 +976,14 @@ static struct ctl_table nf_ct_sysctl_table[] = {
> .mode = 0644,
> .proc_handler = nf_hooks_lwtunnel_sysctl_handler,
> },
> +#endif
> +#if IS_ENABLED(CONFIG_NF_FLOW_TABLE)
> + [NF_SYSCTL_CT_COUNT_HW] = {
> + .procname = "nf_flowtable_count_hw",
> + .maxlen = sizeof(int),
> + .mode = 0444,
> + .proc_handler = proc_dointvec,
> + },
> #endif
> {}
> };
> @@ -1111,6 +1122,7 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
> table[NF_SYSCTL_CT_PROTO_TIMEOUT_UDP_STREAM].data = &un->timeouts[UDP_CT_REPLIED];
> #if IS_ENABLED(CONFIG_NF_FLOW_TABLE)
> table[NF_SYSCTL_CT_PROTO_TIMEOUT_UDP_OFFLOAD].data = &un->offload_timeout;
> + table[NF_SYSCTL_CT_COUNT_HW].data = &net->nft.count_hw;
> #endif
>
> nf_conntrack_standalone_init_tcp_sysctl(net, table);
> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
> index b90eca7a2f22..9f2b68bc6f80 100644
> --- a/net/netfilter/nf_flow_table_core.c
> +++ b/net/netfilter/nf_flow_table_core.c
> @@ -315,6 +315,9 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
> nf_ct_offload_timeout(flow->ct);
>
> if (nf_flowtable_hw_offload(flow_table)) {
> + struct net *net = read_pnet(&flow_table->net);
> +
> + atomic_inc(&net->nft.count_hw);
> __set_bit(NF_FLOW_HW, &flow->flags);
> nf_flow_offload_add(flow_table, flow);
Better increment this new counter from flow_offload_work_add()? There
is offload->flowtable->net, you could use it from there to bump the
counter once this is placed in hardware (by when IPS_HW_OFFLOAD_BIT is
set on).
That also moves the atomic would be away from the packet path.
> }
> @@ -462,10 +465,15 @@ static void nf_flow_offload_gc_step(struct flow_offload *flow, void *data)
>
> if (test_bit(NF_FLOW_TEARDOWN, &flow->flags)) {
> if (test_bit(NF_FLOW_HW, &flow->flags)) {
> - if (!test_bit(NF_FLOW_HW_DYING, &flow->flags))
> + if (!test_bit(NF_FLOW_HW_DYING, &flow->flags)) {
> + struct net *net = read_pnet(&flow_table->net);
> +
> nf_flow_offload_del(flow_table, flow);
> - else if (test_bit(NF_FLOW_HW_DEAD, &flow->flags))
> + if (test_bit(NF_FLOW_HW_DYING, &flow->flags))
> + atomic_dec(&net->nft.count_hw);
Same with this, I'd suggest to decrement it from flow_offload_work_del().
> + } else if (test_bit(NF_FLOW_HW_DEAD, &flow->flags)) {
> flow_offload_del(flow_table, flow);
> + }
> } else {
> flow_offload_del(flow_table, flow);
> }
> --
> 2.31.1
>
