[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
Declan Doherty
declan.doherty at intel.com
Fri Sep 9 18:11:36 CEST 2016
On 07/09/16 15:07, Shreyansh Jain wrote:
> Based on master (e22856313)
>
> Background:
> ===========
>
> It includes two different patch-sets floated on ML earlier:
> * Original patch series is from David Marchand [1], [2].
> `- This focused mainly on PCI (PDEV) part
> `- v7 of this was posted by me [8] in August/2016
> * Patch series [4] from Jan Viktorin
> `- This focused on VDEV and rte_device integration
>
> Introduction:
> =============
>
> This patch series introduces a generic device model, moving away from PCI
> centric code layout. Key change is to introduce rte_driver/rte_device
> structures at the top level which are inherited by
> rte_XXX_driver/rte_XXX_device - where XXX belongs to {pci, vdev, soc (in
> future),...}.
>
> Key motivation for this series is to move away from PCI centric design of
> EAL to a more hierarchical device model - pivoted around a generic driver
> and device. Each specific driver and device can inherit the common
> properties of the generic set and build upon it through driver/device
> specific functions.
>
> Earlier, the EAL device initialization model was:
> (Refer: [3])
>
> --
> Constructor:
> |- PMD_DRIVER_REGISTER(rte_driver)
> `- insert into dev_driver_list, rte_driver object
>
> rte_eal_init():
> |- rte_eal_pci_init()
> | `- scan and fill pci_device_list from sysfs
> |
> |- rte_eal_dev_init()
> | `- For each rte_driver in dev_driver_list
> | `- call the rte_driver->init() function
> | |- PMDs designed to call rte_eth_driver_register(eth_driver)
> | |- eth_driver have rte_pci_driver embedded in them
> | `- rte_eth_driver_register installs the
> | rte_pci_driver->devinit/devuninit callbacks.
> |
> |- rte_eal_pci_probe()
> | |- For each device detected, dev_driver_list is parsed and matching is
> | | done.
> | |- For each matching device, the rte_pci_driver->devinit() is called.
> | |- Default map is to rte_eth_dev_init() which in turn creates a
> | | new ethernet device (eth_dev)
> | | `- eth_drv->eth_dev_init() is called which is implemented by
> `--| individual PMD drivers.
>
> --
>
> The structure of driver looks something like:
>
> +------------+ ._____.
> | rte_driver <-----| PMD |___
> | .init | `-----` \
> +----.-------+ | \
> `-. | What PMD actually is
> \ | |
> +----------v----+ |
> | eth_driver | |
> | .eth_dev_init | |
> +----.----------+ |
> `-. |
> \ |
> +------------v---+
> | rte_pci_driver |
> | .pci_devinit |
> +----------------+
>
> and all devices are part of a following linked lists:
> - dev_driver_list for all rte_drivers
> - pci_device_list for all devices, whether PCI or VDEV
>
>
> From the above:
> * a PMD initializes a rte_driver, eth_driver even though actually it is a
> pci_driver
> * initialization routines are passed from rte_driver->pci_driver->eth_driver
> even though they should ideally be rte_eal_init()->rte_pci_driver()
> * For a single driver/device type model, this is not necessarily a
> functional issue - but more of a design language.
> * But, when number of driver/device type increase, this would create problem
> in how driver<=>device links are represented.
>
> Proposed Architecture:
> ======================
>
> A nice representation has already been created by David in [3]. Copying that
> here:
>
> +------------------+ +-------------------------------+
> | | | |
> | rte_pci_device | | rte_pci_driver |
> | | | |
> +-------------+ | +--------------+ | | +---------------------------+ |
> | | | | | | | | | |
> | rte_eth_dev +---> rte_device +-----> rte_driver | |
> | | | | char name[] | | | | char name[] | |
> +-------------+ | | | | | | int init(rte_device *) | |
> | +--------------+ | | | int uninit(rte_device *) | |
> | | | | | |
> +------------------+ | +---------------------------+ |
> | |
> +-------------------------------+
>
> - for ethdev on top of vdev devices
>
> +------------------+ +-------------------------------+
> | | | |
> | drv specific | | rte_vdev_driver |
> | | | |
> +-------------+ | +--------------+ | | +---------------------------+ |
> | | | | | | | | | |
> | rte_eth_dev +---> rte_device +-----> rte_driver | |
> | | | | char name[] | | | | char name[] | |
> +-------------+ | | | | | | int init(rte_device *) | |
> | +--------------+ | | | int uninit(rte_device *) | |
> | | | | | |
> +------------------+ | +---------------------------+ |
> | |
> | int priv_size |
> | |
> +-------------------------------+
>
> Representing from above, it would be:
>
> +--------------+
> | rte_driver |
> | name |
> | <Future> |
> +------^-------+ pci_driver_list
> | / vdev_driver_list
> `---. <<Inherits>> / /
> |\____________/_______ /
> | / \ /
> +-----------/-----+ +--------/---------+
> | rte_pci_driver | | rte_vdev_driver |
> | pci_devinit() | | vdev_devinit() |
> | pci_devuninit()| | vdev_devuninit()|
> | <more> | | <more> |
> +-----------------+ +------------------+
>
>
> +--------------+
> | rte_device |
> | name |
> | <Future> |
> +------^-------+ pci_device_list
> | / xxx_device_list
> `---. <<Inherits>> / /
> |\____________/________ /
> | / \ /
> +-----------/-----+ +--------/---------+
> | rte_pci_device | | rte_xxx_device |
> | <dev data> | | <dev data> |
> | <flags/intr> | | <flags/intr> |
> | <more> | | <more> |
> +-----------------+ +------------------+
>
> * Each driver type has its own structure which derives from the generic
> rte_driver structure.
> \- Each driver type maintains its own list, at the same time, rte_driver
> list also exists - so that *all* drivers can be looped on, if required.
> * Each device, associated with one or more drivers, has its own type
> derived from rte_device
> \- Each device _may_ maintain its own list (for example, in current
> implementation, vdev is not maintaining it).
>
> ==Introducing a new device/driver type implies==
> - creating their own rte_<xxx>.h file which contains the device/driver
> definitions.
> - defining the DRIVER_REGISTER_XXX helpers
>
> ==Hotplugging Support==
> - devices should be able to support attach/detach operations.
> - Earlier these functions were part of ethdev. They have been moved to eal
> to be more generic.
>
> This patch is part of larger aim to:
>
> --------------------+ <is type of>
> eth_driver (PMD) |-------------> rte_driver
> crypto_driver (PMD) | ^
> <more in future> | |
> --------------------+ | <inherits>
> /
> +-----------------------/+
> | rte_pci_driver |
> | rte_vdev_driver |
> | rte_soc_driver |
> | rte_xxx_driver |
>
> Where PMD devices (rte_eth_dev, rte_cryptodev_dev) are connected to their
> drivers rather than explicitly inheriting type specific driver (PCI, etc).
>
>
....
>
> Notes:
> ======
>
> * Some sign-off were already provided by Jan on the original v5; But, as a
> large number of merges have been made, I am leaving those out just in case
> it is not sync with initial understanding.
>
> * This might not be in-sync with pmdinfogen as PMD_REGISTER_DRIVER is
> removed [7].
>
> Future Work/Pending:
> ===================
> - Presently eth_driver, rte_eth_dev are not aligned to the rte_driver/
> rte_device model. eth_driver still is a PCI specific entity. This
> has been highlighted by comments from Ferruh in [9].
> - cryptodev driver too still remains to be normalized over the rte_driver
> model
> - Some variables, like drv_name (as highlighted by Ferruh), are getting
> duplicated across rte_xxx_driver/device and rte_driver/device.
>
....
>
Hey Shreyansh,
I got some time over the last couple of days to got through this
patchset and your soc changes, and it's really a great clean up of the
driver and device infrastructure code.
I guess I'm coming to this party a bit late but I have some thoughts on
the the object model that you've outlined it improves things a huge
amount but I wonder does it go far enough.
I think that currently the coupling of the rte_driver to instances of
rte_pci_driver, rte_vdev_driver etc still really clouds the abstraction.
I think if we introduced a rte_bus and rte_bus_data abstractions with
all rte_drivers having a rte_bus instance then we could further simplify
the driver and device management, I think this would greatly reduce the
number of driver/device APIs in the EAL as I think most bus
functionality could be abstracted into a simpler API. This would also
make it possible to create new driver and device types and allowing them
to be plug-able components into DPDK and which I don't think would work
currently even with the changes included here.
http://imgur.com/a/3b7XM
The link above is to a png of a visio which outlines the object model
that I'm thinking of. The main extension from your current patchset
would be that rte_eth_driver and rte_crypto_driver would now always be
inheriting from rte_driver and not rte_soc_driver/rte_pci_driver, etv
but the rte_driver object would have rte_bus instance which could be of
type rte_bus_pci/ rte_bus_virtual/ rte_bus_soc etc. This I think would
allow the EAL to be written in such a way that it only every operates on
the base object types rte_device/rte_driver directly never the
derivative objects, like the rte_vdev_driver, rte_pci_driver. Also I
prefer the abstraction that a driver has a bus rather than it is the
bus. Also I think would be the devices layer to written driver/bus
agnostic, so the eth_dev layer shouldn't know if and underlying PMD is
PCI/memory based or virtual. Only the poll mode driver itself, and the
part of the EAL which manages driver initialization should have
knowledge of the drivers bus type.
Anyway have a look, and let me know what you think.
Thanks
Declan
More information about the dev
mailing list