ldpd: use zlog recirculation for subprocesses

This way, full functionality of `zlog_*` is available.

Having `fatal()` be wrappers around `assertf()` also means we get
backtraces, which is not the case for a plain `exit(1)`.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/ldpd/lde.c b/ldpd/lde.c
index ef4aecadadec290a37110a1c3384505c9f3b5f13..876dd4163099492227395dec37d974e264f76a1d 100644 (file)
--- 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 da29a4f20ddf05cc23244f3b4099da61084af568..492a36b3d6a19e0e185e44e84e386295c9ed5bae 100644 (file)
--- 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 81c6ba3ccd57cec31e2a88fcca7b17b0a28b8a2a..ad831a6ea358f2dc457619c8afe1098a3eba4ae9 100644 (file)
--- 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 e66b9e92dd438dbac88e2264e4715978777e68e3..6e844c0aa18d4ccc0cbf8cdd74d676fb046be9d6 100644 (file)
--- 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 (file)
index 7c4d782..0000000
--- 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 641ad8ac5edb6aca28e362e593bcaca2ce6bd623..aa6f700608a7a30c7f31aa1e4f26b4cab03d7464 100644 (file)
--- 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 0b948adb6fc21bb7b6b07bc8677b2650a018492a..ad5933fec36370d2c1b33e63a0df42239be38fb2 100644 (file)
--- 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 \