Re: [PATCH v6 0/8] Introducing EAL Bus-Device-Driver Model

[Shreyansh Jain <shreyansh.jain@nxp.com>]

On Monday 16 January 2017 11:57 PM, Stephen Hemminger wrote:
> On Mon, 16 Jan 2017 21:08:19 +0530
> Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>
>> Link to v5: [15]
>>
>> :: Introduction ::
>>
>> DPDK has been inherently a PCI inclined framework. Because of this, the
>> design of device tree (or list) within DPDK is also PCI inclined. A
>> non-PCI device doesn't have a way of being expressed without using hooks
>> started from EAL to PMD.
>>
>> (Check 'Version Changes' section for changes)
>>
>> :: Overview of the Proposed Changes ::
>>
>> Assuming the below graph for a computing node:
>>
>>         device A1
>>           |
>>   +==.===='==============.============+ Bus A.
>>      |                    `--> driver A11     \
>>   device A2                `-> driver A12      \______
>>                                                 |CPU |
>>                                                 /`````
>>         device B1                              /
>>           |                                   /
>>   +==.===='==============.============+ Bus B`
>>      |                    `--> driver B11
>>   device B2                `-> driver B12
>>
>>
>>  - One or more buses are connected to a CPU (or core)
>>  - One or more devices are conneted to a Bus
>>  - Drivers are running instances which manage one or more devices
>>  - Bus is responsible for identifying devices (and interrupt propogation)
>>  - Driver is responsible for initializing the device
>>
>> In [15], model assumes that rte_bus would be the base class using which
>> all the bus implementations would instantiate the objects. This patches
>> takes a much more basic approach, on the same lines of rte_device/
>> rte_driver and rte_pci_device/rte_pci_driver.
>> This is based on various review comments as well as offline (IRC)
>> discussions.
>>
>>  - rte_bus is an abstract class which includes basic methods supported by
>>    all buses.
>>  - specific implementation, for example for PCI rte_pci_bus, would extend
>>    this class to form their own buses, with their own bus specific device
>>    and driver list.
>>  -
>>
>>  +-----------------+
>>  |rte_pci_bus      |
>>  |+---------------+|
>>  ||rte_bus        ||
>>  |+---------------+|
>>  +-----------------+
>>
>> And example implementation would look like:
>>
>>   .--------------->+-------------------+
>>   |                |rte_pci_bus        |
>>   |                | +----------------+|
>>   |                | |rte_bus         <------.
>>   |                | | name           ||     |
>>   |                | | scan           ||     |
>>   |                | | probe          ||     |
>>   |                | | attach         ||     |
>>   |                | | detach         ||     |
>>   |                | +----------------+|     |
>>   |                | pci_driver_list   |     |
>>   |  .-------------->pci_device_list   |     |
>>   |  |             | ...               |     |
>>   |  |             +-------------------+     |
>>   |  |                                       |
>>   |  +-------------------+                   |
>>   |  |rte_pci_device     |                   |
>>   '----bus               |                   |
>>      | +----------------+|                   |
>>      | |rte_device      ||                   |
>>      | | bus --------------------------------'
>>      | | ...            ||
>>      | +----------------+|
>>      | ...               |
>>      +-------------------+
>>
>>
>> :: Brief about Patch Layout ::
>>
>> 0001~0002: Introducing the basic Bus model and associated test case
>> 0003     : Split the PCI match into a separate function
>> 0004~0005: Introduce bus->scan APIs and integrate with EAL
>> 0006     : Update the Bus and PCI test cases for scanning/probing
>> 0007     : Enable PCI bus and remove code for direct PCI scan/probe from
>>            EAL
>> 0008     : Add device hotplugging over the bus and introduce PCI helpers
>>
>>
>> :: Pending Changes/Caveats ::
>>
>> 1. This patchset only moves the PCI into a bus. And, that movement is also
>>    currently part of the EAL (lib/librte_eal/linux)
>>    Eventual aim is the PCI bus reside in driver/bus/pci
>>
>> 2. Though the implementation for bus is common for Linux and BSD, the PCI
>>    bus implementation has been done/tested only for Linux.
>>
>> 3. RTE_REGISTER_BUS has been declared with contructor priority of 101
>>     It is important that Bus is registered *before* drivers are registered.
>>     Only way I could find to assure that was via
>>     __attribute(contructor(priority)) of GCC. I am not sure how it would
>>     behave on other compilers. Any suggestions?
>>     - One suggestion from David Marchand was to use global bus object
>>       handles, which I have not implemented for now. If that is common
>>       choice, I will change in v3.
>>
>> :: ToDo list ::
>>
>>  - Bump to librte_eal version
>>  - Documentation continues to have references to some _old_ PCI symbols
>>
>> :: References ::
>>
>> [1] http://dpdk.org/ml/archives/dev/2016-November/050186.html
>> [2] http://dpdk.org/ml/archives/dev/2016-November/050622.html
>> [3] http://dpdk.org/ml/archives/dev/2016-November/050416.html
>> [4] http://dpdk.org/ml/archives/dev/2016-November/050567.html
>> [5] http://dpdk.org/ml/archives/dev/2016-November/050628.html
>> [6] http://dpdk.org/ml/archives/dev/2016-November/050415.html
>> [7] http://dpdk.org/ml/archives/dev/2016-November/050443.html
>> [8] http://dpdk.org/ml/archives/dev/2016-November/050624.html
>> [9] http://dpdk.org/ml/archives/dev/2016-November/050296.html
>> [10] http://dpdk.org/ml/archives/dev/2016-December/051349.html
>> [12] http://dpdk.org/ml/archives/dev/2016-December/052092.html
>> [13] http://dpdk.org/ml/archives/dev/2016-December/052381.html
>> [14] http://dpdk.org/ml/archives/dev/2016-December/053302.html
>> [15] http://dpdk.org/ml/archives/dev/2016-December/053315.html
>>
>> :: Version Changes ::
>> v6:
>>  - Rearchitecture to bring bus object parallel to rte_device/driver
>>    This majorly includes:
>>    -- rte_pci_bus class and pci_bus as its object for PCI
>>    -- bus->attach/detach (hotplugging)
>>    -- removing bus->match as that is local to an implementation
>>  - rename symbols rte_eal_bus_* to rte_bus_*
>>  - restructuring patches (order) for simplicity
>>  - update to test_pci
>>
>> v5:
>>  - Fix checkpatch error in Patch 0003
>>
>> v4:
>>  - rebase over master (eac901ce)
>>  - Fix a bug in test_bus while setup and cleanup
>>  - rename rte_eal_get_bus to rte_eal_bus_get
>>  - Add helper (iterator) macros for easy bus,device,driver traversal
>>  - removed 0001 patch as it is already merged in master
>>  - fix missing rte_eal_bus_insert_device symbol in map file
>>
>> v3:
>>  - rebase over master (c431384c8f)
>>  - revert patch 0001 changes for checkpatch (container_of macro)
>>  - qat/rte_qat_cryptodev update for rte_driver->probe
>>  - test_pci update for using a test_pci_bus for verification
>>  - some bug fixes based on internal testing.
>>  -- rte_eal_dev_attach not handling devargs
>>  -- blacklisting not working
>>
>> v2:
>>  - No more bus->probe()
>>    Now, rte_eal_bus_probe() calls rte_driver->probe based on match output
>>  - new functions, rte_eal_pci_probe and rte_eal_pci_remove have been
>>    added as glue code between PCI PMDs and PCI Bus
>>    `-> PMDs are updated to use these new functions as callbacks for
>>        rte_driver
>>  - 'default' keyword has been removed from match and scan
>>  - Fix for incorrect changes in mlx* and nicvf*
>>  - Checkpatch fixes
>>  - Some variable checks have been removed from internal functions;
>>    functions which are externally visible continue to have such checks
>>  - Some rearrangement of patches:
>>    -- changes to drivers have been separated from EAL changes (but this
>>       does make PCI PMDs non-working for a particular patch)
>>
>> Shreyansh Jain (8):
>>   eal/bus: introduce bus abstraction
>>   test: add basic bus infrastructure tests
>>   pci: split match and probe function
>>   eal/bus: support for scanning of bus
>>   eal: introduce bus scan and probe in EAL
>>   test: update bus and pci unit test cases
>>   eal: enable PCI bus
>>   eal: enable hotplugging of devices on bus
>>
>>  app/test/Makefile                               |   2 +-
>>  app/test/test.h                                 |   2 +
>>  app/test/test_bus.c                             | 686 ++++++++++++++++++++++++
>>  app/test/test_pci.c                             | 164 ++++--
>>  lib/librte_eal/bsdapp/eal/Makefile              |   1 +
>>  lib/librte_eal/bsdapp/eal/eal.c                 |  13 +-
>>  lib/librte_eal/bsdapp/eal/eal_pci.c             |  13 +
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  15 +-
>>  lib/librte_eal/common/Makefile                  |   2 +-
>>  lib/librte_eal/common/eal_common_bus.c          | 140 +++++
>>  lib/librte_eal/common/eal_common_dev.c          |  56 +-
>>  lib/librte_eal/common/eal_common_pci.c          | 330 ++++++++----
>>  lib/librte_eal/common/eal_private.h             |  10 -
>>  lib/librte_eal/common/include/rte_bus.h         | 206 +++++++
>>  lib/librte_eal/common/include/rte_dev.h         |   1 +
>>  lib/librte_eal/common/include/rte_pci.h         | 161 +++++-
>>  lib/librte_eal/linuxapp/eal/Makefile            |   1 +
>>  lib/librte_eal/linuxapp/eal/eal.c               |  13 +-
>>  lib/librte_eal/linuxapp/eal/eal_pci.c           |  53 +-
>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  15 +-
>>  20 files changed, 1646 insertions(+), 238 deletions(-)
>>  create mode 100644 app/test/test_bus.c
>>  create mode 100644 lib/librte_eal/common/eal_common_bus.c
>>  create mode 100644 lib/librte_eal/common/include/rte_bus.h
>>
>
>
> All this is excellent, but doesn't go far enough to break the assumptions
> of 'struct eth_driver'
>

That true. But, this is the first step. Next would be:
  - unlinking eth_device from pci_device (already upstream by Jan)
  - unlinking eth_driver from pci_driver dependency
  - Same steps for cryptodev (I will pick this next)
  - moving buses to drivers/bus

And some more minor changes making PCI just one bus exposing services; 
eth_device/eth_driver being just a type of device provided by DPDK 
without any linkage to bus actual device belongs to.