Hi Andrew,

-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Monday, December 28, 2020 9:37 PM
To: Xueming(Steven) Li <redacted>; Slava Ovsiienko
[off-list ref]; NBU-Contact-Thomas Monjalon
[off-list ref]; Ferruh Yigit [off-list ref]; Olivier Matz
[off-list ref]; Matan Azrad [off-list ref]
Cc: dev@dpdk.org; Asaf Penso <redacted>
Subject: Re: [RFC 3/7] devarg: change reprsentor ID to bitmap

On 12/18/20 5:55 PM, Xueming Li wrote:

quoted

In eth representor comparer callback, ethdev was compared with devarg.

comparer -> comparator?

quoted

Since ethdev representor port didn't contain controller(host) and
owner port information, callback only compared representor port and
returned representor port on other PF port.

This patch changes representor port to bitmap encoding, expands and
updates representor port ID after parsing, when device representor ID
uses the same bitmap encoding, the eth representor comparer callback
returns correct ethdev.

Representor port ID bitmap definition:
 Representor ID bitmap:
 xxxx xxxx xxxx xxxx
 |||| |LLL LLLL LLLL vf/sf id
 |||| L 1:sf, 0:vf
 ||LL pf id

Just 2 bits for PF ID is definitely not future proof.

Yes, this is a valid concern, to keep ABI compatibility, need to wait next LTS to
change it to u32 or u64.

quoted

 LL controller(host) id

Same here.

In general, I'm not sure that such approch with bitmap makes sense. I think
we need a new API which returns information about available functions which
could be represented and IDs there could be used as representor IDs.

Agree, will introduce rte_eth_representor_id_encode() and rte_eth_representor_id_parse()
in next vesion.

quoted

Signed-off-by: Xueming Li <redacted>
---
 0000-cover-letter.patch               | 44 +++++++++++++++++++++++++++

I guess it should not be added to the changeset.

quoted

 lib/librte_ethdev/ethdev_private.c    | 42 ++++++++++++++++++++++++-
 lib/librte_ethdev/rte_ethdev_driver.h | 22 ++++++++++++++
 3 files changed, 107 insertions(+), 1 deletion(-)  create mode 100644
0000-cover-letter.patch

diff --git a/0000-cover-letter.patch b/0000-cover-letter.patch new

file mode 100644 index 0000000000..3f8ce2be72

--- /dev/null
+++ b/0000-cover-letter.patch

@@ -0,0 +1,44 @@
+From 4e1f8fc062fa6813e0b57f78ad72760601ca1d98 Mon Sep 17 00:00:00
+2001
+From: Xueming Li <xuemingl@nvidia.com>
+Date: Fri, 18 Dec 2020 22:31:53 +0800
+Subject: [RFC 0/7] *** SUBJECT HERE ***
+To: Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
+    Thomas Monjalon <thomas@monjalon.net>,
+    Ferruh Yigit <ferruh.yigit@intel.com>,
+    Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
+    Olivier Matz <olivier.matz@6wind.com>,
+    Matan Azrad <matan@nvidia.com>
+Cc: dev@dpdk.org,
+    xuemingl@nvidia.com,
+    Asaf Penso <asafp@nvidia.com>
+
+*** BLURB HERE ***
+
+Xueming Li (7):
+  ethdev: support sub function representor
+  ethdev: support multi-host representor
+  devarg: change reprsentor ID to bitmap
+  ethdev: capability for new representor syntax
+  kvargs: update parser for new representor syntax
+  common/mlx5: update representor name parsing
+  net/mlx5: support representor of sub function
+
+ config/rte_config.h                        |   1 +
+ drivers/common/mlx5/linux/mlx5_common_os.c |  32 ++--
+ drivers/common/mlx5/linux/mlx5_nl.c        |   2 +
+ drivers/common/mlx5/mlx5_common.h          |   2 +
+ drivers/net/mlx5/linux/mlx5_ethdev_os.c    |   5 +
+ drivers/net/mlx5/linux/mlx5_os.c           |  69 ++++++++-
+ drivers/net/mlx5/mlx5_ethdev.c             |   2 +
+ lib/librte_ethdev/ethdev_private.c         | 163 ++++++++++++++-------
+ lib/librte_ethdev/ethdev_private.h         |   3 -
+ lib/librte_ethdev/rte_class_eth.c          |   7 +-
+ lib/librte_ethdev/rte_ethdev.c             |   5 +-
+ lib/librte_ethdev/rte_ethdev.h             |   2 +
+ lib/librte_ethdev/rte_ethdev_driver.h      |  35 +++++
+ lib/librte_kvargs/rte_kvargs.c             |  82 +++++++----
+ 14 files changed, 306 insertions(+), 104 deletions(-)
+
+--
+2.25.1
+

diff --git a/lib/librte_ethdev/ethdev_private.c

b/lib/librte_ethdev/ethdev_private.c
index 3e455acea9..a0fc187378 100644

--- a/lib/librte_ethdev/ethdev_private.c
+++ b/lib/librte_ethdev/ethdev_private.c

@@ -93,16 +93,20 @@ rte_eth_devargs_process_list(char *str, uint16_t

*list, uint16_t *len_list,  }

 /*
- * representor format:
+ * Parse representor ports, expand and update representor port ID.
+ * Representor format:
  *   #: range or single number of VF representor - legacy
  *   [[c#]pf#]vf#: VF port representor/s
  *   [[c#]pf#]sf#: SF port representor/s
+ *
+ * See RTE_ETH_REPR() for representor ID format.
  */
 int
 rte_eth_devargs_parse_representor_ports(char *str, void *data)  {
 	struct rte_eth_devargs *eth_da = data;
 	int ret;
+	uint32_t c, p, f, i = 0;

 	eth_da->type = RTE_ETH_REPRESENTOR_NONE;
 	if (str[0] == 'c') {

@@ -136,6 +140,42 @@ rte_eth_devargs_parse_representor_ports(char

*str, void *data)

quoted

 	}
 	ret = rte_eth_devargs_process_list(str, eth_da->representor_ports,
 		&eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
+	if (ret < 0)
+		goto err;
+
+	/* Set default values, expand and update representor ID. */
+	if (!eth_da->nb_mh_controllers) {

DPDK coding style requires to compare vs 0 expliticly.

quoted

+		eth_da->nb_mh_controllers = 1;
+		eth_da->mh_controllers[0] = 0;
+	}
+	if (!eth_da->nb_ports) {

DPDK coding style requires to compare vs 0 expliticly.

quoted

+		eth_da->nb_ports = 1;
+		eth_da->ports[0] = 0;
+	}
+	if (!eth_da->nb_representor_ports) {

DPDK coding style requires to compare vs 0 expliticly.

quoted

+		eth_da->nb_representor_ports = 1;
+		eth_da->representor_ports[0] = 0;
+	}
+	for (c = 0; c < eth_da->nb_mh_controllers; ++c) {
+		for (p = 0; p < eth_da->nb_ports; ++p) {
+			for (f = 0; f < eth_da->nb_representor_ports; ++f) {
+				i = c * eth_da->nb_ports *
+					eth_da->nb_representor_ports +
+				    p * eth_da->nb_representor_ports + f;
+				if (i >= RTE_DIM(eth_da->representor_ports))

{

quoted

+					RTE_LOG(ERR, EAL, "too many

representor specified: %s",

Missing \n

quoted

+						str);
+					return -EINVAL;
+				}
+				eth_da->representor_ports[i] =

RTE_ETH_REPR(

quoted

+					eth_da->mh_controllers[c],
+					eth_da->ports[p],
+					eth_da->type ==

RTE_ETH_REPRESENTOR_SF,

quoted

+					eth_da->representor_ports[f]);
+			}
+		}
+	}
+	eth_da->nb_representor_ports = i + 1;
 err:
 	if (ret < 0)
 		RTE_LOG(ERR, EAL, "wrong representor format: %s", str); diff -

-git

quoted

a/lib/librte_ethdev/rte_ethdev_driver.h
b/lib/librte_ethdev/rte_ethdev_driver.h
index a7969c9408..dbad55c704 100644

--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h

@@ -1218,6 +1218,28 @@ struct rte_eth_devargs {
 	enum rte_eth_representor_type type; /* type of representor */  };

+/**
+ * Encoding representor port ID.
+ *
+ * The compact format is used for device iterator that comparing
+ * ethdev representor ID with target devargs.
+ *
+ * xxxx xxxx xxxx xxxx
+ * |||| |LLL LLLL LLLL vf/sf id
+ * |||| L 1:sf, 0:vf
+ * ||LL pf id
+ * LL controller(host) id
+ */
+#define RTE_ETH_REPR(c, pf, sf, port) \
+	((((c) & 3) << 14) |     \
+	(((pf) & 3) << 12) |     \
+	(!!(sf) << 11) |         \
+	((port) & 0x7ff))
+/** Get 'pf' port id from representor ID */ #define
+RTE_ETH_REPR_PF(repr) (((repr) >> 12) & 3)
+/** Get 'vf' or 'sf' port from representor ID */ #define
+RTE_ETH_REPR_PORT(repr) ((repr) & 0x7ff)
+
 /**
  * PMD helper function to parse ethdev arguments
  *