replace manual poll/ppoll timeout math with ptimeout API
authordjm <djm@openbsd.org>
Fri, 6 Jan 2023 02:38:23 +0000 (02:38 +0000)
committerdjm <djm@openbsd.org>
Fri, 6 Jan 2023 02:38:23 +0000 (02:38 +0000)
feedback markus / ok markus dtucker

usr.bin/ssh/channels.c
usr.bin/ssh/channels.h
usr.bin/ssh/clientloop.c
usr.bin/ssh/serverloop.c

index 8b91995..a1a7f51 100644 (file)
@@ -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 */
index 1896101..c04f5a7 100644 (file)
@@ -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 *);
 
index 5b1d5d8..19a5d4b 100644 (file)
@@ -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) {
                /*
index 2210436..5347e33 100644 (file)
@@ -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));