[dpdk-dev] [RFC PATCH 0/6] Restructure EAL device model for bus support
Jan Blunck
jblunck at infradead.org
Thu Nov 17 12:55:44 CET 2016
On Thu, Nov 17, 2016 at 6:29 AM, Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> 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.
>
> With this cover letter, some patches are presented which try to break this
> strict linkage of EAL with PCI devices. Aim is to generalize the device
> hierarchy on the lines of how Linux handles it:
>
> device A1
> |
> +==.===='==============.============+ Bus A.
> | `--> driver A11 \
> device A2 `-> driver A12 \______
> |CPU |
> /`````
> device A1 /
> | /
> +==.===='==============.============+ Bus A`
> | `--> driver B11
> device A2 `-> driver B12
>
> Simply put:
> - a bus is connect to CPU (or core)
> - devices are conneted to 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
>
> (*Reusing text from email [1])
> In context of DPDK EAL:
> - a generic bus (not a driver, not a device). I don't know how to categorize
> a bus. It is certainly not a device, and then handler for a bus (physical)
> can be considered a 'bus driver'. So, just 'rte_bus'.
> - there is a bus for each physical implementation (or virtual). So, a rte_bus
> Object for PCI, VDEV, ABC, DEF and so on.
> - Buses are registered just like a PMD - RTE_PMD_BUS_REGISTER()
> - Each registered bus is part of a doubly list.
> -- Each device inherits rte_bus
> -- Each driver inherits rte_bus
> -- Device and Drivers lists are part of rte_bus
> - eth_driver is no more required - it was just a holder for PMDs to register
> themselves. It can be replaced with rte_xxx_driver and corresponding init/
> uninit moved to rte_driver
> - rte_eth_dev modified to disassociate itself from rte_pci_device and connect
> to generic rte_device
>
> Once again, improvising from [1]:
>
> __ rte_bus_list
> /
> +----------'---+
> |rte_bus |
> | driver_list------> List of rte_bus specific
> | device_list---- devices
> | scan | `-> List of rte_bus associated
> | match | drivers
> | dump |
> | ..some refcnt| (#)
> +--|------|----+
> _________/ \_________
> +--------/----+ +-\---------------+
> |rte_device | |rte_driver |
> | rte_bus | | rte_bus |
> | rte_driver |(#) | init |
> | | | uninit |
> | devargs | | dev_private_size|
> +---||--------+ | drv_flags |(#)
> || | intr_handle(2*) |(#)
> | \ +----------\\\----+
> | \_____________ \\\
> | \ |||
> +------|---------+ +----|----------+ |||
> |rte_pci_device | |rte_xxx_device | (4*) |||
> | PCI specific | | xxx device | |||
> | info (mem,) | | specific fns | / | \
> +----------------+ +---------------+ / | \
> _____________________/ / \
> / ___/ \
> +-------------'--+ +------------'---+ +--'------------+
> |rte_pci_driver | |rte_vdev_driver | |rte_xxx_driver |
> | PCI id table, | | <probably, | | .... |
> | other driver | | nothing> | +---------------+
> | data | +----------------+
> +----------------+
>
> (1*) Problem is that probe functions have different arguments. So,
> generalizing them might be some rework in the respective device
> layers
> (2*) Interrupt handling for each driver type might be different. I am not
> sure how to generalize that either. This is grey area for me.
> (3*) Probably exposing a bitmask for device capabilities. Nothing similar
> exists now to relate it. Don't know if that is useful. Allowing
> applications to question a device about what it supports and what it
> doesn't - making it more flexible at application layer (but more code
> as well.)
> (4*) Even vdev would be an instantiated as a device. It is not being done
> currently.
> (#) Items which are still pending
>
> With this cover letter, some patches have been posted. These are _only_ for
> discussion purpose. They are not complete and neither compilable.
> All the patches, except 0001, have sufficient context about what the changes
> are and rationale for same. Obviously, code is best answer.
>
> === Patch description: ===
>
> Patch 0001: Introduce container_of macro. Originally a patch from Jan.
>
> Patch 0002: introduce changes to rte_device/rte_driver for rte_bus, and
> rte_bus definition itself.
>
> Patch 0003: Add a new layer for 'bus driver' with linuxapp PCI as an example
>
> Patch 0004: Changes with respect to rte_bus APIs and impact on eal_common_pci
>From my point of view it is beneficial to keep the PCI support in one
place. Moving the match() and scan()
to the drivers directory doesn't seem to be technically required but
it makes the code harder to read and understand.
> Patch 0005: Change to rte_eal_init (of linuxapp only, for now) for supporting
> bus->scan. Probe is still being done old way, but in a new wrapper
>
> Patch 0006: eth_driver removal and corresponding changes in ixgbe_ethdev, as
> an example. Includes changes to rte_ethdev to remove most possible
> PCI references. But, work still remains.
Making rte_ethdev independent from PCI device is not directly related
to the rest of the patches that add bus support. I think it makes
sense to handle this separately because of the impact of refactoring
rte_ethdev.
>
> === Pending Items/Questions: ===
>
> - Interrupt and notification handling. How to allow drivers to be notified
> of presence/plugging of a device.
> - Placement of bus driver/handling code. librte_bus, bus/, drivers/bus?
> -- Also from a pespective of a external library and whether symbols would be
> available in that.
> -- and secondary processes
> - VDEV bus is missing from current set.
> - Locking of list for supporting hotplugging. Or, at the least safe add/
> remove
> - PMDINFOGEN support or lack of it.
> - Is there ever a case where rte_eth_dev needs to be resolved from
> rte_pci_device? I couldn't find any such use and neither a use-case for it.
> - There should be a way for Bus APIs to return a generic list handle so that
> EAL doesn't need to bother about bus->driver_list like dereferencing. This
> is untidy as well as less portable (in terms of code movement, not arch).
> - Are more helper hooks required for a bus?
> -- I can think of scan, match, dump, find, plug (device), unplug (device),
> associate (driver), disassociate (driver). But, most of the work is
> already being done by lower instances (rte_device/driver etc).
>
> Further:
> - In next few days I will make all necessary changes on the lines mentioned
> in the patches. This would include changing the drivers/* and librte_eal/*
> - As an when review comments float in and agreement reached, I will keep
> changing the model
> - There are grey areas like interrupt, notification, locking of bus/list
> which require more discussion. I will try and post a rfc for those as well
> or if someone can help me on those - great
As already hinted on IRC I'm taking a look at the interrupt usage of ethdev.
> - Change would include PCI bus and VDEV bus handling. A new bus (NXP's FSLMC)
> would also be layered over this series to verify the model of 'bus
> registration'. This is also part of 17.02 roadmap.
>
> [1] http://dpdk.org/ml/archives/dev/2016-November/050186.html
>
> Jan Viktorin (1):
> eal: define container macro
>
> Shreyansh Jain (5):
> eal: introduce bus-device-driver structure
> bus: add bus driver layer
> eal/common: handle bus abstraction for device/driver objects
> eal: supporting bus model in init process
> eal: removing eth_driver
>
> bus/Makefile | 36 +++
> bus/pci/Makefile | 37 +++
> bus/pci/linuxapp/pci_bus.c | 418 +++++++++++++++++++++++++++++
> bus/pci/linuxapp/pci_bus.h | 55 ++++
> drivers/net/ixgbe/ixgbe_ethdev.c | 49 ++--
> lib/librte_eal/common/eal_common_bus.c | 188 +++++++++++++
> lib/librte_eal/common/eal_common_dev.c | 31 ++-
> lib/librte_eal/common/eal_common_pci.c | 226 +++++++++-------
> lib/librte_eal/common/include/rte_bus.h | 243 +++++++++++++++++
> lib/librte_eal/common/include/rte_common.h | 18 ++
> lib/librte_eal/common/include/rte_dev.h | 36 +--
> lib/librte_eal/common/include/rte_pci.h | 11 +-
> lib/librte_eal/linuxapp/eal/Makefile | 1 +
> lib/librte_eal/linuxapp/eal/eal.c | 51 +++-
> lib/librte_eal/linuxapp/eal/eal_pci.c | 298 --------------------
> lib/librte_ether/rte_ethdev.c | 36 ++-
> lib/librte_ether/rte_ethdev.h | 6 +-
> 17 files changed, 1262 insertions(+), 478 deletions(-)
> create mode 100644 bus/Makefile
> create mode 100644 bus/pci/Makefile
> create mode 100644 bus/pci/linuxapp/pci_bus.c
> create mode 100644 bus/pci/linuxapp/pci_bus.h
> create mode 100644 lib/librte_eal/common/eal_common_bus.c
> create mode 100644 lib/librte_eal/common/include/rte_bus.h
>
> --
> 2.7.4
>
More information about the dev
mailing list