[v6] app/test: secondary process passes allow parameters

Message ID 20231113104222.85891-1-mingjinx.ye@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v6] app/test: secondary process passes allow parameters |

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/github-robot: build fail github build: failed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-mellanox-Performance success Performance 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
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Mingjin Ye Nov. 13, 2023, 10:42 a.m. UTC
  In EAL related test cases, the allow parameters are not passed to
the secondary process, resulting in unexpected NICs being loaded.

This patch fixes this issue by appending the allow parameters to
the secondary process.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
v5: Optimized.
---
v6: Optimized.
---
 app/test/process.h | 52 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 3 deletions(-)
  

Comments

Bruce Richardson Nov. 13, 2023, 11:11 a.m. UTC | #1
On Mon, Nov 13, 2023 at 10:42:22AM +0000, Mingjin Ye wrote:
> In EAL related test cases, the allow parameters are not passed to
> the secondary process, resulting in unexpected NICs being loaded.
> 
> This patch fixes this issue by appending the allow parameters to
> the secondary process.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
> v5: Optimized.
> ---
> v6: Optimized.
> ---
>  app/test/process.h | 52 +++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test/process.h b/app/test/process.h
> index af7bc3e0de..cd3603b7bb 100644
> --- a/app/test/process.h
> +++ b/app/test/process.h
> @@ -18,6 +18,8 @@
>  
>  #include <rte_string_fns.h> /* strlcpy */
>  
> +#include <rte_devargs.h>
> +
>  #ifdef RTE_EXEC_ENV_FREEBSD
>  #define self "curproc"
>  #define exe "file"
> @@ -34,6 +36,34 @@ extern uint16_t flag_for_send_pkts;
>  #endif
>  #endif
>  
> +#define PREFIX_ALLOW "--allow="
> +
> +static int
> +add_parameter_allow(char **argv, int max_capacity)
> +{
> +	struct rte_devargs *devargs;
> +	int count = 0;
> +
> +	RTE_EAL_DEVARGS_FOREACH(NULL, devargs) {
> +		if (strlen(devargs->name) == 0)
> +			continue;
> +
> +		if (strlen(devargs->data) == 0) {
> +			if (asprintf(&argv[count], PREFIX_ALLOW"%s", devargs->name) < 0)
> +				break;
> +		} else {
> +			if (asprintf(&argv[count], PREFIX_ALLOW"%s,%s",
> +					 devargs->name, devargs->data) < 0)
> +				break;
> +		}
> +
> +		if (++count == max_capacity)
> +			break;
> +	}
> +
> +	return count;
> +}
> +
>  /*
>   * launches a second copy of the test process using the given argv parameters,
>   * which should include argv[0] as the process name. To identify in the
> @@ -44,7 +74,9 @@ static inline int
>  process_dup(const char *const argv[], int numargs, const char *env_value)
>  {
>  	int num;
> -	char *argv_cpy[numargs + 1];
> +	char **argv_cpy;
> +	int allow_num;
> +	int argv_num;
>  	int i, status;
>  	char path[32];
>  #ifdef RTE_LIB_PDUMP
> @@ -58,12 +90,21 @@ process_dup(const char *const argv[], int numargs, const char *env_value)
>  	if (pid < 0)
>  		return -1;
>  	else if (pid == 0) {
> +		allow_num = rte_devargs_type_count(RTE_DEVTYPE_ALLOWED);
> +		argv_num = numargs + allow_num + 1;
> +		argv_cpy = malloc(argv_num * sizeof(char *));
> +		if (!argv_cpy)
> +			rte_panic("Memory allocation failed\n");
> +
>  		/* make a copy of the arguments to be passed to exec */
>  		for (i = 0; i < numargs; i++)
>  			argv_cpy[i] = strdup(argv[i]);
> -		argv_cpy[i] = NULL;
> -		num = numargs;
> +		num = add_parameter_allow(&argv_cpy[i], allow_num);
> +		if (num != allow_num)
> +			rte_panic("Fill allow parameter incomplete\n");
>  
> +		num += numargs;
> +		argv_cpy[argv_num - 1] = NULL;
>  #ifdef RTE_EXEC_ENV_LINUX
>  		{
>  			const char *procdir = "/proc/" self "/fd/";
> @@ -131,6 +172,11 @@ process_dup(const char *const argv[], int numargs, const char *env_value)
>  			}
>  			rte_panic("Cannot exec: %s\n", strerror(errno));
>  		}
> +
> +		for (i = 0; i < num; i++)
> +			free(argv_cpy[i]);
> +
> +		free(argv_cpy);
>  	}
>  	/* parent process does a wait */
>  #ifdef RTE_LIB_PDUMP
> -- 
> 2.25.1
>
  

Patch

diff --git a/app/test/process.h b/app/test/process.h
index af7bc3e0de..cd3603b7bb 100644
--- a/app/test/process.h
+++ b/app/test/process.h
@@ -18,6 +18,8 @@ 
 
 #include <rte_string_fns.h> /* strlcpy */
 
+#include <rte_devargs.h>
+
 #ifdef RTE_EXEC_ENV_FREEBSD
 #define self "curproc"
 #define exe "file"
@@ -34,6 +36,34 @@  extern uint16_t flag_for_send_pkts;
 #endif
 #endif
 
+#define PREFIX_ALLOW "--allow="
+
+static int
+add_parameter_allow(char **argv, int max_capacity)
+{
+	struct rte_devargs *devargs;
+	int count = 0;
+
+	RTE_EAL_DEVARGS_FOREACH(NULL, devargs) {
+		if (strlen(devargs->name) == 0)
+			continue;
+
+		if (strlen(devargs->data) == 0) {
+			if (asprintf(&argv[count], PREFIX_ALLOW"%s", devargs->name) < 0)
+				break;
+		} else {
+			if (asprintf(&argv[count], PREFIX_ALLOW"%s,%s",
+					 devargs->name, devargs->data) < 0)
+				break;
+		}
+
+		if (++count == max_capacity)
+			break;
+	}
+
+	return count;
+}
+
 /*
  * launches a second copy of the test process using the given argv parameters,
  * which should include argv[0] as the process name. To identify in the
@@ -44,7 +74,9 @@  static inline int
 process_dup(const char *const argv[], int numargs, const char *env_value)
 {
 	int num;
-	char *argv_cpy[numargs + 1];
+	char **argv_cpy;
+	int allow_num;
+	int argv_num;
 	int i, status;
 	char path[32];
 #ifdef RTE_LIB_PDUMP
@@ -58,12 +90,21 @@  process_dup(const char *const argv[], int numargs, const char *env_value)
 	if (pid < 0)
 		return -1;
 	else if (pid == 0) {
+		allow_num = rte_devargs_type_count(RTE_DEVTYPE_ALLOWED);
+		argv_num = numargs + allow_num + 1;
+		argv_cpy = malloc(argv_num * sizeof(char *));
+		if (!argv_cpy)
+			rte_panic("Memory allocation failed\n");
+
 		/* make a copy of the arguments to be passed to exec */
 		for (i = 0; i < numargs; i++)
 			argv_cpy[i] = strdup(argv[i]);
-		argv_cpy[i] = NULL;
-		num = numargs;
+		num = add_parameter_allow(&argv_cpy[i], allow_num);
+		if (num != allow_num)
+			rte_panic("Fill allow parameter incomplete\n");
 
+		num += numargs;
+		argv_cpy[argv_num - 1] = NULL;
 #ifdef RTE_EXEC_ENV_LINUX
 		{
 			const char *procdir = "/proc/" self "/fd/";
@@ -131,6 +172,11 @@  process_dup(const char *const argv[], int numargs, const char *env_value)
 			}
 			rte_panic("Cannot exec: %s\n", strerror(errno));
 		}
+
+		for (i = 0; i < num; i++)
+			free(argv_cpy[i]);
+
+		free(argv_cpy);
 	}
 	/* parent process does a wait */
 #ifdef RTE_LIB_PDUMP