[dpdk-dev] [PATCH 0/9] fix hotplug API

[Gaëtan Rivet]

On Sun, Jul 09, 2017 at 06:38:52AM -0400, Jan Blunck wrote:
> On Sat, Jul 8, 2017 at 9:45 PM, Gaetan Rivet <gaetan.rivet at 6wind.com> wrote:
> > Sending those fixes as separate patches as they stand on their own.
> > This series improves usability of the hotplug API and fixes a few issues
> > with existing implementations.
> >
> 
> Interesting that you send this series as fixes. From what I can tell
> this is a collection of changes that you have proposed before and I
> have commented on and requested changes for. But I don't see that they
> have been addressed at all:
> 

The API changes are limited to two elements to make using this API
easier. The rest are fixes: inserting relevant rte_devargs, fixing PCI
hotplug, supporting the hotplug interface in vdev, releasing the
relevant resources on remove. Those are bugs which make hotplug
impossible to use right now, and this series aims to fix those.

> - you still concatenate the bus and device name just to pass it down
> to the buses and parse it into its components again

I use the current API. This API has been discussed in other places and
the reason to keep the current behavior, and how it should evolve
shortly, has already been exposed. In the meantime, even if planified to
evolve, the API should still be respected.

I know you disagreed in the relevant thread[2], as you repeat it here.
However, you received a response both from me and Bruce regarding this
issue and did not press the matter further, which led to an implicit approval.

> - you still delegate the devargs parsing to the buses
> 

I agree that this should be avoided. The relevant API is clearly marked
as going out shortly[3].

But this is not relevant to fixing the hotplug API.

> Please fix this issues. If you really want to return a device handle
> from the hotplug API please present the use-case and in which cases
> this solves a real-world problem.
> 
> 

Other solutions can be used instead, always. It just seems simpler to
work with. I can do without it if that's a hangup.

[2]: http://dpdk.org/ml/archives/dev/2017-June/069256.html
[3]: http://dpdk.org/ml/archives/dev/2017-July/070505.html

> > The hotplug API can be tested with the fail-safe PMD[1]. Its
> > documentation describes how to declare slaves and how to use it.
> >
> > [1]: http://dpdk.org/ml/archives/dev/2017-July/070529.html
> >
> > Gaetan Rivet (9):
> >   eal: return device handle upon plugin
> >   eal: fix hotplug add
> >   devargs: introduce removal function
> >   eal: release devargs on device removal
> >   pci: use given name as generic name
> >   pci: fix generic driver pointer on probe error
> >   pci: fix hotplug operations
> >   vdev: implement plug operation
> >   bus: remove useless plug parameter
> >
> >  lib/librte_eal/bsdapp/eal/eal_pci.c             |  4 +-
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> >  lib/librte_eal/common/eal_common_dev.c          | 83 +++++++++++++++++++++----
> >  lib/librte_eal/common/eal_common_devargs.c      | 18 ++++++
> >  lib/librte_eal/common/eal_common_pci.c          | 49 +++++++--------
> >  lib/librte_eal/common/eal_common_vdev.c         | 12 ++--
> >  lib/librte_eal/common/eal_private.h             |  5 ++
> >  lib/librte_eal/common/include/rte_bus.h         |  6 +-
> >  lib/librte_eal/common/include/rte_dev.h         | 10 +--
> >  lib/librte_eal/common/include/rte_devargs.h     | 18 ++++++
> >  lib/librte_eal/common/include/rte_vdev.h        |  7 +++
> >  lib/librte_eal/linuxapp/eal/eal_pci.c           |  4 +-
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >  13 files changed, 162 insertions(+), 56 deletions(-)
> >
> > --
> > 2.1.4
> >

-- 
Gaëtan Rivet
6WIND