diff mbox series

[v6,5/9] ethdev: support PF index in representor

Message ID 1613272907-22563-6-git-send-email-xuemingl@nvidia.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers show
Series ethdev: support SubFunction representor | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Xueming Li Feb. 14, 2021, 3:21 a.m. UTC 
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

PF type representor is supported by using standalone 'pf' section:
 representor=pf1                - PF representor


Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 doc/guides/prog_guide/poll_mode_drv.rst |  2 ++
 lib/librte_ethdev/ethdev_private.c      | 18 ++++++++++++++++--
 lib/librte_ethdev/rte_ethdev.h          |  1 +
 3 files changed, 19 insertions(+), 2 deletions(-)

Comments

Andrew Rybchenko Feb. 15, 2021, 8:28 a.m. UTC | #1 
On 2/14/21 6:21 AM, Xueming Li wrote:
> 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
> 
> PF type representor is supported by using standalone 'pf' section:
>  representor=pf1                - PF representor
> 
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Andrew Rybchenko Feb. 15, 2021, 8:35 a.m. UTC | #2 
On 2/14/21 6:21 AM, Xueming Li wrote:
> 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
> 
> PF type representor is supported by using standalone 'pf' section:
>  representor=pf1                - PF representor
> 
> 
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  doc/guides/prog_guide/poll_mode_drv.rst |  2 ++
>  lib/librte_ethdev/ethdev_private.c      | 18 ++++++++++++++++--
>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>  3 files changed, 19 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.
> diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c
> index 13c191192e..eea0686020 100644
> --- a/lib/librte_ethdev/ethdev_private.c
> +++ b/lib/librte_ethdev/ethdev_private.c
> @@ -118,8 +118,8 @@ rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
>   *
>   * 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
> @@ -131,6 +131,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;

Is it actually supported? Above documentation does not say so,
i.e. -a DBDF,representor=pf1 is missing.

> +		str += 2;
> +		str = rte_eth_devargs_process_list(str, eth_da->ports,
> +				&eth_da->nb_ports, RTE_DIM(eth_da->ports));
> +		if (str == NULL)
> +			goto err;
> +	}
>  	if (str[0] == 'v' && str[1] == 'f') {
>  		eth_da->type = RTE_ETH_REPRESENTOR_VF;
>  		str += 2;
> @@ -138,11 +146,17 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data)
>  		eth_da->type = RTE_ETH_REPRESENTOR_SF;
>  		str += 2;
>  	} else {

If PF representors are actually supported, shouldn't it be
if (str[0] != '\0')
plus rte_eth_devargs_process_list() after the body should
be inside in fact?

> +		/* Don't mix legacy syntax with 'pf' section. */
> +		if (eth_da->type == RTE_ETH_REPRESENTOR_PF) {
> +			str = NULL;
> +			goto err;
> +		}
>  		eth_da->type = RTE_ETH_REPRESENTOR_VF;
>  	}
>  	str = rte_eth_devargs_process_list(str, eth_da->representor_ports,
>  		&eth_da->nb_representor_ports,
>  		RTE_DIM(eth_da->representor_ports));
> +err:
>  	if (str == NULL)
>  		RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
>  	return str == NULL ? -1 : 0;
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 26b5e109c3..9cd519bf59 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1513,6 +1513,7 @@ enum rte_eth_representor_type {
>  	RTE_ETH_REPRESENTOR_NONE, /**< not a representor. */
>  	RTE_ETH_REPRESENTOR_VF,   /**< representor of Virtual Function. */
>  	RTE_ETH_REPRESENTOR_SF,   /**< representor of Sub Function. */
> +	RTE_ETH_REPRESENTOR_PF,   /**< representor of Physical Function. */
>  };
>  
>  /**
>
Xueming Li Feb. 16, 2021, 2:54 p.m. UTC | #3 
>-----Original Message-----
>From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>Sent: Monday, February 15, 2021 4:35 PM
>To: Xueming(Steven) Li <xuemingl@nvidia.com>
>Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Asaf Penso <asafp@nvidia.com>; NBU-Contact-Thomas Monjalon
><thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>
>Subject: Re: [PATCH v6 5/9] ethdev: support PF index in representor
>
>On 2/14/21 6:21 AM, Xueming Li wrote:
>> 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
>>
>> PF type representor is supported by using standalone 'pf' section:
>>  representor=pf1                - PF representor
>>
>>
>> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
>> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>> ---
>>  doc/guides/prog_guide/poll_mode_drv.rst |  2 ++
>>  lib/librte_ethdev/ethdev_private.c      | 18 ++++++++++++++++--
>>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>>  3 files changed, 19 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.
>> diff --git a/lib/librte_ethdev/ethdev_private.c
>> b/lib/librte_ethdev/ethdev_private.c
>> index 13c191192e..eea0686020 100644
>> --- a/lib/librte_ethdev/ethdev_private.c
>> +++ b/lib/librte_ethdev/ethdev_private.c
>> @@ -118,8 +118,8 @@ rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
>>   *
>>   * 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
>> @@ -131,6 +131,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;
>
>Is it actually supported? Above documentation does not say so, i.e. -a DBDF,representor=pf1 is missing.

Yes, will update.

>
>> +		str += 2;
>> +		str = rte_eth_devargs_process_list(str, eth_da->ports,
>> +				&eth_da->nb_ports, RTE_DIM(eth_da->ports));
>> +		if (str == NULL)
>> +			goto err;
>> +	}
>>  	if (str[0] == 'v' && str[1] == 'f') {
>>  		eth_da->type = RTE_ETH_REPRESENTOR_VF;
>>  		str += 2;
>> @@ -138,11 +146,17 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data)
>>  		eth_da->type = RTE_ETH_REPRESENTOR_SF;
>>  		str += 2;
>>  	} else {
>
>If PF representors are actually supported, shouldn't it be if (str[0] != '\0') plus rte_eth_devargs_process_list() after the body should be
>inside in fact?

Yes, need this check, I'll add it after 'pf' parsing.
Also, 'c' should be followed by 'pf', 'pf' should followed by 'vf' or 'sf', I'll update check.

>
>> +		/* Don't mix legacy syntax with 'pf' section. */
>> +		if (eth_da->type == RTE_ETH_REPRESENTOR_PF) {
>> +			str = NULL;
>> +			goto err;
>> +		}
>>  		eth_da->type = RTE_ETH_REPRESENTOR_VF;
>>  	}
>>  	str = rte_eth_devargs_process_list(str, eth_da->representor_ports,
>>  		&eth_da->nb_representor_ports,
>>  		RTE_DIM(eth_da->representor_ports));
>> +err:
>>  	if (str == NULL)
>>  		RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
>>  	return str == NULL ? -1 : 0;
>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>> b/lib/librte_ethdev/rte_ethdev.h index 26b5e109c3..9cd519bf59 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1513,6 +1513,7 @@ enum rte_eth_representor_type {
>>  	RTE_ETH_REPRESENTOR_NONE, /**< not a representor. */
>>  	RTE_ETH_REPRESENTOR_VF,   /**< representor of Virtual Function. */
>>  	RTE_ETH_REPRESENTOR_SF,   /**< representor of Sub Function. */
>> +	RTE_ETH_REPRESENTOR_PF,   /**< representor of Physical Function. */
>>  };
>>
>>  /**
>>
diff mbox series

Patch

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.
diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c
index 13c191192e..eea0686020 100644
--- a/lib/librte_ethdev/ethdev_private.c
+++ b/lib/librte_ethdev/ethdev_private.c
@@ -118,8 +118,8 @@  rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
  *
  * 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
@@ -131,6 +131,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_DIM(eth_da->ports));
+		if (str == NULL)
+			goto err;
+	}
 	if (str[0] == 'v' && str[1] == 'f') {
 		eth_da->type = RTE_ETH_REPRESENTOR_VF;
 		str += 2;
@@ -138,11 +146,17 @@  rte_eth_devargs_parse_representor_ports(char *str, void *data)
 		eth_da->type = RTE_ETH_REPRESENTOR_SF;
 		str += 2;
 	} else {
+		/* Don't mix legacy syntax with 'pf' section. */
+		if (eth_da->type == RTE_ETH_REPRESENTOR_PF) {
+			str = NULL;
+			goto err;
+		}
 		eth_da->type = RTE_ETH_REPRESENTOR_VF;
 	}
 	str = rte_eth_devargs_process_list(str, eth_da->representor_ports,
 		&eth_da->nb_representor_ports,
 		RTE_DIM(eth_da->representor_ports));
+err:
 	if (str == NULL)
 		RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
 	return str == NULL ? -1 : 0;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 26b5e109c3..9cd519bf59 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1513,6 +1513,7 @@  enum rte_eth_representor_type {
 	RTE_ETH_REPRESENTOR_NONE, /**< not a representor. */
 	RTE_ETH_REPRESENTOR_VF,   /**< representor of Virtual Function. */
 	RTE_ETH_REPRESENTOR_SF,   /**< representor of Sub Function. */
+	RTE_ETH_REPRESENTOR_PF,   /**< representor of Physical Function. */
 };
 
 /**