On 1/19/21 11:59 AM, Xueming(Steven) Li wrote:

Hi Andrew,

quoted

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

On 1/18/21 2:16 PM, Xueming Li wrote:

quoted

To support extended representor syntax, this patch extends the
representor list parsing to support for representor port range in
devargs, examples:
   representor=[1,2,3]         - single list
   representor=[1,3-5,7,9-11]  - list with singles and ranges

Signed-off-by: Xueming Li <redacted>
Acked-by: Viacheslav Ovsiienko <redacted>

See below

quoted

---
 lib/librte_ethdev/ethdev_private.c | 105 ++++++++++++++---------------
 lib/librte_ethdev/ethdev_private.h |   3 -
 lib/librte_ethdev/rte_class_eth.c  |   4 +-
 lib/librte_ethdev/rte_ethdev.c     |   5 +-
 4 files changed, 54 insertions(+), 63 deletions(-)

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

b/lib/librte_ethdev/ethdev_private.c
index c1a411dba4..12bcc7e98d 100644

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

@@ -38,77 +38,71 @@ 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)
+static int
+rte_eth_devargs_enlist(uint16_t *list, uint16_t *len_list,
+		       const uint16_t max_list, uint16_t val)
 {
-	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';
+	uint16_t i;

-	/* 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++;
+	if (*len_list >= max_list)
+		return -1;

If current length is equal to max, but added value is already is in the list, it
should not be an error. So, these two lines should be moved after below for
loop.

quoted

+	for (i = 0; i < *len_list; i++) {
+		if (list[i] == val)
+			return 0;
 	}
+	list[(*len_list)++] = val;
 	return 0;
 }

-static int
+static char *
 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) {
-		if (*len_list >= max_list)
-			return -ENOMEM;
-		list[(*len_list)++] = lo;
+		if (rte_eth_devargs_enlist(list, len_list, max_list, lo) != 0)
+			return NULL;
 	} else if (result == 2) {
-		if (lo >= hi || lo > RTE_MAX_ETHPORTS || hi >

RTE_MAX_ETHPORTS)

Strictly speaking removal of comparision vs RTE_MAX_ETHPORTS is a separate
logical change with a separate motivation.

To make this function comment for controller/pf/vf/sf parsing, the max value is
passed in as parameter and checked in rte_eth_devargs_enlist().
Caller code in rte_eth_devargs_process_list() decide the max value.

Which maximum? Maximum number of elements in array and
maximum element value are different things.

[snip]