FreeBSD logging code was not using syslog and did not have the same options as Linux. Move the log writing code to common source tree.
Also fix a number of argument parsing bugs: 1. If application is given a bogus option, the error message would get printed twice. Once during scan for log level and again during parsing of arguments. 2. A bad argument give to --log-level option was given the code would keep going. 3. The syslog facility argument was parsed too late to all log to be initialized before other errors. Signed-off-by: Stephen Hemminger <step...@networkplumber.org> --- .../freebsd_gsg/freebsd_eal_parameters.rst | 27 ++++ lib/eal/common/eal_log.h | 5 + lib/eal/freebsd/eal.c | 55 ++------ lib/eal/linux/eal.c | 48 +------ lib/eal/linux/eal_log.c | 61 --------- lib/eal/linux/meson.build | 1 - lib/eal/unix/eal_log.c | 121 ++++++++++++++++++ lib/eal/unix/meson.build | 1 + 8 files changed, 173 insertions(+), 146 deletions(-) delete mode 100644 lib/eal/linux/eal_log.c create mode 100644 lib/eal/unix/eal_log.c diff --git a/doc/guides/freebsd_gsg/freebsd_eal_parameters.rst b/doc/guides/freebsd_gsg/freebsd_eal_parameters.rst index fba467a2ce92..9270d9fa3bfc 100644 --- a/doc/guides/freebsd_gsg/freebsd_eal_parameters.rst +++ b/doc/guides/freebsd_gsg/freebsd_eal_parameters.rst @@ -18,3 +18,30 @@ FreeBSD-specific EAL parameters ------------------------------- There are currently no FreeBSD-specific EAL command-line parameters available. + +Other options +~~~~~~~~~~~~~ + +* ``--syslog <syslog facility>`` + + Set syslog facility. Valid syslog facilities are:: + + auth + cron + daemon + ftp + kern + lpr + mail + news + syslog + user + uucp + local0 + local1 + local2 + local3 + local4 + local5 + local6 + local7 diff --git a/lib/eal/common/eal_log.h b/lib/eal/common/eal_log.h index c784fa604389..31dc489350f6 100644 --- a/lib/eal/common/eal_log.h +++ b/lib/eal/common/eal_log.h @@ -13,6 +13,11 @@ */ int eal_log_init(const char *id, int facility); +/* + * Scan command line args for log settings. + */ +int eal_log_level_parse(int argc, char * const argv[]); + /* * Determine where log data is written when no call to rte_openlog_stream. */ diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c index 7008303e112a..bb9a2b1653d9 100644 --- a/lib/eal/freebsd/eal.c +++ b/lib/eal/freebsd/eal.c @@ -52,6 +52,7 @@ #include "eal_hugepages.h" #include "eal_options.h" #include "eal_memcfg.h" +#include "eal_log.h" #include "eal_trace.h" #define MEMSIZE_IF_NO_HUGE_PAGE (64ULL * 1024ULL * 1024ULL) @@ -363,48 +364,6 @@ eal_get_hugepage_mem_size(void) return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX; } -/* Parse the arguments for --log-level only */ -static void -eal_log_level_parse(int argc, char **argv) -{ - int opt; - char **argvopt; - int option_index; - const int old_optind = optind; - const int old_optopt = optopt; - const int old_optreset = optreset; - char * const old_optarg = optarg; - struct internal_config *internal_conf = - eal_get_internal_configuration(); - - argvopt = argv; - optind = 1; - optreset = 1; - - while ((opt = getopt_long(argc, argvopt, eal_short_options, - eal_long_options, &option_index)) != EOF) { - - int ret; - - /* getopt is not happy, stop right now */ - if (opt == '?') - break; - - ret = (opt == OPT_LOG_LEVEL_NUM) ? - eal_parse_common_option(opt, optarg, internal_conf) : 0; - - /* common parser is not happy */ - if (ret < 0) - break; - } - - /* restore getopt lib */ - optind = old_optind; - optopt = old_optopt; - optreset = old_optreset; - optarg = old_optarg; -} - /* Parse the argument given in the command line of the application */ static int eal_parse_args(int argc, char **argv) @@ -610,7 +569,11 @@ rte_eal_init(int argc, char **argv) eal_save_args(argc, argv); /* set log level as early as possible */ - eal_log_level_parse(argc, argv); + if (eal_log_level_parse(argc, argv) < 0) { + rte_eal_init_alert("invalid log option."); + rte_errno = EINVAL; + return -1; + } if (rte_eal_cpu_init() < 0) { rte_eal_init_alert("Cannot detect lcores."); @@ -759,6 +722,12 @@ rte_eal_init(int argc, char **argv) #endif } + if (eal_log_init(getprogname(), internal_conf->syslog_facility) < 0) { + rte_eal_init_alert("Cannot init logging."); + rte_errno = ENOMEM; + return -1; + } + /* in secondary processes, memory init may allocate additional fbarrays * not present in primary processes, so to avoid any potential issues, * initialize memzones first. diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c index 145afafde234..d7f268fc8116 100644 --- a/lib/eal/linux/eal.c +++ b/lib/eal/linux/eal.c @@ -546,45 +546,6 @@ eal_parse_vfio_vf_token(const char *vf_token) return -1; } -/* Parse the arguments for --log-level only */ -static void -eal_log_level_parse(int argc, char **argv) -{ - int opt; - char **argvopt; - int option_index; - const int old_optind = optind; - const int old_optopt = optopt; - char * const old_optarg = optarg; - struct internal_config *internal_conf = - eal_get_internal_configuration(); - - argvopt = argv; - optind = 1; - - while ((opt = getopt_long(argc, argvopt, eal_short_options, - eal_long_options, &option_index)) != EOF) { - - int ret; - - /* getopt is not happy, stop right now */ - if (opt == '?') - break; - - ret = (opt == OPT_LOG_LEVEL_NUM) ? - eal_parse_common_option(opt, optarg, internal_conf) : 0; - - /* common parser is not happy */ - if (ret < 0) - break; - } - - /* restore getopt lib */ - optind = old_optind; - optopt = old_optopt; - optarg = old_optarg; -} - static int eal_parse_huge_worker_stack(const char *arg) { @@ -649,7 +610,7 @@ eal_parse_args(int argc, char **argv) goto out; } - /* eal_log_level_parse() already handled this option */ + /* eal_log_level_parse() already handled these */ if (opt == OPT_LOG_LEVEL_NUM) continue; @@ -993,7 +954,12 @@ rte_eal_init(int argc, char **argv) eal_reset_internal_config(internal_conf); /* set log level as early as possible */ - eal_log_level_parse(argc, argv); + if (eal_log_level_parse(argc, argv) < 0) { + rte_eal_init_alert("invalid log option."); + rte_errno = EINVAL; + __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED); + return -1; + } /* clone argv to report out later in telemetry */ eal_save_args(argc, argv); diff --git a/lib/eal/linux/eal_log.c b/lib/eal/linux/eal_log.c deleted file mode 100644 index d44416fd6570..000000000000 --- a/lib/eal/linux/eal_log.c +++ /dev/null @@ -1,61 +0,0 @@ -/* SPDX-License-Identifier: BSD-3-Clause - * Copyright(c) 2010-2014 Intel Corporation - */ - -#include <stdio.h> -#include <sys/types.h> -#include <syslog.h> - -#include <rte_log.h> - -#include "eal_log.h" - -/* - * default log function - */ -static ssize_t -console_log_write(__rte_unused void *c, const char *buf, size_t size) -{ - ssize_t ret; - - /* write on stderr */ - ret = fwrite(buf, 1, size, stderr); - fflush(stderr); - - /* Syslog error levels are from 0 to 7, so subtract 1 to convert */ - syslog(rte_log_cur_msg_loglevel() - 1, "%.*s", (int)size, buf); - - return ret; -} - -static int -console_log_close(__rte_unused void *c) -{ - closelog(); - return 0; -} - -static cookie_io_functions_t console_log_func = { - .write = console_log_write, - .close = console_log_close, -}; - -/* - * set the log to default function, called during eal init process, - * once memzones are available. - */ -int -eal_log_init(const char *id, int facility) -{ - FILE *log_stream; - - log_stream = fopencookie(NULL, "w+", console_log_func); - if (log_stream == NULL) - return -1; - - openlog(id, LOG_NDELAY | LOG_PID, facility); - - eal_log_set_default(log_stream); - - return 0; -} diff --git a/lib/eal/linux/meson.build b/lib/eal/linux/meson.build index 5af456db9edb..e99ebed25692 100644 --- a/lib/eal/linux/meson.build +++ b/lib/eal/linux/meson.build @@ -11,7 +11,6 @@ sources += files( 'eal_hugepage_info.c', 'eal_interrupts.c', 'eal_lcore.c', - 'eal_log.c', 'eal_memalloc.c', 'eal_memory.c', 'eal_thread.c', diff --git a/lib/eal/unix/eal_log.c b/lib/eal/unix/eal_log.c new file mode 100644 index 000000000000..f8a5c853f2df --- /dev/null +++ b/lib/eal/unix/eal_log.c @@ -0,0 +1,121 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2010-2014 Intel Corporation + */ + +#include <getopt.h> +#include <stdio.h> +#include <sys/types.h> +#include <syslog.h> + +#include <rte_log.h> + +#include "eal_log.h" +#include "eal_internal_cfg.h" +#include "eal_options.h" +#include "eal_private.h" + +/* + * default log function + */ +static ssize_t +console_log_write(__rte_unused void *c, const char *buf, size_t size) +{ + ssize_t ret; + + /* write on stderr */ + ret = fwrite(buf, 1, size, stderr); + fflush(stderr); + + /* Syslog error levels are from 0 to 7, so subtract 1 to convert */ + syslog(rte_log_cur_msg_loglevel() - 1, "%.*s", (int)size, buf); + +#ifdef RTE_EXEC_ENV_LINUX + /* Handle glibc quirk: write function should return the number of bytes + * copied from buf, or 0 on error. (The function must not return a negative value.) + * FreeBSD expects that write function behaves like write(2). + */ + if (ret < 0) + ret = 0; +#endif + + return ret; +} + +static int +console_log_close(__rte_unused void *c) +{ + closelog(); + return 0; +} + +static cookie_io_functions_t console_log_func = { + .write = console_log_write, + .close = console_log_close, +}; + + +/* + * Parse the arguments for --log-level and --syslog + */ +int +eal_log_level_parse(int argc, char *const argv[]) +{ + struct internal_config *internal_conf = eal_get_internal_configuration(); + int option_index, opt; + const int old_optind = optind; + const int old_optopt = optopt; + char * const old_optarg = optarg; +#ifdef RTE_EXEC_ENV_FREEBSD + const int old_optrset = optreset; +#endif + + optind = 1; +#ifdef RTE_EXEC_ENV_FREEBSD + optreset = 1; +#endif + + while ((opt = getopt_long(argc, argv, eal_short_options, + eal_long_options, &option_index)) != EOF) { + + switch (opt) { + case OPT_LOG_LEVEL_NUM: + if (eal_parse_common_option(opt, optarg, internal_conf) < 0) + return -1; + break; + case '?': + /* getopt is not happy, stop right now */ + goto out; + default: + continue; + } + } +out: + /* restore getopt lib */ + optind = old_optind; + optopt = old_optopt; + optarg = old_optarg; +#ifdef RTE_EXEC_ENV_FREEBSD + optreset = old_optreset; +#endif + return 0; +} + +/* + * set the log to default function, called during eal init process, + * once memzones are available. + */ +int +eal_log_init(const char *id, int facility) +{ + FILE *log_stream; + + log_stream = fopencookie(NULL, "w+", console_log_func); + if (log_stream == NULL) + return -1; + + openlog(id, LOG_NDELAY | LOG_PID, facility); + + eal_log_set_default(log_stream); + + return 0; +} diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build index cc7d67dd321d..37d07594df29 100644 --- a/lib/eal/unix/meson.build +++ b/lib/eal/unix/meson.build @@ -6,6 +6,7 @@ sources += files( 'eal_file.c', 'eal_filesystem.c', 'eal_firmware.c', + 'eal_log.c', 'eal_unix_memory.c', 'eal_unix_thread.c', 'eal_unix_timer.c', -- 2.39.2