Hi Gaetan, > -----Original Message----- > From: Gaëtan Rivet [mailto:[email protected]] > Sent: Tuesday, August 28, 2018 12:47 PM > To: Power, Ciara <[email protected]> > Cc: Van Haaren, Harry <[email protected]>; Archbold, Brian > <[email protected]>; Kenny, Emma <[email protected]>; [email protected] > 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 <[email protected]> > > Signed-off-by: Brian Archbold <[email protected]> > > Regards, > -- > Gaëtan Rivet > 6WIND Thanks for review, and there's a lightning talk at Userspace so please do provide input there too :) -Harry
