-----Original Message-----
From: Somnath Kotur <redacted>
Sent: Thursday, January 7, 2021 2:32 PM
To: Xueming(Steven) Li <redacted>
Cc: NBU-Contact-Thomas Monjalon <redacted>; Ferruh Yigit
[off-list ref]; Andrew Rybchenko
[off-list ref]; Olivier Matz [off-list ref];
Slava Ovsiienko [off-list ref]; dev [off-list ref]; Asaf Penso
[off-list ref]
Subject: Re: [dpdk-dev] [PATCH v2 1/9] ethdev: refactor representor
infrastructure

On Wed, Jan 6, 2021 at 9:48 PM Xueming Li [off-list ref] wrote:

quoted

To support extended representor syntax, this patch refactor represntor

Typo in 'representor'

quoted

infrastructure:
1. introduces representor type enum
2. devargs representor port range extraction from partial value

Signed-off-by: Xueming Li <redacted>
---
 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

quoted

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,

quoted

        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!

quoted

+               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",

quoted

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,

quoted

                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,

quoted

                        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

quoted

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,

quoted

        } 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

quoted

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

quoted

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,

quoted

        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,

quoted

                        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,

quoted

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

quoted

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,

quoted

                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,

quoted

-       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

quoted

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)

quoted

        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

quoted

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.