From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Subject: Re: [PATCH v5] ether: add support for vtune task tracing
Date: Mon, 24 Jul 2017 14:57:39 +0530
Message-ID: <20170724092738.GA11211@jerin>
References: <1500311700-5298-1-git-send-email-ilia.kurakin@intel.com>
 <1500454485-11156-1-git-send-email-ilia.kurakin@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: dev@dpdk.org, konstantin.ananyev@intel.com, keith.wiles@intel.com,
 dmitry.galanov@intel.com
To: ilia.kurakin@intel.com
Return-path: <dev-bounces@dpdk.org>
Received: from NAM01-BY2-obe.outbound.protection.outlook.com
 (mail-by2nam01on0078.outbound.protection.outlook.com [104.47.34.78])
 by dpdk.org (Postfix) with ESMTP id 013B87CAC
 for <dev@dpdk.org>; Mon, 24 Jul 2017 11:28:00 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1500454485-11156-1-git-send-email-ilia.kurakin@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

-----Original Message-----
> Date: Wed, 19 Jul 2017 11:54:45 +0300
> From: ilia.kurakin@intel.com
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, konstantin.ananyev@intel.com,
>  keith.wiles@intel.com, dmitry.galanov@intel.com, Ilia Kurakin
>  <ilia.kurakin@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@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@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


> 
> ---
> 
> -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@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.