> -----Original Message----- > From: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com> > Sent: Tuesday, July 21, 2020 6:40 AM > To: Lukasz Wojciechowski <l.wojciec...@partner.samsung.com>; Van Haaren, > Harry <harry.van.haa...@intel.com>; Phil Yang <phil.y...@arm.com>; Aaron > Conole <acon...@redhat.com> > Cc: David Marchand <david.march...@redhat.com>; Igor Romanov > <igor.roma...@oktetlabs.ru>; dev <dev@dpdk.org>; Yigit, Ferruh > <ferruh.yi...@intel.com>; nd <n...@arm.com>; Honnappa Nagarahalli > <honnappa.nagaraha...@arm.com>; nd <n...@arm.com> > Subject: RE: [dpdk-dev] Random failure in service_autotest > > <snip> <more snip>
Thanks Lukasz & Phil for v2 reviews & Acks. > > >> return 0; > > > Changing the implementation loop counting is one option - but changing > > > the implementation as a workaround for a unit test seems like the wrong > > way around. > > I agree. We should fix the test not the service implementation. Of course it > > would be nice to do so without inserting sleeps as it's a workaround for > > true > > synchronization. > I think the service shutdown sequence in the test case is not correct. We > cannot > call 'rte_service_lcore_stop' without first shutting down the service using > 'rte_service_runstate_set' and 'rte_service_component_runstate_set'. The > 'rte_service_lcore_stop' has checks to validate that the service is not > running > currently (among other things). In fact, the call to 'rte_service_lcore_stop' > API is > returning -EBUSY currently in the test case. We are not checking the return > status. > > If I understand the intent of the test case correctly, the sequence of the > calls > needs to be: > rte_service_runstate_set(id, 0) > rte_service_component_runstate_set(id, 0); > rte_service_may_be_active - loop till the service is not active > rte_service_lcore_stop(slcore_id); No need to change service runstates, unmapping the service lcore to the service allows service_lcore_stop() to work as expected, and not return -EBUSY. This change to add an unmap() is integrated in the test case in the v2 patch. > > > Honnappa's suggestion in the other reply seems to not work here, as the > > "service_may_be_active" > > > won't get updated if the service core is stopped after running its final > > iteration..? > This also is a problem. > This needs to be fixed by setting 'service_active_on_lcore' immediately after > the > service completes running in 'service_run' (rather than waiting for the next > iteration of service_run). This is a different bug - not directly related to the service_autotest issue reported here. I'll keep focus & resolve that issue first as it has higher priorty due to CI failures. <snip previous discsussion>
