From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 9B48C1B4DF for ; Tue, 9 Oct 2018 13:41:14 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 2C19221E44; Tue, 9 Oct 2018 07:41:14 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Tue, 09 Oct 2018 07:41:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=r2KU9HZyrd7Fx6cXiPcBjnlYJAsDkzquYJ1O6ZcxwCQ=; b=HyZtjMWwIo6X DFT7/7VR1SVghyc/gDI340GCcVZy8flexAiqFgaO0WT++C+JKFLa0FknGt9SpjiX Hw+7+YMmHU7cDhi1JqD2IGHeIh2M9rnqs8AkHEvL3ZmT3PkPFW3ErYzkxo7clZH7 s7w6BL+iqm1TYP47B7NtRkRmC7nJyH8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=r2KU9HZyrd7Fx6cXiPcBjnlYJAsDkzquYJ1O6Zcxw CQ=; b=wIK4Z6DZGRKt/FNccODJcsDFYgj9dHHt8tGIhaLnFK7F4fIFWJ1/+xuRQ Nr5SuDOATfBv1aKvRfhq1kZkevS/pRoCGS7rCV0QXZsh+eaFH5pBpm5kVEFLU8FD 8v4Jc31zqV38KVi3p0nIs74VU/IPQUH+nHiIGAjNMBv2Pq+8tCSrbdX6amf7K9Za ppE9ftQz9zr/EPke3qj2ImGGChFrAj+BSW6oBKNz2t6YDrDCxlhVFH5pMOAKdEV1 u0Cp7jRGhmw++JKbLcIRR6e6YQLyM16AhHxP8I5PJjr0ikV/BqzHdFlnDx+1WDZR 1zC5/bRzgkpzqrTi2IGRPNzp94D4w== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 84F77102EA; Tue, 9 Oct 2018 07:41:11 -0400 (EDT) From: Thomas Monjalon To: "Van Haaren, Harry" Cc: "Laatz, Kevin" , "dev@dpdk.org" , "stephen@networkplumber.org" , "gaetan.rivet@6wind.com" , "shreyansh.jain@nxp.com" , "Richardson, Bruce" Date: Tue, 09 Oct 2018 13:41:10 +0200 Message-ID: <2030383.RfJjvT9l4E@xps> In-Reply-To: References: <1535026093-101872-1-git-send-email-ciara.power@intel.com> <4034734.dNxXkqf0MS@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 00/10] introduce telemetry library 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: Tue, 09 Oct 2018 11:41:15 -0000 09/10/2018 12:33, Van Haaren, Harry: > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > 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). > > Yes, and this has been resolved by allowing components to register > with EAL to have their _init() function called later. V3 coming up > with this approach, it seems to cover the required use-cases. > > > > 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. > > > > This is my proposal to solve cyclic dependency: move rte_eal_init in a lib > > which depends on everything. > > I have prototyped this approach, and it is not really clean. It means > splitting EAL into two halves, and due to meson library naming we have > to move all eal files to eal_impl or something, and then eal.so keeps rte_eal_init(). > > Removing functions from the .map files is also technically an ABI break, > at which point I didn't think it was the right solution. > > > > About the linking issue, I don't understand the problem. > > If you use the DPDK makefiles, rte.app.mk should manage it. > > If you use the DPDK meson, all libs are linked. > > If you use your own system, of course you need to add telemetry lib. > > Yes agreed, in practice it should be exactly like this. In reality > it can be harder to achieve the exact dependencies correctly with > both Static/Shared builds and constructors etc. > > I believe the current approach of registering an _init() function > will be acceptable, let's wait for v3 to hit the mailing list. I think it is not clean. We should really split EAL in two parts: - low level routines - high level init. About telemetry, you can find any workaround, but it must be temporary.