Merge pull request #3098 from opensourcerouting/watchfrr-delay

tools/frr + watchfrr spring cleaning (southern hemisphere)

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);
@@ -767,13 +770,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();
@@ -783,7 +789,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) */
@@ -810,14 +816,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);
@@ -881,6 +895,12 @@ out:
 	return 0;
 }
 
+void frr_detach(void)
+{
+	nodetach_daemon = false;
+	frr_check_detach();
+}
+
 void frr_run(struct thread_master *master)
 {
 	char instanceinfo[64] = "";
@@ -895,6 +915,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);
@@ -914,9 +936,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 */

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);

tools/frr.in

@@ -28,7 +28,6 @@ FRR_VTY_GROUP="@enable_vty_group@" # frrvty
 DAEMONS="zebra bgpd ripd ripngd ospfd ospf6d isisd babeld pimd ldpd nhrpd eigrpd sharpd pbrd staticd bfdd"
 MAX_INSTANCES=5
 RELOAD_SCRIPT="$D_PATH/frr-reload.py"
-WATCHFRR_STARTED="$V_PATH/watchfrr.started"
 
 if [ -e /lib/lsb/init-functions ]; then
     . /lib/lsb/init-functions
@@ -128,23 +127,13 @@ start()
                echo -n " $1"
            fi
 
-           if [ -e $WATCHFRR_STARTED ] ; then
-             rm $WATCHFRR_STARTED
-           fi
-            ${SSD} \
+           ${SSD} \
                 --start \
                 --pidfile=`pidfile $1` \
                 --exec "$D_PATH/$1" \
                 -- \
                 "${watchfrr_options[@]}"
-            for i in `seq 1 10`;
-            do
-              if [ -e $WATCHFRR_STARTED ] ; then
-		  break
-              else
-                  sleep 1
-              fi
-            done
+
         elif [ -n "$2" ]; then
           echo -n " $1-$2"
            if ! check_daemon $1 $2 ; then

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,99 +1213,18 @@ int main(int argc, char **argv)
 
 	master = frr_init();
 	watchfrr_error_init();
-
-	zlog_set_level(ZLOG_DEST_MONITOR, ZLOG_DISABLED);
-	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
-		zlog_set_level(ZLOG_DEST_STDOUT, MIN(gs.loglevel, LOG_DEBUG));
-
+	watchfrr_init(argc, argv);
 	watchfrr_vty_init();
 
-	frr_vty_serv();
+	frr_config_fork();
 
-	{
-		int i;
-		struct daemon *tail = NULL;
+	zlog_set_level(ZLOG_DEST_MONITOR, ZLOG_DISABLED);
+	if (watchfrr_di.daemon_mode)
+		zlog_set_level(ZLOG_DEST_SYSLOG, MIN(gs.loglevel, LOG_DEBUG));
+	else
+		zlog_set_level(ZLOG_DEST_STDOUT, MIN(gs.loglevel, LOG_DEBUG));
 
-		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) {
-		flog_err(EC_WATCHFRR_UNEXPECTED_DAEMONS,
-			  "Must specify one or more daemons to monitor.");
-		frr_help_exit(1);
-	}
-	if (!watch_only && !gs.special) {
-		flog_err(EC_WATCHFRR_UNEXPECTED_DAEMONS,
-			  "\"%s\" daemon must be in daemon lists", 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. */

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;