Re: [dpdk-dev] [PATCH v3] eal: redefine logtype values

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

Hi Pablo,

On Thu, 13 Apr 2017 14:42:47 +0100, Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:
> After the changes in commit c1b5fa94a46f
> ("eal: support dynamic log types"), logtype is not treated as a
> bitmask, but a decimal value. Therefore, values have to be
> converted.
> 
> Fixes: c1b5fa94a46f ("eal: support dynamic log types")
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> 
> Changes in v3:
> - Created array of structures containing logtype id and string
> - Added left shift to convert new decimal values to bitmask for backward compatibility
> 
> Changes in v2:
> - Used new RTE_LOGTYPE values in rte_log_init()
> 
>  lib/librte_eal/common/eal_common_log.c  | 37 +++----------
>  lib/librte_eal/common/include/rte_log.h | 94 +++++++++++++++++++++++----------
>  2 files changed, 73 insertions(+), 58 deletions(-)

[...]


> @@ -111,6 +111,42 @@ extern struct rte_logs rte_logs;
>  #define RTE_LOG_INFO     7U  /**< Informational.                    */
>  #define RTE_LOG_DEBUG    8U  /**< Debug-level messages.             */
>  
> +struct logtype {
> +	uint32_t log_id;
> +	char logtype[32];
> +};
> +
> +static const struct logtype logtype_strings[] = {
> +	{RTE_LOGTYPE_EAL,        "eal"},
> +	{RTE_LOGTYPE_MALLOC,     "malloc"},
> +	{RTE_LOGTYPE_RING,       "ring"},
> +	{RTE_LOGTYPE_MEMPOOL,    "mempool"},
> +	{RTE_LOGTYPE_TIMER,      "timer"},
> +	{RTE_LOGTYPE_PMD,        "pmd"},
> +	{RTE_LOGTYPE_HASH,       "hash"},
> +	{RTE_LOGTYPE_LPM,        "lpm"},
> +	{RTE_LOGTYPE_KNI,        "kni"},
> +	{RTE_LOGTYPE_ACL,        "acl"},
> +	{RTE_LOGTYPE_POWER,      "power"},
> +	{RTE_LOGTYPE_METER,      "meter"},
> +	{RTE_LOGTYPE_SCHED,      "sched"},
> +	{RTE_LOGTYPE_PORT,       "port"},
> +	{RTE_LOGTYPE_TABLE,      "table"},
> +	{RTE_LOGTYPE_PIPELINE,   "pipeline"},
> +	{RTE_LOGTYPE_MBUF,       "mbuf"},
> +	{RTE_LOGTYPE_CRYPTODEV,  "cryptodev"},
> +	{RTE_LOGTYPE_EFD,        "efd"},
> +	{RTE_LOGTYPE_EVENTDEV,   "eventdev"},
> +	{RTE_LOGTYPE_USER1,      "user1"},
> +	{RTE_LOGTYPE_USER2,      "user2"},
> +	{RTE_LOGTYPE_USER3,      "user3"},
> +	{RTE_LOGTYPE_USER4,      "user4"},
> +	{RTE_LOGTYPE_USER5,      "user5"},
> +	{RTE_LOGTYPE_USER6,      "user6"},
> +	{RTE_LOGTYPE_USER7,      "user7"},
> +	{RTE_LOGTYPE_USER8,      "user8"}
> +};
> +
>  /**
>   * Change the stream that will be used by the logging system.
>   *

Could it go in eal_common_log.c instead? I think we don't need it in
the header file, and it would avoid to pollute the global namespace.

Apart from that, I tested the patch and it works. Thanks!

I wonder if we can now remove the use or USERx logs in apps.
It still has to work for compat, but it would be better to register
a new logtype instead. I have a patch to do that in testpmd, I'll
send it today. Not sure it should go in the release or not.

Regards,
Olivier