On Mon, Nov 21, 2016 at 10:39:00AM +0530, Shreyansh Jain wrote:
> On Sunday 20 November 2016 01:30 PM, Jerin Jacob wrote:
> > 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
> >
> > Signed-off-by: Jerin Jacob <jerin.jacob at caviumnetworks.com>
> > ---
> > lib/librte_eal/bsdapp/eal/eal.c | 6 +++---
> > lib/librte_eal/linuxapp/eal/eal.c | 6 +++---
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal.c
> > b/lib/librte_eal/bsdapp/eal/eal.c
> > index 35e3117..2206277 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal.c
> > @@ -577,9 +577,6 @@ rte_eal_init(int argc, char **argv)
> > rte_config.master_lcore, thread_id, cpuset,
> > ret == 0 ? "" : "...");
> >
> > - if (rte_eal_dev_init() < 0)
> > - rte_panic("Cannot init pmd devices\n");
> > -
> > RTE_LCORE_FOREACH_SLAVE(i) {
> >
> > /*
> > @@ -616,6 +613,9 @@ rte_eal_init(int argc, char **argv)
> > if (rte_eal_pci_probe())
> > rte_panic("Cannot probe PCI\n");
> >
> > + if (rte_eal_dev_init() < 0)
> > + rte_panic("Cannot init pmd devices\n");
> > +
> > rte_eal_mcfg_complete();
> >
> > return fctret;
> > diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> > b/lib/librte_eal/linuxapp/eal/eal.c
> > index 2075282..16dd5b9 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -841,9 +841,6 @@ rte_eal_init(int argc, char **argv)
> > rte_config.master_lcore, (int)thread_id, cpuset,
> > ret == 0 ? "" : "...");
> >
> > - if (rte_eal_dev_init() < 0)
> > - rte_panic("Cannot init pmd devices\n");
> > -
> > if (rte_eal_intr_init() < 0)
> > rte_panic("Cannot init interrupt-handling thread\n");
> >
> > @@ -887,6 +884,9 @@ rte_eal_init(int argc, char **argv)
> > if (rte_eal_pci_probe())
> > rte_panic("Cannot probe PCI\n");
> >
> > + if (rte_eal_dev_init() < 0)
> > + rte_panic("Cannot init pmd devices\n");
> > +
> > rte_eal_mcfg_complete();
> >
> > return fctret;
> >
>
> Movement looks fine to me.
>
> IMO, rte_eal_dev_init() is a misleading name. It actually performs a
> driver->probe for vdev - which is parallel to rte_eal_pci_probe.
Looks good to me. If there are no objection, I can change to
rte_eal_vdev_probe()
as a separate patch in v2. Let the order change patch be separate.
>
> -
> Shreyansh
