From c3c0dfb380fbca4b9553ede646b6714abe172a92 Mon Sep 17 00:00:00 2001 From: djm Date: Sat, 22 Jan 2022 00:49:34 +0000 Subject: [PATCH] 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@ --- usr.bin/ssh/channels.h | 5 ++++- usr.bin/ssh/clientloop.c | 40 +++++++++++++--------------------------- usr.bin/ssh/packet.c | 27 ++++++++++++++++++++++++++- usr.bin/ssh/packet.h | 3 ++- usr.bin/ssh/serverloop.c | 29 +++++++++++++---------------- 5 files changed, 58 insertions(+), 46 deletions(-) diff --git a/usr.bin/ssh/channels.h b/usr.bin/ssh/channels.h index 23e4d4f78e7..b515f83cb9f 100644 --- 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 @@ -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 7a98db945da..6e799daa0d0 100644 --- 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 * Copyright (c) 1995 Tatu Ylonen , 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 96b68289ee6..fc854a3cd95 100644 --- 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 * Copyright (c) 1995 Tatu Ylonen , 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 ac03c27aa21..c8817ffa21a 100644 --- 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 @@ -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 82f8ee14633..3b49719824b 100644 --- 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 * Copyright (c) 1995 Tatu Ylonen , 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; } /* -- 2.20.1