From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com [209.85.128.194]) by dpdk.org (Postfix) with ESMTP id 47B442BAA for ; Thu, 13 Jul 2017 21:34:35 +0200 (CEST) Received: by mail-wr0-f194.google.com with SMTP id n18so2144576wrb.0 for ; Thu, 13 Jul 2017 12:34:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-transfer-encoding; bh=uzYjCcViiNgpcR4l1OdIUn4lyyZGESO2/MWPVwJJheM=; b=d8+1Adfy+zXbVe0RrIUf7PDbzxf3QCbAFClIhwkHYY1sPQfqouFD0Chhv165xuzBDq y0jPP0YWDRj5gskwhWc5Adz9m+mZd9DF5yugtHTHIVXo7telgJtEoeVucPVhYItRNSI+ 1JrXevQ9f64ovWST7YSGOxqLdvKFl4XBG6qq//mUqHM/WxM7aGJcHQSyMB9WYyTxGxTx BgV3V/kMZedkC5iYoi8biYrnHsSwr8aXKkN3xvsWSU6bCeI9b437TiQ+Bq7e9q+oP9LX sbsqO+/L3hK/YO/WpbkJ+rGjmnyvmBXMLRC+3/BDTVoO4dOQuNT2yZmhWQv7jb97B8yE H1RA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc:content-transfer-encoding; bh=uzYjCcViiNgpcR4l1OdIUn4lyyZGESO2/MWPVwJJheM=; b=b5kL3x6Daya3zrPHeyRDbjAh72rFevcg3sptNTLHnQrI/fEuRYLEZJXR2/oRtVVje1 cnRgBGw7NR1yy9/Fg3rp8kkpRDjj/7EnEoKx+EBbfNI2YPrNtufNS77gUPVMfhgy69Jk K0egBVssk4NwDIymZLNodQWER7e/+gZcQJmJyIQ+EA4enYriyVmmyUpsbSyzCoGoUSGv tkEkTkzeje+zJHEzgAGpB8it5uy6WzbDZeoVsAXgzKy5c9IaA/jGf/ekbE5DKT+FSpfS BRlg4Rb7A+AFJUIFCZOcdfxvVURRSIfZwTvA2WWHUZ2qtJAMq1EBf/076v/j6bVRA4r8 FfKA== X-Gm-Message-State: AIVw110mbvd9ey6ktnLihT6AgO5iJmw35KWfIfhirfLjnoYULAx4Qa5x PCaGIALmiBCdGCgLixoYZXMccsEw3g== X-Received: by 10.223.133.211 with SMTP id 19mr2510204wru.27.1499974474600; Thu, 13 Jul 2017 12:34:34 -0700 (PDT) MIME-Version: 1.0 Sender: jblunck@gmail.com Received: by 10.28.45.210 with HTTP; Thu, 13 Jul 2017 12:34:33 -0700 (PDT) In-Reply-To: <20170713134003.GK11154@bidouze.vm.6wind.com> References: <20170711232512.54641-1-jblunck@infradead.org> <20170711232512.54641-10-jblunck@infradead.org> <20170713134003.GK11154@bidouze.vm.6wind.com> From: Jan Blunck Date: Thu, 13 Jul 2017 15:34:33 -0400 X-Google-Sender-Auth: fpInf7Phq53cP28CP1Z9luqAoLo Message-ID: To: =?UTF-8?Q?Ga=C3=ABtan_Rivet?= Cc: dev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Jul 2017 19:34:35 -0000 On Thu, Jul 13, 2017 at 9:40 AM, Ga=C3=ABtan Rivet = wrote: > I still disagree with parsing the body of devargs to retrieve bus > information. They should not be mixed. > I agree. I always said that it should be passed explicitly. If we agree on this I'll fix the signature of rte_eal_devargs_parse() accordingly. > 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 rewor= ked > 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=3D" argument. >> >> Signed-off-by: Jan Blunck >> --- >> 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=3D" argument without adding a dependency on rte_kvargs.h >> + */ >> +static char *parse_bus_arg(const char *args) >> +{ >> + const char *c; >> + char *str =3D NULL; >> + >> + if (!args || args[0] =3D=3D '\0') >> + return NULL; >> + >> + c =3D args; >> + >> + do { >> + if (strncmp(c, "bus=3D", 4) =3D=3D 0) { >> + c +=3D 4; >> + break; >> + } >> + >> + c =3D strchr(c, ','); >> + if (c) >> + c++; >> + } while (c); >> + >> + if (c) { >> + str =3D strdup(c); >> + c =3D strchr(str, ','); >> + if (c) >> + c =3D '\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 =3D NULL; >> - const char *devname; >> - const size_t maxlen =3D sizeof(da->name); >> - size_t i; >> + char *busname; >> + char *devname; >> + char *drvargs; >> + int ret; >> >> if (dev =3D=3D NULL || da =3D=3D NULL) >> return -EINVAL; >> - /* Retrieve eventual bus info */ >> - do { >> - devname =3D dev; >> - bus =3D rte_bus_find(bus, bus_name_cmp, dev); >> - if (bus =3D=3D NULL) >> - break; >> - devname =3D dev + strlen(bus->name) + 1; >> - if (rte_bus_find_by_device_name(devname) =3D=3D bus) >> - break; >> - } while (1); >> - /* Store device name */ >> - i =3D 0; >> - while (devname[i] !=3D '\0' && devname[i] !=3D ',') { >> - da->name[i] =3D devname[i]; >> - i++; >> - if (i =3D=3D maxlen) { >> - fprintf(stderr, "WARNING: Parsing \"%s\": device n= ame should be shorter than %zu\n", >> - dev, maxlen); >> - da->name[i - 1] =3D '\0'; >> - return -EINVAL; >> - } >> - } >> - da->name[i] =3D '\0'; >> - if (bus =3D=3D NULL) { >> - bus =3D rte_bus_find_by_device_name(da->name); >> + >> + ret =3D rte_eal_parse_devargs_str(dev, &devname, &drvargs); >> + if (ret !=3D 0) >> + return ret; >> + >> + RTE_LOG(DEBUG, EAL, "%s: adding devargs for device [%s]: %s\n", >> + __FUNCTION__, devname, drvargs); >> + >> + /* Retrieve eventual bus info (...,bus=3D...)*/ >> + busname =3D parse_bus_arg(drvargs); >> + if (busname =3D=3D NULL) { >> + struct rte_bus *bus; >> + >> + bus =3D rte_bus_find_by_device_name(devname); >> if (bus =3D=3D 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 =3D -EINVAL; >> + goto fail; >> } >> + >> + busname =3D strdup(bus->name); >> } >> - da->bus =3D bus; >> - /* Parse eventual device arguments */ >> - if (devname[i] =3D=3D ',') >> - da->args =3D strdup(&devname[i + 1]); >> - else >> - da->args =3D strdup(""); >> - if (da->args =3D=3D NULL) { >> - fprintf(stderr, "ERROR: not enough memory to parse argumen= ts\n"); >> - return -ENOMEM; >> + >> + ret =3D devargs_add(busname, devname, drvargs); >> + if (ret !=3D 0) { >> + RTE_LOG(ERR, EAL, "devargs_add failed for device [%s]\n", >> + devname); >> + goto fail; >> } >> - return 0; >> + >> + ret =3D snprintf(da->busname, sizeof(da->busname), "%s", busname); >> + if (ret < 0 || ret >=3D (int)sizeof(da->busname)) >> + goto fail; >> + >> + ret =3D snprintf(da->name, sizeof(da->name), "%s", devname); >> + if (ret < 0 || ret >=3D (int)sizeof(da->name)) >> + goto fail; >> + >> + da->args =3D strdup(drvargs); >> + if (da->args =3D=3D NULL) >> + ret =3D -ENOMEM; >> + ret =3D 0; >> + >> +fail: >> + free(busname); >> + free(devname); >> + free(drvargs); >> + return ret; >> } >> >> static const struct rte_bus_conf BUS_CONF_WHITELIST =3D { >> 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 te= st */ >> save_devargs_list =3D devargs_list; >> @@ -109,6 +110,24 @@ test_devargs(void) >> goto fail; >> free_devargs_list(); >> >> + if (rte_eal_devargs_parse("", &devargs_stack) =3D=3D 0) { >> + printf("Error in rte_eal_devargs_parse\n"); >> + goto fail; >> + } >> + >> + if (rte_eal_devargs_parse("08:00.1,foo=3Dbar,bus=3Dpci", &devargs_= stack) !=3D 0) { >> + printf("Error in rte_eal_devargs_parse 08:00.1,bus=3Dpci\n= "); >> + goto fail; >> + } >> + devargs =3D TAILQ_FIRST(&devargs_list); >> + if (strcmp(devargs->name, "08:00.1") !=3D 0) >> + goto fail; >> + if (strcmp(devargs->busname, "pci") !=3D 0) >> + goto fail; >> + if (!devargs->args || strcmp(devargs->args, "foo=3Dbar,bus=3Dpci")= !=3D 0) >> + goto fail; >> + >> + free_devargs_list(); >> devargs_list =3D save_devargs_list; >> return 0; >> >> -- >> 2.13.2 >> > > -- > Ga=C4=97tan Rivet > 6WIND