Re: [dpdk-dev] [PATCH v7 3/6] EAL support for link bonding device initialization

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Declan,

2014-06-24 17:03, Declan Doherty:
> Updating functionality in EAL to support adding link bonding
> devices via –vdev option. Link bonding devices will be
> initialized after all physical devices have been probed and
> initialized.
[...]
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -62,7 +62,7 @@ rte_eal_driver_unregister(struct rte_driver *driver)
>  }
> 
>  int
> -rte_eal_dev_init(void)
> +rte_eal_dev_init(uint8_t init_pri)
>  {
>  	struct rte_devargs *devargs;
>  	struct rte_driver *driver;
> @@ -80,30 +80,52 @@ rte_eal_dev_init(void)
>  			continue;
> 
>  		TAILQ_FOREACH(driver, &dev_driver_list, next) {
> -			if (driver->type != PMD_VDEV)
> -				continue;
> +			/* RTE_DEVTYPE_VIRTUAL can only be a virtual or bonded device,
> +			 * virtual devices are initialized pre PCI probing and bonded
> +			 * device are post pci probing */
> +			if ((driver->type == PMD_VDEV && init_pri ==
> +					PMD_INIT_PRE_PCI_PROBE) ||
> +				(driver->type == PMD_BDEV && init_pri ==
> +						PMD_INIT_POST_PCI_PROBE)) {
> 
> -			/* search a driver prefix in virtual device name */
> -			if (!strncmp(driver->name, devargs->virtual.drv_name,
> -					strlen(driver->name))) {
> -				driver->init(devargs->virtual.drv_name,
> -					devargs->args);
> -				break;
> +				/* search a driver prefix in virtual device name */
> +				if (!strncmp(driver->name, devargs->virtual.drv_name,
> +						strlen(driver->name))) {
> +					printf("init (%u) %s\n", init_pri, devargs-
>virtual.drv_name);
> +					driver->init(devargs->virtual.drv_name,
> +						devargs->args);
> +					break;
> +				}
>  			}
>  		}
> 
> -		if (driver == NULL) {
> -			rte_panic("no driver found for %s\n",
> -				  devargs->virtual.drv_name);
> +		/* If initializing pre PCI probe, then we don't expect a bonded 
driver
> +		 * to be found */
> +		if (init_pri == PMD_INIT_PRE_PCI_PROBE &&
> +				strncmp(RTE_PMD_BOND, devargs->virtual.drv_name,
> +					strlen(RTE_PMD_BOND)) != 0) {
> +			if (driver == NULL) {
> +				rte_panic("no driver found for virtual device %s\n",
> +					devargs->virtual.drv_name);
> +			}
> +		} else if (init_pri == PMD_INIT_POST_PCI_PROBE &&
> +				strncmp(RTE_PMD_BOND, devargs->virtual.drv_name,
> +					strlen(RTE_PMD_BOND)) == 0) {
> +			if (driver == NULL) {
> +				rte_panic("no driver found for bonded device %s\n",
> +					devargs->virtual.drv_name);
> +			}
>  		}
>  	}
> 
> -	/* Once the vdevs are initalized, start calling all the pdev drivers */
> -	TAILQ_FOREACH(driver, &dev_driver_list, next) {
> -		if (driver->type != PMD_PDEV)
> -			continue;
> -		/* PDEV drivers don't get passed any parameters */
> -		driver->init(NULL, NULL);
> +	/* Once the vdevs are initialized, start calling all the pdev drivers */
> +	if (init_pri == PMD_INIT_PRE_PCI_PROBE) {
> +		TAILQ_FOREACH(driver, &dev_driver_list, next) {
> +			if (driver->type != PMD_PDEV)
> +				continue;
> +			/* PDEV drivers don't get passed any parameters */
> +			driver->init(NULL, NULL);
> +		}
>  	}
>  	return 0;
>  }

[...]

> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -75,6 +75,7 @@
>  #include <rte_atomic.h>
>  #include <malloc_heap.h>
>  #include <rte_eth_ring.h>
> +#include <rte_dev.h>
> 
>  #include "eal_private.h"
>  #include "eal_thread.h"
> @@ -1097,7 +1098,7 @@ rte_eal_init(int argc, char **argv)
>  	RTE_LOG(DEBUG, EAL, "Master core %u is ready (tid=%x)\n",
>  		rte_config.master_lcore, (int)thread_id);
> 
> -	if (rte_eal_dev_init() < 0)
> +	if (rte_eal_dev_init(PMD_INIT_PRE_PCI_PROBE) < 0)
>  		rte_panic("Cannot init pmd devices\n");
> 
>  	RTE_LCORE_FOREACH_SLAVE(i) {
> @@ -1127,6 +1128,14 @@ rte_eal_init(int argc, char **argv)
>  	rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MASTER);
>  	rte_eal_mp_wait_lcore();
> 
> +	/* Probe & Initialize PCI devices */
> +	if (rte_eal_pci_probe())
> +			rte_panic("Cannot probe PCI\n");
> +
> +	/* Initialize any outstanding devices */
> +	if (rte_eal_dev_init(PMD_INIT_POST_PCI_PROBE) < 0)
> +		rte_panic("Cannot init pmd devices\n");
> +
>  	return fctret;
>  }


Not sure to understand why you need to split rte_eal_dev_init() in 2 steps.
Should it be possible to keep existing rte_eal_dev_init() behaviour and makes 
further initialization when calling rte_eth_dev_configure()?
I've seen it's empty for bonding device:

static int
bond_ethdev_configure(struct rte_eth_dev *dev __rte_unused)
{
	return 0;
}

Thanks
-- 
Thomas