[dpdk-dev,3/6] service cores: EAL init changes

Message ID 1498208779-166205-3-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Changes Requested, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry June 23, 2017, 9:06 a.m. UTC
  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>
---
 lib/librte_eal/linuxapp/eal/eal.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
  

Comments

Jerin Jacob June 26, 2017, 12:55 p.m. UTC | #1
-----Original Message-----
> Date: Fri, 23 Jun 2017 10:06:16 +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 3/6] 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>
> ---
>  lib/librte_eal/linuxapp/eal/eal.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c

Need to add bsdapp implementation for the same.

> index 7c78f2d..4d6ad0e 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -78,6 +78,7 @@
>  #include <rte_version.h>
>  #include <rte_atomic.h>
>  #include <malloc_heap.h>
> +#include <rte_service_private.h>
>  
>  #include "eal_private.h"
>  #include "eal_thread.h"
> @@ -939,6 +940,20 @@ rte_eal_init(int argc, char **argv)
>  		return -1;
>  	}
>  
> +	/* initialize service core threads and default service-core mapping */
> +	struct rte_config *config = rte_eal_get_configuration();
> +	uint32_t service_cores[RTE_MAX_LCORE];
> +	int count = rte_service_core_list(service_cores, RTE_MAX_LCORE);
> +	for (i = 0; i < count; i++) {
> +		config->lcore_role[service_cores[i]] = ROLE_SERVICE;

Can we move this change inside rte_service_core_start() itself?
otherwise every application needs to the same. Right?

> +		rte_service_core_start(service_cores[i]);
> +	}
> +	ret = rte_service_init_default_mapping();
> +	if (ret) {
> +		rte_errno = ENOEXEC;
> +		return -1;
> +	}
> +
>  	rte_eal_mcfg_complete();
>  
>  	return fctret;

With above changes:
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> -- 
> 2.7.4
>
  
Van Haaren, Harry June 29, 2017, 11:13 a.m. UTC | #2
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]

<snip>

> > 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>
> > ---
> >  lib/librte_eal/linuxapp/eal/eal.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> 
> Need to add bsdapp implementation for the same.

Done.

> > +	/* initialize service core threads and default service-core mapping */
> > +	struct rte_config *config = rte_eal_get_configuration();
> > +	uint32_t service_cores[RTE_MAX_LCORE];
> > +	int count = rte_service_core_list(service_cores, RTE_MAX_LCORE);
> > +	for (i = 0; i < count; i++) {
> > +		config->lcore_role[service_cores[i]] = ROLE_SERVICE;
> 
> Can we move this change inside rte_service_core_start() itself?
> otherwise every application needs to the same. Right?

Done, nice, this removes the requirement to access rte_config, which I didn't like having there.
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 7c78f2d..4d6ad0e 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -78,6 +78,7 @@ 
 #include <rte_version.h>
 #include <rte_atomic.h>
 #include <malloc_heap.h>
+#include <rte_service_private.h>
 
 #include "eal_private.h"
 #include "eal_thread.h"
@@ -939,6 +940,20 @@  rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	/* initialize service core threads and default service-core mapping */
+	struct rte_config *config = rte_eal_get_configuration();
+	uint32_t service_cores[RTE_MAX_LCORE];
+	int count = rte_service_core_list(service_cores, RTE_MAX_LCORE);
+	for (i = 0; i < count; i++) {
+		config->lcore_role[service_cores[i]] = ROLE_SERVICE;
+		rte_service_core_start(service_cores[i]);
+	}
+	ret = rte_service_init_default_mapping();
+	if (ret) {
+		rte_errno = ENOEXEC;
+		return -1;
+	}
+
 	rte_eal_mcfg_complete();
 
 	return fctret;