Re: [dpdk-dev] Random failure in service_autotest

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, July 21, 2020 6:40 AM
> To: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>; Van Haaren,
> Harry <harry.van.haaren@intel.com>; Phil Yang <Phil.Yang@arm.com>; Aaron
> Conole <aconole@redhat.com>
> Cc: David Marchand <david.marchand@redhat.com>; Igor Romanov
> <igor.romanov@oktetlabs.ru>; dev <dev@dpdk.org>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@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>