Modify httpd(8)'s proc.c to use less file descriptors during the daemon

start up. To achieve this proc_init() initiates only the necessary pipes
between child and parent, allocate and distribute fds in proc_connect().

In case of configuration checks ('-n') we do nothing in proc_init() and
proc_connect().

ok reyk@

1 files changed, 73 insertions, 66 deletions

diff --git a/usr.sbin/httpd/proc.c b/usr.sbin/httpd/proc.c
index e595aafaa50..395fe3f21f4 100644
--- a/usr.sbin/httpd/proc.c
+++ b/usr.sbin/httpd/proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.c,v 1.32 2016/10/10 16:31:35 rzalamena Exp $	*/
+/*	$OpenBSD: proc.c,v 1.33 2016/10/10 21:53:46 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2010 - 2016 Reyk Floeter <reyk@openbsd.org>
@@ -37,8 +37,6 @@
 
 void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
 	    int, char **);
-void	 proc_connectpeer(struct privsep *, enum privsep_procid, int,
-	    struct privsep_pipes *);
 void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
 void	 proc_open(struct privsep *, int, int);
 void	 proc_accept(struct privsep *, int, enum privsep_procid,
@@ -157,72 +155,38 @@ proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
 }
 
 void
-proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
-    struct privsep_pipes *pp)
-{
-	unsigned int		 i, j;
-	struct privsep_fd	 pf;
-
-	for (i = 0; i < PROC_MAX; i++) {
-		/* Parent is already connected with everyone. */
-		if (i == PROC_PARENT)
-			continue;
-
-		for (j = 0; j < ps->ps_instances[i]; j++) {
-			/* Don't send socket to child itself. */
-			if (i == (unsigned int)id &&
-			    j == (unsigned int)inst)
-				continue;
-			if (pp->pp_pipes[i][j] == -1)
-				continue;
-
-			pf.pf_procid = i;
-			pf.pf_instance = j;
-			proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
-			    -1, pp->pp_pipes[i][j], &pf, sizeof(pf));
-			pp->pp_pipes[i][j] = -1;
-		}
-	}
-}
-
-/* Inter-connect all process except with ourself. */
-void
 proc_connect(struct privsep *ps)
 {
-	unsigned int		 src, i, j;
-	struct privsep_pipes	*pp;
 	struct imsgev		*iev;
+	unsigned int		 src, dst, inst;
 
-	/* Listen on appropriate pipes. */
-	src = privsep_process;
-	pp = &ps->ps_pipes[src][ps->ps_instance];
+	/* Don't distribute any sockets if we are not really going to run. */
+	if (ps->ps_noaction)
+		return;
 
-	for (i = 0; i < PROC_MAX; i++) {
-		/* Don't listen to ourself. */
-		if (i == src)
+	for (dst = 0; dst < PROC_MAX; dst++) {
+		/* We don't communicate with ourselves. */
+		if (dst == PROC_PARENT)
 			continue;
 
-		for (j = 0; j < ps->ps_instances[i]; j++) {
-			if (pp->pp_pipes[i][j] == -1)
-				continue;
-
-			iev = &ps->ps_ievs[i][j];
-			imsg_init(&iev->ibuf, pp->pp_pipes[i][j]);
+		for (inst = 0; inst < ps->ps_instances[dst]; inst++) {
+			iev = &ps->ps_ievs[dst][inst];
+			imsg_init(&iev->ibuf, ps->ps_pp->pp_pipes[dst][inst]);
 			event_set(&iev->ev, iev->ibuf.fd, iev->events,
 			    iev->handler, iev->data);
 			event_add(&iev->ev, NULL);
 		}
 	}
 
-	/* Exchange pipes between process. */
-	for (i = 0; i < PROC_MAX; i++) {
-		/* Parent is already connected with everyone. */
-		if (i == PROC_PARENT)
-			continue;
+	/* Distribute the socketpair()s for everyone. */
+	for (src = 0; src < PROC_MAX; src++)
+		for (dst = src; dst < PROC_MAX; dst++) {
+			/* Parent already distributed its fds. */
+			if (src == PROC_PARENT || dst == PROC_PARENT)
+				continue;
 
-		for (j = 0; j < ps->ps_instances[i]; j++)
-			proc_connectpeer(ps, i, j, &ps->ps_pipes[i][j]);
-	}
+			proc_open(ps, src, dst);
+		}
 }
 
 void
@@ -230,17 +194,41 @@ proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
     int argc, char **argv, enum privsep_procid proc_id)
 {
 	struct privsep_proc	*p = NULL;
+	struct privsep_pipes	*pa, *pb;
 	unsigned int		 proc;
-	unsigned int		 src, dst;
+	unsigned int		 dst;
+	int			 fds[2];
+
+	/* Don't initiate anything if we are not really going to run. */
+	if (ps->ps_noaction)
+		return;
 
 	if (proc_id == PROC_PARENT) {
 		privsep_process = PROC_PARENT;
 		proc_setup(ps, procs, nproc);
 
-		/* Open socketpair()s for everyone. */
-		for (src = 0; src < PROC_MAX; src++)
-			for (dst = 0; dst < PROC_MAX; dst++)
-				proc_open(ps, src, dst);
+		/*
+		 * Create the children sockets so we can use them 
+		 * to distribute the rest of the socketpair()s using
+		 * proc_connect() later.
+		 */
+		for (dst = 0; dst < PROC_MAX; dst++) {
+			/* Don't create socket for ourselves. */
+			if (dst == PROC_PARENT)
+				continue;
+
+			for (proc = 0; proc < ps->ps_instances[dst]; proc++) {
+				pa = &ps->ps_pipes[PROC_PARENT][0];
+				pb = &ps->ps_pipes[dst][proc];
+				if (socketpair(AF_UNIX,
+				    SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
+				    PF_UNSPEC, fds) == -1)
+					fatal("%s: socketpair", __func__);
+
+				pa->pp_pipes[dst][proc] = fds[0];
+				pb->pp_pipes[PROC_PARENT][0] = fds[1];
+			}
+		}
 
 		/* Engage! */
 		proc_exec(ps, procs, nproc, argc, argv);
@@ -409,9 +397,11 @@ void
 proc_open(struct privsep *ps, int src, int dst)
 {
 	struct privsep_pipes	*pa, *pb;
+	struct privsep_fd	 pf;
 	int			 fds[2];
 	unsigned int		 i, j;
 
+	/* Exchange pipes between process. */
 	for (i = 0; i < ps->ps_instances[src]; i++) {
 		for (j = 0; j < ps->ps_instances[dst]; j++) {
 			/* Don't create sockets for ourself. */
@@ -420,9 +410,6 @@ proc_open(struct privsep *ps, int src, int dst)
 
 			pa = &ps->ps_pipes[src][i];
 			pb = &ps->ps_pipes[dst][j];
-			if (pb->pp_pipes[dst][j] != -1)
-				continue;
-
 			if (socketpair(AF_UNIX,
 			    SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
 			    PF_UNSPEC, fds) == -1)
@@ -430,6 +417,29 @@ proc_open(struct privsep *ps, int src, int dst)
 
 			pa->pp_pipes[dst][j] = fds[0];
 			pb->pp_pipes[src][i] = fds[1];
+
+			pf.pf_procid = src;
+			pf.pf_instance = i;
+			if (proc_compose_imsg(ps, dst, j, IMSG_CTL_PROCFD,
+			    -1, pb->pp_pipes[src][i], &pf, sizeof(pf)) == -1)
+				fatal("%s: proc_compose_imsg", __func__);
+
+			pf.pf_procid = dst;
+			pf.pf_instance = j;
+			if (proc_compose_imsg(ps, src, i, IMSG_CTL_PROCFD,
+			    -1, pa->pp_pipes[dst][j], &pf, sizeof(pf)) == -1)
+				fatal("%s: proc_compose_imsg", __func__);
+
+			/*
+			 * We have to flush to send the descriptors and close
+			 * them to avoid the fd ramp on startup.
+			 */
+			if (imsg_flush(&ps->ps_ievs[src][i].ibuf) == -1 ||
+			    imsg_flush(&ps->ps_ievs[dst][j].ibuf) == -1)
+				fatal("%s: imsg_flush", __func__);
+
+			imsg_event_add(&ps->ps_ievs[src][i]);
+			imsg_event_add(&ps->ps_ievs[dst][j]);
 		}
 	}
 }
@@ -512,9 +522,6 @@ proc_run(struct privsep *ps, struct privsep_proc *p,
 	const char		*root;
 	struct control_sock	*rcs;
 
-	if (ps->ps_noaction)
-		exit(0);
-
 	log_procinit(p->p_title);
 
 	/* Set the process group of the current process */