From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 784E05688 for ; Thu, 22 Jun 2017 11:42:41 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Jun 2017 02:42:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,372,1493708400"; d="scan'208";a="983900573" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by orsmga003.jf.intel.com with ESMTP; 22 Jun 2017 02:42:22 -0700 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.250]) by IRSMSX153.ger.corp.intel.com ([169.254.9.74]) with mapi id 14.03.0319.002; Thu, 22 Jun 2017 10:42:20 +0100 From: "Ananyev, Konstantin" To: "Kurakin, Ilia" , "dev@dpdk.org" CC: "Kurakin, Ilia" Thread-Topic: [dpdk-dev] [PATCH] ether: add support for vtune task tracing Thread-Index: AQHS6SA5j7YSbkO6oUiWTCm9YLvVXqIwon+w Date: Thu, 22 Jun 2017 09:42:20 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583FB0BD8E@IRSMSX109.ger.corp.intel.com> References: <1497892689-27494-1-git-send-email-ilia.kurakin@intel.com> In-Reply-To: <1497892689-27494-1-git-send-email-ilia.kurakin@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] ether: add support for vtune task tracing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Jun 2017 09:42:42 -0000 Hi Ilia, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of ilia.kurakin@intel.c= om > Sent: Monday, June 19, 2017 6:18 PM > To: dev@dpdk.org > Cc: Kurakin, Ilia > Subject: [dpdk-dev] [PATCH] ether: add support for vtune task tracing >=20 > From: Ilia Kurakin >=20 > The patch adds tracing of loop iterations that yielded no packets in a DP= DK > application. It is using ITT task API: > https://software.intel.com/en-us/node/544206 >=20 > We suppose the flow of using this tracing would assume the user has ITT l= ib > and header on his machine and re-build DPDK with additional make paramete= rs: >=20 > make EXTRA_CFLAGS=3D-I > EXTRA_LDLIBS=3D"-L -littnotify" There are few things that worry me with that patch: 1. We add new config variable and add extra dependency here. Usually we try not to do that without really compelling reason. 2. We pollute rte_ethdev with the code that has nothing to do with it major functionality. That makes me wonder why this vtune data collection has to be done inside rx_burst() function? Why it can't be done on the application layer, i.e. straight after rx_burs= t() is finished? Something like: n =3D rte_eth_rx_burst(port, queue, ....); itt_rx_collect_data(port, queue, n, ....); ? Or as alternative, user can install this vtune collection routine as rx callback function. Konstantin >=20 > Signed-off-by: Ilia Kurakin > --- > config/common_base | 1 + > lib/librte_ether/Makefile | 1 + > lib/librte_ether/rte_eth_itt.h | 69 ++++++++++++++++++++++++++++++++++++= ++++++ > lib/librte_ether/rte_ethdev.c | 7 +++++ > lib/librte_ether/rte_ethdev.h | 26 ++++++++++++++++ > 5 files changed, 104 insertions(+) > create mode 100644 lib/librte_ether/rte_eth_itt.h >=20 > 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=3D1024 > CONFIG_RTE_LIBRTE_IEEE1588=3Dn > CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=3D16 > CONFIG_RTE_ETHDEV_RXTX_CALLBACKS=3Dy > +CONFIG_RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS=3Dn >=20 > # > # 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 +=3D rte_eth_ctrl.h > SYMLINK-y-include +=3D rte_dev_info.h > SYMLINK-y-include +=3D rte_flow.h > SYMLINK-y-include +=3D rte_flow_driver.h > +SYMLINK-${CONFIG_RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS}-include +=3D rte= _eth_itt.h >=20 > include $(RTE_SDK)/mk/rte.lib.mk > diff --git a/lib/librte_ether/rte_eth_itt.h b/lib/librte_ether/rte_eth_it= t.h > new file mode 100644 > index 0000000..e7984fb > --- /dev/null > +++ b/lib/librte_ether/rte_eth_itt.h > @@ -0,0 +1,69 @@ > +#ifndef _RTE_ETH_ITT_H_ > +#define _RTE_ETH_ITT_H_ > + > +#include > +#include > + > +#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 iterat= ions > + * - begin or end ITT task using task domain and name > + */ > +struct rte_eth_itt_aux_data { > + /** > + * ITT domains for each queue. > + */ > + __itt_domain *wasted_iteration_itt_domains[RTE_MAX_QUEUES_PER_PORT]; > + /** > + * ITT task names for each queue. > + */ > + __itt_string_handle *wasted_iteration_itt_handles[RTE_MAX_QUEUES_PER_PO= RT]; > + /** > + * Flags indicating the queues state. Possible values: > + * 1 - queue is wasting iterations, 0 - otherwise. > + */ > + uint8_t queue_is_wasting_iterations[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 configure= d. > + * 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 =3D 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_iteration_itt_domains[q_id] > + =3D __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_iteration_itt_handles[q_id] > + =3D __itt_string_handle_create(task_name); > + > + itt_aux_data[port_id].queue_is_wasting_iterations[q_id] =3D 0; > + } > +} > + > +#endif > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.= c > index 81a45c0..9e5ac01 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -818,6 +818,13 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_r= x_q, uint16_t nb_tx_q, > return diag; > } >=20 > +#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; > } >=20 > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.= h > index f6e6c74..4ba90d2 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" >=20 > +#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS > +#include "rte_eth_itt.h" > +#endif > + > struct rte_mbuf; >=20 > /** > @@ -2710,6 +2714,28 @@ rte_eth_rx_burst(uint8_t port_id, uint16_t queue_i= d, > int16_t nb_rx =3D (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], > rx_pkts, nb_pkts); >=20 > +#ifdef RTE_ETHDEV_TRACE_WASTED_RX_ITERATIONS > + /** > + * See rte_eth_itt.h to find comments on code below. > + */ > + if (unlikely(nb_rx =3D=3D 0)) { > + if (!itt_aux_data[port_id].queue_is_wasting_iterations[queue_id]) { > + __itt_task_begin( > + itt_aux_data[port_id].wasted_iteration_itt_domains[queue_id], > + __itt_null, __itt_null, > + itt_aux_data[port_id].wasted_iteration_itt_handles[queue_id]); > + itt_aux_data[port_id].queue_is_wasting_iterations[queue_id] =3D 1; > + } > + } > + else { > + if (unlikely(itt_aux_data[port_id].queue_is_wasting_iterations[queue_i= d])) { > + __itt_task_end( > + itt_aux_data[port_id].wasted_iteration_itt_domains[queue_id]); > + itt_aux_data[port_id].queue_is_wasting_iterations[queue_id] =3D 0; > + } > + } > +#endif > + > #ifdef RTE_ETHDEV_RXTX_CALLBACKS > struct rte_eth_rxtx_callback *cb =3D dev->post_rx_burst_cbs[queue_id]; >=20 > -- > 2.7.4 >=20 >=20 > -------------------------------------------------------------------- > Joint Stock Company Intel A/O > Registered legal address: Krylatsky Hills Business Park, > 17 Krylatskaya Str., Bldg 4, Moscow 121614, > Russian Federation >=20 > 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.