Hi, I'm not sure about the proper way to do a code review here, feel free to educate me if there is a better way to do it. I couldn't find the actual patches text in the mail and patchwork has no interface to write comments on the line itself but I may very well have missed something. https://patches.dpdk.org/project/dpdk/patch/20241016202343.190653-13-stephen@networkplumber.org/ Why not provide an option to disable the syslog with --syslog=none? It will allow overriding a previous setting if one was given as well. (the same applies to the --log-journal option) https://patches.dpdk.org/project/dpdk/patch/20241016202343.190653-15-stephen@networkplumber.org/ You have both --log-color=never and --log-color=none, you probably need to make up your mind, my vote would be with none for better compatibility with the syslog/journal options I made above. But I don't care that much about it. Would also be good to document about the dark mode option so people don't need to find it only in the code. Baruch On Wed, Oct 16, 2024 at 11:24 PM Stephen Hemminger < stephen@networkplumber.org> wrote: > Improvements and unification of logging library. > This version works on all platforms: Linux, Windows and FreeBSD. > > This is update to rework patch set. It adds several new features > to the console log output. > > * Putting a timestamp on console output which is useful for > analyzing performance of startup codes. Timestamp is optional > and must be enabled on command line. > > * Displaying console output with colors. > It uses the standard conventions used by many other Linux commands > for colorized display. The default is to enable color if the > console output is going to a terminal. But it can be always > on or disabled by command line flag. This default was chosen > based on what dmesg(1) command does. > > Color is used by many tools (vi, iproute2, git) because it is helpful; > DPDK drivers and libraries print lots of not very useful messages. > And having error messages highlighted in bold face helps. > This might also get users to pay more attention to error messages. > Many bug reports have earlier messages that are lost because > there are so many info messages. > > * Add support for automatic detection of systemd journal > protocol. If running as systemd service will get enhanced > logging. > > * Use of syslog is optional and the meaning of the > --syslog flag has changed. The default is *not* to use > syslog if output is going to a terminal. > > Add myself as maintainer for log because by now have added > more than previous authors. > > v26 - rebase and change release note > - by default, do not disable color > - support dark mode with colors > - when using color format message to string then print > to avoid getting multiple threads intermixing on output. > > Stephen Hemminger (15): > maintainers: add for log library > windows: make getopt functions have const properties > windows: add os shim for localtime_r > eal: make eal_log_level_parse common > eal: do not duplicate rte_init_alert() messages > eal: change rte_exit() output to match rte_log() > log: move handling of syslog facility out of eal > eal: initialize log before everything else > log: drop syslog support, and make code common > log: add hook for printing log messages > log: add timestamp option > log: add optional support of syslog > log: add support for systemd journal > log: colorize log output > doc: add release note about log library > > MAINTAINERS | 1 + > app/test/test_eal_flags.c | 64 ++++- > doc/guides/linux_gsg/linux_eal_parameters.rst | 27 -- > doc/guides/prog_guide/log_lib.rst | 76 +++++- > doc/guides/rel_notes/release_24_11.rst | 26 ++ > lib/eal/common/eal_common_debug.c | 6 +- > lib/eal/common/eal_common_options.c | 135 ++++++---- > lib/eal/common/eal_internal_cfg.h | 1 - > lib/eal/common/eal_options.h | 7 + > lib/eal/freebsd/eal.c | 64 +---- > lib/eal/linux/eal.c | 68 +---- > lib/eal/windows/eal.c | 49 +--- > lib/eal/windows/getopt.c | 23 +- > lib/eal/windows/include/getopt.h | 8 +- > lib/eal/windows/include/rte_os_shim.h | 10 + > lib/log/log.c | 71 ++++-- > lib/log/log_color.c | 216 ++++++++++++++++ > lib/log/log_freebsd.c | 12 - > lib/log/log_internal.h | 28 +- > lib/log/log_journal.c | 200 +++++++++++++++ > lib/log/log_linux.c | 61 ----- > lib/log/log_private.h | 61 +++++ > lib/log/log_syslog.c | 88 +++++++ > lib/log/log_timestamp.c | 240 ++++++++++++++++++ > lib/log/log_windows.c | 18 -- > lib/log/meson.build | 12 +- > lib/log/version.map | 5 +- > 27 files changed, 1197 insertions(+), 380 deletions(-) > create mode 100644 lib/log/log_color.c > delete mode 100644 lib/log/log_freebsd.c > create mode 100644 lib/log/log_journal.c > delete mode 100644 lib/log/log_linux.c > create mode 100644 lib/log/log_private.h > create mode 100644 lib/log/log_syslog.c > create mode 100644 lib/log/log_timestamp.c > delete mode 100644 lib/log/log_windows.c > > -- > 2.45.2 > > -- Baruch Even Platform Technical Lead at WEKA E baruch@weka.io* ­*W https://www.weka.io* ­* [image: App Banner Image]