From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>,
David Marchand <david.marchand@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] Clarification for eth_driver changes
Date: Mon, 14 Nov 2016 17:38:39 +0000 [thread overview]
Message-ID: <1706a89e-7aa0-6a9d-401b-7529026f8589@intel.com> (raw)
In-Reply-To: <DB5PR0401MB2054B1B4C5EC1BA8412E8B4090BA0@DB5PR0401MB2054.eurprd04.prod.outlook.com>
On 11/12/2016 5:44 PM, Shreyansh Jain wrote:
> Hello Ferruh,
>
> (Please ignore if line wrappings are not correct. Using a possibly
> unconfigured mail client).
>
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Saturday, November 12, 2016 12:46 AM
>> To: Shreyansh Jain <shreyansh.jain@nxp.com>; David Marchand
>> <david.marchand@6wind.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] Clarification for eth_driver changes
>>
>> On 11/10/2016 11:05 AM, Shreyansh Jain wrote:
>>> Hello David,
>>>
>>> On Thursday 10 November 2016 01:46 PM, David Marchand wrote:
>>>> Hello Shreyansh,
>>>>
>>>> On Thu, Nov 10, 2016 at 8:26 AM, Shreyansh Jain <shreyansh.jain@nxp.com>
>> wrote:
>>>>> I need some help and clarification regarding some changes I am doing to
>>>>> cleanup the EAL code.
>>>>>
>>>>> There are some changes which should be done for eth_driver/rte_eth_device
>>>>> structures:
>>>>>
>>>>> 1. most obvious, eth_driver should be renamed to rte_eth_driver.
>>>>> 2. eth_driver currently has rte_pci_driver embedded in it
>>>>> - there can be ethernet devices which are _not_ PCI
>>>>> - in which case, this structure should be removed.
>>>>
>>>> Do we really need to keep a eth_driver ?
>>>
>>> No. As you have rightly mentioned below (as well as in your Jan'16
>>> post), it is a mere convenience.
>>
>> Isn't it good to separate the logic related which bus device connected
>> and what functionality it provides. Because these two can be flexible:
>
> Indeed. The very idea of a Bus model is to make a hierarchy which allows
> for pluggability/flexibility in terms of devices being used. But, until now I
> have only considered placement hierarchy and not functional hierarchy. (more
> below)
>
>>
>> device -> virtual_bus -> ethernet_functionality
>> device -> pci_bus -> crypto_functionality
>> device -> x_bus -> y_function
>>
>
> Ok.
>
>>
>> what about:
>>
>> create generic bus driver/device and all eal level deal with generic
>> bus. different buses inherit from generic bus logic
>
> From what I had in mind: (and very much similar to what you have already
> mentioned in this email)
> - 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()
> -- There is a problem of making sure the constructor for bus registration is
> executed before drivers. Probably by putting the bus code within lib*
> - Each registered bus is part of a doubly list.
> - Each device inherits rte_bus
> - Each driver inherits rte_bus
> - Existing Device and Drivers lists would be moved into rte_bus
>
> (more below)
>
>>
>> create generic functionality device/driver and pmd level deal with
>> these. different functionalities inherit from generic functionality logic
>>
>> and use rte_device/rte_driver as glue logic for all these.
>>
>> This makes easy to add new bus or functionality device/drivers without
>> breaking existing logic.
>>
>>
>> Something like this:
>>
>> struct rte_device {
>> char *name;
>> struct rte_driver *drv;
>> struct rte_bus_device *bus_dev;
>> struct rte_funcional_device *func_dev;
>> *devargs
>> }
>>
>> struct rte_bus_device {
>> struct rte_device *dev;
>> /* generic bus device */
>> }
>
> This is where you lost me. From what I understood from your text:
> (CMIIW)
>
> - Most abstract class is 'rte_device'.
> - A bus is a device. So, it points to a rte_device
> - But, a rte_device belongs to a bus, so it points to a rte_bus_device.
>
> Isn't that contradictory?
What I was thinking is:
rte_device/driver are not abstract classes.
rte_bus device/driver is an abstract class and any bus inherited from
this class.
rte_func device/driver is and abstract class and eth/crypto inherited
from this class.
eal layer only deal with rte_bus
pmd's only deal with functional device/driver
but still, it is required to know device <-> driver, and functional <->
bus, relations. rte_dev/rte_driver are to provide this links.
But yes this add extra layer and with second thought I am not sure if it
is really possible to separate bus and functionality, this was just an
idea ..
>
> This is how I view the physical layout of devices on which DPDK works:
>
> +---------+ +----------+
> |Driver 1A| |Driver 2B |
> |servicing| |servicing | (*)
> |Bus A | |Bus B |
> +---------+ +----------+
> \ /
> +---------+ +-------+
> | bus A |---| bus B |--- ...
> +---------+ +-------+
> / \ \ \
> / \_ \ \
> +---------+ / / \
> | device 1| / +--------+ \
> | on Bus A| / |Device 3| |
> +---------+ / |on bus B| |
> +---------+ +--------+ |
> | device 2| +--------+
> | on Bus A| |Device 4|
> +---------+ |on bus B|
> +--------+
>
> (*) Multiple drivers servicing a Bus.
>
> Now, if we introduce the abstraction for functionality (assuming net/crypto) as
> two functionalities currently applicable:
>
> +---------+ +----------+
> |Driver 1A| |Driver 2B |
> |servicing| |servicing | (*)
> |Bus A | |Bus B |
> +---------' +---.------+
> \ /
> +---------+ +-------+
> | bus A |---| bus B |--- ...
> +---------+ +-------+
> / \ \ \
> / \_ \ \
> +---------' / / \
> | device 1| / +--------' \
> | on Bus A| / |Device 3| |
> +---------+ / |on bus B| |
> / +---------' +-|------+ |
> | | device 2| / +-------'+
> | | on Bus A| / |Device 4|
> \ +--|------+ _____/ |on bus B|
> \ \_____ / +--------+
> | .--\' /
> | / \ ___/
> +-'----'-+ +'------'+
> |Func X | |Func Y |
> |(Net) | |(Crypto)|
> +--------| +--------+
>
> So that means, a device would be a 'net' or 'crypto' device bases on the
> Functionality it attaches to.
>
> From a physical layout view, is that correct understanding of your argument?
>
>>
>> struct rte_pci_device {
>> struct rte_bus_device bus_dev;
>> /* generic pci bus */
>> }
>>
>> struct rte_vdev_device {
>> struct rte_bus_device bus_dev;
>> /* generic vdev bus */
>> }
>>
>> struct rte_funcional_device {
>> struct rte_device *dev;
>> }
>
> I understand your point of 'pluggable' functionality. It would be helpful if
> same driver would like to move between being a crypto and net. But is that a
> plausible use case for DPDK right now?
>
> To me, it seems as one more layer of redirection/abstraction.
>
> This is what the view I have as of now:
>
> __ rte_bus_list
> /
> +----------'---+
> |rte_bus |
> | driver_list |
> | device_list |
> | scan |
> | match |
> | remove |
> | ..some refcnt|
> +--|------|----+
> _________/ \_________
> +--------/----+ +-\--------------+
> |rte_device | |rte_driver |
> | rte_bus | | rte_bus |
> | flags (3*) | | probe (1*) |
> | init | | remove |
> | uninit | | ... |
> +---||--------+ | 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
probe() is now done in bus level, right? pci_probe(), vdev_probe(), ...
And I guess, eth_dev and crypto_dev are created during probe(), how
probe() knows if to create eth_dev or crypto_dev?
> (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.
I think it is good to add this to be consistent.
>
> So, unlike your model, rte_bus remains the topmost class which is neither a
> device, not a driver. It is just a class.
> Further, as specific information exists in each specific device and driver,
> that is not generalized.
>
<...>
>
> I am not against what you have stated. Creating a functional device is just
> one more layer of abstraction in my view. Mostly abstraction/classification
> makes life easier for applications to visualize underlying hierarchy. If this
> serves a purpose, I am OK with that. At least right now, I think it would
> only end up being like eth_driver which is just a holder.
I agree with you, mine was more like a brain exercise, may have gaps and
I need to think more, thanks for your comments.
Thanks,
ferruh
next prev parent reply other threads:[~2016-11-14 17:38 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-10 7:26 Shreyansh Jain
2016-11-10 7:51 ` Jianbo Liu
2016-11-10 8:03 ` Thomas Monjalon
2016-11-10 8:42 ` Shreyansh Jain
2016-11-10 8:58 ` Thomas Monjalon
2016-11-10 9:20 ` Jianbo Liu
2016-11-10 10:51 ` Stephen Hemminger
2016-11-10 11:07 ` Thomas Monjalon
2016-11-10 11:09 ` Shreyansh Jain
2016-11-10 8:38 ` Shreyansh Jain
2016-11-10 8:16 ` David Marchand
2016-11-10 11:05 ` Shreyansh Jain
2016-11-11 19:16 ` Ferruh Yigit
2016-11-12 17:44 ` Shreyansh Jain
2016-11-14 17:38 ` Ferruh Yigit [this message]
2016-11-16 5:09 ` Shreyansh Jain
2016-11-14 9:07 ` David Marchand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1706a89e-7aa0-6a9d-401b-7529026f8589@intel.com \
--to=ferruh.yigit@intel.com \
--cc=david.marchand@6wind.com \
--cc=dev@dpdk.org \
--cc=shreyansh.jain@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).