lib: preparations for RFC5424 syslog support #8656

eqvinox · 2021-05-10T19:35:47Z

#8457 2nd edition. Doesn't a reboot always help?

This is all the preparatory commits for the upcoming RFC5424 PR which I'll open in a minute. This is split off because everything here should be ready to go regardless of polishing or adjustments on the RFC5424 bits.

polychaeta

Thanks for your contribution to FRR!

Click for style suggestions

To apply these suggestions:

curl -s https://gist.githubusercontent.com/polychaeta/ae133430b149492ef91dc92adbac8588/raw/1d2f042d82891b40166b1d141cb4c2bb9e57d55e/cr_8656_1620675351.diff | git apply

diff --git a/lib/log_vty.c b/lib/log_vty.c
index cbb8de897..012ca9cc5 100644
--- a/lib/log_vty.c
+++ b/lib/log_vty.c
@@ -596,13 +596,8 @@ DEFUN (no_config_log_filterfile,
 	return CMD_SUCCESS;
 }
 
-DEFPY (log_filter,
-       log_filter_cmd,
-       "[no] log filter-text WORD$filter",
-       NO_STR
-       "Logging control\n"
-       FILTER_LOG_STR
-       "String to filter by\n")
+DEFPY(log_filter, log_filter_cmd, "[no] log filter-text WORD$filter",
+      NO_STR "Logging control\n" FILTER_LOG_STR "String to filter by\n")
 {
 	int ret = 0;
 
@@ -625,24 +620,16 @@ DEFPY (log_filter,
 }
 
 /* Clear all log filters */
-DEFPY (log_filter_clear,
-       log_filter_clear_cmd,
-       "clear log filter-text",
-       CLEAR_STR
-       "Logging control\n"
-       FILTER_LOG_STR)
+DEFPY(log_filter_clear, log_filter_clear_cmd, "clear log filter-text",
+      CLEAR_STR "Logging control\n" FILTER_LOG_STR)
 {
 	zlog_filter_clear();
 	return CMD_SUCCESS;
 }
 
 /* Show log filter */
-DEFPY (show_log_filter,
-       show_log_filter_cmd,
-       "show logging filter-text",
-       SHOW_STR
-       "Show current logging configuration\n"
-       FILTER_LOG_STR)
+DEFPY(show_log_filter, show_log_filter_cmd, "show logging filter-text",
+      SHOW_STR "Show current logging configuration\n" FILTER_LOG_STR)
 {
 	char log_filters[ZLOG_FILTERS_MAX * (ZLOG_FILTER_LENGTH_MAX + 3)] = "";
 	int len = 0;

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

LabN-CI · 2021-05-10T20:58:27Z

Outdated results 💚

Basic BGPD CI results: SUCCESS, 0 tests failed

_	_
Result	SUCCESS git merge/8656 `ee72f83`
Date	05/10/2021
Start	16:32:56
Finish	16:58:24
Run-Time	25:28
Total	1815
Pass	1815
Fail	0
Valgrind-Errors
Valgrind-Loss
Details	vncregress-2021-05-10-16:32:56.txt
Log	autoscript-2021-05-10-16:34:07.log.bz2
Memory	493 515 428

For details, please contact louberger

NetDEF-CI · 2021-05-11T00:17:22Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18927/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for log_filter.c | 2 issues
===============================================
< ERROR: do not use assignment in if condition
< #139: FILE: /tmp/f1-20179/log_filter.c:139:
Report for log_vty.c | 2 issues
===============================================
< ERROR: "foo * bar" should be "foo *bar"
< #37: FILE: /tmp/f1-20179/log_vty.c:37:
Report for log_vty.h | 2 issues
===============================================
< ERROR: "foo * bar" should be "foo *bar"
< #43: FILE: /tmp/f1-20179/log_vty.h:43:

CLANG Static Analyzer Summary

Github Pull Request 8656, comparing to Git base SHA 3b20069
Base image data for Git 3b20069 does not exist - compare skipped

6 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-18927/artifact/shared/static_analysis/index.html

donaldsharp

I see new clang SA issues that need to be addressed. Other than that I don't have a huge problem with this. Get these cleaned up and I'll get this in

`log-filter WORD` was giving me a serious headache since it also matches `log WORD` due to the way the CLI token handling works. This meant that a mistyped `log something` command would silently be interpreted as a filter string, causing me serious headscratching and WTFs until I figured what was going on. Remove this UX pitfall so noone else falls into it. (Since the command was never saved to config, renaming it shouldn't cause trouble.) [Also I apparently forgot to update the docs when I transferred this over to the new zlog bits...] TODO for a rainy day: since we collect all the CLI commands anyway, we should warn somewhere for "2nd level ambiguous" commands like this. Signed-off-by: David Lamparter <equinox@diac24.net>

... so additional targets can print their state. Signed-off-by: David Lamparter <equinox@diac24.net>

printfrr() recently acquired the capability to record start/end of formatting outputs. Make use of this in the zlog code so logging targets have access to this information. (This also records how long the `[XXXXX-XXXXX][EC 9999999]` prefix was so log targets can choose to skip over it.) Signed-off-by: David Lamparter <equinox@diac24.net>

Signed-off-by: David Lamparter <equinox@diac24.net>

This is old-style syslog, used among other things for /dev/log. Signed-off-by: David Lamparter <equinox@diac24.net>

Since the file targets append one anyway, save them some extra work. syslog can use `%.*s` since it's "forced" printf by API anyway. Signed-off-by: David Lamparter <equinox@diac24.net>

glibc removed its pid cache a while back, and grabbing this from TLS is faster than repeatedly calling the kernel... Also use `intmax_t` which is more appropriate for both PID & TID. Signed-off-by: David Lamparter <equinox@diac24.net>

Signed-off-by: David Lamparter <equinox@diac24.net>

Might've made a few things too many `static` there. Signed-off-by: David Lamparter <equinox@diac24.net>

*sigh*. It doesn't accept `%m` otherwise. Signed-off-by: David Lamparter <equinox@diac24.net>

polychaeta

Thanks for your contribution to FRR!

Click for style suggestions

To apply these suggestions:

curl -s https://gist.githubusercontent.com/polychaeta/223b63e33587456a9fac1ec9531fd1c0/raw/1d2f042d82891b40166b1d141cb4c2bb9e57d55e/cr_8656_1624043182.diff | git apply

diff --git a/lib/log_vty.c b/lib/log_vty.c
index cbb8de897..012ca9cc5 100644
--- a/lib/log_vty.c
+++ b/lib/log_vty.c
@@ -596,13 +596,8 @@ DEFUN (no_config_log_filterfile,
 	return CMD_SUCCESS;
 }
 
-DEFPY (log_filter,
-       log_filter_cmd,
-       "[no] log filter-text WORD$filter",
-       NO_STR
-       "Logging control\n"
-       FILTER_LOG_STR
-       "String to filter by\n")
+DEFPY(log_filter, log_filter_cmd, "[no] log filter-text WORD$filter",
+      NO_STR "Logging control\n" FILTER_LOG_STR "String to filter by\n")
 {
 	int ret = 0;
 
@@ -625,24 +620,16 @@ DEFPY (log_filter,
 }
 
 /* Clear all log filters */
-DEFPY (log_filter_clear,
-       log_filter_clear_cmd,
-       "clear log filter-text",
-       CLEAR_STR
-       "Logging control\n"
-       FILTER_LOG_STR)
+DEFPY(log_filter_clear, log_filter_clear_cmd, "clear log filter-text",
+      CLEAR_STR "Logging control\n" FILTER_LOG_STR)
 {
 	zlog_filter_clear();
 	return CMD_SUCCESS;
 }
 
 /* Show log filter */
-DEFPY (show_log_filter,
-       show_log_filter_cmd,
-       "show logging filter-text",
-       SHOW_STR
-       "Show current logging configuration\n"
-       FILTER_LOG_STR)
+DEFPY(show_log_filter, show_log_filter_cmd, "show logging filter-text",
+      SHOW_STR "Show current logging configuration\n" FILTER_LOG_STR)
 {
 	char log_filters[ZLOG_FILTERS_MAX * (ZLOG_FILTER_LENGTH_MAX + 3)] = "";
 	int len = 0;

If you are a new contributor to FRR, please see our contributing guidelines.

After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.

eqvinox · 2021-06-18T19:07:51Z

@donaldsharp sorry, missed the clang SA warnings and your note…

Should be fixed now, let's see what the CI says about the SA.

(Rebased while at it, no conflicts yet but still.)

LabN-CI · 2021-06-18T20:02:09Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/8656 `ca846ff`
Date	06/18/2021
Start	15:36:36
Finish	16:02:04
Run-Time	25:28
Total	1815
Pass	1815
Fail	0
Valgrind-Errors
Valgrind-Loss
Details	vncregress-2021-06-18-15:36:36.txt
Log	autoscript-2021-06-18-15:37:50.log.bz2
Memory	503 520 430

For details, please contact louberger

NetDEF-CI · 2021-06-19T01:29:57Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-19696/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for log_vty.c | 2 issues
===============================================
< ERROR: "foo * bar" should be "foo *bar"
< #37: FILE: /tmp/f1-5586/log_vty.c:37:
Report for log_vty.h | 2 issues
===============================================
< ERROR: "foo * bar" should be "foo *bar"
< #43: FILE: /tmp/f1-5586/log_vty.h:43:

eqvinox · 2021-06-19T12:59:30Z

@donaldsharp should be good to go now.

(sidenote: the 2 style warnings from checkpatch are "conflicts" with clang-format ... clang-format wants it one way, checkpatch wants it another way, and they can't be satisfied simultaneously.)

eqvinox requested a review from qlyoung May 10, 2021 19:35

polychaeta added bugfix libfrr labels May 10, 2021

polychaeta reviewed May 10, 2021

View reviewed changes

donaldsharp requested changes May 20, 2021

View reviewed changes

eqvinox added 10 commits June 18, 2021 20:56

lib: add hook for show logging CLI

ba9256d

... so additional targets can print their state. Signed-off-by: David Lamparter <equinox@diac24.net>

lib: use fbuf for zlog_msg_ts()

f6caaa6

Signed-off-by: David Lamparter <equinox@diac24.net>

lib: add RFC3164 logging timestamps

243ff22

This is old-style syslog, used among other things for /dev/log. Signed-off-by: David Lamparter <equinox@diac24.net>

lib: include \n in zlog_msg_text()

8b94cb4

Since the file targets append one anyway, save them some extra work. syslog can use `%.*s` since it's "forced" printf by API anyway. Signed-off-by: David Lamparter <equinox@diac24.net>

lib: cache PID & TID in zlog code

78598fd

glibc removed its pid cache a while back, and grabbing this from TLS is faster than repeatedly calling the kernel... Also use `intmax_t` which is more appropriate for both PID & TID. Signed-off-by: David Lamparter <equinox@diac24.net>

lib: save instance number in zlog

5c58f5d

Signed-off-by: David Lamparter <equinox@diac24.net>

lib: make a few log symbols accessible

c6610ea

Might've made a few things too many `static` there. Signed-off-by: David Lamparter <equinox@diac24.net>

lib: fix printf format on NetBSD

ca846ff

*sigh*. It doesn't accept `%m` otherwise. Signed-off-by: David Lamparter <equinox@diac24.net>

eqvinox force-pushed the xref-5424-prep-2 branch from ee72f83 to ca846ff Compare June 18, 2021 19:06

polychaeta reviewed Jun 18, 2021

View reviewed changes

eqvinox requested a review from donaldsharp June 19, 2021 13:03

donaldsharp merged commit 2afd15f into FRRouting:master Jun 23, 2021

eqvinox deleted the xref-5424-prep-2 branch June 23, 2021 12:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: preparations for RFC5424 syslog support #8656

lib: preparations for RFC5424 syslog support #8656

eqvinox commented May 10, 2021

polychaeta left a comment

LabN-CI commented May 10, 2021 •

edited

Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented May 11, 2021 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

CLANG Static Analyzer Summary

6 Static Analyzer issues remaining.

donaldsharp left a comment

polychaeta left a comment

eqvinox commented Jun 18, 2021

LabN-CI commented Jun 18, 2021

NetDEF-CI commented Jun 19, 2021

eqvinox commented Jun 19, 2021

lib: preparations for RFC5424 syslog support #8656

lib: preparations for RFC5424 syslog support #8656

Conversation

eqvinox commented May 10, 2021

polychaeta left a comment

Choose a reason for hiding this comment

LabN-CI commented May 10, 2021 • edited Loading

Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented May 11, 2021 • edited Loading

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

CLANG Static Analyzer Summary

6 Static Analyzer issues remaining.

donaldsharp left a comment

Choose a reason for hiding this comment

polychaeta left a comment

Choose a reason for hiding this comment

eqvinox commented Jun 18, 2021

LabN-CI commented Jun 18, 2021

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Jun 19, 2021

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

eqvinox commented Jun 19, 2021

LabN-CI commented May 10, 2021 •

edited

Loading

NetDEF-CI commented May 11, 2021 •

edited

Loading