[dpdk-dev] Clarification for eth_driver changes

[dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

Hello David, list,

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.
3. Similarly, rte_eth_dev has rte_pci_device which should be replaced 
with rte_device.

This is what the current outline of eth_driver is:

+------------------------+
| eth_driver             |
| +---------------------+|
| | rte_pci_driver      ||
| | +------------------+||
| | | rte_driver       |||
| | |  name[]          |||
| | |  ...             |||
| | +------------------+||
| |  .probe             ||
| |  .remove            ||
| |  ...                ||
| +---------------------+|
|  .eth_dev_init         |
|  .eth_dev_uninit       |
+------------------------+

This is what I was thinking:

+---------------------+        +----------------------+
| rte_pci_driver      |        |eth_driver            |
| +------------------+|       _|_struct rte_driver *p |
| | rte_driver       <-------/ | .eth_dev_init        |
| |  ...             ||        | .eth_dev_uninit      |
| |  name            ||        +----------------------+
| |  <more>          ||
| +------------------+|
|  <PCI specific info>|
+---------------------+

::Impact::
Various drivers use the rte_pci_driver embedded in the eth_driver object 
for device initialization.
  == They assume that rte_pci_driver is directly embedded and hence 
simply dereference.
  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file

With the above change, such drivers would have to access rte_driver and 
then perform container_of to obtain their respective rte_xxx_driver.
  == this would be useful in case there is a non-PCI driver

::Problem::
I am not sure of reason as to why eth_driver embedded rte_pci_driver in 
first place - other than a convenient way to define it before PCI driver 
registration.

As all the existing PMDs are impacted - am I missing something here in 
making the above change?

Probably, similar is the case for rte_eth_dev.

-
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[Jianbo Liu]

On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> Hello David, list,
>
> 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.
> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
> rte_device.
>
> This is what the current outline of eth_driver is:
>
> +------------------------+
> | eth_driver             |
> | +---------------------+|
> | | rte_pci_driver      ||
> | | +------------------+||
> | | | rte_driver       |||
> | | |  name[]          |||
> | | |  ...             |||
> | | +------------------+||
> | |  .probe             ||
> | |  .remove            ||
> | |  ...                ||
> | +---------------------+|
> |  .eth_dev_init         |
> |  .eth_dev_uninit       |
> +------------------------+
>
> This is what I was thinking:
>
> +---------------------+        +----------------------+
> | rte_pci_driver      |        |eth_driver            |
> | +------------------+|       _|_struct rte_driver *p |
> | | rte_driver       <-------/ | .eth_dev_init        |
> | |  ...             ||        | .eth_dev_uninit      |
> | |  name            ||        +----------------------+
> | |  <more>          ||
> | +------------------+|
> |  <PCI specific info>|
> +---------------------+
>
> ::Impact::
> Various drivers use the rte_pci_driver embedded in the eth_driver object for
> device initialization.
>  == They assume that rte_pci_driver is directly embedded and hence simply
> dereference.
>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>
> With the above change, such drivers would have to access rte_driver and then
> perform container_of to obtain their respective rte_xxx_driver.
>  == this would be useful in case there is a non-PCI driver
>
> ::Problem::
> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> first place - other than a convenient way to define it before PCI driver
> registration.
>
> As all the existing PMDs are impacted - am I missing something here in
> making the above change?
>

How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
Maybe you need to add a type/flag in rte_driver.

> Probably, similar is the case for rte_eth_dev.
>
> -
> Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[Thomas Monjalon]

2016-11-10 15:51, Jianbo Liu:
> On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> > This is what the current outline of eth_driver is:
> >
> > +------------------------+
> > | eth_driver             |
> > | +---------------------+|
> > | | rte_pci_driver      ||
> > | | +------------------+||
> > | | | rte_driver       |||
> > | | |  name[]          |||
> > | | |  ...             |||
> > | | +------------------+||
> > | |  .probe             ||
> > | |  .remove            ||
> > | |  ...                ||
> > | +---------------------+|
> > |  .eth_dev_init         |
> > |  .eth_dev_uninit       |
> > +------------------------+
> >
> > This is what I was thinking:
> >
> > +---------------------+        +----------------------+
> > | rte_pci_driver      |        |eth_driver            |
> > | +------------------+|       _|_struct rte_driver *p |
> > | | rte_driver       <-------/ | .eth_dev_init        |
> > | |  ...             ||        | .eth_dev_uninit      |
> > | |  name            ||        +----------------------+
> > | |  <more>          ||
> > | +------------------+|
> > |  <PCI specific info>|
> > +---------------------+
> >
> > ::Impact::
> > Various drivers use the rte_pci_driver embedded in the eth_driver object for
> > device initialization.
> >  == They assume that rte_pci_driver is directly embedded and hence simply
> > dereference.
> >  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
> >
> > With the above change, such drivers would have to access rte_driver and then
> > perform container_of to obtain their respective rte_xxx_driver.
> >  == this would be useful in case there is a non-PCI driver
> >
> > ::Problem::
> > I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> > first place - other than a convenient way to define it before PCI driver
> > registration.
> >
> > As all the existing PMDs are impacted - am I missing something here in
> > making the above change?
> >
> 
> How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
> Maybe you need to add a type/flag in rte_driver.

Why do you need any bus information at ethdev level?

Re: [dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
> 2016-11-10 15:51, Jianbo Liu:
>> On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>> This is what the current outline of eth_driver is:
>>>
>>> +------------------------+
>>> | eth_driver             |
>>> | +---------------------+|
>>> | | rte_pci_driver      ||
>>> | | +------------------+||
>>> | | | rte_driver       |||
>>> | | |  name[]          |||
>>> | | |  ...             |||
>>> | | +------------------+||
>>> | |  .probe             ||
>>> | |  .remove            ||
>>> | |  ...                ||
>>> | +---------------------+|
>>> |  .eth_dev_init         |
>>> |  .eth_dev_uninit       |
>>> +------------------------+
>>>
>>> This is what I was thinking:
>>>
>>> +---------------------+        +----------------------+
>>> | rte_pci_driver      |        |eth_driver            |
>>> | +------------------+|       _|_struct rte_driver *p |
>>> | | rte_driver       <-------/ | .eth_dev_init        |
>>> | |  ...             ||        | .eth_dev_uninit      |
>>> | |  name            ||        +----------------------+
>>> | |  <more>          ||
>>> | +------------------+|
>>> |  <PCI specific info>|
>>> +---------------------+
>>>
>>> ::Impact::
>>> Various drivers use the rte_pci_driver embedded in the eth_driver object for
>>> device initialization.
>>>  == They assume that rte_pci_driver is directly embedded and hence simply
>>> dereference.
>>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>>>
>>> With the above change, such drivers would have to access rte_driver and then
>>> perform container_of to obtain their respective rte_xxx_driver.
>>>  == this would be useful in case there is a non-PCI driver
>>>
>>> ::Problem::
>>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
>>> first place - other than a convenient way to define it before PCI driver
>>> registration.
>>>
>>> As all the existing PMDs are impacted - am I missing something here in
>>> making the above change?
>>>
>>
>> How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
>> Maybe you need to add a type/flag in rte_driver.
>
> Why do you need any bus information at ethdev level?
>

AFAIK, we don't need it. Above text is not stating anything on that 
grounds either, I think. Isn't it?

-
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[Thomas Monjalon]

2016-11-10 14:12, Shreyansh Jain:
> On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
> > 2016-11-10 15:51, Jianbo Liu:
> >> On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >>> This is what the current outline of eth_driver is:
> >>>
> >>> +------------------------+
> >>> | eth_driver             |
> >>> | +---------------------+|
> >>> | | rte_pci_driver      ||
> >>> | | +------------------+||
> >>> | | | rte_driver       |||
> >>> | | |  name[]          |||
> >>> | | |  ...             |||
> >>> | | +------------------+||
> >>> | |  .probe             ||
> >>> | |  .remove            ||
> >>> | |  ...                ||
> >>> | +---------------------+|
> >>> |  .eth_dev_init         |
> >>> |  .eth_dev_uninit       |
> >>> +------------------------+
> >>>
> >>> This is what I was thinking:
> >>>
> >>> +---------------------+        +----------------------+
> >>> | rte_pci_driver      |        |eth_driver            |
> >>> | +------------------+|       _|_struct rte_driver *p |
> >>> | | rte_driver       <-------/ | .eth_dev_init        |
> >>> | |  ...             ||        | .eth_dev_uninit      |
> >>> | |  name            ||        +----------------------+
> >>> | |  <more>          ||
> >>> | +------------------+|
> >>> |  <PCI specific info>|
> >>> +---------------------+
> >>>
> >>> ::Impact::
> >>> Various drivers use the rte_pci_driver embedded in the eth_driver object for
> >>> device initialization.
> >>>  == They assume that rte_pci_driver is directly embedded and hence simply
> >>> dereference.
> >>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
> >>>
> >>> With the above change, such drivers would have to access rte_driver and then
> >>> perform container_of to obtain their respective rte_xxx_driver.
> >>>  == this would be useful in case there is a non-PCI driver
> >>>
> >>> ::Problem::
> >>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
> >>> first place - other than a convenient way to define it before PCI driver
> >>> registration.
> >>>
> >>> As all the existing PMDs are impacted - am I missing something here in
> >>> making the above change?
> >>>
> >>
> >> How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
> >> Maybe you need to add a type/flag in rte_driver.
> >
> > Why do you need any bus information at ethdev level?
> 
> AFAIK, we don't need it. Above text is not stating anything on that 
> grounds either, I think. Isn't it?

No, I was replying to Jianbo.
Anyway, David made a more interesting comment.

Re: [dpdk-dev] Clarification for eth_driver changes

[Jianbo Liu]

Hi Thomas,

On 10 November 2016 at 16:58, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 2016-11-10 14:12, Shreyansh Jain:
>> On Thursday 10 November 2016 01:33 PM, Thomas Monjalon wrote:
>> > 2016-11-10 15:51, Jianbo Liu:
>> >> On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> >>> This is what the current outline of eth_driver is:
>> >>>
>> >>> +------------------------+
>> >>> | eth_driver             |
>> >>> | +---------------------+|
>> >>> | | rte_pci_driver      ||
>> >>> | | +------------------+||
>> >>> | | | rte_driver       |||
>> >>> | | |  name[]          |||
>> >>> | | |  ...             |||
>> >>> | | +------------------+||
>> >>> | |  .probe             ||
>> >>> | |  .remove            ||
>> >>> | |  ...                ||
>> >>> | +---------------------+|
>> >>> |  .eth_dev_init         |
>> >>> |  .eth_dev_uninit       |
>> >>> +------------------------+
>> >>>
>> >>> This is what I was thinking:
>> >>>
>> >>> +---------------------+        +----------------------+
>> >>> | rte_pci_driver      |        |eth_driver            |
>> >>> | +------------------+|       _|_struct rte_driver *p |
>> >>> | | rte_driver       <-------/ | .eth_dev_init        |
>> >>> | |  ...             ||        | .eth_dev_uninit      |
>> >>> | |  name            ||        +----------------------+
>> >>> | |  <more>          ||
>> >>> | +------------------+|
>> >>> |  <PCI specific info>|
>> >>> +---------------------+
>> >>>
>> >>> ::Impact::
>> >>> Various drivers use the rte_pci_driver embedded in the eth_driver object for
>> >>> device initialization.
>> >>>  == They assume that rte_pci_driver is directly embedded and hence simply
>> >>> dereference.
>> >>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>> >>>
>> >>> With the above change, such drivers would have to access rte_driver and then
>> >>> perform container_of to obtain their respective rte_xxx_driver.
>> >>>  == this would be useful in case there is a non-PCI driver
>> >>>
>> >>> ::Problem::
>> >>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
>> >>> first place - other than a convenient way to define it before PCI driver
>> >>> registration.
>> >>>
>> >>> As all the existing PMDs are impacted - am I missing something here in
>> >>> making the above change?
>> >>>
>> >>
>> >> How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
>> >> Maybe you need to add a type/flag in rte_driver.
>> >
>> > Why do you need any bus information at ethdev level?
>>
>> AFAIK, we don't need it. Above text is not stating anything on that
>> grounds either, I think. Isn't it?
>
> No, I was replying to Jianbo.
> Anyway, David made a more interesting comment.

Indeed, no need as I checked the code.
It's not even a issue if using David's design.

Thanks!
Jianbo

Re: [dpdk-dev] Clarification for eth_driver changes

[Stephen Hemminger]

I also think drv_flags should part of device not PCI. Most of the flags
there like link state support are generic. If it isn't changed for this
release will probably have to break ABI to fully support VMBUS

Re: [dpdk-dev] Clarification for eth_driver changes

[Thomas Monjalon]

Hi Stephen,

2016-11-10 02:51, Stephen Hemminger:
> I also think drv_flags should part of device not PCI. Most of the flags
> there like link state support are generic. If it isn't changed for this
> release will probably have to break ABI to fully support VMBUS

When do you plan to send VMBUS patches?
Could you send a deprecation notice for this change?
Are you aware of the work started by Shreyansh to have a generic bus model?
Could you help in 17.02 timeframe to have a solid bus model?

Thanks

Re: [dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Thursday 10 November 2016 04:21 PM, Stephen Hemminger wrote:
> I also think drv_flags should part of device not PCI. Most of the flags
> there like link state support are generic. If it isn't changed for this
> release will probably have to break ABI to fully support VMBUS
>

I didn't get your point.
Currently drv_flags is in rte_pci_driver.
You intend to say that it should be moved to rte_device?

And, all the changes being discussed here are for 17.02.

-
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Thursday 10 November 2016 01:21 PM, Jianbo Liu wrote:
> On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> Hello David, list,
>>
>> 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.
>> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
>> rte_device.
>>
>> This is what the current outline of eth_driver is:
>>
>> +------------------------+
>> | eth_driver             |
>> | +---------------------+|
>> | | rte_pci_driver      ||
>> | | +------------------+||
>> | | | rte_driver       |||
>> | | |  name[]          |||
>> | | |  ...             |||
>> | | +------------------+||
>> | |  .probe             ||
>> | |  .remove            ||
>> | |  ...                ||
>> | +---------------------+|
>> |  .eth_dev_init         |
>> |  .eth_dev_uninit       |
>> +------------------------+
>>
>> This is what I was thinking:
>>
>> +---------------------+        +----------------------+
>> | rte_pci_driver      |        |eth_driver            |
>> | +------------------+|       _|_struct rte_driver *p |
>> | | rte_driver       <-------/ | .eth_dev_init        |
>> | |  ...             ||        | .eth_dev_uninit      |
>> | |  name            ||        +----------------------+
>> | |  <more>          ||
>> | +------------------+|
>> |  <PCI specific info>|
>> +---------------------+
>>
>> ::Impact::
>> Various drivers use the rte_pci_driver embedded in the eth_driver object for
>> device initialization.
>>  == They assume that rte_pci_driver is directly embedded and hence simply
>> dereference.
>>  == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file
>>
>> With the above change, such drivers would have to access rte_driver and then
>> perform container_of to obtain their respective rte_xxx_driver.
>>  == this would be useful in case there is a non-PCI driver
>>
>> ::Problem::
>> I am not sure of reason as to why eth_driver embedded rte_pci_driver in
>> first place - other than a convenient way to define it before PCI driver
>> registration.
>>
>> As all the existing PMDs are impacted - am I missing something here in
>> making the above change?
>>
>
> How do you know eth_driver->p is pointing to a rte_pci_driver or rte_soc_driver?
> Maybe you need to add a type/flag in rte_driver.

My take: PMD implementation would specify this - similar to how it is 
done now. A PCI PMD would perform a container_of(rte_pci_driver,...).
I don't think we need a differentiation here - primarily because 
generic doesn't handle the eth_driver.

>
>> Probably, similar is the case for rte_eth_dev.
>>
>> -
>> Shreyansh
>


-- 
-
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[David Marchand]

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 ?
As far as I can see, it is only a convenient wrapper for existing pci
drivers, but in the end it is just a pci_driver with ethdev context in
it that could be pushed to each existing driver.

In my initial description
http://dpdk.org/ml/archives/dev/2016-January/031390.html, what I had
in mind was only having a rte_eth_device pointing to a generic
rte_device.
If we need to invoke some generic driver ops from ethdev (I can only
see the ethdev hotplug api, maybe I missed something), then we would
go through rte_eth_device -> rte_device -> rte_driver.
The rte_driver keeps its own bus/private logic in its code, and no
need to expose a type.


> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
> rte_device.

Yes, that's the main change for me.


Thanks.

-- 
David Marchand

Re: [dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

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.

> As far as I can see, it is only a convenient wrapper for existing pci
> drivers, but in the end it is just a pci_driver with ethdev context in
> it that could be pushed to each existing driver.

Indeed. My problem (or lack of understanding) is that all PMDs rely on 
it and I don't know in what all pattern they have envisioned using this 
model of ethdev->pci_dev, the initialization sequences.
rte_device->init/uninit would settle most of those worries, I think.

>
> In my initial description
> http://dpdk.org/ml/archives/dev/2016-January/031390.html, what I had
> in mind was only having a rte_eth_device pointing to a generic
> rte_device.

Though I had read it (during the rte_device/driver series) but didn't 
remember it while posting this. I agree with your point of doing away 
with eth_driver.

> If we need to invoke some generic driver ops from ethdev (I can only
> see the ethdev hotplug api, maybe I missed something), then we would
> go through rte_eth_device -> rte_device -> rte_driver.

Agree with you.

> The rte_driver keeps its own bus/private logic in its code, and no
> need to expose a type.

Agree.

>
>
>> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with
>> rte_device.
>
> Yes, that's the main change for me.

Indeed. This is a big change. It impacts a lot of EAL code.

>
>
> Thanks.
>

Intent of this email was to know if I am missing something in assuming 
that eth_driver is actually not being used much. I will keep the 
comments from your email in mind while making changes. Thanks.

_
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[Ferruh Yigit]

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:

device -> virtual_bus -> ethernet_functionality
device -> pci_bus     -> crypto_functionality
device -> x_bus       -> y_function


what about:

create generic bus driver/device and all eal level deal with generic
bus. different buses inherit from generic bus logic

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 */
}

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;
}

struct rte_eth_device {
	struct rte_funcional_device func_dev;
	/* generic eth device */
}

struct rte_crypto_device {
	struct rte_funcional_device func_dev;
	/* generic crypto device */
}


struct rte_driver {
	char *name;
	struct rte_device *dev;
	struct rte_bus_driver *bus_drv;
	struct rte_funcional_driver *func_drv;
}

struct rte_bus_driver {
	struct rte_driver *drv;
	rte_bus_probe_t *probe(dev, drv);
	rte_bus_remove_t *remove(dev);
	/* generic bus driver */
}

struct rte_pci_driver {
	struct rte_bus_driver bus_drv;
	*id_table;
	/* generic pci bus */
}

struct rte_vdev_driver {
	struct rte_bus_driver bus_drv;
	/* generic vdev bus */
}

struct rte_funcional_driver {
	struct rte_driver *drv;
	rte_funcional_init_t *init;
	rte_funcional_uninit_t *uninit;
}

struct rte_eth_driver {
	struct rte_funcional_driver func_drv;
	/* generic eth driver */
}

struct rte_crypto_driver {
	struct rte_funcional_driver func_drv;
	/* generic crypto driver */
}

pci_scan_one()
{
	dev = create();
	pci_dev = create();
	
	dev->bus_dev = pci_dev;
	pci_dev->bus_dev.dev = dev;
	
	insert(bus_dev_list);
}

register_drv(drv)
{
	insert(bus_drv_list)
	insert(func_drv_list)
	insert(drv_list)
}

rte_eal_bus_probe()
  for bus_dev in bus_dev_list
	bus_probe_all_drivers(bus_dev)
	  for bus_drv in bus_drv_list
	    bus_probe_one_driver(bus_drv, bus_dev)
		  bus_dev->dev->drv = bus_drv->drv;
		  bus_drv->drv->dev = bus_dev->dev;
		  probe(drv, dev)

probe(drv, dev)
{
	dev->func_dev = create();
	func_dev->dev = dev;

	func_drv = drv->func_drv;
	func_drv->init(func_dev);
}

eht_init(func_dev)
{
	eth_dev = (struct rte_eth_dev)func_dev;
	drv = func_dev->dev->drv;
}

Re: [dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

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?

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
    (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.

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.

> 
> struct rte_eth_device {
> 	struct rte_funcional_device func_dev;
> 	/* generic eth device */
> }
> 
> struct rte_crypto_device {
> 	struct rte_funcional_device func_dev;
> 	/* generic crypto device */
> }
> 

I tried thinking of this breakup but I couldn't understand what common things
a rte_functional_device would contain except init/uninit (similar to what you
have also mentioned below) which can be part of rte_device itself.

> 
> struct rte_driver {
> 	char *name;
> 	struct rte_device *dev;
> 	struct rte_bus_driver *bus_drv;
> 	struct rte_funcional_driver *func_drv;
> }
> 
> struct rte_bus_driver {
> 	struct rte_driver *drv;
> 	rte_bus_probe_t *probe(dev, drv);
> 	rte_bus_remove_t *remove(dev);
> 	/* generic bus driver */
> }

I still think a bus is neither a device, nor a driver. Yes, if we draw
physical analogy, buses are indeed devices on PCB - but they are not anything
functional with respect to an application view. They exist only to provide way
for application to understand device layout.

> 
> struct rte_pci_driver {
> 	struct rte_bus_driver bus_drv;
> 	*id_table;
> 	/* generic pci bus */
> }
> 
> struct rte_vdev_driver {
> 	struct rte_bus_driver bus_drv;
> 	/* generic vdev bus */
> }
> 
> struct rte_funcional_driver {
> 	struct rte_driver *drv;
> 	rte_funcional_init_t *init;
> 	rte_funcional_uninit_t *uninit;
> }
> 
> struct rte_eth_driver {
> 	struct rte_funcional_driver func_drv;
> 	/* generic eth driver */
> }
> 
> struct rte_crypto_driver {
> 	struct rte_funcional_driver func_drv;
> 	/* generic crypto driver */
> }
> 
> pci_scan_one()
> {
> 	dev = create();
> 	pci_dev = create();
> 
> 	dev->bus_dev = pci_dev;
> 	pci_dev->bus_dev.dev = dev;
> 
> 	insert(bus_dev_list);
> }
> 
> register_drv(drv)
> {
> 	insert(bus_drv_list)
> 	insert(func_drv_list)
> 	insert(drv_list)
> }
> 
> rte_eal_bus_probe()
>   for bus_dev in bus_dev_list
> 	bus_probe_all_drivers(bus_dev)
> 	  for bus_drv in bus_drv_list
> 	    bus_probe_one_driver(bus_drv, bus_dev)
> 		  bus_dev->dev->drv = bus_drv->drv;
> 		  bus_drv->drv->dev = bus_dev->dev;
> 		  probe(drv, dev)
>

Agree.
 
> probe(drv, dev)
> {
> 	dev->func_dev = create();

In my case, it would be creating a rte_device; rte_pci_dev in case of
PCI probe, pointing back to rte_device, rte_bus of PCI type.

> 	func_dev->dev = dev;
> 
> 	func_drv = drv->func_drv;
> 	func_drv->init(func_dev);

Effectively, it would be rte_device->init();

> }
> 
> eht_init(func_dev)
> {
> 	eth_dev = (struct rte_eth_dev)func_dev;
> 	drv = func_dev->dev->drv;
> }
> 
> 

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.

__
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[Ferruh Yigit]

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

Re: [dpdk-dev] Clarification for eth_driver changes

[Shreyansh Jain]

On Monday 14 November 2016 11:08 PM, Ferruh Yigit wrote:
[...]
> 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 ..
[...]

I understand your point. It would really nice if we can achieve that 
level pluggable-ness where drivers would be able to choose a 'profile' - 
where 'profiles' are like net/crypto etc. In your text, 
profile==functionality.

Maybe once the basic model is in place, we can revisit this idea.

-
Shreyansh

Re: [dpdk-dev] Clarification for eth_driver changes

[David Marchand]

On Fri, Nov 11, 2016 at 8:16 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 11/10/2016 11:05 AM, Shreyansh Jain wrote:
>> On Thursday 10 November 2016 01:46 PM, David Marchand wrote:
>>> 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:
>
> device -> virtual_bus -> ethernet_functionality
> device -> pci_bus     -> crypto_functionality
> device -> x_bus       -> y_function

"a device is linked to a bus" (fine)
"a bus knows what a device does" (?!)
Not sure I follow you.


-- 
David Marchand