Re: [PATCH v2] net: Add trace events for all receive exit points

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 12 Nov 2018 14:44:05 -0500
Geneviève Bastien <gbastien@versatic.net> wrote:

> Trace events are already present for the receive entry points, to indicate
> how the reception entered the stack.
> 
> This patch adds the corresponding exit trace events that will bound the
> reception such that all events occurring between the entry and the exit
> can be considered as part of the reception context. This greatly helps
> for dependency and root cause analyses.
> 
> Without this, it is impossible to determine whether a sched_wakeup
> event following a netif_receive_skb event is the result of the packet
> reception or a simple coincidence after further processing by the
> thread.
> 
> Signed-off-by: Geneviève Bastien <gbastien@versatic.net>
> CC: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: David S. Miller <davem@davemloft.net>
> ---
> Changes in v2:
>   - Add the return value to tracepoints where applicable
>   - Verify if tracepoint is enabled before walking list in
>     netif_receive_skb_list
> ---
>  include/trace/events/net.h | 78 ++++++++++++++++++++++++++++++++++++++
>  net/core/dev.c             | 38 ++++++++++++++++---
>  2 files changed, 110 insertions(+), 6 deletions(-)
> 
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index 00aa72ce0e7c..cff1a7b9d0bb 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -117,6 +117,42 @@ DECLARE_EVENT_CLASS(net_dev_template,
>  		__get_str(name), __entry->skbaddr, __entry->len)
>  )
>  
> +DECLARE_EVENT_CLASS(net_dev_template_return,
> +
> +	TP_PROTO(struct sk_buff *skb, int ret),
> +
> +	TP_ARGS(skb, ret),
> +
> +	TP_STRUCT__entry(
> +		__field(void *,	skbaddr)
> +		__field(int,	ret)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->skbaddr = skb;
> +		__entry->ret = ret;
> +	),
> +
> +	TP_printk("skbaddr=%p ret=%d", __entry->skbaddr, __entry->ret)
> +)
> +
> +DECLARE_EVENT_CLASS(net_dev_template_simple,
> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb),
> +
> +	TP_STRUCT__entry(
> +		__field(void *,	skbaddr)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->skbaddr = skb;
> +	),
> +
> +	TP_printk("skbaddr=%p", __entry->skbaddr)
> +)
> +
>  DEFINE_EVENT(net_dev_template, net_dev_queue,
>  
>  	TP_PROTO(struct sk_buff *skb),
> @@ -244,6 +280,48 @@ DEFINE_EVENT(net_dev_rx_verbose_template, netif_rx_ni_entry,
>  	TP_ARGS(skb)
>  );
>  
> +DEFINE_EVENT(net_dev_template_return, napi_gro_frags_exit,
> +
> +	TP_PROTO(struct sk_buff *skb, int ret),
> +
> +	TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, napi_gro_receive_exit,
> +
> +	TP_PROTO(struct sk_buff *skb, int ret),
> +
> +	TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, netif_receive_skb_exit,
> +
> +	TP_PROTO(struct sk_buff *skb, int ret),
> +
> +	TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_list_exit,

One small nit, and I don't care enough to ask you to fix it, but others
might care. We probably should not intermix net_dev_template_simple
events within net_dev_template_return events.

But like I said, I don't really care, it's more cosmetic than anything
else.

For the tracing side:

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve


> +
> +	TP_PROTO(struct sk_buff *skb),
> +
> +	TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, netif_rx_exit,
> +
> +	TP_PROTO(struct sk_buff *skb, int ret),
> +
> +	TP_ARGS(skb, ret)
> +);
> +
> +DEFINE_EVENT(net_dev_template_return, netif_rx_ni_exit,
> +
> +	TP_PROTO(struct sk_buff *skb, int ret),
> +
> +	TP_ARGS(skb, ret)
> +);
> +
>  #endif /* _TRACE_NET_H */
>  
>