Re: [dpdk-dev] [PATCH v2 00/10] introduce telemetry library

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

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.
> > > 
> > > <snip>
> > > 
> > > > 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