Hi Andrew,
>-----Original Message-----
>From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>Sent: Tuesday, January 19, 2021 3:25 PM
>To: Xueming(Steven) Li <xuemi...@nvidia.com>; NBU-Contact-Thomas
>Monjalon <tho...@monjalon.net>; Ferruh Yigit <ferruh.yi...@intel.com>;
>Olivier Matz <olivier.m...@6wind.com>
>Cc: dev@dpdk.org; Slava Ovsiienko <viachesl...@nvidia.com>; Asaf Penso
><as...@nvidia.com>
>Subject: Re: [PATCH v4 1/9] ethdev: introduce representor type
>
>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 <xuemi...@nvidia.com>
>> Acked-by: Viacheslav Ovsiienko <viachesl...@nvidia.com>
>
>One nit below and a question below.
>
>In any case:
>
>Acked-by: Andrew Rybchenko <andrew.rybche...@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.
Yes, later patch in this patchset will support it.
>
>> +};
>> +
>> /** 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?
Correct, current devargs structure can't support mix of different types.
I'll update in next version if any.
>
>> };
>>
>> /**
>>
