Hi Gaetan, > -----Original Message----- > From: Gaëtan Rivet [mailto:gaetan.ri...@6wind.com] > Sent: Tuesday, August 28, 2018 12:47 PM > To: Power, Ciara <ciara.po...@intel.com> > Cc: Van Haaren, Harry <harry.van.haa...@intel.com>; Archbold, Brian > <brian.archb...@intel.com>; Kenny, Emma <emma.ke...@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.po...@intel.com> > > Signed-off-by: Brian Archbold <brian.archb...@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
