[dpdk-dev] [PATCH v8 00/25] Introducing rte_driver/rte_device generalization

Shreyansh Jain shreyansh.jain at nxp.com
Fri Sep 2 07:32:25 CEST 2016


Hi Jan,

> -----Original Message-----
> From: Jan Viktorin [mailto:viktorin at rehivetech.com]
> Sent: Thursday, September 01, 2016 11:59 PM
> To: Shreyansh Jain <shreyansh.jain at nxp.com>
> Cc: dev at dpdk.org; david.marchand at 6wind.com; thomas.monjalon at 6wind.com; Hemant
> Agrawal <hemant.agrawal at nxp.com>
> Subject: Re: [PATCH v8 00/25] Introducing rte_driver/rte_device generalization
> 
> Hi Shreyansh,
> 
> I am sorry to be quiet on this thread. I am traveling in those
> two weeks and have some vacation. However, I passively follow the
> conversation. Thank you for your work so far!

No issues.
Thanks for your help.

> 
> Regards
> Jan
> 
> On Fri, 26 Aug 2016 19:26:38 +0530
> Shreyansh Jain <shreyansh.jain at nxp.com> wrote:
> 
> > Based on master (e22856313fff2)
> >
> > 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.
> >
> >
> > 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~0003 are for introducing the container_of function (so that
> >    rte_device can be obtained from rte_pci_device, for example), and
> >    removing unused code.
> >  - 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~0010 merge the crypto and pci functions for registration and
> >    naming.
> >  - Patches 0011~0014 deal with hotplugging - hotplug no more invokes scan of
> >    complete bus and has been generalized into EAl from ethdev.
> >  - Patches 0015 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 0018~0022 enable the vdev drivers to register using the
> >    DRIVER_REGISTER_* operations, and remove their rte_driver->init()
> >  - Patch 0023 enables the rte_driver registration into a common driver
> >    linked list.
> >  - Patches 0024~0025 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.
> >
> > 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].
> >
> > 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
> >
> > 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
> >
> > Shreyansh Jain (25):
> >   eal: define macro container_of
> >   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 init macros
> >   driver: init/uninit common wrappers for PCI drivers
> >   drivers: convert all pdev drivers as pci drivers
> >   driver: 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
> >   eal: extract vdev infra
> >   eal: Remove PDEV/VDEV unused code
> >   drivers: convert PMD_VDEV drivers to use rte_vdev_driver
> >   eal: move init/uninit to rte_vdev_driver
> >   eal: remove PMD_DRIVER_REGISTER and unused pmd_types
> >   eal: rte_pci.h includes rte_dev.h
> >   eal: rename and move rte_pci_resource
> >   eal/pci: inherit rte_driver by rte_pci_driver
> >   eal: call rte_eal_driver_register
> >   eal: introduce rte_device
> >   eal/pci: Create rte_device list and fallback on its members
> >
> >  app/test/test_pci.c                             |  10 +-
> >  app/test/virtual_pmd.c                          |   8 +-
> >  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       |  11 +-
> >  drivers/net/bnx2x/bnx2x_ethdev.c                |  36 +--
> >  drivers/net/bnxt/bnxt_ethdev.c                  |  17 +-
> >  drivers/net/bonding/rte_eth_bond_api.c          |   2 +-
> >  drivers/net/bonding/rte_eth_bond_pmd.c          |   9 +-
> >  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                         |  21 +-
> >  drivers/net/mlx5/mlx5.c                         |  20 +-
> >  drivers/net/mpipe/mpipe_tilegx.c                |  18 +-
> >  drivers/net/nfp/nfp_net.c                       |  28 +--
> >  drivers/net/null/rte_eth_null.c                 |  11 +-
> >  drivers/net/pcap/rte_eth_pcap.c                 |  11 +-
> >  drivers/net/qede/qede_ethdev.c                  |  42 +---
> >  drivers/net/ring/rte_eth_ring.c                 |  11 +-
> >  drivers/net/szedata2/rte_eth_szedata2.c         |  28 +--
> >  drivers/net/thunderx/nicvf_ethdev.c             |  21 +-
> >  drivers/net/vhost/rte_eth_vhost.c               |  11 +-
> >  drivers/net/virtio/virtio_ethdev.c              |  28 +--
> >  drivers/net/virtio/virtio_pci.c                 |   5 +-
> >  drivers/net/virtio/virtio_user_ethdev.c         |  10 +-
> >  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   |   7 +
> >  lib/librte_eal/common/Makefile                  |   2 +-
> >  lib/librte_eal/common/eal_common_dev.c          |  95 ++++----
> >  lib/librte_eal/common/eal_common_pci.c          |  34 ++-
> >  lib/librte_eal/common/eal_common_vdev.c         | 106 +++++++++
> >  lib/librte_eal/common/eal_private.h             |  20 +-
> >  lib/librte_eal/common/include/rte_common.h      |  16 ++
> >  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         |  53 +++--
> >  lib/librte_eal/common/include/rte_tailq.h       |   4 +-
> >  lib/librte_eal/common/include/rte_vdev.h        |  96 ++++++++
> >  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                   | 280 +++++------------------
> -
> >  lib/librte_ether/rte_ethdev.h                   |  40 ++--
> >  lib/librte_ether/rte_ether_version.map          |   9 +
> >  69 files changed, 784 insertions(+), 1020 deletions(-)
> >  create mode 100644 lib/librte_eal/common/eal_common_vdev.c
> >  create mode 100644 lib/librte_eal/common/include/rte_vdev.h
> >
> 
> 
> 
> --
>   Jan Viktorin                E-mail: Viktorin at RehiveTech.com
>   System Architect            Web:    www.RehiveTech.com
>   RehiveTech
>   Brno, Czech Republic


More information about the dev mailing list