Re: [dpdk-dev] [PATCH v2 1/3] config: introduce global rte debug flag

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Jul 09, 2020 at 03:51:25PM +0200, Lukasz Wojciechowski wrote:
> 
> W dniu 26.06.2020 o 18:56, Bruce Richardson pisze:
> > On Wed, Apr 22, 2020 at 11:45:53PM +0200, Lukasz Wojciechowski wrote:
> >> A new boolean meson build flag is introduced with this patch: rte_debug.
> >> To enable this option add -Drte_debug=true to the meson configure command.
> >>
> >> By enabling this flag the globaly defined macro RTE_DEBUG becomes
> >> defined. It should be used for enabling debug code in all dpdk project
> >> components. Using this flag allows to make additional checks or tests
> >> and provide additional logs even in performance sensitive parts of code.
> >>
> >> The flag is disabled by default.
> >>
> >> Additionally for all enabled to be built libraries
> >> a RTE_DEBUG_{library name} is defined and assigned a value 1.
> >> This standardize names of library specific debug macros. Those can
> >> be used also without rte_debug option by using CFLAGS="-D..." build
> >> parameter.
> >>
> >> Code put inside RTE_DEBUG* #ifdef sections should be also put inside
> >> if (rte_log_can_log(...)) condition to allow run-time filtering of
> >> sanity checks, validations, logs and dumps when using rte_debug option.
> >>
> >> Libraries will be adjusted to the change in separate patches.
> >>
> >> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> >> Signed-off-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> > The drivers are missing debug flag options, which probably need to be
> > defined as <driver_type>_<driver_name> e.g. RTE_DEBUG_NET_I40E,
> > RTE_DEBUG_EVENT_SW, etc.
> I added also flags for drivers constructed as you suggested (see v3).
> It might however be a bit tricky to differentiate library debug flags 
> form drivers, e.g.
> RTE_DEBUG_MEMPOOL (library) vs RTE_DEBUG_MEMPOOL_DPAA (driver).
> 
> If there is a need the macro names can be changed by adding _LIB_ or _DRV_.
> What do you think?

I don't think DRV is needed for the drivers, though perhaps LIB (or RTE) could
be added for the libs. I actually think that mempool is the only case where
we might have any confusion, for other device classes the library names
tend to have "dev" on the end, e.g. cryptodev, rawdev etc.

So overall, I'd suggest it's not a problem, but if you want modify the lib
one.

/Bruce