From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 29D301B29E for ; Tue, 17 Oct 2017 20:17:02 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4436DAB0A2; Tue, 17 Oct 2017 18:17:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4436DAB0A2 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=aconole@redhat.com Received: from dhcp-25-97.bos.redhat.com (unknown [10.18.25.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AAFD186E07; Tue, 17 Oct 2017 18:17:00 +0000 (UTC) From: Aaron Conole To: Gaetan Rivet Cc: dev@dpdk.org References: Date: Tue, 17 Oct 2017 14:16:59 -0400 In-Reply-To: (Gaetan Rivet's message of "Thu, 12 Oct 2017 10:21:09 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 17 Oct 2017 18:17:01 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 02/18] eal: remove generic devtype 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: Tue, 17 Oct 2017 18:17:02 -0000 Gaetan Rivet writes: > The devtype is now entirely defined by the device bus. As such, it is > already characterized by the bus identifier within an rte_devargs. > > The rte_devtype enum can disappear, along with crutches added during > this transition. > > rte_eal_devargs_type_count becomes useless and is removed. > > Signed-off-by: Gaetan Rivet > --- > drivers/bus/pci/pci_common.c | 16 ++------------ > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - > lib/librte_eal/common/eal_common_devargs.c | 20 +---------------- > lib/librte_eal/common/eal_common_options.c | 19 +++++----------- > lib/librte_eal/common/include/rte_dev.h | 8 ------- > lib/librte_eal/common/include/rte_devargs.h | 29 +------------------------ > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - > 7 files changed, 9 insertions(+), 85 deletions(-) > > diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c > index bbe862b..5fbcf11 100644 > --- a/drivers/bus/pci/pci_common.c > +++ b/drivers/bus/pci/pci_common.c > @@ -172,15 +172,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, > loc->domain, loc->bus, loc->devid, loc->function, > dev->device.numa_node); > > - /* no initialization when blacklisted, return without error */ > - if (dev->device.devargs != NULL && > - dev->device.devargs->policy == > - RTE_DEV_BLACKLISTED) { > - RTE_LOG(INFO, EAL, " Device is blacklisted, not" > - " initializing\n"); > - return 1; > - } > - > if (dev->device.numa_node < 0) { > RTE_LOG(WARNING, EAL, " Invalid NUMA socket, default to 0\n"); > dev->device.numa_node = 0; > @@ -380,11 +371,8 @@ rte_pci_probe(void) > probed++; > > devargs = dev->device.devargs; > - /* probe all or only whitelisted devices */ > - if (probe_all) > - ret = pci_probe_all_drivers(dev); > - else if (devargs != NULL && > - devargs->policy == RTE_DEV_WHITELISTED) > + /* probe all or only declared devices */ > + if (probe_all ^ (devargs != NULL)) What is the intent of this? If probe_all is true, and devargs != null, I think this branch isn't taken. Shouldn't this be ||? Maybe I missed something? > ret = pci_probe_all_drivers(dev); > if (ret < 0) { > RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > index 573869a..47416a5 100644 > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > @@ -22,7 +22,6 @@ DPDK_2.0 { > rte_eal_alarm_set; > rte_eal_devargs_add; > rte_eal_devargs_dump; > - rte_eal_devargs_type_count; > rte_eal_get_configuration; > rte_eal_get_lcore_state; > rte_eal_get_physmem_layout; > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c > index e371456..2fddbfa 100644 > --- a/lib/librte_eal/common/eal_common_devargs.c > +++ b/lib/librte_eal/common/eal_common_devargs.c > @@ -153,7 +153,7 @@ rte_eal_devargs_insert(struct rte_devargs *da) > > /* store a whitelist parameter for later parsing */ > int > -rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) > +rte_eal_devargs_add(const char *devargs_str) > { > struct rte_devargs *devargs = NULL; > const char *dev = devargs_str; > @@ -165,9 +165,6 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) > > if (rte_eal_devargs_parse(dev, devargs)) > goto fail; > - devargs->type = devtype; > - if (devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) > - devargs->policy = RTE_DEV_BLACKLISTED; > TAILQ_INSERT_TAIL(&devargs_list, devargs, next); > return 0; > > @@ -198,21 +195,6 @@ rte_eal_devargs_remove(const char *busname, const char *devname) > return 1; > } > > -/* count the number of devices of a specified type */ > -unsigned int > -rte_eal_devargs_type_count(enum rte_devtype devtype) > -{ > - struct rte_devargs *devargs; > - unsigned int count = 0; > - > - TAILQ_FOREACH(devargs, &devargs_list, next) { > - if (devargs->type != devtype) > - continue; > - count++; > - } > - return count; > -} > - > /* dump the user devices on the console */ > void > rte_eal_devargs_dump(FILE *f) > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c > index d57cb5d..603df27 100644 > --- a/lib/librte_eal/common/eal_common_options.c > +++ b/lib/librte_eal/common/eal_common_options.c > @@ -131,7 +131,6 @@ TAILQ_HEAD(device_option_list, device_option); > struct device_option { > TAILQ_ENTRY(device_option) next; > > - enum rte_devtype type; > char arg[]; > }; > > @@ -143,8 +142,7 @@ static int mem_parsed; > static int core_parsed; > > static int > -eal_option_device_add(enum rte_devtype type, > - const char *busname, const char *optarg) > +eal_option_device_add(const char *busname, const char *optarg) > { > struct device_option *devopt; > size_t optlen; > @@ -159,7 +157,6 @@ eal_option_device_add(enum rte_devtype type, > return -ENOMEM; > } > > - devopt->type = type; > if (busname != NULL) > ret = snprintf(devopt->arg, optlen, "%s:%s", > busname, optarg); > @@ -183,7 +180,7 @@ eal_option_device_parse(void) > > TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) { > if (ret == 0) { > - ret = rte_eal_devargs_add(devopt->type, devopt->arg); > + ret = rte_eal_devargs_add(devopt->arg); > if (ret) > RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n", > devopt->arg); > @@ -1009,19 +1006,15 @@ eal_parse_common_option(int opt, const char *optarg, > case 'b': > if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_BLACKLIST) < 0) > return -1; > - if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI, > - "pci", optarg) < 0) { > + if (eal_option_device_add("pci", optarg) < 0) > return -1; > - } > break; > /* whitelist */ > case 'w': > if (rte_bus_probe_mode_set("pci", RTE_BUS_PROBE_WHITELIST) < 0) > return -1; > - if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI, > - "pci", optarg) < 0) { > + if (eal_option_device_add("pci", optarg) < 0) > return -1; > - } > break; > /* coremask */ > case 'c': > @@ -1128,10 +1121,8 @@ eal_parse_common_option(int opt, const char *optarg, > break; > > case OPT_VDEV_NUM: > - if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL, > - "vdev", optarg) < 0) { > + if (eal_option_device_add("vdev", optarg) < 0) > return -1; > - } > break; > > case OPT_SYSLOG_NUM: > diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h > index 4c4ac7e..5f090ed 100644 > --- a/lib/librte_eal/common/include/rte_dev.h > +++ b/lib/librte_eal/common/include/rte_dev.h > @@ -127,14 +127,6 @@ enum rte_kernel_driver { > }; > > /** > - * Device policies. > - */ > -enum rte_dev_policy { > - RTE_DEV_WHITELISTED, > - RTE_DEV_BLACKLISTED, > -}; > - > -/** > * A generic memory resource representation. > */ > struct rte_mem_resource { > diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h > index 58d585d..e50c166 100644 > --- a/lib/librte_eal/common/include/rte_devargs.h > +++ b/lib/librte_eal/common/include/rte_devargs.h > @@ -53,15 +53,6 @@ extern "C" { > #include > > /** > - * Type of generic device > - */ > -enum rte_devtype { > - RTE_DEVTYPE_WHITELISTED_PCI, > - RTE_DEVTYPE_BLACKLISTED_PCI, > - RTE_DEVTYPE_VIRTUAL, > -}; > - > -/** > * Structure that stores a device given by the user with its arguments > * > * A user device is a physical or a virtual device given by the user to > @@ -74,10 +65,6 @@ enum rte_devtype { > struct rte_devargs { > /** Next in list. */ > TAILQ_ENTRY(rte_devargs) next; > - /** Type of device. */ > - enum rte_devtype type; > - /** Device policy. */ > - enum rte_dev_policy policy; > /** Bus handle for the device. */ > struct rte_bus *bus; > /** Name of the device. */ > @@ -166,8 +153,6 @@ rte_eal_devargs_insert(struct rte_devargs *da); > * driver name is not checked by this function, it is done when probing > * the drivers. > * > - * @param devtype > - * The type of the device. > * @param devargs_str > * The arguments as given by the user. > * > @@ -175,7 +160,7 @@ rte_eal_devargs_insert(struct rte_devargs *da); > * - 0 on success > * - A negative value on error > */ > -int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str); > +int rte_eal_devargs_add(const char *devargs_str); > > /** > * Remove a device from the user device list. > @@ -196,18 +181,6 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str); > int rte_eal_devargs_remove(const char *busname, const char *devname); > > /** > - * Count the number of user devices of a specified type > - * > - * @param devtype > - * The type of the devices to counted. > - * > - * @return > - * The number of devices. > - */ > -unsigned int > -rte_eal_devargs_type_count(enum rte_devtype devtype); > - > -/** > * This function dumps the list of user device and their arguments. > * > * @param f > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > index a2709e3..e1e2a50 100644 > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > @@ -22,7 +22,6 @@ DPDK_2.0 { > rte_eal_alarm_set; > rte_eal_devargs_add; > rte_eal_devargs_dump; > - rte_eal_devargs_type_count; > rte_eal_get_configuration; > rte_eal_get_lcore_state; > rte_eal_get_physmem_layout;