Re: [dpdk-dev] [PATCH v2] pipeline: add statistics for librte_pipeline ports and tables

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-05-21 13:33, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2015-05-20 23:41, Dumitrescu, Cristian:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > I think that counting stats is a kind of logging.
> > > > Some stats are always counted (see drivers) and you want to use these
> > > > ones only for debugging (after rebuilding DPDK with some debug options).
> > > > So I suggest, as second solution, to check CONFIG_RTE_LOG_LEVEL is at
> > > > debug level instead of having one option per module.
> > > > It would be implemented with "#if RTE_LOG_LEVEL == RTE_LOG_DEBUG"
> > > > in RTE_PIPELINE_STATS_ADD.
> > >
> > > The problem I see with this approach is that you cannot turn off debug
> > > messages while still having the statistics collected.  We should allow
> > > people to collects the stats without getting the debug messages. How do
> > > we do this?
> > 
> > By setting build-time log level to debug, you would enable stats and debug
> > messages. Then you can adjust the log messages level with the run-time
> > option --log-level.
> 
> This is a really clever trick! I have to say it took me some time to
> make sure I got it right :)
> So when RTE_LOG_LEVEL (build time option) is set to DEBUG (i.e. 8),
> then we get both the stats and the debug messages, but then we can
> adjust the level of debug messages at run-time through the --log-level
> EAL option.
> I can work with this option, thank you!
> 
> There is one more simple proposal that came to my mind late last night:
> how about single config file option RTE_COLLECT_STATS=y/n that should
> be used by all the libs across the board to indicate whether stats
> should be enabled or not?

Stats in drivers must be always collected because they are not considered
as debug information (until now).
So I think a global option RTE_COLLECT_STATS would be confusing.

> This is logically very similar to the solution above, as they both look
> at a single option in the config file, but maybe it is also more
> intuitive for people.
> 
> I will go with your choice. Which one do you pick?

I prefer we choose on a per-lib basis which information are considered
for debug or not. But it's only my opinion.
So if nobody disagrees, I'd prefer going with RTE_LOG_LEVEL.