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 18ED54C92 for ; Tue, 20 Mar 2018 18:51:44 +0100 (CET) Received: by mail-wr0-f177.google.com with SMTP id o1so2598512wro.10 for ; Tue, 20 Mar 2018 10:51:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=UoasQBHDK4tpLfzCsGnvbZR8aX6vqdyHuVZB4YBsNB4=; b=rfGBwOPftQqT9ZHk1aoaXV/EorQf8eyB4Ial0G1DICQtkQNMxUbA/BnFiljHyEF/JA 2XaLJ30vGiwLHWvQpZuYTsJ9EMhseTf6tdWuFBEz3+pKg99DKCeqwwxsUzAHRM4eX0Xz qbpsxsGVtwNpYlQQZjQPgei+gDlHz9Nfylb29cr/0FQINmiJZOOcamsoKktSp625BK9s Pmr8wRH+af9os6LekU0LYzHKVrAd3d9IGztWyeaZnQBz1jB5B7/pC6cMmIz28nNxQ2gx Sw/ttRyPVbXkXJA2qy0fKoZSg45HV8U2ke7pZoOeyoiUCJE3bUayeesDZ5bAMkqX9O8+ 1+8w== 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:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=UoasQBHDK4tpLfzCsGnvbZR8aX6vqdyHuVZB4YBsNB4=; b=cXsSvv/wpZLl9VFwiCljTJHCBAEbh9mSFvkOJu+fj9gz9qlK31AOLg/cGo9Wdiu9Nr VCMQDcyyrugDh596UzE+7U7KZyz7fnSFJvqWM/DscvWP2UQqVtDJiiP9Ifk2LUfib2VB AVGPSIR73fNzWMnO/5QvVddeKGnqggz5m/ERNmou0ShR7o6EA27vmythyXDNTYRLK8IQ 9uZoLFS4ILLUZyRSyAo5JH8g683eJvj/NJY692JIEusQEnAHy1HVzCZ5j+EPqQxUGFFL SoWtS/6yrLpG0ffJLT3GRJS71LandqZw/rmrpdRdKeUGAcGZJ71HO4+O3pAPqaHy4mNa jFaA== X-Gm-Message-State: AElRT7GlrO7DfLhupAVY8Lip3gc7fJJBiGzHojxrjMDgQjLqf3swtZkb qVQFpaPprdCOTsgp1HTxo8iIyG5g X-Google-Smtp-Source: AG47ELsF5eqKmoitpOrpj+Kmga6XgKjX4nrDk4PPLzkcDAhophZLDwOBxtriZZAxAtpX0NYQdcJf3Q== X-Received: by 10.223.197.1 with SMTP id q1mr13133206wrf.268.1521568303149; Tue, 20 Mar 2018 10:51:43 -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 m9sm2415170wrg.79.2018.03.20.10.51.42 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 20 Mar 2018 10:51:42 -0700 (PDT) Date: Tue, 20 Mar 2018 18:51:28 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: dev@dpdk.org Message-ID: <20180320175128.7ddlr7nnazwfcsqg@bidouze.vm.6wind.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v1 00/18] Device querying 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, 20 Mar 2018 17:51:44 -0000 On Thu, Mar 15, 2018 at 06:49:30PM +0100, Gaetan Rivet wrote: > This patchset introduces a new EAL API for querying devices, > filtered by arbitrary properties. > > The following elements are introduced to this end: > > * A new object, "rte_class", is used to describe > the device class abstraction layer (eth, crypto, ...). > > * Both rte_bus and rte_class now offer a way to > list their devices and filter the result > using locally defined properties. > > * The rte_dev API now has an rte_dev_iterator, which > is the way for the user to define the device filter > and iterate upon the resulting set. > > As an example, the "eth" device class is implemented. > > Additionally, the device filters for > > + rte_bus_pci > + rte_bus_vdev > + rte_class_eth > > are implemented and can be used with some > properties each, to show how to extend those. > > Some example of filters: > > "bus=pci/class=eth" > "bus=pci" > "class=eth" > "class=eth,name=net_ring0" > "bus=pci,id=00:00.0" > "bus=vdev,driver=net_ring" > > Gaetan Rivet (18): To ease reviewing and have a state of this work, here is a summary of what to expect / my opinion of what's done: > eal: introduce dtor macros * Not sure this is necessary, but as long as the libc has a dlclose, it makes sense to have a way to cleanly unload objects. > eal: introduce device class abstraction * The class abstraction might seem controversial introduced there, but I do not think there is any other solution than adding it. The EAL needs a way to converse in a generic way with each types of devices, even if the technical solution I proposed afterward is not accepted. > eal/class: register destructor * If the dtor patch above is accepted, this patch could be squashed with the previous one. Otherwise, it can be dropped. > eal: add lightweight kvarg parsing utility > eal/dev: add device iterator interface * An important question regarding device matching is how to deal when the query is ambiguous and several devices could match. I have taken the technical bias of leaving the management of this case to the user. This has the beneficial side-effect of allowing to list all devices matching specific properties. I think this is useful. However, much of the complexity of this solution comes from this added functionality. * The iterator only allows filtering on the two abstractions: + bus + class This limitation is hard-coded, simplifying the implementation. > eal/dev: implement device iteration initialization * An important element of this initialization was to make it entirely static. To help the user correctly use this API, this seemed important. No dynamic allocation was to be performed here. This means that the device description string must live for the time of the iteration. An iterator must be iterated in the same scope it has been initialized. A big problem with this approach, is that the matching strings cannot be duplicated to be properly reformated, helping the subsequent parsing. Thus, the iteration operation implemented both in rte_bus and rte_class are more complex, having to take care of both cases where an ending '/' and '\0' are present, and this, for all implementations. I profoundly dislike this complexity being replicated thusly, but I think the advantage of having a lean iterator init is worth it. > eal/class: add device iteration > eal/bus: add device iteration > eal/dev: implement device iteration * Iteration has been choosen over implementing only a match function, that would be used in conjunction with the existing bus->find_device The reasoning for this is two-fold: - Classes are not streamlined. They usually link rte_devices instead of extending the rte_device object with their own meta-data, like rte_buses are doing. So the iteration itself had to be as flexible as possible, to allow rte_classes to do anything they might need to speed up / simplify the iteration. - The filtering strings could have been made once for all filters, having only to call *_find_device with the proper matching function. However, instead of replicating the string handling, each matching would thus have to parse the kvargs itself or have the kvargs parsed prior to being called. The latter would mean having the EAL dependent on librte_kvargs. The former meaning that kvargs would be repeated again and again, with dynamic allocations with it (meaning that they could not just be kept away until the next iteration as they would be already parsed, as the user could always break the iteration, loosing the reference). I think the choice of iteration vs. match function is central to the design of this solution and should be carefully reviewed. I tried to make the right call, can be wrong. vvvvvvvvv <-- the rest only builds upon the foundations layed out by the previous patches, so choices here are less important. Last point however: I havent yet implemented the rte_eth_dev finder taking a device description as input and giving the corresponding port_id. It should be simple to implement with this new API so I will do it shortly, once I have some time for it. > ethdev: register ether layer as a class > ethdev: add device matching field name > bus/pci: fix find device implementation > bus/pci: implement device iteration and comparison > bus/pci: add device matching field id > bus/vdev: fix find device implementation > bus/vdev: implement device iteration > bus/vdev: add device matching field driver > app/testpmd: add show device command > > app/test-pmd/cmdline.c | 52 +++++++++ > drivers/bus/pci/Makefile | 2 +- > drivers/bus/pci/pci_common.c | 97 ++++++++++++++-- > drivers/bus/pci/rte_bus_pci.h | 3 + > drivers/bus/vdev/Makefile | 2 +- > drivers/bus/vdev/rte_bus_vdev.h | 3 + > drivers/bus/vdev/vdev.c | 86 +++++++++++++- > lib/Makefile | 2 +- > lib/librte_eal/bsdapp/eal/Makefile | 1 + > lib/librte_eal/common/Makefile | 2 +- > lib/librte_eal/common/eal_common_class.c | 62 ++++++++++ > lib/librte_eal/common/eal_common_dev.c | 177 +++++++++++++++++++++++++++++ > lib/librte_eal/common/eal_private.h | 34 ++++++ > lib/librte_eal/common/include/rte_bus.h | 1 + > lib/librte_eal/common/include/rte_class.h | 127 +++++++++++++++++++++ > lib/librte_eal/common/include/rte_common.h | 23 ++++ > lib/librte_eal/common/include/rte_dev.h | 84 ++++++++++++++ > lib/librte_eal/linuxapp/eal/Makefile | 1 + > lib/librte_eal/rte_eal_version.map | 4 + > lib/librte_ether/Makefile | 3 +- > lib/librte_ether/rte_class_eth.c | 100 ++++++++++++++++ > 21 files changed, 847 insertions(+), 19 deletions(-) > create mode 100644 lib/librte_eal/common/eal_common_class.c > create mode 100644 lib/librte_eal/common/include/rte_class.h > create mode 100644 lib/librte_ether/rte_class_eth.c > > -- > 2.11.0 > Regards, -- Gaëtan Rivet 6WIND