[v2,1/9] ethdev: refactor representor infrastructure

Message ID 1609949855-23817-2-git-send-email-xuemingl@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series support SubFunction representor |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Xueming Li Jan. 6, 2021, 4:17 p.m. UTC
  To support extended representor syntax, this patch refactor represntor
infrastructure:
1. introduces representor type enum
2. devargs representor port range extraction from partial value

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 drivers/net/bnxt/bnxt_ethdev.c        | 12 ++++
 drivers/net/enic/enic_ethdev.c        |  7 ++
 drivers/net/i40e/i40e_ethdev.c        |  8 +++
 drivers/net/ixgbe/ixgbe_ethdev.c      |  8 +++
 drivers/net/mlx5/linux/mlx5_os.c      | 11 ++++
 lib/librte_ethdev/ethdev_private.c    | 93 ++++++++++++---------------
 lib/librte_ethdev/ethdev_private.h    |  3 -
 lib/librte_ethdev/rte_class_eth.c     |  4 +-
 lib/librte_ethdev/rte_ethdev.c        |  5 +-
 lib/librte_ethdev/rte_ethdev_driver.h |  7 ++
 10 files changed, 98 insertions(+), 60 deletions(-)
  

Comments

Somnath Kotur Jan. 7, 2021, 6:31 a.m. UTC | #1
On Wed, Jan 6, 2021 at 9:48 PM Xueming Li <xuemingl@nvidia.com> wrote:
>
> To support extended representor syntax, this patch refactor represntor
Typo in 'representor'
> infrastructure:
> 1. introduces representor type enum
> 2. devargs representor port range extraction from partial value
>
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> ---
>  drivers/net/bnxt/bnxt_ethdev.c        | 12 ++++
>  drivers/net/enic/enic_ethdev.c        |  7 ++
>  drivers/net/i40e/i40e_ethdev.c        |  8 +++
>  drivers/net/ixgbe/ixgbe_ethdev.c      |  8 +++
>  drivers/net/mlx5/linux/mlx5_os.c      | 11 ++++
>  lib/librte_ethdev/ethdev_private.c    | 93 ++++++++++++---------------
>  lib/librte_ethdev/ethdev_private.h    |  3 -
>  lib/librte_ethdev/rte_class_eth.c     |  4 +-
>  lib/librte_ethdev/rte_ethdev.c        |  5 +-
>  lib/librte_ethdev/rte_ethdev_driver.h |  7 ++
>  10 files changed, 98 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 81c8f8d79d..844a6c3c66 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -5520,6 +5520,18 @@ static int bnxt_rep_port_probe(struct rte_pci_device *pci_dev,
>         int i, ret = 0;
>         struct rte_kvargs *kvlist = NULL;
>
> +       if (eth_da->type == RTE_ETH_REPRESENTOR_NONE)
> +               return 0;
> +       if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
> +               PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
> +                           eth_da->type);
> +               return -ENOTSUP;
> +       }
> +       if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
Seems like an extra 'if ' condition by mistake? Otherwise there is no
diff b/n this 'if' condition and the one few lines above?
> +               PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
> +                           eth_da->type);
> +               return -EINVAL;
> +       }
>         num_rep = eth_da->nb_representor_ports;
>         if (num_rep > BNXT_MAX_VF_REPS) {
>                 PMD_DRV_LOG(ERR, "nb_representor_ports = %d > %d MAX VF REPS\n",
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index d041a6bee9..dd085caa93 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -1303,6 +1303,13 @@ static int eth_enic_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>                 if (retval)
>                         return retval;
>         }
> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
> +               return 0;
> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> +               ENICPMD_LOG(ERR, "unsupported representor type: %s\n",
> +                           pci_dev->device.devargs->args);
> +               return -ENOTSUP;
> +       }
>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
>                 sizeof(struct enic),
>                 eth_dev_pci_specific_init, pci_dev,
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index f54769c29d..05ed2e1079 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -640,6 +640,14 @@ eth_i40e_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>                         return retval;
>         }
>
> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
> +               return 0;
> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> +               PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
> +                           pci_dev->device.devargs->args);
> +               return -ENOTSUP;
> +       }
> +
>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
>                 sizeof(struct i40e_adapter),
>                 eth_dev_pci_specific_init, pci_dev,
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 9a47a8b262..9ea0139197 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1717,6 +1717,14 @@ eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>         } else
>                 memset(&eth_da, 0, sizeof(eth_da));
>
> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
> +               return 0;
> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> +               PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
> +                           pci_dev->device.devargs->args);
> +               return -ENOTSUP;
> +       }
> +
>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
>                 sizeof(struct ixgbe_adapter),
>                 eth_dev_pci_specific_init, pci_dev,
> diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
> index 6812a1f215..6981ba1f41 100644
> --- a/drivers/net/mlx5/linux/mlx5_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_os.c
> @@ -706,6 +706,17 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>                                 strerror(rte_errno));
>                         return NULL;
>                 }
> +               if (eth_da.type != RTE_ETH_REPRESENTOR_NONE) {
> +                       /* Representor not specified. */
> +                       rte_errno = EBUSY;
> +                       return NULL;
> +               }
> +               if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> +                       rte_errno = ENOTSUP;
> +                       DRV_LOG(ERR, "unsupported representor type: %s",
> +                               dpdk_dev->devargs->args);
> +                       return NULL;
> +               }
>                 for (i = 0; i < eth_da.nb_representor_ports; ++i)
>                         if (eth_da.representor_ports[i] ==
>                             (uint16_t)switch_info->port_name)
> diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c
> index 162a502fe7..c219164a4a 100644
> --- a/lib/librte_ethdev/ethdev_private.c
> +++ b/lib/librte_ethdev/ethdev_private.c
> @@ -38,60 +38,13 @@ eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
>         return NULL;
>  }
>
> -int
> -rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
> -       void *data)
> -{
> -       char *str_start;
> -       int state;
> -       int result;
> -
> -       if (*str != '[')
> -               /* Single element, not a list */
> -               return callback(str, data);
> -
> -       /* Sanity check, then strip the brackets */
> -       str_start = &str[strlen(str) - 1];
> -       if (*str_start != ']') {
> -               RTE_LOG(ERR, EAL, "(%s): List does not end with ']'\n", str);
> -               return -EINVAL;
> -       }
> -       str++;
> -       *str_start = '\0';
> -
> -       /* Process list elements */
> -       state = 0;
> -       while (1) {
> -               if (state == 0) {
> -                       if (*str == '\0')
> -                               break;
> -                       if (*str != ',') {
> -                               str_start = str;
> -                               state = 1;
> -                       }
> -               } else if (state == 1) {
> -                       if (*str == ',' || *str == '\0') {
> -                               if (str > str_start) {
> -                                       /* Non-empty string fragment */
> -                                       *str = '\0';
> -                                       result = callback(str_start, data);
> -                                       if (result < 0)
> -                                               return result;
> -                               }
> -                               state = 0;
> -                       }
> -               }
> -               str++;
> -       }
> -       return 0;
> -}
> -
>  static int
>  rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
>         const uint16_t max_list)
>  {
>         uint16_t lo, hi, val;
>         int result;
> +       char *pos = str;
>
>         result = sscanf(str, "%hu-%hu", &lo, &hi);
>         if (result == 1) {
> @@ -99,7 +52,7 @@ rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
>                         return -ENOMEM;
>                 list[(*len_list)++] = lo;
>         } else if (result == 2) {
> -               if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
> +               if (lo >= hi)
>                         return -EINVAL;
>                 for (val = lo; val <= hi; val++) {
>                         if (*len_list >= max_list)
> @@ -108,14 +61,52 @@ rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
>                 }
>         } else
>                 return -EINVAL;
> -       return 0;
> +       while (*pos != 0 && ((*pos >= '0' && *pos <= '9') || *pos == '-'))
> +               pos++;
> +       return pos - str;
>  }
>
> +static int
> +rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
> +       const uint16_t max_list)
> +{
> +       char *pos = str;
> +       int ret;
> +
> +       if (*pos == '[')
> +               pos++;
> +       while (1) {
> +               ret = rte_eth_devargs_process_range(pos, list, len_list,
> +                                                   max_list);
> +               if (ret < 0)
> +                       return ret;
> +               pos += ret;
> +               if (*pos != ',') /* end of list */
> +                       break;
> +               pos++;
> +       }
> +       if (*str == '[' && *pos != ']')
> +               return -EINVAL;
> +       if (*pos == ']')
> +               pos++;
> +       return pos - str;
> +}
> +
> +/*
> + * representor format:
> + *   #: range or single number of VF representor - legacy
> + */
>  int
>  rte_eth_devargs_parse_representor_ports(char *str, void *data)
>  {
>         struct rte_eth_devargs *eth_da = data;
> +       int ret;
>
> -       return rte_eth_devargs_process_range(str, eth_da->representor_ports,
> +       /* Number # alone implies VF */
> +       eth_da->type = RTE_ETH_REPRESENTOR_VF;
> +       ret = rte_eth_devargs_process_list(str, eth_da->representor_ports,
>                 &eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
> +       if (ret < 0)
> +               RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
> +       return ret < 0 ? ret : 0;
>  }
> diff --git a/lib/librte_ethdev/ethdev_private.h b/lib/librte_ethdev/ethdev_private.h
> index 905a45c337..220ddd4408 100644
> --- a/lib/librte_ethdev/ethdev_private.h
> +++ b/lib/librte_ethdev/ethdev_private.h
> @@ -26,9 +26,6 @@ eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
>                 const void *data);
>
>  /* Parse devargs value for representor parameter. */
> -typedef int (*rte_eth_devargs_callback_t)(char *str, void *data);
> -int rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
> -       void *data);
>  int rte_eth_devargs_parse_representor_ports(char *str, void *data);
>
>  #ifdef __cplusplus
> diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
> index 6338355e25..efe6149df5 100644
> --- a/lib/librte_ethdev/rte_class_eth.c
> +++ b/lib/librte_ethdev/rte_class_eth.c
> @@ -77,9 +77,7 @@ eth_representor_cmp(const char *key __rte_unused,
>         if (values == NULL)
>                 return -1;
>         memset(&representors, 0, sizeof(representors));
> -       ret = rte_eth_devargs_parse_list(values,
> -                       rte_eth_devargs_parse_representor_ports,
> -                       &representors);
> +       ret = rte_eth_devargs_parse_representor_ports(values, &representors);
>         free(values);
>         if (ret != 0)
>                 return -1; /* invalid devargs value */
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 17ddacc78d..2ac51ac149 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -5542,9 +5542,8 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
>         for (i = 0; i < args.count; i++) {
>                 pair = &args.pairs[i];
>                 if (strcmp("representor", pair->key) == 0) {
> -                       result = rte_eth_devargs_parse_list(pair->value,
> -                               rte_eth_devargs_parse_representor_ports,
> -                               eth_da);
> +                       result = rte_eth_devargs_parse_representor_ports(
> +                                       pair->value, eth_da);
>                         if (result < 0)
>                                 goto parse_cleanup;
>                 }
> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index 0eacfd8425..b66a955b18 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -1193,6 +1193,12 @@ __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 */
> +};
> +
>  /** Generic Ethernet device arguments  */
>  struct rte_eth_devargs {
>         uint16_t ports[RTE_MAX_ETHPORTS];
> @@ -1203,6 +1209,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 */
>  };
>
>  /**
> --
> 2.25.1
>
  
Xueming Li Jan. 7, 2021, 6:38 a.m. UTC | #2
>-----Original Message-----
>From: Somnath Kotur <somnath.kotur@broadcom.com>
>Sent: Thursday, January 7, 2021 2:32 PM
>To: Xueming(Steven) Li <xuemingl@nvidia.com>
>Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit
><ferruh.yigit@intel.com>; Andrew Rybchenko
><andrew.rybchenko@oktetlabs.ru>; Olivier Matz <olivier.matz@6wind.com>;
>Slava Ovsiienko <viacheslavo@nvidia.com>; dev <dev@dpdk.org>; Asaf Penso
><asafp@nvidia.com>
>Subject: Re: [dpdk-dev] [PATCH v2 1/9] ethdev: refactor representor
>infrastructure
>
>On Wed, Jan 6, 2021 at 9:48 PM Xueming Li <xuemingl@nvidia.com> wrote:
>>
>> To support extended representor syntax, this patch refactor represntor
>Typo in 'representor'
>> infrastructure:
>> 1. introduces representor type enum
>> 2. devargs representor port range extraction from partial value
>>
>> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
>> ---
>>  drivers/net/bnxt/bnxt_ethdev.c        | 12 ++++
>>  drivers/net/enic/enic_ethdev.c        |  7 ++
>>  drivers/net/i40e/i40e_ethdev.c        |  8 +++
>>  drivers/net/ixgbe/ixgbe_ethdev.c      |  8 +++
>>  drivers/net/mlx5/linux/mlx5_os.c      | 11 ++++
>>  lib/librte_ethdev/ethdev_private.c    | 93 ++++++++++++---------------
>>  lib/librte_ethdev/ethdev_private.h    |  3 -
>>  lib/librte_ethdev/rte_class_eth.c     |  4 +-
>>  lib/librte_ethdev/rte_ethdev.c        |  5 +-
>>  lib/librte_ethdev/rte_ethdev_driver.h |  7 ++
>>  10 files changed, 98 insertions(+), 60 deletions(-)
>>
>> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
>b/drivers/net/bnxt/bnxt_ethdev.c
>> index 81c8f8d79d..844a6c3c66 100644
>> --- a/drivers/net/bnxt/bnxt_ethdev.c
>> +++ b/drivers/net/bnxt/bnxt_ethdev.c
>> @@ -5520,6 +5520,18 @@ static int bnxt_rep_port_probe(struct
>rte_pci_device *pci_dev,
>>         int i, ret = 0;
>>         struct rte_kvargs *kvlist = NULL;
>>
>> +       if (eth_da->type == RTE_ETH_REPRESENTOR_NONE)
>> +               return 0;
>> +       if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
>> +               PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
>> +                           eth_da->type);
>> +               return -ENOTSUP;
>> +       }
>> +       if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
>Seems like an extra 'if ' condition by mistake? Otherwise there is no
>diff b/n this 'if' condition and the one few lines above?

Thanks, good catch!

>> +               PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
>> +                           eth_da->type);
>> +               return -EINVAL;
>> +       }
>>         num_rep = eth_da->nb_representor_ports;
>>         if (num_rep > BNXT_MAX_VF_REPS) {
>>                 PMD_DRV_LOG(ERR, "nb_representor_ports = %d > %d MAX VF
>REPS\n",
>> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
>> index d041a6bee9..dd085caa93 100644
>> --- a/drivers/net/enic/enic_ethdev.c
>> +++ b/drivers/net/enic/enic_ethdev.c
>> @@ -1303,6 +1303,13 @@ static int eth_enic_pci_probe(struct
>rte_pci_driver *pci_drv __rte_unused,
>>                 if (retval)
>>                         return retval;
>>         }
>> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
>> +               return 0;
>> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
>> +               ENICPMD_LOG(ERR, "unsupported representor type: %s\n",
>> +                           pci_dev->device.devargs->args);
>> +               return -ENOTSUP;
>> +       }
>>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
>>                 sizeof(struct enic),
>>                 eth_dev_pci_specific_init, pci_dev,
>> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>> index f54769c29d..05ed2e1079 100644
>> --- a/drivers/net/i40e/i40e_ethdev.c
>> +++ b/drivers/net/i40e/i40e_ethdev.c
>> @@ -640,6 +640,14 @@ eth_i40e_pci_probe(struct rte_pci_driver *pci_drv
>__rte_unused,
>>                         return retval;
>>         }
>>
>> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
>> +               return 0;
>> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
>> +               PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
>> +                           pci_dev->device.devargs->args);
>> +               return -ENOTSUP;
>> +       }
>> +
>>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
>>                 sizeof(struct i40e_adapter),
>>                 eth_dev_pci_specific_init, pci_dev,
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 9a47a8b262..9ea0139197 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -1717,6 +1717,14 @@ eth_ixgbe_pci_probe(struct rte_pci_driver
>*pci_drv __rte_unused,
>>         } else
>>                 memset(&eth_da, 0, sizeof(eth_da));
>>
>> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
>> +               return 0;
>> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
>> +               PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
>> +                           pci_dev->device.devargs->args);
>> +               return -ENOTSUP;
>> +       }
>> +
>>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
>>                 sizeof(struct ixgbe_adapter),
>>                 eth_dev_pci_specific_init, pci_dev,
>> diff --git a/drivers/net/mlx5/linux/mlx5_os.c
>b/drivers/net/mlx5/linux/mlx5_os.c
>> index 6812a1f215..6981ba1f41 100644
>> --- a/drivers/net/mlx5/linux/mlx5_os.c
>> +++ b/drivers/net/mlx5/linux/mlx5_os.c
>> @@ -706,6 +706,17 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>>                                 strerror(rte_errno));
>>                         return NULL;
>>                 }
>> +               if (eth_da.type != RTE_ETH_REPRESENTOR_NONE) {
>> +                       /* Representor not specified. */
>> +                       rte_errno = EBUSY;
>> +                       return NULL;
>> +               }
>> +               if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
>> +                       rte_errno = ENOTSUP;
>> +                       DRV_LOG(ERR, "unsupported representor type: %s",
>> +                               dpdk_dev->devargs->args);
>> +                       return NULL;
>> +               }
>>                 for (i = 0; i < eth_da.nb_representor_ports; ++i)
>>                         if (eth_da.representor_ports[i] ==
>>                             (uint16_t)switch_info->port_name)
>> diff --git a/lib/librte_ethdev/ethdev_private.c
>b/lib/librte_ethdev/ethdev_private.c
>> index 162a502fe7..c219164a4a 100644
>> --- a/lib/librte_ethdev/ethdev_private.c
>> +++ b/lib/librte_ethdev/ethdev_private.c
>> @@ -38,60 +38,13 @@ eth_find_device(const struct rte_eth_dev *start,
>rte_eth_cmp_t cmp,
>>         return NULL;
>>  }
>>
>> -int
>> -rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
>> -       void *data)
>> -{
>> -       char *str_start;
>> -       int state;
>> -       int result;
>> -
>> -       if (*str != '[')
>> -               /* Single element, not a list */
>> -               return callback(str, data);
>> -
>> -       /* Sanity check, then strip the brackets */
>> -       str_start = &str[strlen(str) - 1];
>> -       if (*str_start != ']') {
>> -               RTE_LOG(ERR, EAL, "(%s): List does not end with ']'\n", str);
>> -               return -EINVAL;
>> -       }
>> -       str++;
>> -       *str_start = '\0';
>> -
>> -       /* Process list elements */
>> -       state = 0;
>> -       while (1) {
>> -               if (state == 0) {
>> -                       if (*str == '\0')
>> -                               break;
>> -                       if (*str != ',') {
>> -                               str_start = str;
>> -                               state = 1;
>> -                       }
>> -               } else if (state == 1) {
>> -                       if (*str == ',' || *str == '\0') {
>> -                               if (str > str_start) {
>> -                                       /* Non-empty string fragment */
>> -                                       *str = '\0';
>> -                                       result = callback(str_start, data);
>> -                                       if (result < 0)
>> -                                               return result;
>> -                               }
>> -                               state = 0;
>> -                       }
>> -               }
>> -               str++;
>> -       }
>> -       return 0;
>> -}
>> -
>>  static int
>>  rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
>>         const uint16_t max_list)
>>  {
>>         uint16_t lo, hi, val;
>>         int result;
>> +       char *pos = str;
>>
>>         result = sscanf(str, "%hu-%hu", &lo, &hi);
>>         if (result == 1) {
>> @@ -99,7 +52,7 @@ rte_eth_devargs_process_range(char *str, uint16_t
>*list, uint16_t *len_list,
>>                         return -ENOMEM;
>>                 list[(*len_list)++] = lo;
>>         } else if (result == 2) {
>> -               if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
>> +               if (lo >= hi)
>>                         return -EINVAL;
>>                 for (val = lo; val <= hi; val++) {
>>                         if (*len_list >= max_list)
>> @@ -108,14 +61,52 @@ rte_eth_devargs_process_range(char *str, uint16_t
>*list, uint16_t *len_list,
>>                 }
>>         } else
>>                 return -EINVAL;
>> -       return 0;
>> +       while (*pos != 0 && ((*pos >= '0' && *pos <= '9') || *pos == '-'))
>> +               pos++;
>> +       return pos - str;
>>  }
>>
>> +static int
>> +rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
>> +       const uint16_t max_list)
>> +{
>> +       char *pos = str;
>> +       int ret;
>> +
>> +       if (*pos == '[')
>> +               pos++;
>> +       while (1) {
>> +               ret = rte_eth_devargs_process_range(pos, list, len_list,
>> +                                                   max_list);
>> +               if (ret < 0)
>> +                       return ret;
>> +               pos += ret;
>> +               if (*pos != ',') /* end of list */
>> +                       break;
>> +               pos++;
>> +       }
>> +       if (*str == '[' && *pos != ']')
>> +               return -EINVAL;
>> +       if (*pos == ']')
>> +               pos++;
>> +       return pos - str;
>> +}
>> +
>> +/*
>> + * representor format:
>> + *   #: range or single number of VF representor - legacy
>> + */
>>  int
>>  rte_eth_devargs_parse_representor_ports(char *str, void *data)
>>  {
>>         struct rte_eth_devargs *eth_da = data;
>> +       int ret;
>>
>> -       return rte_eth_devargs_process_range(str, eth_da->representor_ports,
>> +       /* Number # alone implies VF */
>> +       eth_da->type = RTE_ETH_REPRESENTOR_VF;
>> +       ret = rte_eth_devargs_process_list(str, eth_da->representor_ports,
>>                 &eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
>> +       if (ret < 0)
>> +               RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
>> +       return ret < 0 ? ret : 0;
>>  }
>> diff --git a/lib/librte_ethdev/ethdev_private.h
>b/lib/librte_ethdev/ethdev_private.h
>> index 905a45c337..220ddd4408 100644
>> --- a/lib/librte_ethdev/ethdev_private.h
>> +++ b/lib/librte_ethdev/ethdev_private.h
>> @@ -26,9 +26,6 @@ eth_find_device(const struct rte_eth_dev *_start,
>rte_eth_cmp_t cmp,
>>                 const void *data);
>>
>>  /* Parse devargs value for representor parameter. */
>> -typedef int (*rte_eth_devargs_callback_t)(char *str, void *data);
>> -int rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t
>callback,
>> -       void *data);
>>  int rte_eth_devargs_parse_representor_ports(char *str, void *data);
>>
>>  #ifdef __cplusplus
>> diff --git a/lib/librte_ethdev/rte_class_eth.c
>b/lib/librte_ethdev/rte_class_eth.c
>> index 6338355e25..efe6149df5 100644
>> --- a/lib/librte_ethdev/rte_class_eth.c
>> +++ b/lib/librte_ethdev/rte_class_eth.c
>> @@ -77,9 +77,7 @@ eth_representor_cmp(const char *key __rte_unused,
>>         if (values == NULL)
>>                 return -1;
>>         memset(&representors, 0, sizeof(representors));
>> -       ret = rte_eth_devargs_parse_list(values,
>> -                       rte_eth_devargs_parse_representor_ports,
>> -                       &representors);
>> +       ret = rte_eth_devargs_parse_representor_ports(values, &representors);
>>         free(values);
>>         if (ret != 0)
>>                 return -1; /* invalid devargs value */
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index 17ddacc78d..2ac51ac149 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -5542,9 +5542,8 @@ rte_eth_devargs_parse(const char *dargs, struct
>rte_eth_devargs *eth_da)
>>         for (i = 0; i < args.count; i++) {
>>                 pair = &args.pairs[i];
>>                 if (strcmp("representor", pair->key) == 0) {
>> -                       result = rte_eth_devargs_parse_list(pair->value,
>> -                               rte_eth_devargs_parse_representor_ports,
>> -                               eth_da);
>> +                       result = rte_eth_devargs_parse_representor_ports(
>> +                                       pair->value, eth_da);
>>                         if (result < 0)
>>                                 goto parse_cleanup;
>>                 }
>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h
>b/lib/librte_ethdev/rte_ethdev_driver.h
>> index 0eacfd8425..b66a955b18 100644
>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>> @@ -1193,6 +1193,12 @@ __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 */
>> +};
>> +
>>  /** Generic Ethernet device arguments  */
>>  struct rte_eth_devargs {
>>         uint16_t ports[RTE_MAX_ETHPORTS];
>> @@ -1203,6 +1209,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 */
>>  };
>>
>>  /**
>> --
>> 2.25.1
>>
>
>--
>This electronic communication and the information and any files transmitted
>with it, or attached to it, are confidential and are intended solely for
>the use of the individual or entity to whom it is addressed and may contain
>information that is confidential, legally privileged, protected by privacy
>laws, or otherwise restricted from disclosure to anyone else. If you are
>not the intended recipient or the person responsible for delivering the
>e-mail to the intended recipient, you are hereby notified that any use,
>copying, distributing, dissemination, forwarding, printing, or copying of
>this e-mail is strictly prohibited. If you received this e-mail in error,
>please return the e-mail to the sender, delete it from your computer, and
>destroy any printed copy of it.
  
Somnath Kotur Jan. 7, 2021, 6:40 a.m. UTC | #3
On Thu, Jan 7, 2021 at 12:08 PM Xueming(Steven) Li <xuemingl@nvidia.com> wrote:
>
>
>
> >-----Original Message-----
> >From: Somnath Kotur <somnath.kotur@broadcom.com>
> >Sent: Thursday, January 7, 2021 2:32 PM
> >To: Xueming(Steven) Li <xuemingl@nvidia.com>
> >Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit
> ><ferruh.yigit@intel.com>; Andrew Rybchenko
> ><andrew.rybchenko@oktetlabs.ru>; Olivier Matz <olivier.matz@6wind.com>;
> >Slava Ovsiienko <viacheslavo@nvidia.com>; dev <dev@dpdk.org>; Asaf Penso
> ><asafp@nvidia.com>
> >Subject: Re: [dpdk-dev] [PATCH v2 1/9] ethdev: refactor representor
> >infrastructure
> >
> >On Wed, Jan 6, 2021 at 9:48 PM Xueming Li <xuemingl@nvidia.com> wrote:
> >>
> >> To support extended representor syntax, this patch refactor represntor
Please fix this Typo in 'representor' as well ...Thanks
> >> infrastructure:
> >> 1. introduces representor type enum
> >> 2. devargs representor port range extraction from partial value
> >>
> >> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> >> ---
> >>  drivers/net/bnxt/bnxt_ethdev.c        | 12 ++++
> >>  drivers/net/enic/enic_ethdev.c        |  7 ++
> >>  drivers/net/i40e/i40e_ethdev.c        |  8 +++
> >>  drivers/net/ixgbe/ixgbe_ethdev.c      |  8 +++
> >>  drivers/net/mlx5/linux/mlx5_os.c      | 11 ++++
> >>  lib/librte_ethdev/ethdev_private.c    | 93 ++++++++++++---------------
> >>  lib/librte_ethdev/ethdev_private.h    |  3 -
> >>  lib/librte_ethdev/rte_class_eth.c     |  4 +-
> >>  lib/librte_ethdev/rte_ethdev.c        |  5 +-
> >>  lib/librte_ethdev/rte_ethdev_driver.h |  7 ++
> >>  10 files changed, 98 insertions(+), 60 deletions(-)
> >>
> >> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> >b/drivers/net/bnxt/bnxt_ethdev.c
> >> index 81c8f8d79d..844a6c3c66 100644
> >> --- a/drivers/net/bnxt/bnxt_ethdev.c
> >> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> >> @@ -5520,6 +5520,18 @@ static int bnxt_rep_port_probe(struct
> >rte_pci_device *pci_dev,
> >>         int i, ret = 0;
> >>         struct rte_kvargs *kvlist = NULL;
> >>
> >> +       if (eth_da->type == RTE_ETH_REPRESENTOR_NONE)
> >> +               return 0;
> >> +       if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
> >> +               PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
> >> +                           eth_da->type);
> >> +               return -ENOTSUP;
> >> +       }
> >> +       if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
> >Seems like an extra 'if ' condition by mistake? Otherwise there is no
> >diff b/n this 'if' condition and the one few lines above?
>
> Thanks, good catch!
>
> >> +               PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
> >> +                           eth_da->type);
> >> +               return -EINVAL;
> >> +       }
> >>         num_rep = eth_da->nb_representor_ports;
> >>         if (num_rep > BNXT_MAX_VF_REPS) {
> >>                 PMD_DRV_LOG(ERR, "nb_representor_ports = %d > %d MAX VF
> >REPS\n",
> >> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> >> index d041a6bee9..dd085caa93 100644
> >> --- a/drivers/net/enic/enic_ethdev.c
> >> +++ b/drivers/net/enic/enic_ethdev.c
> >> @@ -1303,6 +1303,13 @@ static int eth_enic_pci_probe(struct
> >rte_pci_driver *pci_drv __rte_unused,
> >>                 if (retval)
> >>                         return retval;
> >>         }
> >> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
> >> +               return 0;
> >> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> >> +               ENICPMD_LOG(ERR, "unsupported representor type: %s\n",
> >> +                           pci_dev->device.devargs->args);
> >> +               return -ENOTSUP;
> >> +       }
> >>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
> >>                 sizeof(struct enic),
> >>                 eth_dev_pci_specific_init, pci_dev,
> >> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> >> index f54769c29d..05ed2e1079 100644
> >> --- a/drivers/net/i40e/i40e_ethdev.c
> >> +++ b/drivers/net/i40e/i40e_ethdev.c
> >> @@ -640,6 +640,14 @@ eth_i40e_pci_probe(struct rte_pci_driver *pci_drv
> >__rte_unused,
> >>                         return retval;
> >>         }
> >>
> >> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
> >> +               return 0;
> >> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> >> +               PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
> >> +                           pci_dev->device.devargs->args);
> >> +               return -ENOTSUP;
> >> +       }
> >> +
> >>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
> >>                 sizeof(struct i40e_adapter),
> >>                 eth_dev_pci_specific_init, pci_dev,
> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> index 9a47a8b262..9ea0139197 100644
> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> @@ -1717,6 +1717,14 @@ eth_ixgbe_pci_probe(struct rte_pci_driver
> >*pci_drv __rte_unused,
> >>         } else
> >>                 memset(&eth_da, 0, sizeof(eth_da));
> >>
> >> +       if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
> >> +               return 0;
> >> +       if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> >> +               PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
> >> +                           pci_dev->device.devargs->args);
> >> +               return -ENOTSUP;
> >> +       }
> >> +
> >>         retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
> >>                 sizeof(struct ixgbe_adapter),
> >>                 eth_dev_pci_specific_init, pci_dev,
> >> diff --git a/drivers/net/mlx5/linux/mlx5_os.c
> >b/drivers/net/mlx5/linux/mlx5_os.c
> >> index 6812a1f215..6981ba1f41 100644
> >> --- a/drivers/net/mlx5/linux/mlx5_os.c
> >> +++ b/drivers/net/mlx5/linux/mlx5_os.c
> >> @@ -706,6 +706,17 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
> >>                                 strerror(rte_errno));
> >>                         return NULL;
> >>                 }
> >> +               if (eth_da.type != RTE_ETH_REPRESENTOR_NONE) {
> >> +                       /* Representor not specified. */
> >> +                       rte_errno = EBUSY;
> >> +                       return NULL;
> >> +               }
> >> +               if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
> >> +                       rte_errno = ENOTSUP;
> >> +                       DRV_LOG(ERR, "unsupported representor type: %s",
> >> +                               dpdk_dev->devargs->args);
> >> +                       return NULL;
> >> +               }
> >>                 for (i = 0; i < eth_da.nb_representor_ports; ++i)
> >>                         if (eth_da.representor_ports[i] ==
> >>                             (uint16_t)switch_info->port_name)
> >> diff --git a/lib/librte_ethdev/ethdev_private.c
> >b/lib/librte_ethdev/ethdev_private.c
> >> index 162a502fe7..c219164a4a 100644
> >> --- a/lib/librte_ethdev/ethdev_private.c
> >> +++ b/lib/librte_ethdev/ethdev_private.c
> >> @@ -38,60 +38,13 @@ eth_find_device(const struct rte_eth_dev *start,
> >rte_eth_cmp_t cmp,
> >>         return NULL;
> >>  }
> >>
> >> -int
> >> -rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
> >> -       void *data)
> >> -{
> >> -       char *str_start;
> >> -       int state;
> >> -       int result;
> >> -
> >> -       if (*str != '[')
> >> -               /* Single element, not a list */
> >> -               return callback(str, data);
> >> -
> >> -       /* Sanity check, then strip the brackets */
> >> -       str_start = &str[strlen(str) - 1];
> >> -       if (*str_start != ']') {
> >> -               RTE_LOG(ERR, EAL, "(%s): List does not end with ']'\n", str);
> >> -               return -EINVAL;
> >> -       }
> >> -       str++;
> >> -       *str_start = '\0';
> >> -
> >> -       /* Process list elements */
> >> -       state = 0;
> >> -       while (1) {
> >> -               if (state == 0) {
> >> -                       if (*str == '\0')
> >> -                               break;
> >> -                       if (*str != ',') {
> >> -                               str_start = str;
> >> -                               state = 1;
> >> -                       }
> >> -               } else if (state == 1) {
> >> -                       if (*str == ',' || *str == '\0') {
> >> -                               if (str > str_start) {
> >> -                                       /* Non-empty string fragment */
> >> -                                       *str = '\0';
> >> -                                       result = callback(str_start, data);
> >> -                                       if (result < 0)
> >> -                                               return result;
> >> -                               }
> >> -                               state = 0;
> >> -                       }
> >> -               }
> >> -               str++;
> >> -       }
> >> -       return 0;
> >> -}
> >> -
> >>  static int
> >>  rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
> >>         const uint16_t max_list)
> >>  {
> >>         uint16_t lo, hi, val;
> >>         int result;
> >> +       char *pos = str;
> >>
> >>         result = sscanf(str, "%hu-%hu", &lo, &hi);
> >>         if (result == 1) {
> >> @@ -99,7 +52,7 @@ rte_eth_devargs_process_range(char *str, uint16_t
> >*list, uint16_t *len_list,
> >>                         return -ENOMEM;
> >>                 list[(*len_list)++] = lo;
> >>         } else if (result == 2) {
> >> -               if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
> >> +               if (lo >= hi)
> >>                         return -EINVAL;
> >>                 for (val = lo; val <= hi; val++) {
> >>                         if (*len_list >= max_list)
> >> @@ -108,14 +61,52 @@ rte_eth_devargs_process_range(char *str, uint16_t
> >*list, uint16_t *len_list,
> >>                 }
> >>         } else
> >>                 return -EINVAL;
> >> -       return 0;
> >> +       while (*pos != 0 && ((*pos >= '0' && *pos <= '9') || *pos == '-'))
> >> +               pos++;
> >> +       return pos - str;
> >>  }
> >>
> >> +static int
> >> +rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
> >> +       const uint16_t max_list)
> >> +{
> >> +       char *pos = str;
> >> +       int ret;
> >> +
> >> +       if (*pos == '[')
> >> +               pos++;
> >> +       while (1) {
> >> +               ret = rte_eth_devargs_process_range(pos, list, len_list,
> >> +                                                   max_list);
> >> +               if (ret < 0)
> >> +                       return ret;
> >> +               pos += ret;
> >> +               if (*pos != ',') /* end of list */
> >> +                       break;
> >> +               pos++;
> >> +       }
> >> +       if (*str == '[' && *pos != ']')
> >> +               return -EINVAL;
> >> +       if (*pos == ']')
> >> +               pos++;
> >> +       return pos - str;
> >> +}
> >> +
> >> +/*
> >> + * representor format:
> >> + *   #: range or single number of VF representor - legacy
> >> + */
> >>  int
> >>  rte_eth_devargs_parse_representor_ports(char *str, void *data)
> >>  {
> >>         struct rte_eth_devargs *eth_da = data;
> >> +       int ret;
> >>
> >> -       return rte_eth_devargs_process_range(str, eth_da->representor_ports,
> >> +       /* Number # alone implies VF */
> >> +       eth_da->type = RTE_ETH_REPRESENTOR_VF;
> >> +       ret = rte_eth_devargs_process_list(str, eth_da->representor_ports,
> >>                 &eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
> >> +       if (ret < 0)
> >> +               RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
> >> +       return ret < 0 ? ret : 0;
> >>  }
> >> diff --git a/lib/librte_ethdev/ethdev_private.h
> >b/lib/librte_ethdev/ethdev_private.h
> >> index 905a45c337..220ddd4408 100644
> >> --- a/lib/librte_ethdev/ethdev_private.h
> >> +++ b/lib/librte_ethdev/ethdev_private.h
> >> @@ -26,9 +26,6 @@ eth_find_device(const struct rte_eth_dev *_start,
> >rte_eth_cmp_t cmp,
> >>                 const void *data);
> >>
> >>  /* Parse devargs value for representor parameter. */
> >> -typedef int (*rte_eth_devargs_callback_t)(char *str, void *data);
> >> -int rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t
> >callback,
> >> -       void *data);
> >>  int rte_eth_devargs_parse_representor_ports(char *str, void *data);
> >>
> >>  #ifdef __cplusplus
> >> diff --git a/lib/librte_ethdev/rte_class_eth.c
> >b/lib/librte_ethdev/rte_class_eth.c
> >> index 6338355e25..efe6149df5 100644
> >> --- a/lib/librte_ethdev/rte_class_eth.c
> >> +++ b/lib/librte_ethdev/rte_class_eth.c
> >> @@ -77,9 +77,7 @@ eth_representor_cmp(const char *key __rte_unused,
> >>         if (values == NULL)
> >>                 return -1;
> >>         memset(&representors, 0, sizeof(representors));
> >> -       ret = rte_eth_devargs_parse_list(values,
> >> -                       rte_eth_devargs_parse_representor_ports,
> >> -                       &representors);
> >> +       ret = rte_eth_devargs_parse_representor_ports(values, &representors);
> >>         free(values);
> >>         if (ret != 0)
> >>                 return -1; /* invalid devargs value */
> >> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> >> index 17ddacc78d..2ac51ac149 100644
> >> --- a/lib/librte_ethdev/rte_ethdev.c
> >> +++ b/lib/librte_ethdev/rte_ethdev.c
> >> @@ -5542,9 +5542,8 @@ rte_eth_devargs_parse(const char *dargs, struct
> >rte_eth_devargs *eth_da)
> >>         for (i = 0; i < args.count; i++) {
> >>                 pair = &args.pairs[i];
> >>                 if (strcmp("representor", pair->key) == 0) {
> >> -                       result = rte_eth_devargs_parse_list(pair->value,
> >> -                               rte_eth_devargs_parse_representor_ports,
> >> -                               eth_da);
> >> +                       result = rte_eth_devargs_parse_representor_ports(
> >> +                                       pair->value, eth_da);
> >>                         if (result < 0)
> >>                                 goto parse_cleanup;
> >>                 }
> >> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h
> >b/lib/librte_ethdev/rte_ethdev_driver.h
> >> index 0eacfd8425..b66a955b18 100644
> >> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> >> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> >> @@ -1193,6 +1193,12 @@ __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 */
> >> +};
> >> +
> >>  /** Generic Ethernet device arguments  */
> >>  struct rte_eth_devargs {
> >>         uint16_t ports[RTE_MAX_ETHPORTS];
> >> @@ -1203,6 +1209,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 */
> >>  };
> >>
> >>  /**
> >> --
> >> 2.25.1
> >>
> >
> >--
> >This electronic communication and the information and any files transmitted
> >with it, or attached to it, are confidential and are intended solely for
> >the use of the individual or entity to whom it is addressed and may contain
> >information that is confidential, legally privileged, protected by privacy
> >laws, or otherwise restricted from disclosure to anyone else. If you are
> >not the intended recipient or the person responsible for delivering the
> >e-mail to the intended recipient, you are hereby notified that any use,
> >copying, distributing, dissemination, forwarding, printing, or copying of
> >this e-mail is strictly prohibited. If you received this e-mail in error,
> >please return the e-mail to the sender, delete it from your computer, and
> >destroy any printed copy of it.
  

Patch

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 81c8f8d79d..844a6c3c66 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -5520,6 +5520,18 @@  static int bnxt_rep_port_probe(struct rte_pci_device *pci_dev,
 	int i, ret = 0;
 	struct rte_kvargs *kvlist = NULL;
 
+	if (eth_da->type == RTE_ETH_REPRESENTOR_NONE)
+		return 0;
+	if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
+		PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
+			    eth_da->type);
+		return -ENOTSUP;
+	}
+	if (eth_da->type != RTE_ETH_REPRESENTOR_VF) {
+		PMD_DRV_LOG(ERR, "unsupported representor type %d\n",
+			    eth_da->type);
+		return -EINVAL;
+	}
 	num_rep = eth_da->nb_representor_ports;
 	if (num_rep > BNXT_MAX_VF_REPS) {
 		PMD_DRV_LOG(ERR, "nb_representor_ports = %d > %d MAX VF REPS\n",
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index d041a6bee9..dd085caa93 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -1303,6 +1303,13 @@  static int eth_enic_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		if (retval)
 			return retval;
 	}
+	if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
+		return 0;
+	if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
+		ENICPMD_LOG(ERR, "unsupported representor type: %s\n",
+			    pci_dev->device.devargs->args);
+		return -ENOTSUP;
+	}
 	retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
 		sizeof(struct enic),
 		eth_dev_pci_specific_init, pci_dev,
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f54769c29d..05ed2e1079 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -640,6 +640,14 @@  eth_i40e_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 			return retval;
 	}
 
+	if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
+		return 0;
+	if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
+		PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
+			    pci_dev->device.devargs->args);
+		return -ENOTSUP;
+	}
+
 	retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
 		sizeof(struct i40e_adapter),
 		eth_dev_pci_specific_init, pci_dev,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 9a47a8b262..9ea0139197 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1717,6 +1717,14 @@  eth_ixgbe_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	} else
 		memset(&eth_da, 0, sizeof(eth_da));
 
+	if (eth_da.type == RTE_ETH_REPRESENTOR_NONE)
+		return 0;
+	if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
+		PMD_DRV_LOG(ERR, "unsupported representor type: %s\n",
+			    pci_dev->device.devargs->args);
+		return -ENOTSUP;
+	}
+
 	retval = rte_eth_dev_create(&pci_dev->device, pci_dev->device.name,
 		sizeof(struct ixgbe_adapter),
 		eth_dev_pci_specific_init, pci_dev,
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 6812a1f215..6981ba1f41 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -706,6 +706,17 @@  mlx5_dev_spawn(struct rte_device *dpdk_dev,
 				strerror(rte_errno));
 			return NULL;
 		}
+		if (eth_da.type != RTE_ETH_REPRESENTOR_NONE) {
+			/* Representor not specified. */
+			rte_errno = EBUSY;
+			return NULL;
+		}
+		if (eth_da.type != RTE_ETH_REPRESENTOR_VF) {
+			rte_errno = ENOTSUP;
+			DRV_LOG(ERR, "unsupported representor type: %s",
+				dpdk_dev->devargs->args);
+			return NULL;
+		}
 		for (i = 0; i < eth_da.nb_representor_ports; ++i)
 			if (eth_da.representor_ports[i] ==
 			    (uint16_t)switch_info->port_name)
diff --git a/lib/librte_ethdev/ethdev_private.c b/lib/librte_ethdev/ethdev_private.c
index 162a502fe7..c219164a4a 100644
--- a/lib/librte_ethdev/ethdev_private.c
+++ b/lib/librte_ethdev/ethdev_private.c
@@ -38,60 +38,13 @@  eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
 	return NULL;
 }
 
-int
-rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
-	void *data)
-{
-	char *str_start;
-	int state;
-	int result;
-
-	if (*str != '[')
-		/* Single element, not a list */
-		return callback(str, data);
-
-	/* Sanity check, then strip the brackets */
-	str_start = &str[strlen(str) - 1];
-	if (*str_start != ']') {
-		RTE_LOG(ERR, EAL, "(%s): List does not end with ']'\n", str);
-		return -EINVAL;
-	}
-	str++;
-	*str_start = '\0';
-
-	/* Process list elements */
-	state = 0;
-	while (1) {
-		if (state == 0) {
-			if (*str == '\0')
-				break;
-			if (*str != ',') {
-				str_start = str;
-				state = 1;
-			}
-		} else if (state == 1) {
-			if (*str == ',' || *str == '\0') {
-				if (str > str_start) {
-					/* Non-empty string fragment */
-					*str = '\0';
-					result = callback(str_start, data);
-					if (result < 0)
-						return result;
-				}
-				state = 0;
-			}
-		}
-		str++;
-	}
-	return 0;
-}
-
 static int
 rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
 	const uint16_t max_list)
 {
 	uint16_t lo, hi, val;
 	int result;
+	char *pos = str;
 
 	result = sscanf(str, "%hu-%hu", &lo, &hi);
 	if (result == 1) {
@@ -99,7 +52,7 @@  rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
 			return -ENOMEM;
 		list[(*len_list)++] = lo;
 	} else if (result == 2) {
-		if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi > RTE_MAX_ETHPORTS)
+		if (lo >= hi)
 			return -EINVAL;
 		for (val = lo; val <= hi; val++) {
 			if (*len_list >= max_list)
@@ -108,14 +61,52 @@  rte_eth_devargs_process_range(char *str, uint16_t *list, uint16_t *len_list,
 		}
 	} else
 		return -EINVAL;
-	return 0;
+	while (*pos != 0 && ((*pos >= '0' && *pos <= '9') || *pos == '-'))
+		pos++;
+	return pos - str;
 }
 
+static int
+rte_eth_devargs_process_list(char *str, uint16_t *list, uint16_t *len_list,
+	const uint16_t max_list)
+{
+	char *pos = str;
+	int ret;
+
+	if (*pos == '[')
+		pos++;
+	while (1) {
+		ret = rte_eth_devargs_process_range(pos, list, len_list,
+						    max_list);
+		if (ret < 0)
+			return ret;
+		pos += ret;
+		if (*pos != ',') /* end of list */
+			break;
+		pos++;
+	}
+	if (*str == '[' && *pos != ']')
+		return -EINVAL;
+	if (*pos == ']')
+		pos++;
+	return pos - str;
+}
+
+/*
+ * representor format:
+ *   #: range or single number of VF representor - legacy
+ */
 int
 rte_eth_devargs_parse_representor_ports(char *str, void *data)
 {
 	struct rte_eth_devargs *eth_da = data;
+	int ret;
 
-	return rte_eth_devargs_process_range(str, eth_da->representor_ports,
+	/* Number # alone implies VF */
+	eth_da->type = RTE_ETH_REPRESENTOR_VF;
+	ret = rte_eth_devargs_process_list(str, eth_da->representor_ports,
 		&eth_da->nb_representor_ports, RTE_MAX_ETHPORTS);
+	if (ret < 0)
+		RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str);
+	return ret < 0 ? ret : 0;
 }
diff --git a/lib/librte_ethdev/ethdev_private.h b/lib/librte_ethdev/ethdev_private.h
index 905a45c337..220ddd4408 100644
--- a/lib/librte_ethdev/ethdev_private.h
+++ b/lib/librte_ethdev/ethdev_private.h
@@ -26,9 +26,6 @@  eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp,
 		const void *data);
 
 /* Parse devargs value for representor parameter. */
-typedef int (*rte_eth_devargs_callback_t)(char *str, void *data);
-int rte_eth_devargs_parse_list(char *str, rte_eth_devargs_callback_t callback,
-	void *data);
 int rte_eth_devargs_parse_representor_ports(char *str, void *data);
 
 #ifdef __cplusplus
diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
index 6338355e25..efe6149df5 100644
--- a/lib/librte_ethdev/rte_class_eth.c
+++ b/lib/librte_ethdev/rte_class_eth.c
@@ -77,9 +77,7 @@  eth_representor_cmp(const char *key __rte_unused,
 	if (values == NULL)
 		return -1;
 	memset(&representors, 0, sizeof(representors));
-	ret = rte_eth_devargs_parse_list(values,
-			rte_eth_devargs_parse_representor_ports,
-			&representors);
+	ret = rte_eth_devargs_parse_representor_ports(values, &representors);
 	free(values);
 	if (ret != 0)
 		return -1; /* invalid devargs value */
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 17ddacc78d..2ac51ac149 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -5542,9 +5542,8 @@  rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
 	for (i = 0; i < args.count; i++) {
 		pair = &args.pairs[i];
 		if (strcmp("representor", pair->key) == 0) {
-			result = rte_eth_devargs_parse_list(pair->value,
-				rte_eth_devargs_parse_representor_ports,
-				eth_da);
+			result = rte_eth_devargs_parse_representor_ports(
+					pair->value, eth_da);
 			if (result < 0)
 				goto parse_cleanup;
 		}
diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 0eacfd8425..b66a955b18 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -1193,6 +1193,12 @@  __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 */
+};
+
 /** Generic Ethernet device arguments  */
 struct rte_eth_devargs {
 	uint16_t ports[RTE_MAX_ETHPORTS];
@@ -1203,6 +1209,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 */
 };
 
 /**