From 24fb501d7f72cd5de59849331115283a52b25fab Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 27 Sep 2018 04:18:48 +0200 Subject: [PATCH] watchfrr, lib: cleanup & delay detaching This cleans up watchfrr to be more "normal" like the other daemons in terms of what it does in main(), i.e. using the full frr_*() call set. Also, this changes the startup behaviour on watchfrr to stay attached on the daemon's parent process until startup is really complete. This should allow removing the "watchfrr.started" hack at some point. Signed-off-by: David Lamparter --- lib/libfrr.c | 48 +++++++++---- lib/libfrr.h | 16 +++-- watchfrr/watchfrr.c | 164 ++++++++++++++++++-------------------------- watchfrr/watchfrr.h | 4 ++ 4 files changed, 115 insertions(+), 117 deletions(-) diff --git a/lib/libfrr.c b/lib/libfrr.c index 821c57f37b..86ec8b492a 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -54,6 +54,7 @@ static char pidfile_default[512]; static char vtypath_default[256]; bool debug_memstats_at_exit = 0; +static bool nodetach_term, nodetach_daemon; static char comb_optstr[256]; static struct option comb_lo[64]; @@ -281,6 +282,8 @@ void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv) opt_extend(&os_zclient); if (!(di->flags & FRR_NO_TCPVTY)) opt_extend(&os_vty); + if (di->flags & FRR_DETACH_LATER) + nodetach_daemon = true; snprintf(config_default, sizeof(config_default), "%s/%s.conf", frr_sysconfdir, di->name); @@ -765,13 +768,16 @@ void frr_config_fork(void) { hook_call(frr_late_init, master); - /* Don't start execution if we are in dry-run mode */ - if (di->dryrun) { - frr_config_read_in(NULL); - exit(0); - } + if (!(di->flags & FRR_NO_CFG_PID_DRY)) { + /* Don't start execution if we are in dry-run mode */ + if (di->dryrun) { + frr_config_read_in(NULL); + exit(0); + } - thread_add_event(master, frr_config_read_in, NULL, 0, &di->read_in); + thread_add_event(master, frr_config_read_in, NULL, 0, + &di->read_in); + } if (di->daemon_mode || di->terminal) frr_daemonize(); @@ -781,7 +787,7 @@ void frr_config_fork(void) pid_output(di->pid_file); } -void frr_vty_serv(void) +static void frr_vty_serv(void) { /* allow explicit override of vty_path in the future * (not currently set anywhere) */ @@ -808,14 +814,22 @@ void frr_vty_serv(void) vty_serv_sock(di->vty_addr, di->vty_port, di->vty_path); } +static void frr_check_detach(void) +{ + if (nodetach_term || nodetach_daemon) + return; + + if (daemon_ctl_sock != -1) + close(daemon_ctl_sock); + daemon_ctl_sock = -1; +} + static void frr_terminal_close(int isexit) { int nullfd; - if (daemon_ctl_sock != -1) { - close(daemon_ctl_sock); - daemon_ctl_sock = -1; - } + nodetach_term = false; + frr_check_detach(); if (!di->daemon_mode || isexit) { printf("\n%s exiting\n", di->name); @@ -879,6 +893,12 @@ out: return 0; } +void frr_detach(void) +{ + nodetach_daemon = false; + frr_check_detach(); +} + void frr_run(struct thread_master *master) { char instanceinfo[64] = ""; @@ -893,6 +913,8 @@ void frr_run(struct thread_master *master) instanceinfo, di->vty_port, di->startinfo); if (di->terminal) { + nodetach_term = true; + vty_stdio(frr_terminal_close); if (daemon_ctl_sock != -1) { set_nonblocking(daemon_ctl_sock); @@ -912,9 +934,7 @@ void frr_run(struct thread_master *master) close(nullfd); } - if (daemon_ctl_sock != -1) - close(daemon_ctl_sock); - daemon_ctl_sock = -1; + frr_check_detach(); } /* end fixed stderr startup logging */ diff --git a/lib/libfrr.h b/lib/libfrr.h index d255279906..db58ff92be 100644 --- a/lib/libfrr.h +++ b/lib/libfrr.h @@ -29,11 +29,21 @@ #include "module.h" #include "hook.h" +/* The following options disable specific command line options that + * are not applicable for a particular daemon. + */ #define FRR_NO_PRIVSEP (1 << 0) #define FRR_NO_TCPVTY (1 << 1) #define FRR_LIMITED_CLI (1 << 2) -#define FRR_NO_CFG_PID_DRY (1 << 3) +#define FRR_NO_CFG_PID_DRY (1 << 3) #define FRR_NO_ZCLIENT (1 << 4) +/* If FRR_DETACH_LATER is used, the daemon will keep its parent running + * until frr_detach() is called. Normally "somedaemon -d" returns once the + * main event loop is reached in the daemon; use this for extra startup bits. + * + * Does nothing if -d isn't used. + */ +#define FRR_DETACH_LATER (1 << 5) struct frr_daemon_info { unsigned flags; @@ -102,10 +112,8 @@ extern struct thread_master *frr_init(void); DECLARE_HOOK(frr_late_init, (struct thread_master * tm), (tm)) extern void frr_config_fork(void); -extern void frr_vty_serv(void); - -/* note: contains call to frr_vty_serv() */ extern void frr_run(struct thread_master *master); +extern void frr_detach(void); extern bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len, const char *path); diff --git a/watchfrr/watchfrr.c b/watchfrr/watchfrr.c index 07a29ca6d5..306eacf2ec 100644 --- a/watchfrr/watchfrr.c +++ b/watchfrr/watchfrr.c @@ -55,9 +55,11 @@ #define PING_TOKEN "PING" +DEFINE_MGROUP(WATCHFRR, "watchfrr") +DEFINE_MTYPE_STATIC(WATCHFRR, WATCHFRR_DAEMON, "watchfrr daemon entry") + /* Needs to be global, referenced somewhere inside libfrr. */ struct thread_master *master; -static char pidfile_default[256]; static bool watch_only = false; @@ -230,7 +232,7 @@ Otherwise, the interval is doubled (but capped at the -M value).\n\n", name of the daemon should be substituted.\n\ --dry Do not start or restart anything, just log.\n\ -p, --pid-file Set process identifier file name\n\ - (default is %s).\n\ + (default is %s/watchfrr.pid).\n\ -b, --blank-string\n\ When the supplied argument string is found in any of the\n\ various shell command arguments (-r, -s, or -k), replace\n\ @@ -240,7 +242,7 @@ Otherwise, the interval is doubled (but capped at the -M value).\n\n", -h, --help Display this help and exit\n", frr_vtydir, DEFAULT_LOGLEVEL, LOG_EMERG, LOG_DEBUG, LOG_DEBUG, DEFAULT_MIN_RESTART, DEFAULT_MAX_RESTART, DEFAULT_PERIOD, - DEFAULT_TIMEOUT, DEFAULT_RESTART_TIMEOUT, pidfile_default); + DEFAULT_TIMEOUT, DEFAULT_RESTART_TIMEOUT, frr_vtydir); } static pid_t run_background(char *shell_cmd) @@ -609,12 +611,13 @@ static void daemon_send_ready(void) if (!sent && gs.numdown == 0) { FILE *fp; + zlog_notice("all daemons up, doing startup-complete notify"); + frr_detach(); + fp = fopen(DAEMON_VTY_DIR "/watchfrr.started", "w"); if (fp) fclose(fp); #if defined HAVE_SYSTEMD - zlog_notice( - "Watchfrr: Notifying Systemd we are up and running"); systemd_send_started(master, 0); #endif sent = 1; @@ -961,6 +964,53 @@ static char *translate_blanks(const char *cmd, const char *blankstr) return res; } +static void watchfrr_init(int argc, char **argv) +{ + const char *special = "zebra"; + int i; + struct daemon *dmn, **add = &gs.daemons; + char alldaemons[512] = "", *p = alldaemons; + + for (i = optind; i < argc; i++) { + dmn = XCALLOC(MTYPE_WATCHFRR_DAEMON, sizeof(*dmn)); + + dmn->name = dmn->restart.name = argv[i]; + dmn->state = DAEMON_INIT; + gs.numdaemons++; + gs.numdown++; + dmn->fd = -1; + dmn->t_wakeup = NULL; + thread_add_timer_msec(master, wakeup_init, dmn, + 100 + (random() % 900), + &dmn->t_wakeup); + dmn->restart.interval = gs.min_restart_interval; + *add = dmn; + add = &dmn->next; + + if (!strcmp(dmn->name, special)) + gs.special = dmn; + } + + if (!gs.daemons) { + fprintf(stderr, + "Must specify one or more daemons to monitor.\n\n"); + frr_help_exit(1); + } + if (!watch_only && !gs.special) { + fprintf(stderr, "\"%s\" daemon must be in daemon lists\n\n", + special); + frr_help_exit(1); + } + + for (dmn = gs.daemons; dmn; dmn = dmn->next) { + snprintf(p, alldaemons + sizeof(alldaemons) - p, "%s%s", + (p == alldaemons) ? "" : " ", dmn->name); + p += strlen(p); + } + zlog_notice("%s %s watching [%s]%s", progname, FRR_VERSION, alldaemons, + watch_only ? ", monitor mode" : ""); +} + struct zebra_privs_t watchfrr_privs = { #ifdef VTY_GROUP .vty_group = VTY_GROUP, @@ -984,7 +1034,8 @@ static struct quagga_signal_t watchfrr_signals[] = { FRR_DAEMON_INFO(watchfrr, WATCHFRR, .flags = FRR_NO_PRIVSEP | FRR_NO_TCPVTY | FRR_LIMITED_CLI - | FRR_NO_CFG_PID_DRY | FRR_NO_ZCLIENT, + | FRR_NO_CFG_PID_DRY | FRR_NO_ZCLIENT + | FRR_DETACH_LATER, .printhelp = printhelp, .copyright = "Copyright 2004 Andrew J. Schorr", @@ -999,13 +1050,8 @@ FRR_DAEMON_INFO(watchfrr, WATCHFRR, int main(int argc, char **argv) { int opt; - const char *pidfile = pidfile_default; - const char *special = "zebra"; const char *blankstr = NULL; - snprintf(pidfile_default, sizeof(pidfile_default), "%s/watchfrr.pid", - frr_vtydir); - frr_preinit(&watchfrr_di, argc, argv); progname = watchfrr_di.progname; @@ -1087,7 +1133,7 @@ int main(int argc, char **argv) gs.period = 1000 * period; } break; case 'p': - pidfile = optarg; + watchfrr_di.pid_file = optarg; break; case 'r': if (!valid_command(optarg)) { @@ -1167,98 +1213,18 @@ int main(int argc, char **argv) master = frr_init(); watchfrr_error_init(); + watchfrr_init(argc, argv); + watchfrr_vty_init(); + + frr_config_fork(); zlog_set_level(ZLOG_DEST_MONITOR, ZLOG_DISABLED); - if (watchfrr_di.daemon_mode) { + if (watchfrr_di.daemon_mode) zlog_set_level(ZLOG_DEST_SYSLOG, MIN(gs.loglevel, LOG_DEBUG)); - if (daemon(0, 0) < 0) { - fprintf(stderr, "Watchfrr daemon failed: %s", - strerror(errno)); - exit(1); - } - } else + else zlog_set_level(ZLOG_DEST_STDOUT, MIN(gs.loglevel, LOG_DEBUG)); - watchfrr_vty_init(); - - frr_vty_serv(); - - { - int i; - struct daemon *tail = NULL; - - for (i = optind; i < argc; i++) { - struct daemon *dmn; - - if (!(dmn = (struct daemon *)calloc(1, sizeof(*dmn)))) { - fprintf(stderr, "calloc(1,%u) failed: %s\n", - (unsigned int)sizeof(*dmn), - safe_strerror(errno)); - return 1; - } - dmn->name = dmn->restart.name = argv[i]; - dmn->state = DAEMON_INIT; - gs.numdaemons++; - gs.numdown++; - dmn->fd = -1; - dmn->t_wakeup = NULL; - thread_add_timer_msec(master, wakeup_init, dmn, - 100 + (random() % 900), - &dmn->t_wakeup); - dmn->restart.interval = gs.min_restart_interval; - if (tail) - tail->next = dmn; - else - gs.daemons = dmn; - tail = dmn; - - if (!strcmp(dmn->name, special)) - gs.special = dmn; - } - } - if (!gs.daemons) { - fputs("Must specify one or more daemons to monitor.\n", stderr); - frr_help_exit(1); - } - if (!watch_only && !gs.special) { - fprintf(stderr, "\"%s\" daemon must be in daemon list\n", - special); - frr_help_exit(1); - } - - /* Make sure we're not already running. */ - pid_output(pidfile); - - /* Announce which daemons are being monitored. */ - { - struct daemon *dmn; - size_t len = 0; - - for (dmn = gs.daemons; dmn; dmn = dmn->next) - len += strlen(dmn->name) + 1; - - { - char buf[len + 1]; - char *p = buf; - - for (dmn = gs.daemons; dmn; dmn = dmn->next) { - if (p != buf) - *p++ = ' '; - strcpy(p, dmn->name); - p += strlen(p); - } - zlog_notice("%s %s watching [%s]%s", progname, - FRR_VERSION, buf, - watch_only ? ", monitor mode" : ""); - } - } - - { - struct thread thread; - - while (thread_fetch(master, &thread)) - thread_call(&thread); - } + frr_run(master); systemd_send_stopping(); /* Not reached. */ diff --git a/watchfrr/watchfrr.h b/watchfrr/watchfrr.h index 1a1c19056f..ee16846a1d 100644 --- a/watchfrr/watchfrr.h +++ b/watchfrr/watchfrr.h @@ -21,6 +21,10 @@ #ifndef FRR_WATCHFRR_H #define FRR_WATCHFRR_H +#include "lib/memory.h" + +DECLARE_MGROUP(WATCHFRR) + extern void watchfrr_vty_init(void); extern pid_t integrated_write_pid; -- 2.39.5