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

Message ID 1498569361-5011-1-git-send-email-ilia.kurakin@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

ilia.kurakin@intel.com June 27, 2017, 1:16 p.m. UTC
  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>
---
 config/common_base             |   1 +
 lib/librte_ether/Makefile      |   1 +
 lib/librte_ether/rte_eth_itt.h | 102 +++++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.c  |   5 ++
 lib/librte_ether/rte_ethdev.h  |  23 ++++++++++
 5 files changed, 132 insertions(+)
 create mode 100644 lib/librte_ether/rte_eth_itt.h
  

Comments

Jerin Jacob June 30, 2017, 3:30 a.m. UTC | #1
-----Original Message-----
> Date: Tue, 27 Jun 2017 16:16:01 +0300
> From: ilia.kurakin@intel.com
> To: dev@dpdk.org
> CC: konstantin.ananyev@intel.com, keith.wiles@intel.com,
>  dmitry.galanov@intel.com, Ilia Kurakin <ilia.kurakin@intel.com>
> Subject: [dpdk-dev] [PATCH] ether: add support for vtune task tracing
> X-Mailer: git-send-email 2.7.4
> 
> From: Ilia Kurakin <ilia.kurakin@intel.com>
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index f6e6c74..ee7cc42 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -186,6 +186,10 @@ extern "C" {
>  #include "rte_eth_ctrl.h"
>  #include "rte_dev_info.h"
>  
> +#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS
> +#include "rte_eth_itt.h"
> +#endif
> +
>  struct rte_mbuf;
>  
>  /**
> @@ -2710,6 +2714,25 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
>  	int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
>  			rx_pkts, nb_pkts);
>  
> +#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS

If we give the generic name like above then the solution should be generic
and it should not be tightly coupled with itt.

Different architectures may have different profiling tools like x86 has
itt or it is possible to have generic perf based plugin in future.

Considering the above points, IMO, we can not add code in
rte_eth_rx_burst() for each profiling variants. I think, this code can
go in lib/librte_ether/rte_ethdev_profile.c and based on specific
conditional compilation flag(something like RTE_ETHDEV_TRACE_ITT_WASTED_RX_ITERATIONS)
it can register a rx callback.
or any other scheme without directly modifying rte_eth_rx_burst() for
itt based profiling.


> +	/* See rte_eth_itt.h to find comments on code below. */
> +	if (unlikely(nb_rx == 0)) {
> +		if (!itt_aux_data[port_id].queue_is_wasting_iters[queue_id]) {
> +			__itt_task_begin(
> +				itt_aux_data[port_id].wasted_iter_domains[queue_id],
> +				__itt_null, __itt_null,
> +				itt_aux_data[port_id].wasted_iter_handles[queue_id]);
> +			itt_aux_data[port_id].queue_is_wasting_iters[queue_id] = 1;
> +		}
> +	} else {
> +		if (unlikely(itt_aux_data[port_id].queue_is_wasting_iters[queue_id])) {
> +			__itt_task_end(
> +				itt_aux_data[port_id].wasted_iter_domains[queue_id]);
> +			itt_aux_data[port_id].queue_is_wasting_iters[queue_id] = 0;
> +		}
> +	}
> +#endif
> +
>  #ifdef RTE_ETHDEV_RXTX_CALLBACKS
>  	struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
>  
> -- 
> 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.
>
  
Ananyev, Konstantin June 30, 2017, 10:13 a.m. UTC | #2
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Friday, June 30, 2017 4:31 AM
> To: Kurakin, Ilia <ilia.kurakin@intel.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wiles, Keith <keith.wiles@intel.com>; Galanov, Dmitry
> <dmitry.galanov@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] ether: add support for vtune task tracing
> 
> -----Original Message-----
> > Date: Tue, 27 Jun 2017 16:16:01 +0300
> > From: ilia.kurakin@intel.com
> > To: dev@dpdk.org
> > CC: konstantin.ananyev@intel.com, keith.wiles@intel.com,
> >  dmitry.galanov@intel.com, Ilia Kurakin <ilia.kurakin@intel.com>
> > Subject: [dpdk-dev] [PATCH] ether: add support for vtune task tracing
> > X-Mailer: git-send-email 2.7.4
> >
> > From: Ilia Kurakin <ilia.kurakin@intel.com>
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> > index f6e6c74..ee7cc42 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -186,6 +186,10 @@ extern "C" {
> >  #include "rte_eth_ctrl.h"
> >  #include "rte_dev_info.h"
> >
> > +#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS
> > +#include "rte_eth_itt.h"
> > +#endif
> > +
> >  struct rte_mbuf;
> >
> >  /**
> > @@ -2710,6 +2714,25 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
> >  	int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
> >  			rx_pkts, nb_pkts);
> >
> > +#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS
> 
> If we give the generic name like above then the solution should be generic
> and it should not be tightly coupled with itt.
> 
> Different architectures may have different profiling tools like x86 has
> itt or it is possible to have generic perf based plugin in future.
> 
> Considering the above points, IMO, we can not add code in
> rte_eth_rx_burst() for each profiling variants. 

+ 1

>I think, this code can
> go in lib/librte_ether/rte_ethdev_profile.c and based on specific
> conditional compilation flag(something like RTE_ETHDEV_TRACE_ITT_WASTED_RX_ITERATIONS)
> it can register a rx callback.
> or any other scheme without directly modifying rte_eth_rx_burst() for
> itt based profiling.

Or might be even better to have some wrapper function/macro on top of rx_burst():
rx_burst_profle(...) or so.
Then user can either replace rx_burst() with rx_burst_profile() by conditional compilation,
or just call rx_burst_profile() directly.

Konstantin

> 
> 
> > +	/* See rte_eth_itt.h to find comments on code below. */
> > +	if (unlikely(nb_rx == 0)) {
> > +		if (!itt_aux_data[port_id].queue_is_wasting_iters[queue_id]) {
> > +			__itt_task_begin(
> > +				itt_aux_data[port_id].wasted_iter_domains[queue_id],
> > +				__itt_null, __itt_null,
> > +				itt_aux_data[port_id].wasted_iter_handles[queue_id]);
> > +			itt_aux_data[port_id].queue_is_wasting_iters[queue_id] = 1;
> > +		}
> > +	} else {
> > +		if (unlikely(itt_aux_data[port_id].queue_is_wasting_iters[queue_id])) {
> > +			__itt_task_end(
> > +				itt_aux_data[port_id].wasted_iter_domains[queue_id]);
> > +			itt_aux_data[port_id].queue_is_wasting_iters[queue_id] = 0;
> > +		}
> > +	}
> > +#endif
> > +
> >  #ifdef RTE_ETHDEV_RXTX_CALLBACKS
> >  	struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];
> >
> > --
> > 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.
> >
  

Patch

diff --git a/config/common_base b/config/common_base
index f6aafd1..60d8b63 100644
--- a/config/common_base
+++ b/config/common_base
@@ -135,6 +135,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_WASTED_RX_ITERATIONS=n
 
 #
 # Turn off Tx preparation stage
diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
index 93fdde1..c10153a 100644
--- a/lib/librte_ether/Makefile
+++ b/lib/librte_ether/Makefile
@@ -56,5 +56,6 @@  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_WASTED_RX_ITERATIONS}-include += rte_eth_itt.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ether/rte_eth_itt.h b/lib/librte_ether/rte_eth_itt.h
new file mode 100644
index 0000000..e661782
--- /dev/null
+++ b/lib/librte_ether/rte_eth_itt.h
@@ -0,0 +1,102 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_ETH_ITT_H_
+#define _RTE_ETH_ITT_H_
+
+#include <ittnotify.h>
+#include <rte_config.h>
+
+#define ITT_MAX_NAME_LEN (100)
+
+/**
+ * Auxiliary ITT structure belonging to port and using to:
+ *   -  track queue state to determine whether it is wasting loop iterations
+ *   -  begin or end ITT task using task domain and name
+ */
+struct rte_eth_itt_aux_data {
+	/**
+	 * ITT domains for each queue.
+	 */
+	__itt_domain *wasted_iter_domains[RTE_MAX_QUEUES_PER_PORT];
+	/**
+	 * ITT task names for each queue.
+	 */
+	__itt_string_handle *wasted_iter_handles[RTE_MAX_QUEUES_PER_PORT];
+	/**
+	 * Flags indicating the queues state. Possible values:
+	 * 1 - queue is wasting iterations, 0 - otherwise.
+	 */
+	uint8_t queue_is_wasting_iters[RTE_MAX_QUEUES_PER_PORT];
+};
+
+/**
+ * The pool of *rte_eth_itt_aux_data* structures.
+ */
+struct rte_eth_itt_aux_data itt_aux_data[RTE_MAX_ETHPORTS];
+
+/**
+ * Initialization of rte_eth_itt_aux_data for a given port.
+ * This function must be invoked when ethernet device is being configured.
+ * Result will be stored in the global array *itt_aux_data*.
+ *
+ * @param port_id
+ *  The port identifier of the Ethernet device.
+ * @param port_name
+ *  The name of the Ethernet device.
+ * @param queue_num
+ *  The number of queues on specified port.
+ */
+static inline void
+rte_eth_init_itt(uint8_t port_id, char *port_name, uint8_t queue_num) {
+	uint16_t q_id;
+	for (q_id = 0; q_id < queue_num; ++q_id) {
+		char domain_name[ITT_MAX_NAME_LEN];
+		snprintf(domain_name, sizeof(domain_name),
+			"RXBurst.WastedIterations.Port_%s.Queue_%d",
+			port_name, q_id);
+		itt_aux_data[port_id].wasted_iter_domains[q_id]
+			= __itt_domain_create(domain_name);
+
+		char task_name[ITT_MAX_NAME_LEN];
+		snprintf(task_name, sizeof(task_name),
+			"port id: %d; queue id: %d",
+			port_id, q_id);
+		itt_aux_data[port_id].wasted_iter_handles[q_id]
+			= __itt_string_handle_create(task_name);
+
+		itt_aux_data[port_id].queue_is_wasting_iters[q_id] = 0;
+	}
+}
+
+#endif
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 81a45c0..ebaaf99 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -818,6 +818,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_WASTED_RX_ITERATIONS
+	/* See rte_eth_itt.h to find comments on code below. */
+	rte_eth_init_itt(port_id, dev->data->name, nb_rx_q);
+#endif
+
 	return 0;
 }
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f6e6c74..ee7cc42 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -186,6 +186,10 @@  extern "C" {
 #include "rte_eth_ctrl.h"
 #include "rte_dev_info.h"
 
+#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS
+#include "rte_eth_itt.h"
+#endif
+
 struct rte_mbuf;
 
 /**
@@ -2710,6 +2714,25 @@  rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id,
 	int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
 			rx_pkts, nb_pkts);
 
+#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS
+	/* See rte_eth_itt.h to find comments on code below. */
+	if (unlikely(nb_rx == 0)) {
+		if (!itt_aux_data[port_id].queue_is_wasting_iters[queue_id]) {
+			__itt_task_begin(
+				itt_aux_data[port_id].wasted_iter_domains[queue_id],
+				__itt_null, __itt_null,
+				itt_aux_data[port_id].wasted_iter_handles[queue_id]);
+			itt_aux_data[port_id].queue_is_wasting_iters[queue_id] = 1;
+		}
+	} else {
+		if (unlikely(itt_aux_data[port_id].queue_is_wasting_iters[queue_id])) {
+			__itt_task_end(
+				itt_aux_data[port_id].wasted_iter_domains[queue_id]);
+			itt_aux_data[port_id].queue_is_wasting_iters[queue_id] = 0;
+		}
+	}
+#endif
+
 #ifdef RTE_ETHDEV_RXTX_CALLBACKS
 	struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id];