[v7,01/13] eal: add option register infrastructure
Checks
Commit Message
This commit adds infrastructure to EAL that allows an application to
register it's init function with EAL. This allows libraries to be
initialized at the end of EAL init.
This infrastructure allows libraries that depend on EAL to be initialized
as part of EAL init, removing circular dependency issues.
Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
---
lib/librte_eal/bsdapp/eal/Makefile | 1 +
lib/librte_eal/bsdapp/eal/eal.c | 14 ++++++-
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/eal_private.h | 21 ++++++++++
lib/librte_eal/common/include/rte_option.h | 63 ++++++++++++++++++++++++++++++
lib/librte_eal/common/meson.build | 2 +
lib/librte_eal/common/rte_option.c | 54 +++++++++++++++++++++++++
lib/librte_eal/linuxapp/eal/Makefile | 1 +
lib/librte_eal/linuxapp/eal/eal.c | 14 ++++++-
lib/librte_eal/rte_eal_version.map | 1 +
10 files changed, 170 insertions(+), 2 deletions(-)
create mode 100644 lib/librte_eal/common/include/rte_option.h
create mode 100644 lib/librte_eal/common/rte_option.c
Comments
Hi Kevin,
On Wed, Oct 24, 2018 at 02:27:13PM +0100, Kevin Laatz wrote:
> This commit adds infrastructure to EAL that allows an application to
> register it's init function with EAL. This allows libraries to be
> initialized at the end of EAL init.
>
> This infrastructure allows libraries that depend on EAL to be initialized
> as part of EAL init, removing circular dependency issues.
>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
I think this is good enough,
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
The only remaining issue is rte_option_init().
Sorry I missed your previous message and did not respond in time, I
would have opted for leaving a return value to at least be able to stop
the init on error. It is possible to force the callback type to return
an error value along with a string / hint describing the error. It
should not be hard to add it later, so not blocking IMO.
24/10/2018 16:01, Gaëtan Rivet:
> Hi Kevin,
>
> On Wed, Oct 24, 2018 at 02:27:13PM +0100, Kevin Laatz wrote:
> > This commit adds infrastructure to EAL that allows an application to
> > register it's init function with EAL. This allows libraries to be
> > initialized at the end of EAL init.
> >
> > This infrastructure allows libraries that depend on EAL to be initialized
> > as part of EAL init, removing circular dependency issues.
> >
> > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> > Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> I think this is good enough,
>
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Yes it looks good enough.
And it compiles fine in my test.
> The only remaining issue is rte_option_init().
> Sorry I missed your previous message and did not respond in time, I
> would have opted for leaving a return value to at least be able to stop
> the init on error. It is possible to force the callback type to return
> an error value along with a string / hint describing the error. It
> should not be hard to add it later, so not blocking IMO.
I think you need to set this API as experimental.
On 24/10/2018 15:33, Thomas Monjalon wrote:
> 24/10/2018 16:01, Gaëtan Rivet:
>> Hi Kevin,
>>
>> On Wed, Oct 24, 2018 at 02:27:13PM +0100, Kevin Laatz wrote:
>>> This commit adds infrastructure to EAL that allows an application to
>>> register it's init function with EAL. This allows libraries to be
>>> initialized at the end of EAL init.
>>>
>>> This infrastructure allows libraries that depend on EAL to be initialized
>>> as part of EAL init, removing circular dependency issues.
>>>
>>> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
>>> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
>> I think this is good enough,
>>
>> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> Yes it looks good enough.
> And it compiles fine in my test.
>
>> The only remaining issue is rte_option_init().
>> Sorry I missed your previous message and did not respond in time, I
>> would have opted for leaving a return value to at least be able to stop
>> the init on error. It is possible to force the callback type to return
>> an error value along with a string / hint describing the error. It
>> should not be hard to add it later, so not blocking IMO.
Agree this can be added in the future.
> I think you need to set this API as experimental.
Will do, thanks.
On 24/10/2018 15:52, Laatz, Kevin wrote:
> On 24/10/2018 15:33, Thomas Monjalon wrote:
>> 24/10/2018 16:01, Gaëtan Rivet:
>>> Hi Kevin,
>>>
>>> On Wed, Oct 24, 2018 at 02:27:13PM +0100, Kevin Laatz wrote:
>>>> This commit adds infrastructure to EAL that allows an application to
>>>> register it's init function with EAL. This allows libraries to be
>>>> initialized at the end of EAL init.
>>>>
>>>> This infrastructure allows libraries that depend on EAL to be
>>>> initialized
>>>> as part of EAL init, removing circular dependency issues.
>>>>
>>>> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
>>>> Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
>>> I think this is good enough,
>>>
>>> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>> Yes it looks good enough.
>> And it compiles fine in my test.
>>
>>> The only remaining issue is rte_option_init().
>>> Sorry I missed your previous message and did not respond in time, I
>>> would have opted for leaving a return value to at least be able to stop
>>> the init on error. It is possible to force the callback type to return
>>> an error value along with a string / hint describing the error. It
>>> should not be hard to add it later, so not blocking IMO.
> Agree this can be added in the future.
>> I think you need to set this API as experimental.
> Will do, thanks.
Correction: This is an internal function (eal_private.h), so no need to
mark as experimental. rte_option_register() is marked as experimental.
@@ -67,6 +67,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += malloc_elem.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += malloc_heap.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += malloc_mp.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_keepalive.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_option.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_service.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_reciprocal.c
@@ -42,6 +42,7 @@
#include <rte_devargs.h>
#include <rte_version.h>
#include <rte_vfio.h>
+#include <rte_option.h>
#include <rte_atomic.h>
#include <malloc_heap.h>
@@ -414,12 +415,20 @@ eal_parse_args(int argc, char **argv)
argvopt = argv;
optind = 1;
optreset = 1;
+ opterr = 0;
while ((opt = getopt_long(argc, argvopt, eal_short_options,
eal_long_options, &option_index)) != EOF) {
- /* getopt is not happy, stop right now */
+ /*
+ * getopt didn't recognise the option, lets parse the
+ * registered options to see if the flag is valid
+ */
if (opt == '?') {
+ ret = rte_option_parse(argv[optind-1]);
+ if (ret == 0)
+ continue;
+
eal_usage(prgname);
ret = -1;
goto out;
@@ -791,6 +800,9 @@ rte_eal_init(int argc, char **argv)
rte_eal_mcfg_complete();
+ /* Call each registered callback, if enabled */
+ rte_option_init();
+
return fctret;
}
@@ -12,6 +12,7 @@ INC += rte_tailq.h rte_interrupts.h rte_alarm.h
INC += rte_string_fns.h rte_version.h
INC += rte_eal_memconfig.h rte_malloc_heap.h
INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_class.h
+INC += rte_option.h
INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
INC += rte_malloc.h rte_keepalive.h rte_time.h
INC += rte_service.h rte_service_component.h
@@ -349,4 +349,25 @@ dev_sigbus_handler_register(void);
int
dev_sigbus_handler_unregister(void);
+/**
+ * Check if the option is registered.
+ *
+ * @param option
+ * The option to be parsed.
+ *
+ * @return
+ * 0 on success
+ * @return
+ * -1 on fail
+ */
+int
+rte_option_parse(const char *opt);
+
+/**
+ * Iterate through the registered options and execute the associated
+ * callback if enabled.
+ */
+void
+rte_option_init(void);
+
#endif /* _EAL_PRIVATE_H_ */
new file mode 100644
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation.
+ */
+
+#ifndef __INCLUDE_RTE_OPTION_H__
+#define __INCLUDE_RTE_OPTION_H__
+
+/**
+ * @file
+ *
+ * This API offers the ability to register options to the EAL command line and
+ * map those options to functions that will be executed at the end of EAL
+ * initialization. These options will be available as part of the EAL command
+ * line of applications and are dynamically managed.
+ *
+ * This is used primarily by DPDK libraries offering command line options.
+ * Currently, this API is limited to registering options without argument.
+ *
+ * The register API can be used to resolve circular dependency issues
+ * between EAL and the library. The library uses EAL, but is also initialized
+ * by EAL. Hence, EAL depends on the init function of the library. The API
+ * introduced in rte_option allows us to register the library init with EAL
+ * (passing a function pointer) and avoid the circular dependency.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef int (*rte_option_cb)(void);
+
+/*
+ * Structure describing the EAL command line option being registered.
+ */
+struct rte_option {
+ TAILQ_ENTRY(rte_option) next; /**< Next entry in the list. */
+ char *opt_str; /**< The option name. */
+ rte_option_cb cb; /**< Function called when option is used. */
+ int enabled; /**< Set when the option is used. */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Register an option to the EAL command line.
+ * When recognized, the associated function will be executed at the end of EAL
+ * initialization.
+ *
+ * The associated structure must be available the whole time this option is
+ * registered (i.e. not stack memory).
+ *
+ * @param opt
+ * Structure describing the option to parse.
+ */
+void __rte_experimental
+rte_option_register(struct rte_option *opt);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
@@ -34,6 +34,7 @@ common_sources = files(
'malloc_mp.c',
'rte_keepalive.c',
'rte_malloc.c',
+ 'rte_option.c',
'rte_reciprocal.c',
'rte_service.c'
)
@@ -71,6 +72,7 @@ common_headers = files(
'include/rte_malloc_heap.h',
'include/rte_memory.h',
'include/rte_memzone.h',
+ 'include/rte_option.h',
'include/rte_pci_dev_feature_defs.h',
'include/rte_pci_dev_features.h',
'include/rte_per_lcore.h',
new file mode 100644
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation.
+ */
+
+#include <unistd.h>
+#include <string.h>
+
+#include <rte_eal.h>
+#include <rte_option.h>
+
+#include "eal_private.h"
+
+TAILQ_HEAD(rte_option_list, rte_option);
+
+struct rte_option_list rte_option_list =
+ TAILQ_HEAD_INITIALIZER(rte_option_list);
+
+static struct rte_option *option;
+
+int
+rte_option_parse(const char *opt)
+{
+ /* Check if the option is registered */
+ TAILQ_FOREACH(option, &rte_option_list, next) {
+ if (strcmp(opt, option->opt_str) == 0) {
+ option->enabled = 1;
+ return 0;
+ }
+ }
+
+ return -1;
+}
+
+void __rte_experimental
+rte_option_register(struct rte_option *opt)
+{
+ TAILQ_FOREACH(option, &rte_option_list, next) {
+ if (strcmp(opt->opt_str, option->opt_str) == 0)
+ RTE_LOG(INFO, EAL, "Option %s has already been registered.",
+ opt->opt_str);
+ return;
+ }
+
+ TAILQ_INSERT_HEAD(&rte_option_list, opt, next);
+}
+
+void
+rte_option_init(void)
+{
+ TAILQ_FOREACH(option, &rte_option_list, next) {
+ if (option->enabled)
+ option->cb();
+ }
+}
@@ -75,6 +75,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += malloc_elem.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += malloc_heap.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += malloc_mp.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_keepalive.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_option.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_service.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_reciprocal.c
@@ -48,6 +48,7 @@
#include <rte_atomic.h>
#include <malloc_heap.h>
#include <rte_vfio.h>
+#include <rte_option.h>
#include "eal_private.h"
#include "eal_thread.h"
@@ -600,12 +601,20 @@ eal_parse_args(int argc, char **argv)
argvopt = argv;
optind = 1;
+ opterr = 0;
while ((opt = getopt_long(argc, argvopt, eal_short_options,
eal_long_options, &option_index)) != EOF) {
- /* getopt is not happy, stop right now */
+ /*
+ * getopt didn't recognise the option, lets parse the
+ * registered options to see if the flag is valid
+ */
if (opt == '?') {
+ ret = rte_option_parse(argv[optind-1]);
+ if (ret == 0)
+ continue;
+
eal_usage(prgname);
ret = -1;
goto out;
@@ -1080,6 +1089,9 @@ rte_eal_init(int argc, char **argv)
rte_eal_mcfg_complete();
+ /* Call each registered callback, if enabled */
+ rte_option_init();
+
return fctret;
}
@@ -351,6 +351,7 @@ EXPERIMENTAL {
rte_mp_request_sync;
rte_mp_request_async;
rte_mp_sendmsg;
+ rte_option_register;
rte_service_lcore_attr_get;
rte_service_lcore_attr_reset_all;
rte_service_may_be_active;