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 8E343A0588; Wed, 1 Apr 2020 18:16:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CFDEB1BF26; Wed, 1 Apr 2020 18:16:15 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id DE4641BEE1 for ; Wed, 1 Apr 2020 18:16:13 +0200 (CEST) IronPort-SDR: RlH+IJ6N5DCmiVK7n6LZHYFKOm1pJtppQhoDSrzY1l2OxzCb9g9E6zdcbGeh0RwnEkRPtGCZMX DoXsGOFrB1AA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Apr 2020 09:16:12 -0700 IronPort-SDR: MPPQb6wajLOEbH5Jqulfwi1ZboOKHFjkQXgF6srb6kychUPxSRt4YITDyvN5GZw4r2c2V+vn2u gvwYIpoKy9JA== X-IronPort-AV: E=Sophos;i="5.72,332,1580803200"; d="scan'208";a="249506241" Received: from bricha3-mobl.ger.corp.intel.com ([10.212.65.4]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 01 Apr 2020 09:16:11 -0700 Date: Wed, 1 Apr 2020 17:16:07 +0100 From: Bruce Richardson To: David Marchand Cc: Ciara Power , Kevin Laatz , dev , "Pattan, Reshma" Message-ID: <20200401161607.GA152@bricha3-MOBL.ger.corp.intel.com> References: <20200319171907.60891-1-ciara.power@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH 00/12] update and simplify 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Apr 01, 2020 at 05:42:21PM +0200, David Marchand wrote: > Hello, > > > On Thu, Mar 19, 2020 at 6:35 PM Ciara Power wrote: > > > > This patchset extensively reworks the telemetry library adding new > > functionality and simplifying much of the existing code, while > > maintaining backward compatibility. > > > > This work is based on the previously sent RFC for a "process info" > > library: https://patchwork.dpdk.org/project/dpdk/list/?series=7741 > > However, rather than creating a new library, this patchset takes > > that work and merges it into the existing telemetry library, as > > mentioned above. > > > > The telemetry library as shipped in 19.11 is based upon the metrics > > library and outputs all statistics based on that as a source. However, > > this limits the telemetry output to only port-level statistics > > information, rather than allowing it to be used as a general scheme for > > telemetry information across all DPDK libraries. > > > > With this patchset applied, rather than the telemetry library being > > responsible for pulling ethdev stats and pushing them into the metrics > > library for retrieval later, each library e.g. ethdev, rawdev, and even > > the metrics library itself (for backwards compatiblity) now handle their > > own stats. Any library or app can register a callback function with > > telemetry, which will be called if requested by the client connected via > > the telemetry socket. The callback function in the library/app then > > formats its stats, or other data, into a JSON string, and returns it to > > telemetry to be sent to the client. > > > > To maintain backward compatibility, e.g. to allow the dpdk telemetry > > collectd plugin to continue to work, some of the existing telemetry > > code is kept, but is moved into the metrics library, and callbacks are > > registered with telemetry for the legacy commands that were supported > > previously. > > > > The new version of the library, apart from the legacy interface support > > for backward compatibility, does not have an external dependency on the > > Jansson library, allowing the library to be enabled by default. > > > > Note: In this version of the patchset, telemetry output is provided by > > the ethdev, rawdev and eal libraries, but this may be expanded further > > in later versions which are planned ahead of the merge deadline for > > 20.05 > > This patchset does not apply on current master. > Could you rebase it? > > CI reported a build failure for Windows, please have a look. > Yep, we spotted that and will fix in V2 (which naturally will also be rebased). > Is there a reason to keep a separate telemetry library rather than > integrate this framework into EAL? > No reason this could not be done, however, since telemetry library is already separate, and EAL is already pretty crowded, I think keeping this separate might lead to easier maintenance. However, if people generally prefer it just merged into EAL, that can be done also. > This series removes the only user of the experimental rte_option API, > which can be removed afaiu. > Yep, that's something we spotted and intended to flag for discussion on the V2 patchset. My slight concern would be that having extra args for particular additional libraries is something we might want in the future, so we should think carefully before removing it. However, since overall I like shorter code, I'd personally vote in favour of removal. :-) Regards, /Bruce