replace manual poll/ppoll timeout math with ptimeout API

feedback markus / ok markus dtucker

4 files changed, 47 insertions, 92 deletions

diff --git a/usr.bin/ssh/channels.c b/usr.bin/ssh/channels.c
index 8b9199599c4..a1a7f518934 100644
--- a/usr.bin/ssh/channels.c
+++ b/usr.bin/ssh/channels.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.c,v 1.421 2022/11/18 19:47:40 mbuhl Exp $ */
+/* $OpenBSD: channels.c,v 1.422 2023/01/06 02:38:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -2368,7 +2368,7 @@ channel_garbage_collect(struct ssh *ssh, Channel *c)
 enum channel_table { CHAN_PRE, CHAN_POST };
 
 static void
-channel_handler(struct ssh *ssh, int table, time_t *unpause_secs)
+channel_handler(struct ssh *ssh, int table, struct timespec *timeout)
 {
 	struct ssh_channels *sc = ssh->chanctxt;
 	chan_fn **ftab = table == CHAN_PRE ? sc->channel_pre : sc->channel_post;
@@ -2377,8 +2377,6 @@ channel_handler(struct ssh *ssh, int table, time_t *unpause_secs)
 	time_t now;
 
 	now = monotime();
-	if (unpause_secs != NULL)
-		*unpause_secs = 0;
 	for (i = 0, oalloc = sc->channels_alloc; i < oalloc; i++) {
 		c = sc->channels[i];
 		if (c == NULL)
@@ -2398,24 +2396,17 @@ channel_handler(struct ssh *ssh, int table, time_t *unpause_secs)
 			 */
 			if (c->notbefore <= now)
 				(*ftab[c->type])(ssh, c);
-			else if (unpause_secs != NULL) {
+			else if (timeout != NULL) {
 				/*
-				 * Collect the time that the earliest
-				 * channel comes off pause.
+				 * Arrange for poll wakeup when channel pause
+				 * timer expires.
 				 */
-				debug3_f("chan %d: skip for %d more "
-				    "seconds", c->self,
-				    (int)(c->notbefore - now));
-				if (*unpause_secs == 0 ||
-				    (c->notbefore - now) < *unpause_secs)
-					*unpause_secs = c->notbefore - now;
+				ptimeout_deadline_monotime(timeout,
+				    c->notbefore);
 			}
 		}
 		channel_garbage_collect(ssh, c);
 	}
-	if (unpause_secs != NULL && *unpause_secs != 0)
-		debug3_f("first channel unpauses in %d seconds",
-		    (int)*unpause_secs);
 }
 
 /*
@@ -2561,7 +2552,7 @@ channel_prepare_pollfd(Channel *c, u_int *next_pollfd,
 /* * Allocate/prepare poll structure */
 void
 channel_prepare_poll(struct ssh *ssh, struct pollfd **pfdp, u_int *npfd_allocp,
-    u_int *npfd_activep, u_int npfd_reserved, time_t *minwait_secs)
+    u_int *npfd_activep, u_int npfd_reserved, struct timespec *timeout)
 {
 	struct ssh_channels *sc = ssh->chanctxt;
 	u_int i, oalloc, p, npfd = npfd_reserved;
@@ -2585,7 +2576,7 @@ channel_prepare_poll(struct ssh *ssh, struct pollfd **pfdp, u_int *npfd_allocp,
 	*npfd_activep = npfd_reserved;
 	oalloc = sc->channels_alloc;
 
-	channel_handler(ssh, CHAN_PRE, minwait_secs);
+	channel_handler(ssh, CHAN_PRE, timeout);
 
 	if (oalloc != sc->channels_alloc) {
 		/* shouldn't happen */
diff --git a/usr.bin/ssh/channels.h b/usr.bin/ssh/channels.h
index 18961014307..c04f5a7f971 100644
--- a/usr.bin/ssh/channels.h
+++ b/usr.bin/ssh/channels.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.h,v 1.143 2022/05/05 00:56:58 djm Exp $ */
+/* $OpenBSD: channels.h,v 1.144 2023/01/06 02:38:23 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -312,9 +312,10 @@ int	 channel_input_status_confirm(int, u_int32_t, struct ssh *);
 
 /* file descriptor handling (read/write) */
 struct pollfd;
+struct timespec;
 
 void	 channel_prepare_poll(struct ssh *, struct pollfd **,
-	    u_int *, u_int *, u_int, time_t *);
+	    u_int *, u_int *, u_int, struct timespec *);
 void	 channel_after_poll(struct ssh *, struct pollfd *, u_int);
 void     channel_output_poll(struct ssh *);
 
diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
index 5b1d5d894af..19a5d4b192d 100644
--- a/usr.bin/ssh/clientloop.c
+++ b/usr.bin/ssh/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.385 2022/11/29 22:41:14 dtucker Exp $ */
+/* $OpenBSD: clientloop.c,v 1.386 2023/01/06 02:38:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -509,16 +509,15 @@ client_wait_until_can_do_something(struct ssh *ssh, struct pollfd **pfdp,
     u_int *npfd_allocp, u_int *npfd_activep, int rekeying,
     int *conn_in_readyp, int *conn_out_readyp)
 {
-	int timeout_secs, pollwait;
-	time_t minwait_secs = 0, now = monotime();
+	struct timespec timeout;
 	int ret;
 	u_int p;
 
 	*conn_in_readyp = *conn_out_readyp = 0;
 
 	/* Prepare channel poll. First two pollfd entries are reserved */
-	channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, 2,
-	    &minwait_secs);
+	ptimeout_init(&timeout);
+	channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, 2, &timeout);
 	if (*npfd_activep < 2)
 		fatal_f("bad npfd %u", *npfd_activep); /* shouldn't happen */
 
@@ -542,30 +541,17 @@ client_wait_until_can_do_something(struct ssh *ssh, struct pollfd **pfdp,
 	 * some polled descriptor can be read, written, or has some other
 	 * event pending, or a timeout expires.
 	 */
-
-	timeout_secs = INT_MAX; /* we use INT_MAX to mean no timeout */
+	set_control_persist_exit_time(ssh);
+	if (control_persist_exit_time > 0)
+		ptimeout_deadline_monotime(&timeout, control_persist_exit_time);
 	if (options.server_alive_interval > 0)
-		timeout_secs = MAXIMUM(server_alive_time - now, 0);
-	if (options.rekey_interval > 0 && !rekeying)
-		timeout_secs = MINIMUM(timeout_secs,
+		ptimeout_deadline_monotime(&timeout, server_alive_time);
+	if (options.rekey_interval > 0 && !rekeying) {
+		ptimeout_deadline_sec(&timeout,
 		    ssh_packet_get_rekey_timeout(ssh));
-	set_control_persist_exit_time(ssh);
-	if (control_persist_exit_time > 0) {
-		timeout_secs = MINIMUM(timeout_secs,
-			control_persist_exit_time - now);
-		if (timeout_secs < 0)
-			timeout_secs = 0;
-	}
-	if (minwait_secs != 0)
-		timeout_secs = MINIMUM(timeout_secs, (int)minwait_secs);
-	if (timeout_secs == INT_MAX)
-		pollwait = -1;
-	else if (timeout_secs >= INT_MAX / 1000)
-		pollwait = INT_MAX;
-	else
-		pollwait = timeout_secs * 1000;
+	}
 
-	ret = poll(*pfdp, *npfd_activep, pollwait);
+	ret = poll(*pfdp, *npfd_activep, ptimeout_get_ms(&timeout));
 
 	if (ret == -1) {
 		/*
diff --git a/usr.bin/ssh/serverloop.c b/usr.bin/ssh/serverloop.c
index 221043617d2..5347e3355bc 100644
--- a/usr.bin/ssh/serverloop.c
+++ b/usr.bin/ssh/serverloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: serverloop.c,v 1.232 2022/04/20 04:19:11 djm Exp $ */
+/* $OpenBSD: serverloop.c,v 1.233 2023/01/06 02:38:23 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -162,12 +162,11 @@ client_alive_check(struct ssh *ssh)
 static void
 wait_until_can_do_something(struct ssh *ssh,
     int connection_in, int connection_out, struct pollfd **pfdp,
-    u_int *npfd_allocp, u_int *npfd_activep, u_int64_t max_time_ms,
-    sigset_t *sigsetp, int *conn_in_readyp, int *conn_out_readyp)
+    u_int *npfd_allocp, u_int *npfd_activep, sigset_t *sigsetp,
+    int *conn_in_readyp, int *conn_out_readyp)
 {
-	struct timespec ts, *tsp;
+	struct timespec timeout;
 	int ret;
-	time_t minwait_secs = 0;
 	int client_alive_scheduled = 0;
 	u_int p;
 	/* time we last heard from the client OR sent a keepalive */
@@ -176,14 +175,14 @@ wait_until_can_do_something(struct ssh *ssh,
 	*conn_in_readyp = *conn_out_readyp = 0;
 
 	/* Prepare channel poll. First two pollfd entries are reserved */
-	channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep,
-	    2, &minwait_secs);
+	ptimeout_init(&timeout);
+	channel_prepare_poll(ssh, pfdp, npfd_allocp, npfd_activep, 2, &timeout);
 	if (*npfd_activep < 2)
 		fatal_f("bad npfd %u", *npfd_activep); /* shouldn't happen */
-
-	/* XXX need proper deadline system for rekey/client alive */
-	if (minwait_secs != 0)
-		max_time_ms = MINIMUM(max_time_ms, (u_int)minwait_secs * 1000);
+	if (options.rekey_interval > 0 && !ssh_packet_is_rekeying(ssh)) {
+		ptimeout_deadline_sec(&timeout,
+		    ssh_packet_get_rekey_timeout(ssh));
+	}
 
 	/*
 	 * if using client_alive, set the max timeout accordingly,
@@ -194,15 +193,11 @@ wait_until_can_do_something(struct ssh *ssh,
 	 * analysis more difficult, but we're not doing it yet.
 	 */
 	if (options.client_alive_interval) {
-		uint64_t keepalive_ms =
-		    (uint64_t)options.client_alive_interval * 1000;
-
-		if (max_time_ms == 0 || max_time_ms > keepalive_ms) {
-			max_time_ms = keepalive_ms;
-			client_alive_scheduled = 1;
-		}
 		if (last_client_time == 0)
 			last_client_time = monotime();
+		ptimeout_deadline_sec(&timeout, options.client_alive_interval);
+		/* XXX ? deadline_monotime(last_client_time + alive_interval) */
+		client_alive_scheduled = 1;
 	}
 
 #if 0
@@ -220,19 +215,10 @@ wait_until_can_do_something(struct ssh *ssh,
 	 * from it, then read as much as is available and exit.
 	 */
 	if (child_terminated && ssh_packet_not_very_much_data_to_write(ssh))
-		if (max_time_ms == 0 || client_alive_scheduled)
-			max_time_ms = 100;
-
-	if (max_time_ms == 0)
-		tsp = NULL;
-	else {
-		ts.tv_sec = max_time_ms / 1000;
-		ts.tv_nsec = 1000000 * (max_time_ms % 1000);
-		tsp = &ts;
-	}
+		ptimeout_deadline_ms(&timeout, 100);
 
 	/* Wait for something to happen, or the timeout to expire. */
-	ret = ppoll(*pfdp, *npfd_activep, tsp, sigsetp);
+	ret = ppoll(*pfdp, *npfd_activep, ptimeout_get_tsp(&timeout), sigsetp);
 
 	if (ret == -1) {
 		for (p = 0; p < *npfd_activep; p++)
@@ -245,19 +231,18 @@ wait_until_can_do_something(struct ssh *ssh,
 	*conn_in_readyp = (*pfdp)[0].revents != 0;
 	*conn_out_readyp = (*pfdp)[1].revents != 0;
 
+	/* ClientAliveInterval probing */
 	if (client_alive_scheduled) {
 		time_t now = monotime();
-
-		/*
-		 * If the ppoll timed out, or returned for some other reason
-		 * but we haven't heard from the client in time, send keepalive.
-		 */
-		if (ret == 0 || (last_client_time != 0 && last_client_time +
-		    options.client_alive_interval <= now)) {
+		if (ret == 0 &&
+		    now > last_client_time + options.client_alive_interval) {
+			/* ppoll timed out and we're due to probe */
 			client_alive_check(ssh);
 			last_client_time = now;
-		} else if (*conn_in_readyp)
+		} else if (ret != 0 && *conn_in_readyp) {
+			/* Data from peer; reset probe timer. */
 			last_client_time = now;
+		}
 	}
 }
 
@@ -332,7 +317,6 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
 	u_int npfd_alloc = 0, npfd_active = 0;
 	int r, conn_in_ready, conn_out_ready;
 	u_int connection_in, connection_out;
-	u_int64_t rekey_timeout_ms = 0;
 	sigset_t bsigset, osigset;
 
 	debug("Entering interactive session for SSH2.");
@@ -358,13 +342,6 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
 		if (!ssh_packet_is_rekeying(ssh) &&
 		    ssh_packet_not_very_much_data_to_write(ssh))
 			channel_output_poll(ssh);
-		if (options.rekey_interval > 0 &&
-		    !ssh_packet_is_rekeying(ssh)) {
-			rekey_timeout_ms = ssh_packet_get_rekey_timeout(ssh) *
-			    1000;
-		} else {
-			rekey_timeout_ms = 0;
-		}
 
 		/*
 		 * Block SIGCHLD while we check for dead children, then pass
@@ -375,7 +352,7 @@ server_loop2(struct ssh *ssh, Authctxt *authctxt)
 			error_f("bsigset sigprocmask: %s", strerror(errno));
 		collect_children(ssh);
 		wait_until_can_do_something(ssh, connection_in, connection_out,
-		    &pfd, &npfd_alloc, &npfd_active, rekey_timeout_ms, &osigset,
+		    &pfd, &npfd_alloc, &npfd_active, &osigset,
 		    &conn_in_ready, &conn_out_ready);
 		if (sigprocmask(SIG_UNBLOCK, &bsigset, &osigset) == -1)
 			error_f("osigset sigprocmask: %s", strerror(errno));