On 11/2/23 19:18, Chautru, Nicolas wrote:
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 reallya bug for the python script helper only.The -t option is already reserved for bbdev-test application (and in doc) forsetting the iter_max (see main.c).The problem is that the python script introduced -t by mistake for anadditional 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 unclearI 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 knowRegards, MaximeNic-----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 appearedbut 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, MaximeThanks 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 index9cdb4659724d..8d0145076e4d100755 --- 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 testapp",+ 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
