[dpdk-dev] [PATCH 1/6] eal: fix default log level

[dpdk-dev] [PATCH 1/6] eal: fix default log level

[Olivier Matz]

The initialization of the default log level (from configuration) was
removed by mistake in a previous commit. The global log level was
wrongly set to debug when no --log-level argument was passed. Restore
this initialization.

Before:
  $ ./build/app/test
  RTE>>dump_log_types
  global log level is debug
  ...

After:
  $ ./build/app/test
  RTE>>dump_log_types
  global log level is info
  ...

Fixes: 845afe51e428 ("eal: change specific log levels at startup")

Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c   | 1 +
 lib/librte_eal/linuxapp/eal/eal.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 3b66da6cd..db154db9a 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -324,6 +324,7 @@ eal_log_level_parse(int argc, char **argv)
 	optreset = 1;
 
 	eal_reset_internal_config(&internal_config);
+	rte_log_set_global_level(internal_config.log_level);
 
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
 				  eal_long_options, &option_index)) != EOF) {
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index f3fbeb9c7..b2758799c 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -493,6 +493,7 @@ eal_log_level_parse(int argc, char **argv)
 	optind = 1;
 
 	eal_reset_internal_config(&internal_config);
+	rte_log_set_global_level(internal_config.log_level);
 
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
 				  eal_long_options, &option_index)) != EOF) {
-- 
2.11.0

[dpdk-dev] [PATCH 2/6] eal: move internal config initialization

[Olivier Matz]

It's better to initialize the internal config in rte_eal_init()
instead of eal_log_level_parse(), since this structure is not only
about logs.

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

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index db154db9a..ed16c2e58 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -323,7 +323,6 @@ eal_log_level_parse(int argc, char **argv)
 	optind = 1;
 	optreset = 1;
 
-	eal_reset_internal_config(&internal_config);
 	rte_log_set_global_level(internal_config.log_level);
 
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
@@ -520,6 +519,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
+	eal_reset_internal_config(&internal_config);
+
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index b2758799c..fbfbb94ba 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -492,7 +492,6 @@ eal_log_level_parse(int argc, char **argv)
 	argvopt = argv;
 	optind = 1;
 
-	eal_reset_internal_config(&internal_config);
 	rte_log_set_global_level(internal_config.log_level);
 
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
@@ -777,6 +776,8 @@ rte_eal_init(int argc, char **argv)
 
 	thread_id = pthread_self();
 
+	eal_reset_internal_config(&internal_config);
+
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
-- 
2.11.0

[dpdk-dev] [PATCH 3/6] eal: remove log level from internal config

[Olivier Matz]

This field is only used in the initialization phase. Remove it since the
global log level can also be retrieved using a public API:
rte_log_get_global_level().

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            |  2 --
 lib/librte_eal/common/eal_common_log.c     |  6 ++++++
 lib/librte_eal/common/eal_common_options.c | 11 ++---------
 lib/librte_eal/common/eal_internal_cfg.h   |  1 -
 lib/librte_eal/linuxapp/eal/eal.c          |  2 --
 5 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index ed16c2e58..05f0c1f90 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -323,8 +323,6 @@ eal_log_level_parse(int argc, char **argv)
 	optind = 1;
 	optreset = 1;
 
-	rte_log_set_global_level(internal_config.log_level);
-
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
 				  eal_long_options, &option_index)) != EOF) {
 
diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index dd4d30ca7..7d13cc026 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
 static void
 rte_log_init(void)
 {
+#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
+	rte_log_set_global_level(RTE_LOG_INFO);
+#else
+	rte_log_set_global_level(RTE_LOG_LEVEL);
+#endif
+
 	rte_logs.dynamic_types = calloc(RTE_LOGTYPE_FIRST_EXT_ID,
 		sizeof(struct rte_log_dynamic_type));
 	if (rte_logs.dynamic_types == NULL)
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 32df2ef45..e9b45c49a 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -148,12 +148,6 @@ eal_reset_internal_config(struct internal_config *internal_cfg)
 	internal_cfg->base_virtaddr = 0;
 
 	internal_cfg->syslog_facility = LOG_DAEMON;
-	/* default value from build option */
-#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
-	internal_cfg->log_level = RTE_LOG_INFO;
-#else
-	internal_cfg->log_level = RTE_LOG_LEVEL;
-#endif
 
 	internal_cfg->xen_dom0_support = 0;
 
@@ -739,7 +733,7 @@ eal_parse_syslog(const char *facility, struct internal_config *conf)
 }
 
 static int
-eal_parse_log_level(const char *arg, struct internal_config *conf)
+eal_parse_log_level(const char *arg)
 {
 	char *end, *str, *type, *level;
 	unsigned long tmp;
@@ -772,7 +766,6 @@ eal_parse_log_level(const char *arg, struct internal_config *conf)
 		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",
@@ -926,7 +919,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_LOG_LEVEL_NUM: {
-		if (eal_parse_log_level(optarg, conf) < 0) {
+		if (eal_parse_log_level(optarg) < 0) {
 			RTE_LOG(ERR, EAL,
 				"invalid parameters for --"
 				OPT_LOG_LEVEL "\n");
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 5f1367eb7..7b7e8c887 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -78,7 +78,6 @@ struct internal_config {
 	volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
 	uintptr_t base_virtaddr;          /**< base address to try and reserve memory from */
 	volatile int syslog_facility;	  /**< facility passed to openlog() */
-	volatile uint32_t log_level;	  /**< default log level */
 	/** default interrupt mode for VFIO */
 	volatile enum rte_intr_mode vfio_intr_mode;
 	const char *hugefile_prefix;      /**< the base filename of hugetlbfs files */
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index fbfbb94ba..7c78f2dc2 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -492,8 +492,6 @@ eal_log_level_parse(int argc, char **argv)
 	argvopt = argv;
 	optind = 1;
 
-	rte_log_set_global_level(internal_config.log_level);
-
 	while ((opt = getopt_long(argc, argvopt, eal_short_options,
 				  eal_long_options, &option_index)) != EOF) {
 
-- 
2.11.0

Re: [dpdk-dev] [PATCH 3/6] eal: remove log level from internal config

[Ferruh Yigit]

On 4/18/2017 3:22 PM, Olivier Matz wrote:
> This field is only used in the initialization phase. Remove it since the
> global log level can also be retrieved using a public API:
> rte_log_get_global_level().
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

<...>

> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index dd4d30ca7..7d13cc026 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
>  static void
>  rte_log_init(void)
>  {
> +#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG

Why ">=" (I aware previous one is also like this :), setting global
config option to RTE_LOG_DEBUG cause a INFO level log...

> +	rte_log_set_global_level(RTE_LOG_INFO);
> +#else
> +	rte_log_set_global_level(RTE_LOG_LEVEL);
> +#endif

Re: [dpdk-dev] [PATCH 3/6] eal: remove log level from internal config

[Olivier MATZ]

Hi Ferruh,

On Tue, 18 Apr 2017 16:00:45 +0100, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 4/18/2017 3:22 PM, Olivier Matz wrote:
> > This field is only used in the initialization phase. Remove it since the
> > global log level can also be retrieved using a public API:
> > rte_log_get_global_level().
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>  
> 
> <...>
> 
> > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > index dd4d30ca7..7d13cc026 100644
> > --- a/lib/librte_eal/common/eal_common_log.c
> > +++ b/lib/librte_eal/common/eal_common_log.c
> > @@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
> >  static void
> >  rte_log_init(void)
> >  {
> > +#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG  
> 
> Why ">=" (I aware previous one is also like this :), setting global
> config option to RTE_LOG_DEBUG cause a INFO level log...
> 
> > +	rte_log_set_global_level(RTE_LOG_INFO);
> > +#else
> > +	rte_log_set_global_level(RTE_LOG_LEVEL);
> > +#endif  
> 

From what I see, in the previous commit 9b9d7caa8414 ("log: increase default
level to info"), some code was added to prevent to set the default log
level to DEBUG, because it has an impact on performance.
    
But since commit 5d8f0baf69ea ("log: do not drop debug logs at compile time"),
the logs that impact performance should use RTE_LOG_DP().

I think this can be removed, and changed to:

 rte_log_set_global_level(RTE_LOG_LEVEL);


I did not do it in this patchset because I think it can wait next
version.

Re: [dpdk-dev] [PATCH 3/6] eal: remove log level from internal config

[Ferruh Yigit]

On 4/18/2017 4:26 PM, Olivier MATZ wrote:
> Hi Ferruh,
> 
> On Tue, 18 Apr 2017 16:00:45 +0100, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> On 4/18/2017 3:22 PM, Olivier Matz wrote:
>>> This field is only used in the initialization phase. Remove it since the
>>> global log level can also be retrieved using a public API:
>>> rte_log_get_global_level().
>>>
>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>  
>>
>> <...>
>>
>>> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
>>> index dd4d30ca7..7d13cc026 100644
>>> --- a/lib/librte_eal/common/eal_common_log.c
>>> +++ b/lib/librte_eal/common/eal_common_log.c
>>> @@ -244,6 +244,12 @@ RTE_INIT(rte_log_init);
>>>  static void
>>>  rte_log_init(void)
>>>  {
>>> +#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG  
>>
>> Why ">=" (I aware previous one is also like this :), setting global
>> config option to RTE_LOG_DEBUG cause a INFO level log...
>>
>>> +	rte_log_set_global_level(RTE_LOG_INFO);
>>> +#else
>>> +	rte_log_set_global_level(RTE_LOG_LEVEL);
>>> +#endif  
>>
> 
> From what I see, in the previous commit 9b9d7caa8414 ("log: increase default
> level to info"), some code was added to prevent to set the default log
> level to DEBUG, because it has an impact on performance.
>     
> But since commit 5d8f0baf69ea ("log: do not drop debug logs at compile time"),
> the logs that impact performance should use RTE_LOG_DP().
> 
> I think this can be removed, and changed to:
> 
>  rte_log_set_global_level(RTE_LOG_LEVEL);
> 
> 
> I did not do it in this patchset because I think it can wait next
> version.
> 

Ok, thanks for clarification.

[dpdk-dev] [PATCH 4/6] eal: remove undue printf

[Olivier Matz]

Remove the printf displaying the log level at initialization. It
was introduced for debug purpose, but was not intended to be pushed.

Fixes: 845afe51e428 ("eal: change specific log levels at startup")

Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_options.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index e9b45c49a..f470195f3 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -762,9 +762,6 @@ eal_parse_log_level(const char *arg)
 	if (tmp >= UINT32_MAX)
 		goto fail;
 
-	printf("set log level %s,%lu\n",
-		type, tmp);
-
 	if (type == NULL) {
 		rte_log_set_global_level(tmp);
 	} else if (rte_log_set_level_regexp(type, tmp) < 0) {
-- 
2.11.0

[dpdk-dev] [PATCH 5/6] eal: fix log level regexp matching

[Olivier Matz]

Fix misuse of regular expression functions, which was producing a
segfault.

After the patch, it works properly:

  $ ./build/app/test --no-huge --log-level=pmd,3
  RTE>>dump_log_types
  [...]
  id 30: user7, level is debug
  id 31: user8, level is debug
  id 32: pmd.i40e.init, level is critical
  id 33: pmd.i40e.driver, level is critical

Fixes: a5279180f510 ("eal: change several log levels matching a regexp")
Coverity issue: 143472

Reported-by: Jianfeng Tan <jianfeng.tan@intel.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_log.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 7d13cc026..c2957d167 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -159,10 +159,14 @@ rte_log_set_level_regexp(const char *pattern, uint32_t level)
 	if (level > RTE_LOG_DEBUG)
 		return -1;
 
+	if (regcomp(&r, pattern, 0) != 0)
+		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)
+		if (regexec(&r, rte_logs.dynamic_types[i].name, 0,
+				NULL, 0) == 0)
 			rte_logs.dynamic_types[i].loglevel = level;
 	}
 
-- 
2.11.0

[dpdk-dev] [PATCH 6/6] eal: fix dump of registered logs when disabled

[Olivier Matz]

When we pass --log-level=0, it disables the logs. This level is
not displayed properly by the function that dumps the registered log
types (it shows "unknown"). Show "disabled" instead.

Before:

  ./build/app/test --log-level=0
  RTE>>dump_log_types
  global log level is unknown
  ...

After:

  ./build/app/test --log-level=0
  RTE>>dump_log_types
  global log level is disabled
  ...

Fixes: 432050bfd05b ("eal: dump registered log types")

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eal/common/eal_common_log.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index c2957d167..164078c74 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -294,6 +294,7 @@ static const char *
 loglevel_to_string(uint32_t level)
 {
 	switch (level) {
+	case 0: return "disabled";
 	case RTE_LOG_EMERG: return "emerg";
 	case RTE_LOG_ALERT: return "alert";
 	case RTE_LOG_CRIT: return "critical";
-- 
2.11.0

Re: [dpdk-dev] [PATCH 1/6] eal: fix default log level

[Ferruh Yigit]

On 4/18/2017 3:22 PM, Olivier Matz wrote:
> The initialization of the default log level (from configuration) was
> removed by mistake in a previous commit. The global log level was
> wrongly set to debug when no --log-level argument was passed. Restore
> this initialization.
> 
> Before:
>   $ ./build/app/test
>   RTE>>dump_log_types
>   global log level is debug
>   ...
> 
> After:
>   $ ./build/app/test
>   RTE>>dump_log_types
>   global log level is info
>   ...
> 
> Fixes: 845afe51e428 ("eal: change specific log levels at startup")
> 
> Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Series Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH 1/6] eal: fix default log level

[Thomas Monjalon]

18/04/2017 17:29, Ferruh Yigit:
> On 4/18/2017 3:22 PM, Olivier Matz wrote:
> > The initialization of the default log level (from configuration) was
> > removed by mistake in a previous commit. The global log level was
> > wrongly set to debug when no --log-level argument was passed. Restore
> > this initialization.
> > 
> > Before:
> >   $ ./build/app/test
> >   RTE>>dump_log_types
> >   global log level is debug
> >   ...
> > 
> > After:
> >   $ ./build/app/test
> >   RTE>>dump_log_types
> >   global log level is info
> >   ...
> > 
> > Fixes: 845afe51e428 ("eal: change specific log levels at startup")
> > 
> > Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Series Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Series Applied, thanks