Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coque...@redhat.com> > Sent: Thursday, October 19, 2023 10:47 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 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.coque...@redhat.com> > >> Sent: Tuesday, October 17, 2023 9:43 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 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.var...@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.coque...@redhat.com> > >> > >> Thanks, > >> Maxime > >
