[v7] app/test: secondary process passes allow parameters
Checks
Commit Message
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.
---
v7: Fix CI errors.
---
app/test/process.h | 46 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 42 insertions(+), 4 deletions(-)
Comments
> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Tuesday, November 14, 2023 6:28 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Ye, MingjinX
> <mingjinx.ye@intel.com>; stable@dpdk.org
> Subject: [PATCH v7] app/test: secondary process passes allow parameters
>
> 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.
> ---
> v7: Fix CI errors.
> ---
Verified pass for meson test:
dpdk-devbind.py -b vfio-pci 31:00.0 31:00.1 b1:00.0 b1:00.1
DPDK_TEST=eal_flags_file_prefix_autotest MALLOC_PERTURB_=192 /root/dpdk/x86_64-native-linuxapp-gcc/app/test/dpdk-test --file-prefix=eal_flags_file_prefix_autotest -a 31:00.0 -a 31:00.1
Tested-by: Zhimin Huang <zhiminx.huang@intel.com >
Hi Richardson,
can you please take a look at this patch.
Thanks,
Mingjin
> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Tuesday, November 14, 2023 6:28 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Ye, MingjinX
> <mingjinx.ye@intel.com>; stable@dpdk.org
> Subject: [PATCH v7] app/test: secondary process passes allow parameters
>
> 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.
> ---
> v7: Fix CI errors.
> ---
> app/test/process.h | 46
> ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/app/test/process.h b/app/test/process.h index
> af7bc3e0de..06b2f8b192 100644
> --- a/app/test/process.h
> +++ b/app/test/process.h
> @@ -17,6 +17,7 @@
> #include <dirent.h>
>
> #include <rte_string_fns.h> /* strlcpy */
> +#include <rte_devargs.h>
>
> #ifdef RTE_EXEC_ENV_FREEBSD
> #define self "curproc"
> @@ -34,6 +35,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 (devargs->data == NULL || 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 @@ -
> 43,8 +72,10 @@ extern uint16_t flag_for_send_pkts; static inline int
> process_dup(const char *const argv[], int numargs, const char *env_value) {
> - int num;
> - char *argv_cpy[numargs + 1];
> + int num = 0;
> + char **argv_cpy;
> + int allow_num;
> + int argv_num;
> int i, status;
> char path[32];
> #ifdef RTE_LIB_PDUMP
> @@ -58,11 +89,18 @@ 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 = calloc(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;
> + if (allow_num > 0)
> + num = add_parameter_allow(&argv_cpy[i],
> allow_num);
> + num += numargs;
>
> #ifdef RTE_EXEC_ENV_LINUX
> {
> --
> 2.25.1
On Fri, Nov 17, 2023 at 10:24:41AM +0000, Ye, MingjinX wrote:
> Hi Richardson,
>
> can you please take a look at this patch.
>
> Thanks,
> Mingjin
>
Hi,
I acked v6. If nothing major has changed in the patch, then the ack can be
kept for v7. If there is something that has changed that you think I should
look at, please include it in the change-log, so I can check it out.
For now, assuming no major changes:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
/Bruce
>
> > -----Original Message-----
> > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > Sent: Tuesday, November 14, 2023 6:28 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; Ye, MingjinX
> > <mingjinx.ye@intel.com>; stable@dpdk.org
> > Subject: [PATCH v7] app/test: secondary process passes allow parameters
> >
> > 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.
> > ---
> > v7: Fix CI errors.
> > ---
> > app/test/process.h | 46
> > ++++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test/process.h b/app/test/process.h index
> > af7bc3e0de..06b2f8b192 100644
> > --- a/app/test/process.h
> > +++ b/app/test/process.h
> > @@ -17,6 +17,7 @@
> > #include <dirent.h>
> >
> > #include <rte_string_fns.h> /* strlcpy */
> > +#include <rte_devargs.h>
> >
> > #ifdef RTE_EXEC_ENV_FREEBSD
> > #define self "curproc"
> > @@ -34,6 +35,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 (devargs->data == NULL || 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 @@ -
> > 43,8 +72,10 @@ extern uint16_t flag_for_send_pkts; static inline int
> > process_dup(const char *const argv[], int numargs, const char *env_value) {
> > - int num;
> > - char *argv_cpy[numargs + 1];
> > + int num = 0;
> > + char **argv_cpy;
> > + int allow_num;
> > + int argv_num;
> > int i, status;
> > char path[32];
> > #ifdef RTE_LIB_PDUMP
> > @@ -58,11 +89,18 @@ 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 = calloc(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;
> > + if (allow_num > 0)
> > + num = add_parameter_allow(&argv_cpy[i],
> > allow_num);
> > + num += numargs;
> >
> > #ifdef RTE_EXEC_ENV_LINUX
> > {
> > --
> > 2.25.1
>
Hi,
Thanks for the ack. would like to change the state of the patch, which will be merged into the dpdk.
Change: The "--allow" parameter is added when the number of allow is greater than zero.
Thanks,
Mingjin
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Friday, November 17, 2023 7:17 PM
> To: Ye, MingjinX <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> stable@dpdk.org; Xu, HailinX <hailinx.xu@intel.com>
> Subject: Re: [PATCH v7] app/test: secondary process passes allow
> parameters
>
> On Fri, Nov 17, 2023 at 10:24:41AM +0000, Ye, MingjinX wrote:
> > Hi Richardson,
> >
> > can you please take a look at this patch.
> >
> > Thanks,
> > Mingjin
> >
>
> Hi,
>
> I acked v6. If nothing major has changed in the patch, then the ack can be
> kept for v7. If there is something that has changed that you think I should
> look at, please include it in the change-log, so I can check it out.
>
> For now, assuming no major changes:
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> /Bruce
>
> >
> > > -----Original Message-----
> > > From: Ye, MingjinX <mingjinx.ye@intel.com>
> > > Sent: Tuesday, November 14, 2023 6:28 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Ye, MingjinX
> > > <mingjinx.ye@intel.com>; stable@dpdk.org
> > > Subject: [PATCH v7] app/test: secondary process passes allow
> > > parameters
> > >
> > > 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.
> > > ---
> > > v7: Fix CI errors.
> > > ---
> > > app/test/process.h | 46
> > > ++++++++++++++++++++++++++++++++++++++++++----
> > > 1 file changed, 42 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/app/test/process.h b/app/test/process.h index
> > > af7bc3e0de..06b2f8b192 100644
> > > --- a/app/test/process.h
> > > +++ b/app/test/process.h
> > > @@ -17,6 +17,7 @@
> > > #include <dirent.h>
> > >
> > > #include <rte_string_fns.h> /* strlcpy */
> > > +#include <rte_devargs.h>
> > >
> > > #ifdef RTE_EXEC_ENV_FREEBSD
> > > #define self "curproc"
> > > @@ -34,6 +35,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 (devargs->data == NULL || 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 @@ -
> > > 43,8 +72,10 @@ extern uint16_t flag_for_send_pkts; static inline
> > > int process_dup(const char *const argv[], int numargs, const char
> *env_value) {
> > > - int num;
> > > - char *argv_cpy[numargs + 1];
> > > + int num = 0;
> > > + char **argv_cpy;
> > > + int allow_num;
> > > + int argv_num;
> > > int i, status;
> > > char path[32];
> > > #ifdef RTE_LIB_PDUMP
> > > @@ -58,11 +89,18 @@ 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 = calloc(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;
> > > + if (allow_num > 0)
> > > + num = add_parameter_allow(&argv_cpy[i],
> > > allow_num);
> > > + num += numargs;
> > >
> > > #ifdef RTE_EXEC_ENV_LINUX
> > > {
> > > --
> > > 2.25.1
> >
14/11/2023 11:28, Mingjin Ye:
> 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>
Applied, thanks.
@@ -17,6 +17,7 @@
#include <dirent.h>
#include <rte_string_fns.h> /* strlcpy */
+#include <rte_devargs.h>
#ifdef RTE_EXEC_ENV_FREEBSD
#define self "curproc"
@@ -34,6 +35,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 (devargs->data == NULL || 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
@@ -43,8 +72,10 @@ extern uint16_t flag_for_send_pkts;
static inline int
process_dup(const char *const argv[], int numargs, const char *env_value)
{
- int num;
- char *argv_cpy[numargs + 1];
+ int num = 0;
+ char **argv_cpy;
+ int allow_num;
+ int argv_num;
int i, status;
char path[32];
#ifdef RTE_LIB_PDUMP
@@ -58,11 +89,18 @@ 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 = calloc(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;
+ if (allow_num > 0)
+ num = add_parameter_allow(&argv_cpy[i], allow_num);
+ num += numargs;
#ifdef RTE_EXEC_ENV_LINUX
{