[v11,5/9] log: drop syslog support, and make code common

Message ID 20240324024109.306614-6-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Logging unification and enhancements |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger March 24, 2024, 2:33 a.m. UTC
  This patch removes syslog support from logging. Which allows the
logging code to all be common across Linux, Windows and FreeBSD.
It also initializes log subsystem much earlier in the init
process so that all messages get processed.

It drops syslog was only used on Linux. Modern Linux systems have
systemd and journal support. A later patch will add direct
native journal support. Removing syslog means lots of other
parts of the log system can be simplified and made portable.
Remove the syslog tests and documentation as well.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_eal_flags.c                     | 21 +------
 doc/guides/linux_gsg/linux_eal_parameters.rst | 27 --------
 .../prog_guide/env_abstraction_layer.rst      |  5 +-
 doc/guides/prog_guide/log_lib.rst             |  3 +-
 lib/eal/common/eal_common_options.c           | 62 -------------------
 lib/eal/common/eal_internal_cfg.h             |  2 +-
 lib/eal/common/eal_options.h                  |  2 -
 lib/eal/freebsd/eal.c                         |  9 +--
 lib/eal/linux/eal.c                           | 10 +--
 lib/eal/unix/eal_debug.c                      |  3 +-
 lib/eal/windows/eal.c                         |  4 +-
 lib/log/log.c                                 | 46 +++++---------
 lib/log/log_freebsd.c                         | 12 ----
 lib/log/log_internal.h                        |  2 +-
 lib/log/log_linux.c                           | 61 ------------------
 lib/log/log_windows.c                         | 18 ------
 lib/log/meson.build                           |  5 +-
 17 files changed, 34 insertions(+), 258 deletions(-)
 delete mode 100644 lib/log/log_freebsd.c
 delete mode 100644 lib/log/log_linux.c
 delete mode 100644 lib/log/log_windows.c
  

Patch

diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 6cb4b06757..bea5465168 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -984,17 +984,10 @@  test_misc_flags(void)
 	const char *argv1[] = {prgname, prefix, mp_flag, "--no-pci"};
 	/* With -v */
 	const char *argv2[] = {prgname, prefix, mp_flag, "-v"};
-	/* With valid --syslog */
-	const char *argv3[] = {prgname, prefix, mp_flag,
-			"--syslog", "syslog"};
-	/* With empty --syslog (should fail) */
-	const char *argv4[] = {prgname, prefix, mp_flag, "--syslog"};
-	/* With invalid --syslog */
-	const char *argv5[] = {prgname, prefix, mp_flag, "--syslog", "error"};
+
 	/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
 	const char *argv6[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			no_shconf, nosh_prefix, no_huge};
-
 	/* With --huge-dir */
 	const char *argv7[] = {prgname, "-m", DEFAULT_MEM_SIZE,
 			"--file-prefix=hugedir", "--huge-dir", hugepath};
@@ -1079,18 +1072,6 @@  test_misc_flags(void)
 	return 0;
 #endif
 
-	if (launch_proc(argv3) != 0) {
-		printf("Error - process did not run ok with --syslog flag\n");
-		goto fail;
-	}
-	if (launch_proc(argv4) == 0) {
-		printf("Error - process run ok with empty --syslog flag\n");
-		goto fail;
-	}
-	if (launch_proc(argv5) == 0) {
-		printf("Error - process run ok with invalid --syslog flag\n");
-		goto fail;
-	}
 	if (launch_proc(argv7) != 0) {
 		printf("Error - process did not run ok with --huge-dir flag\n");
 		goto fail;
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst b/doc/guides/linux_gsg/linux_eal_parameters.rst
index ea8f381391..d86f94d8a8 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -108,30 +108,3 @@  Memory-related options
 *   ``--match-allocations``
 
     Free hugepages back to system exactly as they were originally allocated.
-
-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/doc/guides/prog_guide/env_abstraction_layer.rst b/doc/guides/prog_guide/env_abstraction_layer.rst
index 9559c12a98..9a73628907 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -855,9 +855,8 @@  Signal Safety
   Other functions are not signal safe because they use one or more
   library routines that are not themselves signal safe.
   For example, calling ``rte_panic()`` is not safe in a signal handler
-  because it uses ``rte_log()`` and ``rte_log()`` calls the
-  ``syslog()`` library function which is in the list of
-  signal safe functions in
+  because it uses ``rte_log()`` and ``rte_log()`` calls library functions
+  that are not all signal safe.
   `Signal-Safety manual page <https://man7.org/linux/man-pages/man7/signal-safety.7.html>`_.
 
   The set of functions that are expected to be async-signal-safe in DPDK
diff --git a/doc/guides/prog_guide/log_lib.rst b/doc/guides/prog_guide/log_lib.rst
index ff9d1b54a2..17ed8426b2 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -5,8 +5,7 @@  Log Library
 ===========
 
 The DPDK Log library provides the logging functionality for other DPDK libraries and drivers.
-By default, in a Linux application, logs are sent to syslog and also to the console.
-On FreeBSD and Windows applications, logs are sent only to the console.
+By default, log messages are sent to the stderr.
 However, the log function can be overridden by the user to use a different logging mechanism.
 
 Log Levels
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 5435399b85..73167d4603 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -6,9 +6,6 @@ 
 #include <stdlib.h>
 #include <string.h>
 #include <pthread.h>
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include <syslog.h>
-#endif
 #include <ctype.h>
 #include <limits.h>
 #include <errno.h>
@@ -93,7 +90,6 @@  eal_long_options[] = {
 	{OPT_PROC_TYPE,         1, NULL, OPT_PROC_TYPE_NUM        },
 	{OPT_SOCKET_MEM,        1, NULL, OPT_SOCKET_MEM_NUM       },
 	{OPT_SOCKET_LIMIT,      1, NULL, OPT_SOCKET_LIMIT_NUM     },
-	{OPT_SYSLOG,            1, NULL, OPT_SYSLOG_NUM           },
 	{OPT_VDEV,              1, NULL, OPT_VDEV_NUM             },
 	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
 	{OPT_VFIO_VF_TOKEN,     1, NULL, OPT_VFIO_VF_TOKEN_NUM    },
@@ -349,10 +345,6 @@  eal_reset_internal_config(struct internal_config *internal_cfg)
 	}
 	internal_cfg->base_virtaddr = 0;
 
-#ifdef LOG_DAEMON
-	internal_cfg->syslog_facility = LOG_DAEMON;
-#endif
-
 	/* if set to NONE, interrupt mode is determined automatically */
 	internal_cfg->vfio_intr_mode = RTE_INTR_MODE_NONE;
 	memset(internal_cfg->vfio_vf_token, 0,
@@ -1297,47 +1289,6 @@  eal_parse_lcores(const char *lcores)
 	return ret;
 }
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-static int
-eal_parse_syslog(const char *facility, struct internal_config *conf)
-{
-	int i;
-	static const struct {
-		const char *name;
-		int value;
-	} map[] = {
-		{ "auth", LOG_AUTH },
-		{ "cron", LOG_CRON },
-		{ "daemon", LOG_DAEMON },
-		{ "ftp", LOG_FTP },
-		{ "kern", LOG_KERN },
-		{ "lpr", LOG_LPR },
-		{ "mail", LOG_MAIL },
-		{ "news", LOG_NEWS },
-		{ "syslog", LOG_SYSLOG },
-		{ "user", LOG_USER },
-		{ "uucp", LOG_UUCP },
-		{ "local0", LOG_LOCAL0 },
-		{ "local1", LOG_LOCAL1 },
-		{ "local2", LOG_LOCAL2 },
-		{ "local3", LOG_LOCAL3 },
-		{ "local4", LOG_LOCAL4 },
-		{ "local5", LOG_LOCAL5 },
-		{ "local6", LOG_LOCAL6 },
-		{ "local7", LOG_LOCAL7 },
-		{ NULL, 0 }
-	};
-
-	for (i = 0; map[i].name; i++) {
-		if (!strcmp(facility, map[i].name)) {
-			conf->syslog_facility = map[i].value;
-			return 0;
-		}
-	}
-	return -1;
-}
-#endif
-
 static void
 eal_log_usage(void)
 {
@@ -1880,16 +1831,6 @@  eal_parse_common_option(int opt, const char *optarg,
 		}
 		break;
 
-#ifndef RTE_EXEC_ENV_WINDOWS
-	case OPT_SYSLOG_NUM:
-		if (eal_parse_syslog(optarg, conf) < 0) {
-			EAL_LOG(ERR, "invalid parameters for --"
-					OPT_SYSLOG);
-			return -1;
-		}
-		break;
-#endif
-
 	case OPT_LOG_LEVEL_NUM: {
 		if (eal_parse_log_level(optarg) < 0) {
 			EAL_LOG(ERR,
@@ -2259,9 +2200,6 @@  eal_common_usage(void)
 	       "                      (can be used multiple times)\n"
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
 	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
-#ifndef RTE_EXEC_ENV_WINDOWS
-	       "  --"OPT_SYSLOG"            Set syslog facility\n"
-#endif
 	       "  --"OPT_LOG_LEVEL"=<level> Set global log level\n"
 	       "  --"OPT_LOG_LEVEL"=<type-match>:<level>\n"
 	       "                      Set specific log level\n"
diff --git a/lib/eal/common/eal_internal_cfg.h b/lib/eal/common/eal_internal_cfg.h
index 167ec501fa..ed56a58b8b 100644
--- a/lib/eal/common/eal_internal_cfg.h
+++ b/lib/eal/common/eal_internal_cfg.h
@@ -84,7 +84,7 @@  struct internal_config {
 	/**< true if storing all pages within single files (per-page-size,
 	 * per-node) non-legacy mode only.
 	 */
-	volatile int syslog_facility;	  /**< facility passed to openlog() */
+
 	/** default interrupt mode for VFIO */
 	volatile enum rte_intr_mode vfio_intr_mode;
 	/** the shared VF token for VFIO-PCI bound PF and VFs devices */
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index f3f2e104f6..6b204d6698 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -63,8 +63,6 @@  enum {
 	OPT_SOCKET_MEM_NUM,
 #define OPT_SOCKET_LIMIT        "socket-limit"
 	OPT_SOCKET_LIMIT_NUM,
-#define OPT_SYSLOG            "syslog"
-	OPT_SYSLOG_NUM,
 #define OPT_VDEV              "vdev"
 	OPT_VDEV_NUM,
 #define OPT_VFIO_INTR         "vfio-intr"
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 17b56f38aa..d8628ba632 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -11,7 +11,6 @@ 
 #include <stdarg.h>
 #include <unistd.h>
 #include <pthread.h>
-#include <syslog.h>
 #include <getopt.h>
 #include <sys/file.h>
 #include <stddef.h>
@@ -563,12 +562,14 @@  rte_eal_init(int argc, char **argv)
 
 	eal_reset_internal_config(internal_conf);
 
-	/* clone argv to report out later in telemetry */
-	eal_save_args(argc, argv);
-
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
+	eal_log_init(getprogname());
+
+	/* clone argv to report out later in telemetry */
+	eal_save_args(argc, argv);
+
 	if (rte_eal_cpu_init() < 0) {
 		rte_eal_init_alert("Cannot detect lcores.");
 		rte_errno = ENOTSUP;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 23dc26b124..913ee62fc6 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -955,6 +955,8 @@  rte_eal_init(int argc, char **argv)
 	/* set log level as early as possible */
 	eal_log_level_parse(argc, argv);
 
+	eal_log_init(program_invocation_short_name);
+
 	/* clone argv to report out later in telemetry */
 	eal_save_args(argc, argv);
 
@@ -1106,14 +1108,6 @@  rte_eal_init(int argc, char **argv)
 #endif
 	}
 
-	if (eal_log_init(program_invocation_short_name,
-			 internal_conf->syslog_facility) < 0) {
-		rte_eal_init_alert("Cannot init logging.");
-		rte_errno = ENOMEM;
-		rte_atomic_store_explicit(&run_once, 0, rte_memory_order_relaxed);
-		return -1;
-	}
-
 #ifdef VFIO_PRESENT
 	if (rte_eal_vfio_setup() < 0) {
 		rte_eal_init_alert("Cannot init VFIO");
diff --git a/lib/eal/unix/eal_debug.c b/lib/eal/unix/eal_debug.c
index 69ba3758c2..c830cf7f05 100644
--- a/lib/eal/unix/eal_debug.c
+++ b/lib/eal/unix/eal_debug.c
@@ -43,8 +43,7 @@  static char *safe_itoa(long val, char *buf, size_t len, unsigned int radix)
  * used in this code since may be called from inside libc or
  * when malloc poll is corrupt.
  *
- * Most of libc is therefore not safe, include RTE_LOG (calls syslog);
- * backtrace_symbols (calls malloc), etc.
+ * Most of libc is therefore not safe including backtrace_symbols (calls malloc), etc.
  */
 void rte_dump_stack(void)
 {
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index 85171b2768..10f7cb840a 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -250,10 +250,10 @@  rte_eal_init(int argc, char **argv)
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 	char thread_name[RTE_THREAD_NAME_SIZE];
 
-	eal_log_init(NULL, 0);
-
 	eal_log_level_parse(argc, argv);
 
+	eal_log_init(NULL);
+
 	if (eal_create_cpu_map() < 0) {
 		rte_eal_init_alert("Cannot discover CPU and NUMA.");
 		/* rte_errno is set */
diff --git a/lib/log/log.c b/lib/log/log.c
index 255f757d94..4c8666ac93 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -55,9 +55,6 @@  TAILQ_HEAD(rte_eal_opt_loglevel_list, rte_eal_opt_loglevel);
 static struct rte_eal_opt_loglevel_list opt_loglevel_list =
 	TAILQ_HEAD_INITIALIZER(opt_loglevel_list);
 
-/* Stream to use for logging if rte_logs.file is NULL */
-static FILE *default_log_stream;
-
 /**
  * This global structure stores some information about the message
  * that is currently being processed by one lcore
@@ -76,6 +73,9 @@  static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg);
 int
 rte_openlog_stream(FILE *f)
 {
+	if (rte_logs.file != NULL)
+		fclose(rte_logs.file);
+
 	rte_logs.file = f;
 	return 0;
 }
@@ -85,17 +85,7 @@  rte_log_get_stream(void)
 {
 	FILE *f = rte_logs.file;
 
-	if (f == NULL) {
-		/*
-		 * Grab the current value of stderr here, rather than
-		 * just initializing default_log_stream to stderr. This
-		 * ensures that we will always use the current value
-		 * of stderr, even if the application closes and
-		 * reopens it.
-		 */
-		return default_log_stream != NULL ? default_log_stream : stderr;
-	}
-	return f;
+	return (f == NULL) ? stderr : f;
 }
 
 /* Set global log level */
@@ -506,27 +496,25 @@  rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
 }
 
 /*
- * Called by environment-specific initialization functions.
+ * Called by eal_cleanup
  */
 void
-eal_log_set_default(FILE *default_log)
+rte_eal_log_cleanup(void)
 {
-	default_log_stream = default_log;
+	FILE *f = rte_logs.file;
 
-#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
-	RTE_LOG(NOTICE, EAL,
-		"Debug dataplane logs available - lower performance\n");
-#endif
+	if (f != NULL) {
+		fclose(f);
+		rte_logs.file = NULL;
+	}
 }
 
-/*
- * Called by eal_cleanup
- */
+/* initialize logging */
 void
-rte_eal_log_cleanup(void)
+eal_log_init(const char *id __rte_unused)
 {
-	if (default_log_stream) {
-		fclose(default_log_stream);
-		default_log_stream = NULL;
-	}
+#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
+	RTE_LOG(NOTICE, EAL,
+		"Debug dataplane logs available - lower performance\n");
+#endif
 }
diff --git a/lib/log/log_freebsd.c b/lib/log/log_freebsd.c
deleted file mode 100644
index 698d3c5423..0000000000
--- a/lib/log/log_freebsd.c
+++ /dev/null
@@ -1,12 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2023 Intel Corporation
- */
-
-#include <rte_common.h>
-#include "log_internal.h"
-
-int
-eal_log_init(__rte_unused const char *id, __rte_unused int facility)
-{
-	return 0;
-}
diff --git a/lib/log/log_internal.h b/lib/log/log_internal.h
index 451629f1c1..4f1ffe999e 100644
--- a/lib/log/log_internal.h
+++ b/lib/log/log_internal.h
@@ -14,7 +14,7 @@ 
  * Initialize the default log stream.
  */
 __rte_internal
-int eal_log_init(const char *id, int facility);
+void eal_log_init(const char *id);
 
 /*
  * Determine where log data is written when no call to rte_openlog_stream.
diff --git a/lib/log/log_linux.c b/lib/log/log_linux.c
deleted file mode 100644
index 2dfb0c974b..0000000000
--- a/lib/log/log_linux.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 "log_internal.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/log/log_windows.c b/lib/log/log_windows.c
deleted file mode 100644
index a6a0889550..0000000000
--- a/lib/log/log_windows.c
+++ /dev/null
@@ -1,18 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2017-2018 Intel Corporation
- */
-
-#include <rte_common.h>
-#include <rte_log.h>
-#include "log_internal.h"
-
-/* set the log to default function, called during eal init process. */
-int
-eal_log_init(__rte_unused const char *id, __rte_unused int facility)
-{
-	rte_openlog_stream(stderr);
-
-	eal_log_set_default(stderr);
-
-	return 0;
-}
diff --git a/lib/log/meson.build b/lib/log/meson.build
index 0d4319b36f..891f77a237 100644
--- a/lib/log/meson.build
+++ b/lib/log/meson.build
@@ -2,8 +2,5 @@ 
 # Copyright(c) 2023 Intel Corporation
 
 includes += global_inc
-sources = files(
-        'log.c',
-        'log_' + exec_env + '.c',
-)
+sources = files('log.c')
 headers = files('rte_log.h')