bgpd: changes in listener setup

[Henning Brauer <henning_(_at_)_openbsd_(_dot_)_org>]

the current way of setting up the listeners is a bit... strange.

here's the diff that changes it all and fixes some issues i have seen.

the parent keeps a list of the listeners too, and thus knows when it 
actually has to open a new fd. little state keeping and such of course

please try out, provoke it (reloads reloads reloads, with adding and 
removing listeners etc), read teh code, find bugs! :)

Index: bgpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
retrieving revision 1.113
diff -u -p -r1.113 bgpd.c
--- bgpd.c	9 Feb 2005 10:56:28 -0000	1.113
+++ bgpd.c	15 Mar 2005 12:35:54 -0000
@@ -234,12 +234,6 @@ main(int argc, char *argv[])
 		free(r);
 	}
 
-	while ((la = TAILQ_FIRST(conf.listen_addrs)) != NULL) {
-		TAILQ_REMOVE(conf.listen_addrs, la, entry);
-		close(la->fd);
-		free(la);
-	}
-
 	mrt_reconfigure(&mrt_l);
 
 	while (quit == 0) {
@@ -334,6 +328,11 @@ main(int argc, char *argv[])
 		LIST_REMOVE(m, entry);
 		free(m);
 	}
+	while ((la = TAILQ_FIRST(conf.listen_addrs)) != NULL) {
+		TAILQ_REMOVE(conf.listen_addrs, la, entry);
+		close(la->fd);
+		free(la);
+	}
 
 	free(rules_l);
 	control_cleanup();
@@ -448,15 +447,12 @@ reconfigure(char *conffile, struct bgpd_
 		TAILQ_REMOVE(rules_l, r, entry);
 		free(r);
 	}
-	while ((la = TAILQ_FIRST(conf->listen_addrs)) != NULL) {
+	TAILQ_FOREACH(la, conf->listen_addrs, entry) {
 		if (imsg_compose(ibuf_se, IMSG_RECONF_LISTENER, 0, 0, la->fd,
 		    la, sizeof(struct listen_addr)) == -1)
 			return (-1);
-		TAILQ_REMOVE(conf->listen_addrs, la, entry);
-		free(la);
+		la->fd = -1;
 	}
-	free(conf->listen_addrs);
-	conf->listen_addrs = NULL;
 
 	if (imsg_compose(ibuf_se, IMSG_RECONF_DONE, 0, 0, -1, NULL, 0) == -1 ||
 	    imsg_compose(ibuf_rde, IMSG_RECONF_DONE, 0, 0, -1, NULL, 0) == -1)
Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
retrieving revision 1.40
diff -u -p -r1.40 config.c
--- config.c	1 Oct 2004 15:11:12 -0000	1.40
+++ config.c	15 Mar 2005 12:35:54 -0000
@@ -1,7 +1,7 @@
 /*	$OpenBSD: config.c,v 1.40 2004/10/01 15:11:12 henning Exp $ */
 
 /*
- * Copyright (c) 2003, 2004 Henning Brauer <henning_(_at_)_openbsd_(_dot_)_org>
+ * Copyright (c) 2003, 2004, 2005 Henning Brauer <henning_(_at_)_openbsd_(_dot_)_org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -40,9 +40,13 @@ merge_config(struct bgpd_config *xconf, 
     struct peer *peer_l, struct listen_addrs *listen_addrs)
 {
 	struct peer				*p;
-	struct listen_addr			*la;
+	struct listen_addr			*nla, *ola, *next;
 	int					 errs = 0;
 
+	/*
+	 * merge the freshly parsed conf into the running xconf
+	 */
+
 	/* preserve cmd line opts */
 	conf->opts = xconf->opts;
 
@@ -76,17 +80,66 @@ merge_config(struct bgpd_config *xconf, 
 		}
 	}
 
-	if (xconf->listen_addrs != NULL) {
-		while ((la = TAILQ_FIRST(xconf->listen_addrs)) != NULL) {
-			TAILQ_REMOVE(xconf->listen_addrs, la, entry);
-			free(la);
+	conf->listen_addrs = xconf->listen_addrs;
+	memcpy(xconf, conf, sizeof(struct bgpd_config));
+
+	if (conf->listen_addrs == NULL) {
+		/* there is no old conf, just copy new one over */
+		xconf->listen_addrs = listen_addrs;
+		TAILQ_FOREACH(nla, xconf->listen_addrs, entry)
+			nla->reconf = RECONF_REINIT;
+
+	} else {
+		/* 
+		 * merge new listeners:
+		 * -flag all as to be deleted
+		 * -those that are in both new and old: flag to keep
+		 * -new ones get inserted and flagged as to reinit
+		 * -remove all that are still flagged for deletion
+		 */
+
+		TAILQ_FOREACH(nla, xconf->listen_addrs, entry)
+			nla->reconf = RECONF_DELETE;
+
+		/* no new listeners? preserve default ones */
+		if (TAILQ_EMPTY(listen_addrs))
+			TAILQ_FOREACH(ola, xconf->listen_addrs, entry)
+				if (ola->flags & DEFAULT_LISTENER)
+					ola->reconf = RECONF_KEEP;
+
+		for (nla = TAILQ_FIRST(listen_addrs); nla != NULL; nla = next) {
+			next = TAILQ_NEXT(nla, entry);
+
+			TAILQ_FOREACH(ola, xconf->listen_addrs, entry)
+				if (!memcmp(&nla->sa, &ola->sa,
+				    sizeof(nla->sa)))
+					break;
+
+			if (ola == NULL) {
+				/* new listener, copy over */
+				TAILQ_REMOVE(listen_addrs, nla, entry);
+				TAILQ_INSERT_TAIL(xconf->listen_addrs,
+				    nla, entry);
+				nla->reconf = RECONF_REINIT;
+			} else		/* exists, just flag */
+				ola->reconf = RECONF_KEEP;
 		}
-		free(xconf->listen_addrs);
-	}
 
-	memcpy(xconf, conf, sizeof(struct bgpd_config));
+		for (nla = TAILQ_FIRST(xconf->listen_addrs); nla != NULL;
+		    nla = next) {
+			next = TAILQ_NEXT(nla, entry);
+			if (nla->reconf == RECONF_DELETE) {
+				TAILQ_REMOVE(xconf->listen_addrs, nla, entry);
+				free(nla);
+			}
+		}
 
-	xconf->listen_addrs = listen_addrs;
+		while ((ola = TAILQ_FIRST(listen_addrs)) != NULL) {
+			TAILQ_REMOVE(listen_addrs, ola, entry);
+			free(ola);
+		}
+		free(listen_addrs);
+	}
 
 	return (errs);
 }
@@ -236,6 +289,7 @@ prepare_listeners(struct bgpd_config *co
 			fatal("setup_listeners calloc");
 		la->fd = -1;
 		la->flags = DEFAULT_LISTENER;
+		la->reconf = RECONF_REINIT;
 		la->sa.ss_len = sizeof(struct sockaddr_in);
 		((struct sockaddr_in *)&la->sa)->sin_family = AF_INET;
 		((struct sockaddr_in *)&la->sa)->sin_addr.s_addr =
@@ -247,6 +301,7 @@ prepare_listeners(struct bgpd_config *co
 			fatal("setup_listeners calloc");
 		la->fd = -1;
 		la->flags = DEFAULT_LISTENER;
+		la->reconf = RECONF_REINIT;
 		la->sa.ss_len = sizeof(struct sockaddr_in6);
 		((struct sockaddr_in6 *)&la->sa)->sin6_family = AF_INET6;
 		((struct sockaddr_in6 *)&la->sa)->sin6_port = htons(BGP_PORT);
@@ -255,6 +310,9 @@ prepare_listeners(struct bgpd_config *co
 
 	for (la = TAILQ_FIRST(conf->listen_addrs); la != NULL; la = next) {
 		next = TAILQ_NEXT(la, entry);
+		if (la->reconf != RECONF_REINIT)
+			continue;
+
 		if ((la->fd = socket(la->sa.ss_family, SOCK_STREAM,
 		    IPPROTO_TCP)) == -1) {
 			if (la->flags & DEFAULT_LISTENER && (errno ==
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.213
diff -u -p -r1.213 session.c
--- session.c	11 Mar 2005 17:46:11 -0000	1.213
+++ session.c	15 Mar 2005 12:35:55 -0000
@@ -1,7 +1,7 @@
 /*	$OpenBSD: session.c,v 1.213 2005/03/11 17:46:11 henning Exp $ */
 
 /*
- * Copyright (c) 2003, 2004 Henning Brauer <henning_(_at_)_openbsd_(_dot_)_org>
+ * Copyright (c) 2003, 2004, 2005 Henning Brauer <henning_(_at_)_openbsd_(_dot_)_org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -2109,17 +2109,20 @@ session_dispatch_imsg(struct imsgbuf *ib
 			if (idx != PFD_PIPE_MAIN)
 				fatalx("reconf request not from parent");
 			nla = imsg.data;
-			TAILQ_FOREACH(la, conf->listen_addrs, entry) {
+			TAILQ_FOREACH(la, conf->listen_addrs, entry)
 				if (!la_cmp(la, nla))
 					break;
-			}
-
-			if ((nla->fd = imsg_get_fd(ibuf)) == -1)
-				log_warnx("expected to receive fd for %s "
-				    "but didn't receive any",
-				    log_sockaddr((struct sockaddr *)&la->sa));
 
 			if (la == NULL) {
+				if (nla->reconf != RECONF_REINIT)
+					fatal("king bula sez: expected REINIT");
+
+				if ((nla->fd = imsg_get_fd(ibuf)) == -1)
+					log_warnx("expected to receive fd for "
+					    "%s but didn't receive any",
+					    log_sockaddr((struct sockaddr *)
+					    &la->sa));
+
 				la = calloc(1, sizeof(struct listen_addr));
 				if (la == NULL)
 					fatal(NULL);
@@ -2130,9 +2133,11 @@ session_dispatch_imsg(struct imsgbuf *ib
 				TAILQ_INSERT_TAIL(nconf->listen_addrs, la,
 				    entry);
 			} else {
+				if (nla->reconf != RECONF_KEEP)
+					fatal("king bula sez: expected KEEP");
 				la->reconf = RECONF_KEEP;
-				close(nla->fd);
 			}
+
 			break;
 		case IMSG_RECONF_DONE:
 			if (idx != PFD_PIPE_MAIN)