[dpdk-dev,1/2] service: fix del to reset lcore role to rte

  This patch fixes the reset of the service core,
that when rte_service_lcore_del() is called, the
lcore_role is restored to RTE.

This issue was reported as when running the unit tests, an
error was thrown that "failed to allocate lcore". Investigating
revealed that the state of the service-cores after del() was
not allowing a core to be re-used at a later point in time.

Fixes: 21698354c832 ("service: introduce service cores concept")
+CC stable@dpdk.org

Reported-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

@Stable maintainers; this is an EXPERIMENTAL tagged API, so I'm
not sure what the expectation is in terms of backporting.
---
 lib/librte_eal/common/rte_service.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)
  

Hi Harry,

Comments inline.

On Wed, Dec 20, 2017 at 11:21:46AM +0000, Harry van Haaren wrote:
> This patch fixes the reset of the service core,
> that when rte_service_lcore_del() is called, the
> lcore_role is restored to RTE.
>
> This issue was reported as when running the unit tests, an
> error was thrown that "failed to allocate lcore". Investigating
> revealed that the state of the service-cores after del() was
> not allowing a core to be re-used at a later point in time.
>
> Fixes: 21698354c832 ("service: introduce service cores concept")
> +CC stable@dpdk.org
>
> Reported-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> @Stable maintainers; this is an EXPERIMENTAL tagged API, so I'm
> not sure what the expectation is in terms of backporting.
> ---
>  lib/librte_eal/common/rte_service.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
<snip>
>  int32_t rte_service_lcore_reset_all(void)
>  {
>  	/* loop over cores, reset all to mask 0 */
>  	uint32_t i;
>  	for (i = 0; i < RTE_MAX_LCORE; i++) {
>  		lcore_states[i].service_mask = 0;
> -		lcore_states[i].is_service_core = 0;
> +		set_lcore_state(i, ROLE_RTE);

Setting ROLE_RTE for RTE_MAX_LCORE lcores is incorrect. There should be a check
to set only service lcores something like this:

        for (i = 0; i < RTE_MAX_LCORE; i++) {
	-               lcore_states[i].service_mask = 0;
	-               set_lcore_state(i, ROLE_RTE);
	-               lcore_states[i].runstate = RUNSTATE_STOPPED;
	+               if (lcore_states[i].is_service_core) {
	+                       lcore_states[i].service_mask = 0;
	+                       set_lcore_state(i, ROLE_RTE);
	+                       lcore_states[i].runstate = RUNSTATE_STOPPED;
	+               }

Cheers,
Pavan.

>  		lcore_states[i].runstate = RUNSTATE_STOPPED;
>  	}
>  	for (i = 0; i < RTE_SERVICE_NUM_MAX; i++)
> @@ -600,20 +614,6 @@ int32_t rte_service_lcore_reset_all(void)
>  	return 0;
>  }
>
>  int32_t
>  rte_service_lcore_add(uint32_t lcore)
>  {
> --
> 2.7.4
>