From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <gaetan.rivet@6wind.com>
Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47])
by dpdk.org (Postfix) with ESMTP id 6ED7229D9
for <dev@dpdk.org>; Thu, 13 Jul 2017 15:40:12 +0200 (CEST)
Received: by mail-wm0-f47.google.com with SMTP id 62so23997820wmw.1
for <dev@dpdk.org>; Thu, 13 Jul 2017 06:40:12 -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=5dVarlwohgsVQzheyBBLP9IWSCwp3CCN2jIen+W6kVg=;
b=tsVem3MK4G45B+sp3gI5rw/VxRS7u/dBVDasKt4RIiyYPn7RCZ4YwKQ5JzIRLwpFb+
CYxor3+SL9B6JFX5B9eOzFHcq0xKWGsO6T+DCWb9o6jyjGsxccXgEsfw7ZGV5xTrn5M9
W8g9VGaeQ1vTMnPknpxDwEaTHEBH/Aq2qVpNyMAsGR8Teimj5I8pVB7hwFHRtnwywkFJ
elXJZmA15GBnMyNi+Zp8TKXfw+WurcIo8MZ9A5KLz088nPo9r/SSBPNi9pQcxbL/jzyq
jW2VDqtRYz2hse4ydMGen/UOMy5NDCSeaf+UerkcGPrrLpdDx13vVLuCBYy/aDz34jm4
RVyg==
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=5dVarlwohgsVQzheyBBLP9IWSCwp3CCN2jIen+W6kVg=;
b=U9MGB3owsaZO6fDEt7A6uait/HHEInKj752C3OXC9LNya9QQDuVb2sj136WnTMr2xL
nCWuDnCmbmFJOHzA4xQlyjIWCtjwjqXr7fj4tb4FxKFrD2l9nYeewAo0rmHKKuc4nWfZ
hbnk8kkF3i4YokOC+tJ4WqNwxFDuPfGDqo0vfYJO42G97fK3umnnqHOgKgD/yzo9okg7
XUnnp3EKnV2gzIdYcOohaJ/KnRogaspPk7Kjd4wRHrC1TEmxOCuyJLVkfS6aXzUqPdwd
L0nHn9cfcUCbdGTIeBXvrCh1F60AGWG+mX6bsYSoMRIQlHVFeLxfyJV6yznIQs4yIQqh
Io7w==
X-Gm-Message-State: AIVw1125nyjkwOUeNfKL7BTqdAVNIUcdMCYDwwAqYhF053TdkyN8f2Hx
51NM4E9x7ehBvf8/
X-Received: by 10.28.208.69 with SMTP id h66mr2236822wmg.66.1499953211981;
Thu, 13 Jul 2017 06:40:11 -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 31sm4055164wrd.20.2017.07.13.06.40.10
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Thu, 13 Jul 2017 06:40:11 -0700 (PDT)
Date: Thu, 13 Jul 2017 15:40:03 +0200
From: =?iso-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
To: Jan Blunck <jblunck@infradead.org>
Cc: dev@dpdk.org
Message-ID: <20170713134003.GK11154@bidouze.vm.6wind.com>
References: <20170711232512.54641-1-jblunck@infradead.org>
<20170711232512.54641-10-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: <20170711232512.54641-10-jblunck@infradead.org>
User-Agent: Mutt/1.5.23 (2014-03-12)
Subject: Re: [dpdk-dev] [PATCH 09/13] devargs: parse "bus=" argument
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 13 Jul 2017 13:40:12 -0000
I still disagree with parsing the body of devargs to retrieve bus
information. They should not be mixed.
This conceptual discrepancy can be seen plainly in the code structure:
you emulate librte_kvargs because you do not want to depend on it within
the EAL, but copy its behavior and are forced to rewrite it partially.
Can you justify this? The commitlog does not describe the problem being
solved.
I think there should be a discussion about the future format of device
declarations. One version has been integrated in RC1 but it will be reworked
anyway. I'd like to hear more opinions.
On Tue, Jul 11, 2017 at 07:25:08PM -0400, Jan Blunck wrote:
> Let the rte_eal_devargs_parse() function parse the "bus=" argument.
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> ---
> lib/librte_eal/common/eal_common_devargs.c | 131 ++++++++++++++++++-----------
> test/test/test_devargs.c | 19 +++++
> 2 files changed, 102 insertions(+), 48 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index 2bdee9a30..a0d0e6e91 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -77,6 +77,40 @@ rte_eal_parse_devargs_str(const char *devargs_str,
> return 0;
> }
>
> +/*
> + * Parse "bus=" argument without adding a dependency on rte_kvargs.h
> + */
> +static char *parse_bus_arg(const char *args)
> +{
> + const char *c;
> + char *str = NULL;
> +
> + if (!args || args[0] == '\0')
> + return NULL;
> +
> + c = args;
> +
> + do {
> + if (strncmp(c, "bus=", 4) == 0) {
> + c += 4;
> + break;
> + }
> +
> + c = strchr(c, ',');
> + if (c)
> + c++;
> + } while (c);
> +
> + if (c) {
> + str = strdup(c);
> + c = strchr(str, ',');
> + if (c)
> + c = '\0';
> + }
> +
> + return str;
> +}
> +
> static int
> devargs_add(const char *busname, const char *name, const char *args)
> {
> @@ -113,64 +147,65 @@ devargs_add(const char *busname, const char *name, const char *args)
> return -1;
> }
>
> -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)
> {
> - struct rte_bus *bus = NULL;
> - const char *devname;
> - const size_t maxlen = sizeof(da->name);
> - size_t i;
> + char *busname;
> + char *devname;
> + char *drvargs;
> + int ret;
>
> if (dev == NULL || da == NULL)
> return -EINVAL;
> - /* Retrieve eventual bus info */
> - do {
> - devname = dev;
> - bus = rte_bus_find(bus, bus_name_cmp, dev);
> - if (bus == NULL)
> - break;
> - devname = dev + strlen(bus->name) + 1;
> - if (rte_bus_find_by_device_name(devname) == bus)
> - break;
> - } while (1);
> - /* Store device name */
> - i = 0;
> - while (devname[i] != '\0' && devname[i] != ',') {
> - da->name[i] = devname[i];
> - i++;
> - if (i == maxlen) {
> - fprintf(stderr, "WARNING: Parsing \"%s\": device name should be shorter than %zu\n",
> - dev, maxlen);
> - da->name[i - 1] = '\0';
> - return -EINVAL;
> - }
> - }
> - da->name[i] = '\0';
> - if (bus == NULL) {
> - bus = rte_bus_find_by_device_name(da->name);
> +
> + ret = rte_eal_parse_devargs_str(dev, &devname, &drvargs);
> + if (ret != 0)
> + return ret;
> +
> + RTE_LOG(DEBUG, EAL, "%s: adding devargs for device [%s]: %s\n",
> + __FUNCTION__, devname, drvargs);
> +
> + /* Retrieve eventual bus info (...,bus=...)*/
> + busname = parse_bus_arg(drvargs);
> + if (busname == NULL) {
> + struct rte_bus *bus;
> +
> + bus = rte_bus_find_by_device_name(devname);
> if (bus == NULL) {
> - fprintf(stderr, "ERROR: failed to parse device \"%s\"\n",
> - da->name);
> - return -EFAULT;
> + RTE_LOG(ERR, EAL, "failed to parse device [%s]\n",
> + devname);
> + ret = -EINVAL;
> + goto fail;
> }
> +
> + busname = strdup(bus->name);
> }
> - da->bus = bus;
> - /* Parse eventual device arguments */
> - if (devname[i] == ',')
> - da->args = strdup(&devname[i + 1]);
> - else
> - da->args = strdup("");
> - if (da->args == NULL) {
> - fprintf(stderr, "ERROR: not enough memory to parse arguments\n");
> - return -ENOMEM;
> +
> + ret = devargs_add(busname, devname, drvargs);
> + if (ret != 0) {
> + RTE_LOG(ERR, EAL, "devargs_add failed for device [%s]\n",
> + devname);
> + goto fail;
> }
> - return 0;
> +
> + ret = snprintf(da->busname, sizeof(da->busname), "%s", busname);
> + if (ret < 0 || ret >= (int)sizeof(da->busname))
> + goto fail;
> +
> + ret = snprintf(da->name, sizeof(da->name), "%s", devname);
> + if (ret < 0 || ret >= (int)sizeof(da->name))
> + goto fail;
> +
> + da->args = strdup(drvargs);
> + if (da->args == NULL)
> + ret = -ENOMEM;
> + ret = 0;
> +
> +fail:
> + free(busname);
> + free(devname);
> + free(drvargs);
> + return ret;
> }
>
> static const struct rte_bus_conf BUS_CONF_WHITELIST = {
> diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c
> index 048b3d00b..3a1033ca3 100644
> --- a/test/test/test_devargs.c
> +++ b/test/test/test_devargs.c
> @@ -58,6 +58,7 @@ test_devargs(void)
> {
> struct rte_devargs_list save_devargs_list;
> struct rte_devargs *devargs;
> + struct rte_devargs devargs_stack;
>
> /* save the real devargs_list, it is restored at the end of the test */
> save_devargs_list = devargs_list;
> @@ -109,6 +110,24 @@ test_devargs(void)
> goto fail;
> free_devargs_list();
>
> + 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,bus=pci", &devargs_stack) != 0) {
> + printf("Error in rte_eal_devargs_parse 08:00.1,bus=pci\n");
> + goto fail;
> + }
> + devargs = TAILQ_FIRST(&devargs_list);
> + if (strcmp(devargs->name, "08:00.1") != 0)
> + goto fail;
> + if (strcmp(devargs->busname, "pci") != 0)
> + goto fail;
> + if (!devargs->args || strcmp(devargs->args, "foo=bar,bus=pci") != 0)
> + goto fail;
> +
> + free_devargs_list();
> devargs_list = save_devargs_list;
> return 0;
>
> --
> 2.13.2
>
--
Gaëtan Rivet
6WIND