-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Tuesday, January 19, 2021 5:36 PM
To: Xueming(Steven) Li <redacted>
Cc: dev@dpdk.org; Slava Ovsiienko <redacted>; Asaf Penso
[off-list ref]; NBU-Contact-Thomas Monjalon
[off-list ref]; Ferruh Yigit [off-list ref]
Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor

On 1/19/21 12:30 PM, Xueming(Steven) Li wrote:

quoted

Hi Andrew,

quoted

-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Tuesday, January 19, 2021 4:01 PM
To: Xueming(Steven) Li <redacted>
Cc: dev@dpdk.org; Slava Ovsiienko <redacted>; Asaf
Penso [off-list ref]; NBU-Contact-Thomas Monjalon
[off-list ref]; Ferruh Yigit [off-list ref]
Subject: Re: [PATCH v5 5/9] ethdev: support PF index in representor

On 1/19/21 10:14 AM, Xueming Li wrote:

quoted

With Kernel bonding, multiple underlying PFs are bonded, VFs come
from different PF, need to identify representor of VFs unambiguously
by adding PF index.

This patch introduces optional 'pf' section to representor devargs
syntax, examples:
 representor=pf0vf0             - single VF representor
 representor=pf[0-1]sf[0-1023]  - SF representors from 2 PFs

Don't we need
representor=pf3
i.e. without VF or sub-function?

Standalone PF not used by Mellnaox PMD, but should be supported.
Will update.

quoted

quoted

Signed-off-by: Xueming Li <redacted>
Acked-by: Viacheslav Ovsiienko <redacted>
Acked-by: Thomas Monjalon <redacted>
---
 doc/guides/prog_guide/poll_mode_drv.rst |  2 ++
 lib/librte_ethdev/ethdev_private.c      | 13 +++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/doc/guides/prog_guide/poll_mode_drv.rst

b/doc/guides/prog_guide/poll_mode_drv.rst
index 86e5867f1b..b2147aad30 100644

--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst

@@ -382,6 +382,8 @@ parameters to those ports.
    -a DBDF,representor=sf[1,3,5]
    -a DBDF,representor=sf[0-1023]
    -a DBDF,representor=sf[0,2-4,7,9-11]
+   -a DBDF,representor=pf1vf0
+   -a DBDF,representor=pf[0-1]sf[0-127]

 Note: PMDs are not required to support the standard device

arguments and users  should consult the relevant PMD documentation
to see support

devargs.

quoted

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

b/lib/librte_ethdev/ethdev_private.c
index d513f035d0..b9fdbd0f72 100644

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

@@ -120,8 +120,8 @@ rte_eth_devargs_process_list(char *str, uint16_t

*list, uint16_t *len_list,

quoted

  *
  * Representor format:
  *   #: range or single number of VF representor - legacy
- *   vf#: VF port representor/s
- *   sf#: SF port representor/s
+ *   [pf#]vf#: VF port representor/s
+ *   [pf#]sf#: SF port representor/s
  *
  * Examples of #:
  *  2               - single

@@ -133,6 +133,14 @@ rte_eth_devargs_parse_representor_ports(char

*str, void *data)  {
 	struct rte_eth_devargs *eth_da = data;

+	if (str[0] == 'p' && str[1] == 'f') {
+		eth_da->type = RTE_ETH_REPRESENTOR_PF;
+		str += 2;
+		str = rte_eth_devargs_process_list(str, eth_da->ports,
+				&eth_da->nb_ports, RTE_MAX_ETHPORTS);

May be RTE_MAX_ETHPORTS -> RTE_DIM(eth_da->ports) ?

Same here, the dim could be different than MAX value.

Hold on, just for my understanding. The maximum says how many entries
could be added to the array. So, why?

Right, will change to RTE_DIM(), thanks!

quoted

quoted

quoted

+		if (str == NULL)
+			goto err;

Below we should not allow legacy VF syntax without "vf" prefix.

For backward compatibility, default numbers to "vf", otherwise some
existing app like OVS that working with VF will break.

I mean if new syntax is used (i.e. we have pfX prefix), we must deny legacy
syntax for VFs below.

Correct, will add check, thanks!

quoted

quoted

quoted

+	}
 	if (str[0] == 'v' && str[1] == 'f') {
 		eth_da->type = RTE_ETH_REPRESENTOR_VF;
 		str += 2;

@@ -144,6 +152,7 @@ rte_eth_devargs_parse_representor_ports(char

*str,

void *data)

quoted

 	}
 	str = rte_eth_devargs_process_list(str, eth_da->representor_ports,
 		&eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
+err:
 	if (str == NULL)
 		RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
 	return str == NULL ? -1 : 0;