[dpdk-dev,1/8] eal: support dynamic log types

Message ID 1489765882-30497-2-git-send-email-olivier.matz@6wind.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show

Checks

Context Check Description
checkpatch success coding style OK
Intel-compilation success Compilation OK

Commit Message

Olivier Matz March 17, 2017, 3:51 p.m.
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(-)

Comments

Stephen Hemminger March 17, 2017, 4:13 p.m.
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.
Stephen Hemminger March 17, 2017, 4:14 p.m.
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
Stephen Hemminger March 17, 2017, 4:15 p.m.
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?
Stephen Hemminger March 17, 2017, 4:17 p.m.
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.
Olivier Matz March 17, 2017, 4:40 p.m.
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

Patch hide | download patch | download mbox

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