Re: [dpdk-dev] [PATCH] log: do not drop debug logs at compile time

[Olivier Matz <olivier.matz@6wind.com>]

Hi David,

On 10/04/2016 10:26 AM, David Marchand wrote:
> On Fri, Sep 16, 2016 at 9:43 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>> Today, all logs whose level is lower than INFO are dropped at
>> compile-time. This prevents from enabling debug logs at runtime using
>> --log-level=8.
>>
>> The rationale was to remove debug logs from the data path at
>> compile-time, avoiding a test at run-time.
>>
>> This patch changes the behavior of RTE_LOG() to avoid the compile-time
>> optimization, and introduces the RTE_LOG_DP() macro that has the same
>> behavior than the previous RTE_LOG(), for the rare cases where debug
>> logs are in the data path.
>>
>> So it is now possible to enable debug logs at run-time by just
>> specifying --log-level=8. Some drivers still have special compile-time
>> options to enable more debug log. Maintainers may consider to
>> remove/reduce them.
>>
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>> ---
>>  config/common_base                      |  1 +
>>  doc/guides/faq/faq.rst                  |  2 +-
>>  drivers/net/bnxt/bnxt_txr.c             |  2 +-
>>  drivers/net/nfp/nfp_net.c               |  8 +++---
>>  examples/distributor/main.c             |  4 +--
>>  examples/ipsec-secgw/esp.c              |  2 +-
>>  examples/ipsec-secgw/ipsec.c            |  4 +--
>>  examples/packet_ordering/main.c         |  6 ++--
>>  examples/quota_watermark/qw/main.c      |  2 +-
>>  examples/tep_termination/main.c         |  4 +--
>>  examples/vhost/main.c                   | 14 +++++-----
>>  examples/vhost_xen/main.c               | 20 +++++++-------
>>  lib/librte_eal/common/include/rte_log.h | 49 +++++++++++++++++++++------------
>>  13 files changed, 67 insertions(+), 51 deletions(-)
>>
>> diff --git a/config/common_base b/config/common_base
>> index 7830535..04b71e9 100644
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -89,6 +89,7 @@ CONFIG_RTE_MAX_MEMSEG=256
>>  CONFIG_RTE_MAX_MEMZONE=2560
>>  CONFIG_RTE_MAX_TAILQ=32
>>  CONFIG_RTE_LOG_LEVEL=RTE_LOG_INFO
>> +CONFIG_RTE_LOG_DP_LEVEL=RTE_LOG_INFO
>>  CONFIG_RTE_LOG_HISTORY=256
>>  CONFIG_RTE_LIBEAL_USE_HPET=n
>>  CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n
> 
> [snip]
> 
>> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
>> index 919563c..76b198f 100644
>> --- a/lib/librte_eal/common/include/rte_log.h
>> +++ b/lib/librte_eal/common/include/rte_log.h
> 
> [snip]
> 
>> @@ -266,6 +257,30 @@ int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
>>   *   - Negative on error.
>>   */
>>  #define RTE_LOG(l, t, ...)                                     \
>> +        rte_log(RTE_LOG_ ## l,                                 \
>> +                RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__)
>> +
>> +/**
>> + * Generates a log message for data path.
>> + *
>> + * Similar to RTE_LOG(), except that it is removed at compilation time
>> + * if the RTE_LOG_DP_LEVEL configuration option is lower than the log
>> + * level argument.
>> + *
>> + * @param l
>> + *   Log level. A value between EMERG (1) and DEBUG (8). The short name is
>> + *   expanded by the macro, so it cannot be an integer value.
>> + * @param t
>> + *   The log type, for example, EAL. The short name is expanded by the
>> + *   macro, so it cannot be an integer value.
>> + * @param ...
>> + *   The fmt string, as in printf(3), followed by the variable arguments
>> + *   required by the format.
>> + * @return
>> + *   - 0: Success.
>> + *   - Negative on error.
>> + */
>> +#define RTE_LOG_DP(l, t, ...)                                  \
>>         (void)((RTE_LOG_ ## l <= RTE_LOG_LEVEL) ?               \
>>          rte_log(RTE_LOG_ ## l,                                 \
>>                  RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) :     \
>> --
>> 2.8.1
> 
> Hum, I suppose RTE_LOG_DP should look at RTE_LOG_DP_LEVEL.
> 

That's correct, good catch :)
I'll send a v2 with that fix.


Olivier