Re: [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 2/17/2023 2:45 AM, Chaoyong He wrote:
> Register the own RX/TX debug log level type, and get rid of the
> usage of RTE_LOGTYPE_*. Then we can control the log by a independent
> switch.
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> ---
>  drivers/net/nfp/nfp_logs.c | 10 ++++++++++
>  drivers/net/nfp/nfp_logs.h |  8 ++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/nfp/nfp_logs.c b/drivers/net/nfp/nfp_logs.c
> index 48c42fe53f..cd58bcee43 100644
> --- a/drivers/net/nfp/nfp_logs.c
> +++ b/drivers/net/nfp/nfp_logs.c
> @@ -5,6 +5,16 @@
>  
>  #include "nfp_logs.h"
>  
> +#include <rte_ethdev.h>
> +
>  RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE);
>  RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE);
>  RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE);
> +
> +#ifdef RTE_ETHDEV_DEBUG_RX
> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_rx, rx, DEBUG)
> +#endif
> +
> +#ifdef RTE_ETHDEV_DEBUG_TX
> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_tx, tx, DEBUG)
> +#endif
> diff --git a/drivers/net/nfp/nfp_logs.h b/drivers/net/nfp/nfp_logs.h
> index b7632ee72c..315a57811c 100644
> --- a/drivers/net/nfp/nfp_logs.h
> +++ b/drivers/net/nfp/nfp_logs.h
> @@ -15,15 +15,19 @@ extern int nfp_logtype_init;
>  #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
>  
>  #ifdef RTE_ETHDEV_DEBUG_RX
> +extern int nfp_logtype_rx;
>  #define PMD_RX_LOG(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args)
> +	rte_log(RTE_LOG_ ## level, nfp_logtype_rx, \
> +		"%s(): " fmt "\n", __func__, ## args)
>  #else
>  #define PMD_RX_LOG(level, fmt, args...) do { } while (0)
>  #endif
>  
>  #ifdef RTE_ETHDEV_DEBUG_TX
> +extern int nfp_logtype_tx;
>  #define PMD_TX_LOG(level, fmt, args...) \
> -	RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args)
> +	rte_log(RTE_LOG_ ## level, nfp_logtype_tx, \
> +		"%s(): " fmt "\n", __func__, ## args)
>  #else
>  #define PMD_TX_LOG(level, fmt, args...) do { } while (0)
>  #endif

Intention is to replace 'RTE_LOG_DP' with 'PMD_RX_LOG'/'PMD_TX_LOG', but
these are not exactly same (although difference is minor).

When 'RTE_ETHDEV_DEBUG_RX' is set, ethdev layer also adds some
additional load, although I believe that will small comparing to logging
in driver.
If 'RTE_LOG_DP' used, the ethdev layer cost can be removed.

With 'RTE_LOG_DP', log level more verbose than requested won't cause any
performance impact. Like if ERR level requested, INFO, DEBUG etc logs
will be compiled out and won't cause any performance impact.
But with 'RTE_ETHDEV_DEBUG_RX', even log level only request ERR, all
logging will add cost of at least an if branch (checking log level).


For many cases I am not sure these differences matters, and already many
drivers directly uses 'RTE_ETHDEV_DEBUG_RX' as done here. So you may
prefer to keep as it is.

But if there is a desire for this fine grain approach, it is possible to
add a version of 'RTE_LOG_DP' macro that accepts dynamic log type
(instead of static RTE_LOGTYPE_# type),
what do you think?