[openssh-commits] [openssh] 02/07: upstream: Use new limits at openssh.com protocol extension to let the

git+noreply at mindrot.org git+noreply at mindrot.org
Sat Apr 3 16:24:50 AEDT 2021


This is an automated email from the git hooks/post-receive script.

djm pushed a commit to branch master
in repository openssh.

commit 1339800fef8d0dfbfeabff71b34670105bcfddd2
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Wed Mar 31 22:16:34 2021 +0000

    upstream: Use new limits at openssh.com protocol extension to let the
    
    client select good limits based on what the server supports. Split the
    download and upload buffer sizes to allow them to be chosen independently.
    
    In practice (and assuming upgraded sftp/sftp-server at each end), this
    increases the download buffer 32->64KiB and the upload buffer
    32->255KiB.
    
    Patches from Mike Frysinger; ok dtucker@
    
    OpenBSD-Commit-ID: ebd61c80d85b951b794164acc4b2f2fd8e88606c
---
 sftp-client.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
 sftp-client.h |  13 ++++++-
 sftp.c        |   9 ++---
 3 files changed, 115 insertions(+), 18 deletions(-)

diff --git a/sftp-client.c b/sftp-client.c
index 7465c3d9..c76032c8 100644
--- a/sftp-client.c
+++ b/sftp-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.c,v 1.140 2021/03/10 04:58:45 djm Exp $ */
+/* $OpenBSD: sftp-client.c,v 1.141 2021/03/31 22:16:34 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm at openbsd.org>
  *
@@ -61,6 +61,12 @@
 extern volatile sig_atomic_t interrupted;
 extern int showprogress;
 
+/* Default size of buffer for up/download */
+#define DEFAULT_COPY_BUFLEN	32768
+
+/* Default number of concurrent outstanding requests */
+#define DEFAULT_NUM_REQUESTS	64
+
 /* Minimum amount of data to read at a time */
 #define MIN_READ_SIZE	512
 
@@ -77,7 +83,8 @@ extern int showprogress;
 struct sftp_conn {
 	int fd_in;
 	int fd_out;
-	u_int transfer_buflen;
+	u_int download_buflen;
+	u_int upload_buflen;
 	u_int num_requests;
 	u_int version;
 	u_int msg_id;
@@ -87,6 +94,7 @@ struct sftp_conn {
 #define SFTP_EXT_HARDLINK	0x00000008
 #define SFTP_EXT_FSYNC		0x00000010
 #define SFTP_EXT_LSETSTAT	0x00000020
+#define SFTP_EXT_LIMITS		0x00000040
 	u_int exts;
 	u_int64_t limit_kbps;
 	struct bwlimit bwlimit_in, bwlimit_out;
@@ -405,8 +413,10 @@ do_init(int fd_in, int fd_out, u_int transfer_buflen, u_int num_requests,
 	ret->msg_id = 1;
 	ret->fd_in = fd_in;
 	ret->fd_out = fd_out;
-	ret->transfer_buflen = transfer_buflen;
-	ret->num_requests = num_requests;
+	ret->download_buflen = ret->upload_buflen =
+	    transfer_buflen ? transfer_buflen : DEFAULT_COPY_BUFLEN;
+	ret->num_requests =
+	    num_requests ? num_requests : DEFAULT_NUM_REQUESTS;
 	ret->exts = 0;
 	ret->limit_kbps = 0;
 
@@ -469,6 +479,10 @@ do_init(int fd_in, int fd_out, u_int transfer_buflen, u_int num_requests,
 		    strcmp((char *)value, "1") == 0) {
 			ret->exts |= SFTP_EXT_LSETSTAT;
 			known = 1;
+		} else if (strcmp(name, "limits at openssh.com") == 0 &&
+		    strcmp((char *)value, "1") == 0) {
+			ret->exts |= SFTP_EXT_LIMITS;
+			known = 1;
 		}
 		if (known) {
 			debug2("Server supports extension \"%s\" revision %s",
@@ -482,16 +496,41 @@ do_init(int fd_in, int fd_out, u_int transfer_buflen, u_int num_requests,
 
 	sshbuf_free(msg);
 
+	/* Query the server for its limits */
+	if (ret->exts & SFTP_EXT_LIMITS) {
+		struct sftp_limits limits;
+		if (do_limits(ret, &limits) != 0)
+			fatal_f("limits failed");
+
+		/* If the caller did not specify, find a good value */
+		if (transfer_buflen == 0) {
+			ret->download_buflen = limits.read_length;
+			ret->upload_buflen = limits.write_length;
+			debug("Using server download size %u", ret->download_buflen);
+			debug("Using server upload size %u", ret->upload_buflen);
+		}
+
+		/* Use the server limit to scale down our value only */
+		if (num_requests == 0 && limits.open_handles) {
+			ret->num_requests =
+			    MINIMUM(DEFAULT_NUM_REQUESTS, limits.open_handles);
+			debug("Server handle limit %llu; using %u",
+			      (unsigned long long)limits.open_handles, ret->num_requests);
+		}
+	}
+
 	/* Some filexfer v.0 servers don't support large packets */
-	if (ret->version == 0)
-		ret->transfer_buflen = MINIMUM(ret->transfer_buflen, 20480);
+	if (ret->version == 0) {
+		ret->download_buflen = MINIMUM(ret->download_buflen, 20480);
+		ret->upload_buflen = MINIMUM(ret->upload_buflen, 20480);
+	}
 
 	ret->limit_kbps = limit_kbps;
 	if (ret->limit_kbps > 0) {
 		bandwidth_limit_init(&ret->bwlimit_in, ret->limit_kbps,
-		    ret->transfer_buflen);
+		    ret->download_buflen);
 		bandwidth_limit_init(&ret->bwlimit_out, ret->limit_kbps,
-		    ret->transfer_buflen);
+		    ret->upload_buflen);
 	}
 
 	return ret;
@@ -503,6 +542,56 @@ sftp_proto_version(struct sftp_conn *conn)
 	return conn->version;
 }
 
+int
+do_limits(struct sftp_conn *conn, struct sftp_limits *limits)
+{
+	u_int id, msg_id;
+	u_char type;
+	struct sshbuf *msg;
+	int r;
+
+	if ((conn->exts & SFTP_EXT_LIMITS) == 0) {
+		error("Server does not support limits at openssh.com extension");
+		return -1;
+	}
+
+	if ((msg = sshbuf_new()) == NULL)
+		fatal_f("sshbuf_new failed");
+
+	id = conn->msg_id++;
+	if ((r = sshbuf_put_u8(msg, SSH2_FXP_EXTENDED)) != 0 ||
+	    (r = sshbuf_put_u32(msg, id)) != 0 ||
+	    (r = sshbuf_put_cstring(msg, "limits at openssh.com")) != 0)
+		fatal_fr(r, "compose");
+	send_msg(conn, msg);
+	debug3("Sent message limits at openssh.com I:%u", id);
+
+	get_msg(conn, msg);
+
+	if ((r = sshbuf_get_u8(msg, &type)) != 0 ||
+	    (r = sshbuf_get_u32(msg, &msg_id)) != 0)
+		fatal_fr(r, "parse");
+
+	debug3("Received limits reply T:%u I:%u", type, msg_id);
+	if (id != msg_id)
+		fatal("ID mismatch (%u != %u)", msg_id, id);
+	if (type != SSH2_FXP_EXTENDED_REPLY) {
+		fatal("Expected SSH2_FXP_EXTENDED_REPLY(%u) packet, got %u",
+		      SSH2_FXP_EXTENDED_REPLY, type);
+	}
+
+	memset(limits, 0, sizeof(*limits));
+	if ((r = sshbuf_get_u64(msg, &limits->packet_length)) != 0 ||
+	    (r = sshbuf_get_u64(msg, &limits->read_length)) != 0 ||
+	    (r = sshbuf_get_u64(msg, &limits->write_length)) != 0 ||
+	    (r = sshbuf_get_u64(msg, &limits->open_handles)) != 0)
+		fatal_fr(r, "parse limits");
+
+	sshbuf_free(msg);
+
+	return 0;
+}
+
 int
 do_close(struct sftp_conn *conn, const u_char *handle, u_int handle_len)
 {
@@ -1236,7 +1325,7 @@ do_download(struct sftp_conn *conn, const char *remote_path,
 	else
 		size = 0;
 
-	buflen = conn->transfer_buflen;
+	buflen = conn->download_buflen;
 	if ((msg = sshbuf_new()) == NULL)
 		fatal_f("sshbuf_new failed");
 
@@ -1701,7 +1790,7 @@ do_upload(struct sftp_conn *conn, const char *local_path,
 	}
 
 	startid = ackid = id + 1;
-	data = xmalloc(conn->transfer_buflen);
+	data = xmalloc(conn->upload_buflen);
 
 	/* Read from local and write to remote */
 	offset = progress_counter = (resume ? c->size : 0);
@@ -1721,7 +1810,7 @@ do_upload(struct sftp_conn *conn, const char *local_path,
 		if (interrupted || status != SSH2_FX_OK)
 			len = 0;
 		else do
-			len = read(local_fd, data, conn->transfer_buflen);
+			len = read(local_fd, data, conn->upload_buflen);
 		while ((len == -1) &&
 		    (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK));
 
diff --git a/sftp-client.h b/sftp-client.h
index 32a24a3c..6f6c49fb 100644
--- a/sftp-client.h
+++ b/sftp-client.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.h,v 1.29 2020/12/04 02:41:10 djm Exp $ */
+/* $OpenBSD: sftp-client.h,v 1.30 2021/03/31 22:16:34 djm Exp $ */
 
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm at openbsd.org>
@@ -53,6 +53,14 @@ struct sftp_statvfs {
 	u_int64_t f_namemax;
 };
 
+/* Used for limits response on the wire from the server */
+struct sftp_limits {
+	u_int64_t packet_length;
+	u_int64_t read_length;
+	u_int64_t write_length;
+	u_int64_t open_handles;
+};
+
 /*
  * Initialise a SSH filexfer connection. Returns NULL on error or
  * a pointer to a initialized sftp_conn struct on success.
@@ -61,6 +69,9 @@ struct sftp_conn *do_init(int, int, u_int, u_int, u_int64_t);
 
 u_int sftp_proto_version(struct sftp_conn *);
 
+/* Query server limits */
+int do_limits(struct sftp_conn *, struct sftp_limits *);
+
 /* Close file referred to by 'handle' */
 int do_close(struct sftp_conn *, const u_char *, u_int);
 
diff --git a/sftp.c b/sftp.c
index fb3c08d1..b9804b56 100644
--- a/sftp.c
+++ b/sftp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp.c,v 1.206 2021/01/08 02:44:14 djm Exp $ */
+/* $OpenBSD: sftp.c,v 1.207 2021/03/31 22:16:34 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm at openbsd.org>
  *
@@ -70,9 +70,6 @@ typedef void EditLine;
 #include "sftp-common.h"
 #include "sftp-client.h"
 
-#define DEFAULT_COPY_BUFLEN	32768	/* Size of buffer for up/download */
-#define DEFAULT_NUM_REQUESTS	64	/* # concurrent outstanding requests */
-
 /* File to read commands from */
 FILE* infile;
 
@@ -2341,8 +2338,8 @@ main(int argc, char **argv)
 	extern int optind;
 	extern char *optarg;
 	struct sftp_conn *conn;
-	size_t copy_buffer_len = DEFAULT_COPY_BUFLEN;
-	size_t num_requests = DEFAULT_NUM_REQUESTS;
+	size_t copy_buffer_len = 0;
+	size_t num_requests = 0;
 	long long limit_kbps = 0;
 
 	/* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */

-- 
To stop receiving notification emails like this one, please contact
djm at mindrot.org.


More information about the openssh-commits mailing list