Re: [PATCH net-next 8/8] netfilter: flowtable: add hardware offload tracepoints

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Feb 22, 2022 at 05:10:03PM +0200, Vlad Buslov wrote:
> Add tracepoints to trace creation and start of execution of flowtable
> hardware offload 'add', 'del' and 'stats' tasks. Move struct
> flow_offload_work from source into header file to allow access to structure
> fields from tracepoint code.

This patch, I would prefer to keep it back and explore exposing trace
infrastructure for the flowtable through netlink.

> Signed-off-by: Vlad Buslov <vladbu@nvidia.com>
> Signed-off-by: Oz Shlomo <ozsh@nvidia.com>
> Reviewed-by: Paul Blakey <paulb@nvidia.com>
> ---
>  include/net/netfilter/nf_flow_table.h       |  9 ++++
>  net/netfilter/nf_flow_table_offload.c       | 20 +++++----
>  net/netfilter/nf_flow_table_offload_trace.h | 48 +++++++++++++++++++++
>  3 files changed, 68 insertions(+), 9 deletions(-)
>  create mode 100644 net/netfilter/nf_flow_table_offload_trace.h
> 
> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> index a3647fadf1cc..5e2aef34acaa 100644
> --- a/include/net/netfilter/nf_flow_table.h
> +++ b/include/net/netfilter/nf_flow_table.h
> @@ -174,6 +174,15 @@ struct flow_offload {
>  	struct rcu_head				rcu_head;
>  };
>  
> +struct flow_offload_work {
> +	struct list_head list;
> +	enum flow_cls_command cmd;
> +	int priority;
> +	struct nf_flowtable *flowtable;
> +	struct flow_offload *flow;
> +	struct work_struct work;
> +};
> +
>  #define NF_FLOW_TIMEOUT (30 * HZ)
>  #define nf_flowtable_time_stamp	(u32)jiffies
>  
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index ff52d903aad9..bf94050d5b54 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -12,20 +12,13 @@
>  #include <net/netfilter/nf_conntrack_acct.h>
>  #include <net/netfilter/nf_conntrack_core.h>
>  #include <net/netfilter/nf_conntrack_tuple.h>
> +#define CREATE_TRACE_POINTS
> +#include "nf_flow_table_offload_trace.h"
>  
>  static struct workqueue_struct *nf_flow_offload_add_wq;
>  static struct workqueue_struct *nf_flow_offload_del_wq;
>  static struct workqueue_struct *nf_flow_offload_stats_wq;
>  
> -struct flow_offload_work {
> -	struct list_head	list;
> -	enum flow_cls_command	cmd;
> -	int			priority;
> -	struct nf_flowtable	*flowtable;
> -	struct flow_offload	*flow;
> -	struct work_struct	work;
> -};
> -
>  #define NF_FLOW_DISSECTOR(__match, __type, __field)	\
>  	(__match)->dissector.offset[__type] =		\
>  		offsetof(struct nf_flow_key, __field)
> @@ -895,6 +888,8 @@ static void flow_offload_work_add(struct flow_offload_work *offload)
>  	struct nf_flow_rule *flow_rule[FLOW_OFFLOAD_DIR_MAX];
>  	int err;
>  
> +	trace_flow_offload_work_add(offload);
> +
>  	err = nf_flow_offload_alloc(offload, flow_rule);
>  	if (err < 0)
>  		return;
> @@ -911,6 +906,8 @@ static void flow_offload_work_add(struct flow_offload_work *offload)
>  
>  static void flow_offload_work_del(struct flow_offload_work *offload)
>  {
> +	trace_flow_offload_work_del(offload);
> +
>  	clear_bit(IPS_HW_OFFLOAD_BIT, &offload->flow->ct->status);
>  	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_ORIGINAL);
>  	flow_offload_tuple_del(offload, FLOW_OFFLOAD_DIR_REPLY);
> @@ -931,6 +928,8 @@ static void flow_offload_work_stats(struct flow_offload_work *offload)
>  	struct flow_stats stats[FLOW_OFFLOAD_DIR_MAX] = {};
>  	u64 lastused;
>  
> +	trace_flow_offload_work_stats(offload);
> +
>  	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_ORIGINAL, &stats[0]);
>  	flow_offload_tuple_stats(offload, FLOW_OFFLOAD_DIR_REPLY, &stats[1]);
>  
> @@ -1034,6 +1033,7 @@ void nf_flow_offload_add(struct nf_flowtable *flowtable,
>  		return;
>  	}
>  
> +	trace_flow_offload_add(offload);
>  	flow_offload_queue_work(offload);
>  }
>  
> @@ -1048,6 +1048,7 @@ void nf_flow_offload_del(struct nf_flowtable *flowtable,
>  		return;
>  
>  	atomic_inc(&net->nft.count_wq_del);
> +	trace_flow_offload_del(offload);
>  	set_bit(NF_FLOW_HW_DYING, &flow->flags);
>  	flow_offload_queue_work(offload);
>  }
> @@ -1068,6 +1069,7 @@ void nf_flow_offload_stats(struct nf_flowtable *flowtable,
>  		return;
>  
>  	atomic_inc(&net->nft.count_wq_stats);
> +	trace_flow_offload_stats(offload);
>  	flow_offload_queue_work(offload);
>  }
>  
> diff --git a/net/netfilter/nf_flow_table_offload_trace.h b/net/netfilter/nf_flow_table_offload_trace.h
> new file mode 100644
> index 000000000000..49cfbc2ec35d
> --- /dev/null
> +++ b/net/netfilter/nf_flow_table_offload_trace.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM nf
> +
> +#if !defined(_NF_FLOW_TABLE_OFFLOAD_TRACE_) || defined(TRACE_HEADER_MULTI_READ)
> +#define _NF_FLOW_TABLE_OFFLOAD_TRACE_
> +
> +#include <linux/tracepoint.h>
> +#include <net/netfilter/nf_tables.h>
> +
> +DECLARE_EVENT_CLASS(
> +	nf_flow_offload_work_template,
> +	TP_PROTO(struct flow_offload_work *w),
> +	TP_ARGS(w),
> +	TP_STRUCT__entry(
> +		__field(void *, work)
> +		__field(void *, flowtable)
> +		__field(void *, flow)
> +	),
> +	TP_fast_assign(
> +		__entry->work = w;
> +		__entry->flowtable = w->flowtable;
> +		__entry->flow = w->flow;
> +	),
> +	TP_printk("work=%p flowtable=%p flow=%p",
> +		  __entry->work, __entry->flowtable, __entry->flow)
> +);
> +
> +#define DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(name)				\
> +	DEFINE_EVENT(nf_flow_offload_work_template, name,		\
> +		     TP_PROTO(struct flow_offload_work *w), TP_ARGS(w))
> +
> +DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(flow_offload_add);
> +DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(flow_offload_work_add);
> +DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(flow_offload_del);
> +DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(flow_offload_work_del);
> +DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(flow_offload_stats);
> +DEFINE_NF_FLOW_OFFLOAD_WORK_EVENT(flow_offload_work_stats);
> +
> +#endif
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_PATH
> +#define TRACE_INCLUDE_PATH ../../net/netfilter
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_FILE nf_flow_table_offload_trace
> +#include <trace/define_trace.h>
> -- 
> 2.31.1
>