From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 7D8CE2A9 for ; Tue, 25 Nov 2014 11:31:52 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 25 Nov 2014 02:42:43 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,455,1413270000"; d="scan'208";a="637714145" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.15]) by fmsmga002.fm.intel.com with SMTP; 25 Nov 2014 02:42:41 -0800 Received: by (sSMTP sendmail emulation); Tue, 25 Nov 2014 10:42:40 +0025 Date: Tue, 25 Nov 2014 10:42:40 +0000 From: Bruce Richardson To: Thomas Monjalon Message-ID: <20141125104240.GE5260@bricha3-MOBL3> References: <1416692622-28886-1-git-send-email-thomas.monjalon@6wind.com> <1416692622-28886-6-git-send-email-thomas.monjalon@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1416692622-28886-6-git-send-email-thomas.monjalon@6wind.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 05/10] eal: factorize options sanity check X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Nov 2014 10:31:53 -0000 On Sat, Nov 22, 2014 at 10:43:37PM +0100, Thomas Monjalon wrote: > No need to have duplicated check for common options. > > Some flags are set for options -c and -m in order to simplify the > checks. > > Signed-off-by: Thomas Monjalon Acked-by: Bruce Richardson > --- > lib/librte_eal/bsdapp/eal/eal.c | 54 ++------------------------ > lib/librte_eal/common/eal_common_options.c | 53 ++++++++++++++++++++++++++ > lib/librte_eal/common/eal_options.h | 1 + > lib/librte_eal/linuxapp/eal/eal.c | 61 ++++-------------------------- > 4 files changed, 66 insertions(+), 103 deletions(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c > index 391ce53..20a9c5f 100644 > --- a/lib/librte_eal/bsdapp/eal/eal.c > +++ b/lib/librte_eal/bsdapp/eal/eal.c > @@ -316,7 +316,6 @@ eal_parse_args(int argc, char **argv) > int opt, ret, i; > char **argvopt; > int option_index; > - int coremask_ok = 0; > char *prgname = argv[0]; > > argvopt = argv; > @@ -339,13 +338,8 @@ eal_parse_args(int argc, char **argv) > return -1; > } > /* common parser handled this option */ > - if (ret == 0) { > - /* special case, note that the common parser accepted > - * the coremask option */ > - if (opt == 'c') > - coremask_ok = 1; > + if (ret == 0) > continue; > - } > > switch (opt) { > default: > @@ -366,51 +360,11 @@ eal_parse_args(int argc, char **argv) > } > } > > - /* sanity checks */ > - if (!coremask_ok) { > - RTE_LOG(ERR, EAL, "coremask not specified\n"); > - eal_usage(prgname); > - return -1; > - } > - if (internal_config.process_type == RTE_PROC_AUTO){ > + if (internal_config.process_type == RTE_PROC_AUTO) > internal_config.process_type = eal_proc_type_detect(); > - } > - if (internal_config.process_type == RTE_PROC_INVALID){ > - RTE_LOG(ERR, EAL, "Invalid process type specified\n"); > - eal_usage(prgname); > - return -1; > - } > - if (internal_config.process_type == RTE_PROC_PRIMARY && > - internal_config.force_nchannel == 0) { > - RTE_LOG(ERR, EAL, "Number of memory channels (-n) not specified\n"); > - eal_usage(prgname); > - return -1; > - } > - if (index(internal_config.hugefile_prefix,'%') != NULL){ > - RTE_LOG(ERR, EAL, "Invalid char, '%%', in '"OPT_FILE_PREFIX"' option\n"); > - eal_usage(prgname); > - return -1; > - } > - if (internal_config.memory > 0 && internal_config.force_sockets == 1) { > - RTE_LOG(ERR, EAL, "Options -m and --socket-mem cannot be specified " > - "at the same time\n"); > - eal_usage(prgname); > - return -1; > - } > - /* --no-huge doesn't make sense with either -m or --socket-mem */ > - if (internal_config.no_hugetlbfs && > - (internal_config.memory > 0 || > - internal_config.force_sockets == 1)) { > - RTE_LOG(ERR, EAL, "Options -m or --socket-mem cannot be specified " > - "together with --no-huge!\n"); > - eal_usage(prgname); > - return -1; > - } > > - if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 && > - rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) { > - RTE_LOG(ERR, EAL, "Error: blacklist [-b] and whitelist " > - "[-w] options cannot be used at the same time\n"); > + /* sanity checks */ > + if (eal_check_common_options(&internal_config) != 0) { > eal_usage(prgname); > return -1; > } > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > index ffc615a..630dfe0 100644 > --- a/lib/librte_eal/common/eal_common_options.c > +++ b/lib/librte_eal/common/eal_common_options.c > @@ -85,6 +85,9 @@ eal_long_options[] = { > {0, 0, 0, 0} > }; > > +static int lcores_parsed; > +static int mem_parsed; > + > void > eal_reset_internal_config(struct internal_config *internal_cfg) > { > @@ -197,6 +200,7 @@ eal_parse_coremask(const char *coremask) > return -1; > /* Update the count of enabled logical cores of the EAL configuration */ > cfg->lcore_count = count; > + lcores_parsed = 1; > return 0; > } > > @@ -305,6 +309,7 @@ eal_parse_common_option(int opt, const char *optarg, > conf->memory = atoi(optarg); > conf->memory *= 1024ULL; > conf->memory *= 1024ULL; > + mem_parsed = 1; > break; > /* force number of channels */ > case 'n': > @@ -394,6 +399,54 @@ eal_parse_common_option(int opt, const char *optarg, > return 0; > } > > +int > +eal_check_common_options(struct internal_config *internal_cfg) > +{ > + struct rte_config *cfg = rte_eal_get_configuration(); > + > + if (!lcores_parsed) { > + RTE_LOG(ERR, EAL, "CPU cores must be enabled with option " > + "-c\n"); > + return -1; > + } > + > + if (internal_cfg->process_type == RTE_PROC_INVALID) { > + RTE_LOG(ERR, EAL, "Invalid process type specified\n"); > + return -1; > + } > + if (internal_cfg->process_type == RTE_PROC_PRIMARY && > + internal_cfg->force_nchannel == 0) { > + RTE_LOG(ERR, EAL, "Number of memory channels (-n) not " > + "specified\n"); > + return -1; > + } > + if (index(internal_cfg->hugefile_prefix, '%') != NULL) { > + RTE_LOG(ERR, EAL, "Invalid char, '%%', in --"OPT_FILE_PREFIX" " > + "option\n"); > + return -1; > + } > + if (mem_parsed && internal_cfg->force_sockets == 1) { > + RTE_LOG(ERR, EAL, "Options -m and --"OPT_SOCKET_MEM" cannot " > + "be specified at the same time\n"); > + return -1; > + } > + if (internal_cfg->no_hugetlbfs && > + (mem_parsed || internal_cfg->force_sockets == 1)) { > + RTE_LOG(ERR, EAL, "Options -m or --"OPT_SOCKET_MEM" cannot " > + "be specified together with --"OPT_NO_HUGE"\n"); > + return -1; > + } > + > + if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 && > + rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) { > + RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) " > + "cannot be used at the same time\n"); > + return -1; > + } > + > + return 0; > +} > + > void > eal_common_usage(void) > { > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h > index 7a08507..75351c0 100644 > --- a/lib/librte_eal/common/eal_options.h > +++ b/lib/librte_eal/common/eal_options.h > @@ -83,6 +83,7 @@ extern const struct option eal_long_options[]; > > int eal_parse_common_option(int opt, const char *argv, > struct internal_config *conf); > +int eal_check_common_options(struct internal_config *internal_cfg); > void eal_common_usage(void); > > #endif /* EAL_OPTIONS_H */ > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c > index bb35669..f5de277 100644 > --- a/lib/librte_eal/linuxapp/eal/eal.c > +++ b/lib/librte_eal/linuxapp/eal/eal.c > @@ -507,7 +507,6 @@ eal_parse_args(int argc, char **argv) > int opt, ret, i; > char **argvopt; > int option_index; > - int coremask_ok = 0; > char *prgname = argv[0]; > struct shared_driver *solib; > > @@ -531,13 +530,8 @@ eal_parse_args(int argc, char **argv) > return -1; > } > /* common parser handled this option */ > - if (ret == 0) { > - /* special case, note that the common parser accepted > - * the coremask option */ > - if (opt == 'c') > - coremask_ok = 1; > + if (ret == 0) > continue; > - } > > switch (opt) { > /* force loading of external driver */ > @@ -622,58 +616,19 @@ eal_parse_args(int argc, char **argv) > } > } > > - /* sanity checks */ > - if (!coremask_ok) { > - RTE_LOG(ERR, EAL, "coremask not specified\n"); > - eal_usage(prgname); > - return -1; > - } > - if (internal_config.process_type == RTE_PROC_AUTO){ > + if (internal_config.process_type == RTE_PROC_AUTO) > internal_config.process_type = eal_proc_type_detect(); > - } > - if (internal_config.process_type == RTE_PROC_INVALID){ > - RTE_LOG(ERR, EAL, "Invalid process type specified\n"); > - eal_usage(prgname); > - return -1; > - } > - if (internal_config.process_type == RTE_PROC_PRIMARY && > - internal_config.force_nchannel == 0) { > - RTE_LOG(ERR, EAL, "Number of memory channels (-n) not specified\n"); > - eal_usage(prgname); > - return -1; > - } > - if (index(internal_config.hugefile_prefix,'%') != NULL){ > - RTE_LOG(ERR, EAL, "Invalid char, '%%', in '"OPT_FILE_PREFIX"' option\n"); > - eal_usage(prgname); > - return -1; > - } > - if (internal_config.memory > 0 && internal_config.force_sockets == 1) { > - RTE_LOG(ERR, EAL, "Options -m and --socket-mem cannot be specified " > - "at the same time\n"); > - eal_usage(prgname); > - return -1; > - } > - /* --no-huge doesn't make sense with either -m or --socket-mem */ > - if (internal_config.no_hugetlbfs && > - (internal_config.memory > 0 || > - internal_config.force_sockets == 1)) { > - RTE_LOG(ERR, EAL, "Options -m or --socket-mem cannot be specified " > - "together with --no-huge!\n"); > + > + /* sanity checks */ > + if (eal_check_common_options(&internal_config) != 0) { > eal_usage(prgname); > return -1; > } > + > /* --xen-dom0 doesn't make sense with --socket-mem */ > if (internal_config.xen_dom0_support && internal_config.force_sockets == 1) { > - RTE_LOG(ERR, EAL, "Options --socket-mem cannot be specified " > - "together with --xen_dom0!\n"); > - eal_usage(prgname); > - return -1; > - } > - > - if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 && > - rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) { > - RTE_LOG(ERR, EAL, "Error: blacklist [-b] and whitelist " > - "[-w] options cannot be used at the same time\n"); > + RTE_LOG(ERR, EAL, "Options --"OPT_SOCKET_MEM" cannot be specified " > + "together with --"OPT_XEN_DOM0"\n"); > eal_usage(prgname); > return -1; > } > -- > 2.1.3 >