diff mbox series

[v3,8/8] examples/l3fwd-power: update to call arg parser API

Message ID 20231207161818.2590661-9-euan.bourke@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add new command line argument parsing library |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation fail Compilation issues
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Euan Bourke Dec. 7, 2023, 4:18 p.m. UTC 
  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>
---
 examples/l3fwd-power/perf_core.c | 51 +++++---------------------------
 1 file changed, 8 insertions(+), 43 deletions(-)

Comments

Hunt, David Dec. 11, 2023, 12:01 p.m. UTC | #1 
Hi Euan,

On 07/12/2023 16:18, Euan Bourke 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>
> ---
>   examples/l3fwd-power/perf_core.c | 51 +++++---------------------------
>   1 file changed, 8 insertions(+), 43 deletions(-)
>
> diff --git a/examples/l3fwd-power/perf_core.c b/examples/l3fwd-power/perf_core.c
> index 41ef6d0c9a..f8511e30b3 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,20 @@ 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), 1);
>   
> -	/* 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++)


nit: you've used int here, but below you use uint16_t for a for loop. If 
you're re-spinning, it might be worth making consistent. But no biggie.

--snip--

> @@ -234,7 +199,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 (uint16_t i = 0; i < nb_hp_lcores; i++)
>   		printf("\tHigh performance core %d %d\n",
>   				i, hp_lcores[i]);
>   


I've also tested this with a 16-core incantation of l3fwd-power with 
various combinations of cores, seems to work well.

Acked-by: David Hunt <david.hunt@intel.com>
Sivaprasad Tummala Dec. 11, 2023, 4:50 p.m. UTC | #2 
[AMD Official Use Only - General]

Hi Euan,

> -----Original Message-----
> From: Euan Bourke <euan.bourke@intel.com>
> Sent: Thursday, December 7, 2023 9:48 PM
> To: dev@dpdk.org
> Cc: Euan Bourke <euan.bourke@intel.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>; David Hunt <david.hunt@intel.com>; Tummala,
> Sivaprasad <Sivaprasad.Tummala@amd.com>
> Subject: [PATCH v3 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>
> ---
>  examples/l3fwd-power/perf_core.c | 51 +++++---------------------------
>  1 file changed, 8 insertions(+), 43 deletions(-)
>
> diff --git a/examples/l3fwd-power/perf_core.c b/examples/l3fwd-power/perf_core.c
> index 41ef6d0c9a..f8511e30b3 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,20 @@ 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), 1);
Can you replace the magic number with "RTE_ARG_PARSE_TYPE_CORELIST" as default parse type.

>
> -       /* 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 +199,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 (uint16_t i = 0; i < nb_hp_lcores; i++)
>                 printf("\tHigh performance core %d %d\n",
>                                 i, hp_lcores[i]);
>
> --
> 2.34.1
diff mbox series

Patch

diff --git a/examples/l3fwd-power/perf_core.c b/examples/l3fwd-power/perf_core.c
index 41ef6d0c9a..f8511e30b3 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,20 @@  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), 1);
 
-	/* 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 +199,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 (uint16_t i = 0; i < nb_hp_lcores; i++)
 		printf("\tHigh performance core %d %d\n",
 				i, hp_lcores[i]);