diff mbox series

[v4,07/11] eal: add log level help

Message ID 20210321223116.1340974-8-thomas@monjalon.net (mailing list archive)
State Superseded
Delegated to: David Marchand
Headers show
Series improve options help | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon March 21, 2021, 10:31 p.m. UTC
The option --log-level was not completely described in the usage text,
and it was difficult to guess the names of the log types and levels.

A new value "help" is accepted after --log-level to give more details
about the syntax and listing the log types and levels.

The array "levels" used for level name parsing is replaced with
a (modified) existing function which was used in rte_log_dump().

The new function rte_log_list_types() is exported in the API
for allowing an application to give this info to the user
if not exposing the EAL option --log-level.
The list of log types cannot include all drivers if not linked in the
application (shared object plugin case).

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 lib/librte_eal/common/eal_common_log.c     | 24 +++++++++---
 lib/librte_eal/common/eal_common_options.c | 44 +++++++++++++++-------
 lib/librte_eal/common/eal_log.h            |  5 +++
 lib/librte_eal/include/rte_log.h           | 11 ++++++
 lib/librte_eal/version.map                 |  1 +
 5 files changed, 67 insertions(+), 18 deletions(-)

Comments

David Marchand March 23, 2021, 1:37 p.m. UTC | #1
On Sun, Mar 21, 2021 at 11:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The option --log-level was not completely described in the usage text,
> and it was difficult to guess the names of the log types and levels.
>
> A new value "help" is accepted after --log-level to give more details
> about the syntax and listing the log types and levels.
>
> The array "levels" used for level name parsing is replaced with
> a (modified) existing function which was used in rte_log_dump().

If we forget about the slightly different formatting, why not simply
reuse rte_log_dump?
It has the advantage of listing the default level for each logtype
that a user cannot guess otherwise.
Thomas Monjalon March 23, 2021, 3:10 p.m. UTC | #2
23/03/2021 14:37, David Marchand:
> On Sun, Mar 21, 2021 at 11:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > The option --log-level was not completely described in the usage text,
> > and it was difficult to guess the names of the log types and levels.
> >
> > A new value "help" is accepted after --log-level to give more details
> > about the syntax and listing the log types and levels.
> >
> > The array "levels" used for level name parsing is replaced with
> > a (modified) existing function which was used in rte_log_dump().
> 
> If we forget about the slightly different formatting, why not simply
> reuse rte_log_dump?
> It has the advantage of listing the default level for each logtype
> that a user cannot guess otherwise.

I considered rte_log_dump() too much verbose for the help text.
It is printing the global level and the logtype ids.
The need for the end-user in the help is different of a debug dump.
David Marchand March 23, 2021, 6:18 p.m. UTC | #3
On Tue, Mar 23, 2021 at 4:10 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 23/03/2021 14:37, David Marchand:
> > On Sun, Mar 21, 2021 at 11:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > >
> > > The option --log-level was not completely described in the usage text,
> > > and it was difficult to guess the names of the log types and levels.
> > >
> > > A new value "help" is accepted after --log-level to give more details
> > > about the syntax and listing the log types and levels.
> > >
> > > The array "levels" used for level name parsing is replaced with
> > > a (modified) existing function which was used in rte_log_dump().
> >
> > If we forget about the slightly different formatting, why not simply
> > reuse rte_log_dump?
> > It has the advantage of listing the default level for each logtype
> > that a user cannot guess otherwise.
>
> I considered rte_log_dump() too much verbose for the help text.
> It is printing the global level and the logtype ids.

Yes, it gives all the info about the log subsystem.

> The need for the end-user in the help is different of a debug dump.

The result of rte_log_list_types is a list in link-time order.
At least it is worth sorting alphabetically for users.

What about default log levels?
Thomas Monjalon March 23, 2021, 6:41 p.m. UTC | #4
23/03/2021 19:18, David Marchand:
> On Tue, Mar 23, 2021 at 4:10 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 23/03/2021 14:37, David Marchand:
> > > On Sun, Mar 21, 2021 at 11:32 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > >
> > > > The option --log-level was not completely described in the usage text,
> > > > and it was difficult to guess the names of the log types and levels.
> > > >
> > > > A new value "help" is accepted after --log-level to give more details
> > > > about the syntax and listing the log types and levels.
> > > >
> > > > The array "levels" used for level name parsing is replaced with
> > > > a (modified) existing function which was used in rte_log_dump().
> > >
> > > If we forget about the slightly different formatting, why not simply
> > > reuse rte_log_dump?
> > > It has the advantage of listing the default level for each logtype
> > > that a user cannot guess otherwise.
> >
> > I considered rte_log_dump() too much verbose for the help text.
> > It is printing the global level and the logtype ids.
> 
> Yes, it gives all the info about the log subsystem.
> 
> > The need for the end-user in the help is different of a debug dump.
> 
> The result of rte_log_list_types is a list in link-time order.
> At least it is worth sorting alphabetically for users.

Yes sorting could be nice.

> What about default log levels?

No strong opinion. The default is more or less always the same.
If you believe it helps the user, then OK to add default levels.
David Marchand March 24, 2021, 1:41 p.m. UTC | #5
On Tue, Mar 23, 2021 at 7:42 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > The need for the end-user in the help is different of a debug dump.
> >
> > The result of rte_log_list_types is a list in link-time order.
> > At least it is worth sorting alphabetically for users.
>
> Yes sorting could be nice.
>
> > What about default log levels?
>
> No strong opinion. The default is more or less always the same.

"more or less" yes.

Looking at your patch that only cares about listing logtypes.
Ok, let's leave it at that, with a sorted list.

I'll try to revisit this later, as finding/restoring default loglevel
during a debugging session is a pain.
diff mbox series

Patch

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 40cac36f89..d695b04068 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -397,12 +397,12 @@  RTE_INIT_PRIO(log_init, LOG)
 	rte_logs.dynamic_types_len = RTE_LOGTYPE_FIRST_EXT_ID;
 }
 
-static const char *
-loglevel_to_string(uint32_t level)
+const char *
+eal_log_level2str(uint32_t level)
 {
 	switch (level) {
 	case 0: return "disabled";
-	case RTE_LOG_EMERG: return "emerg";
+	case RTE_LOG_EMERG: return "emergency";
 	case RTE_LOG_ALERT: return "alert";
 	case RTE_LOG_CRIT: return "critical";
 	case RTE_LOG_ERR: return "error";
@@ -414,6 +414,20 @@  loglevel_to_string(uint32_t level)
 	}
 }
 
+/* Dump name of each logtype, one per line. */
+void
+rte_log_list_types(FILE *out, const char *prefix)
+{
+	size_t type;
+
+	for (type = 0; type < rte_logs.dynamic_types_len; ++type) {
+		if (rte_logs.dynamic_types[type].name == NULL)
+			continue;
+		fprintf(out, "%s%s\n",
+				prefix, rte_logs.dynamic_types[type].name);
+	}
+}
+
 /* dump global level and registered log types */
 void
 rte_log_dump(FILE *f)
@@ -421,14 +435,14 @@  rte_log_dump(FILE *f)
 	size_t i;
 
 	fprintf(f, "global log level is %s\n",
-		loglevel_to_string(rte_log_get_global_level()));
+		eal_log_level2str(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));
+			eal_log_level2str(rte_logs.dynamic_types[i].loglevel));
 	}
 }
 
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 2df3ae04ea..1da6583d71 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -1227,19 +1227,31 @@  eal_parse_syslog(const char *facility, struct internal_config *conf)
 }
 #endif
 
+static void
+eal_log_usage(void)
+{
+	unsigned int level;
+
+	printf("Log type is a pattern matching items of this list"
+			" (plugins may be missing):\n");
+	rte_log_list_types(stdout, "\t");
+	printf("\n");
+	printf("Syntax using globbing pattern:     ");
+	printf("--"OPT_LOG_LEVEL" pattern:level\n");
+	printf("Syntax using regular expression:   ");
+	printf("--"OPT_LOG_LEVEL" regexp,level\n");
+	printf("Syntax for the global level:       ");
+	printf("--"OPT_LOG_LEVEL" level\n");
+	printf("Logs are emitted if allowed by both global and specific levels.\n");
+	printf("\n");
+	printf("Log level can be a number or the first letters of its name:\n");
+	for (level = 1; level <= RTE_LOG_MAX; level++)
+		printf("\t%d   %s\n", level, eal_log_level2str(level));
+}
+
 static int
 eal_parse_log_priority(const char *level)
 {
-	static const char * const levels[] = {
-		[RTE_LOG_EMERG]   = "emergency",
-		[RTE_LOG_ALERT]   = "alert",
-		[RTE_LOG_CRIT]    = "critical",
-		[RTE_LOG_ERR]     = "error",
-		[RTE_LOG_WARNING] = "warning",
-		[RTE_LOG_NOTICE]  = "notice",
-		[RTE_LOG_INFO]    = "info",
-		[RTE_LOG_DEBUG]   = "debug",
-	};
 	size_t len = strlen(level);
 	unsigned long tmp;
 	char *end;
@@ -1250,7 +1262,7 @@  eal_parse_log_priority(const char *level)
 
 	/* look for named values, skip 0 which is not a valid level */
 	for (i = 1; i <= RTE_LOG_MAX; i++) {
-		if (strncmp(levels[i], level, len) == 0)
+		if (strncmp(eal_log_level2str(i), level, len) == 0)
 			return i;
 	}
 
@@ -1274,6 +1286,11 @@  eal_parse_log_level(const char *arg)
 	char *str, *level;
 	int priority;
 
+	if (strcmp(arg, "help") == 0) {
+		eal_log_usage();
+		exit(EXIT_SUCCESS);
+	}
+
 	str = strdup(arg);
 	if (str == NULL)
 		return -1;
@@ -2067,9 +2084,10 @@  eal_common_usage(void)
 #ifndef RTE_EXEC_ENV_WINDOWS
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
 #endif
-	       "  --"OPT_LOG_LEVEL"=<int>   Set global log level\n"
-	       "  --"OPT_LOG_LEVEL"=<type-match>:<int>\n"
+	       "  --"OPT_LOG_LEVEL"=<level> Set global log level\n"
+	       "  --"OPT_LOG_LEVEL"=<type-match>:<level>\n"
 	       "                      Set specific log level\n"
+	       "  --"OPT_LOG_LEVEL"=help    Show log types and levels\n"
 #ifndef RTE_EXEC_ENV_WINDOWS
 	       "  --"OPT_TRACE"=<regex-match>\n"
 	       "                      Enable trace based on regular expression trace name.\n"
diff --git a/lib/librte_eal/common/eal_log.h b/lib/librte_eal/common/eal_log.h
index 684650a17b..c784fa6043 100644
--- a/lib/librte_eal/common/eal_log.h
+++ b/lib/librte_eal/common/eal_log.h
@@ -24,4 +24,9 @@  void eal_log_set_default(FILE *default_log);
 int eal_log_save_regexp(const char *regexp, uint32_t level);
 int eal_log_save_pattern(const char *pattern, uint32_t level);
 
+/*
+ * Convert log level to string.
+ */
+const char *eal_log_level2str(uint32_t level);
+
 #endif /* EAL_LOG_H */
diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
index 394e8682b9..e6192892c3 100644
--- a/lib/librte_eal/include/rte_log.h
+++ b/lib/librte_eal/include/rte_log.h
@@ -240,6 +240,17 @@  int rte_log_register(const char *name);
 __rte_experimental
 int rte_log_register_type_and_pick_level(const char *name, uint32_t level_def);
 
+/**
+ * Dump name of each logtype, one per line.
+ *
+ * @param out
+ *   Stream where the list is sent.
+ * @param prefix
+ *   String preceding each logtype in the output.
+ */
+__rte_experimental
+void rte_log_list_types(FILE *out, const char *prefix);
+
 /**
  * Dump log information.
  *
diff --git a/lib/librte_eal/version.map b/lib/librte_eal/version.map
index 48a2b55d57..c0686ebaf2 100644
--- a/lib/librte_eal/version.map
+++ b/lib/librte_eal/version.map
@@ -415,6 +415,7 @@  EXPERIMENTAL {
 	rte_thread_tls_value_set;
 
 	# added in 21.05
+	rte_log_list_types;
 	rte_version_minor;
 	rte_version_month;
 	rte_version_prefix;