Re: [dpdk-dev] [PATCH v2 00/18] devargs cleanup

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

On Tue, Oct 17, 2017 at 02:18:02PM -0400, Aaron Conole wrote:
> Gaetan Rivet <gaetan.rivet@6wind.com> 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