[dpdk-dev] [PATCH v5] ether: add support for vtune task tracing

Kurakin, Ilia ilia.kurakin at intel.com
Mon Jul 24 14:33:45 CEST 2017


Hi Jerin,

Thank you for your comments.

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Monday, July 24, 2017 12:28 PM
> To: Kurakin, Ilia <ilia.kurakin at intel.com>
> Cc: dev at dpdk.org; Ananyev, Konstantin <konstantin.ananyev at intel.com>;
> Wiles, Keith <keith.wiles at intel.com>; Galanov, Dmitry
> <dmitry.galanov at intel.com>
> Subject: Re: [PATCH v5] ether: add support for vtune task tracing
> 
> -----Original Message-----
> > Date: Wed, 19 Jul 2017 11:54:45 +0300
> > From: ilia.kurakin at intel.com
> > To: dev at dpdk.org
> > CC: jerin.jacob at caviumnetworks.com, konstantin.ananyev at intel.com,
> > keith.wiles at intel.com, dmitry.galanov at intel.com, Ilia Kurakin
> > <ilia.kurakin at intel.com>
> > Subject: [PATCH v5] ether: add support for vtune task tracing
> > X-Mailer: git-send-email 2.7.4
> >
> > From: Ilia Kurakin <ilia.kurakin at intel.com>
> >
> > The patch adds tracing of loop iterations that yielded no packets in a
> > DPDK application. It is using ITT task API:
> >     https://software.intel.com/en-us/node/544206
> >
> > We suppose the flow of using this tracing would assume the user has
> > ITT lib and header on machine and re-build DPDK with additional make
> parameters:
> >
> >     make EXTRA_CFLAGS=-I<path to ittnotify.h>
> >          EXTRA_LDLIBS="-L<path to libittnotify.a> -littnotify"
> >
> > Signed-off-by: Ilia Kurakin <ilia.kurakin at intel.com>
> 
> Found a checkpatch issue.
> 
> ➜ [master][dpdk.org] $ ./devtools/checkpatches.sh
> 
> ### ether: add support for vtune task tracing
> 
> ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
> #173: FILE: lib/librte_ether/rte_ethdev_profile.c:59:
> +	__itt_domain *domains[RTE_MAX_QUEUES_PER_PORT];
>  	             ^
> 
> ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
> #177: FILE: lib/librte_ether/rte_ethdev_profile.c:63:
> +	__itt_string_handle *handles[RTE_MAX_QUEUES_PER_PORT];
>  	                    ^
> 
> total: 2 errors, 0 warnings, 278 lines checked
> 
> 0/1 valid patch

I suppose this is a mistake in the checkpatch script, since here I use "*" as a pointer.
Probably the script considers "*" as multiplication sign.

> 
> 
> >
> > ---
> >
> > -V2 change:
> >     ITT tasks collection is moved to rx callback
> >
> > -V3 change:
> >     rte_ethdev_profile.c created, all profile specific code moved there.
> >
> >     Added generic profile function
> >
> > -V4 change:
> >     checkpatch issues fixed
> >
> >     Added documentation topic
> >
> > -V5 change:
> >     Documentation fixes
> >
> >
> >  config/common_base                    |   1 +
> >  doc/guides/prog_guide/profile_app.rst |  34 ++++++++
> >  lib/librte_ether/Makefile             |   1 +
> >  lib/librte_ether/rte_ethdev.c         |   4 +
> >  lib/librte_ether/rte_ethdev_profile.c | 156
> > ++++++++++++++++++++++++++++++++++
> >  lib/librte_ether/rte_ethdev_profile.h |  52 ++++++++++++
> >  6 files changed, 248 insertions(+)
> >  create mode 100644 lib/librte_ether/rte_ethdev_profile.c
> >  create mode 100644 lib/librte_ether/rte_ethdev_profile.h
> >
> > diff --git a/config/common_base b/config/common_base index
> > 8ae6e92..dda51db 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -136,6 +136,7 @@ CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> >  CONFIG_RTE_LIBRTE_IEEE1588=n
> >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> >  CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=y
> > +CONFIG_RTE_ETHDEV_PROFILE_ITT_WASTED_RX_ITERATIONS=n
> >
> >  #
> >  # Turn off Tx preparation stage
> > diff --git a/doc/guides/prog_guide/profile_app.rst
> > b/doc/guides/prog_guide/profile_app.rst
> > index 54b546a..590cb72 100644
> > --- a/doc/guides/prog_guide/profile_app.rst
> > +++ b/doc/guides/prog_guide/profile_app.rst
> > @@ -59,6 +59,40 @@ Refer to the
> >  for details about application profiling.
> >
> >
> > +Profiling wasted iterations with ITT
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Iterations that yielded no RX packets (wasted loop iterations) can be
> > +analyzed using Intel® VTune\ :sup:`TM` Amplifier. This profiling
> > +employs the `Instrumentation and Tracing Technology (ITT) API
> > +<https://software.intel.com/en-us/node/544195>`_
> > +feature of VTune Amplifier and requires only reconfiguring of DPDK
> > +library, no changes in a DPDK application are needed.
> > +
> > +To trace wasted iterations on RX queues, first reconfigure DPDK with
> > +``CONFIG_RTE_ETHDEV_RXTX_CALLBACKS`` and
> > +``CONFIG_RTE_ETHDEV_PROFILE_ITT_WASTED_RX_ITERATIONS`` enabled.
> > +
> > +Then rebuild DPDK, specifying paths to the ITT header and library,
> > +which can be found in any VTune Amplifier distribution in the
> > +*include* and *lib* directories respectively:
> > +
> > +.. code-block:: console
> > +
> > +    make EXTRA_CFLAGS=-I<path to ittnotify.h> \
> > +         EXTRA_LDLIBS="-L<path to libittnotify.a> -littnotify"
> > +
> > +Finally, to see wasted iterations in your performance analysis
> > +results, select the *"Analyze user tasks, events, and counters"*
> > +checkbox in the *"Analysis Type"* tab when configuring analysis via VTune
> Amplifier GUI.
> > +Alternatively, when running VTune Amplifier via command line, specify
> > +``-knob enable-user-tasks=true`` option.
> > +
> > +Collected regions of wasted iterations will be marked on VTune
> > +Amplifier's timeline as ITT tasks. These ITT tasks have predefined
> > +names, containing Ethernet device and RX queue identifiers.
> 
> Documentation changes could move to different patch for better review.
> 
> > diff --git a/lib/librte_ether/rte_ethdev_profile.h
> > b/lib/librte_ether/rte_ethdev_profile.h
> > new file mode 100644
> > index 0000000..1eb72bd
> > +/**
> > + * Initialization of profiling RX queues for the Ethernet device.
> > + * Implementation of this function depends on chosen profiling
> > +method,
> > + * defined in configs.
> > + *
> > + * @param port_id
> > + *  The port identifier of the Ethernet device.
> > + * @param dev
> > + *  Pointer to struct rte_eth_dev corresponding to given port_id.
> > + */
> > +void
> > +rte_eth_profile_rx_init(uint8_t port_id, struct rte_eth_dev *dev);
> 
> better to prefix __ for internal function(i.e __rte_eth_profile_rx_init)
> 
> With above suggested changes:
> 
> Acked-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> 
> 
> > +
> > +#endif
> > --
> > 2.7.4
> >
> >
> > --------------------------------------------------------------------
> > Joint Stock Company Intel A/O
> > Registered legal address: Krylatsky Hills Business Park,
> > 17 Krylatskaya Str., Bldg 4, Moscow 121614, Russian Federation
> >
> > This e-mail and any attachments may contain confidential material for
> > the sole use of the intended recipient(s). Any review or distribution
> > by others is strictly prohibited. If you are not the intended
> > recipient, please contact the sender and delete all copies.
> 
> Remove such notice when send to public mailing list.


--------------------------------------------------------------------
Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


More information about the dev mailing list