[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
Hello Shreyansh, On Wed, Sep 7, 2016 at 4:07 PM, Shreyansh Jain wrote: > This patch is part of larger aim to: > > + > eth_driver (PMD)|-> rte_driver > crypto_driver (PMD) | ^ > | | > + | > / >+---/+ >| 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~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]. > > 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. Overall, we are still missing some parts : - in my initial proposition, the rte_driver would embed the probe/remove (previsouly init/uninit) functions that would take rte_device object as arguments (and maybe we should get rid of the double lists, I am not yet convinced it is easy). - the pmdinfo stuff is broken and could be implemented for vdev, I did a quick patch that replaces the "PMD_REGISTER_DRIVER(.*)" regexp as "DRIVER_REGISTER_.*(.*)" then I added a DRIVER_EXPORT_NAME(nm, __COUNTER__) in the vdev register macro, it looks to work fine. pmdinfo is a bit too pci but I think we can go with this. - the names should go to rte_device/rte_driver objects ? -- David Marchand
[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
Hi Stephen, On Thursday 08 September 2016 10:19 PM, Stephen Hemminger wrote: [...] >> > I think yes. There are separate lists for all device types which helps >> > keep the EAL code free of type checks. But, functionally it doesn't make >> > that big a different between a common or specific list. >> > I am in favor of separate lists of each rte_xxx_device/driver type - >> > other than a global list (which is not actually being used, for now). > I was just concerned that doing bookkeeping on multiple lists creates more > possibilities > for bugs where error unwind paths don't match. > That is true. But in this particular situation (in my opinion), maintaining multiple lists helps because: 1. Device/Drivers lists are separate and no functionality except setup and tear-down operation would ever need to parse the common list - at least not during the application run-to-completion-cycle. 2. Keeping them separate helps make the respective scan/probe code cleaner. In fact, in absence of checks for device type, I think debugging would be easier as one would only need to focus on code related to attached physical/virtual device. But again, above is just my opinion and preference - if merging the lists helps, it can be done. In fact, rte_driver/rte_device are already creating a common list - just that it is not being used right now. I don't think there is any technical impact of this (except increasing type validation - but that is only in setup/tear-down path). -- Shreyansh
[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
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 |
> | | |
[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
Hi Stephen,
On Thursday 08 September 2016 12:10 AM, Stephen Hemminger wrote:
> On Wed, 7 Sep 2016 19:37:52 +0530
> 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
>>
>> +--+ +---+
[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
On Thu, 8 Sep 2016 12:40:08 +0530 Shreyansh Jain wrote: > > Overall I like to see the clean separation. > > Are you sure you removed as much as possible from PCI? > > I am not very sure of what you mean. > > If you are referring to whether all PCI PMDs have been taken care of, I > think they are. Only issue being I can't test all of them functionally. > I have some steps provided by Thomas which can help me compile test these. > > Or, if you are referring to whether PCI drivers have been completely > disconnected from existing EAL (and converted to above linkage), I think > yes. > > Key change that still remains is delinking eth_driver from PCI type and > using a more generic approach where eth_driver (or rte_eth_driver, after > name change) can be of any type - PCI, Virtual, SoC etc. > > > I wonder of global PCI device list is needed at all if you now have list of > > all devices. > > > > I think yes. There are separate lists for all device types which helps > keep the EAL code free of type checks. But, functionally it doesn't make > that big a different between a common or specific list. > I am in favor of separate lists of each rte_xxx_device/driver type - > other than a global list (which is not actually being used, for now). I was just concerned that doing bookkeeping on multiple lists creates more possibilities for bugs where error unwind paths don't match.
[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
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
[dpdk-dev] [PATCH v9 00/25] Introducing rte_driver/rte_device generalization
On Wed, 7 Sep 2016 19:37:52 +0530
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 |
> |
