Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coque...@redhat.com> > Sent: Thursday, November 2, 2023 10:00 AM > To: Chautru, Nicolas <nicolas.chau...@intel.com>; Vargas, Hernan > <hernan.var...@intel.com>; dev@dpdk.org; gak...@marvell.com; Rix, Tom > <t...@redhat.com>; Hemant Agrawal <hemant.agra...@nxp.com> > Cc: Zhang, Qi Z <qi.z.zh...@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.coque...@redhat.com>; Vargas, Hernan > >> <hernan.var...@intel.com>; dev@dpdk.org; gak...@marvell.com; Rix, > Tom > >> <t...@redhat.com>; Hemant Agrawal <hemant.agra...@nxp.com> > >> Cc: Zhang, Qi Z <qi.z.zh...@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.coque...@redhat.com> > >>> Sent: Thursday, October 19, 2023 11:19 AM > >>> To: Chautru, Nicolas <nicolas.chau...@intel.com>; Vargas, Hernan > >>> <hernan.var...@intel.com>; dev@dpdk.org; gak...@marvell.com; Rix, > >> Tom > >>> <t...@redhat.com> > >>> Cc: Zhang, Qi Z <qi.z.zh...@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.coque...@redhat.com> > >>>>> Sent: Tuesday, October 17, 2023 9:08 PM > >>>>> To: Vargas, Hernan <hernan.var...@intel.com>; dev@dpdk.org; > >>>>> gak...@marvell.com; Rix, Tom <t...@redhat.com> > >>>>> Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; Zhang, Qi Z > >>>>> <qi.z.zh...@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.var...@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 > >>>> > >
