From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f177.google.com (mail-wr0-f177.google.com [209.85.128.177]) by dpdk.org (Postfix) with ESMTP id AF8572935 for ; Sat, 15 Jul 2017 16:48:47 +0200 (CEST) Received: by mail-wr0-f177.google.com with SMTP id w4so4207936wrb.2 for ; Sat, 15 Jul 2017 07:48:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Vy2P+L5sdUnk9lct6TvpM1f9qqoouI2b97WPOaeAn0s=; b=cNWJTumjH3p1aQ7cfHrcnM3wB9CiB1JqkQINWEuyiLuC+hMGhGTygcPzOpfacoulWo OjMGapQ9zhIj5hHElHgolDPvVxeVws3u8OnbYTCi1bJHVAYgh01G2gZNoA906uKSDdlp tfVOCTi4wCisZ0pngjle8Z8ZE4vttPYSC6ObPlFT4CTDztLprK2eNnPa79AgBX8qBnX7 rC0tXSMLtWh1d0C423zMPDkm8cF3axGIyp/58x9R+2n1VMznRweoTvYGufq73tZlz8Gy JFxB2exCAugFUL/BHcH8moOPxpugmhBicsqjVa5fjGHuseNGng78U/xjRThvQFL4pASc S3Hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Vy2P+L5sdUnk9lct6TvpM1f9qqoouI2b97WPOaeAn0s=; b=Kqb9xqSPWTBoNMJ5Bq5jhIaD/MdZuxxMJ+Xe2Qp0Jkn0r+QxIE8yErjwcmYmSu1PGT 1LuGxW4b+Kgq+3/rUHSh4JZEzBFMMzaLikYZv6fVVGuzzgaiwEuOdEfYtIM0qMcENy0L o5TGxWym1bpfZanvPU/lU7xHbFzgtC3bzDxWlZ94ZA1WdBhPhVjvsu9QZucgYygm4/Sq GpavxwLXBzi/YLwMQTRbWrEapHWN4fY7FgmnvZEtDC9Nf7qPa+vPqt3x5Yw9spCswMLu oqnAY0sx4XibX9pdL2CBqFWPmxWc8/HHou13xweUdQHzmezINPvrUcxsP0BIC4XA8OiS K+Ng== X-Gm-Message-State: AIVw111vLoHCOW4IYkldprIvFOUoTKKmDF3BshhfoUT4mE9F2Bh6m55r 5B92TsVc7S+Le4sLxCY= X-Received: by 10.223.162.156 with SMTP id s28mr6945456wra.2.1500130127001; Sat, 15 Jul 2017 07:48:47 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p99sm10831480wrb.6.2017.07.15.07.48.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 15 Jul 2017 07:48:46 -0700 (PDT) Date: Sat, 15 Jul 2017 16:48:38 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Jan Blunck Cc: dev@dpdk.org Message-ID: <20170715144837.GN11154@bidouze.vm.6wind.com> References: <20170711232512.54641-1-jblunck@infradead.org> <20170714211213.34436-1-jblunck@infradead.org> <20170714211213.34436-14-jblunck@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170714211213.34436-14-jblunck@infradead.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2 13/15] devargs: pass busname argument when parsing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Jul 2017 14:48:47 -0000 On Fri, Jul 14, 2017 at 05:12:11PM -0400, Jan Blunck wrote: > Let the rte_eal_devargs_parse() function explicitly take a "busname" > argument that is validated. > > Now that the busname is known and validated at parse time the validity of > the device name is checked for all device types when they get probed. > What use is there for a parsing API if the fields have already been recognized? Why would someone need to parse an rte_devargs if they already know the busname from the devname? This function becomes useless. You are applying here the same logic you used with the rte_dev API, where you divided the fields because it made no sense to have them merged. However, the rte_dev API is not in contact with users / applications, it is not the interface with the outside world where it must take in external inputs and format them for subsequent systems to use. > Signed-off-by: Jan Blunck > --- > lib/librte_eal/common/eal_common_devargs.c | 72 +++++++++++------------------ > lib/librte_eal/common/include/rte_devargs.h | 17 ++++--- > test/test/test_devargs.c | 21 +++------ > 3 files changed, 45 insertions(+), 65 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c > index 38ba64567..44a8d8562 100644 > --- a/lib/librte_eal/common/eal_common_devargs.c > +++ b/lib/librte_eal/common/eal_common_devargs.c > @@ -77,30 +77,21 @@ rte_eal_parse_devargs_str(const char *devargs_str, > return 0; > } > > -static int > -bus_name_cmp(const struct rte_bus *bus, const void *name) > -{ > - return strncmp(bus->name, name, strlen(bus->name)); > -} > - > int > -rte_eal_devargs_parse(const char *dev, struct rte_devargs *da) > +rte_eal_devargs_parse(const char *busname, const char *dev, > + struct rte_devargs *da) > { > - struct rte_bus *bus = NULL; > char *devname = NULL, *drvargs = NULL; > int ret; > > - if (dev == NULL || da == NULL) > + if (busname == NULL || dev == NULL || da == NULL) > return -EINVAL; > /* Retrieve eventual bus info */ > - do { > - bus = rte_bus_find(bus, bus_name_cmp, dev); > - if (bus == NULL) > - break; > - dev += strlen(bus->name) + 1; > - if (rte_bus_find_by_device_name(dev) == bus) > - break; > - } while (1); > + da->bus = rte_bus_find_by_name(busname); > + if (da->bus == NULL) { > + RTE_LOG(ERR, EAL, "Bus not found: \"%s\"\n", busname); > + return -EFAULT; > + } > > ret = rte_eal_parse_devargs_str(dev, &devname, &drvargs); > if (ret != 0) > @@ -118,21 +109,10 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs *da) > da->args = drvargs; > drvargs = NULL; > > - if (bus == NULL) { > - bus = rte_bus_find_by_device_name(da->name); > - if (bus == NULL) { > - RTE_LOG(ERR, EAL, "Failed to parse device: \"%s\"\n", > - da->name); > - ret = -EFAULT; > - goto fail; > - } > - } > - da->bus = bus; > - > /* Store bus name */ > - ret = snprintf(da->busname, sizeof(da->busname), "%s", bus->name); > + ret = snprintf(da->busname, sizeof(da->busname), "%s", busname); > if (ret < 0 || ret >= (int)sizeof(da->busname)) { > - RTE_LOG(ERR, EAL, "Invalid bus name: \"%s\"\n", bus->name); > + RTE_LOG(ERR, EAL, "Invalid bus name: \"%s\"\n", busname); > ret = -EINVAL; > goto fail; > } > @@ -158,8 +138,7 @@ int > rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) > { > struct rte_devargs *devargs = NULL; > - struct rte_bus *bus = NULL; > - const char *dev = devargs_str; > + const char *busname = NULL; > int ret; > > /* use calloc instead of rte_zmalloc as it's called early at init */ > @@ -167,31 +146,36 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) > if (devargs == NULL) > goto fail; > > - if (rte_eal_devargs_parse(dev, devargs)) > - goto fail; > - devargs->type = devtype; > - bus = devargs->bus; > - if (devtype != RTE_DEVTYPE_VIRTUAL) { > - ret = rte_bus_configure(bus, > + switch (devtype) { > + case RTE_DEVTYPE_WHITELISTED_PCI: > + case RTE_DEVTYPE_BLACKLISTED_PCI: > + busname = "pci"; > + ret = rte_bus_configure(rte_bus_find_by_name(busname), > devtype == RTE_DEVTYPE_WHITELISTED_PCI ? > &BUS_CONF_WHITELIST : &BUS_CONF_BLACKLIST); > if (ret != 0) { > RTE_LOG(ERR, EAL, > "Bus [%s] scan_mode conflicts with devtype: " > - "%s\n", bus->name, devargs_str); > + "%s\n", busname, devargs_str); > goto fail; > } > + break; > + case RTE_DEVTYPE_VIRTUAL: > + busname = "vdev"; > + break; > } > > + if (rte_eal_devargs_parse(busname, devargs_str, devargs)) > + goto fail; > + > + RTE_LOG(DEBUG, EAL, "Adding devargs for device [%s] on bus [%s]: %s\n", > + devargs->name, busname, devargs->args); > + > TAILQ_INSERT_TAIL(&devargs_list, devargs, next); > return 0; > > fail: > - if (devargs) { > - free(devargs->args); > - free(devargs); > - } > - > + free(devargs); > return -1; > } > > diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h > index 8dafb167e..10953327f 100644 > --- a/lib/librte_eal/common/include/rte_devargs.h > +++ b/lib/librte_eal/common/include/rte_devargs.h > @@ -124,10 +124,12 @@ int rte_eal_parse_devargs_str(const char *devargs_str, > /** > * Parse a device string. > * > - * Verify that a bus is capable of handling the device passed > - * in argument. Store which bus will handle the device, its name > - * and the eventual device parameters. > + * The function parses the arguments string to get driver name and driver > + * arguments and stores together with the busname that will handle the device, > + * its name and the eventual device parameters. > * > + * @param busname > + * The bus name the device declaration string applies to. > * @param dev > * The device declaration string. > * @param da > @@ -138,7 +140,7 @@ int rte_eal_parse_devargs_str(const char *devargs_str, > * - Negative errno on error. > */ > int > -rte_eal_devargs_parse(const char *dev, > +rte_eal_devargs_parse(const char *busname, const char *dev, > struct rte_devargs *da); > > /** > @@ -151,9 +153,10 @@ rte_eal_devargs_parse(const char *dev, > * > * For virtual devices, the format of arguments string is "DRIVER_NAME*" > * or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring", > - * "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the > - * driver name is not checked by this function, it is done when probing > - * the drivers. > + * "net_ring0", "net_pmdAnything,arg=0:arg2=1". > + * > + * The validity of the device name is not checked by this function, it is done > + * when probing the drivers. > * > * @param devtype > * The type of the device. > diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c > index 9e0ff5995..84803cf5a 100644 > --- a/test/test/test_devargs.c > +++ b/test/test/test_devargs.c > @@ -110,24 +110,17 @@ test_devargs(void) > goto fail; > free_devargs_list(); > > - /* test error case: bad PCI address */ > - if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:1") == 0) > - goto fail; > - if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "00.1") == 0) > - goto fail; > - if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "foo") == 0) > - goto fail; > - if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, ",") == 0) > - goto fail; > - if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "000f:0:0") == 0) > - goto fail; > - > - if (rte_eal_devargs_parse("", &devargs_stack) == 0) { > + if (rte_eal_devargs_parse("", "", &devargs_stack) == 0) { > printf("Error in rte_eal_devargs_parse()\n"); > goto fail; > } > > - if (rte_eal_devargs_parse("08:00.1,foo=bar", &devargs_stack) != 0) { > + if (rte_eal_devargs_parse("vdev", "not_found", &devargs_stack) != 0) { > + printf("Error in rte_eal_devargs_parse(vdev:not_found)\n"); > + goto fail; > + } > + if (rte_eal_devargs_parse("pci", "08:00.1,foo=bar", > + &devargs_stack) != 0) { > printf("Error in rte_eal_devargs_parse(08:00.1,foo=bar)\n"); > goto fail; > } > -- > 2.13.2 > -- Gaëtan Rivet 6WIND