[PATCH v4 0/8] add new command line argument parsing library

[PATCH v4 0/8] add new command line argument parsing library

[Euan Bourke]

A recent thread on the mailing list[1] discussed corelist and coremask
parsing and the idea of a new library dedicated to command line parsing
was mentioned[2]. This patchset adds the library, along with the new
APIs, and edits the existing EAL, DLB2 driver and some example
application functions to use these APIs, rather than each implementing
their own copies.

The new APIs work similar to the existing functions in EAL, however
instead of filling a core array like this:
[1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
It fills it like this:
[0, 3, 4] (with the value at each index being an 'active core').

The new APIs will also return the number of cores contained in the
passed corelist/coremask, so in the above example, 3 would be returned.

Also included in this patchest is a heuristic parser which searches
for key markers in the core string, returning a enum value based off
this search to indicate if a parameter is likely a coremask or a
corelist. This heuristic function is also wrapped in a parser
function allowing apps to handle both coremasks and corelists
simultaneously.

[1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
[2] https://mails.dpdk.org/archives/dev/2023-November/280966.html


v4:
* functions now return -EINVAL instead of -1.
* enum moved to header file.
* documentation changes for rte_arg_parse_arg_type() function.
* minor changes for issues flagged during review.

v3:
* new 'combined core string parser' and 'heuristic parser'
* changes to eventdev_pipeline and l3fwd-power example applications
* various struct optimisations in arg_parser.c
* fix for windows build relating to RTE_SWAP()
* minor changes for issues flagged during review

v2:
* changes to EAL service core related parsers to call API.
* various optimisations in core_bit related functions in arg_parser.c.
* add lib to list for windows build.
* minor changes for issues flagged during review.

Euan Bourke (8):
  arg_parser: new library for command line parsing
  arg_parser: add new coremask parsing API
  eal: add support for new arg parsing library
  eal: update to service core related parsers
  event/dlb2: add new arg parsing library API support
  arg_parser: added common core string and heuristic parsers
  examples/eventdev_pipeline: update to call arg parser API
  examples/l3fwd-power: update to call arg parser API

 .mailmap                                     |   1 +
 MAINTAINERS                                  |   4 +
 doc/api/doxy-api-index.md                    |   1 +
 doc/api/doxy-api.conf.in                     |   1 +
 drivers/event/dlb2/dlb2_priv.h               |   4 +-
 drivers/event/dlb2/pf/base/dlb2_resource.c   |  51 ++--
 examples/eventdev_pipeline/main.c            |  66 +----
 examples/eventdev_pipeline/pipeline_common.h |   1 +
 examples/l3fwd-power/perf_core.c             |  52 +---
 lib/arg_parser/arg_parser.c                  | 221 ++++++++++++++
 lib/arg_parser/meson.build                   |   7 +
 lib/arg_parser/rte_arg_parser.h              | 164 +++++++++++
 lib/arg_parser/version.map                   |  13 +
 lib/eal/common/eal_common_options.c          | 285 ++++---------------
 lib/eal/meson.build                          |   2 +-
 lib/meson.build                              |   2 +
 16 files changed, 510 insertions(+), 365 deletions(-)
 create mode 100644 lib/arg_parser/arg_parser.c
 create mode 100644 lib/arg_parser/meson.build
 create mode 100644 lib/arg_parser/rte_arg_parser.h
 create mode 100644 lib/arg_parser/version.map

-- 
2.34.1

[PATCH v4 1/8] arg_parser: new library for command line parsing

[Euan Bourke]

Add a new library to make it easier for eal and other libraries to parse
command line arguments.

The first function in this library is one to parse a corelist string
into an array of individual core ids. The function will then return the
total number of cores described in the corelist.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 .mailmap                        |   1 +
 MAINTAINERS                     |   4 ++
 doc/api/doxy-api-index.md       |   1 +
 doc/api/doxy-api.conf.in        |   1 +
 lib/arg_parser/arg_parser.c     | 108 ++++++++++++++++++++++++++++++++
 lib/arg_parser/meson.build      |   7 +++
 lib/arg_parser/rte_arg_parser.h |  66 +++++++++++++++++++
 lib/arg_parser/version.map      |  10 +++
 lib/meson.build                 |   2 +
 9 files changed, 200 insertions(+)
 create mode 100644 lib/arg_parser/arg_parser.c
 create mode 100644 lib/arg_parser/meson.build
 create mode 100644 lib/arg_parser/rte_arg_parser.h
 create mode 100644 lib/arg_parser/version.map

diff --git a/.mailmap b/.mailmap
index ab0742a382..528bc68a30 100644
--- a/.mailmap
+++ b/.mailmap
@@ -379,6 +379,7 @@ Eric Zhang <eric.zhang@windriver.com>
 Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
 Erik Ziegenbalg <eziegenb@brocade.com>
 Erlu Chen <erlu.chen@intel.com>
+Euan Bourke <euan.bourke@intel.com>
 Eugenio Pérez <eperezma@redhat.com>
 Eugeny Parshutin <eugeny.parshutin@linux.intel.com>
 Evan Swanson <evan.swanson@intel.com>
diff --git a/MAINTAINERS b/MAINTAINERS
index 0d1c8126e3..68ef5ba14b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1756,6 +1756,10 @@ M: Nithin Dabilpuram <ndabilpuram@marvell.com>
 M: Pavan Nikhilesh <pbhagavatula@marvell.com>
 F: lib/node/
 
+Argument parsing
+M: Bruce Richardson <bruce.richardson@intel.com>
+F: lib/arg_parser/
+
 
 Test Applications
 -----------------
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index a6a768bd7c..743d3b6773 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -215,6 +215,7 @@ The public API headers are grouped by topics:
     [udp4_input_node](@ref rte_node_udp4_input_api.h)
 
 - **basic**:
+  [argument parsing](@ref rte_arg_parser.h),
   [bitops](@ref rte_bitops.h),
   [approx fraction](@ref rte_approx.h),
   [random](@ref rte_random.h),
diff --git a/doc/api/doxy-api.conf.in b/doc/api/doxy-api.conf.in
index e94c9e4e46..05718ba6ed 100644
--- a/doc/api/doxy-api.conf.in
+++ b/doc/api/doxy-api.conf.in
@@ -28,6 +28,7 @@ INPUT                   = @TOPDIR@/doc/api/doxy-api-index.md \
                           @TOPDIR@/lib/eal/include \
                           @TOPDIR@/lib/eal/include/generic \
                           @TOPDIR@/lib/acl \
+                          @TOPDIR@/lib/arg_parser \
                           @TOPDIR@/lib/bbdev \
                           @TOPDIR@/lib/bitratestats \
                           @TOPDIR@/lib/bpf \
diff --git a/lib/arg_parser/arg_parser.c b/lib/arg_parser/arg_parser.c
new file mode 100644
index 0000000000..d8324a27b1
--- /dev/null
+++ b/lib/arg_parser/arg_parser.c
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Intel Corporation
+ */
+
+#include <errno.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <string.h>
+#include <stdbool.h>
+
+#include <rte_arg_parser.h>
+#include <rte_common.h>
+
+
+struct core_bits {
+	uint8_t bits[(UINT16_MAX + 1) / CHAR_BIT];
+	uint16_t max_bit_set;
+	uint16_t min_bit_set;
+	uint32_t total_bits_set;
+};
+
+static inline bool
+get_core_bit(struct core_bits *mask, uint16_t idx)
+{
+	return !!(mask->bits[idx / CHAR_BIT] & (1 << (idx % CHAR_BIT)));
+}
+
+static inline void
+set_core_bit(struct core_bits *mask, uint16_t idx)
+{
+	if (get_core_bit(mask, idx))
+		return;
+
+	mask->bits[idx / CHAR_BIT] |= 1 << (idx % CHAR_BIT);
+	/* Update min and max bit if its first time setting a bit */
+	if (++(mask->total_bits_set) == 1) {
+		mask->min_bit_set = idx;
+		mask->max_bit_set = idx;
+		return;
+	}
+
+	if (idx > mask->max_bit_set)
+		mask->max_bit_set = idx;
+
+	if (idx < mask->min_bit_set)
+		mask->min_bit_set = idx;
+}
+
+static inline uint32_t
+corebits_to_array(struct core_bits *mask, uint16_t *cores, size_t max_cores)
+{
+	uint32_t count = 0;
+	for (uint32_t i = mask->min_bit_set; i <= mask->max_bit_set && count < max_cores; i++) {
+		if (get_core_bit(mask, i))
+			cores[count++] = i;
+	}
+	return mask->total_bits_set;
+}
+
+
+int
+rte_arg_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len)
+{
+	struct core_bits mask = {0};
+	int32_t min = -1;
+	char *end = NULL;
+
+	min = -1;
+	do {
+		int64_t idx;
+		int32_t max;
+
+		while (isblank(*corelist))
+			corelist++;
+		if (!isdigit(*corelist))
+			return -EINVAL;
+
+		errno = 0;
+		idx = strtol(corelist, &end, 10);
+		if (errno || end == NULL || idx > UINT16_MAX)
+			return -EINVAL;
+		while (isblank(*end))
+			end++;
+		if (*end == '-')
+			min = idx;
+
+		else if (*end == ',' || *end == '\0') {
+			/* Check for first number in range */
+			if (min == -1)
+				min = max = idx;
+			/* Swap if min is larger than max */
+			else if (min > idx) {
+				max = min;
+				min = idx;
+			} else /* Normal case */
+				max = idx;
+
+			for (; min <= max; min++)
+				set_core_bit(&mask, min);
+
+			min = -1;
+		} else
+			return -EINVAL;
+		corelist = end + 1;
+	} while (*end != '\0');
+
+	return corebits_to_array(&mask, cores, cores_len);
+}
diff --git a/lib/arg_parser/meson.build b/lib/arg_parser/meson.build
new file mode 100644
index 0000000000..6ee228bd69
--- /dev/null
+++ b/lib/arg_parser/meson.build
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023 Intel Corporation
+
+sources = files('arg_parser.c')
+headers = files('rte_arg_parser.h')
+
+includes += global_inc
diff --git a/lib/arg_parser/rte_arg_parser.h b/lib/arg_parser/rte_arg_parser.h
new file mode 100644
index 0000000000..a1ef428b95
--- /dev/null
+++ b/lib/arg_parser/rte_arg_parser.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Intel Corporation
+ */
+
+#ifndef _RTE_ARG_PARSER_H_
+#define _RTE_ARG_PARSER_H_
+
+/**
+ * @file
+ *
+ * RTE Argument Parsing API
+ *
+ * The argument parsing API is a collection of functions to help parse
+ * command line arguments. The API takes a string input and will return
+ * it to the user in a more usable format.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdint.h>
+
+#include <rte_compat.h>
+
+
+/**
+ * Convert a string describing a list of core ids into an array of core ids.
+ *
+ * On success, the passed array is filled with the core ids present in the list up
+ * to the "cores_len", and the number of unique cores present in the "corelist"
+ * is returned.
+ * For example, passing a 1-3,6 "corelist" results in an array of [1, 2, 3, 6]
+ * and would return 4.
+ *
+ * NOTE: if the length of the input array is insufficient to hold the number of core ids
+ * in "corelist" the input array is filled to capacity but the return value is the
+ * number of elements which would have been written to the array, had enough space been
+ * available. [This is similar to the behaviour of the snprintf function]. Because of
+ * this, the number of core values in the "corelist" may be determined by calling the
+ * function with a NULL array pointer and array length given as 0.
+ *
+ * @param corelist
+ *   Input string describing a list of core ids.
+ * @param cores
+ *   An array where to store the core ids.
+ *   Array can be NULL if "cores_len" is 0.
+ * @param cores_len
+ *   The length of the "cores" array.
+ *   If the size is smaller than that needed to hold all cores from "corelist",
+ *   only "cores_len" elements will be written to the array.
+ * @return
+ *   n: the number of unique cores present in "corelist".
+ *   -EINVAL if the string was invalid.
+ *   NOTE: if n > "cores_len", then only "cores_len" elements in the "cores" array are valid.
+ */
+__rte_experimental
+int
+rte_arg_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len);
+
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ARG_PARSER_H_ */
diff --git a/lib/arg_parser/version.map b/lib/arg_parser/version.map
new file mode 100644
index 0000000000..b0caaac569
--- /dev/null
+++ b/lib/arg_parser/version.map
@@ -0,0 +1,10 @@
+DPDK_24 {
+	local: *;
+};
+
+EXPERIMENTAL {
+	global:
+
+	# added in 24.03
+	rte_arg_parse_corelist;
+};
diff --git a/lib/meson.build b/lib/meson.build
index 6c143ce5a6..db9e769033 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -11,6 +11,7 @@
 libraries = [
         'log',
         'kvargs', # eal depends on kvargs
+        'arg_parser',
         'telemetry', # basic info querying
         'eal', # everything depends on eal
         'ring',
@@ -72,6 +73,7 @@ if is_ms_compiler
             'log',
             'kvargs',
             'telemetry',
+            'arg_parser',
     ]
 endif
 
-- 
2.34.1

[PATCH v4 2/8] arg_parser: add new coremask parsing API

[Euan Bourke]

Add new coremask parsing API. This API behaves similarly
to the corelist parsing API, taking a coremask string, a cores
array and a cores_len int. Parsing the coremask string, filling
its values into the cores array up to cores_len.

The API also returns a 'count' which corresponds to the total number
of cores in the coremask string.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 lib/arg_parser/arg_parser.c     | 51 +++++++++++++++++++++++++++++++++
 lib/arg_parser/rte_arg_parser.h | 34 ++++++++++++++++++++++
 lib/arg_parser/version.map      |  1 +
 3 files changed, 86 insertions(+)

diff --git a/lib/arg_parser/arg_parser.c b/lib/arg_parser/arg_parser.c
index d8324a27b1..8d86a7b618 100644
--- a/lib/arg_parser/arg_parser.c
+++ b/lib/arg_parser/arg_parser.c
@@ -11,6 +11,9 @@
 #include <rte_arg_parser.h>
 #include <rte_common.h>
 
+#define BITS_PER_HEX 4
+#define MAX_COREMASK_SIZE ((UINT16_MAX + 1) / BITS_PER_HEX)
+
 
 struct core_bits {
 	uint8_t bits[(UINT16_MAX + 1) / CHAR_BIT];
@@ -57,6 +60,15 @@ corebits_to_array(struct core_bits *mask, uint16_t *cores, size_t max_cores)
 	return mask->total_bits_set;
 }
 
+static int xdigit2val(unsigned char c)
+{
+	if (isdigit(c))
+		return c - '0';
+	else if (isupper(c))
+		return c - 'A' + 10;
+	else
+		return c - 'a' + 10;
+}
 
 int
 rte_arg_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len)
@@ -106,3 +118,42 @@ rte_arg_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len
 
 	return corebits_to_array(&mask, cores, cores_len);
 }
+
+int
+rte_arg_parse_coremask(const char *coremask, uint16_t *cores, uint32_t cores_len)
+{
+	struct core_bits mask = {0};
+
+	/* Remove all blank characters ahead and after .
+	 * Remove 0x/0X if exists.
+	 */
+	while (isblank(*coremask))
+		coremask++;
+	if (coremask[0] == '0' && ((coremask[1] == 'x') || (coremask[1] == 'X')))
+		coremask += 2;
+
+	int32_t i = strlen(coremask);
+	while ((i > 0) && isblank(coremask[i - 1]))
+		i--;
+	if (i == 0 || i > MAX_COREMASK_SIZE)
+		return -EINVAL;
+
+	uint32_t idx = 0;
+
+	for (i = i - 1; i >= 0; i--) {
+		int val;
+		char c = coremask[i];
+
+		if (isxdigit(c) == 0)
+			return -EINVAL;
+
+		val = xdigit2val(c);
+
+		for (uint8_t j = 0; j < BITS_PER_HEX; j++, idx++) {
+			if ((1 << j) & val)
+				set_core_bit(&mask, idx);
+		}
+	}
+
+	return corebits_to_array(&mask, cores, cores_len);
+}
diff --git a/lib/arg_parser/rte_arg_parser.h b/lib/arg_parser/rte_arg_parser.h
index a1ef428b95..49d7daa204 100644
--- a/lib/arg_parser/rte_arg_parser.h
+++ b/lib/arg_parser/rte_arg_parser.h
@@ -58,6 +58,40 @@ __rte_experimental
 int
 rte_arg_parse_corelist(const char *corelist, uint16_t *cores, uint32_t cores_len);
 
+/**
+ * Convert a string describing a bitmask of core ids into an array of core ids.
+ *
+ * On success, the passed array is filled with the core ids present in the
+ * bitmask up to the "cores_len", and the number of unique cores present in the
+ * "coremask" is returned.
+ * For example, passing a 0xA "coremask" results in an array of [1, 3]
+ * and would return 2.
+ *
+ * NOTE: if the length of the input array is insufficient to hold the number of core ids
+ * in "coremask" the input array is filled to capacity but the return value is the
+ * number of elements which would have been written to the array, had enough space been
+ * available. [This is similar to the behaviour of the snprintf function]. Because of
+ * this, the number of core values in the "coremask" may be determined by calling the
+ * function with a NULL array pointer and array length given as 0.
+ *
+ * @param coremask
+ *   A string containing a bitmask of core ids.
+ * @param cores
+ *   An array where to store the core ids.
+ *   Array can be NULL if "cores_len" is 0.
+ * @param cores_len
+ *   The length of the "cores" array.
+ *   If the size is smaller than that needed to hold all cores from "coremask",
+ *   only "cores_len" elements will be written to the array.
+ * @return
+ *   n: the number of unique cores present in "coremask".
+ *   -EINVAL if the string was invalid.
+ *   NOTE: if n > "cores_len", then only "cores_len" elements in the "cores" array are valid.
+ */
+__rte_experimental
+int
+rte_arg_parse_coremask(const char *coremask, uint16_t *cores, uint32_t cores_len);
+
 
 #ifdef __cplusplus
 }
diff --git a/lib/arg_parser/version.map b/lib/arg_parser/version.map
index b0caaac569..b44d4b02b7 100644
--- a/lib/arg_parser/version.map
+++ b/lib/arg_parser/version.map
@@ -7,4 +7,5 @@ EXPERIMENTAL {
 
 	# added in 24.03
 	rte_arg_parse_corelist;
+	rte_arg_parse_coremask;
 };
-- 
2.34.1

[PATCH v4 3/8] eal: add support for new arg parsing library

[Euan Bourke]

Update to eal functions relating to corelist and coremask
parsing to support the new arg parsing library. Functions
now call the API instead of implementing their own version.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 lib/eal/common/eal_common_options.c | 114 ++++------------------------
 lib/eal/meson.build                 |   2 +-
 2 files changed, 14 insertions(+), 102 deletions(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index a6d21f1cba..60ba12a368 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -30,6 +30,7 @@
 #include <rte_tailq.h>
 #include <rte_version.h>
 #include <rte_devargs.h>
+#include <rte_arg_parser.h>
 #include <rte_memcpy.h>
 #ifndef RTE_EXEC_ENV_WINDOWS
 #include <rte_telemetry.h>
@@ -706,7 +707,7 @@ update_lcore_config(int *cores)
 }
 
 static int
-check_core_list(int *lcores, unsigned int count)
+check_core_list(uint16_t *lcores, unsigned int count)
 {
 	char lcorestr[RTE_MAX_LCORE * 10];
 	bool overflow = false;
@@ -746,60 +747,18 @@ check_core_list(int *lcores, unsigned int count)
 int
 rte_eal_parse_coremask(const char *coremask, int *cores)
 {
-	const char *coremask_orig = coremask;
-	int lcores[RTE_MAX_LCORE];
-	unsigned int count = 0;
-	int i, j, idx;
-	int val;
-	char c;
+	int count;
+	uint16_t lcores[RTE_MAX_LCORE];
+	int idx;
 
 	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
 		cores[idx] = -1;
-	idx = 0;
 
-	/* Remove all blank characters ahead and after .
-	 * Remove 0x/0X if exists.
-	 */
-	while (isblank(*coremask))
-		coremask++;
-	if (coremask[0] == '0' && ((coremask[1] == 'x')
-		|| (coremask[1] == 'X')))
-		coremask += 2;
-	i = strlen(coremask);
-	while ((i > 0) && isblank(coremask[i - 1]))
-		i--;
-	if (i == 0) {
-		RTE_LOG(ERR, EAL, "No lcores in coremask: [%s]\n",
-			coremask_orig);
-		return -1;
-	}
+	/* Call public coremask parsing API */
+	count = rte_arg_parse_coremask(coremask, lcores, RTE_MAX_LCORE);
 
-	for (i = i - 1; i >= 0; i--) {
-		c = coremask[i];
-		if (isxdigit(c) == 0) {
-			/* invalid characters */
-			RTE_LOG(ERR, EAL, "invalid characters in coremask: [%s]\n",
-				coremask_orig);
-			return -1;
-		}
-		val = xdigit2val(c);
-		for (j = 0; j < BITS_PER_HEX; j++, idx++)
-		{
-			if ((1 << j) & val) {
-				if (count >= RTE_MAX_LCORE) {
-					RTE_LOG(ERR, EAL, "Too many lcores provided. Cannot exceed RTE_MAX_LCORE (%d)\n",
-						RTE_MAX_LCORE);
-					return -1;
-				}
-				lcores[count++] = idx;
-			}
-		}
-	}
-	if (count == 0) {
-		RTE_LOG(ERR, EAL, "No lcores in coremask: [%s]\n",
-			coremask_orig);
+	if (count <= 0 || count > RTE_MAX_LCORE)
 		return -1;
-	}
 
 	if (check_core_list(lcores, count))
 		return -1;
@@ -898,64 +857,17 @@ eal_parse_service_corelist(const char *corelist)
 static int
 eal_parse_corelist(const char *corelist, int *cores)
 {
-	unsigned int count = 0, i;
-	int lcores[RTE_MAX_LCORE];
-	char *end = NULL;
-	int min, max;
+	int count;
+	uint16_t lcores[RTE_MAX_LCORE];
 	int idx;
 
 	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
 		cores[idx] = -1;
 
-	/* Remove all blank characters ahead */
-	while (isblank(*corelist))
-		corelist++;
+	/* Call public corelist parsing API */
+	count = rte_arg_parse_corelist(corelist, lcores, RTE_MAX_LCORE);
 
-	/* Get list of cores */
-	min = -1;
-	do {
-		while (isblank(*corelist))
-			corelist++;
-		if (*corelist == '\0')
-			return -1;
-		errno = 0;
-		idx = strtol(corelist, &end, 10);
-		if (errno || end == NULL)
-			return -1;
-		if (idx < 0)
-			return -1;
-		while (isblank(*end))
-			end++;
-		if (*end == '-') {
-			min = idx;
-		} else if ((*end == ',') || (*end == '\0')) {
-			max = idx;
-			if (min == -1)
-				min = idx;
-			for (idx = min; idx <= max; idx++) {
-				bool dup = false;
-
-				/* Check if this idx is already present */
-				for (i = 0; i < count; i++) {
-					if (lcores[i] == idx)
-						dup = true;
-				}
-				if (dup)
-					continue;
-				if (count >= RTE_MAX_LCORE) {
-					RTE_LOG(ERR, EAL, "Too many lcores provided. Cannot exceed RTE_MAX_LCORE (%d)\n",
-						RTE_MAX_LCORE);
-					return -1;
-				}
-				lcores[count++] = idx;
-			}
-			min = -1;
-		} else
-			return -1;
-		corelist = end + 1;
-	} while (*end != '\0');
-
-	if (count == 0)
+	if (count <= 0 || count > RTE_MAX_LCORE)
 		return -1;
 
 	if (check_core_list(lcores, count))
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index e1d6c4cf17..52a22e2e66 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -14,7 +14,7 @@ subdir(exec_env)
 
 subdir(arch_subdir)
 
-deps += ['log', 'kvargs']
+deps += ['log', 'kvargs', 'arg_parser']
 if not is_windows
     deps += ['telemetry']
 endif
-- 
2.34.1

[PATCH v4 4/8] eal: update to service core related parsers

[Euan Bourke]

Updates to the parse service cores functions in EAL to call the arg
parser API instead of implementing its own versions.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 lib/eal/common/eal_common_options.c | 171 +++++++---------------------
 1 file changed, 41 insertions(+), 130 deletions(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 60ba12a368..d44654f621 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -573,97 +573,49 @@ eal_plugins_init(void)
  * the global configuration (core role and core count) with the parsed
  * value.
  */
-static int xdigit2val(unsigned char c)
-{
-	int val;
-
-	if (isdigit(c))
-		val = c - '0';
-	else if (isupper(c))
-		val = c - 'A' + 10;
-	else
-		val = c - 'a' + 10;
-	return val;
-}
-
 static int
 eal_parse_service_coremask(const char *coremask)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
-	int i, j, idx = 0;
-	unsigned int count = 0;
-	char c;
-	int val;
+	uint16_t cores[RTE_MAX_LCORE];
+	int64_t count;
 	uint32_t taken_lcore_count = 0;
 
-	if (coremask == NULL)
-		return -1;
-	/* Remove all blank characters ahead and after .
-	 * Remove 0x/0X if exists.
-	 */
-	while (isblank(*coremask))
-		coremask++;
-	if (coremask[0] == '0' && ((coremask[1] == 'x')
-		|| (coremask[1] == 'X')))
-		coremask += 2;
-	i = strlen(coremask);
-	while ((i > 0) && isblank(coremask[i - 1]))
-		i--;
-
-	if (i == 0)
+	count = rte_arg_parse_coremask(coremask, cores, RTE_DIM(cores));
+
+	if (count == 0 || count == -1)
 		return -1;
 
-	for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
-		c = coremask[i];
-		if (isxdigit(c) == 0) {
-			/* invalid characters */
+	for (int i = 0; i < count; i++) {
+		uint32_t lcore = cores[i];
+		if (main_lcore_parsed &&
+				cfg->main_lcore == lcore) {
+			RTE_LOG(ERR, EAL,
+				"lcore %u is main lcore, cannot use as service core\n",
+				cores[i]);
 			return -1;
 		}
-		val = xdigit2val(c);
-		for (j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE;
-				j++, idx++) {
-			if ((1 << j) & val) {
-				/* handle main lcore already parsed */
-				uint32_t lcore = idx;
-				if (main_lcore_parsed &&
-						cfg->main_lcore == lcore) {
-					RTE_LOG(ERR, EAL,
-						"lcore %u is main lcore, cannot use as service core\n",
-						idx);
-					return -1;
-				}
 
-				if (eal_cpu_detected(idx) == 0) {
-					RTE_LOG(ERR, EAL,
-						"lcore %u unavailable\n", idx);
-					return -1;
-				}
-
-				if (cfg->lcore_role[idx] == ROLE_RTE)
-					taken_lcore_count++;
-
-				lcore_config[idx].core_role = ROLE_SERVICE;
-				count++;
-			}
-		}
-	}
-
-	for (; i >= 0; i--)
-		if (coremask[i] != '0')
+		if (eal_cpu_detected(cores[i]) == 0) {
+			RTE_LOG(ERR, EAL,
+				"lcore %u unavailable\n", cores[i]);
 			return -1;
+		}
 
-	for (; idx < RTE_MAX_LCORE; idx++)
-		lcore_config[idx].core_index = -1;
-
-	if (count == 0)
-		return -1;
+		if (cfg->lcore_role[cores[i]] == ROLE_RTE)
+			taken_lcore_count++;
 
+		lcore_config[cores[i]].core_role = ROLE_SERVICE;
+	}
 	if (core_parsed && taken_lcore_count != count) {
 		RTE_LOG(WARNING, EAL,
-			"Not all service cores are in the coremask. "
+			"Not all service cores were in the coremask. "
 			"Please ensure -c or -l includes service cores\n");
 	}
 
+	for (uint16_t j = count*4; j < RTE_MAX_LCORE; j++)
+		lcore_config[j].core_index = -1;
+
 	cfg->service_lcore_count = count;
 	return 0;
 }
@@ -780,70 +732,29 @@ static int
 eal_parse_service_corelist(const char *corelist)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
-	int i;
-	unsigned count = 0;
-	char *end = NULL;
-	uint32_t min, max, idx;
+	int64_t i, count;
+	uint16_t cores[RTE_MAX_LCORE];
 	uint32_t taken_lcore_count = 0;
 
-	if (corelist == NULL)
-		return -1;
+	count = rte_arg_parse_corelist(corelist, cores, RTE_DIM(cores));
 
-	/* Remove all blank characters ahead and after */
-	while (isblank(*corelist))
-		corelist++;
-	i = strlen(corelist);
-	while ((i > 0) && isblank(corelist[i - 1]))
-		i--;
+	if (count == 0 || count == -1)
+		return -1;
 
-	/* Get list of cores */
-	min = RTE_MAX_LCORE;
-	do {
-		while (isblank(*corelist))
-			corelist++;
-		if (*corelist == '\0')
-			return -1;
-		errno = 0;
-		idx = strtoul(corelist, &end, 10);
-		if (errno || end == NULL)
-			return -1;
-		if (idx >= RTE_MAX_LCORE)
-			return -1;
-		while (isblank(*end))
-			end++;
-		if (*end == '-') {
-			min = idx;
-		} else if ((*end == ',') || (*end == '\0')) {
-			max = idx;
-			if (min == RTE_MAX_LCORE)
-				min = idx;
-			for (idx = min; idx <= max; idx++) {
-				if (cfg->lcore_role[idx] != ROLE_SERVICE) {
-					/* handle main lcore already parsed */
-					uint32_t lcore = idx;
-					if (cfg->main_lcore == lcore &&
-							main_lcore_parsed) {
-						RTE_LOG(ERR, EAL,
-							"Error: lcore %u is main lcore, cannot use as service core\n",
-							idx);
-						return -1;
-					}
-					if (cfg->lcore_role[idx] == ROLE_RTE)
-						taken_lcore_count++;
-
-					lcore_config[idx].core_role =
-							ROLE_SERVICE;
-					count++;
-				}
-			}
-			min = RTE_MAX_LCORE;
-		} else
+	for (i = 0; i < count; i++) {
+		uint32_t lcore = cores[i];
+		if (cfg->main_lcore == lcore &&
+				main_lcore_parsed) {
+			RTE_LOG(ERR, EAL,
+				"Error: lcore %u is main lcore, cannot use as service core\n",
+				cores[i]);
 			return -1;
-		corelist = end + 1;
-	} while (*end != '\0');
+		}
+		if (cfg->lcore_role[cores[i]] == ROLE_RTE)
+			taken_lcore_count++;
 
-	if (count == 0)
-		return -1;
+		lcore_config[cores[i]].core_role = ROLE_SERVICE;
+	}
 
 	if (core_parsed && taken_lcore_count != count) {
 		RTE_LOG(WARNING, EAL,
-- 
2.34.1

[PATCH v4 5/8] event/dlb2: add new arg parsing library API support

[Euan Bourke]

Switched the dlb2 driver to call the new arg parsing library instead of
eal for coremask parsing, and updated the resource probe function to
support the changed formatting of the API.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 drivers/event/dlb2/dlb2_priv.h             |  4 +-
 drivers/event/dlb2/pf/base/dlb2_resource.c | 51 +++++++++-------------
 2 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
index 31a3beeb6c..c14d83da5b 100644
--- a/drivers/event/dlb2/dlb2_priv.h
+++ b/drivers/event/dlb2/dlb2_priv.h
@@ -10,6 +10,7 @@
 
 #include <rte_eventdev.h>
 #include <rte_config.h>
+#include <rte_arg_parser.h>
 #include "dlb2_user.h"
 #include "dlb2_log.h"
 #include "rte_pmd_dlb2.h"
@@ -729,9 +730,6 @@ void dlb2_event_build_hcws(struct dlb2_port *qm_port,
 			   uint8_t *sched_type,
 			   uint8_t *queue_id);
 
-/* Extern functions */
-extern int rte_eal_parse_coremask(const char *coremask, int *cores);
-
 /* Extern globals */
 extern struct process_local_port_data dlb2_port[][DLB2_NUM_PORT_TYPES];
 
diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
index 7ce3e3531c..b65de9350a 100644
--- a/drivers/event/dlb2/pf/base/dlb2_resource.c
+++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
@@ -922,49 +922,38 @@ dlb2_resource_probe(struct dlb2_hw *hw, const void *probe_args)
 {
 	const struct dlb2_devargs *args = (const struct dlb2_devargs *)probe_args;
 	const char *mask = args ? args->producer_coremask : NULL;
-	int cpu = 0, cnt = 0, cores[RTE_MAX_LCORE], i;
+	int cpu = 0, i;
+	uint16_t cores[RTE_MAX_LCORE];
 
 	if (args) {
 		mask = (const char *)args->producer_coremask;
 	}
 
-	if (mask && rte_eal_parse_coremask(mask, cores)) {
+	int ret = rte_arg_parse_coremask(mask, cores, RTE_DIM(cores));
+
+	if (mask && ret == -1) {
 		DLB2_LOG_ERR(": Invalid producer coremask=%s", mask);
 		return -1;
 	}
 
-	hw->num_prod_cores = 0;
-	for (i = 0; i < RTE_MAX_LCORE; i++) {
-		bool is_pcore = (mask && cores[i] != -1);
-
-		if (rte_lcore_is_enabled(i)) {
-			if (is_pcore) {
-				/*
-				 * Populate the producer cores from parsed
-				 * coremask
-				 */
-				hw->prod_core_list[cores[i]] = i;
-				hw->num_prod_cores++;
-
-			} else if ((++cnt == DLB2_EAL_PROBE_CORE ||
-			   rte_lcore_count() < DLB2_EAL_PROBE_CORE)) {
-				/*
-				 * If no producer coremask is provided, use the
-				 * second EAL core to probe
-				 */
-				cpu = i;
-				break;
-			}
-		} else if (is_pcore) {
+	hw->num_prod_cores = ret;
+	/* Check for no producer cores and then get the second EAL core */
+	if (hw->num_prod_cores > 0)
+		cpu = cores[0];
+	else if (rte_lcore_count() < DLB2_EAL_PROBE_CORE)
+		cpu = rte_get_main_lcore();
+	else
+		cpu = rte_get_next_lcore(-1, 1, 0);
+
+	/* check our producer list is valid and error out if not */
+	for (i = 0; i < hw->num_prod_cores; i++) {
+		if (!rte_lcore_is_enabled(cores[i])) {
 			DLB2_LOG_ERR("Producer coremask(%s) must be a subset of EAL coremask",
-				     mask);
+				mask);
 			return -1;
-		}
-
 	}
-	/* Use the first core in producer coremask to probe */
-	if (hw->num_prod_cores)
-		cpu = hw->prod_core_list[0];
+	hw->prod_core_list[i] = cores[i];
+}
 
 	dlb2_get_pp_allocation(hw, cpu, DLB2_LDB_PORT);
 	dlb2_get_pp_allocation(hw, cpu, DLB2_DIR_PORT);
-- 
2.34.1

[PATCH v4 6/8] arg_parser: added common core string and heuristic parsers

[Euan Bourke]

Two new functions, the first is a 'heuristic parser' which examines a
string describing a set of cores and determines based off heuristics
whether its a coremask or a corelist.

Second is a 'combined parser' which calls the first function and then
based off the returned value will call the relevant core string parser.
This function also takes a 'default_type' int which corresponds to
which parser should be used in the case of an ambiguous string.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 lib/arg_parser/arg_parser.c     | 62 ++++++++++++++++++++++++++++++++
 lib/arg_parser/rte_arg_parser.h | 64 +++++++++++++++++++++++++++++++++
 lib/arg_parser/version.map      |  2 ++
 3 files changed, 128 insertions(+)

diff --git a/lib/arg_parser/arg_parser.c b/lib/arg_parser/arg_parser.c
index 8d86a7b618..1755ecb3b2 100644
--- a/lib/arg_parser/arg_parser.c
+++ b/lib/arg_parser/arg_parser.c
@@ -157,3 +157,65 @@ rte_arg_parse_coremask(const char *coremask, uint16_t *cores, uint32_t cores_len
 
 	return corebits_to_array(&mask, cores, cores_len);
 }
+
+int
+rte_arg_parse_arg_type(const char *core_string)
+{
+	/* Remove leading whitespace */
+	while (isblank(*core_string))
+		core_string++;
+
+	/* Check for 0x prefix */
+	if (core_string[0] == '0' && tolower(core_string[1]) == 'x') {
+		if (core_string[2] != '\0')
+			return RTE_ARG_PARSE_TYPE_COREMASK;
+		return -EINVAL;
+	}
+
+	int i = 0, idx = 0;
+	/* Check for ',' and '-' and check for A-F */
+	do {
+		while (isblank(core_string[idx]))
+			idx++;
+
+		if (core_string[idx] == ',' || core_string[idx] == '-')
+			return RTE_ARG_PARSE_TYPE_CORELIST;
+
+		if (isalpha(core_string[idx])) {
+			if (isxdigit(core_string[idx]))
+				return RTE_ARG_PARSE_TYPE_COREMASK;
+			return -EINVAL;
+		}
+		idx++;
+		i++;
+	} while (core_string[idx] != '\0');
+
+	/* Check length of core_string if ambiguous as max length of a uint16_t is 5 digits
+	 * implying its a coremask.
+	 */
+	if (i > 5)
+		return RTE_ARG_PARSE_TYPE_COREMASK;
+
+	return -EINVAL;
+}
+
+int
+rte_arg_parse_core_string(const char *core_string, uint16_t *cores, uint32_t cores_len,
+		int default_type)
+{
+	if (default_type != RTE_ARG_PARSE_TYPE_COREMASK &&
+			default_type != RTE_ARG_PARSE_TYPE_CORELIST) {
+		return -EINVAL;
+	}
+	switch (rte_arg_parse_arg_type(core_string)) {
+	case RTE_ARG_PARSE_TYPE_COREMASK:
+		return rte_arg_parse_coremask(core_string, cores, cores_len);
+	case RTE_ARG_PARSE_TYPE_CORELIST:
+		return rte_arg_parse_corelist(core_string, cores, cores_len);
+	default:
+		return default_type == RTE_ARG_PARSE_TYPE_COREMASK ?
+			rte_arg_parse_coremask(core_string, cores, cores_len) :
+			rte_arg_parse_corelist(core_string, cores, cores_len);
+		return -EINVAL;
+	}
+}
diff --git a/lib/arg_parser/rte_arg_parser.h b/lib/arg_parser/rte_arg_parser.h
index 49d7daa204..3b2df74d10 100644
--- a/lib/arg_parser/rte_arg_parser.h
+++ b/lib/arg_parser/rte_arg_parser.h
@@ -23,6 +23,9 @@ extern "C" {
 
 #include <rte_compat.h>
 
+#define RTE_ARG_PARSE_TYPE_COREMASK 0
+#define RTE_ARG_PARSE_TYPE_CORELIST 1
+#define RTE_ARG_PARSE_TYPE_UNKNOWN 2
 
 /**
  * Convert a string describing a list of core ids into an array of core ids.
@@ -92,6 +95,67 @@ __rte_experimental
 int
 rte_arg_parse_coremask(const char *coremask, uint16_t *cores, uint32_t cores_len);
 
+/**
+ * Use heuristics to determine if a string contains a coremask or a corelist.
+ *
+ * This function will check a series of conditions and return an int representing which
+ * core type (mask or list) the string represents or report the type as unknown if the
+ * string is ambiguous.
+ *
+ * @param core_string
+ *   A string describing the intended cores to be parsed
+ * @return
+ *   int representing the core type
+ *   RTE_ARG_PARSE_TYPE_COREMASK: coremask.
+ *   RTE_ARG_PARSE_TYPE_CORELIST: corelist.
+ *   RTE_ARG_PARSE_TYPE_UNKNOWN: unknown (ambiguous).
+ *   -EINVAL if the string was invalid.
+ */
+__rte_experimental
+int
+rte_arg_parse_arg_type(const char *core_string);
+
+/**
+ * Convert a string describing either a corelist or coremask into an array of core ids.
+ *
+ * This function will fill the "cores" array up to "cores_len" with the core ids described
+ * in the "core_string". The string can either describe a corelist or a coremask, and
+ * will be parsed accordingly. The number of unique core ids in the string is then returned.
+ * For example:
+ * "1-4" is treated as a corelist and results in an array of [1,2,3,4] with 4 being returned
+ * "0xA1" is treated as a coremask and results in an array of [0,5,7] with 3 being returned
+ *
+ * In the case of an ambiguous string, the function will use the default_type parameter to
+ * decide.
+ *
+ * NOTE: if the length of the input array is insufficient to hold the number of core ids
+ * in "core_string" the input array is filled to capacity but the return value is the
+ * number of elements which would have been written to the array, had enough space been
+ * available. [This is similar to the behaviour of the snprintf function]. Because of
+ * this, the number of core values in the "core_string" may be determined by calling the
+ * function with a NULL array pointer and array length given as 0.
+ *
+ * @param core_string
+ *   A string describing the intended cores to be parsed.
+ * @param cores
+ *   An array where to store the core ids.
+ *   Array can be NULL if "cores_len" is 0.
+ * @param cores_len
+ *   The length of the "cores" array.
+ *   If the size is smaller than that needed to hold all cores from "core_string"
+ * @param default_type
+ *   How to treat ambiguous cases (e.g. '4' could be mask or list).
+ *   RTE_ARG_PARSE_TYPE_COREMASK: coremask.
+ *   RTE_ARG_PARSE_TYPE_CORELIST: corelist.
+ * @return
+ *   n: the number of unique cores present in "core_string".
+ *   -EINVAL if the string was invalid.
+ *   NOTE: if n > "cores_len", then only "cores_len" elements in the "cores" array are valid.
+ */
+__rte_experimental
+int
+rte_arg_parse_core_string(const char *core_string, uint16_t *cores, uint32_t cores_len,
+		int default_type);
 
 #ifdef __cplusplus
 }
diff --git a/lib/arg_parser/version.map b/lib/arg_parser/version.map
index b44d4b02b7..1e54b91dae 100644
--- a/lib/arg_parser/version.map
+++ b/lib/arg_parser/version.map
@@ -6,6 +6,8 @@ EXPERIMENTAL {
 	global:
 
 	# added in 24.03
+	rte_arg_parse_arg_type;
 	rte_arg_parse_corelist;
 	rte_arg_parse_coremask;
+	rte_arg_parse_core_string;
 };
-- 
2.34.1

[PATCH v4 7/8] examples/eventdev_pipeline: update to call arg parser API

[Euan Bourke]

Update to the eventdev_pipeline example application to call the arg parser
library for its 'combined core string parser' instead of implementing its
own coremask parser. The default_type passed into the function call is
a coremask.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
---
 examples/eventdev_pipeline/main.c            | 66 +++-----------------
 examples/eventdev_pipeline/pipeline_common.h |  1 +
 2 files changed, 11 insertions(+), 56 deletions(-)

diff --git a/examples/eventdev_pipeline/main.c b/examples/eventdev_pipeline/main.c
index 0c995d1a70..edd0e2c843 100644
--- a/examples/eventdev_pipeline/main.c
+++ b/examples/eventdev_pipeline/main.c
@@ -56,69 +56,23 @@ core_in_use(unsigned int lcore_id) {
 		fdata->tx_core[lcore_id] || fdata->worker_core[lcore_id]);
 }
 
-/*
- * Parse the coremask given as argument (hexadecimal string) and fill
- * the global configuration (core role and core count) with the parsed
- * value.
- */
-static int xdigit2val(unsigned char c)
-{
-	int val;
-
-	if (isdigit(c))
-		val = c - '0';
-	else if (isupper(c))
-		val = c - 'A' + 10;
-	else
-		val = c - 'a' + 10;
-	return val;
-}
-
 static uint64_t
 parse_coremask(const char *coremask)
 {
-	int i, j, idx = 0;
-	unsigned int count = 0;
-	char c;
-	int val;
+	int count;
+	uint16_t i;
 	uint64_t mask = 0;
-	const int32_t BITS_HEX = 4;
+	uint16_t cores[RTE_MAX_LCORE];
 
-	if (coremask == NULL)
-		return -1;
-	/* Remove all blank characters ahead and after .
-	 * Remove 0x/0X if exists.
-	 */
-	while (isblank(*coremask))
-		coremask++;
-	if (coremask[0] == '0' && ((coremask[1] == 'x')
-		|| (coremask[1] == 'X')))
-		coremask += 2;
-	i = strlen(coremask);
-	while ((i > 0) && isblank(coremask[i - 1]))
-		i--;
-	if (i == 0)
-		return -1;
+	count = rte_arg_parse_core_string(coremask, cores, RTE_DIM(cores),
+				RTE_ARG_PARSE_TYPE_COREMASK);
 
-	for (i = i - 1; i >= 0 && idx < MAX_NUM_CORE; i--) {
-		c = coremask[i];
-		if (isxdigit(c) == 0) {
-			/* invalid characters */
-			return -1;
-		}
-		val = xdigit2val(c);
-		for (j = 0; j < BITS_HEX && idx < MAX_NUM_CORE; j++, idx++) {
-			if ((1 << j) & val) {
-				mask |= (1ULL << idx);
-				count++;
-			}
-		}
-	}
-	for (; i >= 0; i--)
-		if (coremask[i] != '0')
-			return -1;
-	if (count == 0)
+	if (count == 0 || count == -1)
 		return -1;
+
+	for (i = 0; i < count; i++)
+		mask |= 1ULL << cores[i];
+
 	return mask;
 }
 
diff --git a/examples/eventdev_pipeline/pipeline_common.h b/examples/eventdev_pipeline/pipeline_common.h
index 28b6ab85ff..2b97a29bfc 100644
--- a/examples/eventdev_pipeline/pipeline_common.h
+++ b/examples/eventdev_pipeline/pipeline_common.h
@@ -6,6 +6,7 @@
 
 #include <stdbool.h>
 
+#include <rte_arg_parser.h>
 #include <rte_eal.h>
 #include <rte_mempool.h>
 #include <rte_mbuf.h>
-- 
2.34.1

[PATCH v4 8/8] examples/l3fwd-power: update to call arg parser API

[Euan Bourke]

Update to the l3fwd-power example application to call the arg parser
library for its 'combined core string parser' instead of implementing its
own corelist parser. The default_type passed into the function call is
a corelist.

Signed-off-by: Euan Bourke <euan.bourke@intel.com>
Acked-by: David Hunt <david.hunt@intel.com>
---
 examples/l3fwd-power/perf_core.c | 52 ++++++--------------------------
 1 file changed, 9 insertions(+), 43 deletions(-)

diff --git a/examples/l3fwd-power/perf_core.c b/examples/l3fwd-power/perf_core.c
index 41ef6d0c9a..e8ed414d40 100644
--- a/examples/l3fwd-power/perf_core.c
+++ b/examples/l3fwd-power/perf_core.c
@@ -12,6 +12,7 @@
 #include <rte_lcore.h>
 #include <rte_power.h>
 #include <rte_string_fns.h>
+#include <rte_arg_parser.h>
 
 #include "perf_core.h"
 #include "main.h"
@@ -177,56 +178,21 @@ parse_perf_config(const char *q_arg)
 int
 parse_perf_core_list(const char *corelist)
 {
-	int i, idx = 0;
-	unsigned int count = 0;
-	char *end = NULL;
-	int min, max;
+	int count;
+	uint16_t cores[RTE_MAX_LCORE];
 
 	if (corelist == NULL) {
 		printf("invalid core list\n");
 		return -1;
 	}
 
+	count = rte_arg_parse_core_string(corelist, cores, RTE_DIM(cores),
+				RTE_ARG_PARSE_TYPE_CORELIST);
 
-	/* Remove all blank characters ahead and after */
-	while (isblank(*corelist))
-		corelist++;
-	i = strlen(corelist);
-	while ((i > 0) && isblank(corelist[i - 1]))
-		i--;
+	for (int i = 0; i < count; i++)
+		hp_lcores[i] = cores[i];
 
-	/* Get list of cores */
-	min = RTE_MAX_LCORE;
-	do {
-		while (isblank(*corelist))
-			corelist++;
-		if (*corelist == '\0')
-			return -1;
-		errno = 0;
-		idx = strtoul(corelist, &end, 10);
-		if (errno || end == NULL)
-			return -1;
-		while (isblank(*end))
-			end++;
-		if (*end == '-') {
-			min = idx;
-		} else if ((*end == ',') || (*end == '\0')) {
-			max = idx;
-			if (min == RTE_MAX_LCORE)
-				min = idx;
-			for (idx = min; idx <= max; idx++) {
-				hp_lcores[count] = idx;
-				count++;
-			}
-			min = RTE_MAX_LCORE;
-		} else {
-			printf("invalid core list\n");
-			return -1;
-		}
-		corelist = end + 1;
-	} while (*end != '\0');
-
-	if (count == 0) {
+	if (count == 0 || count == -1) {
 		printf("invalid core list\n");
 		return -1;
 	}
@@ -234,7 +200,7 @@ parse_perf_core_list(const char *corelist)
 	nb_hp_lcores = count;
 
 	printf("Configured %d high performance cores\n", nb_hp_lcores);
-	for (i = 0; i < nb_hp_lcores; i++)
+	for (int i = 0; i < nb_hp_lcores; i++)
 		printf("\tHigh performance core %d %d\n",
 				i, hp_lcores[i]);
 
-- 
2.34.1

Re: [PATCH v4 0/8] add new command line argument parsing library

[Stephen Hemminger]

On Fri, 15 Dec 2023 17:26:24 +0000
Euan Bourke <euan.bourke@intel.com> wrote:

> A recent thread on the mailing list[1] discussed corelist and coremask
> parsing and the idea of a new library dedicated to command line parsing
> was mentioned[2]. This patchset adds the library, along with the new
> APIs, and edits the existing EAL, DLB2 driver and some example
> application functions to use these APIs, rather than each implementing
> their own copies.
> 
> The new APIs work similar to the existing functions in EAL, however
> instead of filling a core array like this:
> [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> It fills it like this:
> [0, 3, 4] (with the value at each index being an 'active core').
> 
> The new APIs will also return the number of cores contained in the
> passed corelist/coremask, so in the above example, 3 would be returned.
> 
> Also included in this patchest is a heuristic parser which searches
> for key markers in the core string, returning a enum value based off
> this search to indicate if a parameter is likely a coremask or a
> corelist. This heuristic function is also wrapped in a parser
> function allowing apps to handle both coremasks and corelists
> simultaneously.
> 
> [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> 

The code is ok, but the naming needs to change.

To me the name argparse implies that library implements something
similar to Python argparse. But that isn't what this library does.
It seems limited to just cpu masks. Perhaps cpuparse would be better name
or make it part of a larger implementation of a full library
more like Python argparse.

RE: [PATCH v4 8/8] examples/l3fwd-power: update to call arg parser API

[Tummala, Sivaprasad]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Euan Bourke <euan.bourke@intel.com>
> Sent: Friday, December 15, 2023 10:57 PM
> To: dev@dpdk.org
> Cc: Euan Bourke <euan.bourke@intel.com>; David Hunt <david.hunt@intel.com>;
> Anatoly Burakov <anatoly.burakov@intel.com>; Tummala, Sivaprasad
> <Sivaprasad.Tummala@amd.com>
> Subject: [PATCH v4 8/8] examples/l3fwd-power: update to call arg parser API
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Update to the l3fwd-power example application to call the arg parser library for its
> 'combined core string parser' instead of implementing its own corelist parser. The
> default_type passed into the function call is a corelist.
>
> Signed-off-by: Euan Bourke <euan.bourke@intel.com>
> Acked-by: David Hunt <david.hunt@intel.com>
> ---
>  examples/l3fwd-power/perf_core.c | 52 ++++++--------------------------
>  1 file changed, 9 insertions(+), 43 deletions(-)
>
> diff --git a/examples/l3fwd-power/perf_core.c b/examples/l3fwd-power/perf_core.c
> index 41ef6d0c9a..e8ed414d40 100644
> --- a/examples/l3fwd-power/perf_core.c
> +++ b/examples/l3fwd-power/perf_core.c
> @@ -12,6 +12,7 @@
>  #include <rte_lcore.h>
>  #include <rte_power.h>
>  #include <rte_string_fns.h>
> +#include <rte_arg_parser.h>
>
>  #include "perf_core.h"
>  #include "main.h"
> @@ -177,56 +178,21 @@ parse_perf_config(const char *q_arg)  int
> parse_perf_core_list(const char *corelist)  {
> -       int i, idx = 0;
> -       unsigned int count = 0;
> -       char *end = NULL;
> -       int min, max;
> +       int count;
> +       uint16_t cores[RTE_MAX_LCORE];
>
>         if (corelist == NULL) {
>                 printf("invalid core list\n");
>                 return -1;
>         }
>
> +       count = rte_arg_parse_core_string(corelist, cores, RTE_DIM(cores),
> +                               RTE_ARG_PARSE_TYPE_CORELIST);
>
> -       /* Remove all blank characters ahead and after */
> -       while (isblank(*corelist))
> -               corelist++;
> -       i = strlen(corelist);
> -       while ((i > 0) && isblank(corelist[i - 1]))
> -               i--;
> +       for (int i = 0; i < count; i++)
> +               hp_lcores[i] = cores[i];
>
> -       /* Get list of cores */
> -       min = RTE_MAX_LCORE;
> -       do {
> -               while (isblank(*corelist))
> -                       corelist++;
> -               if (*corelist == '\0')
> -                       return -1;
> -               errno = 0;
> -               idx = strtoul(corelist, &end, 10);
> -               if (errno || end == NULL)
> -                       return -1;
> -               while (isblank(*end))
> -                       end++;
> -               if (*end == '-') {
> -                       min = idx;
> -               } else if ((*end == ',') || (*end == '\0')) {
> -                       max = idx;
> -                       if (min == RTE_MAX_LCORE)
> -                               min = idx;
> -                       for (idx = min; idx <= max; idx++) {
> -                               hp_lcores[count] = idx;
> -                               count++;
> -                       }
> -                       min = RTE_MAX_LCORE;
> -               } else {
> -                       printf("invalid core list\n");
> -                       return -1;
> -               }
> -               corelist = end + 1;
> -       } while (*end != '\0');
> -
> -       if (count == 0) {
> +       if (count == 0 || count == -1) {
>                 printf("invalid core list\n");
>                 return -1;
>         }
> @@ -234,7 +200,7 @@ parse_perf_core_list(const char *corelist)
>         nb_hp_lcores = count;
>
>         printf("Configured %d high performance cores\n", nb_hp_lcores);
> -       for (i = 0; i < nb_hp_lcores; i++)
> +       for (int i = 0; i < nb_hp_lcores; i++)
>                 printf("\tHigh performance core %d %d\n",
>                                 i, hp_lcores[i]);
>
> --
> 2.34.1

Acked-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>

Re: [PATCH v4 0/8] add new command line argument parsing library

[Bruce Richardson]

On Fri, Dec 15, 2023 at 01:53:21PM -0800, Stephen Hemminger wrote:
> On Fri, 15 Dec 2023 17:26:24 +0000
> Euan Bourke <euan.bourke@intel.com> wrote:
> 
> > A recent thread on the mailing list[1] discussed corelist and coremask
> > parsing and the idea of a new library dedicated to command line parsing
> > was mentioned[2]. This patchset adds the library, along with the new
> > APIs, and edits the existing EAL, DLB2 driver and some example
> > application functions to use these APIs, rather than each implementing
> > their own copies.
> > 
> > The new APIs work similar to the existing functions in EAL, however
> > instead of filling a core array like this:
> > [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> > It fills it like this:
> > [0, 3, 4] (with the value at each index being an 'active core').
> > 
> > The new APIs will also return the number of cores contained in the
> > passed corelist/coremask, so in the above example, 3 would be returned.
> > 
> > Also included in this patchest is a heuristic parser which searches
> > for key markers in the core string, returning a enum value based off
> > this search to indicate if a parameter is likely a coremask or a
> > corelist. This heuristic function is also wrapped in a parser
> > function allowing apps to handle both coremasks and corelists
> > simultaneously.
> > 
> > [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> > [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> > 
> 
> The code is ok, but the naming needs to change.
> 
> To me the name argparse implies that library implements something
> similar to Python argparse. But that isn't what this library does.
> It seems limited to just cpu masks. Perhaps cpuparse would be better name
> or make it part of a larger implementation of a full library
> more like Python argparse.

Yes, it is a limited set of functions, so the name is probably not ideal if
that's what it's going to remaini (though what library ever stays
un-expanded once started!). I think we need a general discussion
on-list and probably in tech-board too about argument handling, since in
the last couple of months there have been no less than 3 separate
independent proposals around functionality for arguments.

There has been:
* This set, for extracting out functions for processing coremask and
  corelist arguments. Presumably other argument parsing types may look to
  be included in future e.g. device args, perhaps.
* A set from me [1], looking to take the slightly opposite side of things, and 
  make it easier for apps to initialize EAL with a custom argument set. So
  it can be thought of as an argument-list management library.
* An "argparse" library from Chengwen [2] which does what you describe
  above and be a C implementation like the python argparse module.

We need to decide how to manage all these three, if we want to take them
all, and if they should all be merged into a single library, or some kept
separate from others.

/Bruce

[1] http://patches.dpdk.org/project/dpdk/list/?series=30120
[2] http://patches.dpdk.org/project/dpdk/list/?series=30506

RE: [PATCH v4 1/8] arg_parser: new library for command line parsing

[Morten Brørup]

> From: Euan Bourke [mailto:euan.bourke@intel.com]
> Sent: Friday, 15 December 2023 18.26
> 
> Add a new library to make it easier for eal and other libraries to
> parse
> command line arguments.
> 
> The first function in this library is one to parse a corelist string
> into an array of individual core ids. The function will then return the
> total number of cores described in the corelist.
> 
> Signed-off-by: Euan Bourke <euan.bourke@intel.com>
> ---

It would be nice if the library supported open-ended ranges, i.e. "-3" meaning all available cores up to and including 3, and "3-" meaning all available cores from 3 and up.

Re: [PATCH v4 1/8] arg_parser: new library for command line parsing

[Bruce Richardson]

On Wed, Jan 24, 2024 at 02:16:13PM +0100, Morten Brørup wrote:
> > From: Euan Bourke [mailto:euan.bourke@intel.com]
> > Sent: Friday, 15 December 2023 18.26
> > 
> > Add a new library to make it easier for eal and other libraries to
> > parse
> > command line arguments.
> > 
> > The first function in this library is one to parse a corelist string
> > into an array of individual core ids. The function will then return the
> > total number of cores described in the corelist.
> > 
> > Signed-off-by: Euan Bourke <euan.bourke@intel.com>
> > ---
> 
> It would be nice if the library supported open-ended ranges, i.e. "-3" meaning all available cores up to and including 3, and "3-" meaning all available cores from 3 and up.
> 
Yes, that could be a useful enhancement. For now these functions are just
based off what is already implemented in EAL, with minimal enhancements.

/Bruce

Re: [PATCH v4 0/8] add new command line argument parsing library

[Thomas Monjalon]

18/12/2023 10:18, Bruce Richardson:
> On Fri, Dec 15, 2023 at 01:53:21PM -0800, Stephen Hemminger wrote:
> > On Fri, 15 Dec 2023 17:26:24 +0000
> > Euan Bourke <euan.bourke@intel.com> wrote:
> > 
> > > A recent thread on the mailing list[1] discussed corelist and coremask
> > > parsing and the idea of a new library dedicated to command line parsing
> > > was mentioned[2]. This patchset adds the library, along with the new
> > > APIs, and edits the existing EAL, DLB2 driver and some example
> > > application functions to use these APIs, rather than each implementing
> > > their own copies.
> > > 
> > > The new APIs work similar to the existing functions in EAL, however
> > > instead of filling a core array like this:
> > > [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> > > It fills it like this:
> > > [0, 3, 4] (with the value at each index being an 'active core').
> > > 
> > > The new APIs will also return the number of cores contained in the
> > > passed corelist/coremask, so in the above example, 3 would be returned.
> > > 
> > > Also included in this patchest is a heuristic parser which searches
> > > for key markers in the core string, returning a enum value based off
> > > this search to indicate if a parameter is likely a coremask or a
> > > corelist. This heuristic function is also wrapped in a parser
> > > function allowing apps to handle both coremasks and corelists
> > > simultaneously.
> > > 
> > > [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> > > [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> > > 
> > 
> > The code is ok, but the naming needs to change.
> > 
> > To me the name argparse implies that library implements something
> > similar to Python argparse. But that isn't what this library does.
> > It seems limited to just cpu masks. Perhaps cpuparse would be better name
> > or make it part of a larger implementation of a full library
> > more like Python argparse.
> 
> Yes, it is a limited set of functions, so the name is probably not ideal if
> that's what it's going to remaini (though what library ever stays
> un-expanded once started!).

This is a string parsing library.

> I think we need a general discussion
> on-list and probably in tech-board too about argument handling, since in
> the last couple of months there have been no less than 3 separate
> independent proposals around functionality for arguments.
> 
> There has been:
> * This set, for extracting out functions for processing coremask and
>   corelist arguments. Presumably other argument parsing types may look to
>   be included in future e.g. device args, perhaps.
> * A set from me [1], looking to take the slightly opposite side of things, and 
>   make it easier for apps to initialize EAL with a custom argument set. So
>   it can be thought of as an argument-list management library.
> * An "argparse" library from Chengwen [2] which does what you describe
>   above and be a C implementation like the python argparse module.
> 
> We need to decide how to manage all these three, if we want to take them
> all, and if they should all be merged into a single library, or some kept
> separate from others.

Yes, we need to decide whether we want to keep this string parsing
as a standalone library, or we prefer to integrate it in Chengwen's
global argument parsing library.

The question is do we want to do string parsing out of the global argument parsing?
I suppose the answer is yes, at least when parsing devargs in drivers.
In this case we need this library to be standalone (and reused in Chengwen's argparse).

Re: [PATCH v4 0/8] add new command line argument parsing library

[Thomas Monjalon]

24/01/2024 14:33, Thomas Monjalon:
> 18/12/2023 10:18, Bruce Richardson:
> > On Fri, Dec 15, 2023 at 01:53:21PM -0800, Stephen Hemminger wrote:
> > > On Fri, 15 Dec 2023 17:26:24 +0000
> > > Euan Bourke <euan.bourke@intel.com> wrote:
> > > 
> > > > A recent thread on the mailing list[1] discussed corelist and coremask
> > > > parsing and the idea of a new library dedicated to command line parsing
> > > > was mentioned[2]. This patchset adds the library, along with the new
> > > > APIs, and edits the existing EAL, DLB2 driver and some example
> > > > application functions to use these APIs, rather than each implementing
> > > > their own copies.
> > > > 
> > > > The new APIs work similar to the existing functions in EAL, however
> > > > instead of filling a core array like this:
> > > > [1, -1, -1, 2, 3] (a non -1 refers to an 'active core' at that index)
> > > > It fills it like this:
> > > > [0, 3, 4] (with the value at each index being an 'active core').
> > > > 
> > > > The new APIs will also return the number of cores contained in the
> > > > passed corelist/coremask, so in the above example, 3 would be returned.
> > > > 
> > > > Also included in this patchest is a heuristic parser which searches
> > > > for key markers in the core string, returning a enum value based off
> > > > this search to indicate if a parameter is likely a coremask or a
> > > > corelist. This heuristic function is also wrapped in a parser
> > > > function allowing apps to handle both coremasks and corelists
> > > > simultaneously.
> > > > 
> > > > [1] https://mails.dpdk.org/archives/dev/2023-November/280957.html
> > > > [2] https://mails.dpdk.org/archives/dev/2023-November/280966.html
> > > > 
> > > 
> > > The code is ok, but the naming needs to change.
> > > 
> > > To me the name argparse implies that library implements something
> > > similar to Python argparse. But that isn't what this library does.
> > > It seems limited to just cpu masks. Perhaps cpuparse would be better name
> > > or make it part of a larger implementation of a full library
> > > more like Python argparse.
> > 
> > Yes, it is a limited set of functions, so the name is probably not ideal if
> > that's what it's going to remaini (though what library ever stays
> > un-expanded once started!).
> 
> This is a string parsing library.
> 
> > I think we need a general discussion
> > on-list and probably in tech-board too about argument handling, since in
> > the last couple of months there have been no less than 3 separate
> > independent proposals around functionality for arguments.
> > 
> > There has been:
> > * This set, for extracting out functions for processing coremask and
> >   corelist arguments. Presumably other argument parsing types may look to
> >   be included in future e.g. device args, perhaps.
> > * A set from me [1], looking to take the slightly opposite side of things, and 
> >   make it easier for apps to initialize EAL with a custom argument set. So
> >   it can be thought of as an argument-list management library.
> > * An "argparse" library from Chengwen [2] which does what you describe
> >   above and be a C implementation like the python argparse module.
> > 
> > We need to decide how to manage all these three, if we want to take them
> > all, and if they should all be merged into a single library, or some kept
> > separate from others.
> 
> Yes, we need to decide whether we want to keep this string parsing
> as a standalone library, or we prefer to integrate it in Chengwen's
> global argument parsing library.
> 
> The question is do we want to do string parsing out of the global argument parsing?
> I suppose the answer is yes, at least when parsing devargs in drivers.
> In this case we need this library to be standalone (and reused in Chengwen's argparse).

The argparse library is now merged,
so we can followup with string parsing proposed in this series.

Re: [PATCH v4 8/8] examples/l3fwd-power: update to call arg parser API

[Stephen Hemminger]

On Fri, 15 Dec 2023 17:26:32 +0000
Euan Bourke <euan.bourke@intel.com> wrote:

> Update to the l3fwd-power example application to call the arg parser
> library for its 'combined core string parser' instead of implementing its
> own corelist parser. The default_type passed into the function call is
> a corelist.
> 
> Signed-off-by: Euan Bourke <euan.bourke@intel.com>
> Acked-by: David Hunt <david.hunt@intel.com>
> ---

Compile test failed with older Gcc??

*Build Failed #1:
OS: CentOS79-64
Target: x86_64-native-linuxapp-gcc
FAILED: lib/librte_arg_parser.a.p/arg_parser_arg_parser.c.o 
gcc -Ilib/librte_arg_parser.a.p -Ilib -I../lib -I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -Ilib/arg_parser -I../lib/arg_parser -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c11 -O3 -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -DRTE_LOG_DEFAULT_LOGTYPE=lib.arg_parser -MD -MQ lib/librte_arg_parser.a.p/arg_parser_arg_parser.c.o -MF lib/librte_arg_parser.a.p/arg_parser_arg_parser.c.o.d -o lib/librte_arg_parser.a.p/arg_parser_arg_parser.c.o -c ../lib/arg_parser/arg_parser.c
../lib/arg_parser/arg_parser.c: In function ‘rte_arg_parse_corelist’:
../lib/arg_parser/arg_parser.c:76:9: error: missing braces around initializer [-Werror=missing-braces]
  struct core_bits mask = {0};
         ^
../lib/arg_parser/arg_parser.c:76:9: error: (near initialization for ‘mask.bits’) [-Werror=missing-braces]
../lib/arg_parser/arg_parser.c: In function ‘rte_arg_parse_coremask’:
../lib/arg_parser/arg_parser.c:125:9: error: missing braces around initializer [-Werror=missing-braces]
  struct core_bits mask = {0};
         ^
../lib/arg_parser/arg_parser.c:125:9: error: (near initialization for ‘mask.bits’) [-Werror=missing-braces]
cc1: all warnings being treated as errors
[2/2834] Compiling C object lib/librte_log.a.p/log_log_linux.c.o
[3/2834] Compiling C object lib/librte_eal.a.p/eal_common_eal_common_class.c.o
[4/2834] Compiling C object lib/librte_kvargs.a.p/kvargs_rte_kvargs.c.o
[5/2834] Compiling C object lib/librte_telemetry.a.p/telemetry_telemetry_data.c.o
[6/2834] Compiling C object lib/librte_telemetry.a.p/telemetry_telemetry_legacy.c.o
[7/2834] Compiling C object lib/librte_log.a.p/log_log.c.o
[8/2834] Compiling C object lib/librte_eal.a.p/eal_common_eal_common_bus.c.o
[9/2834] Compiling C object lib/librte_mempool.a.p/mempool_rte_mempool_ops_default.c.o
[10/2834] Compiling C object lib/librte_telemetry.a.p/telemetry_telemetry.c.o
ninja: build stopped