Re: [dpdk-dev] [PATCH 04/13] eal: introduce driver type

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

On Tuesday 20 December 2016 03:29 AM, Stephen Hemminger wrote:
> Since multiple buses and device types need to be supported.
> Provide type field in driver.
> ---
>  lib/librte_eal/common/include/rte_dev.h  | 15 ++++++++++++---
>  lib/librte_eal/common/include/rte_pci.h  |  1 +
>  lib/librte_eal/common/include/rte_vdev.h |  1 +
>  3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index e5471a22..3f4e26e6 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -144,12 +144,21 @@ void rte_eal_device_insert(struct rte_device *dev);
>  void rte_eal_device_remove(struct rte_device *dev);
>
>  /**
> + * Type of device driver
> + */
> +enum rte_driver_type {
> +	PMD_VIRTUAL,
> +	PMD_PCI,
> +};

So the expectation is that if a new device type is introduced (anything 
other than virtual or PCI), this enum is expanded?

Broadly, we have two possible ways being discussed on ML for device 
relationship with PMD/APIs:

1. the way you have done: each device belongs to a particular type (enum 
rte_driver_type) and based on its type, operations would be performed on 
it (using conditional operators, for example). We continue to have a 
common device list containing all type of devices.

2. disassociating the device (rte_device) completely from its type and 
basing it on a Bus type. So, we have separate buses for each device type 
and hence no need for separation of logic based on device type.

I think implementation similar to (1) existed before it was removed in 
6751f6de.

I have an (obvious) inclination towards (2) because that helps plugging 
in more drivers/device types without expecting the contributor to change 
the EAL - however trivial the change is.

> +
> +/**
>   * A structure describing a device driver.
>   */
>  struct rte_driver {
>  	TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> -	const char *name;                   /**< Driver name. */
> -	const char *alias;              /**< Driver alias. */
> +	const char *name;              /**< Driver name. */
> +	const char *alias;             /**< Driver alias. */
> +	enum rte_driver_type type;     /**< Driver type. */
>  };
>
>  /**
> @@ -243,4 +252,4 @@ __attribute__((used)) = str
>  }
>  #endif
>
> -#endif /* _RTE_VDEV_H_ */
> +#endif /* _RTE_DEV_H_ */
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 9ce88472..d377d539 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -492,6 +492,7 @@ RTE_INIT(pciinitfn_ ##nm); \
>  static void pciinitfn_ ##nm(void) \
>  {\
>  	(pci_drv).driver.name = RTE_STR(nm);\
> +	(pci_drv).driver.type = PMD_PCI; \
>  	rte_eal_pci_register(&pci_drv); \
>  } \
>  RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> diff --git a/lib/librte_eal/common/include/rte_vdev.h b/lib/librte_eal/common/include/rte_vdev.h
> index 784e837d..98fb5bb5 100644
> --- a/lib/librte_eal/common/include/rte_vdev.h
> +++ b/lib/librte_eal/common/include/rte_vdev.h
> @@ -88,6 +88,7 @@ static void vdrvinitfn_ ##vdrv(void)\
>  {\
>  	(vdrv).driver.name = RTE_STR(nm);\
>  	(vdrv).driver.alias = vdrvinit_ ## nm ## _alias;\
> +	(vdrv).driver.type = PMD_VIRTUAL;\
>  	rte_eal_vdrv_register(&vdrv);\
>  } \
>  RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>