linux-dpdk: init: parse command line options using rte_strsplit()

Message ID 56574A4F.4080307@linaro.org
State New
Headers show

Commit Message

Zoltan Kiss Nov. 26, 2015, 6:07 p.m.
On 25/11/15 20:52, Miroslav Kiradzhiyski wrote:
> DPDK already implements rte_strsplit() which can be used instead of the

> parse_dpdk_args() function. This saves some dynamic memory allocation

> and simplifies the code.

>

> Signed-off-by: Miroslav Kiradzhiyski <miroslav@virtualopensystems.com>

> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>

> ---

>   platform/linux-dpdk/odp_init.c | 51 +++++++++---------------------------------

>   1 file changed, 10 insertions(+), 41 deletions(-)

>

> diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux-dpdk/odp_init.c

> index e8eab7a..56a7bcd 100644

> --- a/platform/linux-dpdk/odp_init.c

> +++ b/platform/linux-dpdk/odp_init.c

> @@ -12,6 +12,9 @@

>   #include <odp_debug_internal.h>

>   #include <odp/system_info.h>

>   #include <unistd.h>

> +#include <rte_string_fns.h>

> +

> +#define MAX_ARGC (64u)


This is an arbitrary limitation of how many command line arguments you 
can pass to DPDK. And not even an exact one, because there could be 
arguments with more than 2 space separated parts.
It is probably enough for you an me, but what if someone needs more? 
E.g. pass an extensive PCI black/whitelist?
I'm afraid you can't get away from counting the whitespaces in 
full_cmdline and alloc this array accordingly. I'm attaching a patch 
which has a solution for that, feel free to take and modify that. Also, 
I've rebased your patch to the last patch applied.

>

>   #define PMD_EXT(drv)  extern void devinitfn_##drv(void);

>   PMD_EXT(pmd_af_packet_drv)

> @@ -110,43 +113,12 @@ void refer_constructors(void) {

>   #endif

>   }

>

> -static void parse_dpdk_args(const char *args, int *dst_argc, char ***dst_argv)

> -{

> -	char *buf = strdup(args);

> -	int num = 1;

> -	char *delim;

> -	char **argv = calloc(num, sizeof(char *));

> -

> -	if (!buf || !argv)

> -		ODP_ABORT("Can't allocate memory!\n");

> -

> -	argv[0] = buf;

> -

> -	while (1) {

> -		delim = strchr(argv[num - 1], ' ');

> -		if (delim == NULL)

> -			break;

> -		argv = realloc(argv, (num + 1) * sizeof(char *));

> -		if (!argv)

> -			ODP_ABORT("Can't reallocate memory!\n");

> -		argv[num] = delim + 1;

> -		*delim = 0;

> -		num++;

> -	}

> -

> -	*dst_argc = num;

> -	*dst_argv = argv;

> -

> -	return;

> -}

> -

> -

>   static void print_dpdk_env_help(void)

>   {

> -	char **dpdk_argv;

> -	int dpdk_argc, save_optind;

> +	char help_str[] = "--help";

> +	static char *dpdk_argv[] = {help_str};


Static causes "initializer element is not constant" errors.

> +	int save_optind, dpdk_argc = 1;

>

> -	parse_dpdk_args("--help", &dpdk_argc, &dpdk_argv);

>   	ODP_ERR("Neither (char *)platform_params were provided to "

>   		"odp_init_global(),\n");

>   	ODP_ERR("nor ODP_PLATFORM_PARAMS environment variable were "

> @@ -158,14 +130,12 @@ static void print_dpdk_env_help(void)

>   	optind = 1;

>   	rte_eal_init(dpdk_argc, dpdk_argv);

>   	optind = save_optind;

> -	free(dpdk_argv[0]);

> -	free(dpdk_argv);

>   }

>

>

>   int odp_init_dpdk(const char *cmdline)

>   {

> -	char **dpdk_argv;

> +	char *dpdk_argv[MAX_ARGC];

>   	int dpdk_argc;

>   	char *full_cmdline;

>   	int core_mask, i, save_optind;

> @@ -187,19 +157,18 @@ int odp_init_dpdk(const char *cmdline)

>   	/* first argument is facility log, simply bind it to odpdpdk for now.*/

>   	sprintf(full_cmdline, "odpdpdk -c 0x%x %s", core_mask, cmdline);

>

> -	parse_dpdk_args(full_cmdline, &dpdk_argc, &dpdk_argv);

> +	dpdk_argc = rte_strsplit(full_cmdline, strlen(full_cmdline), dpdk_argv,

> +				 MAX_ARGC, ' ');

>   	for (i = 0; i < dpdk_argc; ++i)

>   		ODP_DBG("arg[%d]: %s\n", i, dpdk_argv[i]);

>   	fflush(stdout);

> -	free(full_cmdline);

>

>   	/* reset optind, the caller application might have used it */

>   	save_optind = optind;

>   	optind = 1;

>   	i = rte_eal_init(dpdk_argc, dpdk_argv);

>   	optind = save_optind;

> -	free(dpdk_argv[0]);

> -	free(dpdk_argv);

> +	free(full_cmdline);

>   	if (i < 0) {

>   		ODP_ERR("Cannot init the Intel DPDK EAL!\n");

>   		return -1;

>

Patch hide | download patch | download mbox

From f731548c68a915b6181f9b66c1407f22dad70588 Mon Sep 17 00:00:00 2001
From: Zoltan Kiss <zoltan.kiss@linaro.org>
Date: Thu, 26 Nov 2015 17:58:18 +0000
Subject: [PATCH] linux-dpdk: init: parse command line options using
 rte_strsplit()
To: lng-odp-dpdk@lists.linaro.org

From: Miroslav Kiradzhiyski <miroslav@virtualopensystems.com>

DPDK already implements rte_strsplit() which can be used instead of the
parse_dpdk_args() function. This saves some dynamic memory allocation
and simplifies the code.

Signed-off-by: Miroslav Kiradzhiyski <miroslav@virtualopensystems.com>
Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>
---
 platform/linux-dpdk/odp_init.c | 56 +++++++++++-------------------------------
 1 file changed, 14 insertions(+), 42 deletions(-)

diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux-dpdk/odp_init.c
index 3d7ccd2..a0be811 100644
--- a/platform/linux-dpdk/odp_init.c
+++ b/platform/linux-dpdk/odp_init.c
@@ -13,6 +13,7 @@ 
 #include <odp/system_info.h>
 #include <odp/cpumask.h>
 #include <unistd.h>
+#include <rte_string_fns.h>
 
 #define PMD_EXT(drv)  extern void devinitfn_##drv(void);
 PMD_EXT(pmd_af_packet_drv)
@@ -111,43 +112,12 @@  void refer_constructors(void) {
 #endif
 }
 
-static void parse_dpdk_args(const char *args, int *dst_argc, char ***dst_argv)
-{
-	char *buf = strdup(args);
-	int num = 1;
-	char *delim;
-	char **argv = calloc(num, sizeof(char *));
-
-	if (!buf || !argv)
-		ODP_ABORT("Can't allocate memory!\n");
-
-	argv[0] = buf;
-
-	while (1) {
-		delim = strchr(argv[num - 1], ' ');
-		if (delim == NULL)
-			break;
-		argv = realloc(argv, (num + 1) * sizeof(char *));
-		if (!argv)
-			ODP_ABORT("Can't reallocate memory!\n");
-		argv[num] = delim + 1;
-		*delim = 0;
-		num++;
-	}
-
-	*dst_argc = num;
-	*dst_argv = argv;
-
-	return;
-}
-
-
 static void print_dpdk_env_help(void)
 {
-	char **dpdk_argv;
-	int dpdk_argc, save_optind;
+	char help_str[] = "--help";
+	char *dpdk_argv[] = {help_str};
+	int save_optind, dpdk_argc = 1;
 
-	parse_dpdk_args("--help", &dpdk_argc, &dpdk_argv);
 	ODP_ERR("Neither (char *)platform_params were provided to "
 		"odp_init_global(),\n");
 	ODP_ERR("nor ODP_PLATFORM_PARAMS environment variable were "
@@ -159,8 +129,6 @@  static void print_dpdk_env_help(void)
 	optind = 1;
 	rte_eal_init(dpdk_argc, dpdk_argv);
 	optind = save_optind;
-	free(dpdk_argv[0]);
-	free(dpdk_argv);
 }
 
 
@@ -169,7 +137,7 @@  int odp_init_dpdk(const char *cmdline)
 	char **dpdk_argv;
 	int dpdk_argc;
 	char *full_cmdline;
-	int i, save_optind;
+	int i, save_optind, cmdlen;
 	odp_cpumask_t mask;
 	char mask_str[ODP_CPUMASK_STR_SIZE];
 	int32_t masklen;
@@ -197,21 +165,25 @@  int odp_init_dpdk(const char *cmdline)
 			      strlen(" ") + strlen(cmdline));
 
 	/* first argument is facility log, simply bind it to odpdpdk for now.*/
-	sprintf(full_cmdline, "odpdpdk -c %s %s", mask_str, cmdline);
+	cmdlen = sprintf(full_cmdline, "odpdpdk -c %s %s", mask_str, cmdline);
 
-	parse_dpdk_args(full_cmdline, &dpdk_argc, &dpdk_argv);
+	for (i = 0, dpdk_argc = 1; i < cmdlen; ++i)
+		if (!strncmp(&full_cmdline[i], " ", 1))
+			++dpdk_argc;
+	dpdk_argv = malloc(dpdk_argc * sizeof(char *));
+
+	dpdk_argc = rte_strsplit(full_cmdline, strlen(full_cmdline), dpdk_argv,
+				 dpdk_argc, ' ');
 	for (i = 0; i < dpdk_argc; ++i)
 		ODP_DBG("arg[%d]: %s\n", i, dpdk_argv[i]);
 	fflush(stdout);
-	free(full_cmdline);
 
 	/* reset optind, the caller application might have used it */
 	save_optind = optind;
 	optind = 1;
 	i = rte_eal_init(dpdk_argc, dpdk_argv);
 	optind = save_optind;
-	free(dpdk_argv[0]);
-	free(dpdk_argv);
+	free(full_cmdline);
 	if (i < 0) {
 		ODP_ERR("Cannot init the Intel DPDK EAL!\n");
 		return -1;
-- 
1.9.1