From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C9BF1A04B0; Fri, 4 Dec 2020 11:21:09 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A46F35681; Fri, 4 Dec 2020 11:21:08 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 5136A37AF for ; Fri, 4 Dec 2020 11:21:06 +0100 (CET) IronPort-SDR: 0B4n3novylpVXbEUe/rSd66E39GjZMttIVgVOAbAVS7Hgs7NQQ0W/RpXhxMEmd36ana0icsCvA yAraHQBEflTA== X-IronPort-AV: E=McAfee;i="6000,8403,9824"; a="258069638" X-IronPort-AV: E=Sophos;i="5.78,392,1599548400"; d="scan'208";a="258069638" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Dec 2020 02:21:05 -0800 IronPort-SDR: iO8dHLlU5ee4Kti94NDb/SHR7cX/htBbd70CmHsue5GpyKETcMATST78iWdzEV6gZ/G6BTLRq7 61VllT/0Pz0w== X-IronPort-AV: E=Sophos;i="5.78,392,1599548400"; d="scan'208";a="336327830" Received: from dhunt5-mobl5.ger.corp.intel.com (HELO [10.213.200.59]) ([10.213.200.59]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Dec 2020 02:21:04 -0800 To: Hideyuki Yamashita Cc: dev@dpdk.org References: <20201204075109.14694-1-yamashita.hideyuki@ntt-tx.co.jp> From: David Hunt Message-ID: <2a9a170b-c351-e3f2-36b5-6db9d4c29da5@intel.com> Date: Fri, 4 Dec 2020 10:20:49 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: <20201204075109.14694-1-yamashita.hideyuki@ntt-tx.co.jp> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB Subject: Re: [dpdk-dev] [PATCH 0/5] add apistats function 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 4/12/2020 7:51 AM, Hideyuki Yamashita wrote: > In general, DPDK application consumes CPU usage because it polls > incoming packets using rx_burst API in infinite loop. > This makes difficult to estimate how much CPU usage is really > used to send/receive packets by the DPDK application. > > For example, even if no incoming packets arriving, CPU usage > looks nearly 100% when observed by top command. > > It is beneficial if developers can observe real CPU usage of the > DPDK application. > Such information can be exported to monitoring application like > prometheus/graphana and shows CPU usage graphically. > > To achieve above, this patch set provides apistats functionality. > apistats provides the followiing two counters for each lcore. > - rx_burst_counts[RTE_MAX_LCORE] > - tx_burst_counts[RTE_MAX_LCORE] > Those accumulates rx_burst/tx_burst counts since the application starts. > > By using those values, developers can roughly estimate CPU usage. > Let us assume a DPDK application is simply forwarding packets. > It calls tx_burst only if it receive packets. > If rx_burst_counts=1000 and tx_burst_count=1000 during certain > period of time, one can assume CPU usage is 100%. > If rx_burst_counts=1000 and tx_burst_count=100 during certain > period of time, one can assume CPU usage is 10%. > Here we assumes that tx_burst_count equals counts which rx_burst function > really receives incoming packets. > > > This patch set provides the following. > - basic API counting functionality(apistats) into librte_ethdev > - add code to testpmd to accumulate counter information > - add code to proc-info to retrieve above mentioned counter information > - add description in proc-info document about --apistats parameter > - modify MAINTAINERS file for apistats.c and apistats.h > > Hideyuki Yamashita (5): > maintainers: update maintainers file for apistats > app/proc-info: add to use apistats > app/test-pmd: add to use apistats > docs: add description of apistats parameter into proc-info > librte_ethdev: add to use apistats > > MAINTAINERS | 3 ++ > app/proc-info/main.c | 46 +++++++++++++++++++++++ > app/test-pmd/testpmd.c | 4 ++ > doc/guides/tools/proc_info.rst | 10 ++++- > lib/librte_ethdev/meson.build | 6 ++- > lib/librte_ethdev/rte_apistats.c | 64 ++++++++++++++++++++++++++++++++ > lib/librte_ethdev/rte_apistats.h | 64 ++++++++++++++++++++++++++++++++ > lib/librte_ethdev/rte_ethdev.h | 7 ++++ > lib/librte_ethdev/version.map | 5 +++ > 9 files changed, 205 insertions(+), 4 deletions(-) > create mode 100644 lib/librte_ethdev/rte_apistats.c > create mode 100644 lib/librte_ethdev/rte_apistats.h Hi Hideyuki, I have a few questions on the patch set. How does this compare to the mechanism added to l3fwd-power which counts the number of empty, partial and full polls, and uses them to calculate busyness? We saw pretty good tracking of busyness using those metrics. I would be concerned that just looking at the numebr of rx_bursts and tx_bursts may be limited to only a few use-cases. The l3fwd-power example uses branchless increments to capture empty, non-empty, full, and non-full polls. Why not use the existing telemetry library to store the stats? It would be good if whatever metrics were counted were made available in a standard way, so that external entities such as collectd could pick them up, rather than having to parse the new struct. The l3fwd-power example registers the necessary new metrics, and exposes them through the telemetry library. And a comment on the patch set in general: The order of the patch set seems reversed. The earlier patch do not compile, because they depend on rte_apistats.h, which is introduced in the final patch. Each patch as it is applied needs to build successfully. Also, I would suggest a different name. rte_apistats seems very generic and could apply to anything. How about something like rte_ethdev_stats.h or rte_burst_stats.h? Rgds, Dave.