Hi Andrew,

-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Tuesday, January 19, 2021 3:25 PM
To: Xueming(Steven) Li <redacted>; NBU-Contact-Thomas
Monjalon [off-list ref]; Ferruh Yigit [off-list ref];
Olivier Matz [off-list ref]
Cc: dev@dpdk.org; Slava Ovsiienko <redacted>; Asaf Penso
[off-list ref]
Subject: Re: [PATCH v4 1/9] ethdev: introduce representor type

On 1/18/21 2:16 PM, Xueming Li wrote:

quoted

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 <redacted>
Acked-by: Viacheslav Ovsiienko <redacted>

One nit below and a question below.

In any case:

Acked-by: Andrew Rybchenko <redacted>

[snip]

quoted

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.

quoted

+};
+
 /** 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.

quoted

 };

 /**