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

Reply via email to