diff options
40 files changed, 822 insertions, 331 deletions
diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index aa1d81362b..15738e673b 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -2039,7 +2039,6 @@ uint16_t bgp_open_capability(struct stream *s, struct peer *peer, * or disable its use, and that switch MUST be off by default. */ if (peergroup_flag_check(peer, PEER_FLAG_CAPABILITY_SOFT_VERSION) || - CHECK_FLAG(peer->bgp->flags, BGP_FLAG_SOFT_VERSION_CAPABILITY) || peer->sort == BGP_PEER_IBGP || peer->sub_sort == BGP_PEER_EBGP_OAD) { SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_ADV); stream_putc(s, BGP_OPEN_OPT_CAP); diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 55d3efde23..8a36a3f4c8 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1240,7 +1240,6 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, /* Encode MP_EXT capability. */ switch (capability_code) { case CAPABILITY_CODE_SOFT_VERSION: - SET_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_ADV); stream_putc(s, action); stream_putc(s, CAPABILITY_CODE_SOFT_VERSION); cap_len = stream_get_endp(s); @@ -1271,6 +1270,9 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, : "Removing", capability, iana_afi2str(pkt_afi), iana_safi2str(pkt_safi)); + + COND_FLAG(peer->cap, PEER_CAP_SOFT_VERSION_ADV, + action == CAPABILITY_ACTION_SET); break; case CAPABILITY_CODE_MP: stream_putc(s, action); @@ -1290,11 +1292,6 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, iana_safi2str(pkt_safi)); break; case CAPABILITY_CODE_RESTART: - if (!CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART) && - !CHECK_FLAG(peer->flags, PEER_FLAG_GRACEFUL_RESTART_HELPER)) - return; - - SET_FLAG(peer->cap, PEER_CAP_RESTART_ADV); stream_putc(s, action); stream_putc(s, CAPABILITY_CODE_RESTART); cap_len = stream_get_endp(s); @@ -1343,13 +1340,10 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, capability, iana_afi2str(pkt_afi), iana_safi2str(pkt_safi)); + COND_FLAG(peer->cap, PEER_CAP_RESTART_ADV, + action == CAPABILITY_ACTION_SET); break; case CAPABILITY_CODE_LLGR: - if (!CHECK_FLAG(peer->cap, PEER_CAP_RESTART_ADV)) - return; - - SET_FLAG(peer->cap, PEER_CAP_LLGR_ADV); - stream_putc(s, action); stream_putc(s, CAPABILITY_CODE_LLGR); cap_len = stream_get_endp(s); @@ -1381,10 +1375,11 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, : "Removing", capability, iana_afi2str(pkt_afi), iana_safi2str(pkt_safi)); + + COND_FLAG(peer->cap, PEER_CAP_LLGR_ADV, + action == CAPABILITY_ACTION_SET); break; case CAPABILITY_CODE_ADDPATH: - SET_FLAG(peer->cap, PEER_CAP_ADDPATH_ADV); - FOREACH_AFI_SAFI (afi, safi) { if (peer->afc[afi][safi]) { addpath_afi_safi_count++; @@ -1462,10 +1457,10 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, capability, iana_afi2str(pkt_afi), iana_safi2str(pkt_safi)); + COND_FLAG(peer->cap, PEER_CAP_ADDPATH_ADV, + action == CAPABILITY_ACTION_SET); break; case CAPABILITY_CODE_PATHS_LIMIT: - SET_FLAG(peer->cap, PEER_CAP_PATHS_LIMIT_ADV); - FOREACH_AFI_SAFI (afi, safi) { if (!peer->afc[afi][safi]) continue; @@ -1505,6 +1500,8 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, .send); } + COND_FLAG(peer->cap, PEER_CAP_PATHS_LIMIT_ADV, + action == CAPABILITY_ACTION_SET); break; case CAPABILITY_CODE_ORF: /* Convert AFI, SAFI to values for packet. */ @@ -1578,43 +1575,42 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, iana_safi2str(pkt_safi)); break; case CAPABILITY_CODE_FQDN: - if (CHECK_FLAG(peer->flags, PEER_FLAG_CAPABILITY_FQDN) && - hostname) { - SET_FLAG(peer->cap, PEER_CAP_HOSTNAME_ADV); - stream_putc(s, action); - stream_putc(s, CAPABILITY_CODE_FQDN); - cap_len = stream_get_endp(s); - stream_putc(s, 0); /* Capability Length */ - - len = strlen(hostname); + stream_putc(s, action); + stream_putc(s, CAPABILITY_CODE_FQDN); + cap_len = stream_get_endp(s); + stream_putc(s, 0); /* Capability Length */ + + len = strlen(hostname); + if (len > BGP_MAX_HOSTNAME) + len = BGP_MAX_HOSTNAME; + + stream_putc(s, len); + stream_put(s, hostname, len); + + if (domainname) { + len = strlen(domainname); if (len > BGP_MAX_HOSTNAME) len = BGP_MAX_HOSTNAME; stream_putc(s, len); - stream_put(s, hostname, len); - - if (domainname) { - len = strlen(domainname); - if (len > BGP_MAX_HOSTNAME) - len = BGP_MAX_HOSTNAME; + stream_put(s, domainname, len); + } else + stream_putc(s, 0); - stream_putc(s, len); - stream_put(s, domainname, len); - } else - stream_putc(s, 0); + len = stream_get_endp(s) - cap_len - 1; + stream_putc_at(s, cap_len, len); - len = stream_get_endp(s) - cap_len - 1; - stream_putc_at(s, cap_len, len); + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s", + peer, + action == CAPABILITY_ACTION_SET + ? "Advertising" + : "Removing", + capability, iana_afi2str(pkt_afi), + iana_safi2str(pkt_safi)); - if (bgp_debug_neighbor_events(peer)) - zlog_debug("%pBP sending CAPABILITY has %s %s for afi/safi: %s/%s", - peer, - action == CAPABILITY_ACTION_SET - ? "Advertising" - : "Removing", - capability, iana_afi2str(pkt_afi), - iana_safi2str(pkt_safi)); - } + COND_FLAG(peer->cap, PEER_CAP_HOSTNAME_ADV, + action == CAPABILITY_ACTION_SET); break; case CAPABILITY_CODE_REFRESH: case CAPABILITY_CODE_AS4: @@ -1624,13 +1620,12 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, case CAPABILITY_CODE_EXT_MESSAGE: break; case CAPABILITY_CODE_ROLE: - if (peer->local_role != ROLE_UNDEFINED) { - SET_FLAG(peer->cap, PEER_CAP_ROLE_ADV); - stream_putc(s, action); - stream_putc(s, CAPABILITY_CODE_ROLE); - stream_putc(s, CAPABILITY_CODE_ROLE_LEN); - stream_putc(s, peer->local_role); - } + stream_putc(s, action); + stream_putc(s, CAPABILITY_CODE_ROLE); + stream_putc(s, CAPABILITY_CODE_ROLE_LEN); + stream_putc(s, peer->local_role); + COND_FLAG(peer->cap, PEER_CAP_ROLE_ADV, + action == CAPABILITY_ACTION_SET); break; default: break; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index efc71bc8a7..2920405780 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -3260,6 +3260,8 @@ DEFUN (bgp_graceful_restart_disable, GR_DISABLE) { int ret = BGP_GR_FAILURE; + struct listnode *node, *nnode; + struct peer *peer; if (BGP_DEBUG(graceful_restart, GRACEFUL_RESTART)) zlog_debug( @@ -3278,6 +3280,15 @@ DEFUN (bgp_graceful_restart_disable, vty_out(vty, "Graceful restart configuration changed, reset all peers to take effect\n"); + for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { + bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, + CAPABILITY_CODE_RESTART, + CAPABILITY_ACTION_UNSET); + bgp_capability_send(peer, AFI_IP, SAFI_UNICAST, + CAPABILITY_CODE_LLGR, + CAPABILITY_ACTION_UNSET); + } + return bgp_vty_return(vty, ret); } @@ -18416,9 +18427,19 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp, } /* capability software-version */ - if (peergroup_flag_check(peer, PEER_FLAG_CAPABILITY_SOFT_VERSION)) - vty_out(vty, " neighbor %s capability software-version\n", - addr); + if (CHECK_FLAG(bgp->flags, BGP_FLAG_SOFT_VERSION_CAPABILITY)) { + if (!peergroup_flag_check(peer, + PEER_FLAG_CAPABILITY_SOFT_VERSION)) + vty_out(vty, + " no neighbor %s capability software-version\n", + addr); + } else { + if (peergroup_flag_check(peer, + PEER_FLAG_CAPABILITY_SOFT_VERSION)) + vty_out(vty, + " neighbor %s capability software-version\n", + addr); + } /* dont-capability-negotiation */ if (peergroup_flag_check(peer, PEER_FLAG_DONT_CAPABILITY)) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index e7712f0f3e..be816c3361 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1541,6 +1541,9 @@ struct peer *peer_new(struct bgp *bgp) if (CHECK_FLAG(bgp->flags, BGP_FLAG_ENFORCE_FIRST_AS)) SET_FLAG(peer->flags, PEER_FLAG_ENFORCE_FIRST_AS); + if (CHECK_FLAG(bgp->flags, BGP_FLAG_SOFT_VERSION_CAPABILITY)) + SET_FLAG(peer->flags, PEER_FLAG_CAPABILITY_SOFT_VERSION); + SET_FLAG(peer->flags_invert, PEER_FLAG_CAPABILITY_FQDN); SET_FLAG(peer->flags, PEER_FLAG_CAPABILITY_FQDN); diff --git a/doc/developer/logging.rst b/doc/developer/logging.rst index 52653d3768..6058b8c0fc 100644 --- a/doc/developer/logging.rst +++ b/doc/developer/logging.rst @@ -77,7 +77,14 @@ are available: .. note:: - ``printfrr()`` does not support the ``%n`` format. + ``printfrr()`` does not support the ``%n`` format. It does support ISO C23 + ``%b``, ``%w99d`` and ``%wf99d`` additions, but the latter two are not + supported by the ``frr-format`` plugin yet, and all 3 aren't supported by + the older compilers still in use on some supported platforms. + + ``%b`` can be used with ``FMT_NSTD``, but ``%w99d`` and ``%wf99d`` require + work in the ``frr-format`` plugin before they are really usable. + AS-Safety ^^^^^^^^^ @@ -557,8 +564,9 @@ Integer formats cause compiler warnings when used without the plugin. Use with :c:macro:`FMT_NSTD` if necessary. - It is possible ISO C23 may introduce another format for these, possibly - ``%w64d`` discussed in `JTC 1/SC 22/WG 14/N2680 <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2680.pdf>`_. + As anticipated, ISO C23 has introduced new modifiers for this, specifically + ``%w64d`` (= ``%Ld``) and ``%w64u`` (= ``%Lu``). Unfortunately, these new + modifiers are not supported by ``frr-format`` yet. .. frrfmt:: %Lu (uint64_t) diff --git a/ldpd/lde.c b/ldpd/lde.c index ef4aecadad..876dd41630 100644 --- a/ldpd/lde.c +++ b/ldpd/lde.c @@ -28,6 +28,7 @@ #include "stream.h" #include "network.h" #include "libfrr.h" +#include "zlog_live.h" static void lde_shutdown(void); static void lde_dispatch_imsg(struct event *thread); @@ -116,6 +117,8 @@ static struct frr_signal_t lde_signals[] = void lde(void) { + static struct zlog_live_cfg child_log; + #ifdef HAVE_SETPROCTITLE setproctitle("label decision engine"); #endif @@ -123,6 +126,8 @@ lde(void) log_procname = log_procnames[PROC_LDE_ENGINE]; master = frr_init(); + zlog_live_open_fd(&child_log, LOG_DEBUG, LDPD_FD_LOG); + /* no frr_config_fork() here, allow frr_pthread to create threads */ frr_is_after_fork = true; diff --git a/ldpd/ldpd.c b/ldpd/ldpd.c index da29a4f20d..492a36b3d6 100644 --- a/ldpd/ldpd.c +++ b/ldpd/ldpd.c @@ -35,9 +35,10 @@ #include "qobj.h" #include "libfrr.h" #include "lib_errors.h" +#include "zlog_recirculate.h" static void ldpd_shutdown(void); -static pid_t start_child(enum ldpd_process, char *, int, int); +static pid_t start_child(enum ldpd_process, char *, int, int, int); static void main_dispatch_ldpe(struct event *thread); static void main_dispatch_lde(struct event *thread); static int main_imsg_send_ipc_sockets(struct imsgbuf *, @@ -69,6 +70,8 @@ DEFINE_QOBJ_TYPE(l2vpn_pw); DEFINE_QOBJ_TYPE(l2vpn); DEFINE_QOBJ_TYPE(ldpd_conf); +const char *log_procname; + struct ldpd_global global; struct ldpd_init init; struct ldpd_conf *ldpd_conf, *vty_conf; @@ -231,8 +234,12 @@ main(int argc, char *argv[]) { char *saved_argv0; int lflag = 0, eflag = 0; - int pipe_parent2ldpe[2], pipe_parent2ldpe_sync[2]; - int pipe_parent2lde[2], pipe_parent2lde_sync[2]; + int pipe_parent2ldpe[2]; + int pipe_parent2ldpe_sync[2]; + int pipe_ldpe_log[2]; + int pipe_parent2lde[2]; + int pipe_parent2lde_sync[2]; + int pipe_lde_log[2]; bool ctl_sock_used = false; ldpd_process = PROC_MAIN; @@ -300,15 +307,6 @@ main(int argc, char *argv[]) exit(1); } - if (lflag || eflag) { - struct zprivs_ids_t ids; - - zprivs_preinit(&ldpd_privs); - zprivs_get_ids(&ids); - - zlog_init(ldpd_di.progname, "LDP", 0, - ids.uid_normal, ids.gid_normal); - } if (lflag) lde(); else if (eflag) @@ -321,6 +319,9 @@ main(int argc, char *argv[]) pipe_parent2ldpe_sync) == -1) fatal("socketpair"); + if (socketpair(AF_UNIX, SOCK_DGRAM, PF_UNSPEC, pipe_ldpe_log) == -1) + fatal("socketpair"); + if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_parent2lde) == -1) fatal("socketpair"); @@ -328,6 +329,9 @@ main(int argc, char *argv[]) pipe_parent2lde_sync) == -1) fatal("socketpair"); + if (socketpair(AF_UNIX, SOCK_DGRAM, PF_UNSPEC, pipe_lde_log) == -1) + fatal("socketpair"); + sock_set_nonblock(pipe_parent2ldpe[0]); sock_set_cloexec(pipe_parent2ldpe[0]); sock_set_nonblock(pipe_parent2ldpe[1]); @@ -335,6 +339,11 @@ main(int argc, char *argv[]) sock_set_nonblock(pipe_parent2ldpe_sync[0]); sock_set_cloexec(pipe_parent2ldpe_sync[0]); sock_set_cloexec(pipe_parent2ldpe_sync[1]); + sock_set_nonblock(pipe_ldpe_log[0]); + sock_set_cloexec(pipe_ldpe_log[0]); + sock_set_nonblock(pipe_ldpe_log[1]); + sock_set_cloexec(pipe_ldpe_log[1]); + sock_set_nonblock(pipe_parent2lde[0]); sock_set_cloexec(pipe_parent2lde[0]); sock_set_nonblock(pipe_parent2lde[1]); @@ -342,14 +351,24 @@ main(int argc, char *argv[]) sock_set_nonblock(pipe_parent2lde_sync[0]); sock_set_cloexec(pipe_parent2lde_sync[0]); sock_set_cloexec(pipe_parent2lde_sync[1]); + sock_set_nonblock(pipe_lde_log[0]); + sock_set_cloexec(pipe_lde_log[0]); + sock_set_nonblock(pipe_lde_log[1]); + sock_set_cloexec(pipe_lde_log[1]); /* start children */ lde_pid = start_child(PROC_LDE_ENGINE, saved_argv0, - pipe_parent2lde[1], pipe_parent2lde_sync[1]); + pipe_parent2lde[1], pipe_parent2lde_sync[1], pipe_lde_log[1]); ldpe_pid = start_child(PROC_LDP_ENGINE, saved_argv0, - pipe_parent2ldpe[1], pipe_parent2ldpe_sync[1]); + pipe_parent2ldpe[1], pipe_parent2ldpe_sync[1], pipe_ldpe_log[1]); master = frr_init(); + /* The two child processes use the zlog_live backend to send their + * messages here, where the actual logging config is then applied. + * Look for zlog_live_open_fd() to find the other end of this. + */ + zlog_recirculate_subscribe(master, pipe_lde_log[0]); + zlog_recirculate_subscribe(master, pipe_ldpe_log[0]); vrf_init(NULL, NULL, NULL, NULL); access_list_init(); @@ -484,7 +503,8 @@ ldpd_shutdown(void) } static pid_t -start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync) +start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync, + int fd_log) { char *argv[7]; int argc = 0, nullfd; @@ -499,6 +519,7 @@ start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync) default: close(fd_async); close(fd_sync); + close(fd_log); return (pid); } @@ -520,6 +541,9 @@ start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync) if (dup2(fd_sync, LDPD_FD_SYNC) == -1) fatal("cannot setup imsg sync fd"); + if (dup2(fd_log, LDPD_FD_LOG) == -1) + fatal("cannot setup zlog fd"); + argv[argc++] = argv0; switch (p) { case PROC_MAIN: @@ -569,9 +593,6 @@ static void main_dispatch_ldpe(struct event *thread) break; switch (imsg.hdr.type) { - case IMSG_LOG: - logit(imsg.hdr.pid, "%s", (const char *)imsg.data); - break; case IMSG_REQUEST_SOCKETS: af = imsg.hdr.pid; main_imsg_send_net_sockets(af); @@ -637,9 +658,6 @@ static void main_dispatch_lde(struct event *thread) break; switch (imsg.hdr.type) { - case IMSG_LOG: - logit(imsg.hdr.pid, "%s", (const char *)imsg.data); - break; case IMSG_KLABEL_CHANGE: if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(struct kroute)) diff --git a/ldpd/ldpd.h b/ldpd/ldpd.h index 81c6ba3ccd..ad831a6ea3 100644 --- a/ldpd/ldpd.h +++ b/ldpd/ldpd.h @@ -30,6 +30,7 @@ #define LDPD_FD_ASYNC 3 #define LDPD_FD_SYNC 4 +#define LDPD_FD_LOG 5 #define LDPD_OPT_VERBOSE 0x00000001 #define LDPD_OPT_VERBOSE2 0x00000002 @@ -139,7 +140,6 @@ enum imsg_type { IMSG_RECONF_L2VPN_IPW, IMSG_RECONF_END, IMSG_DEBUG_UPDATE, - IMSG_LOG, IMSG_ACL_CHECK, IMSG_INIT, IMSG_PW_UPDATE, diff --git a/ldpd/ldpe.c b/ldpd/ldpe.c index e66b9e92dd..6e844c0aa1 100644 --- a/ldpd/ldpe.c +++ b/ldpd/ldpe.c @@ -23,6 +23,7 @@ #include "privs.h" #include "sigevent.h" #include "libfrr.h" +#include "zlog_live.h" static void ldpe_shutdown(void); static void ldpe_dispatch_main(struct event *thread); @@ -93,6 +94,8 @@ char *pkt_ptr; /* packet buffer */ void ldpe(void) { + static struct zlog_live_cfg child_log; + #ifdef HAVE_SETPROCTITLE setproctitle("ldp engine"); #endif @@ -100,6 +103,8 @@ ldpe(void) log_procname = log_procnames[ldpd_process]; master = frr_init(); + zlog_live_open_fd(&child_log, LOG_DEBUG, LDPD_FD_LOG); + /* no frr_config_fork() here, allow frr_pthread to create threads */ frr_is_after_fork = true; diff --git a/ldpd/log.c b/ldpd/log.c deleted file mode 100644 index 7c4d782dcf..0000000000 --- a/ldpd/log.c +++ /dev/null @@ -1,138 +0,0 @@ -// SPDX-License-Identifier: ISC -/* $OpenBSD$ */ - -/* - * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org> - */ - -#include <zebra.h> - -#include "ldpd.h" -#include "ldpe.h" -#include "lde.h" -#include "log.h" -#include "printfrr.h" - -#include <lib/log.h> - -const char *log_procname; - -void -logit(int pri, const char *fmt, ...) -{ - va_list ap; - - va_start(ap, fmt); - vlog(pri, fmt, ap); - va_end(ap); -} - -void -vlog(int pri, const char *fmt, va_list ap) -{ - char buf[1024]; - - switch (ldpd_process) { - case PROC_LDE_ENGINE: - vsnprintfrr(buf, sizeof(buf), fmt, ap); - lde_imsg_compose_parent_sync(IMSG_LOG, pri, buf, strlen(buf) + 1); - break; - case PROC_LDP_ENGINE: - vsnprintfrr(buf, sizeof(buf), fmt, ap); - ldpe_imsg_compose_parent_sync(IMSG_LOG, pri, buf, strlen(buf) + 1); - break; - case PROC_MAIN: - vzlog(pri, fmt, ap); - break; - } -} - -void -log_warn(const char *emsg, ...) -{ - char *nfmt; - va_list ap; - - /* best effort to even work in out of memory situations */ - if (emsg == NULL) - logit(LOG_ERR, "%s", strerror(errno)); - else { - va_start(ap, emsg); - - if (asprintf(&nfmt, "%s: %s", emsg, strerror(errno)) == -1) { - /* we tried it... */ - vlog(LOG_ERR, emsg, ap); - logit(LOG_ERR, "%s", strerror(errno)); - } else { -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat-nonliteral" - /* format extended above */ - vlog(LOG_ERR, nfmt, ap); -#pragma GCC diagnostic pop - free(nfmt); - } - va_end(ap); - } -} - -void -log_warnx(const char *emsg, ...) -{ - va_list ap; - - va_start(ap, emsg); - vlog(LOG_ERR, emsg, ap); - va_end(ap); -} - -void -log_info(const char *emsg, ...) -{ - va_list ap; - - va_start(ap, emsg); - vlog(LOG_INFO, emsg, ap); - va_end(ap); -} - -void -log_notice(const char *emsg, ...) -{ - va_list ap; - - va_start(ap, emsg); - vlog(LOG_NOTICE, emsg, ap); - va_end(ap); -} - -void -log_debug(const char *emsg, ...) -{ - va_list ap; - - va_start(ap, emsg); - vlog(LOG_DEBUG, emsg, ap); - va_end(ap); -} - -void -fatal(const char *emsg) -{ - if (emsg == NULL) - logit(LOG_CRIT, "fatal in %s: %s", log_procname, strerror(errno)); - else - if (errno) - logit(LOG_CRIT, "fatal in %s: %s: %s", - log_procname, emsg, strerror(errno)); - else - logit(LOG_CRIT, "fatal in %s: %s", log_procname, emsg); - - exit(1); -} - -void -fatalx(const char *emsg) -{ - errno = 0; - fatal(emsg); -} diff --git a/ldpd/log.h b/ldpd/log.h index 641ad8ac5e..aa6f700608 100644 --- a/ldpd/log.h +++ b/ldpd/log.h @@ -8,29 +8,30 @@ #ifndef LOG_H #define LOG_H -#include <stdarg.h> +#include "log.h" +#include "assert.h" extern const char *log_procname; -void logit(int, const char *, ...) - __attribute__((__format__ (printf, 2, 3))); -void vlog(int, const char *, va_list) - __attribute__((__format__ (printf, 2, 0))); -void log_warn(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_warnx(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_info(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_notice(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_debug(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void fatal(const char *) - __attribute__ ((noreturn)) - __attribute__((__format__ (printf, 1, 0))); -void fatalx(const char *) - __attribute__ ((noreturn)) - __attribute__((__format__ (printf, 1, 0))); +#define log_warnx zlog_err /* yes this is poorly named */ +#define log_warn zlog_warn +#define log_info zlog_info +#define log_notice zlog_notice /* not used anywhere */ +#define log_debug zlog_debug + +#define fatal(msg) \ + do { \ + assertf(0, "fatal in %s: %pSQq (%m)", log_procname, \ + (const char *)msg); \ + __builtin_unreachable(); \ + } while (0) \ + /* end */ +#define fatalx(msg) \ + do { \ + assertf(0, "fatal in %s: %pSQq", log_procname, \ + (const char *)msg); \ + __builtin_unreachable(); \ + } while (0) \ + /* end */ #endif /* LOG_H */ diff --git a/ldpd/subdir.am b/ldpd/subdir.am index 0b948adb6f..ad5933fec3 100644 --- a/ldpd/subdir.am +++ b/ldpd/subdir.am @@ -28,7 +28,6 @@ ldpd_libldp_a_SOURCES = \ ldpd/ldp_vty_exec.c \ ldpd/ldp_zebra.c \ ldpd/ldpe.c \ - ldpd/log.c \ ldpd/logmsg.c \ ldpd/neighbor.c \ ldpd/notification.c \ diff --git a/lib/affinitymap.c b/lib/affinitymap.c index e53d54a443..6ff8e83f91 100644 --- a/lib/affinitymap.c +++ b/lib/affinitymap.c @@ -41,8 +41,6 @@ #include "jhash.h" DEFINE_MTYPE_STATIC(LIB, AFFINITY_MAP, "Affinity map"); -DEFINE_MTYPE(LIB, AFFINITY_MAP_NAME, "Affinity map name"); -DEFINE_MTYPE_STATIC(LIB, AFFINITY_MAP_INDEX, "Affinity map index"); DEFINE_QOBJ_TYPE(affinity_maps); DEFINE_QOBJ_TYPE(affinity_map); diff --git a/lib/mgmt_msg_native.c b/lib/mgmt_msg_native.c index d27c5d3a29..98b7da45ce 100644 --- a/lib/mgmt_msg_native.c +++ b/lib/mgmt_msg_native.c @@ -9,7 +9,6 @@ #include "mgmt_msg_native.h" DEFINE_MGROUP(MSG_NATIVE, "Native message allocations"); -DEFINE_MTYPE(MSG_NATIVE, MSG_NATIVE_MSG, "native mgmt msg"); DEFINE_MTYPE(MSG_NATIVE, MSG_NATIVE_ERROR, "native error msg"); DEFINE_MTYPE(MSG_NATIVE, MSG_NATIVE_GET_TREE, "native get tree msg"); DEFINE_MTYPE(MSG_NATIVE, MSG_NATIVE_TREE_DATA, "native tree data msg"); diff --git a/lib/netns_other.c b/lib/netns_other.c index 30218409dd..545a962b55 100644 --- a/lib/netns_other.c +++ b/lib/netns_other.c @@ -13,10 +13,6 @@ #include "log.h" #include "memory.h" -DEFINE_MTYPE_STATIC(LIB, NS, "NetNS Context"); -DEFINE_MTYPE_STATIC(LIB, NS_NAME, "NetNS Name"); - - static inline int ns_compare(const struct ns *ns, const struct ns *ns2); RB_GENERATE(ns_head, ns, entry, ns_compare) @@ -39,7 +35,6 @@ void ns_init_management(ns_id_t ns_id) { } - /* * NS utilities */ diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index 4942d66850..640334926d 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -19,8 +19,6 @@ #include <sysrepo/values.h> #include <sysrepo/xpath.h> -DEFINE_MTYPE_STATIC(LIB, SYSREPO, "Sysrepo module"); - static struct debug nb_dbg_client_sysrepo = {0, "Northbound client: Sysrepo"}; static struct event_loop *master; diff --git a/lib/printf/README b/lib/printf/README index 46d51cec6a..3c3ef74ca8 100644 --- a/lib/printf/README +++ b/lib/printf/README @@ -1,6 +1,7 @@ This is the printf implementation from FreeBSD. The history of this code is: - imported on 2019-05-12, from SVN revision 347514 - resynced on 2023-09-03, to pick up `%b` implementation +- resynced on 2024-03-10, to pick up `%w[f](8|16|32|64)d` implementation Please don't reindent or otherwise mangle the files to make importing fixes easy (not that there are significant changes likely to happen...) diff --git a/lib/printf/printflocal.h b/lib/printf/printflocal.h index 4b030912fe..93318c8fdb 100644 --- a/lib/printf/printflocal.h +++ b/lib/printf/printflocal.h @@ -52,6 +52,7 @@ #define PTRDIFFT 0x800 /* ptrdiff_t */ #define INTMAXT 0x1000 /* intmax_t */ #define CHARINT 0x2000 /* print char using int format */ +#define FASTINT 0x4000 /* int_fastN_t */ /* * Macros for converting digits to letters and vice versa diff --git a/lib/printf/vfprintf.c b/lib/printf/vfprintf.c index 2083642d5e..3f6700c838 100644 --- a/lib/printf/vfprintf.c +++ b/lib/printf/vfprintf.c @@ -416,6 +416,49 @@ reswitch: switch (ch) { case 't': flags |= PTRDIFFT; goto rflag; + case 'w': + /* + * Fixed-width integer types. On all platforms we + * support, int8_t is equivalent to char, int16_t + * is equivalent to short, int32_t is equivalent + * to int, int64_t is equivalent to long long int. + * Furthermore, int_fast8_t, int_fast16_t and + * int_fast32_t are equivalent to int, and + * int_fast64_t is equivalent to long long int. + */ + flags &= ~(CHARINT|SHORTINT|LONGINT|LLONGINT|INTMAXT); + if (fmt[0] == 'f') { + flags |= FASTINT; + fmt++; + } else { + flags &= ~FASTINT; + } + if (fmt[0] == '8') { + if (!(flags & FASTINT)) + flags |= CHARINT; + else + (void) 0; /* no flag set = 32 */ + fmt += 1; + } else if (fmt[0] == '1' && fmt[1] == '6') { + if (!(flags & FASTINT)) + flags |= SHORTINT; + else + (void) 0; /* no flag set = 32 */ + fmt += 2; + } else if (fmt[0] == '3' && fmt[1] == '2') { + /* no flag set = 32 */ + fmt += 2; + } else if (fmt[0] == '6' && fmt[1] == '4') { + flags |= LLONGINT; + fmt += 2; + } else { + if (flags & FASTINT) { + flags &= ~FASTINT; + fmt--; + } + goto invalid; + } + goto rflag; case 'z': flags |= SIZET; goto rflag; @@ -684,6 +727,7 @@ number: if ((dprec = prec) >= 0) default: /* "%?" prints ?, unless ? is NUL */ if (ch == '\0') goto done; +invalid: /* pretend it was %c with argument ch */ buf[0] = ch; cp = buf; diff --git a/lib/subdir.am b/lib/subdir.am index db11def741..c621ad0658 100644 --- a/lib/subdir.am +++ b/lib/subdir.am @@ -131,6 +131,7 @@ lib_libfrr_la_SOURCES = \ lib/zlog_5424.c \ lib/zlog_5424_cli.c \ lib/zlog_live.c \ + lib/zlog_recirculate.c \ lib/zlog_targets.c \ lib/printf/printf-pos.c \ lib/printf/vfprintf.c \ @@ -329,6 +330,7 @@ pkginclude_HEADERS += \ lib/zlog.h \ lib/zlog_5424.h \ lib/zlog_live.h \ + lib/zlog_recirculate.h \ lib/zlog_targets.h \ lib/pbr.h \ lib/tc.h \ diff --git a/lib/yang.c b/lib/yang.c index 03044fc29e..d71cb2f498 100644 --- a/lib/yang.c +++ b/lib/yang.c @@ -199,6 +199,16 @@ next: if (ret == YANG_ITER_STOP) return ret; } + LY_LIST_FOR ((const struct lysc_node *)lysc_node_notifs(snode), child) { + ret = yang_snodes_iterate_subtree(child, module, cb, flags, arg); + if (ret == YANG_ITER_STOP) + return ret; + } + LY_LIST_FOR ((const struct lysc_node *)lysc_node_actions(snode), child) { + ret = yang_snodes_iterate_subtree(child, module, cb, flags, arg); + if (ret == YANG_ITER_STOP) + return ret; + } return ret; } diff --git a/lib/zlog.c b/lib/zlog.c index 8734fd55af..ffca572609 100644 --- a/lib/zlog.c +++ b/lib/zlog.c @@ -50,6 +50,7 @@ #include "printfrr.h" #include "frrcu.h" #include "zlog.h" +#include "zlog_live.h" #include "libfrr_trace.h" #include "frrevent.h" @@ -109,6 +110,9 @@ struct zlog_msg { size_t textlen; size_t hdrlen; + /* for relayed log messages ONLY (cf. zlog_recirculate_live_msg) */ + intmax_t pid, tid; + /* This is always ISO8601 with sub-second precision 9 here, it's * converted for callers as needed. ts_dot points to the "." * separating sub-seconds. ts_zonetail is "Z" or "+00:00" for the @@ -357,6 +361,16 @@ void zlog_msg_pid(struct zlog_msg *msg, intmax_t *pid, intmax_t *tid) { #ifndef __OpenBSD__ static thread_local intmax_t cached_pid = -1; +#endif + + /* recirculated messages */ + if (msg->pid) { + *pid = msg->pid; + *tid = msg->tid; + return; + } + +#ifndef __OpenBSD__ if (cached_pid != -1) *pid = cached_pid; else @@ -507,6 +521,89 @@ static void vzlog_tls(struct zlog_tls *zlog_tls, const struct xref_logmsg *xref, XFREE(MTYPE_LOG_MESSAGE, msg->text); } +/* reinject log message received by zlog_recirculate_recv(). As of writing, + * only used in the ldpd parent process to proxy messages from lde/ldpe + * subprocesses. + */ +void zlog_recirculate_live_msg(uint8_t *data, size_t len) +{ + struct zlog_target *zt; + struct zlog_msg stackmsg = {}, *msg = &stackmsg; + struct zlog_live_hdr *hdr; + struct xrefdata *xrefdata, ref = {}; + + if (len < sizeof(*hdr)) + return; + + hdr = (struct zlog_live_hdr *)data; + if (hdr->hdrlen < sizeof(*hdr)) + return; + data += hdr->hdrlen; + len -= sizeof(*hdr); + + msg->ts.tv_sec = hdr->ts_sec; + msg->ts.tv_nsec = hdr->ts_nsec; + msg->pid = hdr->pid; + msg->tid = hdr->tid; + msg->prio = hdr->prio; + + if (hdr->textlen > len) + return; + msg->textlen = hdr->textlen; + msg->hdrlen = hdr->texthdrlen; + msg->text = (char *)data; + + /* caller needs to make sure we have a trailing \n\0, it's not + * transmitted on zlog_live + */ + if (msg->text[msg->textlen] != '\n' || + msg->text[msg->textlen + 1] != '\0') + return; + + static_assert(sizeof(msg->argpos[0]) == sizeof(hdr->argpos[0]), + "in-memory struct doesn't match on-wire variant"); + msg->n_argpos = MIN(hdr->n_argpos, array_size(msg->argpos)); + memcpy(msg->argpos, hdr->argpos, msg->n_argpos * sizeof(msg->argpos[0])); + + /* This will only work if we're in the same daemon: we received a log + * message uid and are now doing a lookup in *our* known uids to find + * it. This works for ldpd because it's the same binary containing the + * same log messages, and ldpd is the only use case right now. + * + * When the uid is not found, the log message uid is lost but the + * message itself is still processed correctly. If this is needed, + * this can be made to work in two ways: + * (a) synthesize a temporary xref_logmsg from the received data. + * This is a bit annoying due to lifetimes with per-thread buffers. + * (b) extract and aggregate all log messages. This already happens + * with frr.xref but that would need to be fed back in. + */ + strlcpy(ref.uid, hdr->uid, sizeof(ref.uid)); + xrefdata = xrefdata_uid_find(&xrefdata_uid, &ref); + + if (xrefdata && xrefdata->xref->type == XREFT_LOGMSG) { + struct xref_logmsg *xref_logmsg; + + xref_logmsg = (struct xref_logmsg *)xrefdata->xref; + msg->xref = xref_logmsg; + msg->fmt = xref_logmsg->fmtstring; + } else { + /* fake out format string... */ + msg->fmt = msg->text + hdr->texthdrlen; + } + + rcu_read_lock(); + frr_each_safe (zlog_targets, &zlog_targets, zt) { + if (msg->prio > zt->prio_min) + continue; + if (!zt->logfn) + continue; + + zt->logfn(zt, &msg, 1); + } + rcu_read_unlock(); +} + static void zlog_backtrace_msg(const struct xref_logmsg *xref, int prio) { struct event *tc = pthread_getspecific(thread_current); diff --git a/lib/zlog.h b/lib/zlog.h index 27401d51fb..9d93979957 100644 --- a/lib/zlog.h +++ b/lib/zlog.h @@ -125,6 +125,9 @@ static inline void zlog_ref(const struct xref_logmsg *xref, extern void zlog_sigsafe(const char *text, size_t len); +/* recirculate a log message from zlog_live */ +extern void zlog_recirculate_live_msg(uint8_t *data, size_t len); + /* extra priority value to disable a target without deleting it */ #define ZLOG_DISABLED (LOG_EMERG-1) diff --git a/lib/zlog_recirculate.c b/lib/zlog_recirculate.c new file mode 100644 index 0000000000..abc73eeed0 --- /dev/null +++ b/lib/zlog_recirculate.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: ISC +/* + * Copyright (c) 2024 David Lamparter, for NetDEF, Inc. + */ + +#include "zebra.h" + +#include "log.h" +#include "frrevent.h" + +#include "zlog_recirculate.h" + +/* This is only the event loop part; it's split off from + * zlog_recirculate_live_msg since there's an integration boundary; this + * half deals with events, the other half with zlog interna. + * + * As of writing, this runs in ldpd in the *parent* process and receives log + * messages from the lde/ldpe subprocesses. It is not used anywhere else + * (yet?) + */ +static void zlog_recirculate_recv(struct event *ev) +{ + uint8_t rxbuf[4096]; + ssize_t n_rd; + int fd = EVENT_FD(ev); + + /* see below for -2, "\n\0" are added */ + n_rd = read(fd, rxbuf, sizeof(rxbuf) - 2); + if (n_rd == 0) { + /* EOF */ + close(fd); + /* event_add_read not called yet, nothing to cancel */ + return; + } + if (n_rd < 0 && (errno != EAGAIN) && (errno != EWOULDBLOCK)) { + /* error */ + zlog_warn("error on log relay socket %d: %m", fd); + close(fd); + /* event_add_read not called yet, nothing to cancel */ + return; + } + + event_add_read(ev->master, zlog_recirculate_recv, NULL, fd, NULL); + if (n_rd < 0) + return; + + /* log infrastructure has an implicit \n\0 at the end */ + rxbuf[n_rd] = '\n'; + rxbuf[n_rd + 1] = '\0'; + zlog_recirculate_live_msg(rxbuf, n_rd); +} + +void zlog_recirculate_subscribe(struct event_loop *el, int fd) +{ + event_add_read(el, zlog_recirculate_recv, NULL, fd, NULL); +} diff --git a/lib/zlog_recirculate.h b/lib/zlog_recirculate.h new file mode 100644 index 0000000000..a2ddb4e172 --- /dev/null +++ b/lib/zlog_recirculate.h @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: ISC +/* + * Copyright (c) 2024 David Lamparter, for NetDEF, Inc. + */ + +#ifndef _FRR_ZLOG_RECIRCULATE_H +#define _FRR_ZLOG_RECIRCULATE_H + +/* fd should be one end of a socketpair() */ +extern void zlog_recirculate_subscribe(struct event_loop *tm, int fd); + +#endif diff --git a/ripd/rip_cli.c b/ripd/rip_cli.c index 2e967698ea..4d4349b81a 100644 --- a/ripd/rip_cli.c +++ b/ripd/rip_cli.c @@ -1125,36 +1125,6 @@ void cli_show_ip_rip_bfd_profile(struct vty *vty, const struct lyd_node *dnode, yang_dnode_get_string(dnode, NULL)); } -/* - * XPath: /frr-ripd:clear-rip-route - */ -DEFPY_YANG (clear_ip_rip, - clear_ip_rip_cmd, - "clear ip rip [vrf WORD]", - CLEAR_STR - IP_STR - "Clear IP RIP database\n" - VRF_CMD_HELP_STR) -{ - struct list *input; - int ret; - - input = list_new(); - if (vrf) { - struct yang_data *yang_vrf; - - yang_vrf = yang_data_new("/frr-ripd:clear-rip-route/input/vrf", - vrf); - listnode_add(input, yang_vrf); - } - - ret = nb_cli_rpc(vty, "/frr-ripd:clear-rip-route", input, NULL); - - list_delete(&input); - - return ret; -} - DEFPY_YANG( rip_distribute_list, rip_distribute_list_cmd, "distribute-list ACCESSLIST4_NAME$name <in|out>$dir [WORD$ifname]", @@ -1325,8 +1295,6 @@ void rip_cli_init(void) install_element(INTERFACE_NODE, &ip_rip_bfd_profile_cmd); install_element(INTERFACE_NODE, &no_ip_rip_bfd_profile_cmd); - install_element(ENABLE_NODE, &clear_ip_rip_cmd); - if_rmap_init(RIP_NODE); } /* clang-format off */ diff --git a/ripd/ripd.c b/ripd/ripd.c index b8a140c9ca..e3220a9267 100644 --- a/ripd/ripd.c +++ b/ripd/ripd.c @@ -3254,6 +3254,38 @@ DEFUN (show_ip_rip_status, return CMD_SUCCESS; } +#include "ripd/ripd_clippy.c" + +/* + * XPath: /frr-ripd:clear-rip-route + */ +DEFPY_YANG (clear_ip_rip, + clear_ip_rip_cmd, + "clear ip rip [vrf WORD]", + CLEAR_STR + IP_STR + "Clear IP RIP database\n" + VRF_CMD_HELP_STR) +{ + struct list *input; + int ret; + + input = list_new(); + if (vrf) { + struct yang_data *yang_vrf; + + yang_vrf = yang_data_new("/frr-ripd:clear-rip-route/input/vrf", + vrf); + listnode_add(input, yang_vrf); + } + + ret = nb_cli_rpc(vty, "/frr-ripd:clear-rip-route", input, NULL); + + list_delete(&input); + + return ret; +} + /* Distribute-list update functions. */ static void rip_distribute_update(struct distribute_ctx *ctx, struct distribute *dist) @@ -3628,6 +3660,7 @@ void rip_init(void) /* Install rip commands. */ install_element(VIEW_NODE, &show_ip_rip_cmd); install_element(VIEW_NODE, &show_ip_rip_status_cmd); + install_element(ENABLE_NODE, &clear_ip_rip_cmd); /* Debug related init. */ rip_debug_init(); diff --git a/ripd/subdir.am b/ripd/subdir.am index aed8d249fe..7fb3726077 100644 --- a/ripd/subdir.am +++ b/ripd/subdir.am @@ -33,6 +33,7 @@ ripd_ripd_SOURCES = \ clippy_scan += \ ripd/rip_bfd.c \ ripd/rip_cli.c \ + ripd/ripd.c \ # end noinst_HEADERS += \ diff --git a/ripngd/ripng_cli.c b/ripngd/ripng_cli.c index ed460a239e..4806861fe0 100644 --- a/ripngd/ripng_cli.c +++ b/ripngd/ripng_cli.c @@ -512,36 +512,6 @@ void cli_show_ipv6_ripng_split_horizon(struct vty *vty, } } -/* - * XPath: /frr-ripngd:clear-ripng-route - */ -DEFPY_YANG (clear_ipv6_rip, - clear_ipv6_rip_cmd, - "clear ipv6 ripng [vrf WORD]", - CLEAR_STR - IPV6_STR - "Clear IPv6 RIP database\n" - VRF_CMD_HELP_STR) -{ - struct list *input; - int ret; - - input = list_new(); - if (vrf) { - struct yang_data *yang_vrf; - - yang_vrf = yang_data_new( - "/frr-ripngd:clear-ripng-route/input/vrf", vrf); - listnode_add(input, yang_vrf); - } - - ret = nb_cli_rpc(vty, "/frr-ripngd:clear-ripng-route", input, NULL); - - list_delete(&input); - - return ret; -} - DEFPY_YANG( ripng_ipv6_distribute_list, ripng_ipv6_distribute_list_cmd, "ipv6 distribute-list ACCESSLIST6_NAME$name <in|out>$dir [WORD$ifname]", @@ -693,8 +663,6 @@ void ripng_cli_init(void) install_element(INTERFACE_NODE, &ipv6_ripng_split_horizon_cmd); - install_element(ENABLE_NODE, &clear_ipv6_rip_cmd); - if_rmap_init(RIPNG_NODE); } diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c index f4dadf377d..80b9013e0f 100644 --- a/ripngd/ripngd.c +++ b/ripngd/ripngd.c @@ -2231,6 +2231,38 @@ DEFUN (show_ipv6_ripng_status, return CMD_SUCCESS; } +#include "ripngd/ripngd_clippy.c" + +/* + * XPath: /frr-ripngd:clear-ripng-route + */ +DEFPY_YANG (clear_ipv6_rip, + clear_ipv6_rip_cmd, + "clear ipv6 ripng [vrf WORD]", + CLEAR_STR + IPV6_STR + "Clear IPv6 RIP database\n" + VRF_CMD_HELP_STR) +{ + struct list *input; + int ret; + + input = list_new(); + if (vrf) { + struct yang_data *yang_vrf; + + yang_vrf = yang_data_new( + "/frr-ripngd:clear-ripng-route/input/vrf", vrf); + listnode_add(input, yang_vrf); + } + + ret = nb_cli_rpc(vty, "/frr-ripngd:clear-ripng-route", input, NULL); + + list_delete(&input); + + return ret; +} + /* Update ECMP routes to zebra when ECMP is disabled. */ void ripng_ecmp_disable(struct ripng *ripng) { @@ -2648,6 +2680,7 @@ void ripng_init(void) /* Install ripng commands. */ install_element(VIEW_NODE, &show_ipv6_ripng_cmd); install_element(VIEW_NODE, &show_ipv6_ripng_status_cmd); + install_element(ENABLE_NODE, &clear_ipv6_rip_cmd); ripng_if_init(); ripng_debug_init(); diff --git a/ripngd/subdir.am b/ripngd/subdir.am index 83e376b555..a88114432d 100644 --- a/ripngd/subdir.am +++ b/ripngd/subdir.am @@ -27,6 +27,7 @@ ripngd_ripngd_SOURCES = \ clippy_scan += \ ripngd/ripng_cli.c \ + ripngd/ripngd.c \ # end noinst_HEADERS += \ diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 66699ec7c0..cefa07ec73 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -143,6 +143,8 @@ int main(int argc, char **argv) NAN, }; uint64_t ui64 = 0xfeed1278cafef00d; + uint16_t i16 = -23456; + int_fast8_t if8 = 123; struct in_addr ip; char *p; char buf[256]; @@ -169,6 +171,16 @@ int main(int argc, char **argv) FMT_NSTD(printchk("11110000000011111010010111000011", "%b", 0xf00fa5c3)); FMT_NSTD(printchk("0b01011010", "%#010b", 0x5a)); +/* FMT_NSTD is conditional on the frr-format plugin being NOT enabled. + * However, the frr-format plugin does not support %wd/%wfd yet, so this needs + * to be unconditional. + */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat" + printchk("123 -23456 feed1278cafef00d 9876", "%wf8d %w16d %w64x %d", + if8, i16, ui64, 9876); +#pragma GCC diagnostic pop + inet_aton("192.168.1.2", &ip); printchk("192.168.1.2", "%pI4", &ip); printchk(" 192.168.1.2", "%20pI4", &ip); diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py index f83ee2971c..5202f51e28 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_addpath.py @@ -135,7 +135,7 @@ def test_bgp_dynamic_capability_addpath(): step("Disable Addpath capability RX and check if it's exchanged dynamically") # Clear message stats to check if we receive a notification or not after we - # change the settings fo LLGR. + # disable addpath-rx. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r2.vtysh_cmd( """ @@ -174,6 +174,46 @@ def test_bgp_dynamic_capability_addpath(): _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Session was reset after disabling Addpath RX flags" + # Clear message stats to check if we receive a notification or not after we + # disable Addpath capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + address-family ipv4 unicast + no neighbor 192.168.1.2 addpath-tx-all-paths + """ + ) + + def _bgp_check_if_addpath_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "addPath": { + "ipv4Unicast": { + "txAdvertisedAndReceived": None, + "txAdvertised": None, + "rxAdvertised": True, + } + }, + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_addpath_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable Addpath capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_fqdn.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_fqdn.py new file mode 100644 index 0000000000..338886d5f4 --- /dev/null +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_fqdn.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2024 by +# Donatas Abraitis <donatas@opensourcerouting.org> +# + +""" +Test if fqdn capability is exchanged dynamically. +""" + +import os +import re +import sys +import json +import pytest +import functools + +pytestmark = pytest.mark.bgpd + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.common_config import step + +pytestmark = [pytest.mark.bgpd] + + +def setup_module(mod): + topodef = {"s1": ("r1", "r2")} + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_dynamic_capability_fqdn(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r2 = tgen.gears["r2"] + + def _bgp_converge(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "hostName": { + "advHostName": "r1", + "rcvHostName": "r2", + }, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Can't converge" + + step("Disable fqdn capability and check if it's exchanged dynamically") + + # Clear message stats to check if we receive a notification or not after we + # disable fqdn capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + no neighbor 192.168.1.2 capability fqdn + """ + ) + + def _bgp_check_if_fqdn_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "hostName": { + "advHostName": None, + "rcvHostName": "r2", + }, + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_fqdn_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable fqdn capability" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py index b7e2090eee..4644ef3293 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_graceful_restart.py @@ -159,7 +159,7 @@ def test_bgp_dynamic_capability_graceful_restart(): ) # Clear message stats to check if we receive a notification or not after we - # change the settings fo LLGR. + # disable graceful-restart notification support. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r2.vtysh_cmd( """ @@ -207,6 +207,40 @@ def test_bgp_dynamic_capability_graceful_restart(): result is None ), "Session was reset after changing Graceful-Restart notification support" + # Clear message stats to check if we receive a notification or not after we + # disable GR. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + bgp graceful-restart-disable + """ + ) + + def _bgp_check_if_gr_llgr_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "gracefulRestartCapability": "received", + "longLivedGracefulRestart": "received", + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_gr_llgr_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable GR/LLGR capabilities" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py index f1ad74c05c..ba95bd1614 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_orf.py @@ -172,6 +172,51 @@ def test_bgp_dynamic_capability_orf(): result is None ), "Only 10.10.10.20/32 SHOULD be advertised due to ORF filtering" + # Clear message stats to check if we receive a notification or not after we + # disable ORF capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + address-family ipv4 unicast + no neighbor 192.168.1.2 capability orf prefix-list both + """ + ) + + def _bgp_check_if_orf_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + }, + "messageStats": { + "notificationsRecv": 0, + "notificationsSent": 0, + }, + "addressFamilyInfo": { + "ipv4Unicast": { + "acceptedPrefixCounter": 1, + "afDependentCap": { + "orfPrefixList": { + "sendMode": "received", + "recvMode": "received", + } + }, + } + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_orf_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable ORF capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py index 700d4c130d..aa9ad5f0e8 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_role.py @@ -84,7 +84,7 @@ def test_bgp_dynamic_capability_role(): step("Set local-role and check if it's exchanged dynamically") # Clear message stats to check if we receive a notification or not after we - # change the settings fo LLGR. + # change the role. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r1.vtysh_cmd( """ @@ -132,6 +132,41 @@ def test_bgp_dynamic_capability_role(): _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) assert result is None, "Session was reset after setting role capability" + # Clear message stats to check if we receive a notification or not after we + # change the role. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + no neighbor 192.168.1.2 local-role customer + """ + ) + + def _bgp_check_if_role_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "localRole": "undefined", + "remoteRole": "provider", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "role": "received", + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_role_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable role capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py index 11840b4c61..737e694701 100644 --- a/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py +++ b/tests/topotests/bgp_dynamic_capability/test_bgp_dynamic_capability_software_version.py @@ -86,7 +86,7 @@ def test_bgp_dynamic_capability_software_version(): step("Enable software version capability and check if it's exchanged dynamically") # Clear message stats to check if we receive a notification or not after we - # change the settings fo LLGR. + # enable software-version capability. r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") r1.vtysh_cmd( """ @@ -155,6 +155,41 @@ def test_bgp_dynamic_capability_software_version(): result is None ), "Session was reset after enabling software version capability" + # Clear message stats to check if we receive a notification or not after we + # disable software-version capability. + r1.vtysh_cmd("clear bgp 192.168.1.2 message-stats") + r1.vtysh_cmd( + """ + configure terminal + router bgp + no neighbor 192.168.1.2 capability software-version + """ + ) + + def _bgp_check_if_software_version_capability_is_absent(): + output = json.loads(r1.vtysh_cmd("show bgp neighbor json")) + expected = { + "192.168.1.2": { + "bgpState": "Established", + "neighborCapabilities": { + "dynamic": "advertisedAndReceived", + "softwareVersion": { + "advertisedSoftwareVersion": None, + }, + }, + "messageStats": { + "notificationsRecv": 0, + }, + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_check_if_software_version_capability_is_absent, + ) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "Failed to disable software version capability" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/zebra/main.c b/zebra/main.c index 606ecc7279..27e98ed999 100644 --- a/zebra/main.c +++ b/zebra/main.c @@ -203,6 +203,7 @@ static void sigint(void) rib_update_finish(); list_delete(&zrouter.client_list); + list_delete(&zrouter.stale_client_list); /* Indicate that all new dplane work has been enqueued. When that * work is complete, the dataplane will enqueue an event diff --git a/zebra/zserv.c b/zebra/zserv.c index 6a64176d98..3671fdb507 100644 --- a/zebra/zserv.c +++ b/zebra/zserv.c @@ -306,6 +306,14 @@ zwrite_fail: * this task reschedules itself. * * Any failure in any of these actions is handled by terminating the client. + * + * The client's input buffer ibuf_fifo can have a maximum items as configured + * in the packets_to_process. This way we are not filling up the FIFO more + * than the maximum when the zebra main is busy. If the fifo has space, we + * reschedule ourselves to read more. + * + * The main thread processes the items in ibuf_fifo and always signals the + * client IO thread. */ static void zserv_read(struct event *thread) { @@ -313,15 +321,25 @@ static void zserv_read(struct event *thread) int sock; size_t already; struct stream_fifo *cache; - uint32_t p2p_orig; - - uint32_t p2p; + uint32_t p2p; /* Temp p2p used to process */ + uint32_t p2p_orig; /* Configured p2p (Default-1000) */ + int p2p_avail; /* How much space is available for p2p */ struct zmsghdr hdr; + size_t client_ibuf_fifo_cnt = stream_fifo_count_safe(client->ibuf_fifo); p2p_orig = atomic_load_explicit(&zrouter.packets_to_process, memory_order_relaxed); + p2p_avail = p2p_orig - client_ibuf_fifo_cnt; + + /* + * Do nothing if ibuf_fifo count has reached its max limit. Otherwise + * proceed and reschedule ourselves if there is space in the ibuf_fifo. + */ + if (p2p_avail <= 0) + return; + + p2p = p2p_avail; cache = stream_fifo_new(); - p2p = p2p_orig; sock = EVENT_FD(thread); while (p2p) { @@ -421,7 +439,7 @@ static void zserv_read(struct event *thread) p2p--; } - if (p2p < p2p_orig) { + if (p2p < (uint32_t)p2p_avail) { uint64_t time_now = monotime(NULL); /* update session statistics */ @@ -435,19 +453,23 @@ static void zserv_read(struct event *thread) while (cache->head) stream_fifo_push(client->ibuf_fifo, stream_fifo_pop(cache)); + /* Need to update count as main thread could have processed few */ + client_ibuf_fifo_cnt = + stream_fifo_count_safe(client->ibuf_fifo); } /* Schedule job to process those packets */ zserv_event(client, ZSERV_PROCESS_MESSAGES); - } if (IS_ZEBRA_DEBUG_PACKET) - zlog_debug("Read %d packets from client: %s", p2p_orig - p2p, - zebra_route_string(client->proto)); + zlog_debug("Read %d packets from client: %s. Current ibuf fifo count: %zu. Conf P2p %d", + p2p_avail - p2p, zebra_route_string(client->proto), + client_ibuf_fifo_cnt, p2p_orig); - /* Reschedule ourselves */ - zserv_client_event(client, ZSERV_CLIENT_READ); + /* Reschedule ourselves since we have space in ibuf_fifo */ + if (client_ibuf_fifo_cnt < p2p_orig) + zserv_client_event(client, ZSERV_CLIENT_READ); stream_fifo_free(cache); @@ -483,14 +505,20 @@ static void zserv_client_event(struct zserv *client, * as the task argument. * * Each message is popped off the client's input queue and the action associated - * with the message is executed. This proceeds until there are no more messages, - * an error occurs, or the processing limit is reached. + * with the message is executed. This proceeds until an error occurs, or the + * processing limit is reached. * * The client's I/O thread can push at most zrouter.packets_to_process messages * onto the input buffer before notifying us there are packets to read. As long * as we always process zrouter.packets_to_process messages here, then we can * rely on the read thread to handle queuing this task enough times to process * everything on the input queue. + * + * If the client ibuf always schedules a wakeup to the client IO to read more + * items from the socked buffer. This way we ensure + * - Client IO thread always tries to read the socket buffer and add more + * items to the ibuf_fifo (until max limit) + * - the hidden config change (zebra zapi-packets <>) is taken into account. */ static void zserv_process_messages(struct event *thread) { @@ -524,6 +552,9 @@ static void zserv_process_messages(struct event *thread) /* Reschedule ourselves if necessary */ if (need_resched) zserv_event(client, ZSERV_PROCESS_MESSAGES); + + /* Ensure to include the read socket in the select/poll/etc.. */ + zserv_client_event(client, ZSERV_CLIENT_READ); } int zserv_send_message(struct zserv *client, struct stream *msg) |