From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathieu Desnoyers Subject: Re: [PATCH] net: Add trace events for all receive exit points Date: Thu, 8 Nov 2018 15:25:44 -0500 (EST) Message-ID: <1057796879.2336.1541708744442.JavaMail.zimbra@efficios.com> References: <20181108195648.31846-1-gbastien@versatic.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: "David S. Miller" , netdev , rostedt , Ingo Molnar To: =?utf-8?Q?Genevi=C3=A8ve?= Bastien Return-path: Received: from mail.efficios.com ([167.114.142.138]:42968 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725723AbeKIGCw (ORCPT ); Fri, 9 Nov 2018 01:02:52 -0500 In-Reply-To: <20181108195648.31846-1-gbastien@versatic.net> Sender: netdev-owner@vger.kernel.org List-ID: ----- On Nov 8, 2018, at 2:56 PM, Genevi=C3=A8ve Bastien gbastien@versatic.= net wrote: > Trace events are already present for the receive entry points, to indicat= e > how the reception entered the stack. >=20 > 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. >=20 > 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. As a clarification: it is indeed not possible with tracepoint instrumentati= on, which I think is your point here. It might be possible to do so by using ot= her mechanisms like kretprobes, but considering that the "entry" point was deem= ed important enough to have a tracepoint, it would be good to add the matching= exit events. Being able to link the packet reception entry/exit with wakeups is key to allow tools to perform automated network stack latency analysis, so I think the use case justifies adding the missing "exit" events. It might be great if you can provide a glimpse of the wakeup dependency cha= in analysis prototypes you have created, which rely on this new event, in orde= r to justify adding it. Thanks, Mathieu >=20 > Signed-off-by: Genevi=C3=A8ve Bastien > CC: Mathieu Desnoyers > CC: Steven Rostedt > CC: Ingo Molnar > CC: David S. Miller > --- > include/trace/events/net.h | 59 ++++++++++++++++++++++++++++++++++++++ > net/core/dev.c | 30 ++++++++++++++++--- > 2 files changed, 85 insertions(+), 4 deletions(-) >=20 > diff --git a/include/trace/events/net.h b/include/trace/events/net.h > index 00aa72ce0e7c..318307511018 100644 > --- a/include/trace/events/net.h > +++ b/include/trace/events/net.h > @@ -117,6 +117,23 @@ DECLARE_EVENT_CLASS(net_dev_template, > =09=09__get_str(name), __entry->skbaddr, __entry->len) > ) >=20 > +DECLARE_EVENT_CLASS(net_dev_template_simple, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb), > + > +=09TP_STRUCT__entry( > +=09=09__field(void *,=09skbaddr) > +=09), > + > +=09TP_fast_assign( > +=09=09__entry->skbaddr =3D skb; > +=09), > + > +=09TP_printk("skbaddr=3D%p", __entry->skbaddr) > +) > + > DEFINE_EVENT(net_dev_template, net_dev_queue, >=20 > =09TP_PROTO(struct sk_buff *skb), > @@ -244,6 +261,48 @@ DEFINE_EVENT(net_dev_rx_verbose_template, > netif_rx_ni_entry, > =09TP_ARGS(skb) > ); >=20 > +DEFINE_EVENT(net_dev_template_simple, napi_gro_frags_exit, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb) > +); > + > +DEFINE_EVENT(net_dev_template_simple, napi_gro_receive_exit, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb) > +); > + > +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_exit, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb) > +); > + > +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_list_exit, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb) > +); > + > +DEFINE_EVENT(net_dev_template_simple, netif_rx_exit, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb) > +); > + > +DEFINE_EVENT(net_dev_template_simple, netif_rx_ni_exit, > + > +=09TP_PROTO(struct sk_buff *skb), > + > +=09TP_ARGS(skb) > +); > + > #endif /* _TRACE_NET_H */ >=20 > /* This part must be outside protection */ > diff --git a/net/core/dev.c b/net/core/dev.c > index 0ffcbdd55fa9..e670ca27e829 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -4520,9 +4520,14 @@ static int netif_rx_internal(struct sk_buff *skb) >=20 > int netif_rx(struct sk_buff *skb) > { > +=09int ret; > + > =09trace_netif_rx_entry(skb); >=20 > -=09return netif_rx_internal(skb); > +=09ret =3D netif_rx_internal(skb); > +=09trace_netif_rx_exit(skb); > + > +=09return ret; > } > EXPORT_SYMBOL(netif_rx); >=20 > @@ -4537,6 +4542,7 @@ int netif_rx_ni(struct sk_buff *skb) > =09if (local_softirq_pending()) > =09=09do_softirq(); > =09preempt_enable(); > +=09trace_netif_rx_ni_exit(skb); >=20 > =09return err; > } > @@ -5222,9 +5228,14 @@ static void netif_receive_skb_list_internal(struct > list_head *head) > */ > int netif_receive_skb(struct sk_buff *skb) > { > +=09int ret; > + > =09trace_netif_receive_skb_entry(skb); >=20 > -=09return netif_receive_skb_internal(skb); > +=09ret =3D netif_receive_skb_internal(skb); > +=09trace_netif_receive_skb_exit(skb); > + > +=09return ret; > } > EXPORT_SYMBOL(netif_receive_skb); >=20 > @@ -5247,6 +5258,8 @@ void netif_receive_skb_list(struct list_head *head) > =09list_for_each_entry(skb, head, list) > =09=09trace_netif_receive_skb_list_entry(skb); > =09netif_receive_skb_list_internal(head); > +=09list_for_each_entry(skb, head, list) > +=09=09trace_netif_receive_skb_list_exit(skb); > } > EXPORT_SYMBOL(netif_receive_skb_list); >=20 > @@ -5634,12 +5647,17 @@ static gro_result_t napi_skb_finish(gro_result_t = ret, > struct sk_buff *skb) >=20 > gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *s= kb) > { > +=09gro_result_t ret; > + > =09skb_mark_napi_id(skb, napi); > =09trace_napi_gro_receive_entry(skb); >=20 > =09skb_gro_reset_offset(skb); >=20 > -=09return napi_skb_finish(dev_gro_receive(napi, skb), skb); > +=09ret =3D napi_skb_finish(dev_gro_receive(napi, skb), skb); > +=09trace_napi_gro_receive_exit(skb); > + > +=09return ret; > } > EXPORT_SYMBOL(napi_gro_receive); >=20 > @@ -5753,6 +5771,7 @@ static struct sk_buff *napi_frags_skb(struct napi_s= truct > *napi) >=20 > gro_result_t napi_gro_frags(struct napi_struct *napi) > { > +=09gro_result_t ret; > =09struct sk_buff *skb =3D napi_frags_skb(napi); >=20 > =09if (!skb) > @@ -5760,7 +5779,10 @@ gro_result_t napi_gro_frags(struct napi_struct *na= pi) >=20 > =09trace_napi_gro_frags_entry(skb); >=20 > -=09return napi_frags_finish(napi, skb, dev_gro_receive(napi, skb)); > +=09ret =3D napi_frags_finish(napi, skb, dev_gro_receive(napi, skb)); > +=09trace_napi_gro_frags_exit(skb); > + > +=09return ret; > } > EXPORT_SYMBOL(napi_gro_frags); >=20 > -- > 2.19.1 --=20 Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com