[v1,02/11] test/bbdev: update python script parameters
Checks
Commit Message
Update the timeout argument and default values.
Update EAL help message and default value.
Add iter_max and snr arguments.
Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
---
app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
app/test-bbdev/test_bbdev_perf.c | 2 +-
2 files changed, 19 insertions(+), 5 deletions(-)
Comments
On 9/29/23 20:13, Hernan Vargas wrote:
> Update the timeout argument and default values.
> Update EAL help message and default value.
> Add iter_max and snr arguments.
>
> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> ---
> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
> app/test-bbdev/test_bbdev_perf.c | 2 +-
> 2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/app/test-bbdev/test-bbdev.py b/app/test-bbdev/test-bbdev.py
> index 9cdb4659724d..8d0145076e4d 100755
> --- a/app/test-bbdev/test-bbdev.py
> +++ b/app/test-bbdev/test-bbdev.py
> @@ -25,12 +25,12 @@ def kill(process):
> help="specifies path to the bbdev test app",
> default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-bbdev")
> parser.add_argument("-e", "--eal-params",
> - help="EAL arguments which are passed to the test app",
> - default="--vdev=baseband_null0")
> -parser.add_argument("-t", "--timeout",
> + help="EAL arguments which must be passed to the test app",
> + default="--vdev=baseband_null0 -a00:00.0")
> +parser.add_argument("-T", "--timeout",
> type=int,
> help="Timeout in seconds",
> - default=300)
> + default=600)
> parser.add_argument("-c", "--test-cases",
> nargs="+",
> help="Defines test cases to run. Run all if not specified")
> @@ -48,6 +48,14 @@ def kill(process):
> type=int,
> help="Operations enqueue/dequeue burst size.",
> default=[32])
> +parser.add_argument("-s", "--snr",
> + type=int,
> + help="SNR in dB for BLER tests",
> + default=0)
> +parser.add_argument("-t", "--iter-max",
We shouldn't change parameters meaning, it will silently break existing
scripts making use of it.
> + type=int,
> + help="Max iterations",
> + default=6)
> parser.add_argument("-l", "--num-lcores",
> type=int,
> help="Number of lcores to run.",
> @@ -68,6 +76,12 @@ def kill(process):
>
> params.extend(["--"])
>
> +if args.snr:
> + params.extend(["-s", str(args.snr)])
> +
> +if args.iter_max:
> + params.extend(["-t", str(args.iter_max)])
> +
> if args.num_ops:
> params.extend(["-n", str(args.num_ops)])
>
> diff --git a/app/test-bbdev/test_bbdev_perf.c b/app/test-bbdev/test_bbdev_perf.c
> index 276bbf0a2e6d..faea26c10eed 100644
> --- a/app/test-bbdev/test_bbdev_perf.c
> +++ b/app/test-bbdev/test_bbdev_perf.c
> @@ -26,7 +26,7 @@
>
> #define MAX_QUEUES RTE_MAX_LCORE
> #define TEST_REPETITIONS 100
> -#define TIME_OUT_POLL 1e8
> +#define TIME_OUT_POLL 1e9
> #define WAIT_OFFLOAD_US 1000
>
> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
Hi Maxime,
I believe there was some historical discrepancy, even in doc both appeared but none of the 2 -t options with the cap.
https://doc.dpdk.org/guides/tools/testbbdev.html
Resolving this historical issue here.
Thanks
Nic
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, October 17, 2023 9:08 PM
> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters
>
>
>
> On 9/29/23 20:13, Hernan Vargas wrote:
> > Update the timeout argument and default values.
> > Update EAL help message and default value.
> > Add iter_max and snr arguments.
> >
> > Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > ---
> > app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
> > app/test-bbdev/test_bbdev_perf.c | 2 +-
> > 2 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/app/test-bbdev/test-bbdev.py
> > b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d 100755
> > --- a/app/test-bbdev/test-bbdev.py
> > +++ b/app/test-bbdev/test-bbdev.py
> > @@ -25,12 +25,12 @@ def kill(process):
> > help="specifies path to the bbdev test app",
> > default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-bbdev")
> > parser.add_argument("-e", "--eal-params",
> > - help="EAL arguments which are passed to the test app",
> > - default="--vdev=baseband_null0")
> > -parser.add_argument("-t", "--timeout",
> > + help="EAL arguments which must be passed to the test app",
> > + default="--vdev=baseband_null0 -a00:00.0")
> > +parser.add_argument("-T", "--timeout",
> > type=int,
> > help="Timeout in seconds",
> > - default=300)
> > + default=600)
> > parser.add_argument("-c", "--test-cases",
> > nargs="+",
> > help="Defines test cases to run. Run all if not
> > specified") @@ -48,6 +48,14 @@ def kill(process):
> > type=int,
> > help="Operations enqueue/dequeue burst size.",
> > default=[32])
> > +parser.add_argument("-s", "--snr",
> > + type=int,
> > + help="SNR in dB for BLER tests",
> > + default=0)
> > +parser.add_argument("-t", "--iter-max",
>
> We shouldn't change parameters meaning, it will silently break existing scripts
> making use of it.
>
> > + type=int,
> > + help="Max iterations",
> > + default=6)
> > parser.add_argument("-l", "--num-lcores",
> > type=int,
> > help="Number of lcores to run.", @@ -68,6 +76,12
> > @@ def kill(process):
> >
> > params.extend(["--"])
> >
> > +if args.snr:
> > + params.extend(["-s", str(args.snr)])
> > +
> > +if args.iter_max:
> > + params.extend(["-t", str(args.iter_max)])
> > +
> > if args.num_ops:
> > params.extend(["-n", str(args.num_ops)])
> >
> > diff --git a/app/test-bbdev/test_bbdev_perf.c
> > b/app/test-bbdev/test_bbdev_perf.c
> > index 276bbf0a2e6d..faea26c10eed 100644
> > --- a/app/test-bbdev/test_bbdev_perf.c
> > +++ b/app/test-bbdev/test_bbdev_perf.c
> > @@ -26,7 +26,7 @@
> >
> > #define MAX_QUEUES RTE_MAX_LCORE
> > #define TEST_REPETITIONS 100
> > -#define TIME_OUT_POLL 1e8
> > +#define TIME_OUT_POLL 1e9
> > #define WAIT_OFFLOAD_US 1000
> >
> > #ifdef RTE_BASEBAND_FPGA_LTE_FEC
On 10/19/23 11:01, Chautru, Nicolas wrote:
> Hi Maxime,
>
> I believe there was some historical discrepancy, even in doc both appeared but none of the 2 -t options with the cap.
> https://doc.dpdk.org/guides/tools/testbbdev.html
> Resolving this historical issue here.
Ok, then we should fix the doc, not the code.
Thanks,
Maxime
> Thanks
> Nic
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, October 17, 2023 9:08 PM
>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters
>>
>>
>>
>> On 9/29/23 20:13, Hernan Vargas wrote:
>>> Update the timeout argument and default values.
>>> Update EAL help message and default value.
>>> Add iter_max and snr arguments.
>>>
>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>> ---
>>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
>>> app/test-bbdev/test_bbdev_perf.c | 2 +-
>>> 2 files changed, 19 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/app/test-bbdev/test-bbdev.py
>>> b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d 100755
>>> --- a/app/test-bbdev/test-bbdev.py
>>> +++ b/app/test-bbdev/test-bbdev.py
>>> @@ -25,12 +25,12 @@ def kill(process):
>>> help="specifies path to the bbdev test app",
>>> default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-bbdev")
>>> parser.add_argument("-e", "--eal-params",
>>> - help="EAL arguments which are passed to the test app",
>>> - default="--vdev=baseband_null0")
>>> -parser.add_argument("-t", "--timeout",
>>> + help="EAL arguments which must be passed to the test app",
>>> + default="--vdev=baseband_null0 -a00:00.0")
>>> +parser.add_argument("-T", "--timeout",
>>> type=int,
>>> help="Timeout in seconds",
>>> - default=300)
>>> + default=600)
>>> parser.add_argument("-c", "--test-cases",
>>> nargs="+",
>>> help="Defines test cases to run. Run all if not
>>> specified") @@ -48,6 +48,14 @@ def kill(process):
>>> type=int,
>>> help="Operations enqueue/dequeue burst size.",
>>> default=[32])
>>> +parser.add_argument("-s", "--snr",
>>> + type=int,
>>> + help="SNR in dB for BLER tests",
>>> + default=0)
>>> +parser.add_argument("-t", "--iter-max",
>>
>> We shouldn't change parameters meaning, it will silently break existing scripts
>> making use of it.
>>
>>> + type=int,
>>> + help="Max iterations",
>>> + default=6)
>>> parser.add_argument("-l", "--num-lcores",
>>> type=int,
>>> help="Number of lcores to run.", @@ -68,6 +76,12
>>> @@ def kill(process):
>>>
>>> params.extend(["--"])
>>>
>>> +if args.snr:
>>> + params.extend(["-s", str(args.snr)])
>>> +
>>> +if args.iter_max:
>>> + params.extend(["-t", str(args.iter_max)])
>>> +
>>> if args.num_ops:
>>> params.extend(["-n", str(args.num_ops)])
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>> b/app/test-bbdev/test_bbdev_perf.c
>>> index 276bbf0a2e6d..faea26c10eed 100644
>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>> @@ -26,7 +26,7 @@
>>>
>>> #define MAX_QUEUES RTE_MAX_LCORE
>>> #define TEST_REPETITIONS 100
>>> -#define TIME_OUT_POLL 1e8
>>> +#define TIME_OUT_POLL 1e9
>>> #define WAIT_OFFLOAD_US 1000
>>>
>>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
>
Hi Maxime,
In practice anyone using that API is already using the one defined in the patch below and not using -t for time out. So not a concern to do it properly through that patch.
Heman, any concern on your side with this change?
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, October 19, 2023 11:19 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
> <trix@redhat.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters
>
>
>
> On 10/19/23 11:01, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> > I believe there was some historical discrepancy, even in doc both appeared
> but none of the 2 -t options with the cap.
> > https://doc.dpdk.org/guides/tools/testbbdev.html
> > Resolving this historical issue here.
>
> Ok, then we should fix the doc, not the code.
>
> Thanks,
> Maxime
>
> > Thanks
> > Nic
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Tuesday, October 17, 2023 9:08 PM
> >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> >> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>
> >> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
> >> parameters
> >>
> >>
> >>
> >> On 9/29/23 20:13, Hernan Vargas wrote:
> >>> Update the timeout argument and default values.
> >>> Update EAL help message and default value.
> >>> Add iter_max and snr arguments.
> >>>
> >>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> >>> ---
> >>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
> >>> app/test-bbdev/test_bbdev_perf.c | 2 +-
> >>> 2 files changed, 19 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/app/test-bbdev/test-bbdev.py
> >>> b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d
> >>> 100755
> >>> --- a/app/test-bbdev/test-bbdev.py
> >>> +++ b/app/test-bbdev/test-bbdev.py
> >>> @@ -25,12 +25,12 @@ def kill(process):
> >>> help="specifies path to the bbdev test app",
> >>> default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-
> bbdev")
> >>> parser.add_argument("-e", "--eal-params",
> >>> - help="EAL arguments which are passed to the test app",
> >>> - default="--vdev=baseband_null0")
> >>> -parser.add_argument("-t", "--timeout",
> >>> + help="EAL arguments which must be passed to the test app",
> >>> + default="--vdev=baseband_null0 -a00:00.0")
> >>> +parser.add_argument("-T", "--timeout",
> >>> type=int,
> >>> help="Timeout in seconds",
> >>> - default=300)
> >>> + default=600)
> >>> parser.add_argument("-c", "--test-cases",
> >>> nargs="+",
> >>> help="Defines test cases to run. Run all if
> >>> not
> >>> specified") @@ -48,6 +48,14 @@ def kill(process):
> >>> type=int,
> >>> help="Operations enqueue/dequeue burst size.",
> >>> default=[32])
> >>> +parser.add_argument("-s", "--snr",
> >>> + type=int,
> >>> + help="SNR in dB for BLER tests",
> >>> + default=0)
> >>> +parser.add_argument("-t", "--iter-max",
> >>
> >> We shouldn't change parameters meaning, it will silently break
> >> existing scripts making use of it.
> >>
> >>> + type=int,
> >>> + help="Max iterations",
> >>> + default=6)
> >>> parser.add_argument("-l", "--num-lcores",
> >>> type=int,
> >>> help="Number of lcores to run.", @@ -68,6
> >>> +76,12 @@ def kill(process):
> >>>
> >>> params.extend(["--"])
> >>>
> >>> +if args.snr:
> >>> + params.extend(["-s", str(args.snr)])
> >>> +
> >>> +if args.iter_max:
> >>> + params.extend(["-t", str(args.iter_max)])
> >>> +
> >>> if args.num_ops:
> >>> params.extend(["-n", str(args.num_ops)])
> >>>
> >>> diff --git a/app/test-bbdev/test_bbdev_perf.c
> >>> b/app/test-bbdev/test_bbdev_perf.c
> >>> index 276bbf0a2e6d..faea26c10eed 100644
> >>> --- a/app/test-bbdev/test_bbdev_perf.c
> >>> +++ b/app/test-bbdev/test_bbdev_perf.c
> >>> @@ -26,7 +26,7 @@
> >>>
> >>> #define MAX_QUEUES RTE_MAX_LCORE
> >>> #define TEST_REPETITIONS 100
> >>> -#define TIME_OUT_POLL 1e8
> >>> +#define TIME_OUT_POLL 1e9
> >>> #define WAIT_OFFLOAD_US 1000
> >>>
> >>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
> >
Hi Nicolas,
On 10/19/23 14:09, Chautru, Nicolas wrote:
> Hi Maxime,
> In practice anyone using that API is already using the one defined in the patch below and not using -t for time out. So not a concern to do it properly through that patch.
Sorry, I'm not sure to follow you.
For example in RHEL, we use this script for BBDEV validation for our
downstream packages.
Maxime
> Heman, any concern on your side with this change?
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, October 19, 2023 11:19 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
>> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
>> <trix@redhat.com>
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters
>>
>>
>>
>> On 10/19/23 11:01, Chautru, Nicolas wrote:
>>> Hi Maxime,
>>>
>>> I believe there was some historical discrepancy, even in doc both appeared
>> but none of the 2 -t options with the cap.
>>> https://doc.dpdk.org/guides/tools/testbbdev.html
>>> Resolving this historical issue here.
>>
>> Ok, then we should fix the doc, not the code.
>>
>> Thanks,
>> Maxime
>>
>>> Thanks
>>> Nic
>>>
>>>> -----Original Message-----
>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Sent: Tuesday, October 17, 2023 9:08 PM
>>>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>>>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>>>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>
>>>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
>>>> parameters
>>>>
>>>>
>>>>
>>>> On 9/29/23 20:13, Hernan Vargas wrote:
>>>>> Update the timeout argument and default values.
>>>>> Update EAL help message and default value.
>>>>> Add iter_max and snr arguments.
>>>>>
>>>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>>>> ---
>>>>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
>>>>> app/test-bbdev/test_bbdev_perf.c | 2 +-
>>>>> 2 files changed, 19 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/app/test-bbdev/test-bbdev.py
>>>>> b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d
>>>>> 100755
>>>>> --- a/app/test-bbdev/test-bbdev.py
>>>>> +++ b/app/test-bbdev/test-bbdev.py
>>>>> @@ -25,12 +25,12 @@ def kill(process):
>>>>> help="specifies path to the bbdev test app",
>>>>> default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-
>> bbdev")
>>>>> parser.add_argument("-e", "--eal-params",
>>>>> - help="EAL arguments which are passed to the test app",
>>>>> - default="--vdev=baseband_null0")
>>>>> -parser.add_argument("-t", "--timeout",
>>>>> + help="EAL arguments which must be passed to the test app",
>>>>> + default="--vdev=baseband_null0 -a00:00.0")
>>>>> +parser.add_argument("-T", "--timeout",
>>>>> type=int,
>>>>> help="Timeout in seconds",
>>>>> - default=300)
>>>>> + default=600)
>>>>> parser.add_argument("-c", "--test-cases",
>>>>> nargs="+",
>>>>> help="Defines test cases to run. Run all if
>>>>> not
>>>>> specified") @@ -48,6 +48,14 @@ def kill(process):
>>>>> type=int,
>>>>> help="Operations enqueue/dequeue burst size.",
>>>>> default=[32])
>>>>> +parser.add_argument("-s", "--snr",
>>>>> + type=int,
>>>>> + help="SNR in dB for BLER tests",
>>>>> + default=0)
>>>>> +parser.add_argument("-t", "--iter-max",
>>>>
>>>> We shouldn't change parameters meaning, it will silently break
>>>> existing scripts making use of it.
>>>>
>>>>> + type=int,
>>>>> + help="Max iterations",
>>>>> + default=6)
>>>>> parser.add_argument("-l", "--num-lcores",
>>>>> type=int,
>>>>> help="Number of lcores to run.", @@ -68,6
>>>>> +76,12 @@ def kill(process):
>>>>>
>>>>> params.extend(["--"])
>>>>>
>>>>> +if args.snr:
>>>>> + params.extend(["-s", str(args.snr)])
>>>>> +
>>>>> +if args.iter_max:
>>>>> + params.extend(["-t", str(args.iter_max)])
>>>>> +
>>>>> if args.num_ops:
>>>>> params.extend(["-n", str(args.num_ops)])
>>>>>
>>>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>>>> b/app/test-bbdev/test_bbdev_perf.c
>>>>> index 276bbf0a2e6d..faea26c10eed 100644
>>>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>>>> @@ -26,7 +26,7 @@
>>>>>
>>>>> #define MAX_QUEUES RTE_MAX_LCORE
>>>>> #define TEST_REPETITIONS 100
>>>>> -#define TIME_OUT_POLL 1e8
>>>>> +#define TIME_OUT_POLL 1e9
>>>>> #define WAIT_OFFLOAD_US 1000
>>>>>
>>>>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
>>>
>
Hi Maxime,
Back on this discussion and there was some misunderstanding. This is really a bug for the python script helper only.
The -t option is already reserved for bbdev-test application (and in doc) for setting the iter_max (see main.c).
The problem is that the python script introduced -t by mistake for an additional time out when calling the binary to be handled in the script only, now resolving this by using -T to avoid clash with existing -t option.
No one is genuinely using -t for timeout.
Ping me if still unclear
Nic
> -----Original Message-----
> From: Chautru, Nicolas
> Sent: Thursday, October 19, 2023 8:10 AM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Vargas, Hernan
> <Hernan.Vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
> <trix@redhat.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: RE: [PATCH v1 02/11] test/bbdev: update python script parameters
>
> Hi Maxime,
> In practice anyone using that API is already using the one defined in the
> patch below and not using -t for time out. So not a concern to do it properly
> through that patch.
> Heman, any concern on your side with this change?
>
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Thursday, October 19, 2023 11:19 AM
> > To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
> > <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix,
> Tom
> > <trix@redhat.com>
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
> > parameters
> >
> >
> >
> > On 10/19/23 11:01, Chautru, Nicolas wrote:
> > > Hi Maxime,
> > >
> > > I believe there was some historical discrepancy, even in doc both
> > > appeared
> > but none of the 2 -t options with the cap.
> > > https://doc.dpdk.org/guides/tools/testbbdev.html
> > > Resolving this historical issue here.
> >
> > Ok, then we should fix the doc, not the code.
> >
> > Thanks,
> > Maxime
> >
> > > Thanks
> > > Nic
> > >
> > >> -----Original Message-----
> > >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > >> Sent: Tuesday, October 17, 2023 9:08 PM
> > >> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> > >> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> > >> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> > >> <qi.z.zhang@intel.com>
> > >> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
> > >> parameters
> > >>
> > >>
> > >>
> > >> On 9/29/23 20:13, Hernan Vargas wrote:
> > >>> Update the timeout argument and default values.
> > >>> Update EAL help message and default value.
> > >>> Add iter_max and snr arguments.
> > >>>
> > >>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> > >>> ---
> > >>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
> > >>> app/test-bbdev/test_bbdev_perf.c | 2 +-
> > >>> 2 files changed, 19 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/app/test-bbdev/test-bbdev.py
> > >>> b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d
> > >>> 100755
> > >>> --- a/app/test-bbdev/test-bbdev.py
> > >>> +++ b/app/test-bbdev/test-bbdev.py
> > >>> @@ -25,12 +25,12 @@ def kill(process):
> > >>> help="specifies path to the bbdev test app",
> > >>> default=dpdk_path + "/" + dpdk_target +
> > >>> "/app/dpdk-test-
> > bbdev")
> > >>> parser.add_argument("-e", "--eal-params",
> > >>> - help="EAL arguments which are passed to the test app",
> > >>> - default="--vdev=baseband_null0")
> > >>> -parser.add_argument("-t", "--timeout",
> > >>> + help="EAL arguments which must be passed to the test
> app",
> > >>> + default="--vdev=baseband_null0 -a00:00.0")
> > >>> +parser.add_argument("-T", "--timeout",
> > >>> type=int,
> > >>> help="Timeout in seconds",
> > >>> - default=300)
> > >>> + default=600)
> > >>> parser.add_argument("-c", "--test-cases",
> > >>> nargs="+",
> > >>> help="Defines test cases to run. Run all if
> > >>> not
> > >>> specified") @@ -48,6 +48,14 @@ def kill(process):
> > >>> type=int,
> > >>> help="Operations enqueue/dequeue burst size.",
> > >>> default=[32])
> > >>> +parser.add_argument("-s", "--snr",
> > >>> + type=int,
> > >>> + help="SNR in dB for BLER tests",
> > >>> + default=0)
> > >>> +parser.add_argument("-t", "--iter-max",
> > >>
> > >> We shouldn't change parameters meaning, it will silently break
> > >> existing scripts making use of it.
> > >>
> > >>> + type=int,
> > >>> + help="Max iterations",
> > >>> + default=6)
> > >>> parser.add_argument("-l", "--num-lcores",
> > >>> type=int,
> > >>> help="Number of lcores to run.", @@ -68,6
> > >>> +76,12 @@ def kill(process):
> > >>>
> > >>> params.extend(["--"])
> > >>>
> > >>> +if args.snr:
> > >>> + params.extend(["-s", str(args.snr)])
> > >>> +
> > >>> +if args.iter_max:
> > >>> + params.extend(["-t", str(args.iter_max)])
> > >>> +
> > >>> if args.num_ops:
> > >>> params.extend(["-n", str(args.num_ops)])
> > >>>
> > >>> diff --git a/app/test-bbdev/test_bbdev_perf.c
> > >>> b/app/test-bbdev/test_bbdev_perf.c
> > >>> index 276bbf0a2e6d..faea26c10eed 100644
> > >>> --- a/app/test-bbdev/test_bbdev_perf.c
> > >>> +++ b/app/test-bbdev/test_bbdev_perf.c
> > >>> @@ -26,7 +26,7 @@
> > >>>
> > >>> #define MAX_QUEUES RTE_MAX_LCORE
> > >>> #define TEST_REPETITIONS 100
> > >>> -#define TIME_OUT_POLL 1e8
> > >>> +#define TIME_OUT_POLL 1e9
> > >>> #define WAIT_OFFLOAD_US 1000
> > >>>
> > >>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
> > >
Hi Nicolas,
On 10/27/23 22:02, Chautru, Nicolas wrote:
> Hi Maxime,
>
> Back on this discussion and there was some misunderstanding. This is really a bug for the python script helper only.
> The -t option is already reserved for bbdev-test application (and in doc) for setting the iter_max (see main.c).
> The problem is that the python script introduced -t by mistake for an additional time out when calling the binary to be handled in the script only, now resolving this by using -T to avoid clash with existing -t option.
> No one is genuinely using -t for timeout.
> Ping me if still unclear
I actually understood it from the beginning, -t was used as timeout
option in the test-bbdev.py script, this patch changes -t to now
represent the maximum number of iterations.
I'm sure you can see the problem if someone was using -t for timeout in
some CI (Neither me or you can guarantee this is not used)?
If you think we really should change the meaning of this option, we
should have a deprecation notice, have both -t and -T to represent
timeout during the deprecation period and emit a warning when -t is
used. Once deprecated, you can assign -t to max iterations.
Sounds good?
Regards,
Maxime
> Nic
>
>> -----Original Message-----
>> From: Chautru, Nicolas
>> Sent: Thursday, October 19, 2023 8:10 AM
>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Vargas, Hernan
>> <Hernan.Vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
>> <trix@redhat.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
>> Subject: RE: [PATCH v1 02/11] test/bbdev: update python script parameters
>>
>> Hi Maxime,
>> In practice anyone using that API is already using the one defined in the
>> patch below and not using -t for time out. So not a concern to do it properly
>> through that patch.
>> Heman, any concern on your side with this change?
>>
>>> -----Original Message-----
>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Sent: Thursday, October 19, 2023 11:19 AM
>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
>>> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix,
>> Tom
>>> <trix@redhat.com>
>>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
>>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
>>> parameters
>>>
>>>
>>>
>>> On 10/19/23 11:01, Chautru, Nicolas wrote:
>>>> Hi Maxime,
>>>>
>>>> I believe there was some historical discrepancy, even in doc both
>>>> appeared
>>> but none of the 2 -t options with the cap.
>>>> https://doc.dpdk.org/guides/tools/testbbdev.html
>>>> Resolving this historical issue here.
>>>
>>> Ok, then we should fix the doc, not the code.
>>>
>>> Thanks,
>>> Maxime
>>>
>>>> Thanks
>>>> Nic
>>>>
>>>>> -----Original Message-----
>>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>> Sent: Tuesday, October 17, 2023 9:08 PM
>>>>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>>>>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>>>>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>>>>> <qi.z.zhang@intel.com>
>>>>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
>>>>> parameters
>>>>>
>>>>>
>>>>>
>>>>> On 9/29/23 20:13, Hernan Vargas wrote:
>>>>>> Update the timeout argument and default values.
>>>>>> Update EAL help message and default value.
>>>>>> Add iter_max and snr arguments.
>>>>>>
>>>>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>>>>> ---
>>>>>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
>>>>>> app/test-bbdev/test_bbdev_perf.c | 2 +-
>>>>>> 2 files changed, 19 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/app/test-bbdev/test-bbdev.py
>>>>>> b/app/test-bbdev/test-bbdev.py index 9cdb4659724d..8d0145076e4d
>>>>>> 100755
>>>>>> --- a/app/test-bbdev/test-bbdev.py
>>>>>> +++ b/app/test-bbdev/test-bbdev.py
>>>>>> @@ -25,12 +25,12 @@ def kill(process):
>>>>>> help="specifies path to the bbdev test app",
>>>>>> default=dpdk_path + "/" + dpdk_target +
>>>>>> "/app/dpdk-test-
>>> bbdev")
>>>>>> parser.add_argument("-e", "--eal-params",
>>>>>> - help="EAL arguments which are passed to the test app",
>>>>>> - default="--vdev=baseband_null0")
>>>>>> -parser.add_argument("-t", "--timeout",
>>>>>> + help="EAL arguments which must be passed to the test
>> app",
>>>>>> + default="--vdev=baseband_null0 -a00:00.0")
>>>>>> +parser.add_argument("-T", "--timeout",
>>>>>> type=int,
>>>>>> help="Timeout in seconds",
>>>>>> - default=300)
>>>>>> + default=600)
>>>>>> parser.add_argument("-c", "--test-cases",
>>>>>> nargs="+",
>>>>>> help="Defines test cases to run. Run all if
>>>>>> not
>>>>>> specified") @@ -48,6 +48,14 @@ def kill(process):
>>>>>> type=int,
>>>>>> help="Operations enqueue/dequeue burst size.",
>>>>>> default=[32])
>>>>>> +parser.add_argument("-s", "--snr",
>>>>>> + type=int,
>>>>>> + help="SNR in dB for BLER tests",
>>>>>> + default=0)
>>>>>> +parser.add_argument("-t", "--iter-max",
>>>>>
>>>>> We shouldn't change parameters meaning, it will silently break
>>>>> existing scripts making use of it.
>>>>>
>>>>>> + type=int,
>>>>>> + help="Max iterations",
>>>>>> + default=6)
>>>>>> parser.add_argument("-l", "--num-lcores",
>>>>>> type=int,
>>>>>> help="Number of lcores to run.", @@ -68,6
>>>>>> +76,12 @@ def kill(process):
>>>>>>
>>>>>> params.extend(["--"])
>>>>>>
>>>>>> +if args.snr:
>>>>>> + params.extend(["-s", str(args.snr)])
>>>>>> +
>>>>>> +if args.iter_max:
>>>>>> + params.extend(["-t", str(args.iter_max)])
>>>>>> +
>>>>>> if args.num_ops:
>>>>>> params.extend(["-n", str(args.num_ops)])
>>>>>>
>>>>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>>>>> b/app/test-bbdev/test_bbdev_perf.c
>>>>>> index 276bbf0a2e6d..faea26c10eed 100644
>>>>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>>>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>>>>> @@ -26,7 +26,7 @@
>>>>>>
>>>>>> #define MAX_QUEUES RTE_MAX_LCORE
>>>>>> #define TEST_REPETITIONS 100
>>>>>> -#define TIME_OUT_POLL 1e8
>>>>>> +#define TIME_OUT_POLL 1e9
>>>>>> #define WAIT_OFFLOAD_US 1000
>>>>>>
>>>>>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
>>>>
>
Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, November 2, 2023 10:00 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
> <trix@redhat.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters
>
> Hi Nicolas,
>
> On 10/27/23 22:02, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> > Back on this discussion and there was some misunderstanding. This is really
> a bug for the python script helper only.
> > The -t option is already reserved for bbdev-test application (and in doc) for
> setting the iter_max (see main.c).
> > The problem is that the python script introduced -t by mistake for an
> additional time out when calling the binary to be handled in the script only,
> now resolving this by using -T to avoid clash with existing -t option.
> > No one is genuinely using -t for timeout.
> > Ping me if still unclear
>
> I actually understood it from the beginning, -t was used as timeout option in
> the test-bbdev.py script, this patch changes -t to now represent the maximum
> number of iterations.
>
> I'm sure you can see the problem if someone was using -t for timeout in some
> CI (Neither me or you can guarantee this is not used)?
>
> If you think we really should change the meaning of this option, we should
> have a deprecation notice, have both -t and -T to represent timeout during
> the deprecation period and emit a warning when -t is used. Once deprecated,
> you can assign -t to max iterations.
>
> Sounds good?
The problem with your option is that during that period of time the script is still broken since you cannot set the number of iterations which is what -t is meant for in the binary application.
This is really a fix, and until that fix is applied the python script should not be used, instead the binary should be called directly.
If you want to do it your suggested way and then do the formal fix in 24.03 that is possible even if not ideal to me, but it would still mean the script should arguably not be used during that period of time from my point of view.
Let me know
>
> Regards,
> Maxime
>
> > Nic
> >
> >> -----Original Message-----
> >> From: Chautru, Nicolas
> >> Sent: Thursday, October 19, 2023 8:10 AM
> >> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Vargas, Hernan
> >> <Hernan.Vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix,
> Tom
> >> <trix@redhat.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
> >> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> >> Subject: RE: [PATCH v1 02/11] test/bbdev: update python script
> >> parameters
> >>
> >> Hi Maxime,
> >> In practice anyone using that API is already using the one defined in
> >> the patch below and not using -t for time out. So not a concern to do
> >> it properly through that patch.
> >> Heman, any concern on your side with this change?
> >>
> >>> -----Original Message-----
> >>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>> Sent: Thursday, October 19, 2023 11:19 AM
> >>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
> >>> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix,
> >> Tom
> >>> <trix@redhat.com>
> >>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
> >>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
> >>> parameters
> >>>
> >>>
> >>>
> >>> On 10/19/23 11:01, Chautru, Nicolas wrote:
> >>>> Hi Maxime,
> >>>>
> >>>> I believe there was some historical discrepancy, even in doc both
> >>>> appeared
> >>> but none of the 2 -t options with the cap.
> >>>> https://doc.dpdk.org/guides/tools/testbbdev.html
> >>>> Resolving this historical issue here.
> >>>
> >>> Ok, then we should fix the doc, not the code.
> >>>
> >>> Thanks,
> >>> Maxime
> >>>
> >>>> Thanks
> >>>> Nic
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>>> Sent: Tuesday, October 17, 2023 9:08 PM
> >>>>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
> >>>>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
> >>>>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
> >>>>> <qi.z.zhang@intel.com>
> >>>>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
> >>>>> parameters
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 9/29/23 20:13, Hernan Vargas wrote:
> >>>>>> Update the timeout argument and default values.
> >>>>>> Update EAL help message and default value.
> >>>>>> Add iter_max and snr arguments.
> >>>>>>
> >>>>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
> >>>>>> ---
> >>>>>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
> >>>>>> app/test-bbdev/test_bbdev_perf.c | 2 +-
> >>>>>> 2 files changed, 19 insertions(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/app/test-bbdev/test-bbdev.py
> >>>>>> b/app/test-bbdev/test-bbdev.py index
> 9cdb4659724d..8d0145076e4d
> >>>>>> 100755
> >>>>>> --- a/app/test-bbdev/test-bbdev.py
> >>>>>> +++ b/app/test-bbdev/test-bbdev.py
> >>>>>> @@ -25,12 +25,12 @@ def kill(process):
> >>>>>> help="specifies path to the bbdev test app",
> >>>>>> default=dpdk_path + "/" + dpdk_target +
> >>>>>> "/app/dpdk-test-
> >>> bbdev")
> >>>>>> parser.add_argument("-e", "--eal-params",
> >>>>>> - help="EAL arguments which are passed to the test app",
> >>>>>> - default="--vdev=baseband_null0")
> >>>>>> -parser.add_argument("-t", "--timeout",
> >>>>>> + help="EAL arguments which must be passed to
> >>>>>> + the test
> >> app",
> >>>>>> + default="--vdev=baseband_null0 -a00:00.0")
> >>>>>> +parser.add_argument("-T", "--timeout",
> >>>>>> type=int,
> >>>>>> help="Timeout in seconds",
> >>>>>> - default=300)
> >>>>>> + default=600)
> >>>>>> parser.add_argument("-c", "--test-cases",
> >>>>>> nargs="+",
> >>>>>> help="Defines test cases to run. Run all
> >>>>>> if not
> >>>>>> specified") @@ -48,6 +48,14 @@ def kill(process):
> >>>>>> type=int,
> >>>>>> help="Operations enqueue/dequeue burst size.",
> >>>>>> default=[32])
> >>>>>> +parser.add_argument("-s", "--snr",
> >>>>>> + type=int,
> >>>>>> + help="SNR in dB for BLER tests",
> >>>>>> + default=0)
> >>>>>> +parser.add_argument("-t", "--iter-max",
> >>>>>
> >>>>> We shouldn't change parameters meaning, it will silently break
> >>>>> existing scripts making use of it.
> >>>>>
> >>>>>> + type=int,
> >>>>>> + help="Max iterations",
> >>>>>> + default=6)
> >>>>>> parser.add_argument("-l", "--num-lcores",
> >>>>>> type=int,
> >>>>>> help="Number of lcores to run.", @@ -68,6
> >>>>>> +76,12 @@ def kill(process):
> >>>>>>
> >>>>>> params.extend(["--"])
> >>>>>>
> >>>>>> +if args.snr:
> >>>>>> + params.extend(["-s", str(args.snr)])
> >>>>>> +
> >>>>>> +if args.iter_max:
> >>>>>> + params.extend(["-t", str(args.iter_max)])
> >>>>>> +
> >>>>>> if args.num_ops:
> >>>>>> params.extend(["-n", str(args.num_ops)])
> >>>>>>
> >>>>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
> >>>>>> b/app/test-bbdev/test_bbdev_perf.c
> >>>>>> index 276bbf0a2e6d..faea26c10eed 100644
> >>>>>> --- a/app/test-bbdev/test_bbdev_perf.c
> >>>>>> +++ b/app/test-bbdev/test_bbdev_perf.c
> >>>>>> @@ -26,7 +26,7 @@
> >>>>>>
> >>>>>> #define MAX_QUEUES RTE_MAX_LCORE
> >>>>>> #define TEST_REPETITIONS 100
> >>>>>> -#define TIME_OUT_POLL 1e8
> >>>>>> +#define TIME_OUT_POLL 1e9
> >>>>>> #define WAIT_OFFLOAD_US 1000
> >>>>>>
> >>>>>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
> >>>>
> >
On 11/2/23 19:18, Chautru, Nicolas wrote:
> Hi Maxime,
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Thursday, November 2, 2023 10:00 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
>> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix, Tom
>> <trix@redhat.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script parameters
>>
>> Hi Nicolas,
>>
>> On 10/27/23 22:02, Chautru, Nicolas wrote:
>>> Hi Maxime,
>>>
>>> Back on this discussion and there was some misunderstanding. This is really
>> a bug for the python script helper only.
>>> The -t option is already reserved for bbdev-test application (and in doc) for
>> setting the iter_max (see main.c).
>>> The problem is that the python script introduced -t by mistake for an
>> additional time out when calling the binary to be handled in the script only,
>> now resolving this by using -T to avoid clash with existing -t option.
>>> No one is genuinely using -t for timeout.
>>> Ping me if still unclear
>>
>> I actually understood it from the beginning, -t was used as timeout option in
>> the test-bbdev.py script, this patch changes -t to now represent the maximum
>> number of iterations.
>>
>> I'm sure you can see the problem if someone was using -t for timeout in some
>> CI (Neither me or you can guarantee this is not used)?
>>
>> If you think we really should change the meaning of this option, we should
>> have a deprecation notice, have both -t and -T to represent timeout during
>> the deprecation period and emit a warning when -t is used. Once deprecated,
>> you can assign -t to max iterations.
>>
>> Sounds good?
>
> The problem with your option is that during that period of time the script is still broken since you cannot set the number of iterations which is what -t is meant for in the binary application.
> This is really a fix, and until that fix is applied the python script should not be used, instead the binary should be called directly.
> If you want to do it your suggested way and then do the formal fix in 24.03 that is possible even if not ideal to me, but it would still mean the script should arguably not be used during that period of time from my point of view.
You will be able to use --iter-max during the transition period, I think
it is a good compromise.
Ok for you?
Thanks,
Maxime
> Let me know
>
>
>>
>> Regards,
>> Maxime
>>
>>> Nic
>>>
>>>> -----Original Message-----
>>>> From: Chautru, Nicolas
>>>> Sent: Thursday, October 19, 2023 8:10 AM
>>>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Vargas, Hernan
>>>> <Hernan.Vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix,
>> Tom
>>>> <trix@redhat.com>; Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
>>>> Subject: RE: [PATCH v1 02/11] test/bbdev: update python script
>>>> parameters
>>>>
>>>> Hi Maxime,
>>>> In practice anyone using that API is already using the one defined in
>>>> the patch below and not using -t for time out. So not a concern to do
>>>> it properly through that patch.
>>>> Heman, any concern on your side with this change?
>>>>
>>>>> -----Original Message-----
>>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>> Sent: Thursday, October 19, 2023 11:19 AM
>>>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Vargas, Hernan
>>>>> <hernan.vargas@intel.com>; dev@dpdk.org; gakhil@marvell.com; Rix,
>>>> Tom
>>>>> <trix@redhat.com>
>>>>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>
>>>>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
>>>>> parameters
>>>>>
>>>>>
>>>>>
>>>>> On 10/19/23 11:01, Chautru, Nicolas wrote:
>>>>>> Hi Maxime,
>>>>>>
>>>>>> I believe there was some historical discrepancy, even in doc both
>>>>>> appeared
>>>>> but none of the 2 -t options with the cap.
>>>>>> https://doc.dpdk.org/guides/tools/testbbdev.html
>>>>>> Resolving this historical issue here.
>>>>>
>>>>> Ok, then we should fix the doc, not the code.
>>>>>
>>>>> Thanks,
>>>>> Maxime
>>>>>
>>>>>> Thanks
>>>>>> Nic
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>>> Sent: Tuesday, October 17, 2023 9:08 PM
>>>>>>> To: Vargas, Hernan <hernan.vargas@intel.com>; dev@dpdk.org;
>>>>>>> gakhil@marvell.com; Rix, Tom <trix@redhat.com>
>>>>>>> Cc: Chautru, Nicolas <nicolas.chautru@intel.com>; Zhang, Qi Z
>>>>>>> <qi.z.zhang@intel.com>
>>>>>>> Subject: Re: [PATCH v1 02/11] test/bbdev: update python script
>>>>>>> parameters
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 9/29/23 20:13, Hernan Vargas wrote:
>>>>>>>> Update the timeout argument and default values.
>>>>>>>> Update EAL help message and default value.
>>>>>>>> Add iter_max and snr arguments.
>>>>>>>>
>>>>>>>> Signed-off-by: Hernan Vargas <hernan.vargas@intel.com>
>>>>>>>> ---
>>>>>>>> app/test-bbdev/test-bbdev.py | 22 ++++++++++++++++++----
>>>>>>>> app/test-bbdev/test_bbdev_perf.c | 2 +-
>>>>>>>> 2 files changed, 19 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/app/test-bbdev/test-bbdev.py
>>>>>>>> b/app/test-bbdev/test-bbdev.py index
>> 9cdb4659724d..8d0145076e4d
>>>>>>>> 100755
>>>>>>>> --- a/app/test-bbdev/test-bbdev.py
>>>>>>>> +++ b/app/test-bbdev/test-bbdev.py
>>>>>>>> @@ -25,12 +25,12 @@ def kill(process):
>>>>>>>> help="specifies path to the bbdev test app",
>>>>>>>> default=dpdk_path + "/" + dpdk_target +
>>>>>>>> "/app/dpdk-test-
>>>>> bbdev")
>>>>>>>> parser.add_argument("-e", "--eal-params",
>>>>>>>> - help="EAL arguments which are passed to the test app",
>>>>>>>> - default="--vdev=baseband_null0")
>>>>>>>> -parser.add_argument("-t", "--timeout",
>>>>>>>> + help="EAL arguments which must be passed to
>>>>>>>> + the test
>>>> app",
>>>>>>>> + default="--vdev=baseband_null0 -a00:00.0")
>>>>>>>> +parser.add_argument("-T", "--timeout",
>>>>>>>> type=int,
>>>>>>>> help="Timeout in seconds",
>>>>>>>> - default=300)
>>>>>>>> + default=600)
>>>>>>>> parser.add_argument("-c", "--test-cases",
>>>>>>>> nargs="+",
>>>>>>>> help="Defines test cases to run. Run all
>>>>>>>> if not
>>>>>>>> specified") @@ -48,6 +48,14 @@ def kill(process):
>>>>>>>> type=int,
>>>>>>>> help="Operations enqueue/dequeue burst size.",
>>>>>>>> default=[32])
>>>>>>>> +parser.add_argument("-s", "--snr",
>>>>>>>> + type=int,
>>>>>>>> + help="SNR in dB for BLER tests",
>>>>>>>> + default=0)
>>>>>>>> +parser.add_argument("-t", "--iter-max",
>>>>>>>
>>>>>>> We shouldn't change parameters meaning, it will silently break
>>>>>>> existing scripts making use of it.
>>>>>>>
>>>>>>>> + type=int,
>>>>>>>> + help="Max iterations",
>>>>>>>> + default=6)
>>>>>>>> parser.add_argument("-l", "--num-lcores",
>>>>>>>> type=int,
>>>>>>>> help="Number of lcores to run.", @@ -68,6
>>>>>>>> +76,12 @@ def kill(process):
>>>>>>>>
>>>>>>>> params.extend(["--"])
>>>>>>>>
>>>>>>>> +if args.snr:
>>>>>>>> + params.extend(["-s", str(args.snr)])
>>>>>>>> +
>>>>>>>> +if args.iter_max:
>>>>>>>> + params.extend(["-t", str(args.iter_max)])
>>>>>>>> +
>>>>>>>> if args.num_ops:
>>>>>>>> params.extend(["-n", str(args.num_ops)])
>>>>>>>>
>>>>>>>> diff --git a/app/test-bbdev/test_bbdev_perf.c
>>>>>>>> b/app/test-bbdev/test_bbdev_perf.c
>>>>>>>> index 276bbf0a2e6d..faea26c10eed 100644
>>>>>>>> --- a/app/test-bbdev/test_bbdev_perf.c
>>>>>>>> +++ b/app/test-bbdev/test_bbdev_perf.c
>>>>>>>> @@ -26,7 +26,7 @@
>>>>>>>>
>>>>>>>> #define MAX_QUEUES RTE_MAX_LCORE
>>>>>>>> #define TEST_REPETITIONS 100
>>>>>>>> -#define TIME_OUT_POLL 1e8
>>>>>>>> +#define TIME_OUT_POLL 1e9
>>>>>>>> #define WAIT_OFFLOAD_US 1000
>>>>>>>>
>>>>>>>> #ifdef RTE_BASEBAND_FPGA_LTE_FEC
>>>>>>
>>>
>
@@ -25,12 +25,12 @@ def kill(process):
help="specifies path to the bbdev test app",
default=dpdk_path + "/" + dpdk_target + "/app/dpdk-test-bbdev")
parser.add_argument("-e", "--eal-params",
- help="EAL arguments which are passed to the test app",
- default="--vdev=baseband_null0")
-parser.add_argument("-t", "--timeout",
+ help="EAL arguments which must be passed to the test app",
+ default="--vdev=baseband_null0 -a00:00.0")
+parser.add_argument("-T", "--timeout",
type=int,
help="Timeout in seconds",
- default=300)
+ default=600)
parser.add_argument("-c", "--test-cases",
nargs="+",
help="Defines test cases to run. Run all if not specified")
@@ -48,6 +48,14 @@ def kill(process):
type=int,
help="Operations enqueue/dequeue burst size.",
default=[32])
+parser.add_argument("-s", "--snr",
+ type=int,
+ help="SNR in dB for BLER tests",
+ default=0)
+parser.add_argument("-t", "--iter-max",
+ type=int,
+ help="Max iterations",
+ default=6)
parser.add_argument("-l", "--num-lcores",
type=int,
help="Number of lcores to run.",
@@ -68,6 +76,12 @@ def kill(process):
params.extend(["--"])
+if args.snr:
+ params.extend(["-s", str(args.snr)])
+
+if args.iter_max:
+ params.extend(["-t", str(args.iter_max)])
+
if args.num_ops:
params.extend(["-n", str(args.num_ops)])
@@ -26,7 +26,7 @@
#define MAX_QUEUES RTE_MAX_LCORE
#define TEST_REPETITIONS 100
-#define TIME_OUT_POLL 1e8
+#define TIME_OUT_POLL 1e9
#define WAIT_OFFLOAD_US 1000
#ifdef RTE_BASEBAND_FPGA_LTE_FEC