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

[Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>]

W dniu 09.07.2020 o 16:09, Bruce Richardson pisze:
> 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.
maybe also with net
>
> So overall, I'd suggest it's not a problem, but if you want modify the lib
> one.

If it's not a problem, I'll leave it as it is for now.

>
> /Bruce
>
-- 
Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com