add a ssh_packet_process_read() function that reads from a fd

directly into the transport input buffer.

Use this in the client and server mainloops to avoid unnecessary
copying. It also lets us use a more greedy read size without penalty.

Yields a 2-3% performance gain on cipher-speed.sh (in a fairly
unscientific test tbf)

feedback dtucker@ ok markus@

diff --git a/usr.bin/ssh/channels.h b/usr.bin/ssh/channels.h
index 23e4d4f..b515f83 100644 (file)
--- a/usr.bin/ssh/channels.h
+++ b/usr.bin/ssh/channels.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: channels.h,v 1.140 2022/01/06 21:48:38 djm Exp $ */
+/* $OpenBSD: channels.h,v 1.141 2022/01/22 00:49:34 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -244,6 +244,9 @@ struct Channel {
 /* Read buffer size */
 #define CHAN_RBUF      (16*1024)
 
+/* Maximum size for direct reads to buffers */
+#define CHANNEL_MAX_READ       CHAN_SES_PACKET_DEFAULT
+
 /* Maximum channel input buffer size */
 #define CHAN_INPUT_MAX (16*1024*1024)
 

diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
index 7a98db9..6e799da 100644 (file)
--- a/usr.bin/ssh/clientloop.c
+++ b/usr.bin/ssh/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.377 2022/01/21 07:04:19 djm Exp $ */
+/* $OpenBSD: clientloop.c,v 1.378 2022/01/22 00:49:34 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -623,39 +623,25 @@ client_suspend_self(struct sshbuf *bin, struct sshbuf *bout, struct sshbuf *berr
 static void
 client_process_net_input(struct ssh *ssh)
 {
-       char buf[8192];
-       int len;
+       int r;
 
        /*
         * Read input from the server, and add any such data to the buffer of
         * the packet subsystem.
         */
        schedule_server_alive_check();
-       /* Read as much as possible. */
-       len = read(connection_in, buf, sizeof(buf));
-       if (len == 0) {
-               /* Received EOF. The remote host has closed the connection. */
-               quit_message("Connection to %.300s closed by remote host.",
-                   host);
-               return;
-       }
-       /*
-        * There is a kernel bug on Solaris that causes poll to
-        * sometimes wake up even though there is no data available.
-        */
-       if (len == -1 && (errno == EAGAIN || errno == EINTR))
-               len = 0;
-
-       if (len == -1) {
-               /*
-                * An error has encountered.  Perhaps there is a
-                * network problem.
-                */
-               quit_message("Read from remote host %s: %s",
-                   host, strerror(errno));
-               return;
+       if ((r = ssh_packet_process_read(ssh, connection_in)) == 0)
+               return; /* success */
+       if (r == SSH_ERR_SYSTEM_ERROR) {
+               if (errno == EAGAIN || errno == EINTR)
+                       return;
+               if (errno == EPIPE) {
+                       quit_message("Connection to %s closed by remote host.",
+                           host);
+                       return;
+               }
        }
-       ssh_packet_process_incoming(ssh, buf, len);
+       quit_message("Read from remote host %s: %s", host, ssh_err(r));
 }
 
 static void

diff --git a/usr.bin/ssh/packet.c b/usr.bin/ssh/packet.c
index 96b6828..fc854a3 100644 (file)
--- a/usr.bin/ssh/packet.c
+++ b/usr.bin/ssh/packet.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.c,v 1.306 2022/01/21 06:58:06 djm Exp $ */
+/* $OpenBSD: packet.c,v 1.307 2022/01/22 00:49:34 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1763,6 +1763,31 @@ ssh_packet_process_incoming(struct ssh *ssh, const char *buf, u_int len)
        return 0;
 }
 
+/* Reads and buffers data from the specified fd */
+int
+ssh_packet_process_read(struct ssh *ssh, int fd)
+{
+       struct session_state *state = ssh->state;
+       int r;
+       size_t rlen;
+
+       if ((r = sshbuf_read(fd, state->input, PACKET_MAX_SIZE, &rlen)) != 0)
+               return r;
+
+       if (state->packet_discard) {
+               if ((r = sshbuf_consume_end(state->input, rlen)) != 0)
+                       return r;
+               state->keep_alive_timeouts = 0; /* ?? */
+               if (rlen >= state->packet_discard) {
+                       if ((r = ssh_packet_stop_discard(ssh)) != 0)
+                               return r;
+               }
+               state->packet_discard -= rlen;
+               return 0;
+       }
+       return 0;
+}
+
 int
 ssh_packet_remaining(struct ssh *ssh)
 {

diff --git a/usr.bin/ssh/packet.h b/usr.bin/ssh/packet.h
index ac03c27..c8817ff 100644 (file)
--- a/usr.bin/ssh/packet.h
+++ b/usr.bin/ssh/packet.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.h,v 1.93 2021/07/16 09:00:23 djm Exp $ */
+/* $OpenBSD: packet.h,v 1.94 2022/01/22 00:49:34 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -122,6 +122,7 @@ int  ssh_packet_read_expect(struct ssh *, u_int type);
 int      ssh_packet_read_poll(struct ssh *);
 int ssh_packet_read_poll2(struct ssh *, u_char *, u_int32_t *seqnr_p);
 int     ssh_packet_process_incoming(struct ssh *, const char *buf, u_int len);
+int     ssh_packet_process_read(struct ssh *, int);
 int      ssh_packet_read_seqnr(struct ssh *, u_char *, u_int32_t *seqnr_p);
 int      ssh_packet_read_poll_seqnr(struct ssh *, u_char *, u_int32_t *seqnr_p);
 

diff --git a/usr.bin/ssh/serverloop.c b/usr.bin/ssh/serverloop.c
index 82f8ee1..3b49719 100644 (file)
--- a/usr.bin/ssh/serverloop.c
+++ b/usr.bin/ssh/serverloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: serverloop.c,v 1.230 2022/01/06 21:55:23 djm Exp $ */
+/* $OpenBSD: serverloop.c,v 1.231 2022/01/22 00:49:34 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -268,27 +268,24 @@ wait_until_can_do_something(struct ssh *ssh,
 static int
 process_input(struct ssh *ssh, int connection_in)
 {
-       int r, len;
-       char buf[16384];
-
-       /* Read and buffer any input data from the client. */
-       len = read(connection_in, buf, sizeof(buf));
-       if (len == 0) {
-               verbose("Connection closed by %.100s port %d",
-                   ssh_remote_ipaddr(ssh), ssh_remote_port(ssh));
-               return -1;
-       } else if (len == -1) {
-               if (errno == EINTR || errno == EAGAIN)
+       int r;
+
+       if ((r = ssh_packet_process_read(ssh, connection_in)) == 0)
+               return 0; /* success */
+       if (r == SSH_ERR_SYSTEM_ERROR) {
+               if (errno == EAGAIN || errno == EINTR)
                        return 0;
+               if (errno == EPIPE) {
+                       verbose("Connection closed by %.100s port %d",
+                           ssh_remote_ipaddr(ssh), ssh_remote_port(ssh));
+                       return -1;
+               }
                verbose("Read error from remote host %s port %d: %s",
                    ssh_remote_ipaddr(ssh), ssh_remote_port(ssh),
                    strerror(errno));
                cleanup_exit(255);
        }
-       /* Buffer any received data. */
-       if ((r = ssh_packet_process_incoming(ssh, buf, len)) != 0)
-               fatal_fr(r, "ssh_packet_process_incoming");
-       return 0;
+       return -1;
 }
 
 /*