On 1/19/21 10:37 AM, Xueming(Steven) Li wrote:

Hi Andrew,

quoted

-----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.

I know. The question is why it is not added in the
later patches when these types are actually supported.

quoted

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

quoted

 };

 /**