Re: [dpdk-dev] [PATCH v5 00/33] DPDK Trace support

[Thomas Monjalon <thomas@monjalon.net>]

15/04/2020 15:26, David Marchand:
> - What do you think of splitting the API in two headers, thinking
> about who will use them?
> * rte_trace.h (rte_trace_ prefix for all functions/macros/types) for
> users of the trace framework that want to
>  * get the status of the whole trace subsystem,
>  * enable/disable tracepoints by pattern/regexp,
>  * dump the current events,
> * rte_tracepoint.h (rte_tracepoint_ prefix for all
> functions/macros/types) for developers that want to add tracepoints to
> their code

I like this idea.


> - Having functions "is_disabled" has little value when a "is_enabled"
> counterpart exists.

Yes, which one do we choose? is_enabled?


> - What is the value of having a _public_ rte_trace_is_invalid() ?
> A final user would need to lookup by name to get a trace descriptor
> and we should hope that such a lookup returns a valid descriptor :-).
> A developer would already have the descriptor hook point in his own
> code: by construction, if the tracepoint is registered, then the
> descriptor is valid, else, it is unknown.
> 
> 
> - I did not get why we put the trace descriptors in a specific elf
> section, can you explain the benefits?
> 
> 
> - I can see no protection on the tracepoint list. Could we have issues
> with control/application threads that dpdk does not control, dynamic
> loading of libraries.. ?
> 
> 
> - Following comment on v4 and the removal of the mode per tracepoint
> api, don't we need to put the current select mode in each tracepoint
> descriptor when registering a trace point ?