[RFC 0/8] eal: dynamic logs

[RFC 0/8] eal: dynamic logs

[Olivier Matz]

The objective of this RFC patchset is to introduce a framework to
support dynamic log types in EAL. It also provides one example of use
(in i40e).

Features:
- log types are identified by a string
- at registration, a uniq identifier is associated to a log type
- each log type can have its level changed dynamically
- extend command line parameters to set the log level of a specific
  type, or logs matching a regular expression
- keep compat with other legacy types (eal, malloc, ring, user*,
  etc... keep their hardcoded log type value)

At the end, when, we can expect that all non-dataplane logs are moved to
be dynamic, so we can enable/disable them at runtime, without
recompiling. Many debug options can probably be removed from
configuration:
  $ git grep DEBUG config/common_base | wc -l
  89

Comments are welcome!


Olivier Matz (8):
  eal: support dynamic log types
  eal: dump registered log types
  eal: change several log levels matching a regexp
  eal: change specific log levels at startup
  eal: deprecate log functions
  app/test: new command to dump log types
  app/testpmd: new command to dump log types
  net/i40e: use dynamic log type for control logs

 app/test-pmd/cmdline.c                          |   5 +-
 app/test/commands.c                             |   4 +-
 app/test/test_logs.c                            |  12 +-
 doc/guides/contributing/coding_style.rst        |  30 ++--
 drivers/net/i40e/i40e_ethdev.c                  |  18 +-
 drivers/net/i40e/i40e_fdir.c                    |   4 -
 drivers/net/i40e/i40e_logs.h                    |  17 +-
 examples/quota_watermark/qw/main.c              |   2 +-
 examples/quota_watermark/qwctl/qwctl.c          |   2 +-
 lib/librte_eal/bsdapp/eal/eal.c                 |   4 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   6 +
 lib/librte_eal/common/eal_common_log.c          | 219 +++++++++++++++++++++++-
 lib/librte_eal/common/eal_common_options.c      |  48 ++++--
 lib/librte_eal/common/include/rte_log.h         |  76 +++++++-
 lib/librte_eal/linuxapp/eal/eal.c               |   4 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   6 +
 16 files changed, 394 insertions(+), 63 deletions(-)

-- 
2.8.1

[RFC 1/8] eal: support dynamic log types

[Olivier Matz]

Introduce 2 new functions to support dynamic log types:

- rte_log_register(): register a log name, and return a log type id
- rte_log_set_level(): set the log level of a given log type

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test/test_logs.c                            |   6 +-
 doc/guides/contributing/coding_style.rst        |  30 +++--
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   2 +
 lib/librte_eal/common/eal_common_log.c          | 140 +++++++++++++++++++++++-
 lib/librte_eal/common/include/rte_log.h         |  35 +++++-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   2 +
 6 files changed, 197 insertions(+), 18 deletions(-)

diff --git a/app/test/test_logs.c b/app/test/test_logs.c
index 6985ddd..805c568 100644
--- a/app/test/test_logs.c
+++ b/app/test/test_logs.c
@@ -62,8 +62,8 @@ static int
 test_logs(void)
 {
 	/* enable these logs type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP1, RTE_LOG_EMERG);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
@@ -76,7 +76,7 @@ test_logs(void)
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
 	/* disable one log type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 0);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index 4163960..d8e4a0f 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -603,22 +603,30 @@ In the DPDK environment, use the logging interface provided:
 
 .. code-block:: c
 
- #define RTE_LOGTYPE_TESTAPP1 RTE_LOGTYPE_USER1
- #define RTE_LOGTYPE_TESTAPP2 RTE_LOGTYPE_USER2
+ /* register log types for this application */
+ int my_logtype1 = rte_log_register("myapp.log1");
+ int my_logtype2 = rte_log_register("myapp.log2");
 
- /* enable these logs type */
- rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
- rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+ /* set global log level to INFO */
+ rte_log_set_global_level(RTE_LOG_INFO);
+
+ /* only display messages higher than NOTICE for log2 (default
+  * is DEBUG) */
+ rte_log_set_level(my_logtype2, RTE_LOG_NOTICE);
+
+ /* enable all PMD logs (whose identifier string starts with "pmd") */
+ rte_log_set_level_regexp("pmd.*", RTE_LOG_DEBUG);
 
  /* log in debug level */
- rte_set_log_level(RTE_LOG_DEBUG);
- RTE_LOG(DEBUG, TESTAPP1, "this is is a debug level message\n");
- RTE_LOG(INFO, TESTAPP1, "this is is a info level message\n");
- RTE_LOG(WARNING, TESTAPP1, "this is is a warning level message\n");
+ rte_log_set_global_level(RTE_LOG_DEBUG);
+ RTE_LOG(DEBUG, my_logtype1, "this is is a debug level message\n");
+ RTE_LOG(INFO, my_logtype1, "this is is a info level message\n");
+ RTE_LOG(WARNING, my_logtype1, "this is is a warning level message\n");
+ RTE_LOG(WARNING, my_logtype2, "this is is a debug level message (not displayed)\n");
 
  /* log in info level */
- rte_set_log_level(RTE_LOG_INFO);
- RTE_LOG(DEBUG, TESTAPP2, "debug level message (not displayed)\n");
+ rte_log_set_global_level(RTE_LOG_INFO);
+ RTE_LOG(DEBUG, my_logtype1, "debug level message (not displayed)\n");
 
 Branch Prediction
 ~~~~~~~~~~~~~~~~~
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2cf1ac8..85d051c 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -183,5 +183,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_register;
+	rte_log_set_level;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 2197558..cc8da26 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -35,7 +35,10 @@
 #include <stdint.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <string.h>
+#include <errno.h>
 
+#include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
 
@@ -46,6 +49,8 @@ struct rte_logs rte_logs = {
 	.type = ~0,
 	.level = RTE_LOG_DEBUG,
 	.file = NULL,
+	.dynamic_types_len = 0,
+	.dynamic_types = NULL,
 };
 
 /* Stream to use for logging if rte_logs.file is NULL */
@@ -60,6 +65,11 @@ struct log_cur_msg {
 	uint32_t logtype;  /**< log type  - see rte_log.h */
 };
 
+struct rte_log_dynamic_type {
+	const char *name;
+	uint32_t loglevel;
+};
+
  /* per core log */
 static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg);
 
@@ -91,10 +101,17 @@ rte_get_log_level(void)
 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_logs.type |= type;
+		rte_log_set_level(type, 0);
 	else
-		rte_logs.type &= (~type);
+		rte_log_set_level(type, RTE_LOG_DEBUG);
 }
 
 /* Get global log type */
@@ -104,6 +121,19 @@ rte_get_log_type(void)
 	return rte_logs.type;
 }
 
+int
+rte_log_set_level(uint32_t type, uint32_t level)
+{
+	if (type >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
@@ -116,6 +146,106 @@ int rte_log_cur_msg_logtype(void)
 	return RTE_PER_LCORE(log_cur_msg).logtype;
 }
 
+static int
+rte_log_lookup(const char *name)
+{
+	size_t i;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (strcmp(name, rte_logs.dynamic_types[i].name) == 0)
+			return i;
+	}
+
+	return -1;
+}
+
+/* register an extended log type, assuming table is large enough, and id
+ * is not yet registered.
+ */
+static int
+__rte_log_register(const char *name, int id)
+{
+	char *dup_name = NULL;
+
+	dup_name = strdup(name);
+	if (dup_name == NULL)
+		return -ENOMEM;
+
+	rte_logs.dynamic_types[id].name = dup_name;
+	rte_logs.dynamic_types[id].loglevel = RTE_LOG_DEBUG;
+
+	return id;
+}
+
+/* register an extended log type */
+int
+rte_log_register(const char *name)
+{
+	struct rte_log_dynamic_type *new_dynamic_types;
+	int id, ret;
+
+	id = rte_log_lookup(name);
+	if (id >= 0)
+		return id;
+
+	new_dynamic_types = realloc(rte_logs.dynamic_types,
+		sizeof(struct rte_log_dynamic_type) *
+		(rte_logs.dynamic_types_len + 1));
+	if (new_dynamic_types == NULL)
+		return -ENOMEM;
+	rte_logs.dynamic_types = new_dynamic_types;
+
+	ret = __rte_log_register(name, rte_logs.dynamic_types_len);
+	if (ret < 0)
+		return ret;
+
+	rte_logs.dynamic_types_len++;
+
+	return ret;
+}
+
+RTE_INIT(rte_log_init);
+static void
+rte_log_init(void)
+{
+	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);
+
+	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
@@ -139,7 +269,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 		}
 	}
 
-	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
+	if (level > rte_logs.level)
+		return 0;
+	if (logtype >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > rte_logs.dynamic_types[logtype].loglevel)
 		return 0;
 
 	/* save loglevel and logtype in a global per-lcore variable */
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 954b96c..e71887f 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,17 +50,21 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+struct rte_log_dynamic_type;
+
 /** The rte_log structure. */
 struct rte_logs {
 	uint32_t type;  /**< Bitfield with enabled logs. */
 	uint32_t level; /**< Log level. */
 	FILE *file;     /**< Output file set by rte_openlog_stream, or NULL. */
+	size_t dynamic_types_len;
+	struct rte_log_dynamic_type *dynamic_types;
 };
 
 /** Global log informations */
 extern struct rte_logs rte_logs;
 
-/* SDK log type */
+/* 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. */
@@ -91,6 +95,9 @@ extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_USER7   0x40000000 /**< User-defined log type 7. */
 #define RTE_LOGTYPE_USER8   0x80000000 /**< User-defined log type 8. */
 
+/** First identifier for extended logs */
+#define RTE_LOGTYPE_FIRST_EXT_ID 32
+
 /* Can't use 0, as it gives compiler warnings */
 #define RTE_LOG_EMERG    1U  /**< System is unusable.               */
 #define RTE_LOG_ALERT    2U  /**< Action must be taken immediately. */
@@ -148,6 +155,18 @@ void rte_set_log_type(uint32_t type, int enable);
 uint32_t rte_get_log_type(void);
 
 /**
+ * Set the log level for a given type.
+ *
+ * @param logtype
+ *   The log type identifier.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if logtype or level is invalid.
+ */
+int rte_log_set_level(uint32_t logtype, uint32_t level);
+
+/**
  * Get the current loglevel for the message being processed.
  *
  * Before calling the user-defined stream for logging, the log
@@ -176,6 +195,20 @@ int rte_log_cur_msg_loglevel(void);
 int rte_log_cur_msg_logtype(void);
 
 /**
+ * Register a dynamic log type
+ *
+ * If a log is already registered with the same type, the returned value
+ * is the same than the previous one.
+ *
+ * @param name
+ *   The string identifying the log type.
+ * @return
+ *   - >0: success, the returned value is the log type identifier.
+ *   - (-ENONEM): cannot allocate memory.
+ */
+int rte_log_register(const char *name);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 3c68ff5..1471c41 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -187,5 +187,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_register;
+	rte_log_set_level;
 
 } DPDK_16.11;
-- 
2.8.1

[RFC 2/8] eal: dump registered log types

[Olivier Matz]

Introduce a function to dump the global level and the registered log
types.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 34 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 10 ++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 46 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 85d051c..f0a35b1 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -183,6 +183,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index cc8da26..a311279 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -246,6 +246,40 @@ rte_log_init(void)
 	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
 }
 
+static const char *
+loglevel_to_string(uint32_t level)
+{
+	switch (level) {
+	case RTE_LOG_EMERG: return "emerg";
+	case RTE_LOG_ALERT: return "alert";
+	case RTE_LOG_CRIT: return "critical";
+	case RTE_LOG_ERR: return "error";
+	case RTE_LOG_WARNING: return "warning";
+	case RTE_LOG_NOTICE: return "notice";
+	case RTE_LOG_INFO: return "info";
+	case RTE_LOG_DEBUG: return "debug";
+	default: return "unknown";
+	}
+}
+
+/* dump global level and registered log types */
+void
+rte_log_dump(FILE *f)
+{
+	size_t i;
+
+	fprintf(f, "global log level is %s\n",
+		loglevel_to_string(rte_log_get_global_level()));
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		fprintf(f, "id %zu: %s, level is %s\n",
+			i, rte_logs.dynamic_types[i].name,
+			loglevel_to_string(rte_logs.dynamic_types[i].loglevel));
+	}
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index e71887f..97e0c5e 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -209,6 +209,16 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * Dump log information.
+ *
+ * Dump the global level and the registered log types.
+ *
+ * @param f
+ *   The output stream where the dump should be sent.
+ */
+void rte_log_dump(FILE *f);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 1471c41..9991bd1 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -187,6 +187,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
-- 
2.8.1

[RFC 3/8] eal: change several log levels matching a regexp

[Olivier Matz]

Introduce a function to set the log level of several log types that
match a regular expression.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 21 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 12 ++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 35 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f0a35b1..5668d48 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -186,5 +186,6 @@ DPDK_17.02 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index a311279..b35c8b2 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -37,6 +37,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <regex.h>
 
 #include <rte_eal.h>
 #include <rte_log.h>
@@ -134,6 +135,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
 	return 0;
 }
 
+/* set level */
+int
+rte_log_set_level_regexp(const char *pattern, uint32_t level)
+{
+	regex_t r;
+	size_t i;
+
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (regexec(&r, pattern, 0, NULL, 0) == 0)
+			rte_logs.dynamic_types[i].loglevel = level;
+	}
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 97e0c5e..ce48b07 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -157,6 +157,18 @@ uint32_t rte_get_log_type(void);
 /**
  * Set the log level for a given type.
  *
+ * @param pattern
+ *   The regexp identifying the log type.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if level is invalid.
+ */
+int rte_log_set_level_regexp(const char *pattern, uint32_t level);
+
+/**
+ * Set the log level for a given type.
+ *
  * @param logtype
  *   The log type identifier.
  * @param level
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 9991bd1..f133c4f 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -190,5 +190,6 @@ DPDK_17.02 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_16.11;
-- 
2.8.1

[RFC 4/8] eal: change specific log levels at startup

[Olivier Matz]

Example of use:
  ./app/test-pmd --log-level='pmd\.i40e.*,8'

  This enables debug logs for all dynamic logs whose type starts with
  'pmd.i40e'.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            |  4 +--
 lib/librte_eal/common/eal_common_options.c | 48 +++++++++++++++++++++++-------
 lib/librte_eal/linuxapp/eal/eal.c          |  4 +--
 3 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index ee7c9de..e945468 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -505,10 +505,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0)
 		rte_panic("Cannot detect lcores\n");
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index f36bc55..2682881 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -739,25 +739,53 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
 }
 
 static int
-eal_parse_log_level(const char *level, uint32_t *log_level)
+eal_parse_log_level(const char *arg, struct internal_config *conf)
 {
-	char *end;
+	char *end, *str, *type, *level;
 	unsigned long tmp;
 
+	str = strdup(arg);
+	if (str == NULL)
+		return -1;
+
+	if (strchr(str, ',') == NULL) {
+		type = NULL;
+		level = str;
+	} else {
+		type = strsep(&str, ",");
+		level = strsep(&str, ",");
+	}
+
 	errno = 0;
 	tmp = strtoul(level, &end, 0);
 
 	/* check for errors */
 	if ((errno != 0) || (level[0] == '\0') ||
-	    end == NULL || (*end != '\0'))
-		return -1;
+		    end == NULL || (*end != '\0'))
+		goto fail;
 
 	/* log_level is a uint32_t */
 	if (tmp >= UINT32_MAX)
-		return -1;
+		goto fail;
+
+	printf("set log level %s,%lu\n",
+		type, tmp);
+
+	if (type == NULL) {
+		conf->log_level = tmp;
+		rte_log_set_global_level(tmp);
+	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
+		printf("cannot set log level %s,%lu\n",
+			type, tmp);
+		goto fail;
+	}
 
-	*log_level = tmp;
+	free(str);
 	return 0;
+
+fail:
+	free(str);
+	return -1;
 }
 
 static enum rte_proc_type_t
@@ -898,15 +926,12 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_LOG_LEVEL_NUM: {
-		uint32_t log;
-
-		if (eal_parse_log_level(optarg, &log) < 0) {
+		if (eal_parse_log_level(optarg, conf) < 0) {
 			RTE_LOG(ERR, EAL,
 				"invalid parameters for --"
 				OPT_LOG_LEVEL "\n");
 			return -1;
 		}
-		conf->log_level = log;
 		break;
 	}
 	case OPT_LCORES_NUM:
@@ -1057,7 +1082,8 @@ eal_common_usage(void)
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
-	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
+	       "  --"OPT_LOG_LEVEL"=<int>          Set global log level\n"
+	       "  --"OPT_LOG_LEVEL"=<type>,<int>   Set specific log level\n"
 	       "  -v                  Display version information on startup\n"
 	       "  -h, --help          This help\n"
 	       "\nEAL options for DEBUG use only:\n"
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index bf6b818..0f61447 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -762,10 +762,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0)
 		rte_panic("Cannot detect lcores\n");
-- 
2.8.1

[RFC 5/8] eal: deprecate log functions

[Olivier Matz]

Deprecate the following functions:
- rte_set_log_level(), replaced by rte_log_set_global_level()
- rte_get_log_level(), replaced by rte_log_get_global_level()
- rte_set_log_type(), rte_log_set_level()
- rte_get_log_type(), rte_log_get_level()

The new functions provide a better control of the per-type log level,
and have a better name prefix (rte_log_).

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test/test_logs.c                            |  6 +++---
 examples/quota_watermark/qw/main.c              |  2 +-
 examples/quota_watermark/qwctl/qwctl.c          |  2 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
 lib/librte_eal/common/eal_common_log.c          | 24 ++++++++++++++++++++----
 lib/librte_eal/common/include/rte_log.h         | 19 +++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
 7 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/app/test/test_logs.c b/app/test/test_logs.c
index 805c568..730a86b 100644
--- a/app/test/test_logs.c
+++ b/app/test/test_logs.c
@@ -66,12 +66,12 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(CRIT, TESTAPP1, "critical message\n");
 
 	/* log in critical level */
-	rte_set_log_level(RTE_LOG_CRIT);
+	rte_log_set_global_level(RTE_LOG_CRIT);
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
@@ -79,7 +79,7 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 
diff --git a/examples/quota_watermark/qw/main.c b/examples/quota_watermark/qw/main.c
index 9162e28..b13df5b 100644
--- a/examples/quota_watermark/qw/main.c
+++ b/examples/quota_watermark/qw/main.c
@@ -311,7 +311,7 @@ main(int argc, char **argv)
 
     uint8_t port_id;
 
-    rte_set_log_level(RTE_LOG_INFO);
+    rte_log_set_global_level(RTE_LOG_INFO);
 
     ret = rte_eal_init(argc, argv);
     if (ret < 0)
diff --git a/examples/quota_watermark/qwctl/qwctl.c b/examples/quota_watermark/qwctl/qwctl.c
index 29c501c..d2c48b8 100644
--- a/examples/quota_watermark/qwctl/qwctl.c
+++ b/examples/quota_watermark/qwctl/qwctl.c
@@ -75,7 +75,7 @@ int main(int argc, char **argv)
     int ret;
     struct cmdline *cl;
 
-    rte_set_log_level(RTE_LOG_INFO);
+    rte_log_set_global_level(RTE_LOG_INFO);
 
     ret = rte_eal_init(argc, argv);
     if (ret < 0)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 5668d48..7e190cf 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -185,6 +185,8 @@ DPDK_17.02 {
 	rte_bus_unregister;
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index b35c8b2..3888044 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -86,20 +86,36 @@ rte_openlog_stream(FILE *f)
 
 /* Set global log level */
 void
-rte_set_log_level(uint32_t level)
+rte_log_set_global_level(uint32_t level)
 {
 	rte_logs.level = (uint32_t)level;
 }
 
+/* Set global log level */
+/* replaced by rte_log_set_global_level */
+__rte_deprecated void
+rte_set_log_level(uint32_t level)
+{
+	rte_log_set_global_level(level);
+}
+
 /* Get global log level */
 uint32_t
-rte_get_log_level(void)
+rte_log_get_global_level(void)
 {
 	return rte_logs.level;
 }
 
+/* Get global log level */
+/* replaced by rte_log_get_global_level */
+uint32_t
+rte_get_log_level(void)
+{
+	return rte_log_get_global_level();
+}
+
 /* Set global log type */
-void
+__rte_deprecated void
 rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
@@ -116,7 +132,7 @@ rte_set_log_type(uint32_t type, int enable)
 }
 
 /* Get global log type */
-uint32_t
+__rte_deprecated uint32_t
 rte_get_log_type(void)
 {
 	return rte_logs.type;
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index ce48b07..27c40b4 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,6 +50,8 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+#include <rte_common.h>
+
 struct rte_log_dynamic_type;
 
 /** The rte_log structure. */
@@ -132,11 +134,26 @@ int rte_openlog_stream(FILE *f);
  * @param level
  *   Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
  */
+void rte_log_set_global_level(uint32_t level);
+
+/**
+ * Deprecated, replaced by rte_log_set_global_level().
+ */
+__rte_deprecated
 void rte_set_log_level(uint32_t level);
 
 /**
  * Get the global log level.
+ *
+ * @return
+ *   The current global log level.
+ */
+uint32_t rte_log_get_global_level(void);
+
+/**
+ * Deprecated, replaced by rte_log_get_global_level().
  */
+__rte_deprecated
 uint32_t rte_get_log_level(void);
 
 /**
@@ -147,11 +164,13 @@ uint32_t rte_get_log_level(void);
  * @param enable
  *   True for enable; false for disable.
  */
+__rte_deprecated
 void rte_set_log_type(uint32_t type, int enable);
 
 /**
  * Get the global log type.
  */
+__rte_deprecated
 uint32_t rte_get_log_type(void);
 
 /**
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index f133c4f..81c092f 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -189,6 +189,8 @@ DPDK_17.02 {
 	rte_bus_unregister;
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
-- 
2.8.1

[RFC 6/8] app/test: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test/commands.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/test/commands.c b/app/test/commands.c
index 2df46b0..488dc41 100644
--- a/app/test/commands.c
+++ b/app/test/commands.c
@@ -158,13 +158,15 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
 	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
 				 "dump_physmem#dump_memzone#"
 				 "dump_struct_sizes#dump_ring#dump_mempool#"
-				 "dump_devargs");
+				 "dump_devargs#dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.8.1

[RFC 7/8] app/testpmd: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/cmdline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 43fc636..849d807 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7673,6 +7673,8 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
@@ -7682,7 +7684,8 @@ cmdline_parse_token_string_t cmd_dump_dump =
 		"dump_struct_sizes#"
 		"dump_ring#"
 		"dump_mempool#"
-		"dump_devargs");
+		"dump_devargs#"
+		"dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.8.1

[RFC 8/8] net/i40e: use dynamic log type for control logs

[Olivier Matz]

This is an example of how a dynamic log type can be used in a
PMD.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 drivers/net/i40e/i40e_ethdev.c | 18 ++++++++++++++++--
 drivers/net/i40e/i40e_fdir.c   |  4 ----
 drivers/net/i40e/i40e_logs.h   | 17 ++++++-----------
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 4492bcc..0beb37f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -40,6 +40,7 @@
 #include <inttypes.h>
 #include <assert.h>
 
+#include <rte_eal.h>
 #include <rte_string_fns.h>
 #include <rte_pci.h>
 #include <rte_ether.h>
@@ -419,6 +420,9 @@ static void i40e_ethertype_filter_restore(struct i40e_pf *pf);
 static void i40e_tunnel_filter_restore(struct i40e_pf *pf);
 static void i40e_filter_restore(struct i40e_pf *pf);
 
+int i40e_logtype_init;
+int i40e_logtype_driver;
+
 static const struct rte_pci_id pci_id_i40e_map[] = {
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_SFP_XL710) },
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_QEMU) },
@@ -5788,7 +5792,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(INFO, "No interrupt event");
 		goto done;
 	}
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	if (icr0 & I40E_PFINT_ICR0_ECC_ERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: unrecoverable ECC error");
 	if (icr0 & I40E_PFINT_ICR0_MAL_DETECT_MASK)
@@ -5803,7 +5806,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(ERR, "ICR0: HMC error");
 	if (icr0 & I40E_PFINT_ICR0_PE_CRITERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: protocol engine critical error");
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	if (icr0 & I40E_PFINT_ICR0_VFLR_MASK) {
 		PMD_DRV_LOG(INFO, "ICR0: VF reset detected");
@@ -11187,3 +11189,15 @@ rte_pmd_i40e_reset_vf_stats(uint8_t port,
 
 	return 0;
 }
+
+RTE_INIT(i40e_init_log);
+static void
+i40e_init_log(void)
+{
+	i40e_logtype_init = rte_log_register("pmd.i40e.init");
+	if (i40e_logtype_init >= 0)
+		rte_log_set_level(i40e_logtype_init, RTE_LOG_NOTICE);
+	i40e_logtype_driver = rte_log_register("pmd.i40e.driver");
+	if (i40e_logtype_driver >= 0)
+		rte_log_set_level(i40e_logtype_driver, RTE_LOG_NOTICE);
+}
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 0700253..32d3b19 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1592,17 +1592,14 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	struct rte_eth_dev *dev = I40E_VSI_TO_ETH_DEV(pf->main_vsi);
 	struct i40e_fdir_filter_list *fdir_list = &pf->fdir.fdir_list;
 	struct i40e_fdir_filter *f;
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint32_t fdstat;
 	uint32_t guarant_cnt;  /**< Number of filters in guaranteed spaces. */
 	uint32_t best_cnt;     /**< Number of filters in best effort spaces. */
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	TAILQ_FOREACH(f, fdir_list, rules)
 		i40e_add_del_fdir_filter(dev, &f->fdir, TRUE);
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	fdstat = I40E_READ_REG(hw, I40E_PFQF_FDSTAT);
 	guarant_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_GUARANT_CNT_MASK) >>
@@ -1610,7 +1607,6 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	best_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_BEST_CNT_MASK) >>
 			   I40E_PFQF_FDSTAT_BEST_CNT_SHIFT);
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	PMD_DRV_LOG(INFO, "FDIR: Guarant count: %d,  Best count: %d",
 		    guarant_cnt, best_cnt);
diff --git a/drivers/net/i40e/i40e_logs.h b/drivers/net/i40e/i40e_logs.h
index e042e24..8e99cd5 100644
--- a/drivers/net/i40e/i40e_logs.h
+++ b/drivers/net/i40e/i40e_logs.h
@@ -34,14 +34,11 @@
 #ifndef _I40E_LOGS_H_
 #define _I40E_LOGS_H_
 
+extern int i40e_logtype_init;
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
-
-#ifdef RTE_LIBRTE_I40E_DEBUG_INIT
+	rte_log(RTE_LOG_ ## level, i40e_logtype_init, "%s(): " fmt "\n", \
+		__func__, ##args)
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-#else
-#define PMD_INIT_FUNC_TRACE() do { } while(0)
-#endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
@@ -64,12 +61,10 @@
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
+extern int i40e_logtype_driver;
 #define PMD_DRV_LOG_RAW(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
-#else
-#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
-#endif
+	rte_log(RTE_LOG_ ## level, i40e_logtype_driver, "%s(): " fmt, \
+		__func__, ## args)
 
 #define PMD_DRV_LOG(level, fmt, args...) \
 	PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
-- 
2.8.1

Re: [RFC 0/8] eal: dynamic logs

[Bruce Richardson]

On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:
> The objective of this RFC patchset is to introduce a framework to
> support dynamic log types in EAL. It also provides one example of use
> (in i40e).
> 
> Features:
> - log types are identified by a string
> - at registration, a uniq identifier is associated to a log type
> - each log type can have its level changed dynamically
> - extend command line parameters to set the log level of a specific
>   type, or logs matching a regular expression
> - keep compat with other legacy types (eal, malloc, ring, user*,
>   etc... keep their hardcoded log type value)
> 
> At the end, when, we can expect that all non-dataplane logs are moved to
> be dynamic, so we can enable/disable them at runtime, without
> recompiling. Many debug options can probably be removed from
> configuration:
>   $ git grep DEBUG config/common_base | wc -l
>   89
> 
> Comments are welcome!
> 
Initial scan through the patches this looks pretty good. However, rather
than continuing with our own logging system, have you investigated what
other tracing and logging systems might be out there that we could
possibly re-use? Could LTTng be a good fit for DPDK, perhaps?

/Bruce

Re: [RFC 0/8] eal: dynamic logs

[Olivier Matz]

Hi Bruce,

On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson
<bruce.richardson@intel.com> wrote:
> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:
> > The objective of this RFC patchset is to introduce a framework to
> > support dynamic log types in EAL. It also provides one example of
> > use (in i40e).
> > 
> > Features:
> > - log types are identified by a string
> > - at registration, a uniq identifier is associated to a log type
> > - each log type can have its level changed dynamically
> > - extend command line parameters to set the log level of a specific
> >   type, or logs matching a regular expression
> > - keep compat with other legacy types (eal, malloc, ring, user*,
> >   etc... keep their hardcoded log type value)
> > 
> > At the end, when, we can expect that all non-dataplane logs are
> > moved to be dynamic, so we can enable/disable them at runtime,
> > without recompiling. Many debug options can probably be removed from
> > configuration:
> >   $ git grep DEBUG config/common_base | wc -l
> >   89
> > 
> > Comments are welcome!
> >   
> Initial scan through the patches this looks pretty good. However,
> rather than continuing with our own logging system, have you
> investigated what other tracing and logging systems might be out
> there that we could possibly re-use? Could LTTng be a good fit for
> DPDK, perhaps?

I did not investigate much about existing logging system. I agree
that could be a good alternative too.

On the other hand, I'm not really confident in adding a dependency to
an external lib for a core component like eal. What if we deicide
tomorrow to port dpdk to windows or any baremetal env?

Also, as far as I remember, the components that depend on external
libraries are disabled today because we don't know how to properly
manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, ...).


Olivier

Re: [RFC 0/8] eal: dynamic logs

[Wiles, Keith]

> On Feb 6, 2017, at 8:10 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> Hi Bruce,
> 
> On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson
> <bruce.richardson@intel.com> wrote:
>> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:
>>> The objective of this RFC patchset is to introduce a framework to
>>> support dynamic log types in EAL. It also provides one example of
>>> use (in i40e).
>>> 
>>> Features:
>>> - log types are identified by a string
>>> - at registration, a uniq identifier is associated to a log type
>>> - each log type can have its level changed dynamically
>>> - extend command line parameters to set the log level of a specific
>>>  type, or logs matching a regular expression
>>> - keep compat with other legacy types (eal, malloc, ring, user*,
>>>  etc... keep their hardcoded log type value)
>>> 
>>> At the end, when, we can expect that all non-dataplane logs are
>>> moved to be dynamic, so we can enable/disable them at runtime,
>>> without recompiling. Many debug options can probably be removed from
>>> configuration:
>>>  $ git grep DEBUG config/common_base | wc -l
>>>  89
>>> 
>>> Comments are welcome!
>>> 
>> Initial scan through the patches this looks pretty good. However,
>> rather than continuing with our own logging system, have you
>> investigated what other tracing and logging systems might be out
>> there that we could possibly re-use? Could LTTng be a good fit for
>> DPDK, perhaps?
> 
> I did not investigate much about existing logging system. I agree
> that could be a good alternative too.
> 
> On the other hand, I'm not really confident in adding a dependency to
> an external lib for a core component like eal. What if we deicide
> tomorrow to port dpdk to windows or any baremetal env?
> 
> Also, as far as I remember, the components that depend on external
> libraries are disabled today because we don't know how to properly
> manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, …).

In a previous project I worked in we did not use log levels per say for debugging code. We did have a couple general logging for misc type logging.

When debugging you normally only need a couple files or functions that need to produce logging output. In that case we defined logging using the file and function as the key to determine if the dynamic log messages are output. We use a string in the format of "filename:function” we allowed for a wildcard to enable all functions in a file or you can select a single function.

Using this type of logging for debugging a system is the most useful if you just want general logging then we define something similar to what we have today.

> 
> 
> Olivier

Regards,
Keith

Re: [RFC 0/8] eal: dynamic logs

[Olivier Matz]

On Mon, 6 Feb 2017 15:01:30 +0000, "Wiles, Keith"
<keith.wiles@intel.com> wrote:
> > On Feb 6, 2017, at 8:10 AM, Olivier Matz <olivier.matz@6wind.com>
> > wrote:
> > 
> > Hi Bruce,
> > 
> > On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson
> > <bruce.richardson@intel.com> wrote:  
> >> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:  
> >>> The objective of this RFC patchset is to introduce a framework to
> >>> support dynamic log types in EAL. It also provides one example of
> >>> use (in i40e).
> >>> 
> >>> Features:
> >>> - log types are identified by a string
> >>> - at registration, a uniq identifier is associated to a log type
> >>> - each log type can have its level changed dynamically
> >>> - extend command line parameters to set the log level of a
> >>> specific type, or logs matching a regular expression
> >>> - keep compat with other legacy types (eal, malloc, ring, user*,
> >>>  etc... keep their hardcoded log type value)
> >>> 
> >>> At the end, when, we can expect that all non-dataplane logs are
> >>> moved to be dynamic, so we can enable/disable them at runtime,
> >>> without recompiling. Many debug options can probably be removed
> >>> from configuration:
> >>>  $ git grep DEBUG config/common_base | wc -l
> >>>  89
> >>> 
> >>> Comments are welcome!
> >>>   
> >> Initial scan through the patches this looks pretty good. However,
> >> rather than continuing with our own logging system, have you
> >> investigated what other tracing and logging systems might be out
> >> there that we could possibly re-use? Could LTTng be a good fit for
> >> DPDK, perhaps?  
> > 
> > I did not investigate much about existing logging system. I agree
> > that could be a good alternative too.
> > 
> > On the other hand, I'm not really confident in adding a dependency
> > to an external lib for a core component like eal. What if we deicide
> > tomorrow to port dpdk to windows or any baremetal env?
> > 
> > Also, as far as I remember, the components that depend on external
> > libraries are disabled today because we don't know how to properly
> > manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, …).  
> 
> In a previous project I worked in we did not use log levels per say
> for debugging code. We did have a couple general logging for misc
> type logging.
> 
> When debugging you normally only need a couple files or functions
> that need to produce logging output. In that case we defined logging
> using the file and function as the key to determine if the dynamic
> log messages are output. We use a string in the format of
> "filename:function” we allowed for a wildcard to enable all functions
> in a file or you can select a single function.
> 
> Using this type of logging for debugging a system is the most useful
> if you just want general logging then we define something similar to
> what we have today.

I think the "filename:function" is not adequate if you are not the
developer of that code. Example: you have a problem with a PMD, you
just enable all logs for this PMD (or even all PMDs), you can check it
and if you don't find the problem, you can send them on the ML for help.
I think you don't care where the code is located.

Regards,
Olivier

Re: [RFC 0/8] eal: dynamic logs

[Wiles, Keith]

> On Feb 6, 2017, at 9:27 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> On Mon, 6 Feb 2017 15:01:30 +0000, "Wiles, Keith"
> <keith.wiles@intel.com> wrote:
>>> On Feb 6, 2017, at 8:10 AM, Olivier Matz <olivier.matz@6wind.com>
>>> wrote:
>>> 
>>> Hi Bruce,
>>> 
>>> On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson
>>> <bruce.richardson@intel.com> wrote:  
>>>> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:  
>>>>> The objective of this RFC patchset is to introduce a framework to
>>>>> support dynamic log types in EAL. It also provides one example of
>>>>> use (in i40e).
>>>>> 
>>>>> Features:
>>>>> - log types are identified by a string
>>>>> - at registration, a uniq identifier is associated to a log type
>>>>> - each log type can have its level changed dynamically
>>>>> - extend command line parameters to set the log level of a
>>>>> specific type, or logs matching a regular expression
>>>>> - keep compat with other legacy types (eal, malloc, ring, user*,
>>>>> etc... keep their hardcoded log type value)
>>>>> 
>>>>> At the end, when, we can expect that all non-dataplane logs are
>>>>> moved to be dynamic, so we can enable/disable them at runtime,
>>>>> without recompiling. Many debug options can probably be removed
>>>>> from configuration:
>>>>> $ git grep DEBUG config/common_base | wc -l
>>>>> 89
>>>>> 
>>>>> Comments are welcome!
>>>>> 
>>>> Initial scan through the patches this looks pretty good. However,
>>>> rather than continuing with our own logging system, have you
>>>> investigated what other tracing and logging systems might be out
>>>> there that we could possibly re-use? Could LTTng be a good fit for
>>>> DPDK, perhaps?  
>>> 
>>> I did not investigate much about existing logging system. I agree
>>> that could be a good alternative too.
>>> 
>>> On the other hand, I'm not really confident in adding a dependency
>>> to an external lib for a core component like eal. What if we deicide
>>> tomorrow to port dpdk to windows or any baremetal env?
>>> 
>>> Also, as far as I remember, the components that depend on external
>>> libraries are disabled today because we don't know how to properly
>>> manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, …).  
>> 
>> In a previous project I worked in we did not use log levels per say
>> for debugging code. We did have a couple general logging for misc
>> type logging.
>> 
>> When debugging you normally only need a couple files or functions
>> that need to produce logging output. In that case we defined logging
>> using the file and function as the key to determine if the dynamic
>> log messages are output. We use a string in the format of
>> "filename:function” we allowed for a wildcard to enable all functions
>> in a file or you can select a single function.
>> 
>> Using this type of logging for debugging a system is the most useful
>> if you just want general logging then we define something similar to
>> what we have today.
> 
> I think the "filename:function" is not adequate if you are not the
> developer of that code. Example: you have a problem with a PMD, you
> just enable all logs for this PMD (or even all PMDs), you can check it
> and if you don't find the problem, you can send them on the ML for help.
> I think you don't care where the code is located.

I do not understand your concern the design allows you to enable a single file, which means all functions within a file “filename:*". In the case of the all PMDs it not the best way to debug as you get a lot of output that may not be even related to the problem you are trying to solve. The design does allow you to enable one or more PMDs if say you are debugging say two PMDs. The output would be more readable and less cluttered with output that is not germane to the problem.

If I was debugging the TAP driver I would like to just enable “rte_eth_tap_pmd.c:*” or maybe we can define a something registered other then file name e.g. rte_log_register(“tap_pmd”); “tap_pmd:*” or “tap_pmd:pmd_rx_burst” or “tap_pmd:rte_tap_pmd_probe”. We could for the PMDs just use the PMD name we define at registration.

Maybe the register option brings us closer to the same goal, but add the function or selecting a specific set functions. The design does require a more active lookup at run time for dynamic debugging and we would have to make sure if enabled it does not effect performance. We used a hash table to locate the enabled debug log output.

The design allowed us to use the command line or CLI to enable/disable logging output.

> 
> Regards,
> Olivier

Regards,
Keith

Re: [RFC 0/8] eal: dynamic logs

[Olivier Matz]

On Mon, 6 Feb 2017 15:55:20 +0000, "Wiles, Keith"
<keith.wiles@intel.com> wrote:
> > On Feb 6, 2017, at 9:27 AM, Olivier Matz <olivier.matz@6wind.com>
> > wrote:
> > 
> > On Mon, 6 Feb 2017 15:01:30 +0000, "Wiles, Keith"
> > <keith.wiles@intel.com> wrote:  
> >>> On Feb 6, 2017, at 8:10 AM, Olivier Matz <olivier.matz@6wind.com>
> >>> wrote:
> >>> 
> >>> Hi Bruce,
> >>> 
> >>> On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson
> >>> <bruce.richardson@intel.com> wrote:    
> >>>> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:    
> >>>>> The objective of this RFC patchset is to introduce a framework
> >>>>> to support dynamic log types in EAL. It also provides one
> >>>>> example of use (in i40e).
> >>>>> 
> >>>>> Features:
> >>>>> - log types are identified by a string
> >>>>> - at registration, a uniq identifier is associated to a log type
> >>>>> - each log type can have its level changed dynamically
> >>>>> - extend command line parameters to set the log level of a
> >>>>> specific type, or logs matching a regular expression
> >>>>> - keep compat with other legacy types (eal, malloc, ring, user*,
> >>>>> etc... keep their hardcoded log type value)
> >>>>> 
> >>>>> At the end, when, we can expect that all non-dataplane logs are
> >>>>> moved to be dynamic, so we can enable/disable them at runtime,
> >>>>> without recompiling. Many debug options can probably be removed
> >>>>> from configuration:
> >>>>> $ git grep DEBUG config/common_base | wc -l
> >>>>> 89
> >>>>> 
> >>>>> Comments are welcome!
> >>>>>   
> >>>> Initial scan through the patches this looks pretty good. However,
> >>>> rather than continuing with our own logging system, have you
> >>>> investigated what other tracing and logging systems might be out
> >>>> there that we could possibly re-use? Could LTTng be a good fit
> >>>> for DPDK, perhaps?    
> >>> 
> >>> I did not investigate much about existing logging system. I agree
> >>> that could be a good alternative too.
> >>> 
> >>> On the other hand, I'm not really confident in adding a dependency
> >>> to an external lib for a core component like eal. What if we
> >>> deicide tomorrow to port dpdk to windows or any baremetal env?
> >>> 
> >>> Also, as far as I remember, the components that depend on external
> >>> libraries are disabled today because we don't know how to properly
> >>> manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, …).    
> >> 
> >> In a previous project I worked in we did not use log levels per say
> >> for debugging code. We did have a couple general logging for misc
> >> type logging.
> >> 
> >> When debugging you normally only need a couple files or functions
> >> that need to produce logging output. In that case we defined
> >> logging using the file and function as the key to determine if the
> >> dynamic log messages are output. We use a string in the format of
> >> "filename:function” we allowed for a wildcard to enable all
> >> functions in a file or you can select a single function.
> >> 
> >> Using this type of logging for debugging a system is the most
> >> useful if you just want general logging then we define something
> >> similar to what we have today.  
> > 
> > I think the "filename:function" is not adequate if you are not the
> > developer of that code. Example: you have a problem with a PMD, you
> > just enable all logs for this PMD (or even all PMDs), you can check
> > it and if you don't find the problem, you can send them on the ML
> > for help. I think you don't care where the code is located.  
> 
> I do not understand your concern the design allows you to enable a
> single file, which means all functions within a file “filename:*".

I think the user doesn't want to care about the file name of the C
source.


> In
> the case of the all PMDs it not the best way to debug as you get a
> lot of output that may not be even related to the problem you are
> trying to solve. The design does allow you to enable one or more PMDs
> if say you are debugging say two PMDs. The output would be more
> readable and less cluttered with output that is not germane to the
> problem.
> 
> If I was debugging the TAP driver I would like to just enable
> “rte_eth_tap_pmd.c:*” or maybe we can define a something registered
> other then file name e.g. rte_log_register(“tap_pmd”); “tap_pmd:*” or
> “tap_pmd:pmd_rx_burst” or “tap_pmd:rte_tap_pmd_probe”. We could for
> the PMDs just use the PMD name we define at registration.

That's very similar to what is proposed in the patch...

See:

+	i40e_logtype_init = rte_log_register("pmd.i40e.init");

+	i40e_logtype_driver = rte_log_register("pmd.i40e.driver");


> 
> Maybe the register option brings us closer to the same goal, but add
> the function or selecting a specific set functions. The design does
> require a more active lookup at run time for dynamic debugging and we
> would have to make sure if enabled it does not effect performance. We
> used a hash table to locate the enabled debug log output.

Again, I don't really see the added value to be able enable logs on a
per function basis. For users, it brings more complexity.

 
> The design allowed us to use the command line or CLI to
> enable/disable logging output.

Same with this proposal.

Re: [RFC 0/8] eal: dynamic logs

[Wiles, Keith]

> On Feb 6, 2017, at 10:18 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> On Mon, 6 Feb 2017 15:55:20 +0000, "Wiles, Keith"
> <keith.wiles@intel.com> wrote:
>>> On Feb 6, 2017, at 9:27 AM, Olivier Matz <olivier.matz@6wind.com>
>>> wrote:
>>> 
>>> On Mon, 6 Feb 2017 15:01:30 +0000, "Wiles, Keith"
>>> <keith.wiles@intel.com> wrote:  
>>>>> On Feb 6, 2017, at 8:10 AM, Olivier Matz <olivier.matz@6wind.com>
>>>>> wrote:
>>>>> 
>>>>> Hi Bruce,
>>>>> 
>>>>> On Mon, 6 Feb 2017 13:49:03 +0000, Bruce Richardson
>>>>> <bruce.richardson@intel.com> wrote:    
>>>>>> On Mon, Feb 06, 2017 at 02:29:08PM +0100, Olivier Matz wrote:    
>>>>>>> The objective of this RFC patchset is to introduce a framework
>>>>>>> to support dynamic log types in EAL. It also provides one
>>>>>>> example of use (in i40e).
>>>>>>> 
>>>>>>> Features:
>>>>>>> - log types are identified by a string
>>>>>>> - at registration, a uniq identifier is associated to a log type
>>>>>>> - each log type can have its level changed dynamically
>>>>>>> - extend command line parameters to set the log level of a
>>>>>>> specific type, or logs matching a regular expression
>>>>>>> - keep compat with other legacy types (eal, malloc, ring, user*,
>>>>>>> etc... keep their hardcoded log type value)
>>>>>>> 
>>>>>>> At the end, when, we can expect that all non-dataplane logs are
>>>>>>> moved to be dynamic, so we can enable/disable them at runtime,
>>>>>>> without recompiling. Many debug options can probably be removed
>>>>>>> from configuration:
>>>>>>> $ git grep DEBUG config/common_base | wc -l
>>>>>>> 89
>>>>>>> 
>>>>>>> Comments are welcome!
>>>>>>> 
>>>>>> Initial scan through the patches this looks pretty good. However,
>>>>>> rather than continuing with our own logging system, have you
>>>>>> investigated what other tracing and logging systems might be out
>>>>>> there that we could possibly re-use? Could LTTng be a good fit
>>>>>> for DPDK, perhaps?    
>>>>> 
>>>>> I did not investigate much about existing logging system. I agree
>>>>> that could be a good alternative too.
>>>>> 
>>>>> On the other hand, I'm not really confident in adding a dependency
>>>>> to an external lib for a core component like eal. What if we
>>>>> deicide tomorrow to port dpdk to windows or any baremetal env?
>>>>> 
>>>>> Also, as far as I remember, the components that depend on external
>>>>> libraries are disabled today because we don't know how to properly
>>>>> manage the dependency (ex: pmd-pcap, vhost-numa, pmd-mlx, …).    
>>>> 
>>>> In a previous project I worked in we did not use log levels per say
>>>> for debugging code. We did have a couple general logging for misc
>>>> type logging.
>>>> 
>>>> When debugging you normally only need a couple files or functions
>>>> that need to produce logging output. In that case we defined
>>>> logging using the file and function as the key to determine if the
>>>> dynamic log messages are output. We use a string in the format of
>>>> "filename:function” we allowed for a wildcard to enable all
>>>> functions in a file or you can select a single function.
>>>> 
>>>> Using this type of logging for debugging a system is the most
>>>> useful if you just want general logging then we define something
>>>> similar to what we have today.  
>>> 
>>> I think the "filename:function" is not adequate if you are not the
>>> developer of that code. Example: you have a problem with a PMD, you
>>> just enable all logs for this PMD (or even all PMDs), you can check
>>> it and if you don't find the problem, you can send them on the ML
>>> for help. I think you don't care where the code is located.  
>> 
>> I do not understand your concern the design allows you to enable a
>> single file, which means all functions within a file “filename:*".
> 
> I think the user doesn't want to care about the file name of the C
> source.

The filename is just one way to isolate the logging output for debugging it needs to be easily defined/found. IMO 80% of debugging is locating and isolating the problem quickly and in most cases the output from a single or very small set of files even isolated to a small set of functions in those files is the best.

> 
> 
>> In
>> the case of the all PMDs it not the best way to debug as you get a
>> lot of output that may not be even related to the problem you are
>> trying to solve. The design does allow you to enable one or more PMDs
>> if say you are debugging say two PMDs. The output would be more
>> readable and less cluttered with output that is not germane to the
>> problem.
>> 
>> If I was debugging the TAP driver I would like to just enable
>> “rte_eth_tap_pmd.c:*” or maybe we can define a something registered
>> other then file name e.g. rte_log_register(“tap_pmd”); “tap_pmd:*” or
>> “tap_pmd:pmd_rx_burst” or “tap_pmd:rte_tap_pmd_probe”. We could for
>> the PMDs just use the PMD name we define at registration.
> 
> That's very similar to what is proposed in the patch...
> 
> See:
> 
> +	i40e_logtype_init = rte_log_register("pmd.i40e.init");
> 
> +	i40e_logtype_driver = rte_log_register("pmd.i40e.driver”);

These strings are just arbitrary strings unless you are forcing a format, which I did not see. The filename and function are constant strings I could easily determine as a developer. The problem with registering a string means I need to find that string or have a way to display all of the strings via a CLI.

The ‘:’ in the string is required in my suggestion and I would suggest the format of the string be defined, so the developer can wildcarding to enable messages. Requiring a format helps the developer and user to quickly locate the set of messages to log, the function level is just a clean way to isolate the logs again.

> 
> 
>> 
>> Maybe the register option brings us closer to the same goal, but add
>> the function or selecting a specific set functions. The design does
>> require a more active lookup at run time for dynamic debugging and we
>> would have to make sure if enabled it does not effect performance. We
>> used a hash table to locate the enabled debug log output.
> 
> Again, I don't really see the added value to be able enable logs on a
> per function basis. For users, it brings more complexity.

I really do not see you argument here as I am sure when debugging a function you add log messages in the function, which means you are most likely debugging that function.

Allowing the developers to selectively enable logging output in a CLI is very powerful and you can see this type of design in VPP and other systems.

Enabling a single or select set of functions allows the developer to remove all of the clutter and get only what they want, in some cases the log storage maybe limited or debugging over a serial line/internet to a remote location, ...

> 
> 
>> The design allowed us to use the command line or CLI to
>> enable/disable logging output.
> 
> Same with this proposal.

BTW, in your i40e example you do connect the logging debug to the CLI. Unless you meant something different.

I am not trying to argue here only wanting to make sure you address all of the reasons to have dynamic logging. I have not looked at the code in depth, but think it is close and needs to have a few stricter requirements we can enforce to make it simpler for developers to use.

Can the patch set be applied to a version of DPDK, so I can better understand your design?

Regards,
Keith

Re: [RFC 0/8] eal: dynamic logs

[Thomas Monjalon]

2017-02-06 14:29, Olivier Matz:
> The objective of this RFC patchset is to introduce a framework to
> support dynamic log types in EAL. It also provides one example of use
> (in i40e).
> 
> Features:
> - log types are identified by a string
> - at registration, a uniq identifier is associated to a log type
> - each log type can have its level changed dynamically
> - extend command line parameters to set the log level of a specific
>   type, or logs matching a regular expression
> - keep compat with other legacy types (eal, malloc, ring, user*,
>   etc... keep their hardcoded log type value)
> 
> At the end, when, we can expect that all non-dataplane logs are moved to
> be dynamic, so we can enable/disable them at runtime, without
> recompiling. Many debug options can probably be removed from
> configuration:
>   $ git grep DEBUG config/common_base | wc -l
>   89

I think it would be a very nice cleanup and usability improvement.

It seems that everybody agrees that dynamic logging config is better.
There were 2 comments that I want to sum up here:

1/ Why not use an external log library?

It is not obvious that there is a real benefit to use another system.
And Olivier already wrote the code for this system.
If someone writes the integration of another log system, we could
consider it.

2/ Why filtering by log type instead of file/function?

File/function filtering targets DPDK debug use cases.
For application developers or system integrators, the log type seems
a good level of abstraction for logging part of the system.
Anyway, the file/function filtering could be added later if
someone integrates it in the dynamic logging configuration system.

The conclusion is that this series seems good to integrate.
As it is a RFC, do you plan to send a refresh or can we merge it as is?

Re: [RFC 0/8] eal: dynamic logs

[Olivier Matz]

Hi Thomas,

On Wed, 15 Mar 2017 17:35:32 +0100, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 2017-02-06 14:29, Olivier Matz:
> > The objective of this RFC patchset is to introduce a framework to
> > support dynamic log types in EAL. It also provides one example of use
> > (in i40e).
> > 
> > Features:
> > - log types are identified by a string
> > - at registration, a uniq identifier is associated to a log type
> > - each log type can have its level changed dynamically
> > - extend command line parameters to set the log level of a specific
> >   type, or logs matching a regular expression
> > - keep compat with other legacy types (eal, malloc, ring, user*,
> >   etc... keep their hardcoded log type value)
> > 
> > At the end, when, we can expect that all non-dataplane logs are moved to
> > be dynamic, so we can enable/disable them at runtime, without
> > recompiling. Many debug options can probably be removed from
> > configuration:
> >   $ git grep DEBUG config/common_base | wc -l
> >   89  
> 
> I think it would be a very nice cleanup and usability improvement.
> 
> It seems that everybody agrees that dynamic logging config is better.
> There were 2 comments that I want to sum up here:
> 
> 1/ Why not use an external log library?
> 
> It is not obvious that there is a real benefit to use another system.
> And Olivier already wrote the code for this system.
> If someone writes the integration of another log system, we could
> consider it.
> 
> 2/ Why filtering by log type instead of file/function?
> 
> File/function filtering targets DPDK debug use cases.
> For application developers or system integrators, the log type seems
> a good level of abstraction for logging part of the system.
> Anyway, the file/function filtering could be added later if
> someone integrates it in the dynamic logging configuration system.
> 
> The conclusion is that this series seems good to integrate.
> As it is a RFC, do you plan to send a refresh or can we merge it as is?

I'll send a refresh, the patchset does not apply on current master.

Regards,
Olivier

[PATCH 0/8] eal: dynamic logs

[Olivier Matz]

The objective of this patchset is to introduce a framework to
support dynamic log types in EAL. It also provides one example of use
(in i40e).

Features:
- log types are identified by a string
- at registration, a uniq identifier is associated to a log type
- each log type can have its level changed dynamically
- extend command line parameters to set the log level of a specific
  type, or logs matching a regular expression
- keep compat with other legacy types (eal, malloc, ring, user*,
  etc... keep their hardcoded log type value)

Next step is to adapt drivers, libs and apps to use this new API. At the
end, we can expect that all non-dataplane logs are moved to be dynamic,
so we can enable/disable them at runtime, without recompiling. Many
debug options can probably be removed from configuration:
  $ git grep DEBUG config/common_base | wc -l
  89


RFC -> v1:
- rebase on top of current master
- fix eal help alignment
- remove unused i40e compilation options


Olivier Matz (8):
  eal: support dynamic log types
  eal: dump registered log types
  eal: change several log levels matching a regexp
  eal: change specific log levels at startup
  eal: deprecate log functions
  app/test: new command to dump log types
  app/testpmd: new command to dump log types
  net/i40e: use dynamic log type for control logs

 app/test-pmd/cmdline.c                          |   5 +-
 config/common_base                              |   2 -
 doc/guides/contributing/coding_style.rst        |  30 ++--
 drivers/net/i40e/i40e_ethdev.c                  |  18 +-
 drivers/net/i40e/i40e_fdir.c                    |   4 -
 drivers/net/i40e/i40e_logs.h                    |  17 +-
 examples/quota_watermark/qw/main.c              |   2 +-
 examples/quota_watermark/qwctl/qwctl.c          |   2 +-
 lib/librte_eal/bsdapp/eal/eal.c                 |   4 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   6 +
 lib/librte_eal/common/eal_common_log.c          | 219 +++++++++++++++++++++++-
 lib/librte_eal/common/eal_common_options.c      |  49 ++++--
 lib/librte_eal/common/include/rte_log.h         |  76 +++++++-
 lib/librte_eal/linuxapp/eal/eal.c               |   4 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   6 +
 test/test/commands.c                            |   4 +-
 test/test/test_logs.c                           |  12 +-
 17 files changed, 395 insertions(+), 65 deletions(-)

-- 
2.8.1

[PATCH 1/8] eal: support dynamic log types

[Olivier Matz]

Introduce 2 new functions to support dynamic log types:

- rte_log_register(): register a log name, and return a log type id
- rte_log_set_level(): set the log level of a given log type

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/contributing/coding_style.rst        |  30 +++--
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   2 +
 lib/librte_eal/common/eal_common_log.c          | 140 +++++++++++++++++++++++-
 lib/librte_eal/common/include/rte_log.h         |  35 +++++-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   2 +
 test/test/test_logs.c                           |   6 +-
 6 files changed, 197 insertions(+), 18 deletions(-)

diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index 4163960..d8e4a0f 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -603,22 +603,30 @@ In the DPDK environment, use the logging interface provided:
 
 .. code-block:: c
 
- #define RTE_LOGTYPE_TESTAPP1 RTE_LOGTYPE_USER1
- #define RTE_LOGTYPE_TESTAPP2 RTE_LOGTYPE_USER2
+ /* register log types for this application */
+ int my_logtype1 = rte_log_register("myapp.log1");
+ int my_logtype2 = rte_log_register("myapp.log2");
 
- /* enable these logs type */
- rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
- rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+ /* set global log level to INFO */
+ rte_log_set_global_level(RTE_LOG_INFO);
+
+ /* only display messages higher than NOTICE for log2 (default
+  * is DEBUG) */
+ rte_log_set_level(my_logtype2, RTE_LOG_NOTICE);
+
+ /* enable all PMD logs (whose identifier string starts with "pmd") */
+ rte_log_set_level_regexp("pmd.*", RTE_LOG_DEBUG);
 
  /* log in debug level */
- rte_set_log_level(RTE_LOG_DEBUG);
- RTE_LOG(DEBUG, TESTAPP1, "this is is a debug level message\n");
- RTE_LOG(INFO, TESTAPP1, "this is is a info level message\n");
- RTE_LOG(WARNING, TESTAPP1, "this is is a warning level message\n");
+ rte_log_set_global_level(RTE_LOG_DEBUG);
+ RTE_LOG(DEBUG, my_logtype1, "this is is a debug level message\n");
+ RTE_LOG(INFO, my_logtype1, "this is is a info level message\n");
+ RTE_LOG(WARNING, my_logtype1, "this is is a warning level message\n");
+ RTE_LOG(WARNING, my_logtype2, "this is is a debug level message (not displayed)\n");
 
  /* log in info level */
- rte_set_log_level(RTE_LOG_INFO);
- RTE_LOG(DEBUG, TESTAPP2, "debug level message (not displayed)\n");
+ rte_log_set_global_level(RTE_LOG_INFO);
+ RTE_LOG(DEBUG, my_logtype1, "debug level message (not displayed)\n");
 
 Branch Prediction
 ~~~~~~~~~~~~~~~~~
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 67f2ffb..6b3b386 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -180,5 +180,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_register;
+	rte_log_set_level;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 2197558..cc8da26 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -35,7 +35,10 @@
 #include <stdint.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <string.h>
+#include <errno.h>
 
+#include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
 
@@ -46,6 +49,8 @@ struct rte_logs rte_logs = {
 	.type = ~0,
 	.level = RTE_LOG_DEBUG,
 	.file = NULL,
+	.dynamic_types_len = 0,
+	.dynamic_types = NULL,
 };
 
 /* Stream to use for logging if rte_logs.file is NULL */
@@ -60,6 +65,11 @@ struct log_cur_msg {
 	uint32_t logtype;  /**< log type  - see rte_log.h */
 };
 
+struct rte_log_dynamic_type {
+	const char *name;
+	uint32_t loglevel;
+};
+
  /* per core log */
 static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg);
 
@@ -91,10 +101,17 @@ rte_get_log_level(void)
 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_logs.type |= type;
+		rte_log_set_level(type, 0);
 	else
-		rte_logs.type &= (~type);
+		rte_log_set_level(type, RTE_LOG_DEBUG);
 }
 
 /* Get global log type */
@@ -104,6 +121,19 @@ rte_get_log_type(void)
 	return rte_logs.type;
 }
 
+int
+rte_log_set_level(uint32_t type, uint32_t level)
+{
+	if (type >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
@@ -116,6 +146,106 @@ int rte_log_cur_msg_logtype(void)
 	return RTE_PER_LCORE(log_cur_msg).logtype;
 }
 
+static int
+rte_log_lookup(const char *name)
+{
+	size_t i;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (strcmp(name, rte_logs.dynamic_types[i].name) == 0)
+			return i;
+	}
+
+	return -1;
+}
+
+/* register an extended log type, assuming table is large enough, and id
+ * is not yet registered.
+ */
+static int
+__rte_log_register(const char *name, int id)
+{
+	char *dup_name = NULL;
+
+	dup_name = strdup(name);
+	if (dup_name == NULL)
+		return -ENOMEM;
+
+	rte_logs.dynamic_types[id].name = dup_name;
+	rte_logs.dynamic_types[id].loglevel = RTE_LOG_DEBUG;
+
+	return id;
+}
+
+/* register an extended log type */
+int
+rte_log_register(const char *name)
+{
+	struct rte_log_dynamic_type *new_dynamic_types;
+	int id, ret;
+
+	id = rte_log_lookup(name);
+	if (id >= 0)
+		return id;
+
+	new_dynamic_types = realloc(rte_logs.dynamic_types,
+		sizeof(struct rte_log_dynamic_type) *
+		(rte_logs.dynamic_types_len + 1));
+	if (new_dynamic_types == NULL)
+		return -ENOMEM;
+	rte_logs.dynamic_types = new_dynamic_types;
+
+	ret = __rte_log_register(name, rte_logs.dynamic_types_len);
+	if (ret < 0)
+		return ret;
+
+	rte_logs.dynamic_types_len++;
+
+	return ret;
+}
+
+RTE_INIT(rte_log_init);
+static void
+rte_log_init(void)
+{
+	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);
+
+	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
@@ -139,7 +269,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 		}
 	}
 
-	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
+	if (level > rte_logs.level)
+		return 0;
+	if (logtype >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > rte_logs.dynamic_types[logtype].loglevel)
 		return 0;
 
 	/* save loglevel and logtype in a global per-lcore variable */
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 954b96c..e71887f 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,17 +50,21 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+struct rte_log_dynamic_type;
+
 /** The rte_log structure. */
 struct rte_logs {
 	uint32_t type;  /**< Bitfield with enabled logs. */
 	uint32_t level; /**< Log level. */
 	FILE *file;     /**< Output file set by rte_openlog_stream, or NULL. */
+	size_t dynamic_types_len;
+	struct rte_log_dynamic_type *dynamic_types;
 };
 
 /** Global log informations */
 extern struct rte_logs rte_logs;
 
-/* SDK log type */
+/* 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. */
@@ -91,6 +95,9 @@ extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_USER7   0x40000000 /**< User-defined log type 7. */
 #define RTE_LOGTYPE_USER8   0x80000000 /**< User-defined log type 8. */
 
+/** First identifier for extended logs */
+#define RTE_LOGTYPE_FIRST_EXT_ID 32
+
 /* Can't use 0, as it gives compiler warnings */
 #define RTE_LOG_EMERG    1U  /**< System is unusable.               */
 #define RTE_LOG_ALERT    2U  /**< Action must be taken immediately. */
@@ -148,6 +155,18 @@ void rte_set_log_type(uint32_t type, int enable);
 uint32_t rte_get_log_type(void);
 
 /**
+ * Set the log level for a given type.
+ *
+ * @param logtype
+ *   The log type identifier.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if logtype or level is invalid.
+ */
+int rte_log_set_level(uint32_t logtype, uint32_t level);
+
+/**
  * Get the current loglevel for the message being processed.
  *
  * Before calling the user-defined stream for logging, the log
@@ -176,6 +195,20 @@ int rte_log_cur_msg_loglevel(void);
 int rte_log_cur_msg_logtype(void);
 
 /**
+ * Register a dynamic log type
+ *
+ * If a log is already registered with the same type, the returned value
+ * is the same than the previous one.
+ *
+ * @param name
+ *   The string identifying the log type.
+ * @return
+ *   - >0: success, the returned value is the log type identifier.
+ *   - (-ENONEM): cannot allocate memory.
+ */
+int rte_log_register(const char *name);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 9c134b4..8375a9d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -184,5 +184,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_register;
+	rte_log_set_level;
 
 } DPDK_16.11;
diff --git a/test/test/test_logs.c b/test/test/test_logs.c
index 6985ddd..805c568 100644
--- a/test/test/test_logs.c
+++ b/test/test/test_logs.c
@@ -62,8 +62,8 @@ static int
 test_logs(void)
 {
 	/* enable these logs type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP1, RTE_LOG_EMERG);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
@@ -76,7 +76,7 @@ test_logs(void)
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
 	/* disable one log type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 0);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
-- 
2.8.1

Re: [PATCH 1/8] eal: support dynamic log types

[Stephen Hemminger]

On Fri, 17 Mar 2017 16:51:15 +0100
Olivier Matz <olivier.matz@6wind.com> wrote:

> +	.dynamic_types_len = 0,
> +	.dynamic_types = NULL,
>  };
>  
You don't need to add elements to initializer if the are 0.

Re: [PATCH 1/8] eal: support dynamic log types

[Stephen Hemminger]

On Fri, 17 Mar 2017 16:51:15 +0100
Olivier Matz <olivier.matz@6wind.com> wrote:

> 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
> +		if (enable)
> +			rte_logs.type |= type;
> +		else
> +			rte_logs.type &= (~type)

No need for () around ~type

Re: [PATCH 1/8] eal: support dynamic log types

[Stephen Hemminger]

On Fri, 17 Mar 2017 16:51:15 +0100
Olivier Matz <olivier.matz@6wind.com> wrote:

> +static int
> +rte_log_lookup(const char *name)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
> +		if (rte_logs.dynamic_types[i].name == NULL)
> +			continue;
> +		if (strcmp(name, rte_logs.dynamic_types[i].name) == 0)
> +			return i;
> +	}
> +
> +	return -1;
> +}

Maybe use strcasecmp to allow for compatibility with old upper case names?

Re: [PATCH 1/8] eal: support dynamic log types

[Olivier Matz]

Hi Stephen,

On Fri, 17 Mar 2017 09:15:28 -0700, Stephen Hemminger <stephen@networkplumber.org> wrote:
> On Fri, 17 Mar 2017 16:51:15 +0100
> Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> > +static int
> > +rte_log_lookup(const char *name)
> > +{
> > +	size_t i;
> > +
> > +	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
> > +		if (rte_logs.dynamic_types[i].name == NULL)
> > +			continue;
> > +		if (strcmp(name, rte_logs.dynamic_types[i].name) == 0)
> > +			return i;
> > +	}
> > +
> > +	return -1;
> > +}  
> 
> Maybe use strcasecmp to allow for compatibility with old upper case names?

There was no upper case name before (just macros), so I don't think
this is needed.


I'll take care of your other remarks (cf other mails).

Thanks for the review
Olivier

Re: [PATCH 1/8] eal: support dynamic log types

[Stephen Hemminger]

On Fri, 17 Mar 2017 16:51:15 +0100
Olivier Matz <olivier.matz@6wind.com> wrote:

> +static int
> +__rte_log_register(const char *name, int id)
> +{
> +	char *dup_name = NULL;
> +
> +	dup_name = strdup(name);

Useless initialization!

Many people were taught to always initialize variables. But that was before
compilers were smart enough to catch those errors.

[PATCH 2/8] eal: dump registered log types

[Olivier Matz]

Introduce a function to dump the global level and the registered log
types.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 34 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 10 ++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 46 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 6b3b386..d7a46a7 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -180,6 +180,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index cc8da26..a311279 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -246,6 +246,40 @@ rte_log_init(void)
 	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
 }
 
+static const char *
+loglevel_to_string(uint32_t level)
+{
+	switch (level) {
+	case RTE_LOG_EMERG: return "emerg";
+	case RTE_LOG_ALERT: return "alert";
+	case RTE_LOG_CRIT: return "critical";
+	case RTE_LOG_ERR: return "error";
+	case RTE_LOG_WARNING: return "warning";
+	case RTE_LOG_NOTICE: return "notice";
+	case RTE_LOG_INFO: return "info";
+	case RTE_LOG_DEBUG: return "debug";
+	default: return "unknown";
+	}
+}
+
+/* dump global level and registered log types */
+void
+rte_log_dump(FILE *f)
+{
+	size_t i;
+
+	fprintf(f, "global log level is %s\n",
+		loglevel_to_string(rte_log_get_global_level()));
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		fprintf(f, "id %zu: %s, level is %s\n",
+			i, rte_logs.dynamic_types[i].name,
+			loglevel_to_string(rte_logs.dynamic_types[i].loglevel));
+	}
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index e71887f..97e0c5e 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -209,6 +209,16 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * Dump log information.
+ *
+ * Dump the global level and the registered log types.
+ *
+ * @param f
+ *   The output stream where the dump should be sent.
+ */
+void rte_log_dump(FILE *f);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 8375a9d..96cb961 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -184,6 +184,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
-- 
2.8.1

[PATCH 3/8] eal: change several log levels matching a regexp

[Olivier Matz]

Introduce a function to set the log level of several log types that
match a regular expression.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 21 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 12 ++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 35 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index d7a46a7..be829cb 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -183,5 +183,6 @@ DPDK_17.02 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index a311279..b35c8b2 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -37,6 +37,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <regex.h>
 
 #include <rte_eal.h>
 #include <rte_log.h>
@@ -134,6 +135,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
 	return 0;
 }
 
+/* set level */
+int
+rte_log_set_level_regexp(const char *pattern, uint32_t level)
+{
+	regex_t r;
+	size_t i;
+
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (regexec(&r, pattern, 0, NULL, 0) == 0)
+			rte_logs.dynamic_types[i].loglevel = level;
+	}
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 97e0c5e..ce48b07 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -157,6 +157,18 @@ uint32_t rte_get_log_type(void);
 /**
  * Set the log level for a given type.
  *
+ * @param pattern
+ *   The regexp identifying the log type.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if level is invalid.
+ */
+int rte_log_set_level_regexp(const char *pattern, uint32_t level);
+
+/**
+ * Set the log level for a given type.
+ *
  * @param logtype
  *   The log type identifier.
  * @param level
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 96cb961..a8116ed 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -187,5 +187,6 @@ DPDK_17.02 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_16.11;
-- 
2.8.1

[PATCH 4/8] eal: change specific log levels at startup

[Olivier Matz]

Example of use:
  ./app/test-pmd --log-level='pmd\.i40e.*,8'

  This enables debug logs for all dynamic logs whose type starts with
  'pmd.i40e'.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            |  4 +--
 lib/librte_eal/common/eal_common_options.c | 49 +++++++++++++++++++++++-------
 lib/librte_eal/linuxapp/eal/eal.c          |  4 +--
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index ee7c9de..e945468 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -505,10 +505,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0)
 		rte_panic("Cannot detect lcores\n");
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index f36bc55..32df2ef 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -739,25 +739,53 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
 }
 
 static int
-eal_parse_log_level(const char *level, uint32_t *log_level)
+eal_parse_log_level(const char *arg, struct internal_config *conf)
 {
-	char *end;
+	char *end, *str, *type, *level;
 	unsigned long tmp;
 
+	str = strdup(arg);
+	if (str == NULL)
+		return -1;
+
+	if (strchr(str, ',') == NULL) {
+		type = NULL;
+		level = str;
+	} else {
+		type = strsep(&str, ",");
+		level = strsep(&str, ",");
+	}
+
 	errno = 0;
 	tmp = strtoul(level, &end, 0);
 
 	/* check for errors */
 	if ((errno != 0) || (level[0] == '\0') ||
-	    end == NULL || (*end != '\0'))
-		return -1;
+		    end == NULL || (*end != '\0'))
+		goto fail;
 
 	/* log_level is a uint32_t */
 	if (tmp >= UINT32_MAX)
-		return -1;
+		goto fail;
+
+	printf("set log level %s,%lu\n",
+		type, tmp);
+
+	if (type == NULL) {
+		conf->log_level = tmp;
+		rte_log_set_global_level(tmp);
+	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
+		printf("cannot set log level %s,%lu\n",
+			type, tmp);
+		goto fail;
+	}
 
-	*log_level = tmp;
+	free(str);
 	return 0;
+
+fail:
+	free(str);
+	return -1;
 }
 
 static enum rte_proc_type_t
@@ -898,15 +926,12 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_LOG_LEVEL_NUM: {
-		uint32_t log;
-
-		if (eal_parse_log_level(optarg, &log) < 0) {
+		if (eal_parse_log_level(optarg, conf) < 0) {
 			RTE_LOG(ERR, EAL,
 				"invalid parameters for --"
 				OPT_LOG_LEVEL "\n");
 			return -1;
 		}
-		conf->log_level = log;
 		break;
 	}
 	case OPT_LCORES_NUM:
@@ -1057,7 +1082,9 @@ eal_common_usage(void)
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
-	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
+	       "  --"OPT_LOG_LEVEL"=<int>   Set global log level\n"
+	       "  --"OPT_LOG_LEVEL"=<type-regexp>,<int>\n"
+	       "                      Set specific log level\n"
 	       "  -v                  Display version information on startup\n"
 	       "  -h, --help          This help\n"
 	       "\nEAL options for DEBUG use only:\n"
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index bf6b818..0f61447 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -762,10 +762,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0)
 		rte_panic("Cannot detect lcores\n");
-- 
2.8.1

[PATCH 5/8] eal: deprecate log functions

[Olivier Matz]

Deprecate the following functions:
- rte_set_log_level(), replaced by rte_log_set_global_level()
- rte_get_log_level(), replaced by rte_log_get_global_level()
- rte_set_log_type(), rte_log_set_level()
- rte_get_log_type(), rte_log_get_level()

The new functions provide a better control of the per-type log level,
and have a better name prefix (rte_log_).

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 examples/quota_watermark/qw/main.c              |  2 +-
 examples/quota_watermark/qwctl/qwctl.c          |  2 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
 lib/librte_eal/common/eal_common_log.c          | 24 ++++++++++++++++++++----
 lib/librte_eal/common/include/rte_log.h         | 19 +++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
 test/test/test_logs.c                           |  6 +++---
 7 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/examples/quota_watermark/qw/main.c b/examples/quota_watermark/qw/main.c
index 57df8ef..5ba74e2 100644
--- a/examples/quota_watermark/qw/main.c
+++ b/examples/quota_watermark/qw/main.c
@@ -321,7 +321,7 @@ main(int argc, char **argv)
 
 	uint8_t port_id;
 
-	rte_set_log_level(RTE_LOG_INFO);
+	rte_log_set_global_level(RTE_LOG_INFO);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
diff --git a/examples/quota_watermark/qwctl/qwctl.c b/examples/quota_watermark/qwctl/qwctl.c
index 3a85cc3..7db66a1 100644
--- a/examples/quota_watermark/qwctl/qwctl.c
+++ b/examples/quota_watermark/qwctl/qwctl.c
@@ -75,7 +75,7 @@ int main(int argc, char **argv)
 	int ret;
 	struct cmdline *cl;
 
-	rte_set_log_level(RTE_LOG_INFO);
+	rte_log_set_global_level(RTE_LOG_INFO);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index be829cb..bccaafa 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -182,6 +182,8 @@ DPDK_17.02 {
 	rte_bus_unregister;
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index b35c8b2..3888044 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -86,20 +86,36 @@ rte_openlog_stream(FILE *f)
 
 /* Set global log level */
 void
-rte_set_log_level(uint32_t level)
+rte_log_set_global_level(uint32_t level)
 {
 	rte_logs.level = (uint32_t)level;
 }
 
+/* Set global log level */
+/* replaced by rte_log_set_global_level */
+__rte_deprecated void
+rte_set_log_level(uint32_t level)
+{
+	rte_log_set_global_level(level);
+}
+
 /* Get global log level */
 uint32_t
-rte_get_log_level(void)
+rte_log_get_global_level(void)
 {
 	return rte_logs.level;
 }
 
+/* Get global log level */
+/* replaced by rte_log_get_global_level */
+uint32_t
+rte_get_log_level(void)
+{
+	return rte_log_get_global_level();
+}
+
 /* Set global log type */
-void
+__rte_deprecated void
 rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
@@ -116,7 +132,7 @@ rte_set_log_type(uint32_t type, int enable)
 }
 
 /* Get global log type */
-uint32_t
+__rte_deprecated uint32_t
 rte_get_log_type(void)
 {
 	return rte_logs.type;
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index ce48b07..27c40b4 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,6 +50,8 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+#include <rte_common.h>
+
 struct rte_log_dynamic_type;
 
 /** The rte_log structure. */
@@ -132,11 +134,26 @@ int rte_openlog_stream(FILE *f);
  * @param level
  *   Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
  */
+void rte_log_set_global_level(uint32_t level);
+
+/**
+ * Deprecated, replaced by rte_log_set_global_level().
+ */
+__rte_deprecated
 void rte_set_log_level(uint32_t level);
 
 /**
  * Get the global log level.
+ *
+ * @return
+ *   The current global log level.
+ */
+uint32_t rte_log_get_global_level(void);
+
+/**
+ * Deprecated, replaced by rte_log_get_global_level().
  */
+__rte_deprecated
 uint32_t rte_get_log_level(void);
 
 /**
@@ -147,11 +164,13 @@ uint32_t rte_get_log_level(void);
  * @param enable
  *   True for enable; false for disable.
  */
+__rte_deprecated
 void rte_set_log_type(uint32_t type, int enable);
 
 /**
  * Get the global log type.
  */
+__rte_deprecated
 uint32_t rte_get_log_type(void);
 
 /**
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index a8116ed..3b50c4c 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -186,6 +186,8 @@ DPDK_17.02 {
 	rte_bus_unregister;
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/test/test/test_logs.c b/test/test/test_logs.c
index 805c568..730a86b 100644
--- a/test/test/test_logs.c
+++ b/test/test/test_logs.c
@@ -66,12 +66,12 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(CRIT, TESTAPP1, "critical message\n");
 
 	/* log in critical level */
-	rte_set_log_level(RTE_LOG_CRIT);
+	rte_log_set_global_level(RTE_LOG_CRIT);
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
@@ -79,7 +79,7 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 
-- 
2.8.1

[PATCH 6/8] app/test: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 test/test/commands.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test/commands.c b/test/test/commands.c
index 2df46b0..488dc41 100644
--- a/test/test/commands.c
+++ b/test/test/commands.c
@@ -158,13 +158,15 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
 	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
 				 "dump_physmem#dump_memzone#"
 				 "dump_struct_sizes#dump_ring#dump_mempool#"
-				 "dump_devargs");
+				 "dump_devargs#dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.8.1

[PATCH 7/8] app/testpmd: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/cmdline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..697228f 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7679,6 +7679,8 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
@@ -7688,7 +7690,8 @@ cmdline_parse_token_string_t cmd_dump_dump =
 		"dump_struct_sizes#"
 		"dump_ring#"
 		"dump_mempool#"
-		"dump_devargs");
+		"dump_devargs#"
+		"dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.8.1

[PATCH 8/8] net/i40e: use dynamic log type for control logs

[Olivier Matz]

This is an example of how a dynamic log type can be used in a
PMD.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 config/common_base             |  2 --
 drivers/net/i40e/i40e_ethdev.c | 18 ++++++++++++++++--
 drivers/net/i40e/i40e_fdir.c   |  4 ----
 drivers/net/i40e/i40e_logs.h   | 17 ++++++-----------
 4 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/config/common_base b/config/common_base
index 37aa1e1..2cf2b9c 100644
--- a/config/common_base
+++ b/config/common_base
@@ -179,11 +179,9 @@ CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=y
 # Compile burst-oriented I40E PMD driver
 #
 CONFIG_RTE_LIBRTE_I40E_PMD=y
-CONFIG_RTE_LIBRTE_I40E_DEBUG_INIT=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX_FREE=n
-CONFIG_RTE_LIBRTE_I40E_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y
 CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y
 CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE=y
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 303027b..1ffa6d5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -40,6 +40,7 @@
 #include <inttypes.h>
 #include <assert.h>
 
+#include <rte_eal.h>
 #include <rte_string_fns.h>
 #include <rte_pci.h>
 #include <rte_ether.h>
@@ -419,6 +420,9 @@ static void i40e_ethertype_filter_restore(struct i40e_pf *pf);
 static void i40e_tunnel_filter_restore(struct i40e_pf *pf);
 static void i40e_filter_restore(struct i40e_pf *pf);
 
+int i40e_logtype_init;
+int i40e_logtype_driver;
+
 static const struct rte_pci_id pci_id_i40e_map[] = {
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_SFP_XL710) },
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_QEMU) },
@@ -5788,7 +5792,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(INFO, "No interrupt event");
 		goto done;
 	}
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	if (icr0 & I40E_PFINT_ICR0_ECC_ERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: unrecoverable ECC error");
 	if (icr0 & I40E_PFINT_ICR0_MAL_DETECT_MASK)
@@ -5803,7 +5806,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(ERR, "ICR0: HMC error");
 	if (icr0 & I40E_PFINT_ICR0_PE_CRITERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: protocol engine critical error");
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	if (icr0 & I40E_PFINT_ICR0_VFLR_MASK) {
 		PMD_DRV_LOG(INFO, "ICR0: VF reset detected");
@@ -11212,3 +11214,15 @@ rte_pmd_i40e_reset_vf_stats(uint8_t port,
 
 	return 0;
 }
+
+RTE_INIT(i40e_init_log);
+static void
+i40e_init_log(void)
+{
+	i40e_logtype_init = rte_log_register("pmd.i40e.init");
+	if (i40e_logtype_init >= 0)
+		rte_log_set_level(i40e_logtype_init, RTE_LOG_NOTICE);
+	i40e_logtype_driver = rte_log_register("pmd.i40e.driver");
+	if (i40e_logtype_driver >= 0)
+		rte_log_set_level(i40e_logtype_driver, RTE_LOG_NOTICE);
+}
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 0700253..32d3b19 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1592,17 +1592,14 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	struct rte_eth_dev *dev = I40E_VSI_TO_ETH_DEV(pf->main_vsi);
 	struct i40e_fdir_filter_list *fdir_list = &pf->fdir.fdir_list;
 	struct i40e_fdir_filter *f;
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint32_t fdstat;
 	uint32_t guarant_cnt;  /**< Number of filters in guaranteed spaces. */
 	uint32_t best_cnt;     /**< Number of filters in best effort spaces. */
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	TAILQ_FOREACH(f, fdir_list, rules)
 		i40e_add_del_fdir_filter(dev, &f->fdir, TRUE);
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	fdstat = I40E_READ_REG(hw, I40E_PFQF_FDSTAT);
 	guarant_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_GUARANT_CNT_MASK) >>
@@ -1610,7 +1607,6 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	best_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_BEST_CNT_MASK) >>
 			   I40E_PFQF_FDSTAT_BEST_CNT_SHIFT);
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	PMD_DRV_LOG(INFO, "FDIR: Guarant count: %d,  Best count: %d",
 		    guarant_cnt, best_cnt);
diff --git a/drivers/net/i40e/i40e_logs.h b/drivers/net/i40e/i40e_logs.h
index e042e24..8e99cd5 100644
--- a/drivers/net/i40e/i40e_logs.h
+++ b/drivers/net/i40e/i40e_logs.h
@@ -34,14 +34,11 @@
 #ifndef _I40E_LOGS_H_
 #define _I40E_LOGS_H_
 
+extern int i40e_logtype_init;
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
-
-#ifdef RTE_LIBRTE_I40E_DEBUG_INIT
+	rte_log(RTE_LOG_ ## level, i40e_logtype_init, "%s(): " fmt "\n", \
+		__func__, ##args)
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-#else
-#define PMD_INIT_FUNC_TRACE() do { } while(0)
-#endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
@@ -64,12 +61,10 @@
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
+extern int i40e_logtype_driver;
 #define PMD_DRV_LOG_RAW(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
-#else
-#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
-#endif
+	rte_log(RTE_LOG_ ## level, i40e_logtype_driver, "%s(): " fmt, \
+		__func__, ## args)
 
 #define PMD_DRV_LOG(level, fmt, args...) \
 	PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
-- 
2.8.1

[PATCH v2 0/8] eal: dynamic logs

[Olivier Matz]

The objective of this patchset is to introduce a framework to
support dynamic log types in EAL. It also provides one example of use
(in i40e).

Features:
- log types are identified by a string
- at registration, a uniq identifier is associated to a log type
- each log type can have its level changed dynamically
- extend command line parameters to set the log level of a specific
  type, or logs matching a regular expression
- keep compat with other legacy types (eal, malloc, ring, user*,
  etc... keep their hardcoded log type value)

Next step is to adapt drivers, libs and apps to use this new API. At the
end, we can expect that all non-dataplane logs are moved to be dynamic,
so we can enable/disable them at runtime, without recompiling. Many
debug options can probably be removed from configuration:
  $ git grep DEBUG config/common_base | wc -l
  89


v1 -> v2:
- fix issues reported by Stephen
- add entry in release note and deprecation notice
- rebase on top of master

RFC -> v1:
- rebase on top of current master
- fix eal help alignment
- remove unused i40e compilation options


Olivier Matz (8):
  eal: support dynamic log types
  eal: dump registered log types
  eal: change several log levels matching a regexp
  eal: change specific log levels at startup
  eal: deprecate log functions
  app/test: new command to dump log types
  app/testpmd: new command to dump log types
  net/i40e: use dynamic log type for control logs

 app/test-pmd/cmdline.c                          |   5 +-
 config/common_base                              |   2 -
 doc/guides/contributing/coding_style.rst        |  30 ++--
 doc/guides/rel_notes/deprecation.rst            |   8 +
 doc/guides/rel_notes/release_17_02.rst          |   5 +
 drivers/net/i40e/i40e_ethdev.c                  |  18 +-
 drivers/net/i40e/i40e_fdir.c                    |   4 -
 drivers/net/i40e/i40e_logs.h                    |  17 +-
 examples/quota_watermark/qw/main.c              |   2 +-
 examples/quota_watermark/qwctl/qwctl.c          |   2 +-
 lib/librte_eal/bsdapp/eal/eal.c                 |   4 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   6 +
 lib/librte_eal/common/eal_common_log.c          | 216 +++++++++++++++++++++++-
 lib/librte_eal/common/eal_common_options.c      |  49 ++++--
 lib/librte_eal/common/include/rte_log.h         |  76 ++++++++-
 lib/librte_eal/linuxapp/eal/eal.c               |   4 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   6 +
 test/test/commands.c                            |   4 +-
 test/test/test_logs.c                           |  12 +-
 19 files changed, 405 insertions(+), 65 deletions(-)

-- 
2.11.0

[PATCH v2 1/8] eal: support dynamic log types

[Olivier Matz]

Introduce 2 new functions to support dynamic log types:

- rte_log_register(): register a log name, and return a log type id
- rte_log_set_level(): set the log level of a given log type

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/contributing/coding_style.rst        |  30 ++++--
 doc/guides/rel_notes/release_17_02.rst          |   5 +
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   2 +
 lib/librte_eal/common/eal_common_log.c          | 137 +++++++++++++++++++++++-
 lib/librte_eal/common/include/rte_log.h         |  35 +++++-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   2 +
 test/test/test_logs.c                           |   6 +-
 7 files changed, 199 insertions(+), 18 deletions(-)

diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index 4163960c9..d8e4a0f9c 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -603,22 +603,30 @@ In the DPDK environment, use the logging interface provided:
 
 .. code-block:: c
 
- #define RTE_LOGTYPE_TESTAPP1 RTE_LOGTYPE_USER1
- #define RTE_LOGTYPE_TESTAPP2 RTE_LOGTYPE_USER2
+ /* register log types for this application */
+ int my_logtype1 = rte_log_register("myapp.log1");
+ int my_logtype2 = rte_log_register("myapp.log2");
 
- /* enable these logs type */
- rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
- rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+ /* set global log level to INFO */
+ rte_log_set_global_level(RTE_LOG_INFO);
+
+ /* only display messages higher than NOTICE for log2 (default
+  * is DEBUG) */
+ rte_log_set_level(my_logtype2, RTE_LOG_NOTICE);
+
+ /* enable all PMD logs (whose identifier string starts with "pmd") */
+ rte_log_set_level_regexp("pmd.*", RTE_LOG_DEBUG);
 
  /* log in debug level */
- rte_set_log_level(RTE_LOG_DEBUG);
- RTE_LOG(DEBUG, TESTAPP1, "this is is a debug level message\n");
- RTE_LOG(INFO, TESTAPP1, "this is is a info level message\n");
- RTE_LOG(WARNING, TESTAPP1, "this is is a warning level message\n");
+ rte_log_set_global_level(RTE_LOG_DEBUG);
+ RTE_LOG(DEBUG, my_logtype1, "this is is a debug level message\n");
+ RTE_LOG(INFO, my_logtype1, "this is is a info level message\n");
+ RTE_LOG(WARNING, my_logtype1, "this is is a warning level message\n");
+ RTE_LOG(WARNING, my_logtype2, "this is is a debug level message (not displayed)\n");
 
  /* log in info level */
- rte_set_log_level(RTE_LOG_INFO);
- RTE_LOG(DEBUG, TESTAPP2, "debug level message (not displayed)\n");
+ rte_log_set_global_level(RTE_LOG_INFO);
+ RTE_LOG(DEBUG, my_logtype1, "debug level message (not displayed)\n");
 
 Branch Prediction
 ~~~~~~~~~~~~~~~~~
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 357965ac9..d37f6f5db 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -250,6 +250,11 @@ New Features
   See the :ref:`Elastic Flow Distributor Library <Efd_Library>` documentation in
   the Programmers Guide document, for more information.
 
+* **Added EAL dynamic log framework.**
+
+  Added new APIs to dynamically register named log types, and control
+  the level of each type independently.
+
 
 Resolved Issues
 ---------------
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 67f2ffb29..6b3b3868d 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -180,5 +180,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_register;
+	rte_log_set_level;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 219755838..68d879ae2 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -35,7 +35,10 @@
 #include <stdint.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <string.h>
+#include <errno.h>
 
+#include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
 
@@ -60,6 +63,11 @@ struct log_cur_msg {
 	uint32_t logtype;  /**< log type  - see rte_log.h */
 };
 
+struct rte_log_dynamic_type {
+	const char *name;
+	uint32_t loglevel;
+};
+
  /* per core log */
 static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg);
 
@@ -91,10 +99,17 @@ rte_get_log_level(void)
 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_logs.type |= type;
+		rte_log_set_level(type, 0);
 	else
-		rte_logs.type &= (~type);
+		rte_log_set_level(type, RTE_LOG_DEBUG);
 }
 
 /* Get global log type */
@@ -104,6 +119,19 @@ rte_get_log_type(void)
 	return rte_logs.type;
 }
 
+int
+rte_log_set_level(uint32_t type, uint32_t level)
+{
+	if (type >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
@@ -116,6 +144,105 @@ int rte_log_cur_msg_logtype(void)
 	return RTE_PER_LCORE(log_cur_msg).logtype;
 }
 
+static int
+rte_log_lookup(const char *name)
+{
+	size_t i;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (strcmp(name, rte_logs.dynamic_types[i].name) == 0)
+			return i;
+	}
+
+	return -1;
+}
+
+/* register an extended log type, assuming table is large enough, and id
+ * is not yet registered.
+ */
+static int
+__rte_log_register(const char *name, int id)
+{
+	char *dup_name = strdup(name);
+
+	if (dup_name == NULL)
+		return -ENOMEM;
+
+	rte_logs.dynamic_types[id].name = dup_name;
+	rte_logs.dynamic_types[id].loglevel = RTE_LOG_DEBUG;
+
+	return id;
+}
+
+/* register an extended log type */
+int
+rte_log_register(const char *name)
+{
+	struct rte_log_dynamic_type *new_dynamic_types;
+	int id, ret;
+
+	id = rte_log_lookup(name);
+	if (id >= 0)
+		return id;
+
+	new_dynamic_types = realloc(rte_logs.dynamic_types,
+		sizeof(struct rte_log_dynamic_type) *
+		(rte_logs.dynamic_types_len + 1));
+	if (new_dynamic_types == NULL)
+		return -ENOMEM;
+	rte_logs.dynamic_types = new_dynamic_types;
+
+	ret = __rte_log_register(name, rte_logs.dynamic_types_len);
+	if (ret < 0)
+		return ret;
+
+	rte_logs.dynamic_types_len++;
+
+	return ret;
+}
+
+RTE_INIT(rte_log_init);
+static void
+rte_log_init(void)
+{
+	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);
+
+	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
@@ -139,7 +266,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 		}
 	}
 
-	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
+	if (level > rte_logs.level)
+		return 0;
+	if (logtype >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > rte_logs.dynamic_types[logtype].loglevel)
 		return 0;
 
 	/* save loglevel and logtype in a global per-lcore variable */
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 954b96cfd..e71887fca 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,17 +50,21 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+struct rte_log_dynamic_type;
+
 /** The rte_log structure. */
 struct rte_logs {
 	uint32_t type;  /**< Bitfield with enabled logs. */
 	uint32_t level; /**< Log level. */
 	FILE *file;     /**< Output file set by rte_openlog_stream, or NULL. */
+	size_t dynamic_types_len;
+	struct rte_log_dynamic_type *dynamic_types;
 };
 
 /** Global log informations */
 extern struct rte_logs rte_logs;
 
-/* SDK log type */
+/* 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. */
@@ -91,6 +95,9 @@ extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_USER7   0x40000000 /**< User-defined log type 7. */
 #define RTE_LOGTYPE_USER8   0x80000000 /**< User-defined log type 8. */
 
+/** First identifier for extended logs */
+#define RTE_LOGTYPE_FIRST_EXT_ID 32
+
 /* Can't use 0, as it gives compiler warnings */
 #define RTE_LOG_EMERG    1U  /**< System is unusable.               */
 #define RTE_LOG_ALERT    2U  /**< Action must be taken immediately. */
@@ -148,6 +155,18 @@ void rte_set_log_type(uint32_t type, int enable);
 uint32_t rte_get_log_type(void);
 
 /**
+ * Set the log level for a given type.
+ *
+ * @param logtype
+ *   The log type identifier.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if logtype or level is invalid.
+ */
+int rte_log_set_level(uint32_t logtype, uint32_t level);
+
+/**
  * Get the current loglevel for the message being processed.
  *
  * Before calling the user-defined stream for logging, the log
@@ -176,6 +195,20 @@ int rte_log_cur_msg_loglevel(void);
 int rte_log_cur_msg_logtype(void);
 
 /**
+ * Register a dynamic log type
+ *
+ * If a log is already registered with the same type, the returned value
+ * is the same than the previous one.
+ *
+ * @param name
+ *   The string identifying the log type.
+ * @return
+ *   - >0: success, the returned value is the log type identifier.
+ *   - (-ENONEM): cannot allocate memory.
+ */
+int rte_log_register(const char *name);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 9c134b470..8375a9db9 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -184,5 +184,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_register;
+	rte_log_set_level;
 
 } DPDK_16.11;
diff --git a/test/test/test_logs.c b/test/test/test_logs.c
index 6985ddde3..805c56810 100644
--- a/test/test/test_logs.c
+++ b/test/test/test_logs.c
@@ -62,8 +62,8 @@ static int
 test_logs(void)
 {
 	/* enable these logs type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP1, RTE_LOG_EMERG);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
@@ -76,7 +76,7 @@ test_logs(void)
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
 	/* disable one log type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 0);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
-- 
2.11.0

[PATCH v2 2/8] eal: dump registered log types

[Olivier Matz]

Introduce a function to dump the global level and the registered log
types.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 34 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 10 ++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 46 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 6b3b3868d..d7a46a7b9 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -180,6 +180,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 68d879ae2..42dc9d033 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -243,6 +243,40 @@ rte_log_init(void)
 	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
 }
 
+static const char *
+loglevel_to_string(uint32_t level)
+{
+	switch (level) {
+	case RTE_LOG_EMERG: return "emerg";
+	case RTE_LOG_ALERT: return "alert";
+	case RTE_LOG_CRIT: return "critical";
+	case RTE_LOG_ERR: return "error";
+	case RTE_LOG_WARNING: return "warning";
+	case RTE_LOG_NOTICE: return "notice";
+	case RTE_LOG_INFO: return "info";
+	case RTE_LOG_DEBUG: return "debug";
+	default: return "unknown";
+	}
+}
+
+/* dump global level and registered log types */
+void
+rte_log_dump(FILE *f)
+{
+	size_t i;
+
+	fprintf(f, "global log level is %s\n",
+		loglevel_to_string(rte_log_get_global_level()));
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		fprintf(f, "id %zu: %s, level is %s\n",
+			i, rte_logs.dynamic_types[i].name,
+			loglevel_to_string(rte_logs.dynamic_types[i].loglevel));
+	}
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index e71887fca..97e0c5e52 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -209,6 +209,16 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * Dump log information.
+ *
+ * Dump the global level and the registered log types.
+ *
+ * @param f
+ *   The output stream where the dump should be sent.
+ */
+void rte_log_dump(FILE *f);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 8375a9db9..96cb961c3 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -184,6 +184,7 @@ DPDK_17.02 {
 	rte_bus_register;
 	rte_bus_scan;
 	rte_bus_unregister;
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
-- 
2.11.0

Re: [PATCH v2 2/8] eal: dump registered log types

[Thomas Monjalon]

2017-03-29 17:53, Olivier Matz:
> +/* dump global level and registered log types */
> +void
> +rte_log_dump(FILE *f)
> +{
> +       size_t i;
> +
> +       fprintf(f, "global log level is %s\n",
> +               loglevel_to_string(rte_log_get_global_level()));
> 

Compilation fails here:
fatal error: implicit declaration of function 'rte_log_get_global_level'

Please, could you also move new symbols in 17.05 map blocks
and release notes in 17.05?

Thanks

[PATCH v2 3/8] eal: change several log levels matching a regexp

[Olivier Matz]

Introduce a function to set the log level of several log types that
match a regular expression.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 21 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 12 ++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 35 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index d7a46a7b9..be829cb58 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -183,5 +183,6 @@ DPDK_17.02 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 42dc9d033..513249e51 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -37,6 +37,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <regex.h>
 
 #include <rte_eal.h>
 #include <rte_log.h>
@@ -132,6 +133,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
 	return 0;
 }
 
+/* set level */
+int
+rte_log_set_level_regexp(const char *pattern, uint32_t level)
+{
+	regex_t r;
+	size_t i;
+
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (regexec(&r, pattern, 0, NULL, 0) == 0)
+			rte_logs.dynamic_types[i].loglevel = level;
+	}
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 97e0c5e52..ce48b0785 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -157,6 +157,18 @@ uint32_t rte_get_log_type(void);
 /**
  * Set the log level for a given type.
  *
+ * @param pattern
+ *   The regexp identifying the log type.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if level is invalid.
+ */
+int rte_log_set_level_regexp(const char *pattern, uint32_t level);
+
+/**
+ * Set the log level for a given type.
+ *
  * @param logtype
  *   The log type identifier.
  * @param level
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 96cb961c3..a8116edee 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -187,5 +187,6 @@ DPDK_17.02 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_16.11;
-- 
2.11.0

[PATCH v2 4/8] eal: change specific log levels at startup

[Olivier Matz]

Example of use:
  ./app/test-pmd --log-level='pmd\.i40e.*,8'

  This enables debug logs for all dynamic logs whose type starts with
  'pmd.i40e'.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            |  4 +--
 lib/librte_eal/common/eal_common_options.c | 49 +++++++++++++++++++++++-------
 lib/librte_eal/linuxapp/eal/eal.c          |  4 +--
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 4ee9c66fe..fae6c7e0a 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -519,10 +519,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index f36bc5568..32df2ef45 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -739,25 +739,53 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
 }
 
 static int
-eal_parse_log_level(const char *level, uint32_t *log_level)
+eal_parse_log_level(const char *arg, struct internal_config *conf)
 {
-	char *end;
+	char *end, *str, *type, *level;
 	unsigned long tmp;
 
+	str = strdup(arg);
+	if (str == NULL)
+		return -1;
+
+	if (strchr(str, ',') == NULL) {
+		type = NULL;
+		level = str;
+	} else {
+		type = strsep(&str, ",");
+		level = strsep(&str, ",");
+	}
+
 	errno = 0;
 	tmp = strtoul(level, &end, 0);
 
 	/* check for errors */
 	if ((errno != 0) || (level[0] == '\0') ||
-	    end == NULL || (*end != '\0'))
-		return -1;
+		    end == NULL || (*end != '\0'))
+		goto fail;
 
 	/* log_level is a uint32_t */
 	if (tmp >= UINT32_MAX)
-		return -1;
+		goto fail;
+
+	printf("set log level %s,%lu\n",
+		type, tmp);
+
+	if (type == NULL) {
+		conf->log_level = tmp;
+		rte_log_set_global_level(tmp);
+	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
+		printf("cannot set log level %s,%lu\n",
+			type, tmp);
+		goto fail;
+	}
 
-	*log_level = tmp;
+	free(str);
 	return 0;
+
+fail:
+	free(str);
+	return -1;
 }
 
 static enum rte_proc_type_t
@@ -898,15 +926,12 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_LOG_LEVEL_NUM: {
-		uint32_t log;
-
-		if (eal_parse_log_level(optarg, &log) < 0) {
+		if (eal_parse_log_level(optarg, conf) < 0) {
 			RTE_LOG(ERR, EAL,
 				"invalid parameters for --"
 				OPT_LOG_LEVEL "\n");
 			return -1;
 		}
-		conf->log_level = log;
 		break;
 	}
 	case OPT_LCORES_NUM:
@@ -1057,7 +1082,9 @@ eal_common_usage(void)
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
-	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
+	       "  --"OPT_LOG_LEVEL"=<int>   Set global log level\n"
+	       "  --"OPT_LOG_LEVEL"=<type-regexp>,<int>\n"
+	       "                      Set specific log level\n"
 	       "  -v                  Display version information on startup\n"
 	       "  -h, --help          This help\n"
 	       "\nEAL options for DEBUG use only:\n"
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index f0ded185b..d98d56d2f 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -776,10 +776,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
-- 
2.11.0

[PATCH v2 5/8] eal: deprecate log functions

[Olivier Matz]

Deprecate the following functions:
- rte_set_log_level(), replaced by rte_log_set_global_level()
- rte_get_log_level(), replaced by rte_log_get_global_level()
- rte_set_log_type(), replaced by rte_log_set_level()
- rte_get_log_type(), replaced by rte_log_get_level()

The new functions provide a better control of the per-type log level,
and have a better name prefix (rte_log_).

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst            |  8 ++++++++
 examples/quota_watermark/qw/main.c              |  2 +-
 examples/quota_watermark/qwctl/qwctl.c          |  2 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
 lib/librte_eal/common/eal_common_log.c          | 24 ++++++++++++++++++++----
 lib/librte_eal/common/include/rte_log.h         | 19 +++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
 test/test/test_logs.c                           |  6 +++---
 8 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d6544ed0a..3f7824b75 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -118,3 +118,11 @@ Deprecation Notices
   to specify which type of instance to create (single or burst), and
   additional calls for ``rte_distributor_poll_pkt_burst`` and
   ``rte_distributor_return_pkt_burst``, among others.
+
+* eal: the following functions are deprecated starting from 17.02 and will
+  be removed in 17.08:
+
+  - ``rte_set_log_level``, replaced by ``rte_log_set_global_level``
+  - ``rte_get_log_level``, replaced by ``rte_log_get_global_level``
+  - ``rte_set_log_type``, replaced by ``rte_log_set_level``
+  - ``rte_get_log_type``, replaced by ``rte_log_get_level``
diff --git a/examples/quota_watermark/qw/main.c b/examples/quota_watermark/qw/main.c
index 57df8efdb..5ba74e2e9 100644
--- a/examples/quota_watermark/qw/main.c
+++ b/examples/quota_watermark/qw/main.c
@@ -321,7 +321,7 @@ main(int argc, char **argv)
 
 	uint8_t port_id;
 
-	rte_set_log_level(RTE_LOG_INFO);
+	rte_log_set_global_level(RTE_LOG_INFO);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
diff --git a/examples/quota_watermark/qwctl/qwctl.c b/examples/quota_watermark/qwctl/qwctl.c
index 3a85cc32a..7db66a1d6 100644
--- a/examples/quota_watermark/qwctl/qwctl.c
+++ b/examples/quota_watermark/qwctl/qwctl.c
@@ -75,7 +75,7 @@ int main(int argc, char **argv)
 	int ret;
 	struct cmdline *cl;
 
-	rte_set_log_level(RTE_LOG_INFO);
+	rte_log_set_global_level(RTE_LOG_INFO);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index be829cb58..bccaafad3 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -182,6 +182,8 @@ DPDK_17.02 {
 	rte_bus_unregister;
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 513249e51..dd4d30ca7 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -84,20 +84,36 @@ rte_openlog_stream(FILE *f)
 
 /* Set global log level */
 void
-rte_set_log_level(uint32_t level)
+rte_log_set_global_level(uint32_t level)
 {
 	rte_logs.level = (uint32_t)level;
 }
 
+/* Set global log level */
+/* replaced by rte_log_set_global_level */
+__rte_deprecated void
+rte_set_log_level(uint32_t level)
+{
+	rte_log_set_global_level(level);
+}
+
 /* Get global log level */
 uint32_t
-rte_get_log_level(void)
+rte_log_get_global_level(void)
 {
 	return rte_logs.level;
 }
 
+/* Get global log level */
+/* replaced by rte_log_get_global_level */
+uint32_t
+rte_get_log_level(void)
+{
+	return rte_log_get_global_level();
+}
+
 /* Set global log type */
-void
+__rte_deprecated void
 rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
@@ -114,7 +130,7 @@ rte_set_log_type(uint32_t type, int enable)
 }
 
 /* Get global log type */
-uint32_t
+__rte_deprecated uint32_t
 rte_get_log_type(void)
 {
 	return rte_logs.type;
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index ce48b0785..27c40b452 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,6 +50,8 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+#include <rte_common.h>
+
 struct rte_log_dynamic_type;
 
 /** The rte_log structure. */
@@ -132,11 +134,26 @@ int rte_openlog_stream(FILE *f);
  * @param level
  *   Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
  */
+void rte_log_set_global_level(uint32_t level);
+
+/**
+ * Deprecated, replaced by rte_log_set_global_level().
+ */
+__rte_deprecated
 void rte_set_log_level(uint32_t level);
 
 /**
  * Get the global log level.
+ *
+ * @return
+ *   The current global log level.
+ */
+uint32_t rte_log_get_global_level(void);
+
+/**
+ * Deprecated, replaced by rte_log_get_global_level().
  */
+__rte_deprecated
 uint32_t rte_get_log_level(void);
 
 /**
@@ -147,11 +164,13 @@ uint32_t rte_get_log_level(void);
  * @param enable
  *   True for enable; false for disable.
  */
+__rte_deprecated
 void rte_set_log_type(uint32_t type, int enable);
 
 /**
  * Get the global log type.
  */
+__rte_deprecated
 uint32_t rte_get_log_type(void);
 
 /**
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index a8116edee..3b50c4c3c 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -186,6 +186,8 @@ DPDK_17.02 {
 	rte_bus_unregister;
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/test/test/test_logs.c b/test/test/test_logs.c
index 805c56810..730a86bd9 100644
--- a/test/test/test_logs.c
+++ b/test/test/test_logs.c
@@ -66,12 +66,12 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(CRIT, TESTAPP1, "critical message\n");
 
 	/* log in critical level */
-	rte_set_log_level(RTE_LOG_CRIT);
+	rte_log_set_global_level(RTE_LOG_CRIT);
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
@@ -79,7 +79,7 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 
-- 
2.11.0

[PATCH v2 6/8] app/test: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 test/test/commands.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test/commands.c b/test/test/commands.c
index 2df46b054..488dc4187 100644
--- a/test/test/commands.c
+++ b/test/test/commands.c
@@ -158,13 +158,15 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
 	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
 				 "dump_physmem#dump_memzone#"
 				 "dump_struct_sizes#dump_ring#dump_mempool#"
-				 "dump_devargs");
+				 "dump_devargs#dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.11.0

[PATCH v2 7/8] app/testpmd: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/cmdline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d20..697228fa6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7679,6 +7679,8 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
@@ -7688,7 +7690,8 @@ cmdline_parse_token_string_t cmd_dump_dump =
 		"dump_struct_sizes#"
 		"dump_ring#"
 		"dump_mempool#"
-		"dump_devargs");
+		"dump_devargs#"
+		"dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.11.0

[PATCH v2 8/8] net/i40e: use dynamic log type for control logs

[Olivier Matz]

This is an example of how a dynamic log type can be used in a
PMD.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 config/common_base             |  2 --
 drivers/net/i40e/i40e_ethdev.c | 18 ++++++++++++++++--
 drivers/net/i40e/i40e_fdir.c   |  4 ----
 drivers/net/i40e/i40e_logs.h   | 17 ++++++-----------
 4 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/config/common_base b/config/common_base
index 37aa1e163..2cf2b9c3b 100644
--- a/config/common_base
+++ b/config/common_base
@@ -179,11 +179,9 @@ CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=y
 # Compile burst-oriented I40E PMD driver
 #
 CONFIG_RTE_LIBRTE_I40E_PMD=y
-CONFIG_RTE_LIBRTE_I40E_DEBUG_INIT=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX_FREE=n
-CONFIG_RTE_LIBRTE_I40E_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y
 CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y
 CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE=y
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 303027b6a..1ffa6d569 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -40,6 +40,7 @@
 #include <inttypes.h>
 #include <assert.h>
 
+#include <rte_eal.h>
 #include <rte_string_fns.h>
 #include <rte_pci.h>
 #include <rte_ether.h>
@@ -419,6 +420,9 @@ static void i40e_ethertype_filter_restore(struct i40e_pf *pf);
 static void i40e_tunnel_filter_restore(struct i40e_pf *pf);
 static void i40e_filter_restore(struct i40e_pf *pf);
 
+int i40e_logtype_init;
+int i40e_logtype_driver;
+
 static const struct rte_pci_id pci_id_i40e_map[] = {
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_SFP_XL710) },
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_QEMU) },
@@ -5788,7 +5792,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(INFO, "No interrupt event");
 		goto done;
 	}
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	if (icr0 & I40E_PFINT_ICR0_ECC_ERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: unrecoverable ECC error");
 	if (icr0 & I40E_PFINT_ICR0_MAL_DETECT_MASK)
@@ -5803,7 +5806,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(ERR, "ICR0: HMC error");
 	if (icr0 & I40E_PFINT_ICR0_PE_CRITERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: protocol engine critical error");
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	if (icr0 & I40E_PFINT_ICR0_VFLR_MASK) {
 		PMD_DRV_LOG(INFO, "ICR0: VF reset detected");
@@ -11212,3 +11214,15 @@ rte_pmd_i40e_reset_vf_stats(uint8_t port,
 
 	return 0;
 }
+
+RTE_INIT(i40e_init_log);
+static void
+i40e_init_log(void)
+{
+	i40e_logtype_init = rte_log_register("pmd.i40e.init");
+	if (i40e_logtype_init >= 0)
+		rte_log_set_level(i40e_logtype_init, RTE_LOG_NOTICE);
+	i40e_logtype_driver = rte_log_register("pmd.i40e.driver");
+	if (i40e_logtype_driver >= 0)
+		rte_log_set_level(i40e_logtype_driver, RTE_LOG_NOTICE);
+}
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 0700253b1..32d3b1910 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1592,17 +1592,14 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	struct rte_eth_dev *dev = I40E_VSI_TO_ETH_DEV(pf->main_vsi);
 	struct i40e_fdir_filter_list *fdir_list = &pf->fdir.fdir_list;
 	struct i40e_fdir_filter *f;
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint32_t fdstat;
 	uint32_t guarant_cnt;  /**< Number of filters in guaranteed spaces. */
 	uint32_t best_cnt;     /**< Number of filters in best effort spaces. */
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	TAILQ_FOREACH(f, fdir_list, rules)
 		i40e_add_del_fdir_filter(dev, &f->fdir, TRUE);
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	fdstat = I40E_READ_REG(hw, I40E_PFQF_FDSTAT);
 	guarant_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_GUARANT_CNT_MASK) >>
@@ -1610,7 +1607,6 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	best_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_BEST_CNT_MASK) >>
 			   I40E_PFQF_FDSTAT_BEST_CNT_SHIFT);
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	PMD_DRV_LOG(INFO, "FDIR: Guarant count: %d,  Best count: %d",
 		    guarant_cnt, best_cnt);
diff --git a/drivers/net/i40e/i40e_logs.h b/drivers/net/i40e/i40e_logs.h
index e042e2429..8e99cd52a 100644
--- a/drivers/net/i40e/i40e_logs.h
+++ b/drivers/net/i40e/i40e_logs.h
@@ -34,14 +34,11 @@
 #ifndef _I40E_LOGS_H_
 #define _I40E_LOGS_H_
 
+extern int i40e_logtype_init;
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
-
-#ifdef RTE_LIBRTE_I40E_DEBUG_INIT
+	rte_log(RTE_LOG_ ## level, i40e_logtype_init, "%s(): " fmt "\n", \
+		__func__, ##args)
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-#else
-#define PMD_INIT_FUNC_TRACE() do { } while(0)
-#endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
@@ -64,12 +61,10 @@
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
+extern int i40e_logtype_driver;
 #define PMD_DRV_LOG_RAW(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
-#else
-#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
-#endif
+	rte_log(RTE_LOG_ ## level, i40e_logtype_driver, "%s(): " fmt, \
+		__func__, ## args)
 
 #define PMD_DRV_LOG(level, fmt, args...) \
 	PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
-- 
2.11.0

[PATCH v3 0/8] eal: dynamic logs

[Olivier Matz]

The objective of this patchset is to introduce a framework to
support dynamic log types in EAL. It also provides one example of use
(in i40e).

Features:
- log types are identified by a string
- at registration, a uniq identifier is associated to a log type
- each log type can have its level changed dynamically
- extend command line parameters to set the log level of a specific
  type, or logs matching a regular expression
- keep compat with other legacy types (eal, malloc, ring, user*,
  etc... keep their hardcoded log type value)

Next step is to adapt drivers, libs and apps to use this new API. At the
end, we can expect that all non-dataplane logs are moved to be dynamic,
so we can enable/disable them at runtime, without recompiling. Many
debug options can probably be removed from configuration:
  $ git grep DEBUG config/common_base | wc -l
  89

v2 -> v3:
- fix compilation of intermediate patches
- replace references to 17.02 by 17.05 (doc and .map)

v1 -> v2:
- fix issues reported by Stephen
- add entry in release note and deprecation notice
- rebase on top of master

RFC -> v1:
- rebase on top of current master
- fix eal help alignment
- remove unused i40e compilation options


Olivier Matz (8):
  eal: support dynamic log types
  eal: dump registered log types
  eal: change several log levels matching a regexp
  eal: change specific log levels at startup
  eal: deprecate log functions
  app/test: new command to dump log types
  app/testpmd: new command to dump log types
  net/i40e: use dynamic log type for control logs

 app/test-pmd/cmdline.c                          |   5 +-
 config/common_base                              |   2 -
 doc/guides/contributing/coding_style.rst        |  30 ++--
 doc/guides/rel_notes/deprecation.rst            |   8 +
 doc/guides/rel_notes/release_17_05.rst          |   5 +
 drivers/net/i40e/i40e_ethdev.c                  |  18 +-
 drivers/net/i40e/i40e_fdir.c                    |   4 -
 drivers/net/i40e/i40e_logs.h                    |  17 +-
 examples/quota_watermark/qw/main.c              |   2 +-
 examples/quota_watermark/qwctl/qwctl.c          |   2 +-
 lib/librte_eal/bsdapp/eal/eal.c                 |   4 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  12 ++
 lib/librte_eal/common/eal_common_log.c          | 216 +++++++++++++++++++++++-
 lib/librte_eal/common/eal_common_options.c      |  49 ++++--
 lib/librte_eal/common/include/rte_log.h         |  76 ++++++++-
 lib/librte_eal/linuxapp/eal/eal.c               |   4 +-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  12 ++
 test/test/commands.c                            |   4 +-
 test/test/test_logs.c                           |  12 +-
 19 files changed, 417 insertions(+), 65 deletions(-)

-- 
2.11.0

[PATCH v3 1/8] eal: support dynamic log types

[Olivier Matz]

Introduce 2 new functions to support dynamic log types:

- rte_log_register(): register a log name, and return a log type id
- rte_log_set_level(): set the log level of a given log type

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/contributing/coding_style.rst        |  30 ++++--
 doc/guides/rel_notes/release_17_05.rst          |   5 +
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   8 ++
 lib/librte_eal/common/eal_common_log.c          | 137 +++++++++++++++++++++++-
 lib/librte_eal/common/include/rte_log.h         |  35 +++++-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   8 ++
 test/test/test_logs.c                           |   6 +-
 7 files changed, 211 insertions(+), 18 deletions(-)

diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index 4163960c9..d8e4a0f9c 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -603,22 +603,30 @@ In the DPDK environment, use the logging interface provided:
 
 .. code-block:: c
 
- #define RTE_LOGTYPE_TESTAPP1 RTE_LOGTYPE_USER1
- #define RTE_LOGTYPE_TESTAPP2 RTE_LOGTYPE_USER2
+ /* register log types for this application */
+ int my_logtype1 = rte_log_register("myapp.log1");
+ int my_logtype2 = rte_log_register("myapp.log2");
 
- /* enable these logs type */
- rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
- rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+ /* set global log level to INFO */
+ rte_log_set_global_level(RTE_LOG_INFO);
+
+ /* only display messages higher than NOTICE for log2 (default
+  * is DEBUG) */
+ rte_log_set_level(my_logtype2, RTE_LOG_NOTICE);
+
+ /* enable all PMD logs (whose identifier string starts with "pmd") */
+ rte_log_set_level_regexp("pmd.*", RTE_LOG_DEBUG);
 
  /* log in debug level */
- rte_set_log_level(RTE_LOG_DEBUG);
- RTE_LOG(DEBUG, TESTAPP1, "this is is a debug level message\n");
- RTE_LOG(INFO, TESTAPP1, "this is is a info level message\n");
- RTE_LOG(WARNING, TESTAPP1, "this is is a warning level message\n");
+ rte_log_set_global_level(RTE_LOG_DEBUG);
+ RTE_LOG(DEBUG, my_logtype1, "this is is a debug level message\n");
+ RTE_LOG(INFO, my_logtype1, "this is is a info level message\n");
+ RTE_LOG(WARNING, my_logtype1, "this is is a warning level message\n");
+ RTE_LOG(WARNING, my_logtype2, "this is is a debug level message (not displayed)\n");
 
  /* log in info level */
- rte_set_log_level(RTE_LOG_INFO);
- RTE_LOG(DEBUG, TESTAPP2, "debug level message (not displayed)\n");
+ rte_log_set_global_level(RTE_LOG_INFO);
+ RTE_LOG(DEBUG, my_logtype1, "debug level message (not displayed)\n");
 
 Branch Prediction
 ~~~~~~~~~~~~~~~~~
diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index c4bb49ad1..0b34632b9 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -67,6 +67,11 @@ New Features
 
   sPAPR IOMMU based pci probing enabled for vfio-pci devices.
 
+* **Added EAL dynamic log framework.**
+
+  Added new APIs to dynamically register named log types, and control
+  the level of each type independently.
+
 
 Resolved Issues
 ---------------
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 67f2ffb29..df859666d 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -182,3 +182,11 @@ DPDK_17.02 {
 	rte_bus_unregister;
 
 } DPDK_16.11;
+
+DPDK_17.05 {
+	global:
+
+	rte_log_register;
+	rte_log_set_level;
+
+} DPDK_17.02;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 219755838..68d879ae2 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -35,7 +35,10 @@
 #include <stdint.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <string.h>
+#include <errno.h>
 
+#include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_per_lcore.h>
 
@@ -60,6 +63,11 @@ struct log_cur_msg {
 	uint32_t logtype;  /**< log type  - see rte_log.h */
 };
 
+struct rte_log_dynamic_type {
+	const char *name;
+	uint32_t loglevel;
+};
+
  /* per core log */
 static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg);
 
@@ -91,10 +99,17 @@ rte_get_log_level(void)
 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_logs.type |= type;
+		rte_log_set_level(type, 0);
 	else
-		rte_logs.type &= (~type);
+		rte_log_set_level(type, RTE_LOG_DEBUG);
 }
 
 /* Get global log type */
@@ -104,6 +119,19 @@ rte_get_log_type(void)
 	return rte_logs.type;
 }
 
+int
+rte_log_set_level(uint32_t type, uint32_t level)
+{
+	if (type >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
@@ -116,6 +144,105 @@ int rte_log_cur_msg_logtype(void)
 	return RTE_PER_LCORE(log_cur_msg).logtype;
 }
 
+static int
+rte_log_lookup(const char *name)
+{
+	size_t i;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (strcmp(name, rte_logs.dynamic_types[i].name) == 0)
+			return i;
+	}
+
+	return -1;
+}
+
+/* register an extended log type, assuming table is large enough, and id
+ * is not yet registered.
+ */
+static int
+__rte_log_register(const char *name, int id)
+{
+	char *dup_name = strdup(name);
+
+	if (dup_name == NULL)
+		return -ENOMEM;
+
+	rte_logs.dynamic_types[id].name = dup_name;
+	rte_logs.dynamic_types[id].loglevel = RTE_LOG_DEBUG;
+
+	return id;
+}
+
+/* register an extended log type */
+int
+rte_log_register(const char *name)
+{
+	struct rte_log_dynamic_type *new_dynamic_types;
+	int id, ret;
+
+	id = rte_log_lookup(name);
+	if (id >= 0)
+		return id;
+
+	new_dynamic_types = realloc(rte_logs.dynamic_types,
+		sizeof(struct rte_log_dynamic_type) *
+		(rte_logs.dynamic_types_len + 1));
+	if (new_dynamic_types == NULL)
+		return -ENOMEM;
+	rte_logs.dynamic_types = new_dynamic_types;
+
+	ret = __rte_log_register(name, rte_logs.dynamic_types_len);
+	if (ret < 0)
+		return ret;
+
+	rte_logs.dynamic_types_len++;
+
+	return ret;
+}
+
+RTE_INIT(rte_log_init);
+static void
+rte_log_init(void)
+{
+	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);
+
+	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
@@ -139,7 +266,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 		}
 	}
 
-	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
+	if (level > rte_logs.level)
+		return 0;
+	if (logtype >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > rte_logs.dynamic_types[logtype].loglevel)
 		return 0;
 
 	/* save loglevel and logtype in a global per-lcore variable */
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 954b96cfd..e71887fca 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,17 +50,21 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+struct rte_log_dynamic_type;
+
 /** The rte_log structure. */
 struct rte_logs {
 	uint32_t type;  /**< Bitfield with enabled logs. */
 	uint32_t level; /**< Log level. */
 	FILE *file;     /**< Output file set by rte_openlog_stream, or NULL. */
+	size_t dynamic_types_len;
+	struct rte_log_dynamic_type *dynamic_types;
 };
 
 /** Global log informations */
 extern struct rte_logs rte_logs;
 
-/* SDK log type */
+/* 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. */
@@ -91,6 +95,9 @@ extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_USER7   0x40000000 /**< User-defined log type 7. */
 #define RTE_LOGTYPE_USER8   0x80000000 /**< User-defined log type 8. */
 
+/** First identifier for extended logs */
+#define RTE_LOGTYPE_FIRST_EXT_ID 32
+
 /* Can't use 0, as it gives compiler warnings */
 #define RTE_LOG_EMERG    1U  /**< System is unusable.               */
 #define RTE_LOG_ALERT    2U  /**< Action must be taken immediately. */
@@ -148,6 +155,18 @@ void rte_set_log_type(uint32_t type, int enable);
 uint32_t rte_get_log_type(void);
 
 /**
+ * Set the log level for a given type.
+ *
+ * @param logtype
+ *   The log type identifier.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if logtype or level is invalid.
+ */
+int rte_log_set_level(uint32_t logtype, uint32_t level);
+
+/**
  * Get the current loglevel for the message being processed.
  *
  * Before calling the user-defined stream for logging, the log
@@ -176,6 +195,20 @@ int rte_log_cur_msg_loglevel(void);
 int rte_log_cur_msg_logtype(void);
 
 /**
+ * Register a dynamic log type
+ *
+ * If a log is already registered with the same type, the returned value
+ * is the same than the previous one.
+ *
+ * @param name
+ *   The string identifying the log type.
+ * @return
+ *   - >0: success, the returned value is the log type identifier.
+ *   - (-ENONEM): cannot allocate memory.
+ */
+int rte_log_register(const char *name);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 9c134b470..f75f52efb 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -186,3 +186,11 @@ DPDK_17.02 {
 	rte_bus_unregister;
 
 } DPDK_16.11;
+
+DPDK_17.05 {
+	global:
+
+	rte_log_register;
+	rte_log_set_level;
+
+} DPDK_17.02;
diff --git a/test/test/test_logs.c b/test/test/test_logs.c
index 6985ddde3..805c56810 100644
--- a/test/test/test_logs.c
+++ b/test/test/test_logs.c
@@ -62,8 +62,8 @@ static int
 test_logs(void)
 {
 	/* enable these logs type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP1, RTE_LOG_EMERG);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
@@ -76,7 +76,7 @@ test_logs(void)
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
 	/* disable one log type */
-	rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 0);
+	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
 	rte_set_log_level(RTE_LOG_ERR);
-- 
2.11.0

[PATCH v3 2/8] eal: dump registered log types

[Olivier Matz]

Introduce a function to dump the global level and the registered log
types.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 34 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 10 ++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 46 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index df859666d..bd63ea66b 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -186,6 +186,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 68d879ae2..90326215b 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -243,6 +243,40 @@ rte_log_init(void)
 	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
 }
 
+static const char *
+loglevel_to_string(uint32_t level)
+{
+	switch (level) {
+	case RTE_LOG_EMERG: return "emerg";
+	case RTE_LOG_ALERT: return "alert";
+	case RTE_LOG_CRIT: return "critical";
+	case RTE_LOG_ERR: return "error";
+	case RTE_LOG_WARNING: return "warning";
+	case RTE_LOG_NOTICE: return "notice";
+	case RTE_LOG_INFO: return "info";
+	case RTE_LOG_DEBUG: return "debug";
+	default: return "unknown";
+	}
+}
+
+/* dump global level and registered log types */
+void
+rte_log_dump(FILE *f)
+{
+	size_t i;
+
+	fprintf(f, "global log level is %s\n",
+		loglevel_to_string(rte_get_log_level()));
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		fprintf(f, "id %zu: %s, level is %s\n",
+			i, rte_logs.dynamic_types[i].name,
+			loglevel_to_string(rte_logs.dynamic_types[i].loglevel));
+	}
+}
+
 /*
  * Generates a log message The message will be sent in the stream
  * defined by the previous call to rte_openlog_stream().
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index e71887fca..97e0c5e52 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -209,6 +209,16 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * Dump log information.
+ *
+ * Dump the global level and the registered log types.
+ *
+ * @param f
+ *   The output stream where the dump should be sent.
+ */
+void rte_log_dump(FILE *f);
+
+/**
  * Generates a log message.
  *
  * The message will be sent in the stream defined by the previous call
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index f75f52efb..ef48500e9 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -190,6 +190,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
 
-- 
2.11.0

[PATCH v3 3/8] eal: change several log levels matching a regexp

[Olivier Matz]

Introduce a function to set the log level of several log types that
match a regular expression.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_log.c          | 21 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_log.h         | 12 ++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 35 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index bd63ea66b..de74ff9ff 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -189,5 +189,6 @@ DPDK_17.05 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_17.02;
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 90326215b..d02689390 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -37,6 +37,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
+#include <regex.h>
 
 #include <rte_eal.h>
 #include <rte_log.h>
@@ -132,6 +133,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
 	return 0;
 }
 
+/* set level */
+int
+rte_log_set_level_regexp(const char *pattern, uint32_t level)
+{
+	regex_t r;
+	size_t i;
+
+	if (level > RTE_LOG_DEBUG)
+		return -1;
+
+	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
+		if (rte_logs.dynamic_types[i].name == NULL)
+			continue;
+		if (regexec(&r, pattern, 0, NULL, 0) == 0)
+			rte_logs.dynamic_types[i].loglevel = level;
+	}
+
+	return 0;
+}
+
 /* get the current loglevel for the message beeing processed */
 int rte_log_cur_msg_loglevel(void)
 {
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 97e0c5e52..ce48b0785 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -157,6 +157,18 @@ uint32_t rte_get_log_type(void);
 /**
  * Set the log level for a given type.
  *
+ * @param pattern
+ *   The regexp identifying the log type.
+ * @param level
+ *   The level to be set.
+ * @return
+ *   0 on success, a negative value if level is invalid.
+ */
+int rte_log_set_level_regexp(const char *pattern, uint32_t level);
+
+/**
+ * Set the log level for a given type.
+ *
  * @param logtype
  *   The log type identifier.
  * @param level
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index ef48500e9..f70b4b937 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -193,5 +193,6 @@ DPDK_17.05 {
 	rte_log_dump;
 	rte_log_register;
 	rte_log_set_level;
+	rte_log_set_level_regexp;
 
 } DPDK_17.02;
-- 
2.11.0

Re: [PATCH v3 3/8] eal: change several log levels matching a regexp

[Tan, Jianfeng]

Hi Olivier,


On 4/5/2017 12:40 AM, Olivier Matz wrote:
> Introduce a function to set the log level of several log types that
> match a regular expression.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>   lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>   lib/librte_eal/common/eal_common_log.c          | 21 +++++++++++++++++++++
>   lib/librte_eal/common/include/rte_log.h         | 12 ++++++++++++
>   lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>   4 files changed, 35 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index bd63ea66b..de74ff9ff 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -189,5 +189,6 @@ DPDK_17.05 {
>   	rte_log_dump;
>   	rte_log_register;
>   	rte_log_set_level;
> +	rte_log_set_level_regexp;
>   
>   } DPDK_17.02;
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index 90326215b..d02689390 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -37,6 +37,7 @@
>   #include <stdlib.h>
>   #include <string.h>
>   #include <errno.h>
> +#include <regex.h>
>   
>   #include <rte_eal.h>
>   #include <rte_log.h>
> @@ -132,6 +133,26 @@ rte_log_set_level(uint32_t type, uint32_t level)
>   	return 0;
>   }
>   
> +/* set level */
> +int
> +rte_log_set_level_regexp(const char *pattern, uint32_t level)
> +{
> +	regex_t r;
> +	size_t i;
> +
> +	if (level > RTE_LOG_DEBUG)
> +		return -1;
> +
> +	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
> +		if (rte_logs.dynamic_types[i].name == NULL)
> +			continue;
> +		if (regexec(&r, pattern, 0, NULL, 0) == 0)
> +			rte_logs.dynamic_types[i].loglevel = level;

When I try this option, it causes segment fault. The problem might lie 
in this function. As far as I know, when we use regexec(), we need to 
compile a regular expression firstly using regcomp(); and then call 
regexec() with the compiled pattern with the full string. In other words:

         regcomp(&r, pattern, 0);
         for (i = 0; i < rte_logs.dynamic_types_len; i++) {
                 ...
                if (regexec(&r, rte_logs.dynamic_types[i].name, 0, NULL, 
0) == 0)
                         rte_logs.dynamic_types[i].loglevel = level;
         }

Thanks,
Jianfeng

> +	}
> +
> +	return 0;
> +}
> +
>   /* get the current loglevel for the message beeing processed */
>   int rte_log_cur_msg_loglevel(void)
>   {
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index 97e0c5e52..ce48b0785 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -157,6 +157,18 @@ uint32_t rte_get_log_type(void);
>   /**
>    * Set the log level for a given type.
>    *
> + * @param pattern
> + *   The regexp identifying the log type.
> + * @param level
> + *   The level to be set.
> + * @return
> + *   0 on success, a negative value if level is invalid.
> + */
> +int rte_log_set_level_regexp(const char *pattern, uint32_t level);
> +
> +/**
> + * Set the log level for a given type.
> + *
>    * @param logtype
>    *   The log type identifier.
>    * @param level
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index ef48500e9..f70b4b937 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -193,5 +193,6 @@ DPDK_17.05 {
>   	rte_log_dump;
>   	rte_log_register;
>   	rte_log_set_level;
> +	rte_log_set_level_regexp;
>   
>   } DPDK_17.02;

[PATCH v3 4/8] eal: change specific log levels at startup

[Olivier Matz]

Example of use:
  ./app/test-pmd --log-level='pmd\.i40e.*,8'

  This enables debug logs for all dynamic logs whose type starts with
  'pmd.i40e'.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            |  4 +--
 lib/librte_eal/common/eal_common_options.c | 49 +++++++++++++++++++++++-------
 lib/librte_eal/linuxapp/eal/eal.c          |  4 +--
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 4ee9c66fe..fae6c7e0a 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -519,10 +519,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index f36bc5568..606695a7f 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -739,25 +739,53 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
 }
 
 static int
-eal_parse_log_level(const char *level, uint32_t *log_level)
+eal_parse_log_level(const char *arg, struct internal_config *conf)
 {
-	char *end;
+	char *end, *str, *type, *level;
 	unsigned long tmp;
 
+	str = strdup(arg);
+	if (str == NULL)
+		return -1;
+
+	if (strchr(str, ',') == NULL) {
+		type = NULL;
+		level = str;
+	} else {
+		type = strsep(&str, ",");
+		level = strsep(&str, ",");
+	}
+
 	errno = 0;
 	tmp = strtoul(level, &end, 0);
 
 	/* check for errors */
 	if ((errno != 0) || (level[0] == '\0') ||
-	    end == NULL || (*end != '\0'))
-		return -1;
+		    end == NULL || (*end != '\0'))
+		goto fail;
 
 	/* log_level is a uint32_t */
 	if (tmp >= UINT32_MAX)
-		return -1;
+		goto fail;
+
+	printf("set log level %s,%lu\n",
+		type, tmp);
+
+	if (type == NULL) {
+		conf->log_level = tmp;
+		rte_set_log_level(tmp);
+	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
+		printf("cannot set log level %s,%lu\n",
+			type, tmp);
+		goto fail;
+	}
 
-	*log_level = tmp;
+	free(str);
 	return 0;
+
+fail:
+	free(str);
+	return -1;
 }
 
 static enum rte_proc_type_t
@@ -898,15 +926,12 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_LOG_LEVEL_NUM: {
-		uint32_t log;
-
-		if (eal_parse_log_level(optarg, &log) < 0) {
+		if (eal_parse_log_level(optarg, conf) < 0) {
 			RTE_LOG(ERR, EAL,
 				"invalid parameters for --"
 				OPT_LOG_LEVEL "\n");
 			return -1;
 		}
-		conf->log_level = log;
 		break;
 	}
 	case OPT_LCORES_NUM:
@@ -1057,7 +1082,9 @@ eal_common_usage(void)
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
-	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
+	       "  --"OPT_LOG_LEVEL"=<int>   Set global log level\n"
+	       "  --"OPT_LOG_LEVEL"=<type-regexp>,<int>\n"
+	       "                      Set specific log level\n"
 	       "  -v                  Display version information on startup\n"
 	       "  -h, --help          This help\n"
 	       "\nEAL options for DEBUG use only:\n"
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index f0ded185b..d98d56d2f 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -776,10 +776,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
-	eal_log_level_parse(argc, argv);
-
 	/* set log level as early as possible */
-	rte_set_log_level(internal_config.log_level);
+	eal_log_level_parse(argc, argv);
 
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
-- 
2.11.0

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Tan, Jianfeng]

Hi Olivier,

If I understand it correctly, this patch is to shift log level setting 
earlier. But we did not remove the one in eal_parse_common_option(). So 
we can see this parameter will be analyzed twice. Does it make sense to 
remove analysis of log level in eal_parse_common_option()?

Thanks,
Jianfeng

On 4/5/2017 12:40 AM, Olivier Matz wrote:
> Example of use:
>    ./app/test-pmd --log-level='pmd\.i40e.*,8'
>
>    This enables debug logs for all dynamic logs whose type starts with
>    'pmd.i40e'.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>   lib/librte_eal/bsdapp/eal/eal.c            |  4 +--
>   lib/librte_eal/common/eal_common_options.c | 49 +++++++++++++++++++++++-------
>   lib/librte_eal/linuxapp/eal/eal.c          |  4 +--
>   3 files changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 4ee9c66fe..fae6c7e0a 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -519,10 +519,8 @@ rte_eal_init(int argc, char **argv)
>   
>   	thread_id = pthread_self();
>   
> -	eal_log_level_parse(argc, argv);
> -
>   	/* set log level as early as possible */
> -	rte_set_log_level(internal_config.log_level);
> +	eal_log_level_parse(argc, argv);
>   
>   	if (rte_eal_cpu_init() < 0) {
>   		rte_eal_init_alert("Cannot detect lcores.");
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index f36bc5568..606695a7f 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -739,25 +739,53 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
>   }
>   
>   static int
> -eal_parse_log_level(const char *level, uint32_t *log_level)
> +eal_parse_log_level(const char *arg, struct internal_config *conf)
>   {
> -	char *end;
> +	char *end, *str, *type, *level;
>   	unsigned long tmp;
>   
> +	str = strdup(arg);
> +	if (str == NULL)
> +		return -1;
> +
> +	if (strchr(str, ',') == NULL) {
> +		type = NULL;
> +		level = str;
> +	} else {
> +		type = strsep(&str, ",");
> +		level = strsep(&str, ",");
> +	}
> +
>   	errno = 0;
>   	tmp = strtoul(level, &end, 0);
>   
>   	/* check for errors */
>   	if ((errno != 0) || (level[0] == '\0') ||
> -	    end == NULL || (*end != '\0'))
> -		return -1;
> +		    end == NULL || (*end != '\0'))
> +		goto fail;
>   
>   	/* log_level is a uint32_t */
>   	if (tmp >= UINT32_MAX)
> -		return -1;
> +		goto fail;
> +
> +	printf("set log level %s,%lu\n",
> +		type, tmp);
> +
> +	if (type == NULL) {
> +		conf->log_level = tmp;
> +		rte_set_log_level(tmp);
> +	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
> +		printf("cannot set log level %s,%lu\n",
> +			type, tmp);
> +		goto fail;
> +	}
>   
> -	*log_level = tmp;
> +	free(str);
>   	return 0;
> +
> +fail:
> +	free(str);
> +	return -1;
>   }
>   
>   static enum rte_proc_type_t
> @@ -898,15 +926,12 @@ eal_parse_common_option(int opt, const char *optarg,
>   		break;
>   
>   	case OPT_LOG_LEVEL_NUM: {
> -		uint32_t log;
> -
> -		if (eal_parse_log_level(optarg, &log) < 0) {
> +		if (eal_parse_log_level(optarg, conf) < 0) {
>   			RTE_LOG(ERR, EAL,
>   				"invalid parameters for --"
>   				OPT_LOG_LEVEL "\n");
>   			return -1;
>   		}
> -		conf->log_level = log;
>   		break;
>   	}
>   	case OPT_LCORES_NUM:
> @@ -1057,7 +1082,9 @@ eal_common_usage(void)
>   	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
>   	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
>   	       "  --"OPT_SYSLOG"            Set syslog facility\n"
> -	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
> +	       "  --"OPT_LOG_LEVEL"=<int>   Set global log level\n"
> +	       "  --"OPT_LOG_LEVEL"=<type-regexp>,<int>\n"
> +	       "                      Set specific log level\n"
>   	       "  -v                  Display version information on startup\n"
>   	       "  -h, --help          This help\n"
>   	       "\nEAL options for DEBUG use only:\n"
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index f0ded185b..d98d56d2f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -776,10 +776,8 @@ rte_eal_init(int argc, char **argv)
>   
>   	thread_id = pthread_self();
>   
> -	eal_log_level_parse(argc, argv);
> -
>   	/* set log level as early as possible */
> -	rte_set_log_level(internal_config.log_level);
> +	eal_log_level_parse(argc, argv);
>   
>   	if (rte_eal_cpu_init() < 0) {
>   		rte_eal_init_alert("Cannot detect lcores.");

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Olivier MATZ]

Hi Jianfeng,

On Fri, 14 Apr 2017 13:33:49 +0800, "Tan, Jianfeng" <jianfeng.tan@intel.com> wrote:
> Hi Olivier,
> 
> If I understand it correctly, this patch is to shift log level setting 
> earlier. But we did not remove the one in eal_parse_common_option(). So 
> we can see this parameter will be analyzed twice. Does it make sense to 
> remove analysis of log level in eal_parse_common_option()?
> 

The patch does not change the way the log level is parsed: it was
already parsed twice, because we want to know the log level as soon
as possible.

But the patch introduces a bug, as seen by Ferruh: the default log
level is not set properly when no --log-level parameter is passed.

Regards,
Olivier


> Thanks,
> Jianfeng
> 
> On 4/5/2017 12:40 AM, Olivier Matz wrote:
> > Example of use:
> >    ./app/test-pmd --log-level='pmd\.i40e.*,8'
> >
> >    This enables debug logs for all dynamic logs whose type starts with
> >    'pmd.i40e'.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >   lib/librte_eal/bsdapp/eal/eal.c            |  4 +--
> >   lib/librte_eal/common/eal_common_options.c | 49 +++++++++++++++++++++++-------
> >   lib/librte_eal/linuxapp/eal/eal.c          |  4 +--
> >   3 files changed, 40 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> > index 4ee9c66fe..fae6c7e0a 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal.c
> > @@ -519,10 +519,8 @@ rte_eal_init(int argc, char **argv)
> >   
> >   	thread_id = pthread_self();
> >   
> > -	eal_log_level_parse(argc, argv);
> > -
> >   	/* set log level as early as possible */
> > -	rte_set_log_level(internal_config.log_level);
> > +	eal_log_level_parse(argc, argv);
> >   
> >   	if (rte_eal_cpu_init() < 0) {
> >   		rte_eal_init_alert("Cannot detect lcores.");
> > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > index f36bc5568..606695a7f 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -739,25 +739,53 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
> >   }
> >   
> >   static int
> > -eal_parse_log_level(const char *level, uint32_t *log_level)
> > +eal_parse_log_level(const char *arg, struct internal_config *conf)
> >   {
> > -	char *end;
> > +	char *end, *str, *type, *level;
> >   	unsigned long tmp;
> >   
> > +	str = strdup(arg);
> > +	if (str == NULL)
> > +		return -1;
> > +
> > +	if (strchr(str, ',') == NULL) {
> > +		type = NULL;
> > +		level = str;
> > +	} else {
> > +		type = strsep(&str, ",");
> > +		level = strsep(&str, ",");
> > +	}
> > +
> >   	errno = 0;
> >   	tmp = strtoul(level, &end, 0);
> >   
> >   	/* check for errors */
> >   	if ((errno != 0) || (level[0] == '\0') ||
> > -	    end == NULL || (*end != '\0'))
> > -		return -1;
> > +		    end == NULL || (*end != '\0'))
> > +		goto fail;
> >   
> >   	/* log_level is a uint32_t */
> >   	if (tmp >= UINT32_MAX)
> > -		return -1;
> > +		goto fail;
> > +
> > +	printf("set log level %s,%lu\n",
> > +		type, tmp);
> > +
> > +	if (type == NULL) {
> > +		conf->log_level = tmp;
> > +		rte_set_log_level(tmp);
> > +	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
> > +		printf("cannot set log level %s,%lu\n",
> > +			type, tmp);
> > +		goto fail;
> > +	}
> >   
> > -	*log_level = tmp;
> > +	free(str);
> >   	return 0;
> > +
> > +fail:
> > +	free(str);
> > +	return -1;
> >   }
> >   
> >   static enum rte_proc_type_t
> > @@ -898,15 +926,12 @@ eal_parse_common_option(int opt, const char *optarg,
> >   		break;
> >   
> >   	case OPT_LOG_LEVEL_NUM: {
> > -		uint32_t log;
> > -
> > -		if (eal_parse_log_level(optarg, &log) < 0) {
> > +		if (eal_parse_log_level(optarg, conf) < 0) {
> >   			RTE_LOG(ERR, EAL,
> >   				"invalid parameters for --"
> >   				OPT_LOG_LEVEL "\n");
> >   			return -1;
> >   		}
> > -		conf->log_level = log;
> >   		break;
> >   	}
> >   	case OPT_LCORES_NUM:
> > @@ -1057,7 +1082,9 @@ eal_common_usage(void)
> >   	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
> >   	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
> >   	       "  --"OPT_SYSLOG"            Set syslog facility\n"
> > -	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
> > +	       "  --"OPT_LOG_LEVEL"=<int>   Set global log level\n"
> > +	       "  --"OPT_LOG_LEVEL"=<type-regexp>,<int>\n"
> > +	       "                      Set specific log level\n"
> >   	       "  -v                  Display version information on startup\n"
> >   	       "  -h, --help          This help\n"
> >   	       "\nEAL options for DEBUG use only:\n"
> > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> > index f0ded185b..d98d56d2f 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -776,10 +776,8 @@ rte_eal_init(int argc, char **argv)
> >   
> >   	thread_id = pthread_self();
> >   
> > -	eal_log_level_parse(argc, argv);
> > -
> >   	/* set log level as early as possible */
> > -	rte_set_log_level(internal_config.log_level);
> > +	eal_log_level_parse(argc, argv);
> >   
> >   	if (rte_eal_cpu_init() < 0) {
> >   		rte_eal_init_alert("Cannot detect lcores.");  
> 

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Tan, Jianfeng]

On 4/18/2017 4:50 PM, Olivier MATZ wrote:
> Hi Jianfeng,
>
> On Fri, 14 Apr 2017 13:33:49 +0800, "Tan, Jianfeng" <jianfeng.tan@intel.com> wrote:
>> Hi Olivier,
>>
>> If I understand it correctly, this patch is to shift log level setting
>> earlier. But we did not remove the one in eal_parse_common_option(). So
>> we can see this parameter will be analyzed twice. Does it make sense to
>> remove analysis of log level in eal_parse_common_option()?
>>
> The patch does not change the way the log level is parsed: it was
> already parsed twice, because we want to know the log level as soon
> as possible.

Oh, yes, it's not introduced in this patch. How do you think if we move 
log parser out from eal_parse_common_option to avoid parsing log twice?

>
> But the patch introduces a bug, as seen by Ferruh: the default log
> level is not set properly when no --log-level parameter is passed.
>

Before this one, there is another one on usage of regexec(), which I put 
it here: http://dpdk.org/ml/archives/dev/2017-April/064031.html.

Thanks,
Jianfeng

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Olivier MATZ]

Hi Jianfeng,

On Tue, 18 Apr 2017 19:15:51 +0800, "Tan, Jianfeng" <jianfeng.tan@intel.com> wrote:
> On 4/18/2017 4:50 PM, Olivier MATZ wrote:
> > Hi Jianfeng,
> >
> > On Fri, 14 Apr 2017 13:33:49 +0800, "Tan, Jianfeng" <jianfeng.tan@intel.com> wrote:  
> >> Hi Olivier,
> >>
> >> If I understand it correctly, this patch is to shift log level setting
> >> earlier. But we did not remove the one in eal_parse_common_option(). So
> >> we can see this parameter will be analyzed twice. Does it make sense to
> >> remove analysis of log level in eal_parse_common_option()?
> >>  
> > The patch does not change the way the log level is parsed: it was
> > already parsed twice, because we want to know the log level as soon
> > as possible.  
> 
> Oh, yes, it's not introduced in this patch. How do you think if we move 
> log parser out from eal_parse_common_option to avoid parsing log twice?

Not sure it's so easy to do. The second pass can ignore the --log-level
argument, but it has to be recognized, else we would have an error. I
think it's ok like this.

> 
> >
> > But the patch introduces a bug, as seen by Ferruh: the default log
> > level is not set properly when no --log-level parameter is passed.
> >  
> 
> Before this one, there is another one on usage of regexec(), which I put 
> it here: http://dpdk.org/ml/archives/dev/2017-April/064031.html.

Yes, I've seen it, I'll send a patch for this today. Thanks.


Olivier

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Ferruh Yigit]

Hi Olivier,

On 4/4/2017 5:40 PM, Olivier Matz wrote:
> Example of use:
>   ./app/test-pmd --log-level='pmd\.i40e.*,8'
> 
>   This enables debug logs for all dynamic logs whose type starts with
>   'pmd.i40e'.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

<...>

> index f0ded185b..d98d56d2f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -776,10 +776,8 @@ rte_eal_init(int argc, char **argv)
>  
>  	thread_id = pthread_self();
>  
> -	eal_log_level_parse(argc, argv);
> -
>  	/* set log level as early as possible */
> -	rte_set_log_level(internal_config.log_level);

Removing this line prevents using RTE_LOG_LEVEL config option.

When there is no command line argument provided, I think it makes sense
to use config option. Currently config option is set to INFO level but
app shows DEBUG level logs, this is confusing.

> +	eal_log_level_parse(argc, argv);
>  
>  	if (rte_eal_cpu_init() < 0) {
>  		rte_eal_init_alert("Cannot detect lcores.");
> 

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Olivier MATZ]

Hi Jianfeng, Ferruh,

Thank you for spotting these issues. I'm on it, I will send a patch
to fix them asap.

Olivier


On Fri, 14 Apr 2017 16:32:57 +0100, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> Hi Olivier,
> 
> On 4/4/2017 5:40 PM, Olivier Matz wrote:
> > Example of use:
> >   ./app/test-pmd --log-level='pmd\.i40e.*,8'
> > 
> >   This enables debug logs for all dynamic logs whose type starts with
> >   'pmd.i40e'.
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>  
> 
> <...>
> 
> > index f0ded185b..d98d56d2f 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -776,10 +776,8 @@ rte_eal_init(int argc, char **argv)
> >  
> >  	thread_id = pthread_self();
> >  
> > -	eal_log_level_parse(argc, argv);
> > -
> >  	/* set log level as early as possible */
> > -	rte_set_log_level(internal_config.log_level);  
> 
> Removing this line prevents using RTE_LOG_LEVEL config option.
> 
> When there is no command line argument provided, I think it makes sense
> to use config option. Currently config option is set to INFO level but
> app shows DEBUG level logs, this is confusing.
> 
> > +	eal_log_level_parse(argc, argv);
> >  
> >  	if (rte_eal_cpu_init() < 0) {
> >  		rte_eal_init_alert("Cannot detect lcores.");
> >   
> 

Re: [PATCH v3 4/8] eal: change specific log levels at startup

[Ferruh Yigit]

On 4/4/2017 5:40 PM, Olivier Matz wrote:
> Example of use:
>   ./app/test-pmd --log-level='pmd\.i40e.*,8'
> 
>   This enables debug logs for all dynamic logs whose type starts with
>   'pmd.i40e'.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

<...>

> +
> +	printf("set log level %s,%lu\n",
> +		type, tmp);

A printf seems left here, this prints always independent from log level,
do we want this?

[PATCH v3 5/8] eal: deprecate log functions

[Olivier Matz]

Deprecate the following functions:
- rte_set_log_level(), replaced by rte_log_set_global_level()
- rte_get_log_level(), replaced by rte_log_get_global_level()
- rte_set_log_type(), replaced by rte_log_set_level()
- rte_get_log_type(), replaced by rte_log_get_level()

The new functions provide a better control of the per-type log level,
and have a better name prefix (rte_log_).

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst            |  8 ++++++++
 examples/quota_watermark/qw/main.c              |  2 +-
 examples/quota_watermark/qwctl/qwctl.c          |  2 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
 lib/librte_eal/common/eal_common_log.c          | 26 ++++++++++++++++++++-----
 lib/librte_eal/common/eal_common_options.c      |  2 +-
 lib/librte_eal/common/include/rte_log.h         | 19 ++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
 test/test/test_logs.c                           |  6 +++---
 9 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d6544ed0a..4148c4c43 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -118,3 +118,11 @@ Deprecation Notices
   to specify which type of instance to create (single or burst), and
   additional calls for ``rte_distributor_poll_pkt_burst`` and
   ``rte_distributor_return_pkt_burst``, among others.
+
+* eal: the following functions are deprecated starting from 17.05 and will
+  be removed in 17.08:
+
+  - ``rte_set_log_level``, replaced by ``rte_log_set_global_level``
+  - ``rte_get_log_level``, replaced by ``rte_log_get_global_level``
+  - ``rte_set_log_type``, replaced by ``rte_log_set_level``
+  - ``rte_get_log_type``, replaced by ``rte_log_get_level``
diff --git a/examples/quota_watermark/qw/main.c b/examples/quota_watermark/qw/main.c
index bdb8a4399..d4fcfde48 100644
--- a/examples/quota_watermark/qw/main.c
+++ b/examples/quota_watermark/qw/main.c
@@ -326,7 +326,7 @@ main(int argc, char **argv)
 
 	uint8_t port_id;
 
-	rte_set_log_level(RTE_LOG_INFO);
+	rte_log_set_global_level(RTE_LOG_INFO);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
diff --git a/examples/quota_watermark/qwctl/qwctl.c b/examples/quota_watermark/qwctl/qwctl.c
index 7e7a396e5..18ec17a17 100644
--- a/examples/quota_watermark/qwctl/qwctl.c
+++ b/examples/quota_watermark/qwctl/qwctl.c
@@ -77,7 +77,7 @@ int main(int argc, char **argv)
 	int ret;
 	struct cmdline *cl;
 
-	rte_set_log_level(RTE_LOG_INFO);
+	rte_log_set_global_level(RTE_LOG_INFO);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index de74ff9ff..e4d3665d8 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -188,6 +188,8 @@ DPDK_17.05 {
 
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index d02689390..dd4d30ca7 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -84,20 +84,36 @@ rte_openlog_stream(FILE *f)
 
 /* Set global log level */
 void
-rte_set_log_level(uint32_t level)
+rte_log_set_global_level(uint32_t level)
 {
 	rte_logs.level = (uint32_t)level;
 }
 
+/* Set global log level */
+/* replaced by rte_log_set_global_level */
+__rte_deprecated void
+rte_set_log_level(uint32_t level)
+{
+	rte_log_set_global_level(level);
+}
+
 /* Get global log level */
 uint32_t
-rte_get_log_level(void)
+rte_log_get_global_level(void)
 {
 	return rte_logs.level;
 }
 
+/* Get global log level */
+/* replaced by rte_log_get_global_level */
+uint32_t
+rte_get_log_level(void)
+{
+	return rte_log_get_global_level();
+}
+
 /* Set global log type */
-void
+__rte_deprecated void
 rte_set_log_type(uint32_t type, int enable)
 {
 	if (type < RTE_LOGTYPE_FIRST_EXT_ID) {
@@ -114,7 +130,7 @@ rte_set_log_type(uint32_t type, int enable)
 }
 
 /* Get global log type */
-uint32_t
+__rte_deprecated uint32_t
 rte_get_log_type(void)
 {
 	return rte_logs.type;
@@ -287,7 +303,7 @@ rte_log_dump(FILE *f)
 	size_t i;
 
 	fprintf(f, "global log level is %s\n",
-		loglevel_to_string(rte_get_log_level()));
+		loglevel_to_string(rte_log_get_global_level()));
 
 	for (i = 0; i < rte_logs.dynamic_types_len; i++) {
 		if (rte_logs.dynamic_types[i].name == NULL)
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 606695a7f..32df2ef45 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -773,7 +773,7 @@ eal_parse_log_level(const char *arg, struct internal_config *conf)
 
 	if (type == NULL) {
 		conf->log_level = tmp;
-		rte_set_log_level(tmp);
+		rte_log_set_global_level(tmp);
 	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
 		printf("cannot set log level %s,%lu\n",
 			type, tmp);
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index ce48b0785..27c40b452 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -50,6 +50,8 @@ extern "C" {
 #include <stdio.h>
 #include <stdarg.h>
 
+#include <rte_common.h>
+
 struct rte_log_dynamic_type;
 
 /** The rte_log structure. */
@@ -132,11 +134,26 @@ int rte_openlog_stream(FILE *f);
  * @param level
  *   Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
  */
+void rte_log_set_global_level(uint32_t level);
+
+/**
+ * Deprecated, replaced by rte_log_set_global_level().
+ */
+__rte_deprecated
 void rte_set_log_level(uint32_t level);
 
 /**
  * Get the global log level.
+ *
+ * @return
+ *   The current global log level.
+ */
+uint32_t rte_log_get_global_level(void);
+
+/**
+ * Deprecated, replaced by rte_log_get_global_level().
  */
+__rte_deprecated
 uint32_t rte_get_log_level(void);
 
 /**
@@ -147,11 +164,13 @@ uint32_t rte_get_log_level(void);
  * @param enable
  *   True for enable; false for disable.
  */
+__rte_deprecated
 void rte_set_log_type(uint32_t type, int enable);
 
 /**
  * Get the global log type.
  */
+__rte_deprecated
 uint32_t rte_get_log_type(void);
 
 /**
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index f70b4b937..4c55114d6 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -192,6 +192,8 @@ DPDK_17.05 {
 
 	rte_log_dump;
 	rte_log_register;
+	rte_log_get_global_level;
+	rte_log_set_global_level;
 	rte_log_set_level;
 	rte_log_set_level_regexp;
 
diff --git a/test/test/test_logs.c b/test/test/test_logs.c
index 805c56810..730a86bd9 100644
--- a/test/test/test_logs.c
+++ b/test/test/test_logs.c
@@ -66,12 +66,12 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_EMERG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(CRIT, TESTAPP1, "critical message\n");
 
 	/* log in critical level */
-	rte_set_log_level(RTE_LOG_CRIT);
+	rte_log_set_global_level(RTE_LOG_CRIT);
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 	RTE_LOG(CRIT, TESTAPP2, "critical message\n");
 
@@ -79,7 +79,7 @@ test_logs(void)
 	rte_log_set_level(RTE_LOGTYPE_TESTAPP2, RTE_LOG_DEBUG);
 
 	/* log in error level */
-	rte_set_log_level(RTE_LOG_ERR);
+	rte_log_set_global_level(RTE_LOG_ERR);
 	RTE_LOG(ERR, TESTAPP1, "error message\n");
 	RTE_LOG(ERR, TESTAPP2, "error message (not displayed)\n");
 
-- 
2.11.0

[PATCH v3 6/8] app/test: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 test/test/commands.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test/commands.c b/test/test/commands.c
index 551c81dcf..4097a3310 100644
--- a/test/test/commands.c
+++ b/test/test/commands.c
@@ -158,13 +158,15 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
 	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
 				 "dump_physmem#dump_memzone#"
 				 "dump_struct_sizes#dump_ring#dump_mempool#"
-				 "dump_devargs");
+				 "dump_devargs#dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.11.0

[PATCH v3 7/8] app/testpmd: new command to dump log types

[Olivier Matz]

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/cmdline.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d20..697228fa6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7679,6 +7679,8 @@ static void cmd_dump_parsed(void *parsed_result,
 		rte_mempool_list_dump(stdout);
 	else if (!strcmp(res->dump, "dump_devargs"))
 		rte_eal_devargs_dump(stdout);
+	else if (!strcmp(res->dump, "dump_log_types"))
+		rte_log_dump(stdout);
 }
 
 cmdline_parse_token_string_t cmd_dump_dump =
@@ -7688,7 +7690,8 @@ cmdline_parse_token_string_t cmd_dump_dump =
 		"dump_struct_sizes#"
 		"dump_ring#"
 		"dump_mempool#"
-		"dump_devargs");
+		"dump_devargs#"
+		"dump_log_types");
 
 cmdline_parse_inst_t cmd_dump = {
 	.f = cmd_dump_parsed,  /* function to call */
-- 
2.11.0

[PATCH v3 8/8] net/i40e: use dynamic log type for control logs

[Olivier Matz]

This is an example of how a dynamic log type can be used in a
PMD.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 config/common_base             |  2 --
 drivers/net/i40e/i40e_ethdev.c | 18 ++++++++++++++++--
 drivers/net/i40e/i40e_fdir.c   |  4 ----
 drivers/net/i40e/i40e_logs.h   | 17 ++++++-----------
 4 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/config/common_base b/config/common_base
index 41191c878..d29808c7a 100644
--- a/config/common_base
+++ b/config/common_base
@@ -179,11 +179,9 @@ CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=y
 # Compile burst-oriented I40E PMD driver
 #
 CONFIG_RTE_LIBRTE_I40E_PMD=y
-CONFIG_RTE_LIBRTE_I40E_DEBUG_INIT=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_I40E_DEBUG_TX_FREE=n
-CONFIG_RTE_LIBRTE_I40E_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC=y
 CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y
 CONFIG_RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE=y
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 8b5fd54e7..a6de86280 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -40,6 +40,7 @@
 #include <inttypes.h>
 #include <assert.h>
 
+#include <rte_eal.h>
 #include <rte_string_fns.h>
 #include <rte_pci.h>
 #include <rte_ether.h>
@@ -419,6 +420,9 @@ static void i40e_ethertype_filter_restore(struct i40e_pf *pf);
 static void i40e_tunnel_filter_restore(struct i40e_pf *pf);
 static void i40e_filter_restore(struct i40e_pf *pf);
 
+int i40e_logtype_init;
+int i40e_logtype_driver;
+
 static const struct rte_pci_id pci_id_i40e_map[] = {
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_SFP_XL710) },
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_QEMU) },
@@ -5790,7 +5794,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(INFO, "No interrupt event");
 		goto done;
 	}
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	if (icr0 & I40E_PFINT_ICR0_ECC_ERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: unrecoverable ECC error");
 	if (icr0 & I40E_PFINT_ICR0_MAL_DETECT_MASK)
@@ -5805,7 +5808,6 @@ i40e_dev_interrupt_handler(struct rte_intr_handle *intr_handle,
 		PMD_DRV_LOG(ERR, "ICR0: HMC error");
 	if (icr0 & I40E_PFINT_ICR0_PE_CRITERR_MASK)
 		PMD_DRV_LOG(ERR, "ICR0: protocol engine critical error");
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	if (icr0 & I40E_PFINT_ICR0_VFLR_MASK) {
 		PMD_DRV_LOG(INFO, "ICR0: VF reset detected");
@@ -11214,3 +11216,15 @@ rte_pmd_i40e_reset_vf_stats(uint8_t port,
 
 	return 0;
 }
+
+RTE_INIT(i40e_init_log);
+static void
+i40e_init_log(void)
+{
+	i40e_logtype_init = rte_log_register("pmd.i40e.init");
+	if (i40e_logtype_init >= 0)
+		rte_log_set_level(i40e_logtype_init, RTE_LOG_NOTICE);
+	i40e_logtype_driver = rte_log_register("pmd.i40e.driver");
+	if (i40e_logtype_driver >= 0)
+		rte_log_set_level(i40e_logtype_driver, RTE_LOG_NOTICE);
+}
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 0700253b1..32d3b1910 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1592,17 +1592,14 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	struct rte_eth_dev *dev = I40E_VSI_TO_ETH_DEV(pf->main_vsi);
 	struct i40e_fdir_filter_list *fdir_list = &pf->fdir.fdir_list;
 	struct i40e_fdir_filter *f;
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	uint32_t fdstat;
 	uint32_t guarant_cnt;  /**< Number of filters in guaranteed spaces. */
 	uint32_t best_cnt;     /**< Number of filters in best effort spaces. */
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	TAILQ_FOREACH(f, fdir_list, rules)
 		i40e_add_del_fdir_filter(dev, &f->fdir, TRUE);
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 	fdstat = I40E_READ_REG(hw, I40E_PFQF_FDSTAT);
 	guarant_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_GUARANT_CNT_MASK) >>
@@ -1610,7 +1607,6 @@ i40e_fdir_filter_restore(struct i40e_pf *pf)
 	best_cnt =
 		(uint32_t)((fdstat & I40E_PFQF_FDSTAT_BEST_CNT_MASK) >>
 			   I40E_PFQF_FDSTAT_BEST_CNT_SHIFT);
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 
 	PMD_DRV_LOG(INFO, "FDIR: Guarant count: %d,  Best count: %d",
 		    guarant_cnt, best_cnt);
diff --git a/drivers/net/i40e/i40e_logs.h b/drivers/net/i40e/i40e_logs.h
index e042e2429..8e99cd52a 100644
--- a/drivers/net/i40e/i40e_logs.h
+++ b/drivers/net/i40e/i40e_logs.h
@@ -34,14 +34,11 @@
 #ifndef _I40E_LOGS_H_
 #define _I40E_LOGS_H_
 
+extern int i40e_logtype_init;
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ##args)
-
-#ifdef RTE_LIBRTE_I40E_DEBUG_INIT
+	rte_log(RTE_LOG_ ## level, i40e_logtype_init, "%s(): " fmt "\n", \
+		__func__, ##args)
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
-#else
-#define PMD_INIT_FUNC_TRACE() do { } while(0)
-#endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
@@ -64,12 +61,10 @@
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
+extern int i40e_logtype_driver;
 #define PMD_DRV_LOG_RAW(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
-#else
-#define PMD_DRV_LOG_RAW(level, fmt, args...) do { } while (0)
-#endif
+	rte_log(RTE_LOG_ ## level, i40e_logtype_driver, "%s(): " fmt, \
+		__func__, ## args)
 
 #define PMD_DRV_LOG(level, fmt, args...) \
 	PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
-- 
2.11.0

Re: [PATCH v3 0/8] eal: dynamic logs

[Thomas Monjalon]

2017-04-04 18:40, Olivier Matz:
> The objective of this patchset is to introduce a framework to
> support dynamic log types in EAL. It also provides one example of use
> (in i40e).
> 
> Features:
> - log types are identified by a string
> - at registration, a uniq identifier is associated to a log type
> - each log type can have its level changed dynamically
> - extend command line parameters to set the log level of a specific
>   type, or logs matching a regular expression
> - keep compat with other legacy types (eal, malloc, ring, user*,
>   etc... keep their hardcoded log type value)

Applied, thanks

> Next step is to adapt drivers, libs and apps to use this new API. At the
> end, we can expect that all non-dataplane logs are moved to be dynamic,
> so we can enable/disable them at runtime, without recompiling. Many
> debug options can probably be removed from configuration:
>   $ git grep DEBUG config/common_base | wc -l
>   89

Please maintainers, let's remove these useless DEBUG options!
Thanks

Re: [PATCH v3 0/8] eal: dynamic logs

[De Lara Guarch, Pablo]

Hi Olivier,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, April 04, 2017 5:41 PM
> To: dev@dpdk.org
> Cc: david.marchand@6wind.com; Richardson, Bruce;
> thomas.monjalon@6wind.com; Wiles, Keith;
> stephen@networkplumber.org
> Subject: [dpdk-dev] [PATCH v3 0/8] eal: dynamic logs
> 
> The objective of this patchset is to introduce a framework to
> support dynamic log types in EAL. It also provides one example of use
> (in i40e).
> 
> Features:
> - log types are identified by a string
> - at registration, a uniq identifier is associated to a log type
> - each log type can have its level changed dynamically
> - extend command line parameters to set the log level of a specific
>   type, or logs matching a regular expression
> - keep compat with other legacy types (eal, malloc, ring, user*,
>   etc... keep their hardcoded log type value)
> 
> Next step is to adapt drivers, libs and apps to use this new API. At the
> end, we can expect that all non-dataplane logs are moved to be dynamic,
> so we can enable/disable them at runtime, without recompiling. Many
> debug options can probably be removed from configuration:
>   $ git grep DEBUG config/common_base | wc -l
>   89
> 
> v2 -> v3:
> - fix compilation of intermediate patches
> - replace references to 17.02 by 17.05 (doc and .map)
> 
> v1 -> v2:
> - fix issues reported by Stephen
> - add entry in release note and deprecation notice
> - rebase on top of master
> 
> RFC -> v1:
> - rebase on top of current master
> - fix eal help alignment
> - remove unused i40e compilation options
> 
> 

With this patch, all logs that use logtype "USERX" (e.g.  RTE_LOGTYPE_USER1) are not shown anymore.
Should these macro be removed?

Right now, all applications using this won't show these, so I assume that all of them
should be fixed before the release is out.
Is that correct?

Thanks,
Pablo

Re: [PATCH v3 0/8] eal: dynamic logs

[Thomas Monjalon]

2017-04-12 09:26, De Lara Guarch, Pablo:
> Hi Olivier,
> 
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > 
> > The objective of this patchset is to introduce a framework to
> > support dynamic log types in EAL. It also provides one example of use
> > (in i40e).
> > 
> > Features:
> > - log types are identified by a string
> > - at registration, a uniq identifier is associated to a log type
> > - each log type can have its level changed dynamically
> > - extend command line parameters to set the log level of a specific
> >   type, or logs matching a regular expression
> > - keep compat with other legacy types (eal, malloc, ring, user*,
> >   etc... keep their hardcoded log type value)
> > 
> > Next step is to adapt drivers, libs and apps to use this new API. At the
> > end, we can expect that all non-dataplane logs are moved to be dynamic,
> > so we can enable/disable them at runtime, without recompiling. Many
> > debug options can probably be removed from configuration:
> >   $ git grep DEBUG config/common_base | wc -l
> >   89
[...]
> With this patch, all logs that use logtype "USERX" (e.g.  RTE_LOGTYPE_USER1) are not shown anymore.
> Should these macro be removed?
> 
> Right now, all applications using this won't show these, so I assume that all of them
> should be fixed before the release is out.
> Is that correct?

Is it a bug in the commit http://dpdk.org/commit/c1b5fa9 ?
Note this line:
	__rte_log_register("user1", 24);

Re: [PATCH v3 0/8] eal: dynamic logs

[De Lara Guarch, Pablo]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, April 12, 2017 11:38 AM
> To: De Lara Guarch, Pablo
> Cc: Olivier Matz; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/8] eal: dynamic logs
> 
> 2017-04-12 09:26, De Lara Guarch, Pablo:
> > Hi Olivier,
> >
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > >
> > > The objective of this patchset is to introduce a framework to
> > > support dynamic log types in EAL. It also provides one example of use
> > > (in i40e).
> > >
> > > Features:
> > > - log types are identified by a string
> > > - at registration, a uniq identifier is associated to a log type
> > > - each log type can have its level changed dynamically
> > > - extend command line parameters to set the log level of a specific
> > >   type, or logs matching a regular expression
> > > - keep compat with other legacy types (eal, malloc, ring, user*,
> > >   etc... keep their hardcoded log type value)
> > >
> > > Next step is to adapt drivers, libs and apps to use this new API. At the
> > > end, we can expect that all non-dataplane logs are moved to be
> dynamic,
> > > so we can enable/disable them at runtime, without recompiling. Many
> > > debug options can probably be removed from configuration:
> > >   $ git grep DEBUG config/common_base | wc -l
> > >   89
> [...]
> > With this patch, all logs that use logtype "USERX" (e.g.
> RTE_LOGTYPE_USER1) are not shown anymore.
> > Should these macro be removed?
> >
> > Right now, all applications using this won't show these, so I assume that
> all of them
> > should be fixed before the release is out.
> > Is that correct?
> 
> Is it a bug in the commit http://dpdk.org/commit/c1b5fa9 ?
> Note this line:
> 	__rte_log_register("user1", 24);

This also happens with libraries, like HASH. I think the problem might be here:

@@ -139,7 +266,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 		}
 	}
 
-	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
+	if (level > rte_logs.level)
+		return 0;
+	if (logtype >= rte_logs.dynamic_types_len)
+		return -1;
+	if (level > rte_logs.dynamic_types[logtype].loglevel)
 		return 0;

Type used to be a mask, so for instance, RTE_LOGTYPE_HASH 0x0...40 = 64 in decimal.
Therefore, logtype = 64 >= 34 = rte_logs.dynamic_types_len), so this won't be shown.
However, the PMDs or EAL will show it, because their logtype is less than 34.

Am I missing something?

Thanks,
Pablo

Re: [PATCH v3 0/8] eal: dynamic logs

[Thomas Monjalon]

2017-04-12 13:11, De Lara Guarch, Pablo:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Wednesday, April 12, 2017 11:38 AM
> > 2017-04-12 09:26, De Lara Guarch, Pablo:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > >
> > > > The objective of this patchset is to introduce a framework to
> > > > support dynamic log types in EAL. It also provides one example of use
> > > > (in i40e).
> > > >
> > > > Features:
> > > > - log types are identified by a string
> > > > - at registration, a uniq identifier is associated to a log type
> > > > - each log type can have its level changed dynamically
> > > > - extend command line parameters to set the log level of a specific
> > > >   type, or logs matching a regular expression
> > > > - keep compat with other legacy types (eal, malloc, ring, user*,
> > > >   etc... keep their hardcoded log type value)
> > > >
> > > > Next step is to adapt drivers, libs and apps to use this new API. At the
> > > > end, we can expect that all non-dataplane logs are moved to be
> > dynamic,
> > > > so we can enable/disable them at runtime, without recompiling. Many
> > > > debug options can probably be removed from configuration:
> > > >   $ git grep DEBUG config/common_base | wc -l
> > > >   89
> > [...]
> > > With this patch, all logs that use logtype "USERX" (e.g.
> > RTE_LOGTYPE_USER1) are not shown anymore.
> > > Should these macro be removed?
> > >
> > > Right now, all applications using this won't show these, so I assume that
> > all of them
> > > should be fixed before the release is out.
> > > Is that correct?
> > 
> > Is it a bug in the commit http://dpdk.org/commit/c1b5fa9 ?
> > Note this line:
> > 	__rte_log_register("user1", 24);
> 
> This also happens with libraries, like HASH. I think the problem might be here:
> 
> @@ -139,7 +266,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
>  		}
>  	}
>  
> -	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
> +	if (level > rte_logs.level)
> +		return 0;
> +	if (logtype >= rte_logs.dynamic_types_len)
> +		return -1;
> +	if (level > rte_logs.dynamic_types[logtype].loglevel)
>  		return 0;
> 
> Type used to be a mask, so for instance, RTE_LOGTYPE_HASH 0x0...40 = 64 in decimal.
> Therefore, logtype = 64 >= 34 = rte_logs.dynamic_types_len), so this won't be shown.
> However, the PMDs or EAL will show it, because their logtype is less than 34.
> 
> Am I missing something?

I think you are right.
We are mixing old logtype and new ones.
Should we redefine RTE_LOGTYPE_* values to the new values registered
in rte_log_init?

Re: [PATCH v3 0/8] eal: dynamic logs

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, April 12, 2017 2:29 PM
> To: De Lara Guarch, Pablo
> Cc: Olivier Matz; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/8] eal: dynamic logs
> 
> 2017-04-12 13:11, De Lara Guarch, Pablo:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > Sent: Wednesday, April 12, 2017 11:38 AM
> > > 2017-04-12 09:26, De Lara Guarch, Pablo:
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > > >
> > > > > The objective of this patchset is to introduce a framework to
> > > > > support dynamic log types in EAL. It also provides one example of
> use
> > > > > (in i40e).
> > > > >
> > > > > Features:
> > > > > - log types are identified by a string
> > > > > - at registration, a uniq identifier is associated to a log type
> > > > > - each log type can have its level changed dynamically
> > > > > - extend command line parameters to set the log level of a specific
> > > > >   type, or logs matching a regular expression
> > > > > - keep compat with other legacy types (eal, malloc, ring, user*,
> > > > >   etc... keep their hardcoded log type value)
> > > > >
> > > > > Next step is to adapt drivers, libs and apps to use this new API. At
> the
> > > > > end, we can expect that all non-dataplane logs are moved to be
> > > dynamic,
> > > > > so we can enable/disable them at runtime, without recompiling.
> Many
> > > > > debug options can probably be removed from configuration:
> > > > >   $ git grep DEBUG config/common_base | wc -l
> > > > >   89
> > > [...]
> > > > With this patch, all logs that use logtype "USERX" (e.g.
> > > RTE_LOGTYPE_USER1) are not shown anymore.
> > > > Should these macro be removed?
> > > >
> > > > Right now, all applications using this won't show these, so I assume
> that
> > > all of them
> > > > should be fixed before the release is out.
> > > > Is that correct?
> > >
> > > Is it a bug in the commit http://dpdk.org/commit/c1b5fa9 ?
> > > Note this line:
> > > 	__rte_log_register("user1", 24);
> >
> > This also happens with libraries, like HASH. I think the problem might be
> here:
> >
> > @@ -139,7 +266,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const
> char *format, va_list ap)
> >  		}
> >  	}
> >
> > -	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
> > +	if (level > rte_logs.level)
> > +		return 0;
> > +	if (logtype >= rte_logs.dynamic_types_len)
> > +		return -1;
> > +	if (level > rte_logs.dynamic_types[logtype].loglevel)
> >  		return 0;
> >
> > Type used to be a mask, so for instance, RTE_LOGTYPE_HASH 0x0...40 =
> 64 in decimal.
> > Therefore, logtype = 64 >= 34 = rte_logs.dynamic_types_len), so this
> won't be shown.
> > However, the PMDs or EAL will show it, because their logtype is less than
> 34.
> >
> > Am I missing something?
> 
> I think you are right.
> We are mixing old logtype and new ones.
> Should we redefine RTE_LOGTYPE_* values to the new values registered
> in rte_log_init?

I guess. I can send a patch for it and any concerns can be raised there?

Pablo

Re: [PATCH v3 0/8] eal: dynamic logs

[Thomas Monjalon]

2017-04-12 13:47, De Lara Guarch, Pablo:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2017-04-12 13:11, De Lara Guarch, Pablo:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > Sent: Wednesday, April 12, 2017 11:38 AM
> > > > 2017-04-12 09:26, De Lara Guarch, Pablo:
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > > > >
> > > > > > The objective of this patchset is to introduce a framework to
> > > > > > support dynamic log types in EAL. It also provides one example of
> > use
> > > > > > (in i40e).
> > > > > >
> > > > > > Features:
> > > > > > - log types are identified by a string
> > > > > > - at registration, a uniq identifier is associated to a log type
> > > > > > - each log type can have its level changed dynamically
> > > > > > - extend command line parameters to set the log level of a specific
> > > > > >   type, or logs matching a regular expression
> > > > > > - keep compat with other legacy types (eal, malloc, ring, user*,
> > > > > >   etc... keep their hardcoded log type value)
> > > > > >
> > > > > > Next step is to adapt drivers, libs and apps to use this new API. At
> > the
> > > > > > end, we can expect that all non-dataplane logs are moved to be
> > > > dynamic,
> > > > > > so we can enable/disable them at runtime, without recompiling.
> > Many
> > > > > > debug options can probably be removed from configuration:
> > > > > >   $ git grep DEBUG config/common_base | wc -l
> > > > > >   89
> > > > [...]
> > > > > With this patch, all logs that use logtype "USERX" (e.g.
> > > > RTE_LOGTYPE_USER1) are not shown anymore.
> > > > > Should these macro be removed?
> > > > >
> > > > > Right now, all applications using this won't show these, so I assume
> > that
> > > > all of them
> > > > > should be fixed before the release is out.
> > > > > Is that correct?
> > > >
> > > > Is it a bug in the commit http://dpdk.org/commit/c1b5fa9 ?
> > > > Note this line:
> > > > 	__rte_log_register("user1", 24);
> > >
> > > This also happens with libraries, like HASH. I think the problem might be
> > here:
> > >
> > > @@ -139,7 +266,11 @@ rte_vlog(uint32_t level, uint32_t logtype, const
> > char *format, va_list ap)
> > >  		}
> > >  	}
> > >
> > > -	if ((level > rte_logs.level) || !(logtype & rte_logs.type))
> > > +	if (level > rte_logs.level)
> > > +		return 0;
> > > +	if (logtype >= rte_logs.dynamic_types_len)
> > > +		return -1;
> > > +	if (level > rte_logs.dynamic_types[logtype].loglevel)
> > >  		return 0;
> > >
> > > Type used to be a mask, so for instance, RTE_LOGTYPE_HASH 0x0...40 =
> > 64 in decimal.
> > > Therefore, logtype = 64 >= 34 = rte_logs.dynamic_types_len), so this
> > won't be shown.
> > > However, the PMDs or EAL will show it, because their logtype is less than
> > 34.
> > >
> > > Am I missing something?
> > 
> > I think you are right.
> > We are mixing old logtype and new ones.
> > Should we redefine RTE_LOGTYPE_* values to the new values registered
> > in rte_log_init?
> 
> I guess. I can send a patch for it and any concerns can be raised there?

OK, thank you