[dpdk-dev,3/7] eal: move virtual device probing into a bus
Checks
Commit Message
This is a refactoring of the virtual device probing which moves into into
a proper bus structure.
Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
lib/librte_eal/common/eal_common_dev.c | 22 -----------------
lib/librte_eal/common/eal_common_vdev.c | 44 +++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 22 deletions(-)
Comments
On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
> This is a refactoring of the virtual device probing which moves into into
> a proper bus structure.
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> ---
> lib/librte_eal/common/eal_common_dev.c | 22 -----------------
> lib/librte_eal/common/eal_common_vdev.c | 44 +++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 22 deletions(-)
>
[...]
>
> diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
> index 7d6e54f..523a3d6 100644
> --- a/lib/librte_eal/common/eal_common_vdev.c
> +++ b/lib/librte_eal/common/eal_common_vdev.c
> @@ -37,8 +37,10 @@
> #include <stdint.h>
> #include <sys/queue.h>
>
[...]
> +
> +static struct rte_bus rte_vdev_bus = {
> + .scan = vdev_scan,
> + .probe = vdev_probe,
> +};
> +
> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
>
Does it matter if VDEV buses are registered before or after other
buses? Either way, the callbacks would be called in the order specified
in EAL.
On Wed, Feb 15, 2017 at 3:11 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
>>
>
>> +
>> +static struct rte_bus rte_vdev_bus = {
>> + .scan = vdev_scan,
>> + .probe = vdev_probe,
>> +};
>> +
>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
>>
>
> Does it matter if VDEV buses are registered before or after other
> buses?
Yes, it does. Also see commit f4ce209a ("eal: postpone vdev initialization").
> Either way, the callbacks would be called in the order specified
> in EAL.
They are called in order of registration. That is why this defers the
registration of the vdev bus.
On Wednesday 15 February 2017 07:41 PM, Shreyansh Jain wrote:
> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
>> This is a refactoring of the virtual device probing which moves into into
>> a proper bus structure.
>>
>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>> ---
>> lib/librte_eal/common/eal_common_dev.c | 22 -----------------
>> lib/librte_eal/common/eal_common_vdev.c | 44
>> +++++++++++++++++++++++++++++++++
>> 2 files changed, 44 insertions(+), 22 deletions(-)
>>
>
> [...]
>
>>
>> diff --git a/lib/librte_eal/common/eal_common_vdev.c
>> b/lib/librte_eal/common/eal_common_vdev.c
>> index 7d6e54f..523a3d6 100644
>> --- a/lib/librte_eal/common/eal_common_vdev.c
>> +++ b/lib/librte_eal/common/eal_common_vdev.c
>> @@ -37,8 +37,10 @@
>> #include <stdint.h>
>> #include <sys/queue.h>
>>
> [...]
>
>> +
>> +static struct rte_bus rte_vdev_bus = {
>> + .scan = vdev_scan,
>> + .probe = vdev_probe,
>> +};
>> +
>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
>>
>
> Does it matter if VDEV buses are registered before or after other
> buses? Either way, the callbacks would be called in the order specified
> in EAL.
>
>
Just ignore this comment - I am misunderstood something.
But another question: Is there specific reason VDEV should be
registered/scanned *after* other devices? Is there some specific problem
if we do otherwise? (I think this is should be done, but I don't have a
specific reason).
On Wednesday 15 February 2017 07:43 PM, Jan Blunck wrote:
> On Wed, Feb 15, 2017 at 3:11 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
>>>
>>
>>> +
>>> +static struct rte_bus rte_vdev_bus = {
>>> + .scan = vdev_scan,
>>> + .probe = vdev_probe,
>>> +};
>>> +
>>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
>>>
>>
>> Does it matter if VDEV buses are registered before or after other
>> buses?
>
> Yes, it does. Also see commit f4ce209a ("eal: postpone vdev initialization").
>
>> Either way, the callbacks would be called in the order specified
>> in EAL.
>
> They are called in order of registration. That is why this defers the
> registration of the vdev bus.
Agree - I realized the problem in my statement after I had sent the email.
> On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>
> On Wednesday 15 February 2017 07:41 PM, Shreyansh Jain wrote:
>> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
>>> This is a refactoring of the virtual device probing which moves into into
>>> a proper bus structure.
>>>
>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>> ---
>>> lib/librte_eal/common/eal_common_dev.c | 22 -----------------
>>> lib/librte_eal/common/eal_common_vdev.c | 44
>>> +++++++++++++++++++++++++++++++++
>>> 2 files changed, 44 insertions(+), 22 deletions(-)
>>>
>>
>> [...]
>>
>>>
>>> diff --git a/lib/librte_eal/common/eal_common_vdev.c
>>> b/lib/librte_eal/common/eal_common_vdev.c
>>> index 7d6e54f..523a3d6 100644
>>> --- a/lib/librte_eal/common/eal_common_vdev.c
>>> +++ b/lib/librte_eal/common/eal_common_vdev.c
>>> @@ -37,8 +37,10 @@
>>> #include <stdint.h>
>>> #include <sys/queue.h>
>>>
>> [...]
>>
>>> +
>>> +static struct rte_bus rte_vdev_bus = {
>>> + .scan = vdev_scan,
>>> + .probe = vdev_probe,
>>> +};
>>> +
>>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
>>>
>>
>> Does it matter if VDEV buses are registered before or after other
>> buses? Either way, the callbacks would be called in the order specified
>> in EAL.
>>
>>
>
> Just ignore this comment - I am misunderstood something.
>
> But another question: Is there specific reason VDEV should be registered/scanned *after* other devices? Is there some specific problem if we do otherwise? (I think this is should be done, but I don't have a specific reason).
Does the bonding driver which uses physical devices need to be registered after physical ones? In Pktgen I noticed the vdev after the physical ports and I could not blacklist them as the bonding driver needed them, which caused the bonding ports to have a greater port number. In the case of pktgen the bonding ports were up around 8 or 10 and caused the display to not show the bonding ports. This is really just a usability problem for the developer using Pktgen. I would like to see the vdev devices first, but as long as the drivers (like bonding) are fine with them being first.
Regards,
Keith
> -----Original Message-----
> From: Wiles, Keith [mailto:keith.wiles@intel.com]
> Sent: Wednesday, February 15, 2017 7:53 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: Jan Blunck <jblunck@infradead.org>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 3/7] eal: move virtual device probing into a
> bus
>
>
> > On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >
> > On Wednesday 15 February 2017 07:41 PM, Shreyansh Jain wrote:
> >> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
> >>> This is a refactoring of the virtual device probing which moves into into
> >>> a proper bus structure.
> >>>
> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >>> ---
> >>> lib/librte_eal/common/eal_common_dev.c | 22 -----------------
> >>> lib/librte_eal/common/eal_common_vdev.c | 44
> >>> +++++++++++++++++++++++++++++++++
> >>> 2 files changed, 44 insertions(+), 22 deletions(-)
> >>>
> >>
> >> [...]
> >>
> >>>
> >>> diff --git a/lib/librte_eal/common/eal_common_vdev.c
> >>> b/lib/librte_eal/common/eal_common_vdev.c
> >>> index 7d6e54f..523a3d6 100644
> >>> --- a/lib/librte_eal/common/eal_common_vdev.c
> >>> +++ b/lib/librte_eal/common/eal_common_vdev.c
> >>> @@ -37,8 +37,10 @@
> >>> #include <stdint.h>
> >>> #include <sys/queue.h>
> >>>
> >> [...]
> >>
> >>> +
> >>> +static struct rte_bus rte_vdev_bus = {
> >>> + .scan = vdev_scan,
> >>> + .probe = vdev_probe,
> >>> +};
> >>> +
> >>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
> >>>
> >>
> >> Does it matter if VDEV buses are registered before or after other
> >> buses? Either way, the callbacks would be called in the order specified
> >> in EAL.
> >>
> >>
> >
> > Just ignore this comment - I am misunderstood something.
> >
> > But another question: Is there specific reason VDEV should be
> registered/scanned *after* other devices? Is there some specific problem if
> we do otherwise? (I think this is should be done, but I don't have a specific
> reason).
>
> Does the bonding driver which uses physical devices need to be registered
> after physical ones? In Pktgen I noticed the vdev after the physical ports
> and I could not blacklist them as the bonding driver needed them, which
> caused the bonding ports to have a greater port number. In the case of pktgen
> the bonding ports were up around 8 or 10 and caused the display to not show
> the bonding ports. This is really just a usability problem for the developer
> using Pktgen. I would like to see the vdev devices first, but as long as the
> drivers (like bonding) are fine with them being first.
Ah, now I remember - there was a patch from Jerin for this.
Probably he is the best person to comment here.
(I don't have much insight here).
>
> Regards,
> Keith
On Wed, Feb 15, 2017 at 3:22 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>
>> On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>
>>
>> Just ignore this comment - I am misunderstood something.
>>
>> But another question: Is there specific reason VDEV should be registered/scanned *after* other devices? Is there some specific problem if we do otherwise? (I think this is should be done, but I don't have a specific reason).
>
Just for context: the vdev's are probed after the physical devices
because of commit f4ce209a ("eal: postpone vdev initialization").
> Does the bonding driver which uses physical devices need to be registered after physical ones? In Pktgen I noticed the vdev after the physical ports and I could not blacklist them as the bonding driver needed them, which caused the bonding ports to have a greater port number. In the case of pktgen the bonding ports were up around 8 or 10 and caused the display to not show the bonding ports. This is really just a usability problem for the developer using Pktgen. I would like to see the vdev devices first, but as long as the drivers (like bonding) are fine with them being first.
>
The bonding devargs might specify slaves that get attached during
device probe. If the referenced devices are physical interfaces we
need to probe them first. This is really a chicken-egg-problem.
Maybe you could improve the usability in your case and sort the
virtual devices first or even hide enslaved ports?
> On Feb 15, 2017, at 11:06 AM, Jan Blunck <jblunck@infradead.org> wrote:
>
> On Wed, Feb 15, 2017 at 3:22 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>
>>> On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>>
>>>
>>> Just ignore this comment - I am misunderstood something.
>>>
>>> But another question: Is there specific reason VDEV should be registered/scanned *after* other devices? Is there some specific problem if we do otherwise? (I think this is should be done, but I don't have a specific reason).
>>
>
> Just for context: the vdev's are probed after the physical devices
> because of commit f4ce209a ("eal: postpone vdev initialization").
>
>> Does the bonding driver which uses physical devices need to be registered after physical ones? In Pktgen I noticed the vdev after the physical ports and I could not blacklist them as the bonding driver needed them, which caused the bonding ports to have a greater port number. In the case of pktgen the bonding ports were up around 8 or 10 and caused the display to not show the bonding ports. This is really just a usability problem for the developer using Pktgen. I would like to see the vdev devices first, but as long as the drivers (like bonding) are fine with them being first.
>>
>
> The bonding devargs might specify slaves that get attached during
> device probe. If the referenced devices are physical interfaces we
> need to probe them first. This is really a chicken-egg-problem.
>
> Maybe you could improve the usability in your case and sort the
> virtual devices first or even hide enslaved ports?
The port numbering comes from DPDK and I use that directly, was trying to avoid a translation of real port to Pktgen port :-(
Regards,
Keith
> On Feb 15, 2017, at 11:10 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>
>
>> On Feb 15, 2017, at 11:06 AM, Jan Blunck <jblunck@infradead.org> wrote:
>>
>> On Wed, Feb 15, 2017 at 3:22 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>>
>>>> On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>>>
>>>>
>>>> Just ignore this comment - I am misunderstood something.
>>>>
>>>> But another question: Is there specific reason VDEV should be registered/scanned *after* other devices? Is there some specific problem if we do otherwise? (I think this is should be done, but I don't have a specific reason).
>>>
>>
>> Just for context: the vdev's are probed after the physical devices
>> because of commit f4ce209a ("eal: postpone vdev initialization").
>>
>>> Does the bonding driver which uses physical devices need to be registered after physical ones? In Pktgen I noticed the vdev after the physical ports and I could not blacklist them as the bonding driver needed them, which caused the bonding ports to have a greater port number. In the case of pktgen the bonding ports were up around 8 or 10 and caused the display to not show the bonding ports. This is really just a usability problem for the developer using Pktgen. I would like to see the vdev devices first, but as long as the drivers (like bonding) are fine with them being first.
>>>
>>
>> The bonding devargs might specify slaves that get attached during
>> device probe. If the referenced devices are physical interfaces we
>> need to probe them first. This is really a chicken-egg-problem.
>>
>> Maybe you could improve the usability in your case and sort the
>> virtual devices first or even hide enslaved ports?
>
> The port numbering comes from DPDK and I use that directly, was trying to avoid a translation of real port to Pktgen port :-(
>
> Regards,
> Keith
>
Looking at the bonding driver it does not attempt to access the physical ports until bond_ethdev_configure call (I believe). This means the vdev devices should be following the same flow and moving them to before the physical probes would be fine, right?
Regards,
Keith
On Wed, Feb 15, 2017 at 02:27:47PM +0000, Shreyansh Jain wrote:
> > -----Original Message-----
> > From: Wiles, Keith [mailto:keith.wiles@intel.com]
> > Sent: Wednesday, February 15, 2017 7:53 PM
> > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > Cc: Jan Blunck <jblunck@infradead.org>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 3/7] eal: move virtual device probing into a
> > bus
> >
> >
> > > On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> > >
> > > On Wednesday 15 February 2017 07:41 PM, Shreyansh Jain wrote:
> > >> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
> > >>> This is a refactoring of the virtual device probing which moves into into
> > >>> a proper bus structure.
> > >>>
> > >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > >>> ---
> > >>> lib/librte_eal/common/eal_common_dev.c | 22 -----------------
> > >>> lib/librte_eal/common/eal_common_vdev.c | 44
> > >>> +++++++++++++++++++++++++++++++++
> > >>> 2 files changed, 44 insertions(+), 22 deletions(-)
> > >>>
> > >>
> > >> [...]
> > >>
> > >>>
> > >>> diff --git a/lib/librte_eal/common/eal_common_vdev.c
> > >>> b/lib/librte_eal/common/eal_common_vdev.c
> > >>> index 7d6e54f..523a3d6 100644
> > >>> --- a/lib/librte_eal/common/eal_common_vdev.c
> > >>> +++ b/lib/librte_eal/common/eal_common_vdev.c
> > >>> @@ -37,8 +37,10 @@
> > >>> #include <stdint.h>
> > >>> #include <sys/queue.h>
> > >>>
> > >> [...]
> > >>
> > >>> +
> > >>> +static struct rte_bus rte_vdev_bus = {
> > >>> + .scan = vdev_scan,
> > >>> + .probe = vdev_probe,
> > >>> +};
> > >>> +
> > >>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
> > >>>
> > >>
> > >> Does it matter if VDEV buses are registered before or after other
> > >> buses? Either way, the callbacks would be called in the order specified
> > >> in EAL.
> > >>
> > >>
> > >
> > > Just ignore this comment - I am misunderstood something.
> > >
> > > But another question: Is there specific reason VDEV should be
> > registered/scanned *after* other devices? Is there some specific problem if
> > we do otherwise? (I think this is should be done, but I don't have a specific
> > reason).
> >
> > Does the bonding driver which uses physical devices need to be registered
> > after physical ones? In Pktgen I noticed the vdev after the physical ports
> > and I could not blacklist them as the bonding driver needed them, which
> > caused the bonding ports to have a greater port number. In the case of pktgen
> > the bonding ports were up around 8 or 10 and caused the display to not show
> > the bonding ports. This is really just a usability problem for the developer
> > using Pktgen. I would like to see the vdev devices first, but as long as the
> > drivers (like bonding) are fine with them being first.
>
> Ah, now I remember - there was a patch from Jerin for this.
> Probably he is the best person to comment here.
> (I don't have much insight here).
commit f4ce209a8ce5f416b61c76cee773bc54749e2048
Author: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Date: Sun Nov 20 13:30:50 2016 +0530
eal: postpone vdev initialization
Some platform like octeontx may use pci and
vdev based combined device to represent a logical
dpdk functional device.In such case, postponing the
vdev initialization after pci device
initialization will provide the better view of
the pci device resources in the system in
vdev's probe function, and it allows better
functional subsystem registration in vdev probe
function.
As a bonus, This patch fixes a bond device
initialization use case.
example command to reproduce the issue:
./testpmd -c 0x2 --vdev 'eth_bond0,mode=0,
slave=0000:02:00.0,slave=0000:03:00.0' --
--port-topology=chained
root cause:
In existing case(vdev initialization and then pci
initialization), creates three Ethernet ports with
following port ids
0 - Bond device
1 - PCI device 0
2 - PCI devive 1
Since testpmd, calls the configure/start on all the ports on
start up,it will translate to following illegal setup sequence
1)bond device configure/start
1.1) pci device0 stop/configure/start
1.2) pci device1 stop/configure/start
2)pci device 0 configure(illegal setup case,
as device in start state)
The fix changes the initialization sequence and
allow initialization in following valid setup order
1) pcie device 0 configure/start
2) pcie device 1 configure/start
3) bond device 2 configure/start
3.1) pcie device 0/stop/configure/start
3.2) pcie device 1/stop/configure/start
> On Feb 15, 2017, at 11:25 AM, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote:
>
> On Wed, Feb 15, 2017 at 02:27:47PM +0000, Shreyansh Jain wrote:
>>> -----Original Message-----
>>> From: Wiles, Keith [mailto:keith.wiles@intel.com]
>>> Sent: Wednesday, February 15, 2017 7:53 PM
>>> To: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> Cc: Jan Blunck <jblunck@infradead.org>; dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH 3/7] eal: move virtual device probing into a
>>> bus
>>>
>>>
>>>> On Feb 15, 2017, at 8:15 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>>>
>>>> On Wednesday 15 February 2017 07:41 PM, Shreyansh Jain wrote:
>>>>> On Wednesday 15 February 2017 03:32 PM, Jan Blunck wrote:
>>>>>> This is a refactoring of the virtual device probing which moves into into
>>>>>> a proper bus structure.
>>>>>>
>>>>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>>>>> ---
>>>>>> lib/librte_eal/common/eal_common_dev.c | 22 -----------------
>>>>>> lib/librte_eal/common/eal_common_vdev.c | 44
>>>>>> +++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 44 insertions(+), 22 deletions(-)
>>>>>>
>>>>>
>>>>> [...]
>>>>>
>>>>>>
>>>>>> diff --git a/lib/librte_eal/common/eal_common_vdev.c
>>>>>> b/lib/librte_eal/common/eal_common_vdev.c
>>>>>> index 7d6e54f..523a3d6 100644
>>>>>> --- a/lib/librte_eal/common/eal_common_vdev.c
>>>>>> +++ b/lib/librte_eal/common/eal_common_vdev.c
>>>>>> @@ -37,8 +37,10 @@
>>>>>> #include <stdint.h>
>>>>>> #include <sys/queue.h>
>>>>>>
>>>>> [...]
>>>>>
>>>>>> +
>>>>>> +static struct rte_bus rte_vdev_bus = {
>>>>>> + .scan = vdev_scan,
>>>>>> + .probe = vdev_probe,
>>>>>> +};
>>>>>> +
>>>>>> +RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);
>>>>>>
>>>>>
>>>>> Does it matter if VDEV buses are registered before or after other
>>>>> buses? Either way, the callbacks would be called in the order specified
>>>>> in EAL.
>>>>>
>>>>>
>>>>
>>>> Just ignore this comment - I am misunderstood something.
>>>>
>>>> But another question: Is there specific reason VDEV should be
>>> registered/scanned *after* other devices? Is there some specific problem if
>>> we do otherwise? (I think this is should be done, but I don't have a specific
>>> reason).
>>>
>>> Does the bonding driver which uses physical devices need to be registered
>>> after physical ones? In Pktgen I noticed the vdev after the physical ports
>>> and I could not blacklist them as the bonding driver needed them, which
>>> caused the bonding ports to have a greater port number. In the case of pktgen
>>> the bonding ports were up around 8 or 10 and caused the display to not show
>>> the bonding ports. This is really just a usability problem for the developer
>>> using Pktgen. I would like to see the vdev devices first, but as long as the
>>> drivers (like bonding) are fine with them being first.
>>
>> Ah, now I remember - there was a patch from Jerin for this.
>> Probably he is the best person to comment here.
>> (I don't have much insight here).
>
> commit f4ce209a8ce5f416b61c76cee773bc54749e2048
> Author: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Date: Sun Nov 20 13:30:50 2016 +0530
>
> eal: postpone vdev initialization
>
> Some platform like octeontx may use pci and
> vdev based combined device to represent a logical
> dpdk functional device.In such case, postponing the
> vdev initialization after pci device
> initialization will provide the better view of
> the pci device resources in the system in
> vdev's probe function, and it allows better
> functional subsystem registration in vdev probe
> function.
>
> As a bonus, This patch fixes a bond device
> initialization use case.
>
> example command to reproduce the issue:
> ./testpmd -c 0x2 --vdev 'eth_bond0,mode=0,
> slave=0000:02:00.0,slave=0000:03:00.0' --
> --port-topology=chained
>
> root cause:
> In existing case(vdev initialization and then pci
> initialization), creates three Ethernet ports with
> following port ids
> 0 - Bond device
> 1 - PCI device 0
> 2 - PCI devive 1
>
> Since testpmd, calls the configure/start on all the ports on
> start up,it will translate to following illegal setup sequence
I guess I see this differently, meaning we modified the system to put vdev devices last only because we do not have clean way to startup the system for pdev/vdev devices. The application should be agnostic to the devices being started and the system needs to determine the correct order without a chicken and egg problem. The test-pmd application just starts from 0 to n to initialize devices, which he should be able to do in any order. It is possible the application could initialize the devices (pdev/vdev) in any order, which the current design would break if they tried to init the bonding driver first.
What happens if a vdev needs to be initialized before a pdev device?
Not saying we need to solve this problem now, but need to figure this out some how. Maybe we need a priority for pdev/vdev devices to determine init order????
>
> 1)bond device configure/start
> 1.1) pci device0 stop/configure/start
> 1.2) pci device1 stop/configure/start
> 2)pci device 0 configure(illegal setup case,
> as device in start state)
>
> The fix changes the initialization sequence and
> allow initialization in following valid setup order
> 1) pcie device 0 configure/start
> 2) pcie device 1 configure/start
> 3) bond device 2 configure/start
> 3.1) pcie device 0/stop/configure/start
> 3.2) pcie device 1/stop/configure/start
Regards,
Keith
On Wed, Feb 15, 2017 at 7:09 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>
> I guess I see this differently, meaning we modified the system to put vdev devices last only because we do not have clean way to startup the system for pdev/vdev devices. The application should be agnostic to the devices being started and the system needs to determine the correct order without a chicken and egg problem. The test-pmd application just starts from 0 to n to initialize devices, which he should be able to do in any order. It is possible the application could initialize the devices (pdev/vdev) in any order, which the current design would break if they tried to init the bonding driver first.
>
Apart from the usability (vdevs always first) I wonder what kind of
usecase you are after. If I understand you correctly you want to:
- probe the virtual devices first
- start/configure the virtual devices last
... and only in some cases. From what I understand this requires a
domain specific way to model dependencies between ports, e.g. some
standardized device arguments parsed by EAL, and combined with your
requirement to assign the lowest port numbers for the vdev devices
even a scheduler.
Maybe we could reduce complexity by doing some simple things instead:
if you present the ports in reverse order to the users the vdev come
first. Probably this even increases usability because the most recent
created port is the one that the user is anyway most interested in.
> What happens if a vdev needs to be initialized before a pdev device?
>
This should never happen. The pdev devices offer a plain view on the
"system", which means no topology at all. The vdev devices are devices
that do not have a "system" representation, e.g. a library. I don't
think the EAL should offer an alternative API to system programming in
a way that you enumerate your PCI devices through a vdev that is
accessing hardware through another library.
> Not saying we need to solve this problem now, but need to figure this out some how. Maybe we need a priority for pdev/vdev devices to determine init order????
>
> On Feb 15, 2017, at 2:06 PM, Jan Blunck <jblunck@infradead.org> wrote:
>
> On Wed, Feb 15, 2017 at 7:09 PM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>
>> I guess I see this differently, meaning we modified the system to put vdev devices last only because we do not have clean way to startup the system for pdev/vdev devices. The application should be agnostic to the devices being started and the system needs to determine the correct order without a chicken and egg problem. The test-pmd application just starts from 0 to n to initialize devices, which he should be able to do in any order. It is possible the application could initialize the devices (pdev/vdev) in any order, which the current design would break if they tried to init the bonding driver first.
>>
>
> Apart from the usability (vdevs always first) I wonder what kind of
> usecase you are after. If I understand you correctly you want to:
> - probe the virtual devices first
> - start/configure the virtual devices last
>
> ... and only in some cases. From what I understand this requires a
> domain specific way to model dependencies between ports, e.g. some
> standardized device arguments parsed by EAL, and combined with your
> requirement to assign the lowest port numbers for the vdev devices
> even a scheduler.
I do not think moving vdev to the front is going to really solve my use case it, as it will create more problems then solve. The real case here is if the application developer inits devices out of order then he can hit the bonding driver bug. The current solution does not really fix the problem only bandaid the problem for the common ordered case.
Do we need to solve this problem, maybe not, but it needs to be documented or we need to provide a cleaner way for the application to startup all devices in the correct order. A possible way is to have callback to the application in the correct order for him to initialize the devices, but not a great solution per say.
>
> Maybe we could reduce complexity by doing some simple things instead:
> if you present the ports in reverse order to the users the vdev come
> first. Probably this even increases usability because the most recent
> created port is the one that the user is anyway most interested in.
>
>> What happens if a vdev needs to be initialized before a pdev device?
>>
>
> This should never happen. The pdev devices offer a plain view on the
> "system", which means no topology at all. The vdev devices are devices
> that do not have a "system" representation, e.g. a library. I don't
> think the EAL should offer an alternative API to system programming in
> a way that you enumerate your PCI devices through a vdev that is
> accessing hardware through another library.
>
>> Not saying we need to solve this problem now, but need to figure this out some how. Maybe we need a priority for pdev/vdev devices to determine init order????
>>
Regards,
Keith
@@ -79,28 +79,6 @@ void rte_eal_device_remove(struct rte_device *dev)
int
rte_eal_dev_init(void)
{
- struct rte_devargs *devargs;
-
- /*
- * Note that the dev_driver_list is populated here
- * from calls made to rte_eal_driver_register from constructor functions
- * embedded into PMD modules via the RTE_PMD_REGISTER_VDEV macro
- */
-
- /* call the init function for each virtual device */
- TAILQ_FOREACH(devargs, &devargs_list, next) {
-
- if (devargs->type != RTE_DEVTYPE_VIRTUAL)
- continue;
-
- if (rte_eal_vdev_init(devargs->virt.drv_name,
- devargs->args)) {
- RTE_LOG(ERR, EAL, "failed to initialize %s device\n",
- devargs->virt.drv_name);
- return -1;
- }
- }
-
return 0;
}
@@ -37,8 +37,10 @@
#include <stdint.h>
#include <sys/queue.h>
+#include <rte_bus.h>
#include <rte_vdev.h>
#include <rte_common.h>
+#include <rte_devargs.h>
struct vdev_driver_list vdev_driver_list =
TAILQ_HEAD_INITIALIZER(vdev_driver_list);
@@ -122,3 +124,45 @@ rte_eal_vdev_uninit(const char *name)
RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
return -EINVAL;
}
+
+static int
+vdev_scan(void)
+{
+ /* for virtual devices we don't need to scan anything */
+ return 0;
+}
+
+static int
+vdev_probe(void)
+{
+ struct rte_devargs *devargs;
+
+ /*
+ * Note that the dev_driver_list is populated here
+ * from calls made to rte_eal_driver_register from constructor functions
+ * embedded into PMD modules via the RTE_PMD_REGISTER_VDEV macro
+ */
+
+ /* call the init function for each virtual device */
+ TAILQ_FOREACH(devargs, &devargs_list, next) {
+
+ if (devargs->type != RTE_DEVTYPE_VIRTUAL)
+ continue;
+
+ if (rte_eal_vdev_init(devargs->virt.drv_name,
+ devargs->args)) {
+ RTE_LOG(ERR, EAL, "failed to initialize %s device\n",
+ devargs->virt.drv_name);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+static struct rte_bus rte_vdev_bus = {
+ .scan = vdev_scan,
+ .probe = vdev_probe,
+};
+
+RTE_REGISTER_BUS_LATE(virtual, rte_vdev_bus);