Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry infrastructure

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

Hi Gaetan,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Tuesday, August 28, 2018 12:47 PM
> To: Power, Ciara <ciara.power@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Archbold, Brian
> <brian.archbold@intel.com>; Kenny, Emma <emma.kenny@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/11] telemetry: initial telemetry
> infrastructure
> 
> Hi Ciara,
> 
> On Thu, Aug 23, 2018 at 01:08:03PM +0100, Ciara Power wrote:
> > This patch adds the infrastructure and initial code for the
> > telemetry library.
> >
> > A virtual device is used for telemetry, which is included in
> > this patch. To use telemetry, a command-line argument must be
> > used - "--vdev=telemetry".
> >
> 
> Why use a virtual device?
> 
> It seems that you are only using the device semantic as a way to carry
> around a tag telling the DPDK framework to init your library once it has
> finished its initialization.
> 
> I guess you wanted to avoid having to add the call to rte_telemetry_init
> to all applications. In the absence of a proper EAL option framework,
> you can workaround by adding a --telemetry EAL parameter, setting a flag
> on, and checking this flag from librte_telemetry, within a routine
> declared with RTE_INIT_PRIO.

I suppose that an EAL --flag could work too, it would mean that EAL would
depend on this library. The --vdev trick keeps the library standalone.

I don't have a strong opinion either way. :)


> I only see afterward the selftest being triggered via kvargs. I haven't
> yet looked at the testing done, but if it is only unit test, the "test"
> app would be better suited. If it is integration testing to verify the
> behavior of the library with other PMDs, you probably need specific
> context, thus selftest being insufficient on its own and useless for
> other users.

Correct, self tests are triggered by kvargs. This same model is used
in eg: eventdev PMDs to run selftests, where the tests are pretty complex
and specific to the device under test.

Again, I don't have a strong opinion but I don't see any issue with it
being included in the vdev / telemetry library. We could write a shim
test that the "official" test binary runs the telemetry tests if that is
your concern?


> > Control threads are used to get CPU cycles for telemetry, which
> > are configured in this patch also.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > Signed-off-by: Brian Archbold <brian.archbold@intel.com>
> 
> Regards,
> --
> Gaëtan Rivet
> 6WIND

Thanks for review, and there's a lightning talk at Userspace so please
do provide input there too :) -Harry