Skip to content

Convert logging config to YANG/mgmtd, and add missing mgmtd functionality#19060

Merged
Jafaral merged 6 commits intoFRRouting:masterfrom
LabNConsulting:chopps/log-to-nb
Jun 27, 2025
Merged

Convert logging config to YANG/mgmtd, and add missing mgmtd functionality#19060
Jafaral merged 6 commits intoFRRouting:masterfrom
LabNConsulting:chopps/log-to-nb

Conversation

@choppsv1
Copy link
Contributor

@choppsv1 choppsv1 commented Jun 20, 2025

Add a frr-logging.yang module, and implement it converting to Mgmtd/YANG.

Add a topotest for logging config.

This is the penultimate conversion to be done for lib/*. logging 5424 is the only remaining functionality not converted, and this appears to not actually be used.

Prior to this PR mgmtd was a YANG broker only, it didn't actually process
any YANG configuration for itself. These changes add support for configuring
mgmtd with YANG in addition to the backend cilents.

Finally add a log record-severity to add syslog standard tags to file/consol logs (i.e., DEBUG, INFO, ...).

@frrbot frrbot bot added libfrr mgmt FRR Management Infra tests Topotests, make check, etc tools yang labels Jun 20, 2025
@frrbot frrbot bot added the docker label Jun 20, 2025
@choppsv1 choppsv1 force-pushed the chopps/log-to-nb branch 14 times, most recently from a464420 to 5ea1666 Compare June 22, 2025 04:26
@frrbot frrbot bot added the documentation label Jun 23, 2025
@choppsv1 choppsv1 marked this pull request as draft June 23, 2025 07:47
@choppsv1 choppsv1 force-pushed the chopps/log-to-nb branch 2 times, most recently from c289f7e to 92727f7 Compare June 24, 2025 11:43
@choppsv1 choppsv1 marked this pull request as ready for review June 24, 2025 15:24
leaf record-severity {
type boolean;
default false;
description "Include priority in non-syslog messages.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We maybe use priority and severity interchangeably, but did you mean severity here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, should be Include severity ...

choppsv1 added 3 commits June 24, 2025 16:19
Add module for configuring logging. Add and use ietf-syslog-types for syslog
based configuration.

Signed-off-by: Christian Hopps <chopps@labn.net>
Signed-off-by: Christian Hopps <chopps@labn.net>
Record log level using standard syslog severity tags which is understood
by auto-styling/coloring log viewers, etc.

Signed-off-by: Christian Hopps <chopps@labn.net>
@@ -173,7 +173,8 @@ struct timestamp_control {
"Local use\n"


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in the commit msg cilents

mgmtd/mgmt_txn.c Outdated
enum mgmt_txn_req_type req_type;
uint64_t req_id;
union {
/* XXX allocated or not, pick one, but cc is by far the largest! */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this comment, drop or clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The union is made up of pointers to allocated sub-structs except struct mgmt_commit_cfg_req commit_cfg (i.e., cc) which is declared inline. Usually one would do this bit of oddness b/c the allocated things are very large and the inline thing was small to save on the extra allocation.

In this case the inline thing is way bigger than any of the other allocated things in the union so it makes no sense.

Copy link
Contributor Author

@choppsv1 choppsv1 Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this comment here so someone maybe cleans it up. But since it will change all references from commit_cfg.foobar to commit_cfg->foobar everywhere in the code (or rpc.foobar, get_tree.foobar from rpc->foobar and get_tree->foobar if the choice is the other direction to embed all of the variants rather than allocate) I wanted to leave this to a cleanup PR, and not generate a lot of noise in this functionality PR. .

choppsv1 added 2 commits June 24, 2025 16:45
Prior to this mgmtd was a YANG broker only, it didn't actually process
any YANG configuration for itself. These changes add support for configuring
mgmtd with YANG in addition to the backend clients.

Signed-off-by: Christian Hopps <chopps@labn.net>
Signed-off-by: Christian Hopps <chopps@labn.net>
Test logging configuration variations including syslog.

Signed-off-by: Christian Hopps <chopps@labn.net>
@Jafaral Jafaral merged commit 883a715 into FRRouting:master Jun 27, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants