DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: ilia.kurakin@intel.com
Cc: dev@dpdk.org, konstantin.ananyev@intel.com,
	keith.wiles@intel.com, dmitry.galanov@intel.com
Subject: Re: [dpdk-dev] [PATCH v2] ether: add support for vtune task tracing
Date: Mon, 10 Jul 2017 18:00:49 +0530	[thread overview]
Message-ID: <20170710123048.GA20179@jerin> (raw)
In-Reply-To: <1499359372-6658-1-git-send-email-ilia.kurakin@intel.com>

-----Original Message-----
> Date: Thu, 6 Jul 2017 19:42:52 +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 v2] 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>
> 
> ---
> 
> -V2 change:
>     ITT tasks collection is moved to rx callback
> 
> 
>  config/common_base                    |   1 +
>  lib/librte_ether/Makefile             |   2 +
>  lib/librte_ether/rte_ethdev.c         |   9 +++
>  lib/librte_ether/rte_ethdev_profile.h | 137 ++++++++++++++++++++++++++++++++++

Please create rte_ethdev_profile.c for the actual source code.

>  4 files changed, 149 insertions(+)
>  create mode 100644 lib/librte_ether/rte_ethdev_profile.h
> 
> diff --git a/config/common_base b/config/common_base
> index 660588a..8795a95 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_TRACE_ITT_WASTED_RX_ITERATIONS=n

Should we change to CONFIG_RTE_ETHDEV_PROFILE_ITT_WASTED_RX_ITERATIONS
as its going to under rte_ethdev_profile.c

>  
>  #
>  # Turn off Tx preparation stage
> diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
> index 93fdde1..4d4017f 100644
> --- a/lib/librte_ether/Makefile
> +++ b/lib/librte_ether/Makefile
> @@ -56,5 +56,7 @@ SYMLINK-y-include += rte_eth_ctrl.h
>  SYMLINK-y-include += rte_dev_info.h
>  SYMLINK-y-include += rte_flow.h
>  SYMLINK-y-include += rte_flow_driver.h
> +SYMLINK-${CONFIG_RTE_ETHDEV_TRACE_ITT_WASTED_RX_ITERATIONS}-include \
> +	+= rte_ethdev_profile.h

With your current scheme,  you don't need to expose this header to
application as application is not directly including it.

One option is, Wherever there is reference to this header file in ethdev library, you
can update the CFLAGS in lib/librte_ether/Makefile

>  
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 957ae2a..a99eeea 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -68,6 +68,10 @@
>  #include "rte_ether.h"
>  #include "rte_ethdev.h"
>  
> +#ifdef RTE_ETHDEV_TRACE_ITT_WASTED_RX_ITERATIONS

You can avoid conditional compilation here by including
rte_ethdev_profile.h always and ifdef in the rte_ethdev_profile.c
source code. With that scheme, rte_ethdev.c will be clean.

> +#include "rte_ethdev_profile.h"
> +#endif
> +
>  static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
>  struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
>  static struct rte_eth_dev_data *rte_eth_dev_data;
> @@ -825,6 +829,11 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>  		return diag;
>  	}
>  
> +#ifdef RTE_ETHDEV_TRACE_ITT_WASTED_RX_ITERATIONS

Same as above.

Call a generic function from here say(rte_eth_rx_profile_init()) and
add ifdef in rte_ethdev_profile.c

> +	/* See rte_ethdev_profile.h to find comments on code below. */
> +	rte_eth_init_itt(port_id, dev->data->name, nb_rx_q);
> +#endif
> +
>  	return 0;
>  }
>  

Other than the above comments, it looks good to me.

  reply	other threads:[~2017-07-10 12:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-19 17:18 [dpdk-dev] [PATCH] " ilia.kurakin
2017-06-22  9:42 ` Ananyev, Konstantin
2017-06-22 12:12   ` Kurakin, Ilia
2017-06-22 16:46     ` Galanov, Dmitry
2017-06-27 13:16 ` ilia.kurakin
2017-06-30  3:30   ` Jerin Jacob
2017-06-30 10:13     ` Ananyev, Konstantin
2017-07-06 16:42   ` [dpdk-dev] [PATCH v2] " ilia.kurakin
2017-07-10 12:30     ` Jerin Jacob [this message]
2017-07-11 17:24     ` [dpdk-dev] [PATCH v3] 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 ilia.kurakin
2017-07-11 17:48     ` [dpdk-dev] [PATCH v3] ether: add support for vtune task tracing ilia.kurakin
2017-07-14  5:45       ` Jerin Jacob
2017-07-17 17:15       ` [dpdk-dev] [PATCH v4] " ilia.kurakin
2017-07-19  8:54         ` [dpdk-dev] [PATCH v5] " ilia.kurakin
2017-07-24  9:27           ` Jerin Jacob
2017-07-24 12:33             ` Kurakin, Ilia
2017-09-22 10:19             ` Thomas Monjalon
2017-07-24 17:06           ` [dpdk-dev] [PATCH v6] " ilia.kurakin
2017-09-08 12:57             ` [dpdk-dev] [PATCH v7] " ilia.kurakin
2017-09-22 10:42               ` Thomas Monjalon
2017-09-22 14:52               ` [dpdk-dev] [PATCH v8] " ilia.kurakin
2017-09-22 17:04                 ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170710123048.GA20179@jerin \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.galanov@intel.com \
    --cc=ilia.kurakin@intel.com \
    --cc=keith.wiles@intel.com \
    --cc=konstantin.ananyev@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).