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

[Thomas Monjalon <thomas@monjalon.net>]

09/04/2020 17:36, Jerin Jacob:
> On Thu, Apr 9, 2020 at 7:30 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 03/04/2020 17:36, jerinj@marvell.com:
> > > Features:
> > > ~~~~~~~~~
> > > - APIs and Features are similar to rte_log dynamic framework
> > > API(expect log prints on stdout vs it dumps on trace file)
> >
> > A log can print to syslog as well.
> >
> > As discussed somewhere else, please do not introduce global level
> > in rte_trace. I think it is useless. If we need to change the level
> > of all trace types, we can just use a wildcard (globbing or regexp).
> 
> 
> Currently, In the log library, when EAL command-line argument
> specifies the "--log-level=val" it, will call
> the  rte_log_global_level_set(val)
> 
> Is the suggestion to make rte_log_global_level_set() as an internal
> EAL API or remove that feature?

Completely remove global level.
Why would we need 2 levels of level?

> If we remove, then I dont know, how we can map --log-level or
> --trace-level EAL command-line argument.

They are setting levels with regex or globbing.
--log-level supports 3 syntaxes today:
	- int (global level)
	- globbing:int
	- regex,int
I propose to drop the first syntax because it became useless
now that we have powerful globbing and regex.


> > And about wording, "pattern" is too vague and should be replaced
> > with "globbing".
> 
> Currently, we are using the word "shell pattern" in log and trace.
> According to man 3 fnmatch, both being used.
> I can send a patch for changing the "shell pattern" to  "glob" in
> rte_log.h and update the trace if needed.
> Let me know?. IMO, Both has to use same word.

Yes I think globbing is more accurate than "shell pattern",
because we can use regex pattern in shell as well.


> > > - No specific limit on the events. A string-based event like rte_log
> > > for pattern matching
> >
> > Would it be possible to replace rte_log with rte_trace?
> 
> Yes, I kept public API similar to log to have that use case in mind.

OK I think it would be good to have only one system.


> > > - Dynamic enable/disable support.
> >
> > How dynamic it is? Can we start/stop a trace after starting DPDK?
> 
> Yes, we have fine control enabling and disabling a specific or group of events.
> See rte_trace_enable(), rte_trace_disable() and rte_trace_pattern()
> 
> > I think we need a control channel for this.
> > I propose to introduce a general control channel in DPDK.
> 
> Yes. Once we have a general control channel in DPDK. We can hook
> rte_trace_enable() and friends
> in the message handler.

That would be a nice feature.


> > > - Instructmention overhead is ~1 cycle. i.e cost of adding the code
> >
> > Nice
> >
> >
> > > wth out using trace feature.
> > > - Timestamp support for all the events using DPDK rte_rtdsc
> >
> > Could we use other timestamp sources, like the mbuf timestamp
> > for Rx traces?
> 
> Trace will be running at the global wall clock. Rx specific tracepoint
> can add mbuf->timestamp as FIELD in the future if needed.

OK makes sense.

Note: I prefer rte_get_timer_cycles() because rdtsc is an x86 instruction.


> > > - No dependency on another library. Clean room native implementation of
> > >   CTF.
> >
> > No benefit of using an external CTF library maintained somewhere else?
> 
> See performance data comparison with LTTng in the cover letter.
> 
> We discussed this enough in the mailing list on RFC and decide to use
> Faster trace support for Fastpath use case and avoid any external library to EAL
> (We need tracepoints in the EAL too for tracing).

I can understand the performance reason.
But if another library can provide the same performance,
I don't see any reason not depending on it.
If the dependency is not found, the feature can be disabled.