> From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Monday, 3 July 2023 09.20 > > 03/07/2023 05:58, fengchengwen: > > > > On 2023/2/20 21:05, Thomas Monjalon wrote: > > > 17/02/2023 10:44, fengchengwen: > > >> On 2023/2/16 20:54, Bruce Richardson wrote: > > >>> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote: > > >>>> On 2023/2/16 20:06, Ferruh Yigit wrote: > > >>>>> On 2/16/2023 11:53 AM, fengchengwen wrote: > > >>>>>> On 2023/2/15 11:19, Dongdong Liu wrote: > > >>>>>>> Hi Chengwen > > >>>>>>> > > >>>>>>> On 2023/2/9 10:32, Chengwen Feng wrote: > > >>>>>>>> The xstats reset is useful for debugging, so add it to the > ethdev > > >>>>>>>> telemetry command lists. > > >>>>>>>> > > >>>>>>>> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com> > > >>>>>>> This patch looks good, so > > >>>>>>> Reviewed-by: Dongdong Liu <liudongdo...@huawei.com> > > >>>>>>> > > >>>>>>> A minior question > > >>>>>>> Do we need to support stats reset ? > > >>>>>> > > >>>>>> Stats is contained by xstats, and future direction I think is > xstats. > > >>>>>> So I think we don't need support stats reset. > > >>>>>> > > >>>>> > > >>>>> I have similar question with Dongdong, readonly values are safe > for > > >>>>> telemetry, but modifying data can be more tricky since we don't > have > > >>>>> locking in ethdev APIs, this can cause concurrency issues. > > >>>> > > >>>> Yes, it indeed has concurrency issues. > > >>>> > > >>>>> > > >>>>> Overall do we want telemetry go that way and become something > that > > >>>>> alters ethdev data/config? > > >>>> > > >>>> There are at least two part of data: config and status. > > >>>> For stats (which belong status data) could help for debugging, I > think it's acceptable. > > >>>> > > >>>> As for concurrency issues. People should know what to do and when > to do, just like > > >>>> the don't invoke config API (e.g. dev_configure/dev_start/...) > concurrency. > > >>>> > > >>> While this is probably ok for now, I think in next release we > should look > > >>> to add some sort of support for locking for destructive ops in a > future > > >>> release. For example, we could: > > >>> > > >>> 1. Add support for marking a callback as "destructive" and only > allow it to > > >>> be called if only one connection is present or > > >>> > > >>> 2. Make it possible for callbacks to query the number of > connections so > > >>> that the callback itself is non-destructive in more than one > connection is > > >>> open. > > >>> > > >>> [Both of these will require locking support so that new > connections aren't > > >>> openned when the callback is in-flight!] > > >> > > >> Except telemetry, the application may have other console could > execute DPDK API. > > >> So I think trying to keep it simple, it's up to the user to invoke. > > > > > > No, the user should not be responsible for concurrency issues. > > > We can ask the app developper to take care, > > > but not to the user who has no control on what happens in the app. > > > > > > On a more general note, I feel the expansion of telemetry is not > controlled enough. > > > I would like to stop on adding more telemetry until we have a clear > guideline > > > about what is telemetry for and how to use it. > > > > Hi Thomas, > > > > Should this be discussed on TB? > > What would be your question exactly?
A general comment about telemetry: If an application exposes telemetry through an end user facing API, e.g. http(s) REST, it would be nice if non-read-only telemetry paths are easy to identify by following some DPDK standard convention, so the application does not need to manually maintain an allow-list of read-only paths. Bruce's documentation about trace/log/telemetry/dump might also need to be updated regarding non-read-only telemetry actions.
