From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH v2] eal: redefine logtype values Date: Wed, 12 Apr 2017 21:23:47 +0200 Message-ID: <20170412212347.0cd1966d@neon> References: <1492006471-114636-1-git-send-email-pablo.de.lara.guarch@intel.com> <1492011332-5846-1-git-send-email-pablo.de.lara.guarch@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: thomas.monjalon@6wind.com, dev@dpdk.org To: Pablo de Lara Return-path: Received: from mail-wr0-f173.google.com (mail-wr0-f173.google.com [209.85.128.173]) by dpdk.org (Postfix) with ESMTP id AFE852C27 for ; Wed, 12 Apr 2017 21:23:50 +0200 (CEST) Received: by mail-wr0-f173.google.com with SMTP id l28so23517317wre.0 for ; Wed, 12 Apr 2017 12:23:50 -0700 (PDT) In-Reply-To: <1492011332-5846-1-git-send-email-pablo.de.lara.guarch@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Pablo, On Wed, 12 Apr 2017 16:35:32 +0100 Pablo de Lara 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 > --- > > 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