Re: [dpdk-dev] [PATCH] eal: postpone vdev initialization

[Shreyansh Jain <shreyansh.jain@nxp.com>]

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

-
Shreyansh