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

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

Checks

Context Check Description
ci/checkpatch warning coding style issues
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

Commit Message

Mingjin Ye Sept. 25, 2023, 9: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: 086eb64db39e ("test/pdump: add unit test for pdump library")
Fixes: 148f963fb532 ("xen: core library changes")
Fixes: af75078fece3 ("first public release")
Fixes: b8d5e544e73e ("test: add procfs error message for multi-process launch")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
v3:new solution.
---
 app/test/process.h | 52 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)
  

Comments

David Marchand Sept. 25, 2023, 9:58 a.m. UTC | #1
On Mon, Sep 25, 2023 at 11:54 AM Mingjin Ye <mingjinx.ye@intel.com> 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.

This patch looks wrong.

Can you provide a description of the issue?
  
Mingjin Ye Sept. 25, 2023, 10:09 a.m. UTC | #2
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, September 25, 2023 5:58 PM
> To: Ye, MingjinX <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhou, YidingX
> <yidingx.zhou@intel.com>; stable@dpdk.org
> Subject: Re: [PATCH v3] app/test: secondary process passes allow
> parameters
> 
> On Mon, Sep 25, 2023 at 11:54 AM Mingjin Ye <mingjinx.ye@intel.com>
> 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.
> 
> This patch looks wrong.
> 
> Can you provide a description of the issue?

CI/Checkpatch a warning has appeared. Marked as Superseded.
The new patch will replace it.
  
David Marchand Sept. 25, 2023, 10:19 a.m. UTC | #3
On Mon, Sep 25, 2023 at 12:09 PM Ye, MingjinX <mingjinx.ye@intel.com> 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.
> >
> > This patch looks wrong.
> >
> > Can you provide a description of the issue?
>
> CI/Checkpatch a warning has appeared. Marked as Superseded.
> The new patch will replace it.

Rather than a new patch because of some compilation issue on the
implementation, please explain what you are trying to fix.
It may avoid wasting time.
  

Patch

diff --git a/app/test/process.h b/app/test/process.h
index 1f073b9c5c..57d58309d8 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,44 @@  extern uint16_t flag_for_send_pkts;
 #endif
 #endif
 
+#define PREFIX_A "-a"
+
+static int
+add_parameter_a(char **argv, int max_capacity)
+{
+	struct rte_devargs *devargs;
+	int count = 0;
+	int name_length, data_length;
+	char *dev;
+	int malloc_szie;
+
+	RTE_EAL_DEVARGS_FOREACH(NULL, devargs) {
+		if (count >= max_capacity)
+			return count;
+
+		name_length = strlen(devargs->name);
+		if (devargs->data != NULL)
+			data_length = strlen(devargs->data);
+		else
+			data_length = 0;
+
+		malloc_szie = name_length + data_length + 1;
+		dev = malloc(malloc_szie);
+
+		strncpy(dev, devargs->name, name_length);
+		if (data_length > 0)
+			strncpy(dev + name_length, devargs->data, data_length);
+		memset(dev + malloc_szie - 1, 0, 1);
+
+		*(argv + count) =  strdup(PREFIX_A);
+		count++;
+
+		*(argv + count) = dev;
+		count++;
+	}
+	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 +84,7 @@  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[numargs + RTE_MAX_ETHPORTS * 2 + 1];
 	int i, status;
 	char path[32];
 #ifdef RTE_LIB_PDUMP
@@ -58,11 +98,12 @@  process_dup(const char *const argv[], int numargs, const char *env_value)
 	if (pid < 0)
 		return -1;
 	else if (pid == 0) {
+		memset(argv_cpy, 0, numargs + RTE_MAX_ETHPORTS * 2 + 1);
 		/* 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_a(&argv_cpy[i], RTE_MAX_ETHPORTS * 2);
 
 #ifdef RTE_EXEC_ENV_LINUX
 		{
@@ -131,6 +172,13 @@  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++) {
+			if (argv_cpy[i] != NULL) {
+				free(argv_cpy[i]);
+				argv_cpy[i] = NULL;
+			}
+		}
 	}
 	/* parent process does a wait */
 #ifdef RTE_LIB_PDUMP