[dpdk-dev] [PATCH v2 00/11] bus: attach / detach API
Gaëtan Rivet
gaetan.rivet at 6wind.com
Mon Jun 26 02:27:02 CEST 2017
Hi Stephen,
On Wed, May 31, 2017 at 08:34:26AM -0700, Stephen Hemminger wrote:
> On Wed, 31 May 2017 15:17:45 +0200
> Gaetan Rivet <gaetan.rivet at 6wind.com> wrote:
>
> > Following the work from Jan:
> >
> > This patchset introduces the attach / detach API to rte_bus.
> > The rte_device structure is used as the generic device representation.
> >
> > This API is implemented for the virtual bus.
> > The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
> > use this new interface.
> >
> > --
> >
> > 0. API rework
> > -------------
> >
> > I would like to propose an evolution on the API developed by Jan.
> >
> > The attach / detach rte_bus API is necessary to support the attach/detach
> > rte_dev API. Those are two different levels for one similar functionality.
> >
> > Attach / detach does not allow true hotplugging, because the attach
> > function expects the devices operated upon to already exist within the
> > buses / sub-layers. This means that this API expects devices meta-datas
> > (bus-internal device representation and associated device information
> > read from the system) to be present upon attach. This part of the work
> > is done during scanning.
> >
> > While it is best to avoid changing the public rte_dev API as it already
> > exists, nothing prevents this new rte_bus API from superseeding it.
> > It has been said during the previous release cycle that device hotplug
> > was a feature that interested users. True hotplug is not allowed by the
> > current attach / detach API. Worse, this API hinders the effort to bring
> > this new functionality by squatting its semantic field.
> >
> > Thus, I propose to rename rte_bus attach / detach; plug / unplug. As it
> > is a superset of the attach / detach functionality, it can be used to
> > implement rte_dev attach / detach. Now is the right time to pivot to
> > this new feature.
> >
> > This should help maintainers understanding the aim of this API and the
> > differences with the APIs higher-up, clarify the field and allow a new
> > functionality to be proposed.
> >
> > The vdev bus is inherently supporting the new API, however it has been
> > made explicit. My implementation in the PCI bus in further patchset also
> > follows the rte_bus hotplug API instead of only attach / detach.
> >
> > One remaining problem with the vdev bus is the rte_dev attach
> > implementation, which needs the rte_devargs rework to be properly fixed.
> >
> > 1. Additional evolutions in the patchset
> > ----------------------------------------
> >
> > The RTE_VERIFY on the find_device is too stringent I think and forces
> > all buses to implement a public device iterator. While it could be
> > argued that it would push for quicker support for the functionality, I
> > think it's possible that some buses are not interested at all in it and
> > should simply be ignored.
> >
> > The bus devices iterator has been fixed.
> >
> > The internal rte_device handle was not properly setup within the
> > net_ring PMD.
> >
> > Gaetan Rivet (2):
> > vdev: implement hotplug functionality
> > net/ring: fix dev handle in eth_dev
> >
> > Jan Blunck (9):
> > bus: add bus iterator to find a particular bus
> > bus: add device iterator
> > bus: add helper to find bus for a particular device
> > bus: add bus helper iterator to find a particular device
> > bus: introduce hotplug functionality
> > vdev: implement find_device bus operation
> > vdev: implement unplug bus operation
> > eal: make virtual driver probe and remove take rte_vdev_device
> > ethdev: Use embedded rte_device to detach driver
> >
> > drivers/net/ring/rte_eth_ring.c | 7 ++
> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
> > lib/librte_eal/common/eal_common_bus.c | 65 +++++++++++++++
> > lib/librte_eal/common/eal_common_dev.c | 100 ++++++++++++++++++------
> > lib/librte_eal/common/eal_common_vdev.c | 27 +++++++
> > lib/librte_eal/common/include/rte_bus.h | 87 +++++++++++++++++++++
> > lib/librte_eal/common/include/rte_dev.h | 26 ++++++
> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 3 +
> > lib/librte_ether/rte_ethdev.c | 3 +-
> > 9 files changed, 299 insertions(+), 23 deletions(-)
> >
>
> LGTM
>
> Maybe we should evolve it by having both rte_bus and rte_dev API for one release and mark
> the rte_dev API for attach/detach as deprecated?
Sorry for the late response.
I think that if the hotplug API is correctly designed, the rte_dev API
could indeed be deprecated and then removed. I do not see a reason right
now to keep it.
--
Gaëtan Rivet
6WIND
More information about the dev
mailing list