From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH v2 00/10] introduce telemetry library Date: Sat, 6 Oct 2018 00:05:09 +0200 Message-ID: <20181005220509.lkxhoaamra7legba@bidouze.vm.6wind.com> References: <1535026093-101872-1-git-send-email-ciara.power@intel.com> <20181003173612.67101-1-kevin.laatz@intel.com> <4034734.dNxXkqf0MS@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: "Van Haaren, Harry" , "Laatz, Kevin" , "dev@dpdk.org" , "stephen@networkplumber.org" , "shreyansh.jain@nxp.com" , "Richardson, Bruce" To: Thomas Monjalon Return-path: Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id F1C9E1B184 for ; Sat, 6 Oct 2018 00:05:28 +0200 (CEST) Received: by mail-wm1-f68.google.com with SMTP id 185-v6so3181722wmt.2 for ; Fri, 05 Oct 2018 15:05:28 -0700 (PDT) Content-Disposition: inline In-Reply-To: <4034734.dNxXkqf0MS@xps> 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 Thu, Oct 04, 2018 at 05:53:53PM +0200, Thomas Monjalon wrote: > 04/10/2018 15:25, Van Haaren, Harry: > > From: Van Haaren, Harry > > > From: Laatz, Kevin > > > > > > > > This patchset introduces a Telemetry library for DPDK Service Assurance. > > > > This library provides an easy way to query DPDK Ethdev metrics. > > > > > > > > > > > > > Note: We are aware that the --telemetry flag is not working for meson > > > > builds, we are working on it for a future patch. Despite opterr being set > > > > to 0, --telemetry said to be 'unrecognized' as a startup print. This is a > > > > cosmetic issue and will also be addressed. > > > > > > > > --- > > > > v2: > > > > - Reworked telemetry as part of EAL instead of using vdev (Gaetan) > > > > - Refactored rte_telemetry_command (Gaetan) > > > > - Added MAINTAINERS file entry (Stephen) > > > > - Updated docs to reflect vdev to eal rework > > > > - Removed collectd patch from patchset (Thomas) > > > > - General code clean up from v1 feedback > > > > > > > > > Hi Gaetan, Thomas, Stephen and Shreyansh! > > > > > > > > > goto TL_DR; // if time is short :) > > > > > > > > > In this v2 patchset, we've reworked the Telemetry to no longer use the vdev > > > infrastructure, instead having EAL enable it directly. This was requested as > > > feedback to the v1 patchset. I'll detail the approach below, and highlight > > > some issues we identified while implementing it. > > > > > > Currently, EAL does not depend on any "DPDK" libraries (ignore kvargs etc > > > for a minute). > > > Telemetry is a DPDK library, so it depends on EAL. In order to have EAL > > > initialize > > > Telemetry, it must depend on it - this causes a circular dependency. > > > > > > This patchset resolves that circular dependency by using a "weak" symbol for > > > telemetry init, and then the "strong" version of telemetry init will replace > > > it when the library is compiled in. > > > > Correction: we attempted this approach - but ended up adding a TAILQ of library > > initializers functions to EAL, which was then iterated during rte_eal_init(). > > This also resolved the circular-dependency, but the same linking issue as > > described below still exists. > > > > So - the same question still stands - what is the best solution for 18.11? > > > > > > > Although this *technically* works, it > > > requires > > > that applications *LINK* against Telemetry library explicitly - as EAL won't > > > pull > > > in the Telemetry .so automatically... This means application-level build- > > > system > > > changes to enable --telemetry on the DPDK EAL command line. > > > > > > Given the complexity in enabling EAL to handle the Telemetry init() and its > > > dependencies, I'd like to ask you folks for input on how to better solve > > > this? > > First, the telemetry feature must be enabled via a public function (API). > The application can decide to enable the feature at any time, right? > If the application wants to enable the feature at initialization > (and considers user input from the command line), > then the init function has a dependency on telemetry. > Your dependency concern is that the init function (which is high level) > is in EAL (which is the lowest layer in DPDK). > > I think the command line should not be managed directly by EAL. > My suggestion in last summit was to move this code in a different library. > We should also move the init function(s) to a new high level library. Part of the proposed solution here is to add a thin layer in EAL to register new parameters. I think this could kickstart what you want to do. -- Gaëtan Rivet 6WIND