[dpdk-dev,v4,2/7] service cores: EAL init changes
Checks
Commit Message
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(+)
Comments
-----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>
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Tuesday, July 11, 2017 8:42 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; Wiles, Keith <keith.wiles@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>
> Subject: Re: [PATCH v4 2/7] service cores: EAL init changes
<snip>
> > + /* 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.
Good idea - done. Does indeed make things cleaner - thanks!
> With that change:
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Added to patch! Cheers, -Harry
@@ -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;
+ }
+
rte_eal_mcfg_complete();
return fctret;
@@ -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"
@@ -932,6 +933,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");
@@ -939,6 +951,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;
+ }
+
rte_eal_mcfg_complete();
return fctret;