From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH v2] eal: read and parse device option separately Date: Wed, 2 Aug 2017 17:23:17 +0200 Message-ID: <20170802152317.GC8124@bidouze.vm.6wind.com> References: <12b23f229a5a3b953c1c6063e5a381800f4ccdb9.1501611129.git.gaetan.rivet@6wind.com> <3126300.2FJSgUKTSm@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, Dirk-Holger Lenz To: Thomas Monjalon Return-path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id E73F29183 for ; Wed, 2 Aug 2017 17:23:27 +0200 (CEST) Received: by mail-wm0-f54.google.com with SMTP id t201so44559995wmt.1 for ; Wed, 02 Aug 2017 08:23:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <3126300.2FJSgUKTSm@xps> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Aug 02, 2017 at 04:29:13PM +0200, Thomas Monjalon wrote: > Hi Gaetan > > 02/08/2017 10:56, Gaetan Rivet: > > When the EAL parses the common options given to the application, > > not all subsystems are available. Some device drivers are registered > > afterward upon dynamic plugin loading. > > > > Devices using those drivers are thus unable to be parsed by any drivers > > and are rejected. > > > > Store the device options first and keep them for later processing. > > Parse these right before initializing the buses, the drivers must have > > been stabilized at this point. > > > > Signed-off-by: Gaetan Rivet > > --- > > > > v2: > > > > - Fix the -w / -b incompatibility check. > > It was checking that no two incompatible rte_devargs were inserted. > > As rte_devargs are now generated right before buses scan, this check > > was always correct, even when it should have failed. > > You are doing this check in eal_check_common_options(). > I think you can do the check earlier in eal_parse_common_option() > with 2 static variables, instead of implementing the > new function eal_option_device_type_count(). > Okay, will simplify. > > +static unsigned int > > +eal_option_device_type_count(enum rte_devtype type) > [...] > > @@ -944,14 +1013,14 @@ eal_parse_common_option(int opt, const char *optarg, > > switch (opt) { > > /* blacklist */ > > case 'b': > > - if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI, > > + if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI, > > optarg) < 0) { > > return -1; > > } > > break; > > /* whitelist */ > > case 'w': > > - if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, > > + if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI, > > optarg) < 0) { > > return -1; > > } > > @@ -1061,7 +1130,7 @@ eal_parse_common_option(int opt, const char *optarg, > > break; > > > > case OPT_VDEV_NUM: > > - if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, > > + if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL, > > optarg) < 0) { > > return -1; > > } > > @@ -1187,8 +1256,8 @@ eal_check_common_options(struct internal_config *internal_cfg) > > return -1; > > } > > > > - if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 && > > - rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) { > > + if (eal_option_device_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 && > > + eal_option_device_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; > > [...] > > +static int > > +eal_option_device_add(enum rte_devtype type, const char *optarg) > > +{ > > + struct device_option *deo; > > + size_t optlen; > > Just a last comment about variable name: > Instead of deo, opt or option would be more meaningful. > Will do. > Thanks for the important fix! -- Gaëtan Rivet 6WIND