* [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels
@ 2015-11-13 6:47 Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level Matthew Hall
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
This is a simple but very helpful patch for app developers. It allows a wider
range of log levels from in the rte_log framework.
It allows developers to avoid resorting to a lot of external log frameworks in
their DPDK code.
Matthew Hall (6):
librte_log: add function to retrieve log_level
rte_log.h: add detailed log levels RTE_LOG{FINE,FINER,FINEST}
eal_common_log.c: reset default level to RTE_LOG_FINEST
rte_log.h: update logging docs to include FINEST level
rte_log.h: add RTE_SYSLOG_LEVEL_MAX
eal_log.c: limit syslog level to RTE_SYSLOG_LEVEL_MAX
lib/librte_eal/common/eal_common_log.c | 2 +-
lib/librte_eal/common/include/rte_log.h | 31 +++++++++++++++++++------------
lib/librte_eal/linuxapp/eal/eal_log.c | 1 +
3 files changed, 21 insertions(+), 13 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
@ 2015-11-13 6:47 ` Matthew Hall
2015-11-13 11:40 ` Bruce Richardson
2015-11-13 6:47 ` [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST} Matthew Hall
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
lib/librte_eal/common/include/rte_log.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index ede0dca..9dad24e 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -131,6 +131,7 @@ void rte_set_log_level(uint32_t level);
/**
* Get the global log level.
+ *
*/
uint32_t rte_get_log_level(void);
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level Matthew Hall
@ 2015-11-13 6:47 ` Matthew Hall
2015-11-13 11:44 ` Bruce Richardson
2015-11-13 16:07 ` Stephen Hemminger
2015-11-13 6:47 ` [dpdk-dev] [PATCH 3/6] eal_common_log.c: reset default level to RTE_LOG_FINEST Matthew Hall
` (3 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
lib/librte_eal/common/include/rte_log.h | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 9dad24e..7dc19e1 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -90,14 +90,17 @@ extern struct rte_logs rte_logs;
#define RTE_LOGTYPE_USER8 0x80000000 /**< User-defined log type 8. */
/* 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. */
-#define RTE_LOG_CRIT 3U /**< Critical conditions. */
-#define RTE_LOG_ERR 4U /**< Error conditions. */
-#define RTE_LOG_WARNING 5U /**< Warning conditions. */
-#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
-#define RTE_LOG_INFO 7U /**< Informational. */
-#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
+#define RTE_LOG_EMERG 1U /**< System is unusable. */
+#define RTE_LOG_ALERT 2U /**< Action must be taken immediately. */
+#define RTE_LOG_CRIT 3U /**< Critical conditions. */
+#define RTE_LOG_ERR 4U /**< Error conditions. */
+#define RTE_LOG_WARNING 5U /**< Warning conditions. */
+#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
+#define RTE_LOG_INFO 7U /**< Informational. */
+#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
+#define RTE_LOG_FINE 9U /**< Fine-level messages. */
+#define RTE_LOG_FINER 10U /**< Finer-level messages. */
+#define RTE_LOG_FINEST 11U /**< Finest-level messages. */
/** The default log stream. */
extern FILE *eal_default_log_stream;
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 3/6] eal_common_log.c: reset default level to RTE_LOG_FINEST
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST} Matthew Hall
@ 2015-11-13 6:47 ` Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 4/6] rte_log.h: update logging docs to include FINEST level Matthew Hall
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
lib/librte_eal/common/eal_common_log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 1ae8de7..510eeff 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -82,7 +82,7 @@ static struct log_history_list log_history;
/* global log structure */
struct rte_logs rte_logs = {
.type = ~0,
- .level = RTE_LOG_DEBUG,
+ .level = RTE_LOG_FINEST,
.file = NULL,
};
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 4/6] rte_log.h: update logging docs to include FINEST level
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
` (2 preceding siblings ...)
2015-11-13 6:47 ` [dpdk-dev] [PATCH 3/6] eal_common_log.c: reset default level to RTE_LOG_FINEST Matthew Hall
@ 2015-11-13 6:47 ` Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 5/6] rte_log.h: add RTE_SYSLOG_LEVEL_MAX Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 6/6] eal_log.c: limit syslog level to RTE_SYSLOG_LEVEL_MAX Matthew Hall
5 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
lib/librte_eal/common/include/rte_log.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 7dc19e1..4a70ce5 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -128,7 +128,7 @@ int rte_openlog_stream(FILE *f);
* displayed.
*
* @param level
- * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
+ * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_FINEST (11).
*/
void rte_set_log_level(uint32_t level);
@@ -228,7 +228,7 @@ int rte_log_add_in_history(const char *buf, size_t size);
* logs are automatically prefixed by type when using the macro.
*
* @param level
- * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
+ * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_FINEST (11).
* @param logtype
* The log type, for example, RTE_LOGTYPE_EAL.
* @param format
@@ -260,7 +260,7 @@ int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
* removed at compilation time.
*
* @param level
- * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8).
+ * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_FINEST (11).
* @param logtype
* The log type, for example, RTE_LOGTYPE_EAL.
* @param format
@@ -289,7 +289,7 @@ int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
* RTE_LOG(INFO, EAL, "this is a %s", "log");
*
* @param l
- * Log level. A value between EMERG (1) and DEBUG (8). The short name is
+ * Log level. A value between EMERG (1) and FINEST (11). The short name is
* expanded by the macro, so it cannot be an integer value.
* @param t
* The log type, for example, EAL. The short name is expanded by the
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 5/6] rte_log.h: add RTE_SYSLOG_LEVEL_MAX
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
` (3 preceding siblings ...)
2015-11-13 6:47 ` [dpdk-dev] [PATCH 4/6] rte_log.h: update logging docs to include FINEST level Matthew Hall
@ 2015-11-13 6:47 ` Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 6/6] eal_log.c: limit syslog level to RTE_SYSLOG_LEVEL_MAX Matthew Hall
5 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
lib/librte_eal/common/include/rte_log.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 4a70ce5..d7e11f1 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -102,6 +102,9 @@ extern struct rte_logs rte_logs;
#define RTE_LOG_FINER 10U /**< Finer-level messages. */
#define RTE_LOG_FINEST 11U /**< Finest-level messages. */
+/* Syslog only supports 7 (DEBUG) and higher levels. */
+#define RTE_SYSLOG_LEVEL_MAX 7U
+
/** The default log stream. */
extern FILE *eal_default_log_stream;
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH 6/6] eal_log.c: limit syslog level to RTE_SYSLOG_LEVEL_MAX
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
` (4 preceding siblings ...)
2015-11-13 6:47 ` [dpdk-dev] [PATCH 5/6] rte_log.h: add RTE_SYSLOG_LEVEL_MAX Matthew Hall
@ 2015-11-13 6:47 ` Matthew Hall
5 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 6:47 UTC (permalink / raw)
To: dev
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
lib/librte_eal/linuxapp/eal/eal_log.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_log.c b/lib/librte_eal/linuxapp/eal/eal_log.c
index 0b133c3..dbeff75 100644
--- a/lib/librte_eal/linuxapp/eal/eal_log.c
+++ b/lib/librte_eal/linuxapp/eal/eal_log.c
@@ -73,6 +73,7 @@ console_log_write(__attribute__((unused)) void *c, const char *buf, size_t size)
/* Syslog error levels are from 0 to 7, so subtract 1 to convert */
loglevel = rte_log_cur_msg_loglevel() - 1;
+ loglevel = loglevel > RTE_SYSLOG_LEVEL_MAX ? RTE_SYSLOG_LEVEL_MAX : loglevel;
memcpy(copybuf, buf, size);
copybuf[size] = '\0';
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level
2015-11-13 6:47 ` [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level Matthew Hall
@ 2015-11-13 11:40 ` Bruce Richardson
2015-11-13 11:49 ` Thomas Monjalon
2015-11-13 19:01 ` Matthew Hall
0 siblings, 2 replies; 17+ messages in thread
From: Bruce Richardson @ 2015-11-13 11:40 UTC (permalink / raw)
To: Matthew Hall; +Cc: dev
On Fri, Nov 13, 2015 at 06:47:33AM +0000, Matthew Hall wrote:
> Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
> ---
> lib/librte_eal/common/include/rte_log.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index ede0dca..9dad24e 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -131,6 +131,7 @@ void rte_set_log_level(uint32_t level);
>
> /**
> * Get the global log level.
> + *
> */
> uint32_t rte_get_log_level(void);
>
> --
> 1.9.1
>
I don't think this patch is necessary, as all it adds is a single extra line to
a comment.
/Bruce
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 6:47 ` [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST} Matthew Hall
@ 2015-11-13 11:44 ` Bruce Richardson
2015-11-13 11:48 ` Ananyev, Konstantin
2015-11-13 19:16 ` Matthew Hall
2015-11-13 16:07 ` Stephen Hemminger
1 sibling, 2 replies; 17+ messages in thread
From: Bruce Richardson @ 2015-11-13 11:44 UTC (permalink / raw)
To: Matthew Hall; +Cc: dev
On Fri, Nov 13, 2015 at 06:47:34AM +0000, Matthew Hall wrote:
> Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
> ---
> lib/librte_eal/common/include/rte_log.h | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index 9dad24e..7dc19e1 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -90,14 +90,17 @@ extern struct rte_logs rte_logs;
> #define RTE_LOGTYPE_USER8 0x80000000 /**< User-defined log type 8. */
>
> /* 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. */
> -#define RTE_LOG_CRIT 3U /**< Critical conditions. */
> -#define RTE_LOG_ERR 4U /**< Error conditions. */
> -#define RTE_LOG_WARNING 5U /**< Warning conditions. */
> -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
> -#define RTE_LOG_INFO 7U /**< Informational. */
> -#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
> +#define RTE_LOG_EMERG 1U /**< System is unusable. */
> +#define RTE_LOG_ALERT 2U /**< Action must be taken immediately. */
> +#define RTE_LOG_CRIT 3U /**< Critical conditions. */
> +#define RTE_LOG_ERR 4U /**< Error conditions. */
> +#define RTE_LOG_WARNING 5U /**< Warning conditions. */
> +#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
> +#define RTE_LOG_INFO 7U /**< Informational. */
> +#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
> +#define RTE_LOG_FINE 9U /**< Fine-level messages. */
> +#define RTE_LOG_FINER 10U /**< Finer-level messages. */
> +#define RTE_LOG_FINEST 11U /**< Finest-level messages. */
>
> /** The default log stream. */
> extern FILE *eal_default_log_stream;
> --
> 1.9.1
>
Why 11 log levels - it seems an odd number?
Also, not sure about the {fine, finer, finest} names. My thinking would be to
just start numbering them after DEBUG, so RTE_LOG_L9, RTE_LOG_L10 etc., which
would allow us to add on an arbitrary number of extra log levels as needed.
/Bruce
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 11:44 ` Bruce Richardson
@ 2015-11-13 11:48 ` Ananyev, Konstantin
2015-11-13 19:14 ` Matthew Hall
2015-11-13 19:16 ` Matthew Hall
1 sibling, 1 reply; 17+ messages in thread
From: Ananyev, Konstantin @ 2015-11-13 11:48 UTC (permalink / raw)
To: Richardson, Bruce, Matthew Hall; +Cc: dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Friday, November 13, 2015 11:44 AM
> To: Matthew Hall
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
>
> On Fri, Nov 13, 2015 at 06:47:34AM +0000, Matthew Hall wrote:
> > Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
> > ---
> > lib/librte_eal/common/include/rte_log.h | 19 +++++++++++--------
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> > index 9dad24e..7dc19e1 100644
> > --- a/lib/librte_eal/common/include/rte_log.h
> > +++ b/lib/librte_eal/common/include/rte_log.h
> > @@ -90,14 +90,17 @@ extern struct rte_logs rte_logs;
> > #define RTE_LOGTYPE_USER8 0x80000000 /**< User-defined log type 8. */
> >
> > /* 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. */
> > -#define RTE_LOG_CRIT 3U /**< Critical conditions. */
> > -#define RTE_LOG_ERR 4U /**< Error conditions. */
> > -#define RTE_LOG_WARNING 5U /**< Warning conditions. */
> > -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
> > -#define RTE_LOG_INFO 7U /**< Informational. */
> > -#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
> > +#define RTE_LOG_EMERG 1U /**< System is unusable. */
> > +#define RTE_LOG_ALERT 2U /**< Action must be taken immediately. */
> > +#define RTE_LOG_CRIT 3U /**< Critical conditions. */
> > +#define RTE_LOG_ERR 4U /**< Error conditions. */
> > +#define RTE_LOG_WARNING 5U /**< Warning conditions. */
> > +#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
> > +#define RTE_LOG_INFO 7U /**< Informational. */
> > +#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
> > +#define RTE_LOG_FINE 9U /**< Fine-level messages. */
> > +#define RTE_LOG_FINER 10U /**< Finer-level messages. */
> > +#define RTE_LOG_FINEST 11U /**< Finest-level messages. */
> >
> > /** The default log stream. */
> > extern FILE *eal_default_log_stream;
> > --
> > 1.9.1
> >
> Why 11 log levels - it seems an odd number?
> Also, not sure about the {fine, finer, finest} names. My thinking would be to
> just start numbering them after DEBUG, so RTE_LOG_L9, RTE_LOG_L10 etc., which
> would allow us to add on an arbitrary number of extra log levels as needed.
Actually another question: are existing 8 levels not enough?
Konstantin
>
> /Bruce
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level
2015-11-13 11:40 ` Bruce Richardson
@ 2015-11-13 11:49 ` Thomas Monjalon
2015-11-13 19:04 ` Matthew Hall
2015-11-13 19:01 ` Matthew Hall
1 sibling, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2015-11-13 11:49 UTC (permalink / raw)
To: Bruce Richardson, Matthew Hall; +Cc: dev
2015-11-13 11:40, Bruce Richardson:
> On Fri, Nov 13, 2015 at 06:47:33AM +0000, Matthew Hall wrote:
> > --- a/lib/librte_eal/common/include/rte_log.h
> > +++ b/lib/librte_eal/common/include/rte_log.h
> > @@ -131,6 +131,7 @@ void rte_set_log_level(uint32_t level);
> >
> > /**
> > * Get the global log level.
> > + *
> > */
> > uint32_t rte_get_log_level(void);
> >
> I don't think this patch is necessary, as all it adds is a single extra line to
> a comment.
I'm sad for you Bruce: you only see an empty line where you could catch
the beauty of the star ;)
Matthew, obviously you failed your send.
As a general comment, please group patches by logic meaning and
check them before sending ;)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 6:47 ` [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST} Matthew Hall
2015-11-13 11:44 ` Bruce Richardson
@ 2015-11-13 16:07 ` Stephen Hemminger
2015-11-13 19:15 ` Matthew Hall
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2015-11-13 16:07 UTC (permalink / raw)
To: Matthew Hall; +Cc: dev
On Fri, 13 Nov 2015 06:47:34 +0000
Matthew Hall <mhall@mhcomputing.net> wrote:
> Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
> ---
> lib/librte_eal/common/include/rte_log.h | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
> index 9dad24e..7dc19e1 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -90,14 +90,17 @@ extern struct rte_logs rte_logs;
> #define RTE_LOGTYPE_USER8 0x80000000 /**< User-defined log type 8. */
>
> /* 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. */
> -#define RTE_LOG_CRIT 3U /**< Critical conditions. */
> -#define RTE_LOG_ERR 4U /**< Error conditions. */
> -#define RTE_LOG_WARNING 5U /**< Warning conditions. */
> -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
> -#define RTE_LOG_INFO 7U /**< Informational. */
> -#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
> +#define RTE_LOG_EMERG 1U /**< System is unusable. */
> +#define RTE_LOG_ALERT 2U /**< Action must be taken immediately. */
> +#define RTE_LOG_CRIT 3U /**< Critical conditions. */
> +#define RTE_LOG_ERR 4U /**< Error conditions. */
> +#define RTE_LOG_WARNING 5U /**< Warning conditions. */
> +#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */
> +#define RTE_LOG_INFO 7U /**< Informational. */
> +#define RTE_LOG_DEBUG 8U /**< Debug-level messages. */
> +#define RTE_LOG_FINE 9U /**< Fine-level messages. */
> +#define RTE_LOG_FINER 10U /**< Finer-level messages. */
> +#define RTE_LOG_FINEST 11U /**< Finest-level messages. */
>
> /** The default log stream. */
> extern FILE *eal_default_log_stream;
I understand the motivation but the existing levels match syslog
which are what you want for a production application.
The new levels are only for developer logs. I don't think we want all
the developer levels beyond debug in the upstream tree anyway.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level
2015-11-13 11:40 ` Bruce Richardson
2015-11-13 11:49 ` Thomas Monjalon
@ 2015-11-13 19:01 ` Matthew Hall
1 sibling, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 19:01 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev
On Fri, Nov 13, 2015 at 11:40:09AM +0000, Bruce Richardson wrote:
> I don't think this patch is necessary, as all it adds is a single extra line to
> a comment.
>
> /Bruce
This one was previously merged. So indeed we can toss it.
This is what happens when you are restricted to 1 AM coding.
Matthew.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level
2015-11-13 11:49 ` Thomas Monjalon
@ 2015-11-13 19:04 ` Matthew Hall
0 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 19:04 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On Fri, Nov 13, 2015 at 12:49:36PM +0100, Thomas Monjalon wrote:
> I'm sad for you Bruce: you only see an empty line where you could catch
> the beauty of the star ;)
+1
> Matthew, obviously you failed your send.
You might find a more polite way than calling contributions failures. ;)
> As a general comment, please group patches by logic meaning and
> check them before sending ;)
I did. But I'm a community guy and this is not full-time employment, and I am
admittedly quite bad at git email workflow and don't like it much as discussed
in previous threads.
So I did the best I could do but screwed up something minor at approx 1 or 2
AM. It happens. I don't send you guys crazy emails when there is a build
failure upstream either you know. (:
Matthew.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 11:48 ` Ananyev, Konstantin
@ 2015-11-13 19:14 ` Matthew Hall
0 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 19:14 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev
On Fri, Nov 13, 2015 at 11:48:41AM +0000, Ananyev, Konstantin wrote:
> Actually another question: are existing 8 levels not enough?
> Konstantin
Depends who you ask. I was modeling it based upon the following:
https://docs.oracle.com/javase/7/docs/api/java/util/logging/Level.html
The reason I added a previously-merged patch for a new function
rte_get_log_level() and all these extra levels is that it allows me to do
crazy stuff like this:
if (rte_get_log_level() >= RTE_LOG_FINEST) {
rte_hexdump(stderr, "header_bytes", packet->header.bytes, packet->header.header_size);
}
It is extremely valuable to have levels which go below INFO, so you can make
detailed trace code compiled directly into your app. Which can be readily
enabled or disabled by setting the log_level. WAY easier to debug the early
green-field code in your app when it crashes somehow.
Obviously the first thing you guys will say is... "BUT THAT RUNS WAY TOO
SLOW!!!!!" and you might be right if you are processing 100 gbits/sec. But I
am using DPDK for security not for SDN switching or core routing. And my patch
will not introduce any performance change for the guys doing the other stuff
either.
So if this patch is not accepted in some form, I'd be stuck with my DPDK fork
just for this feature, and the pre-existing patches I sent to Bruce & Co.
regarding rte_lpm and rte_lpm6 expansion and so on.
Matthew.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 16:07 ` Stephen Hemminger
@ 2015-11-13 19:15 ` Matthew Hall
0 siblings, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 19:15 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
On Fri, Nov 13, 2015 at 08:07:41AM -0800, Stephen Hemminger wrote:
> I understand the motivation but the existing levels match syslog
> which are what you want for a production application.
>
> The new levels are only for developer logs. I don't think we want all
> the developer levels beyond debug in the upstream tree anyway.
I figured everybody would say this.
So then I have to fork DPDK for a trivial aid to coders which doesn't affect
performance.
This is silly to me.
Matthew.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST}
2015-11-13 11:44 ` Bruce Richardson
2015-11-13 11:48 ` Ananyev, Konstantin
@ 2015-11-13 19:16 ` Matthew Hall
1 sibling, 0 replies; 17+ messages in thread
From: Matthew Hall @ 2015-11-13 19:16 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev
On Fri, Nov 13, 2015 at 11:44:03AM +0000, Bruce Richardson wrote:
> Why 11 log levels - it seems an odd number?
> Also, not sure about the {fine, finer, finest} names. My thinking would be to
> just start numbering them after DEBUG, so RTE_LOG_L9, RTE_LOG_L10 etc., which
> would allow us to add on an arbitrary number of extra log levels as needed.
>
> /Bruce
I based it on this, it worked very well for me in the past.
https://docs.oracle.com/javase/7/docs/api/java/util/logging/Level.html
But happy to name them anything people want and have any number of them people
want. I am not religious about the details, just want the feature.
Matthew.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-11-13 19:16 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-13 6:47 [dpdk-dev] [PATCH 0/6] librte_eal: allow wider range of log levels Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 1/6] librte_log: add function to retrieve log_level Matthew Hall
2015-11-13 11:40 ` Bruce Richardson
2015-11-13 11:49 ` Thomas Monjalon
2015-11-13 19:04 ` Matthew Hall
2015-11-13 19:01 ` Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 2/6] rte_log.h: add detailed log levels RTE_LOG{FINE, FINER, FINEST} Matthew Hall
2015-11-13 11:44 ` Bruce Richardson
2015-11-13 11:48 ` Ananyev, Konstantin
2015-11-13 19:14 ` Matthew Hall
2015-11-13 19:16 ` Matthew Hall
2015-11-13 16:07 ` Stephen Hemminger
2015-11-13 19:15 ` Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 3/6] eal_common_log.c: reset default level to RTE_LOG_FINEST Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 4/6] rte_log.h: update logging docs to include FINEST level Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 5/6] rte_log.h: add RTE_SYSLOG_LEVEL_MAX Matthew Hall
2015-11-13 6:47 ` [dpdk-dev] [PATCH 6/6] eal_log.c: limit syslog level to RTE_SYSLOG_LEVEL_MAX Matthew Hall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).