DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal: redefine logtype values
@ 2017-04-12 14:14 Pablo de Lara
  2017-04-12 15:15 ` Thomas Monjalon
  2017-04-12 15:35 ` [dpdk-dev] [PATCH v2] " Pablo de Lara
  0 siblings, 2 replies; 20+ messages in thread
From: Pablo de Lara @ 2017-04-12 14:14 UTC (permalink / raw)
  To: olivier.matz; +Cc: dev, Pablo de Lara

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>
---
 lib/librte_eal/common/include/rte_log.h | 56 ++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index c2ff975..9cedaa2 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -67,36 +67,36 @@ struct rte_logs {
 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. */
+#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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: redefine logtype values
  2017-04-12 14:14 [dpdk-dev] [PATCH] eal: redefine logtype values 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
  1 sibling, 1 reply; 20+ messages in thread
From: Thomas Monjalon @ 2017-04-12 15:15 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: dev, olivier.matz

2017-04-12 15:14, Pablo de Lara:
> 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>
[...]
>  /* SDK log type, keep sync'd with rte_log_init() */
[...]
> +#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. */

I think you should use these values in rte_log_init,
instead of the hardcoded ones.
So the messages about keeping them sync'd could be removed :)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: redefine logtype values
  2017-04-12 15:15 ` Thomas Monjalon
@ 2017-04-12 15:22   ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-12 15:22 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, olivier.matz



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, April 12, 2017 4:16 PM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org; olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] eal: redefine logtype values
> 
> 2017-04-12 15:14, Pablo de Lara:
> > 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>
> [...]
> >  /* SDK log type, keep sync'd with rte_log_init() */
> [...]
> > +#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. */
> 
> I think you should use these values in rte_log_init,
> instead of the hardcoded ones.
> So the messages about keeping them sync'd could be removed :)

Good idea, will send a v2 shortly.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v2] eal: redefine logtype values
  2017-04-12 14:14 [dpdk-dev] [PATCH] eal: redefine logtype values Pablo de Lara
  2017-04-12 15:15 ` Thomas Monjalon
@ 2017-04-12 15:35 ` Pablo de Lara
  2017-04-12 19:23   ` Olivier MATZ
                     ` (2 more replies)
  1 sibling, 3 replies; 20+ messages in thread
From: Pablo de Lara @ 2017-04-12 15:35 UTC (permalink / raw)
  To: olivier.matz; +Cc: thomas.monjalon, dev, Pablo de Lara

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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
  2017-04-12 15:35 ` [dpdk-dev] [PATCH v2] " Pablo de Lara
@ 2017-04-12 19:23   ` Olivier MATZ
  2017-04-13  8:32     ` De Lara Guarch, Pablo
  2017-04-12 21:41   ` Stephen Hemminger
  2017-04-13 13:42   ` [dpdk-dev] [PATCH v3] " Pablo de Lara
  2 siblings, 1 reply; 20+ messages in thread
From: Olivier MATZ @ 2017-04-12 19:23 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: thomas.monjalon, dev

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
  2017-04-12 15:35 ` [dpdk-dev] [PATCH v2] " Pablo de Lara
  2017-04-12 19:23   ` Olivier MATZ
@ 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
  2 siblings, 1 reply; 20+ messages in thread
From: Stephen Hemminger @ 2017-04-12 21:41 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: olivier.matz, thomas.monjalon, dev

On Wed, 12 Apr 2017 16:35:32 +0100
Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:

> +	/* 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);

Why is this not a table rather than code?
Data driven design is better.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
  2017-04-12 19:23   ` Olivier MATZ
@ 2017-04-13  8:32     ` De Lara Guarch, Pablo
  2017-04-13  9:09       ` De Lara Guarch, Pablo
  0 siblings, 1 reply; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-13  8:32 UTC (permalink / raw)
  To: Olivier MATZ; +Cc: thomas.monjalon, dev

Hi Olivier,

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, April 12, 2017 8:24 PM
> To: De Lara Guarch, Pablo
> Cc: thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [PATCH v2] eal: redefine logtype values
> 
> Hi Pablo,
> 
> 
> On Wed, 12 Apr 2017 16:35:32 +0100
> Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:

...

> 
> 
> 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.

Oh yes, I missed that too. First option looks ok to me, so send a patch if you like.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
  2017-04-13  8:32     ` De Lara Guarch, Pablo
@ 2017-04-13  9:09       ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-13  9:09 UTC (permalink / raw)
  To: De Lara Guarch, Pablo, Olivier MATZ; +Cc: thomas.monjalon, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Thursday, April 13, 2017 9:32 AM
> To: Olivier MATZ
> Cc: thomas.monjalon@6wind.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
> 
> Hi Olivier,
> 
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > Sent: Wednesday, April 12, 2017 8:24 PM
> > To: De Lara Guarch, Pablo
> > Cc: thomas.monjalon@6wind.com; dev@dpdk.org
> > Subject: Re: [PATCH v2] eal: redefine logtype values
> >
> > Hi Pablo,
> >
> >
> > On Wed, 12 Apr 2017 16:35:32 +0100
> > Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:
> 
> ...
> 
> >
> >
> > 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.
> 
> Oh yes, I missed that too. First option looks ok to me, so send a patch if you
> like.

Hi Olivier,

I just saw a comment from Stephen Hemminger, so do not send the patch yet, as I am making changes to the v2.
Actually, I can make this change too, since it is a minor one.

Thanks,
Pablo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v3] eal: redefine logtype values
  2017-04-12 15:35 ` [dpdk-dev] [PATCH v2] " Pablo de Lara
  2017-04-12 19:23   ` Olivier MATZ
  2017-04-12 21:41   ` Stephen Hemminger
@ 2017-04-13 13:42   ` Pablo de Lara
  2017-04-18  9:57     ` Olivier MATZ
                       ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Pablo de Lara @ 2017-04-13 13:42 UTC (permalink / raw)
  To: olivier.matz; +Cc: thomas.monjalon, stephen, dev, Pablo de Lara

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(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index dd4d30c..9c449e3 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -118,9 +118,9 @@ rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
 		if (enable)
-			rte_logs.type |= type;
+			rte_logs.type |= 1 << type;
 		else
-			rte_logs.type &= ~type;
+			rte_logs.type &= ~(1 << type);
 	}
 
 	if (enable)
@@ -244,38 +244,17 @@ RTE_INIT(rte_log_init);
 static void
 rte_log_init(void)
 {
+	uint32_t i;
+
 	rte_logs.dynamic_types = calloc(RTE_LOGTYPE_FIRST_EXT_ID,
 		sizeof(struct rte_log_dynamic_type));
 	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 */
+	for (i = 0; i < RTE_DIM(logtype_strings); i++)
+		__rte_log_register(logtype_strings[i].logtype,
+				logtype_strings[i].log_id);
 
 	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..dde6ad2 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
@@ -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.
  *
-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
  2017-04-12 21:41   ` Stephen Hemminger
@ 2017-04-13 13:43     ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-13 13:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: olivier.matz, thomas.monjalon, dev



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, April 12, 2017 10:41 PM
> To: De Lara Guarch, Pablo
> Cc: olivier.matz@6wind.com; thomas.monjalon@6wind.com;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] eal: redefine logtype values
> 
> On Wed, 12 Apr 2017 16:35:32 +0100
> Pablo de Lara <pablo.de.lara.guarch@intel.com> wrote:
> 
> > +	/* 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);
> 
> Why is this not a table rather than code?
> Data driven design is better.

Hi Stephen,

I just sent a v3 with your change proposed.

Thanks!
Pablo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3] eal: redefine logtype values
  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:24     ` [dpdk-dev] [PATCH v4] " Pablo de Lara
  2 siblings, 1 reply; 20+ messages in thread
From: Olivier MATZ @ 2017-04-18  9:57 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: thomas.monjalon, stephen, dev

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3] eal: redefine logtype values
  2017-04-18  9:57     ` Olivier MATZ
@ 2017-04-19 11:12       ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-19 11:12 UTC (permalink / raw)
  To: Olivier MATZ; +Cc: thomas.monjalon, stephen, dev



> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, April 18, 2017 10:58 AM
> To: De Lara Guarch, Pablo
> Cc: thomas.monjalon@6wind.com; stephen@networkplumber.org;
> dev@dpdk.org
> Subject: Re: [PATCH v3] eal: redefine logtype values
> 
> 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!

Sure, good idea. Will send a v4 shortly.

> 
> 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.

Yes, I think we need to start moving the apps to use the new logs (and the libraries, right?).
Probably a bit late to do it in this release, I think it can wait until the next one.

Pablo
> 
> Regards,
> Olivier

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH] eal: redefine logtype values
  2017-04-13 13:42   ` [dpdk-dev] [PATCH v3] " Pablo de Lara
  2017-04-18  9:57     ` Olivier MATZ
@ 2017-04-19 11:22     ` Pablo de Lara
  2017-04-19 11:23       ` De Lara Guarch, Pablo
  2017-04-19 11:24     ` [dpdk-dev] [PATCH v4] " Pablo de Lara
  2 siblings, 1 reply; 20+ messages in thread
From: Pablo de Lara @ 2017-04-19 11:22 UTC (permalink / raw)
  To: thomas, olivier.matz; +Cc: dev, Pablo de Lara

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>
---
 lib/librte_eal/common/eal_common_log.c  | 73 ++++++++++++++++++++-------------
 lib/librte_eal/common/include/rte_log.h | 58 +++++++++++++-------------
 2 files changed, 73 insertions(+), 58 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index dd4d30c..fd76612 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -118,9 +118,9 @@ rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
 		if (enable)
-			rte_logs.type |= type;
+			rte_logs.type |= 1 << type;
 		else
-			rte_logs.type &= ~type;
+			rte_logs.type &= ~(1 << type);
 	}
 
 	if (enable)
@@ -240,42 +240,57 @@ rte_log_register(const char *name)
 	return ret;
 }
 
+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"}
+};
+
 RTE_INIT(rte_log_init);
 static void
 rte_log_init(void)
 {
+	uint32_t i;
+
 	rte_logs.dynamic_types = calloc(RTE_LOGTYPE_FIRST_EXT_ID,
 		sizeof(struct rte_log_dynamic_type));
 	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 */
+	for (i = 0; i < RTE_DIM(logtype_strings); i++)
+		__rte_log_register(logtype_strings[i].logtype,
+				logtype_strings[i].log_id);
 
 	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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: redefine logtype values
  2017-04-19 11:22     ` [dpdk-dev] [PATCH] " Pablo de Lara
@ 2017-04-19 11:23       ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-19 11:23 UTC (permalink / raw)
  To: thomas, olivier.matz; +Cc: dev



> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, April 19, 2017 12:23 PM
> To: thomas@monjalon.net; olivier.matz@6wind.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo
> Subject: [PATCH] eal: redefine logtype values
> 
> 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>

NACK, missing patch revision and changes.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v4] eal: redefine logtype values
  2017-04-13 13:42   ` [dpdk-dev] [PATCH v3] " Pablo de Lara
  2017-04-18  9:57     ` Olivier MATZ
  2017-04-19 11:22     ` [dpdk-dev] [PATCH] " Pablo de Lara
@ 2017-04-19 11:24     ` Pablo de Lara
  2017-04-19 12:15       ` Olivier MATZ
  2017-04-19 14:06       ` [dpdk-dev] [PATCH v5] " Pablo de Lara
  2 siblings, 2 replies; 20+ messages in thread
From: Pablo de Lara @ 2017-04-19 11:24 UTC (permalink / raw)
  To: thomas, olivier.matz; +Cc: dev, Pablo de Lara

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 v4:
- Moved log type strings to eal_common_log.c

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  | 73 ++++++++++++++++++++-------------
 lib/librte_eal/common/include/rte_log.h | 58 +++++++++++++-------------
 2 files changed, 73 insertions(+), 58 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index dd4d30c..fd76612 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -118,9 +118,9 @@ rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
 		if (enable)
-			rte_logs.type |= type;
+			rte_logs.type |= 1 << type;
 		else
-			rte_logs.type &= ~type;
+			rte_logs.type &= ~(1 << type);
 	}
 
 	if (enable)
@@ -240,42 +240,57 @@ rte_log_register(const char *name)
 	return ret;
 }
 
+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"}
+};
+
 RTE_INIT(rte_log_init);
 static void
 rte_log_init(void)
 {
+	uint32_t i;
+
 	rte_logs.dynamic_types = calloc(RTE_LOGTYPE_FIRST_EXT_ID,
 		sizeof(struct rte_log_dynamic_type));
 	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 */
+	for (i = 0; i < RTE_DIM(logtype_strings); i++)
+		__rte_log_register(logtype_strings[i].logtype,
+				logtype_strings[i].log_id);
 
 	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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v4] eal: redefine logtype values
  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
  1 sibling, 1 reply; 20+ messages in thread
From: Olivier MATZ @ 2017-04-19 12:15 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: thomas, dev

Hi Pablo,

On Wed, 19 Apr 2017 12:24:04 +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 v4:
> - Moved log type strings to eal_common_log.c
> 
> 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  | 73 ++++++++++++++++++++-------------
>  lib/librte_eal/common/include/rte_log.h | 58 +++++++++++++-------------
>  2 files changed, 73 insertions(+), 58 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index dd4d30c..fd76612 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -118,9 +118,9 @@ rte_set_log_type(uint32_t type, int enable)
>  {
>  	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
>  		if (enable)
> -			rte_logs.type |= type;
> +			rte_logs.type |= 1 << type;
>  		else
> -			rte_logs.type &= ~type;
> +			rte_logs.type &= ~(1 << type);
>  	}
>  
>  	if (enable)
> @@ -240,42 +240,57 @@ rte_log_register(const char *name)
>  	return ret;
>  }
>  
> +struct logtype {
> +	uint32_t log_id;
> +	char logtype[32];
> +};

Sorry I missed it in the previous review, but what do you think of
using "const char *" instead of "char[32]"?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v4] eal: redefine logtype values
  2017-04-19 12:15       ` Olivier MATZ
@ 2017-04-19 13:46         ` De Lara Guarch, Pablo
  0 siblings, 0 replies; 20+ messages in thread
From: De Lara Guarch, Pablo @ 2017-04-19 13:46 UTC (permalink / raw)
  To: Olivier MATZ; +Cc: thomas, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ
> Sent: Wednesday, April 19, 2017 1:15 PM
> To: De Lara Guarch, Pablo
> Cc: thomas@monjalon.net; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] eal: redefine logtype values
> 
> Hi Pablo,
> 
> On Wed, 19 Apr 2017 12:24:04 +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 v4:
> > - Moved log type strings to eal_common_log.c
> >
> > 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  | 73
> ++++++++++++++++++++-------------
> >  lib/librte_eal/common/include/rte_log.h | 58 +++++++++++++-------------
> >  2 files changed, 73 insertions(+), 58 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_log.c
> b/lib/librte_eal/common/eal_common_log.c
> > index dd4d30c..fd76612 100644
> > --- a/lib/librte_eal/common/eal_common_log.c
> > +++ b/lib/librte_eal/common/eal_common_log.c
> > @@ -118,9 +118,9 @@ rte_set_log_type(uint32_t type, int enable)
> >  {
> >  	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
> >  		if (enable)
> > -			rte_logs.type |= type;
> > +			rte_logs.type |= 1 << type;
> >  		else
> > -			rte_logs.type &= ~type;
> > +			rte_logs.type &= ~(1 << type);
> >  	}
> >
> >  	if (enable)
> > @@ -240,42 +240,57 @@ rte_log_register(const char *name)
> >  	return ret;
> >  }
> >
> > +struct logtype {
> > +	uint32_t log_id;
> > +	char logtype[32];
> > +};
> 
> Sorry I missed it in the previous review, but what do you think of
> using "const char *" instead of "char[32]"?

Right, I thought I needed to reserve the memory in the structure definition,
but sure, that works too :)

v5 on the way...

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v5] eal: redefine logtype values
  2017-04-19 11:24     ` [dpdk-dev] [PATCH v4] " Pablo de Lara
  2017-04-19 12:15       ` Olivier MATZ
@ 2017-04-19 14:06       ` Pablo de Lara
  2017-04-19 14:16         ` Olivier MATZ
  1 sibling, 1 reply; 20+ messages in thread
From: Pablo de Lara @ 2017-04-19 14:06 UTC (permalink / raw)
  To: thomas, olivier.matz; +Cc: dev, Pablo de Lara

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 v5:
- Changed variable type from char [32] to const char *

Changes in v4:
- Moved log type strings to eal_common_log.c

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  | 73 ++++++++++++++++++++-------------
 lib/librte_eal/common/include/rte_log.h | 58 +++++++++++++-------------
 2 files changed, 73 insertions(+), 58 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index dd4d30c..ddc99bd 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -118,9 +118,9 @@ rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
 		if (enable)
-			rte_logs.type |= type;
+			rte_logs.type |= 1 << type;
 		else
-			rte_logs.type &= ~type;
+			rte_logs.type &= ~(1 << type);
 	}
 
 	if (enable)
@@ -240,42 +240,57 @@ rte_log_register(const char *name)
 	return ret;
 }
 
+struct logtype {
+	uint32_t log_id;
+	const char *logtype;
+};
+
+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"}
+};
+
 RTE_INIT(rte_log_init);
 static void
 rte_log_init(void)
 {
+	uint32_t i;
+
 	rte_logs.dynamic_types = calloc(RTE_LOGTYPE_FIRST_EXT_ID,
 		sizeof(struct rte_log_dynamic_type));
 	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 */
+	for (i = 0; i < RTE_DIM(logtype_strings); i++)
+		__rte_log_register(logtype_strings[i].logtype,
+				logtype_strings[i].log_id);
 
 	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
-- 
2.7.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v5] eal: redefine logtype values
  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
  0 siblings, 1 reply; 20+ messages in thread
From: Olivier MATZ @ 2017-04-19 14:16 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: thomas, dev

On Wed, 19 Apr 2017 15:06:34 +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>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thank you

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v5] eal: redefine logtype values
  2017-04-19 14:16         ` Olivier MATZ
@ 2017-04-19 22:49           ` Thomas Monjalon
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Monjalon @ 2017-04-19 22:49 UTC (permalink / raw)
  To: Pablo de Lara; +Cc: dev, Olivier MATZ

19/04/2017 16:16, Olivier MATZ:
> On Wed, 19 Apr 2017 15:06:34 +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>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Thank you

Applied, thanks

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-04-19 22:49 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-12 14:14 [dpdk-dev] [PATCH] eal: redefine logtype values 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
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

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).