[dpdk-dev] [PATCH v11 00/24] Introducing rte_driver/rte_device generalization
Matej Vido
vido at cesnet.cz
Wed Sep 21 21:03:51 CEST 2016
Hi Shreyansh,
I have tested this patch-set with szedata2 driver and it works fine.
Regards,
Matej
On 20.09.2016 14:41, Shreyansh Jain wrote:
> Based on master (e15922d75)
>
> 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_probe() | | vdev_devinit() |
> | pci_remove() | | 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).
>
>
> About the Patches:
> ==================
>
> There are a large number of patches for this - primarily because the changes
> are quite varied and keeping them logically separate yet compilable is
> important. Most of the patches are small and those which are large touch the
> drivers (PMDs) to accommodate the structure changes:
>
> - Patches 0001~0002 are for introducing the container_of function (so that
> rte_device can be obtained from rte_pci_device, for example), and
> removing unused code.
> - Patch 0003 converts the PCI devinit/devuninit method names to probe/
> remove and correspondingly updates all drivers where impact it. VDEV
> based init/uninit have not been modified.
> - Patches 0004~0007 just perform the ground work for enabling change from
> rte_driver/eth_driver based PMDs to rte_xxx_driver based PMDs
> - In patch 0008, all the pdev PMDs are changed to support rte_pci_driver (
> including cryptodev, which is eventually generalized with PCI)
> - Patch 0009~0011 merge the crypto and pci functions for registration and
> naming.
> - Patches 0011~0013 deal with hotplugging - hotplug no more invokes scan of
> complete bus and has been generalized into EAl from ethdev.
> - Patches 0014 introduces vdev init/uninit into separate C units and
> enables its compilation. Patch 0016~0017 build on it and remove the
> remaining legacy support for vdev/pdev distinctions.
> - Patches 0017~0021 enable the vdev drivers to register using the
> DRIVER_REGISTER_* operations, and remove their rte_driver->init()
> - Patch 0022 enables the rte_driver registration into a common driver
> linked list.
> - Patche 0024 introduce the rte_device, a generalization of
> rte_xxx_device, and associated operation of creating rte_device linked
> list. It also enables the drivers to use rte_device.name/numa_node
> members rather than rte_xxx_device specific members.
>
> 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].
> - Some variables, like drv_name (as highlighted by Ferruh), are getting
> duplicated across rte_xxx_driver/device and rte_driver/device.
>
> References:
> ===========
>
> [1] http://dpdk.org/ml/archives/dev/2016-January/032387.html
> [2] http://dpdk.org/ml/archives/dev/2016-April/037686.html
> [3] http://dpdk.org/ml/archives/dev/2016-January/031390.html
> [4] http://dpdk.org/ml/archives/dev/2016-July/043645.html
> [5] http://dpdk.org/ml/archives/dev/2016-June/042439.html
> [6] http://dpdk.org/ml/archives/dev/2016-June/042444.html
> [7] http://dpdk.org/ml/archives/dev/2016-July/043172.html
> [8] http://dpdk.org/ml/archives/dev/2016-August/044941.html
> [9] http://dpdk.org/ml/archives/dev/2016-August/045947.html
> [10] http://dpdk.org/ml/archives/dev/2016-September/046919.html
> Following are Some Review comments:
> [R1] http://dpdk.org/ml/archives/dev/2016-September/046548.html
> [R2] http://dpdk.org/ml/archives/dev/2016-September/046549.html
> [R3] http://dpdk.org/ml/archives/dev/2016-September/046550.html
> [R4] http://dpdk.org/ml/archives/dev/2016-September/046551.html
> [R5] http://dpdk.org/ml/archives/dev/2016-September/046399.html
> [R6] http://dpdk.org/ml/archives/dev/2016-September/046552.html
>
> Changes since v10:
> - Rebased over master (e15922d75)
> - Removed patch for container_of posted in v10 (01/25)
> - Fix review comments from David: [10]
>
> Changes since v9:
> - Rebased over master (58efd680d5e)
> - Fix issues reported by checkpatch and check-git-log, including changing
> headline of the patches to adhere to these scripts.
> - Corrected the patch author field
> - Renamed devinit/devuninit for pci_driver to probe/remove as per
> suggestion from David
> - Fix for PMD Info gen tool using patch from David
> - Fixed review comments [R1], some part of [R2], [R3] using patch from
> David, [R4], [R5] and some inputs from [R6].
>
> Changes since v8:
> - Some review comments from Ferruh Yigit & Reshma Pattan have been fixed.
> = Though changes in mlx4/mlx5/szedata2 have been done, I am still unable
> to verify those in absence of a proper environment at my end.
> = Comment from Ferruh for eth_driver, drv_name are not fixed yet.
> - Added a 'Future work' section in Covering letter
>
> Changes since v7:
> - Rebase over master (e22856313fff2)
> - Merge the patch series by David [1][2] and Jan [4] into a single set
> hereafter, PCI and VDEV, both are re-factored for rte_device/driver
> model
>
> Changes since v6:
> - rebase over 16.07 (b0a1419)
> - DRIVER_REGISTER_PCI macro is now passed pci_drv rather than drv
> - review comments regarding missing information in log messages
> - new API additions to 16.11 map objects
> - review comment in [5] and [7] are not included in this series.
>
> Changes since v5:
> - Rebase over master (11c5e45d8)
> - Rename RTE_EAL_PCI_REGISTER helper macro to DRIVER_REGISTER_PCI to be
> in sync with DRIVER_REGISTER_PCI_TABLE. [Probably, in future, both can
> be merged]
> - Modifications to bnxt and thunderx driver PMD registration files for
> using the simplified PCI device registration helper macro
>
> Changes since v4:
> - Fix compilation issue after rebase on HEAD (913154e) in previous series
> - Retain rte_eth_dev_get_port_by_name and rte_eth_dev_get_name_by_port
> which were removed by previous patchset. These are being used by pdump
> library
>
> Changes since v3:
> - rebase over HEAD (913154e)
> - Update arguments to RTE_EAL_PCI_REGISTER macro as per Jan's suggestion
> - modify qede driver to use RTE_EAL_PCI_REGISTER
> - Argument check in hotplug functions
>
> Changes since v2:
> - rebase over HEAD (d76c193)
> - Move SYSFS_PCI_DRIVERS macro to rte_pci.h to avoid compilation issue
>
> Changes since v1:
> - rebased on HEAD, new drivers should be okay
> - patches have been split into smaller pieces
> - RTE_INIT macro has been added, but in the end, I am not sure it is useful
> - device type has been removed from ethdev, as it was used only by hotplug
> - getting rid of pmd type in eal patch (patch 5 of initial series) has been
> dropped for now, we can do this once vdev drivers have been converted
>
> David Marchand (13):
> eal: remove duplicate function declaration
> pci: no need for dynamic tailq init
> crypto: no need for a crypto pmd type
> drivers: align PCI driver definitions
> eal: introduce PCI device init macros
> driver: probe/remove common wrappers for PCI drivers
> drivers: convert all phy drivers as PCI drivers
> drivers: remove driver register callbacks for crypto/net
> eal/pci: helpers for device name parsing/update
> ethdev: do not scan all PCI devices on attach
> eal: add hotplug operations for PCI and VDEV
> ethdev: convert to EAL hotplug
> ethdev: get rid of device type
>
> Jan Viktorin (10):
> eal: extract vdev infra
> eal: remove PDEV/VDEV unused code
> drivers: convert VDRV to use RTE VDEV Driver
> eal: remove unused PMD types
> eal: include dev headers in place of PCI headers
> eal: rename and move RTE PCI Resources
> eal/pci: inherit RTE driver in PCI driver
> eal: register EAL drivers explicitly
> eal: introduce generalized RTE device
> eal/pci: create RTE device list and fallback on its members
>
> Shreyansh Jain (1):
> eal/pci: replace PCI devinit/devuninit with probe/remove
>
> app/test/test_pci.c | 18 +-
> app/test/virtual_pmd.c | 8 +-
> doc/guides/prog_guide/dev_kit_build_system.rst | 2 +-
> drivers/crypto/aesni_gcm/aesni_gcm_pmd.c | 7 +-
> drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 7 +-
> drivers/crypto/kasumi/rte_kasumi_pmd.c | 7 +-
> drivers/crypto/null/null_crypto_pmd.c | 7 +-
> drivers/crypto/qat/qat_qp.c | 2 +-
> drivers/crypto/qat/rte_qat_cryptodev.c | 18 +-
> drivers/crypto/snow3g/rte_snow3g_pmd.c | 7 +-
> drivers/net/af_packet/rte_eth_af_packet.c | 9 +-
> drivers/net/bnx2x/bnx2x_ethdev.c | 36 +--
> drivers/net/bnx2x/bnx2x_rxtx.c | 3 +-
> drivers/net/bnxt/bnxt_ethdev.c | 17 +-
> drivers/net/bonding/rte_eth_bond_api.c | 2 +-
> drivers/net/bonding/rte_eth_bond_pmd.c | 7 +-
> drivers/net/cxgbe/cxgbe_ethdev.c | 25 +--
> drivers/net/cxgbe/cxgbe_main.c | 2 +-
> drivers/net/cxgbe/sge.c | 7 +-
> drivers/net/e1000/em_ethdev.c | 17 +-
> drivers/net/e1000/igb_ethdev.c | 41 +---
> drivers/net/ena/ena_ethdev.c | 20 +-
> drivers/net/enic/enic_ethdev.c | 24 +-
> drivers/net/fm10k/fm10k_ethdev.c | 30 +--
> drivers/net/i40e/i40e_ethdev.c | 31 +--
> drivers/net/i40e/i40e_ethdev_vf.c | 26 +--
> drivers/net/i40e/i40e_fdir.c | 2 +-
> drivers/net/ixgbe/ixgbe_ethdev.c | 48 +---
> drivers/net/mlx4/mlx4.c | 26 +--
> drivers/net/mlx5/mlx5.c | 27 +--
> drivers/net/mpipe/mpipe_tilegx.c | 14 +-
> drivers/net/nfp/nfp_net.c | 28 +--
> drivers/net/null/rte_eth_null.c | 9 +-
> drivers/net/pcap/rte_eth_pcap.c | 9 +-
> drivers/net/qede/qede_ethdev.c | 42 +---
> drivers/net/ring/rte_eth_ring.c | 9 +-
> drivers/net/szedata2/rte_eth_szedata2.c | 29 +--
> drivers/net/thunderx/nicvf_ethdev.c | 21 +-
> drivers/net/vhost/rte_eth_vhost.c | 9 +-
> drivers/net/virtio/virtio_ethdev.c | 29 +--
> drivers/net/virtio/virtio_pci.c | 5 +-
> drivers/net/virtio/virtio_user_ethdev.c | 8 +-
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 27 +--
> drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
> drivers/net/xenvirt/rte_eth_xenvirt.c | 9 +-
> examples/ip_pipeline/init.c | 22 --
> lib/librte_cryptodev/rte_cryptodev.c | 71 ++----
> lib/librte_cryptodev/rte_cryptodev.h | 2 -
> lib/librte_cryptodev/rte_cryptodev_pmd.h | 45 ++--
> lib/librte_cryptodev/rte_cryptodev_version.map | 8 +-
> lib/librte_eal/bsdapp/eal/Makefile | 1 +
> lib/librte_eal/bsdapp/eal/eal_pci.c | 54 ++++-
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 10 +
> lib/librte_eal/common/Makefile | 2 +-
> lib/librte_eal/common/eal_common_dev.c | 95 ++++----
> lib/librte_eal/common/eal_common_pci.c | 51 +++--
> lib/librte_eal/common/eal_common_vdev.c | 108 +++++++++
> lib/librte_eal/common/eal_private.h | 20 +-
> lib/librte_eal/common/include/rte_dev.h | 77 +++++--
> lib/librte_eal/common/include/rte_eal.h | 3 +
> lib/librte_eal/common/include/rte_pci.h | 62 ++++--
> lib/librte_eal/common/include/rte_tailq.h | 4 +-
> lib/librte_eal/common/include/rte_vdev.h | 97 ++++++++
> lib/librte_eal/linuxapp/eal/Makefile | 1 +
> lib/librte_eal/linuxapp/eal/eal.c | 1 +
> lib/librte_eal/linuxapp/eal/eal_pci.c | 23 +-
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 10 +
> lib/librte_ether/rte_ethdev.c | 279 +++++-------------------
> lib/librte_ether/rte_ethdev.h | 40 ++--
> lib/librte_ether/rte_ether_version.map | 10 +-
> mk/internal/rte.compile-pre.mk | 2 +-
> 71 files changed, 795 insertions(+), 1036 deletions(-)
> create mode 100644 lib/librte_eal/common/eal_common_vdev.c
> create mode 100644 lib/librte_eal/common/include/rte_vdev.h
>
More information about the dev
mailing list