Re: [dpdk-dev] [PATCH v4 2/7] service cores: EAL init changes

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Fri, 7 Jul 2017 17:41:02 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: thomas@monjalon.net, jerin.jacob@caviumnetworks.com,
>  keith.wiles@intel.com, bruce.richardson@intel.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v4 2/7] service cores: EAL init changes
> X-Mailer: git-send-email 2.7.4
> 
> This commit shows the changes required in rte_eal_init()
> to transparently launch the service threads. The threads
> are launched into the service worker functions here because
> after rte_eal_init() the application is not gauranteed to
> call any other DPDK API.
> 
> As the registration of services happens at initialization
> time, the services that require CPU time are already available
> when we reach the end of rte_eal_init().
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> v4:
> - Added #include for service cores in BSD eal.c
> 
> v2 comments:
> - Include BSD implementation (Jerin)
> - Move details of core-tracking into rte_service_lcore_add(Jerin)
> - Given there are changes other to suggested, not using Ack
> ---
>  lib/librte_eal/bsdapp/eal/eal.c   | 23 +++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal.c | 23 +++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 05f0c1f..09e3301 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -72,6 +72,7 @@
>  #include <rte_common.h>
>  #include <rte_version.h>
>  #include <rte_atomic.h>
> +#include <rte_service_private.h>
>  #include <malloc_heap.h>
>  
>  #include "eal_private.h"
> @@ -653,6 +654,17 @@ rte_eal_init(int argc, char **argv)
>  	rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MASTER);
>  	rte_eal_mp_wait_lcore();
>  
> +	/* initialize services first so vdevs can register during bus_probe.
> +	 * Ignore return value of already initialized, this means EAL parameter
> +	 * -s was used to set a service-core mask.
> +	 */
> +	ret = rte_service_init();
> +	if (ret) {
> +		rte_eal_init_alert("rte_service_init() failed\n");
> +		rte_errno = ENOEXEC;
> +		return -1;
> +	}
> +
>  	/* Probe all the buses and devices/drivers on them */
>  	if (rte_bus_probe()) {
>  		rte_eal_init_alert("Cannot probe devices\n");
> @@ -660,6 +672,17 @@ rte_eal_init(int argc, char **argv)
>  		return -1;
>  	}
>  
> +	/* initialize default services configuration */
> +	uint32_t service_cores[RTE_MAX_LCORE];
> +	int count = rte_service_lcore_list(service_cores, RTE_MAX_LCORE);
> +	for (i = 0; i < count; i++)
> +		rte_service_lcore_start(service_cores[i]);
> +	ret = rte_service_set_default_mapping();
> +	if (ret) {
> +		rte_errno = ENOEXEC;
> +		return -1;
> +	}

How about moving, rte_service_lcore_start() inside
rte_service_set_default_mapping() so that rte_eal_init() level change will be
less in linuxapp and bsdapp?(and both changes are tightly coupled too).

You could change the function name to rte_service_enable_default_mapping()
or something like that to include rte_service_lcore_start() start change.

With that change:
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>