[PATCH v1 06/11] test/bbdev: assert failed test for queue configure

Chautru, Nicolas nicolas.chautru at intel.com
Thu Oct 19 14:12:05 CEST 2023


Hi Maxime, 

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin at redhat.com>
> Sent: Thursday, October 19, 2023 10:47 AM
> To: Chautru, Nicolas <nicolas.chautru at intel.com>; Vargas, Hernan
> <hernan.vargas at intel.com>; dev at dpdk.org; gakhil at marvell.com; Rix, Tom
> <trix at redhat.com>
> Cc: Zhang, Qi Z <qi.z.zhang at intel.com>
> Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for queue configure
> 
> Hi Nicolas,
> 
> On 10/19/23 10:41, Chautru, Nicolas wrote:
> > Hi Maxime,
> >
> > Do we really want to make these kind of changes on to the stable release, it
> tends to artificially increase the amount of churn on the stable release which
> can be counterproductive for such changes which don't add much value if any
> to user/developper.
> > Happy to follow your suggestion but a general feedback is lack of appetite for
> very large amount of changes in stable patches which inhibit adoption, so
> would expect to put things there that we would genuinely flag as a bug.
> > Kindly share your thoughts.
> 
> Checking for configuration failure in a test application is quite useful in my
> opinion, as it can help catching regressions, isn't it?

I don’t personally think this (or for other commit on that serie) hits that bar for being required in stable release. This ends up being counterproductive having stable release with a huge amount of commits that are not really required, and it ends up being a reason for people not to move to stable release. 
But if you are really convinced, ok to follow your reco. 

> 
> Maxime
> > Thanks
> > Nic
> >
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin at redhat.com>
> >> Sent: Tuesday, October 17, 2023 9:43 PM
> >> To: Vargas, Hernan <hernan.vargas at intel.com>; dev at dpdk.org;
> >> gakhil at marvell.com; Rix, Tom <trix at redhat.com>
> >> Cc: Chautru, Nicolas <nicolas.chautru at intel.com>; Zhang, Qi Z
> >> <qi.z.zhang at intel.com>
> >> Subject: Re: [PATCH v1 06/11] test/bbdev: assert failed test for
> >> queue configure
> >>
> >>
> >>
> >> On 9/29/23 20:13, Hernan Vargas wrote:
> >>> Stop test if rte_bbdev_queue_configure fails to configure queue.
> >>>
> >>> Signed-off-by: Hernan Vargas <hernan.vargas at intel.com>
> >>> ---
> >>>    app/test-bbdev/test_bbdev.c | 3 ++-
> >>>    1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/app/test-bbdev/test_bbdev.c
> >>> b/app/test-bbdev/test_bbdev.c index 65805977aead..cf224dca5d04
> >>> 100644
> >>> --- a/app/test-bbdev/test_bbdev.c
> >>> +++ b/app/test-bbdev/test_bbdev.c
> >>> @@ -366,7 +366,8 @@ test_bbdev_configure_stop_queue(void)
> >>>    	 * - queue should be started if deferred_start ==
> >>>    	 */
> >>>    	ts_params->qconf.deferred_start = 0;
> >>> -	rte_bbdev_queue_configure(dev_id, queue_id, &ts_params->qconf);
> >>> +	TEST_ASSERT_SUCCESS(rte_bbdev_queue_configure(dev_id, queue_id,
> >> &ts_params->qconf),
> >>> +			"Failed test for rte_bbdev_queue_configure");
> >>>    	rte_bbdev_start(dev_id);
> >>>
> >>>    	TEST_ASSERT_SUCCESS(return_value =
> >> rte_bbdev_queue_info_get(dev_id,
> >>
> >> If should be a fix IMO.
> >> With fixes tag added and stable cc'ed:
> >>
> >> Reviewed-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> >>
> >> Thanks,
> >> Maxime
> >



More information about the dev mailing list