From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH v3 4/8] eal: change specific log levels at startup Date: Tue, 18 Apr 2017 10:50:47 +0200 Message-ID: <20170418105047.266cd8a0@glumotte.dev.6wind.com> References: <20170329155323.4760-1-olivier.matz@6wind.com> <20170404164040.24132-1-olivier.matz@6wind.com> <20170404164040.24132-5-olivier.matz@6wind.com> <88ff6283-4b3f-f3a7-0d44-9be8fe5441c1@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, david.marchand@6wind.com, bruce.richardson@intel.com, thomas.monjalon@6wind.com, keith.wiles@intel.com, stephen@networkplumber.org, "De Lara Guarch, Pablo" To: "Tan, Jianfeng" Return-path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id EE759DE0 for ; Tue, 18 Apr 2017 10:50:20 +0200 (CEST) Received: by mail-wm0-f49.google.com with SMTP id o81so50307638wmb.1 for ; Tue, 18 Apr 2017 01:50:20 -0700 (PDT) In-Reply-To: <88ff6283-4b3f-f3a7-0d44-9be8fe5441c1@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jianfeng, On Fri, 14 Apr 2017 13:33:49 +0800, "Tan, Jianfeng" 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 > > --- > > 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"= Set global log level\n" > > + " --"OPT_LOG_LEVEL"=,\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."); >