[dpdk-dev] [PATCH v3 1/3] eal: add functions parsing EAL arguments
Bruce Richardson
bruce.richardson at intel.com
Fri Jun 30 18:04:45 CEST 2017
On Tue, Jun 27, 2017 at 12:52:38PM +0200, Jacek Piasecki wrote:
> From: Kuba Kozak <kubax.kozak at intel.com>
>
> added function rte_eal_configure which configure
> Environment Abstraction Layer (EAL) using
> configuration structure.
>
> Signed-off-by: Kuba Kozak <kubax.kozak at intel.com>
> Suggested-by: Bruce Richardson <bruce.richardson at intel.com>
> ---
Thanks for looking to implement this idea, to see how it would work.
Comments on the implementation inline below.
Regards,
/Bruce
> This patch depends on cfgfile patchset with:
> "cfgfile: remove EAL dependency"
> "cfgfile: add new functions to API"
> "cfgfile: rework of load function"
> "test/cfgfile: add new unit test"
> ---
> config/common_base | 1 +
> lib/Makefile | 6 +-
> lib/librte_eal/bsdapp/eal/Makefile | 4 +
> lib/librte_eal/bsdapp/eal/eal.c | 249 ++++++++++++++---
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
> lib/librte_eal/common/eal_common_cpuflags.c | 14 +-
> lib/librte_eal/common/eal_common_lcore.c | 11 +-
> lib/librte_eal/common/eal_common_options.c | 5 +
> lib/librte_eal/common/include/rte_eal.h | 21 ++
> lib/librte_eal/linuxapp/eal/Makefile | 3 +
> lib/librte_eal/linuxapp/eal/eal.c | 353 ++++++++++++++++++------
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 +
> mk/rte.app.mk | 2 +-
> 13 files changed, 543 insertions(+), 134 deletions(-)
>
> diff --git a/config/common_base b/config/common_base
> index f6aafd1..c1d0e69 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -569,6 +569,7 @@ CONFIG_RTE_LIBRTE_TIMER_DEBUG=n
> # Compile librte_cfgfile
> #
> CONFIG_RTE_LIBRTE_CFGFILE=y
> +CONFIG_RTE_LIBRTE_CFGFILE_DEBUG=n
This change does not belong in this patchset. In fact, I don't think it
belongs anywhere as there are no debug statements in the cfgfile
library.
>
> #
> # Compile librte_cmdline
> diff --git a/lib/Makefile b/lib/Makefile
> index 07e1fd0..fc5df3a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -32,7 +32,11 @@
> include $(RTE_SDK)/mk/rte.vars.mk
>
> DIRS-y += librte_compat
> +DIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += librte_cfgfile
> DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal
> +ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y)
> +DEPDIRS-librte_eal := librte_cfgfile
> +endif
> DIRS-$(CONFIG_RTE_LIBRTE_RING) += librte_ring
> DEPDIRS-librte_ring := librte_eal
> DIRS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += librte_mempool
> @@ -41,8 +45,6 @@ DIRS-$(CONFIG_RTE_LIBRTE_MBUF) += librte_mbuf
> DEPDIRS-librte_mbuf := librte_eal librte_mempool
> DIRS-$(CONFIG_RTE_LIBRTE_TIMER) += librte_timer
> DEPDIRS-librte_timer := librte_eal
> -DIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += librte_cfgfile
> -DEPDIRS-librte_cfgfile := librte_eal
> DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += librte_cmdline
> DEPDIRS-librte_cmdline := librte_eal
> DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether
I think the parts of this change - the deleteion of the line saying
cfgfile depends on EAL, and the moving of the declaration of the cfgfile
library up in the file, should go in patch 1 of the earlier cfgfile set.
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index a0f9950..d70eefb 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -50,6 +50,10 @@ EXPORT_MAP := rte_eal_version.map
>
> LIBABIVER := 4
>
> +ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y)
> +LDLIBS += -lrte_cfgfile
> +endif
> +
This should not be needed here. Other DPDK libs which depend on yet
other libs don't go modifying the LDLIBS like this.
> # specific to bsdapp exec-env
> SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) := eal.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_memory.c
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 05f0c1f..7baf848 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -73,6 +73,7 @@
> #include <rte_version.h>
> #include <rte_atomic.h>
> #include <malloc_heap.h>
> +#include <rte_cfgfile.h>
>
> #include "eal_private.h"
> #include "eal_thread.h"
> @@ -309,6 +310,18 @@ eal_get_hugepage_mem_size(void)
>
> /* Parse the arguments for --log-level only */
> static void
> +eal_log_level_cfg(struct rte_cfgfile *cfg)
> +{
> + const char *entry;
> +
> + entry = rte_cfgfile_get_entry(cfg, "DPDK", OPT_LOG_LEVEL);
> + if (entry)
> + eal_parse_common_option(OPT_LOG_LEVEL_NUM, entry,
> + &internal_config);
> +}
> +
> +/* Parse the arguments for --log-level only */
> +static void
> eal_log_level_parse(int argc, char **argv)
> {
> int opt;
> @@ -347,6 +360,58 @@ eal_log_level_parse(int argc, char **argv)
> optarg = old_optarg;
> }
>
> +/* Parse single argument */
> +static int
> +eal_parse_option(int opt, char *optarg, int option_index, char *prgname)
> +{
> + int ret;
> +
> + /* getopt is not happy, stop right now */
> + if (opt == '?') {
> + eal_usage(prgname);
> + ret = -1;
> + goto out;
> + }
> +
> + ret = eal_parse_common_option(opt, optarg, &internal_config);
> + /* common parser is not happy */
> + if (ret < 0) {
> + eal_usage(prgname);
> + ret = -1;
> + goto out;
> + }
> + /* common parser handled this option */
> + if (ret == 0)
> + return 0;
> +
> + switch (opt) {
> + case 'h':
> + eal_usage(prgname);
> + exit(EXIT_SUCCESS);
> + break;
> +
> + default:
> + if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
> + RTE_LOG(ERR, EAL, "Option %c is not supported "
> + "on FreeBSD\n", opt);
> + } else if (opt >= OPT_LONG_MIN_NUM &&
> + opt < OPT_LONG_MAX_NUM) {
> + RTE_LOG(ERR, EAL, "Option %s is not supported "
> + "on FreeBSD\n",
> + eal_long_options[option_index].name);
> + } else {
> + RTE_LOG(ERR, EAL, "Option %d is not supported "
> + "on FreeBSD\n", opt);
> + }
> + eal_usage(prgname);
> + ret = -1;
> + goto out;
> + }
> + return 0;
> +out:
> + return ret;
> +}
> +
> /* Parse the argument given in the command line of the application */
> static int
> eal_parse_args(int argc, char **argv)
> @@ -367,45 +432,9 @@ eal_parse_args(int argc, char **argv)
> while ((opt = getopt_long(argc, argvopt, eal_short_options,
> eal_long_options, &option_index)) != EOF) {
>
> - /* getopt is not happy, stop right now */
> - if (opt == '?') {
> - eal_usage(prgname);
> - ret = -1;
> - goto out;
> - }
> -
> - ret = eal_parse_common_option(opt, optarg, &internal_config);
> - /* common parser is not happy */
> - if (ret < 0) {
> - eal_usage(prgname);
> - ret = -1;
> - goto out;
> - }
> - /* common parser handled this option */
> - if (ret == 0)
> - continue;
> -
> - switch (opt) {
> - case 'h':
> - eal_usage(prgname);
> - exit(EXIT_SUCCESS);
> - default:
> - if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
> - RTE_LOG(ERR, EAL, "Option %c is not supported "
> - "on FreeBSD\n", opt);
> - } else if (opt >= OPT_LONG_MIN_NUM &&
> - opt < OPT_LONG_MAX_NUM) {
> - RTE_LOG(ERR, EAL, "Option %s is not supported "
> - "on FreeBSD\n",
> - eal_long_options[option_index].name);
> - } else {
> - RTE_LOG(ERR, EAL, "Option %d is not supported "
> - "on FreeBSD\n", opt);
> - }
> - eal_usage(prgname);
> - ret = -1;
> + ret = eal_parse_option(opt, optarg, option_index, prgname);
> + if (ret < 0)
> goto out;
> - }
> }
>
> if (eal_adjust_config(&internal_config) != 0) {
> @@ -677,3 +706,147 @@ rte_eal_process_type(void)
> {
> return rte_config.process_type;
> }
> +
> +#ifdef RTE_LIBRTE_CFGFILE
> +#define vdev_buff_size 200
> +#define sectionname_size 20
> +static int
> +parse_vdev_devices(struct rte_cfgfile *cfg)
> +{
> + char sectionname[sectionname_size];
> + char buffer1[vdev_buff_size];
> + int vdev_nb = 0;
> + int n_entries;
> + int i;
> +
> + /* ----------- parsing VDEVS */
> + snprintf(sectionname, sectionname_size, "DPDK.vdev%d", vdev_nb);
move this inside the loop to be the first thing done, rather than having
it outside and then the last thing on each iteration.
> +
> + for (vdev_nb = 1; rte_cfgfile_has_section(cfg, sectionname);
> + vdev_nb++) {
> + n_entries = rte_cfgfile_section_num_entries(cfg, sectionname);
> +
> + struct rte_cfgfile_entry entries[n_entries];
> +
> + if (n_entries != rte_cfgfile_section_entries(cfg, sectionname,
> + entries, n_entries)) {
> + rte_eal_init_alert("Unexpected fault.");
> + rte_errno = EFAULT;
> + return -1;
> + }
> +
> + buffer1[0] = 0;
> + for (i = 0; i < n_entries; i++) {
> + if (strlen(entries[i].value)) {
> +
> + if ((strlen(buffer1) +
> + strlen(entries[i].name) +
> + strlen(entries[i].value) + 3)
> + >= vdev_buff_size)
> + goto buff_size_err;
> + strcat(buffer1, entries[i].name);
> + strcat(buffer1, "=");
> + strcat(buffer1, entries[i].value);
> + } else {
> + if ((strlen(buffer1) +
> + strlen(entries[i].name) + 2)
> + >= vdev_buff_size)
> + goto buff_size_err;
> + strcat(buffer1, entries[i].name);
> + }
> +
Doing these strlen calls and calculations that way is horribly
inefficient IMHO, even if this is not in a perf-critical path. Instead,
I would look to start with a buf_len variable which tracks the available
buffer size, and a buf_ptr variable which points to the first available
character. Then do the following each time you want to add something:
ret = snprintf(buf_ptr, buf_len, "%s%s%s", entries[i].name,
entries[i].value[0] != '\0' ? "=" : "",
entries[i].value);
if (ret >= buf_len)
goto buff_size_err;
buf_ptr += ret;
buf_len -= ret;
Shorter and simpler, as well as being more efficient as it does not need
any string length computation calls, using snprintf safely instead.
> + if (i < (n_entries - 1))
> + strcat(buffer1, ",");
You can add this bit into the above snprintf too, in a similar way to
how I optionally added the "=".
> + }
> +
> + /* parsing vdev */
> + if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
> + buffer1) < 0) {
> + return -1;
> + }
> + snprintf(sectionname, sectionname_size, "DPDK.vdev%d", vdev_nb);
> + }
> + /* ----------- parsing VDEVS */
> + return 0;
> +
> +buff_size_err:
> + printf("parse_vdev_devices(): buffer size is to small\n");
> + return -1;
If the code is reworked so that there is only one place where this error
occurs, remove the label and goto, and just put the error handling
inline in the code.
> +}
> +
> +static void
> +eal_getopt(const char *str, int *opt, int *option_index)
> +{
> + int i;
> +
> + *opt = '?';
> + *option_index = 0;
> +
> + if (strlen(str) == 1) {
> + *opt = *str;
> + return;
> + }
> +
> + for (i = 0; eal_long_options[i].name != NULL; i++) {
> + if (strcmp(str, eal_long_options[i].name) == 0) {
> + *opt = eal_long_options[i].val;
> + *option_index = i;
> + break;
> + }
> + }
> +}
> +
> +int
> +rte_eal_configure(struct rte_cfgfile *cfg, char *prgname)
> +{
> + int n_entries;
> + int i;
> + int opt;
> + int option_index;
> +
> + if (cfg == NULL) {
> + rte_errno = -EINVAL;
> + return -1;
> + }
> +
> + n_entries = rte_cfgfile_section_num_entries(cfg, "DPDK");
> +
> + if (n_entries < 1) {
> + printf("No DPDK section entries in cfgfile object\n");
> + return 0;
> + }
> +
> + struct rte_cfgfile_entry entries[n_entries];
> +
> + if (n_entries !=
> + rte_cfgfile_section_entries(cfg, "DPDK", entries,
> + n_entries)) {
> + rte_eal_init_alert("Unexpected fault.");
> + rte_errno = EFAULT;
> + return -1;
> + }
> +
> + eal_reset_internal_config(&internal_config);
> +
> + /* set log level as early as possible */
> + eal_log_level_cfg(cfg);
> +
> + for (i = 0; i < n_entries; i++) {
> + eal_getopt(entries[i].name, &opt, &option_index);
> +
> + if (eal_parse_option(opt, entries[i].value,
> + option_index, prgname) != 0) {
> + rte_eal_init_alert("Invalid config file arguments.");
> + rte_errno = EINVAL;
> + return -1;
> + }
> + }
> +
> + if (parse_vdev_devices(cfg) < 0) {
> + rte_eal_init_alert("Couldn't parse vdevs");
> + rte_errno = ENOMEM;
> + return -1;
> + }
> + return 0;
> +}
> +#endif
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2e48a73..a939b03 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -193,3 +193,7 @@ DPDK_17.05 {
> vfio_get_group_no;
>
> } DPDK_17.02;
> +
> +DPDK_17.08 {
> + rte_eal_configure;
> +} DPDK_17.05;
> diff --git a/lib/librte_eal/common/eal_common_cpuflags.c b/lib/librte_eal/common/eal_common_cpuflags.c
> index 9a2d080..6a365f3 100644
> --- a/lib/librte_eal/common/eal_common_cpuflags.c
> +++ b/lib/librte_eal/common/eal_common_cpuflags.c
> @@ -50,12 +50,18 @@ rte_cpu_check_supported(void)
> int
> rte_cpu_is_supported(void)
> {
> + static int run_once;
> + static int ret;
> /* This is generated at compile-time by the build system */
> static const enum rte_cpu_flag_t compile_time_flags[] = {
> RTE_COMPILE_TIME_CPUFLAGS
> };
> unsigned count = RTE_DIM(compile_time_flags), i;
> - int ret;
> +
> + /* No need to calculate this function again if we know the result */
> + if (run_once)
> + return ret;
> + run_once = 1;
>
> for (i = 0; i < count; i++) {
> ret = rte_cpu_get_flag_enabled(compile_time_flags[i]);
> @@ -64,16 +70,16 @@ rte_cpu_is_supported(void)
> fprintf(stderr,
> "ERROR: CPU feature flag lookup failed with error %d\n",
> ret);
> - return 0;
> + return ret = 0;
> }
> if (!ret) {
> fprintf(stderr,
> "ERROR: This system does not support \"%s\".\n"
> "Please check that RTE_MACHINE is set correctly.\n",
> rte_cpu_get_flag_name(compile_time_flags[i]));
> - return 0;
> + return ret = 0;
> }
> }
>
> - return 1;
> + return ret = 1;
> }
This is not a bad change in and of itself. However, I'm not sure it
belongs in this patchset. With the new configure call, we should still
only be calling these initialization functions once - in eal_init().
> diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
> index 84fa0cb..ce3ef34 100644
> --- a/lib/librte_eal/common/eal_common_lcore.c
> +++ b/lib/librte_eal/common/eal_common_lcore.c
> @@ -53,11 +53,18 @@
> int
> rte_eal_cpu_init(void)
> {
> + static int run_once;
> + static int ret;
> /* pointer to global configuration */
> struct rte_config *config = rte_eal_get_configuration();
> unsigned lcore_id;
> unsigned count = 0;
>
> + /* No need to calculate this function again if we know the result */
> + if (run_once)
> + return ret;
> + run_once = 1;
> +
> /*
> * Parse the maximum set of logical cores, detect the subset of running
> * ones and enable them by default.
> @@ -91,7 +98,7 @@ rte_eal_cpu_init(void)
> "RTE_MAX_NUMA_NODES (%d)\n",
> lcore_config[lcore_id].socket_id,
> RTE_MAX_NUMA_NODES);
> - return -1;
> + return ret = -1;
> #endif
> }
> RTE_LOG(DEBUG, EAL, "Detected lcore %u as "
> @@ -107,5 +114,5 @@ rte_eal_cpu_init(void)
> RTE_MAX_LCORE);
> RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
>
> - return 0;
> + return ret = 0;
> }
As above, should not be needed.
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index f470195..138a27f 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -131,8 +131,13 @@ static int core_parsed;
> void
> eal_reset_internal_config(struct internal_config *internal_cfg)
> {
> + static int run_once;
> int i;
>
> + if (run_once)
> + return;
> + run_once = 1;
> +
> internal_cfg->memory = 0;
> internal_cfg->force_nrank = 0;
> internal_cfg->force_nchannel = 0;
I don't think for this function this is the way to do things, given that
the function does one job - resetting the internal config - and the new
flag just disables it. It's different from e.g. the cpu check which is
able to cache a return value.
Instead, this should be tracked at the higher eal level, and not call
the function at all if it is unneeded. This mean that eal_init should
check itself if configure has already called the function and then skip
calling it itself.
> diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
> index abf020b..e0705de 100644
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -46,6 +46,8 @@
> #include <rte_per_lcore.h>
> #include <rte_config.h>
>
> +struct rte_cfgfile; /* forward declaration of struct */
> +
> #ifdef __cplusplus
> extern "C" {
> #endif
> @@ -188,6 +190,25 @@ int rte_eal_iopl_init(void);
> */
> int rte_eal_init(int argc, char **argv);
>
> +#ifdef RTE_LIBRTE_CFGFILE
> +/**
> + * Initialize the Environment Abstraction Layer (EAL) using
> + * configuration structure
> + *
> + * @param cfg
> + * pointer to config file structure.
> + * If 0 - function free allocated for **cfg_argv memory
Don't understand this comment.
> + * @param prgname
> + * pointer to string with execution path
> + *
> + * @return
> + * - On success, return 0
> + * - On failure, returns -1.
> + */
> +int
> +rte_eal_configure(struct rte_cfgfile *cfg, char *prgname);
> +#endif
> +
> /**
> * Check if a primary process is currently alive
> *
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 640afd0..656033e 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -50,6 +50,9 @@ LDLIBS += -ldl
> LDLIBS += -lpthread
> LDLIBS += -lgcc_s
> LDLIBS += -lrt
> +ifeq ($(CONFIG_RTE_LIBRTE_CFGFILE),y)
> +LDLIBS += -lrte_cfgfile
> +endif
>
> # specific to linuxapp exec-env
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) := eal.c
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 7c78f2d..f5973f4 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -78,6 +78,9 @@
> #include <rte_version.h>
> #include <rte_atomic.h>
> #include <malloc_heap.h>
> +#ifdef RTE_LIBRTE_CFGFILE
> +#include <rte_cfgfile.h>
> +#endif
>
> #include "eal_private.h"
> #include "eal_thread.h"
> @@ -478,6 +481,20 @@ eal_parse_vfio_intr(const char *mode)
> return -1;
> }
>
> +#ifdef RTE_LIBRTE_CFGFILE
> +/* Parse the arguments for --log-level only */
> +static void
> +eal_log_level_cfg(struct rte_cfgfile *cfg)
> +{
> + const char *entry;
> +
> + entry = rte_cfgfile_get_entry(cfg, "DPDK", OPT_LOG_LEVEL);
> + if (entry)
> + eal_parse_common_option(OPT_LOG_LEVEL_NUM, entry,
> + &internal_config);
> +}
> +#endif
> +
This looks the same as the BSD version. Can it go in a common file?
> /* Parse the arguments for --log-level only */
> static void
> eal_log_level_parse(int argc, char **argv)
> @@ -515,119 +532,135 @@ eal_log_level_parse(int argc, char **argv)
> optarg = old_optarg;
> }
>
> -/* Parse the argument given in the command line of the application */
> +/* Parse single argument */
> static int
> -eal_parse_args(int argc, char **argv)
> +eal_parse_option(int opt, char *optarg, int option_index, char *prgname)
> {
> - int opt, ret;
> - char **argvopt;
> - int option_index;
> - char *prgname = argv[0];
> - const int old_optind = optind;
> - const int old_optopt = optopt;
> - char * const old_optarg = optarg;
> + int ret;
>
> - argvopt = argv;
> - optind = 1;
> + /* getopt is not happy, stop right now */
> + if (opt == '?') {
> + eal_usage(prgname);
> + ret = -1;
> + goto out;
> + }
>
> - while ((opt = getopt_long(argc, argvopt, eal_short_options,
> - eal_long_options, &option_index)) != EOF) {
> + ret = eal_parse_common_option(opt, optarg, &internal_config);
> + /* common parser is not happy */
> + if (ret < 0) {
> + eal_usage(prgname);
> + ret = -1;
> + goto out;
> + }
> + /* common parser handled this option */
> + if (ret == 0)
> + return 0;
>
> - /* getopt is not happy, stop right now */
> - if (opt == '?') {
> + switch (opt) {
> + case 'h':
> + eal_usage(prgname);
> + exit(EXIT_SUCCESS);
> + break;
> +
> + /* long options */
> + case OPT_XEN_DOM0_NUM:
> +#ifdef RTE_LIBRTE_XEN_DOM0
> + internal_config.xen_dom0_support = 1;
> + break;
> +#else
> + RTE_LOG(ERR, EAL, "Can't support DPDK app "
> + "running on Dom0, please configure"
> + " RTE_LIBRTE_XEN_DOM0=y\n");
> + ret = -1;
> + goto out;
> +#endif
> +
> + case OPT_HUGE_DIR_NUM:
> + internal_config.hugepage_dir = optarg;
> + break;
> +
> + case OPT_FILE_PREFIX_NUM:
> + internal_config.hugefile_prefix = optarg;
> + break;
> +
> + case OPT_SOCKET_MEM_NUM:
> + if (eal_parse_socket_mem(optarg) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameters for --"
> + OPT_SOCKET_MEM "\n");
> eal_usage(prgname);
> ret = -1;
> goto out;
> }
> + break;
>
> - ret = eal_parse_common_option(opt, optarg, &internal_config);
> - /* common parser is not happy */
> - if (ret < 0) {
> + case OPT_BASE_VIRTADDR_NUM:
> + if (eal_parse_base_virtaddr(optarg) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_BASE_VIRTADDR "\n");
> eal_usage(prgname);
> ret = -1;
> goto out;
> }
> - /* common parser handled this option */
> - if (ret == 0)
> - continue;
> + break;
>
> - switch (opt) {
> - case 'h':
> + case OPT_VFIO_INTR_NUM:
> + if (eal_parse_vfio_intr(optarg) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameters for --"
> + OPT_VFIO_INTR "\n");
> eal_usage(prgname);
> - exit(EXIT_SUCCESS);
> -
> - /* long options */
> - case OPT_XEN_DOM0_NUM:
> -#ifdef RTE_LIBRTE_XEN_DOM0
> - internal_config.xen_dom0_support = 1;
> -#else
> - RTE_LOG(ERR, EAL, "Can't support DPDK app "
> - "running on Dom0, please configure"
> - " RTE_LIBRTE_XEN_DOM0=y\n");
> ret = -1;
> goto out;
> -#endif
> - break;
> + }
> + break;
>
> - case OPT_HUGE_DIR_NUM:
> - internal_config.hugepage_dir = optarg;
> - break;
> + case OPT_CREATE_UIO_DEV_NUM:
> + internal_config.create_uio_dev = 1;
> + break;
>
> - case OPT_FILE_PREFIX_NUM:
> - internal_config.hugefile_prefix = optarg;
> - break;
> + default:
> + if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
> + RTE_LOG(ERR, EAL, "Option %c is not supported "
> + "on Linux\n", opt);
> + } else if (opt >= OPT_LONG_MIN_NUM &&
> + opt < OPT_LONG_MAX_NUM) {
> + RTE_LOG(ERR, EAL, "Option %s is not supported "
> + "on Linux\n",
> + eal_long_options[option_index].name);
> + } else {
> + RTE_LOG(ERR, EAL, "Option %d is not supported "
> + "on Linux\n", opt);
> + }
> + eal_usage(prgname);
> + ret = -1;
> + goto out;
> + }
>
> - case OPT_SOCKET_MEM_NUM:
> - if (eal_parse_socket_mem(optarg) < 0) {
> - RTE_LOG(ERR, EAL, "invalid parameters for --"
> - OPT_SOCKET_MEM "\n");
> - eal_usage(prgname);
> - ret = -1;
> - goto out;
> - }
> - break;
> + return 0;
> +out:
> + return ret;
> +}
>
> - case OPT_BASE_VIRTADDR_NUM:
> - if (eal_parse_base_virtaddr(optarg) < 0) {
> - RTE_LOG(ERR, EAL, "invalid parameter for --"
> - OPT_BASE_VIRTADDR "\n");
> - eal_usage(prgname);
> - ret = -1;
> - goto out;
> - }
> - break;
> +/* Parse the argument given in the command line of the application */
> +static int
> +eal_parse_args(int argc, char **argv)
> +{
> + int opt, ret;
> + char **argvopt;
> + int option_index;
> + char *prgname = argv[0];
> + const int old_optind = optind;
> + const int old_optopt = optopt;
> + char * const old_optarg = optarg;
>
> - case OPT_VFIO_INTR_NUM:
> - if (eal_parse_vfio_intr(optarg) < 0) {
> - RTE_LOG(ERR, EAL, "invalid parameters for --"
> - OPT_VFIO_INTR "\n");
> - eal_usage(prgname);
> - ret = -1;
> - goto out;
> - }
> - break;
> + argvopt = argv;
> + optind = 1;
>
> - case OPT_CREATE_UIO_DEV_NUM:
> - internal_config.create_uio_dev = 1;
> - break;
> + while ((opt = getopt_long(argc, argvopt, eal_short_options,
> + eal_long_options, &option_index)) != EOF) {
>
> - default:
> - if (opt < OPT_LONG_MIN_NUM && isprint(opt)) {
> - RTE_LOG(ERR, EAL, "Option %c is not supported "
> - "on Linux\n", opt);
> - } else if (opt >= OPT_LONG_MIN_NUM &&
> - opt < OPT_LONG_MAX_NUM) {
> - RTE_LOG(ERR, EAL, "Option %s is not supported "
> - "on Linux\n",
> - eal_long_options[option_index].name);
> - } else {
> - RTE_LOG(ERR, EAL, "Option %d is not supported "
> - "on Linux\n", opt);
> - }
> - eal_usage(prgname);
> - ret = -1;
> + ret = eal_parse_option(opt, optarg, option_index, prgname);
> + if (ret < 0)
> goto out;
> - }
> }
>
> if (eal_adjust_config(&internal_config) != 0) {
> @@ -995,3 +1028,149 @@ rte_eal_check_module(const char *module_name)
> /* Module has been found */
> return 1;
> }
> +
> +#ifdef RTE_LIBRTE_CFGFILE
> +#define vdev_buff_size 200
> +#define sectionname_size 20
> +static int
> +parse_vdev_devices(struct rte_cfgfile *cfg)
> +{
> + char sectionname[sectionname_size];
> + char buffer1[vdev_buff_size];
> + int vdev_nb = 0;
> + int n_entries;
> +
> + int i;
> +
> + /* ----------- parsing VDEVS */
> + snprintf(sectionname, sectionname_size, "DPDK.vdev%d", vdev_nb);
> +
> + for (vdev_nb = 1; rte_cfgfile_has_section(cfg, sectionname);
> + vdev_nb++) {
> + n_entries = rte_cfgfile_section_num_entries(cfg, sectionname);
> +
> + struct rte_cfgfile_entry entries[n_entries];
> +
> +
> + if (n_entries != rte_cfgfile_section_entries(cfg, sectionname,
> + entries, n_entries)) {
> + rte_eal_init_alert("Unexpected fault.");
> + rte_errno = EFAULT;
> + return -1;
> + }
> +
> + buffer1[0] = 0;
> + for (i = 0; i < n_entries; i++) {
> + if (strlen(entries[i].value)) {
> +
> + if ((strlen(buffer1) +
> + strlen(entries[i].name) +
> + strlen(entries[i].value) + 3)
> + >= vdev_buff_size)
> + goto buff_size_err;
> + strcat(buffer1, entries[i].name);
> + strcat(buffer1, "=");
> + strcat(buffer1, entries[i].value);
> + } else {
> + if ((strlen(buffer1) +
> + strlen(entries[i].name) + 2)
> + >= vdev_buff_size)
> + goto buff_size_err;
> + strcat(buffer1, entries[i].name);
> + }
> +
> + if (i < (n_entries - 1))
> + strcat(buffer1, ",");
> + }
> +
> + /* parsing vdev */
> + if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
> + buffer1) < 0) {
> + return -1;
> + }
> + snprintf(sectionname, sectionname_size, "DPDK.vdev%d", vdev_nb);
> + }
> + /* ----------- parsing VDEVS */
> + return 0;
> +
> +buff_size_err:
> + printf("parse_vdev_devices(): buffer size is to small\n");
> + return -1;
> +}
> +
> +static void
> +eal_getopt(const char *str, int *opt, int *option_index)
> +{
> + int i;
> +
> + *opt = '?';
> + *option_index = 0;
> +
> + if (strlen(str) == 1) {
> + *opt = *str;
> + return;
> + }
> +
> + for (i = 0; eal_long_options[i].name != NULL; i++) {
> + if (strcmp(str, eal_long_options[i].name) == 0) {
> + *opt = eal_long_options[i].val;
> + *option_index = i;
> + break;
> + }
> + }
> +}
> +
> +int
> +rte_eal_configure(struct rte_cfgfile *cfg, char *prgname)
> +{
> + int n_entries;
> + int i;
> + int opt;
> + int option_index;
> +
> + if (cfg == NULL) {
> + rte_errno = -EINVAL;
> + return -1;
> + }
> +
> + n_entries = rte_cfgfile_section_num_entries(cfg, "DPDK");
> +
> + if (n_entries < 1) {
> + printf("No DPDK section entries in cfgfile object\n");
> + return 0;
> + }
> +
> + struct rte_cfgfile_entry entries[n_entries];
> +
> + if (n_entries !=
> + rte_cfgfile_section_entries(cfg, "DPDK", entries,
> + n_entries)) {
> + rte_eal_init_alert("Unexpected fault.");
> + rte_errno = EFAULT;
> + return -1;
> + }
> +
> + eal_reset_internal_config(&internal_config);
> +
> + /* set log level as early as possible */
> + eal_log_level_cfg(cfg);
> +
> + for (i = 0; i < n_entries; i++) {
> + eal_getopt(entries[i].name, &opt, &option_index);
> +
> + if (eal_parse_option(opt, entries[i].value,
> + option_index, prgname) != 0) {
> + rte_eal_init_alert("Invalid config file arguments.");
> + rte_errno = EINVAL;
> + return -1;
> + }
> + }
> +
> + if (parse_vdev_devices(cfg) < 0) {
> + rte_eal_init_alert("Couldn't parse vdevs");
> + rte_errno = ENOMEM;
> + return -1;
> + }
> + return 0;
> +}
> +#endif
Can this code in the #ifdef #endif block above go in a common file, as
again it looks very alike the BSD version?
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 670bab3..c93e6d9 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -198,3 +198,7 @@ DPDK_17.05 {
> vfio_get_group_no;
>
> } DPDK_17.02;
> +
> +DPDK_17.08 {
> + rte_eal_configure;
> +} DPDK_17.05;
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index bcaf1b3..642af92 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -80,7 +80,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power
>
> _LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER) += -lrte_timer
> _LDLIBS-$(CONFIG_RTE_LIBRTE_EFD) += -lrte_efd
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile
>
> _LDLIBS-y += --whole-archive
>
> @@ -96,6 +95,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += -lrte_mempool
> _LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += -lrte_mempool_ring
> _LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring
> _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile
> _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline
> _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder
>
> --
> 2.7.4
>
More information about the dev
mailing list