-----Original Message-----
From: Andrew Rybchenko <redacted>
Sent: Tuesday, January 19, 2021 5:04 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/19/21 11:59 AM, Xueming(Steven) Li wrote:

quoted

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,

quoted

quoted

quoted

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

quoted

quoted

quoted

 	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.

My bad, after another check, it should be max number of elements in array,
not max element value. Will update, thanks! 

[snip]