[dpdk-dev] [PATCH v4 1/9] ethdev: introduce representor type

[Andrew Rybchenko]

On 1/18/21 2:16 PM, Xueming Li wrote:
> To support more representor type, this patch introduces representor type
> enum. The enum is subject to extend for new types upcoming.
> 
> Signed-off-by: Xueming Li <xuemingl at nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>

One nit below and a question below.

In any case:

Acked-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>

[snip]

> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index 0eacfd8425..3bc5c5bbbb 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -1193,6 +1193,14 @@ __rte_internal
>  int
>  rte_eth_switch_domain_free(uint16_t domain_id);
>  
> +/** Ethernet device representor type */
> +enum rte_eth_representor_type {
> +	RTE_ETH_REPRESENTOR_NONE, /**< not a representor. */
> +	RTE_ETH_REPRESENTOR_VF,   /**< representor of VF. */
> +	RTE_ETH_REPRESENTOR_SF,   /**< representor of SF. */
> +	RTE_ETH_REPRESENTOR_PF,   /**< representor of host PF. */

RTE_ETH_REPRESENTOR_SF and PF looks dead in the patch.
IMHO, addition of these members here make future patches
which add support inconsistent.

> +};
> +
>  /** Generic Ethernet device arguments  */
>  struct rte_eth_devargs {
>  	uint16_t ports[RTE_MAX_ETHPORTS];
> @@ -1203,6 +1211,7 @@ struct rte_eth_devargs {
>  	/** representor port/s identifier to enable on device */
>  	uint16_t nb_representor_ports;
>  	/** number of ports in representor port field */
> +	enum rte_eth_representor_type type; /* type of representor */

Is it intended and documented limitation that we can't add
different type representors in one request? Or am I missing
something and it is possible?

>  };
>  
>  /**
>