From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH v2 00/18] devargs cleanup Date: Wed, 18 Oct 2017 10:36:41 +0200 Message-ID: <20171018083641.GB3596@bidouze.vm.6wind.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org To: Aaron Conole Return-path: Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46]) by dpdk.org (Postfix) with ESMTP id 1F0E81B5F9 for ; Wed, 18 Oct 2017 10:36:54 +0200 (CEST) Received: by mail-wm0-f46.google.com with SMTP id 196so11747623wma.1 for ; Wed, 18 Oct 2017 01:36:54 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Oct 17, 2017 at 02:18:02PM -0400, Aaron Conole wrote: > Gaetan Rivet writes: > > > The use of rte_devargs is inconsistent in the light of new functionalities > > such as device hotplug. > > > > Most of its API is still experimental and needs stabilization. > > Older functions were deprecated and need to be rewritten or removed. > > The rte_devtype is meant to disappear. > > > > v2: > > > > Big rework. > > > > * Enact requiring bus name prepended in rte_devargs parsing functions. > > * Remove rte_devtype. Use new probe mode setter along with generic > > bus reference within rte_devargs. > > * Rework devargs parsing API. > > The function is now variadic, does not enforce bus rules on the devargs > > being inserted as the bus has been configured previously. > > Old parsing function is removed. > > * Expose bus guessing from device name. > > This uses the "parse" bus operator, which may be meant to disappear. > > This is optional, but nice to have in a transition period. > > * Introduce new --dev generic device declaration parameter. > > > > This patchset depends on: > > It is weird to me that you introduce patch sets, and then introduce > cleanups later? Shouldn't we revise the existing patchsets? > > Maybe I missed a discussion somewhere? > No discussion missed :) The way it happened here was that I started cleaning up the mess I did last release in the rte_devargs to streamline somewhat the subsystem. Several things needed to disappear: 1. List of devargs manipulated by buses. With hotplug feature they had to be properly managed (i.e. freed on device unplug). 2. rte_devtype, which was meant to disappear last release but was kept to smooth the transition to the new API. 3. Different usages of devargs_add among applications / PMDs There had been some different last release about the exposed API, thus I made a compromise that could potentially benefit everyone. Points 1 and 3 did not require additional work on the buses. Point 2 however, required a way for buses to be configured in whitelist / blacklist mode. To clean up the rte_devtype, a configuration facility is necessary. While working on this facility, I found that I was once again breaking the ABI of rte_bus, along with the IOVA configuration. I thought it could be streamlined in a more stable way, thus I wrote the separate patchset for a cleaner integration of these changes in this release. All in all, the PCI move is not necessary but was sent a few weeks ago already and it could be skipped with a small rework. The bus control could be done in a more hackish way as well, without having a whole separate patchset to introduce the control facility. I wanted to propose a cleaner approach that could possibly keep the rte_bus ABI stable for some time. > > Move PCI away from the EAL > > http://dpdk.org/ml/archives/dev/2017-August/073512.html > > > > Bus control framework > > http://dpdk.org/ml/archives/dev/2017-October/078752.html > > > > Gaetan Rivet (18): > > eal: prepend busname on legacy device declaration > > eal: remove generic devtype > > devargs: introduce iterator > > devargs: introduce foreach macro > > vdev: do not reference devargs list > > bus/pci: do not reference devargs list > > test: remove devargs unit tests > > devargs: make devargs list private > > devargs: make parsing variadic > > devargs: require bus name prefix > > devargs: simplify implementation > > eal: add generic device declaration parameter > > bus: make device recognition function public > > net/failsafe: keep legacy sub-device declaration > > ether: use new devargs parsing function > > devargs: remove old devargs parsing function > > devargs: use proper prefix > > doc: remove devargs deprecation notices > > > > MAINTAINERS | 1 - > > app/test-pmd/cmdline.c | 2 +- > > doc/guides/rel_notes/deprecation.rst | 13 --- > > drivers/bus/pci/pci_common.c | 22 +--- > > drivers/net/failsafe/failsafe_args.c | 11 +- > > examples/bond/main.c | 2 +- > > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 ++- > > lib/librte_eal/common/eal_common_dev.c | 39 ++----- > > lib/librte_eal/common/eal_common_devargs.c | 129 +++++++++-------------- > > lib/librte_eal/common/eal_common_options.c | 47 ++++++--- > > lib/librte_eal/common/eal_common_vdev.c | 11 +- > > lib/librte_eal/common/eal_options.h | 2 + > > lib/librte_eal/common/include/rte_bus.h | 12 +++ > > lib/librte_eal/common/include/rte_dev.h | 8 -- > > lib/librte_eal/common/include/rte_devargs.h | 120 ++++++++-------------- > > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 ++- > > lib/librte_ether/rte_ethdev.c | 11 +- > > test/test/Makefile | 1 - > > test/test/commands.c | 2 +- > > test/test/test_devargs.c | 131 ------------------------ > > 20 files changed, 186 insertions(+), 408 deletions(-) > > delete mode 100644 test/test/test_devargs.c -- Gaëtan Rivet 6WIND