From: Olivier MATZ <olivier.matz@6wind.com>
To: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Cc: thomas.monjalon@6wind.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
Date: Wed, 12 Apr 2017 21:23:47 +0200 [thread overview]
Message-ID: <20170412212347.0cd1966d@neon> (raw)
In-Reply-To: <1492011332-5846-1-git-send-email-pablo.de.lara.guarch@intel.com>
Hi Pablo,
On Wed, 12 Apr 2017 16:35:32 +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 v2:
> - Used new RTE_LOGTYPE values in rte_log_init()
>
> lib/librte_eal/common/eal_common_log.c | 56 ++++++++++++++++---------------
> lib/librte_eal/common/include/rte_log.h | 58 ++++++++++++++++-----------------
> 2 files changed, 58 insertions(+), 56 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index dd4d30c..b745811 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -249,33 +249,35 @@ rte_log_init(void)
> if (rte_logs.dynamic_types == NULL)
> return;
>
> - /* register legacy log types, keep sync'd with RTE_LOGTYPE_* */
> - __rte_log_register("eal", 0);
> - __rte_log_register("malloc", 1);
> - __rte_log_register("ring", 2);
> - __rte_log_register("mempool", 3);
> - __rte_log_register("timer", 4);
> - __rte_log_register("pmd", 5);
> - __rte_log_register("hash", 6);
> - __rte_log_register("lpm", 7);
> - __rte_log_register("kni", 8);
> - __rte_log_register("acl", 9);
> - __rte_log_register("power", 10);
> - __rte_log_register("meter", 11);
> - __rte_log_register("sched", 12);
> - __rte_log_register("port", 13);
> - __rte_log_register("table", 14);
> - __rte_log_register("pipeline", 15);
> - __rte_log_register("mbuf", 16);
> - __rte_log_register("cryptodev", 17);
> - __rte_log_register("user1", 24);
> - __rte_log_register("user2", 25);
> - __rte_log_register("user3", 26);
> - __rte_log_register("user4", 27);
> - __rte_log_register("user5", 28);
> - __rte_log_register("user6", 29);
> - __rte_log_register("user7", 30);
> - __rte_log_register("user8", 31);
> + /* register legacy log types */
> + __rte_log_register("eal", RTE_LOGTYPE_EAL);
> + __rte_log_register("malloc", RTE_LOGTYPE_MALLOC);
> + __rte_log_register("ring", RTE_LOGTYPE_RING);
> + __rte_log_register("mempool", RTE_LOGTYPE_MEMPOOL);
> + __rte_log_register("timer", RTE_LOGTYPE_TIMER);
> + __rte_log_register("pmd", RTE_LOGTYPE_PMD);
> + __rte_log_register("hash", RTE_LOGTYPE_HASH);
> + __rte_log_register("lpm", RTE_LOGTYPE_LPM);
> + __rte_log_register("kni", RTE_LOGTYPE_KNI);
> + __rte_log_register("acl", RTE_LOGTYPE_ACL);
> + __rte_log_register("power", RTE_LOGTYPE_POWER);
> + __rte_log_register("meter", RTE_LOGTYPE_METER);
> + __rte_log_register("sched", RTE_LOGTYPE_SCHED);
> + __rte_log_register("port", RTE_LOGTYPE_PORT);
> + __rte_log_register("table", RTE_LOGTYPE_TABLE);
> + __rte_log_register("pipeline", RTE_LOGTYPE_PIPELINE);
> + __rte_log_register("mbuf", RTE_LOGTYPE_MBUF);
> + __rte_log_register("cryptodev", RTE_LOGTYPE_CRYPTODEV);
> + __rte_log_register("efd", RTE_LOGTYPE_EFD);
> + __rte_log_register("eventdev", RTE_LOGTYPE_EVENTDEV);
> + __rte_log_register("user1", RTE_LOGTYPE_USER1);
> + __rte_log_register("user2", RTE_LOGTYPE_USER2);
> + __rte_log_register("user3", RTE_LOGTYPE_USER3);
> + __rte_log_register("user4", RTE_LOGTYPE_USER4);
> + __rte_log_register("user5", RTE_LOGTYPE_USER5);
> + __rte_log_register("user6", RTE_LOGTYPE_USER6);
> + __rte_log_register("user7", RTE_LOGTYPE_USER7);
> + __rte_log_register("user8", RTE_LOGTYPE_USER8);
>
> rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
> }
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index c2ff975..3419138 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -66,37 +66,37 @@ struct rte_logs {
> /** Global log informations */
> extern struct rte_logs rte_logs;
>
> -/* SDK log type, keep sync'd with rte_log_init() */
> -#define RTE_LOGTYPE_EAL 0x00000001 /**< Log related to eal. */
> -#define RTE_LOGTYPE_MALLOC 0x00000002 /**< Log related to malloc. */
> -#define RTE_LOGTYPE_RING 0x00000004 /**< Log related to ring. */
> -#define RTE_LOGTYPE_MEMPOOL 0x00000008 /**< Log related to mempool. */
> -#define RTE_LOGTYPE_TIMER 0x00000010 /**< Log related to timers. */
> -#define RTE_LOGTYPE_PMD 0x00000020 /**< Log related to poll mode driver. */
> -#define RTE_LOGTYPE_HASH 0x00000040 /**< Log related to hash table. */
> -#define RTE_LOGTYPE_LPM 0x00000080 /**< Log related to LPM. */
> -#define RTE_LOGTYPE_KNI 0x00000100 /**< Log related to KNI. */
> -#define RTE_LOGTYPE_ACL 0x00000200 /**< Log related to ACL. */
> -#define RTE_LOGTYPE_POWER 0x00000400 /**< Log related to power. */
> -#define RTE_LOGTYPE_METER 0x00000800 /**< Log related to QoS meter. */
> -#define RTE_LOGTYPE_SCHED 0x00001000 /**< Log related to QoS port scheduler. */
> -#define RTE_LOGTYPE_PORT 0x00002000 /**< Log related to port. */
> -#define RTE_LOGTYPE_TABLE 0x00004000 /**< Log related to table. */
> -#define RTE_LOGTYPE_PIPELINE 0x00008000 /**< Log related to pipeline. */
> -#define RTE_LOGTYPE_MBUF 0x00010000 /**< Log related to mbuf. */
> -#define RTE_LOGTYPE_CRYPTODEV 0x00020000 /**< Log related to cryptodev. */
> -#define RTE_LOGTYPE_EFD 0x00040000 /**< Log related to EFD. */
> -#define RTE_LOGTYPE_EVENTDEV 0x00080000 /**< Log related to eventdev. */
> +/* SDK log type */
> +#define RTE_LOGTYPE_EAL 0 /**< Log related to eal. */
> +#define RTE_LOGTYPE_MALLOC 1 /**< Log related to malloc. */
> +#define RTE_LOGTYPE_RING 2 /**< Log related to ring. */
> +#define RTE_LOGTYPE_MEMPOOL 3 /**< Log related to mempool. */
> +#define RTE_LOGTYPE_TIMER 4 /**< Log related to timers. */
> +#define RTE_LOGTYPE_PMD 5 /**< Log related to poll mode driver. */
> +#define RTE_LOGTYPE_HASH 6 /**< Log related to hash table. */
> +#define RTE_LOGTYPE_LPM 7 /**< Log related to LPM. */
> +#define RTE_LOGTYPE_KNI 8 /**< Log related to KNI. */
> +#define RTE_LOGTYPE_ACL 9 /**< Log related to ACL. */
> +#define RTE_LOGTYPE_POWER 10 /**< Log related to power. */
> +#define RTE_LOGTYPE_METER 11 /**< Log related to QoS meter. */
> +#define RTE_LOGTYPE_SCHED 12 /**< Log related to QoS port scheduler. */
> +#define RTE_LOGTYPE_PORT 13 /**< Log related to port. */
> +#define RTE_LOGTYPE_TABLE 14 /**< Log related to table. */
> +#define RTE_LOGTYPE_PIPELINE 15 /**< Log related to pipeline. */
> +#define RTE_LOGTYPE_MBUF 16 /**< Log related to mbuf. */
> +#define RTE_LOGTYPE_CRYPTODEV 17 /**< Log related to cryptodev. */
> +#define RTE_LOGTYPE_EFD 18 /**< Log related to EFD. */
> +#define RTE_LOGTYPE_EVENTDEV 19 /**< Log related to eventdev. */
>
> /* these log types can be used in an application */
> -#define RTE_LOGTYPE_USER1 0x01000000 /**< User-defined log type 1. */
> -#define RTE_LOGTYPE_USER2 0x02000000 /**< User-defined log type 2. */
> -#define RTE_LOGTYPE_USER3 0x04000000 /**< User-defined log type 3. */
> -#define RTE_LOGTYPE_USER4 0x08000000 /**< User-defined log type 4. */
> -#define RTE_LOGTYPE_USER5 0x10000000 /**< User-defined log type 5. */
> -#define RTE_LOGTYPE_USER6 0x20000000 /**< User-defined log type 6. */
> -#define RTE_LOGTYPE_USER7 0x40000000 /**< User-defined log type 7. */
> -#define RTE_LOGTYPE_USER8 0x80000000 /**< User-defined log type 8. */
> +#define RTE_LOGTYPE_USER1 24 /**< User-defined log type 1. */
> +#define RTE_LOGTYPE_USER2 25 /**< User-defined log type 2. */
> +#define RTE_LOGTYPE_USER3 26 /**< User-defined log type 3. */
> +#define RTE_LOGTYPE_USER4 27 /**< User-defined log type 4. */
> +#define RTE_LOGTYPE_USER5 28 /**< User-defined log type 5. */
> +#define RTE_LOGTYPE_USER6 29 /**< User-defined log type 6. */
> +#define RTE_LOGTYPE_USER7 30 /**< User-defined log type 7. */
> +#define RTE_LOGTYPE_USER8 31 /**< User-defined log type 8. */
>
> /** First identifier for extended logs */
> #define RTE_LOGTYPE_FIRST_EXT_ID 32
Thanks for spotting this issue. I think there is still a problem with
the deprecated functions for which we want to keep compat:
/* Set global log type */
__rte_deprecated void
rte_set_log_type(uint32_t type, int enable)
{
if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
if (enable)
rte_logs.type |= type;
else
rte_logs.type &= ~type;
}
if (enable)
rte_log_set_level(type, 0);
else
rte_log_set_level(type, RTE_LOG_DEBUG);
}
/* Get global log type */
__rte_deprecated uint32_t
rte_get_log_type(void)
{
return rte_logs.type;
}
There is a problem in these functions because they expect bitmasks
for the first part.
I does not look so easy to both fix the issue and keep a full compat
with previous functions.
The first solution is your patch + add a shift in rte_set_log_type().
It would work except if an application uses rte_get_log_type() and masks
the result with a RTE_LOGTYPE_* (which will not be a bitmask). I think
it's a rare case.
The second approach would be to define new constants for the new
functions and keep the old ones for the old functions. This approach
is probably more confusing. It also requires to check the doc and
examples again.
Any opinion?
I would prefer the first solution. If you want I can send a patch
updated based on yours.
Sorry for having missed that.
Olivier
next prev parent reply other threads:[~2017-04-12 19:23 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-12 14:14 [dpdk-dev] [PATCH] " Pablo de Lara
2017-04-12 15:15 ` Thomas Monjalon
2017-04-12 15:22 ` De Lara Guarch, Pablo
2017-04-12 15:35 ` [dpdk-dev] [PATCH v2] " Pablo de Lara
2017-04-12 19:23 ` Olivier MATZ [this message]
2017-04-13 8:32 ` De Lara Guarch, Pablo
2017-04-13 9:09 ` De Lara Guarch, Pablo
2017-04-12 21:41 ` Stephen Hemminger
2017-04-13 13:43 ` De Lara Guarch, Pablo
2017-04-13 13:42 ` [dpdk-dev] [PATCH v3] " Pablo de Lara
2017-04-18 9:57 ` Olivier MATZ
2017-04-19 11:12 ` De Lara Guarch, Pablo
2017-04-19 11:22 ` [dpdk-dev] [PATCH] " Pablo de Lara
2017-04-19 11:23 ` De Lara Guarch, Pablo
2017-04-19 11:24 ` [dpdk-dev] [PATCH v4] " Pablo de Lara
2017-04-19 12:15 ` Olivier MATZ
2017-04-19 13:46 ` De Lara Guarch, Pablo
2017-04-19 14:06 ` [dpdk-dev] [PATCH v5] " Pablo de Lara
2017-04-19 14:16 ` Olivier MATZ
2017-04-19 22:49 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170412212347.0cd1966d@neon \
--to=olivier.matz@6wind.com \
--cc=dev@dpdk.org \
--cc=pablo.de.lara.guarch@intel.com \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).