add a -X option to both scp(1) and sftp(1) to allow control over

some SFTP protocol knobs: the copy buffer length and the number of
inflight requests, both of which are used during upload/download.

Previously these could be controlled in sftp(1) using the -b/-R options.
This makes them available in both SFTP protocol clients using the same
option character sequence.

ok dtucker@

diff --git a/usr.bin/ssh/scp.1 b/usr.bin/ssh/scp.1
index cd23f97..61e4963 100644 (file)
--- a/usr.bin/ssh/scp.1
+++ b/usr.bin/ssh/scp.1
@@ -8,9 +8,9 @@
 .\"
 .\" Created: Sun May  7 00:14:37 1995 ylo
 .\"
-.\" $OpenBSD: scp.1,v 1.110 2022/09/19 21:39:16 djm Exp $
+.\" $OpenBSD: scp.1,v 1.111 2022/12/16 03:40:03 djm Exp $
 .\"
-.Dd $Mdocdate: September 19 2022 $
+.Dd $Mdocdate: December 16 2022 $
 .Dt SCP 1
 .Os
 .Sh NAME
@@ -28,6 +28,7 @@
 .Op Fl o Ar ssh_option
 .Op Fl P Ar port
 .Op Fl S Ar program
+.Op Fl X Ar sftp_option
 .Ar source ... target
 .Sh DESCRIPTION
 .Nm
@@ -278,6 +279,19 @@ and
 to print debugging messages about their progress.
 This is helpful in
 debugging connection, authentication, and configuration problems.
+.It Fl X Ar sftp_option
+Specify an option that controls aspects of SFTP protocol behaviour.
+The valid options are:
+.Bl -tag -width Ds
+.It Cm nrequests Ns = Ns Ar value
+Controls how many concurrent SFTP read or write requests may be in progress
+at any point in time during a download or upload.
+By default 64 requests may be active concurrently.
+.It Cm buffer Ns = Ns Ar value
+Controls the maximum buffer size for a single SFTP read/write operation used
+during download or upload.
+By default a 32KB buffer is used.
+.El
 .El
 .Sh EXIT STATUS
 .Ex -std scp

diff --git a/usr.bin/ssh/scp.c b/usr.bin/ssh/scp.c
index c7194c2..0363e55 100644 (file)
--- a/usr.bin/ssh/scp.c
+++ b/usr.bin/ssh/scp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.249 2022/10/24 21:51:55 djm Exp $ */
+/* $OpenBSD: scp.c,v 1.250 2022/12/16 03:40:03 djm Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -96,6 +96,7 @@
 #include <time.h>
 #include <unistd.h>
 #include <limits.h>
+#include <util.h>
 #include <vis.h>
 
 #include "xmalloc.h"
@@ -150,6 +151,10 @@ char *ssh_program = _PATH_SSH_PROGRAM;
 pid_t do_cmd_pid = -1;
 pid_t do_cmd_pid2 = -1;
 
+/* SFTP copy parameters */
+size_t sftp_copy_buflen;
+size_t sftp_nrequests;
+
 /* Needed for sftp */
 volatile sig_atomic_t interrupted = 0;
 
@@ -418,13 +423,14 @@ void throughlocal_sftp(struct sftp_conn *, struct sftp_conn *,
 int
 main(int argc, char **argv)
 {
-       int ch, fflag, tflag, status, n;
+       int ch, fflag, tflag, status, r, n;
        char **newargv, *argv0;
        const char *errstr;
        extern char *optarg;
        extern int optind;
        enum scp_mode_e mode = MODE_SFTP;
        char *sftp_direct = NULL;
+       long long llv;
 
        /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
        sanitise_stdfd();
@@ -452,7 +458,7 @@ main(int argc, char **argv)
 
        fflag = Tflag = tflag = 0;
        while ((ch = getopt(argc, argv,
-           "12346ABCTdfOpqRrstvD:F:J:M:P:S:c:i:l:o:")) != -1) {
+           "12346ABCTdfOpqRrstvD:F:J:M:P:S:c:i:l:o:X:")) != -1) {
                switch (ch) {
                /* User-visible flags. */
                case '1':
@@ -533,6 +539,31 @@ main(int argc, char **argv)
                        addargs(&remote_remote_args, "-q");
                        showprogress = 0;
                        break;
+               case 'X':
+                       /* Please keep in sync with sftp.c -X */
+                       if (strncmp(optarg, "buffer=", 7) == 0) {
+                               r = scan_scaled(optarg + 7, &llv);
+                               if (r == 0 && (llv <= 0 || llv > 256 * 1024)) {
+                                       r = -1;
+                                       errno = EINVAL;
+                               }
+                               if (r == -1) {
+                                       fatal("Invalid buffer size \"%s\": %s",
+                                            optarg + 7, strerror(errno));
+                               }
+                               sftp_copy_buflen = (size_t)llv;
+                       } else if (strncmp(optarg, "nrequests=", 10) == 0) {
+                               llv = strtonum(optarg + 10, 1, 256 * 1024,
+                                   &errstr);
+                               if (errstr != NULL) {
+                                       fatal("Invalid number of requests "
+                                           "\"%s\": %s", optarg + 10, errstr);
+                               }
+                               sftp_nrequests = (size_t)llv;
+                       } else {
+                               fatal("Invalid -X option");
+                       }
+                       break;
 
                /* Server options. */
                case 'd':
@@ -941,7 +972,8 @@ do_sftp_connect(char *host, char *user, int port, char *sftp_direct,
                    reminp, remoutp, pidp) < 0)
                        return NULL;
        }
-       return do_init(*reminp, *remoutp, 32768, 64, limit_kbps);
+       return do_init(*reminp, *remoutp,
+           sftp_copy_buflen, sftp_nrequests, limit_kbps);
 }
 
 void

diff --git a/usr.bin/ssh/sftp-client.c b/usr.bin/ssh/sftp-client.c
index a3e7a5d..73b598a 100644 (file)
--- a/usr.bin/ssh/sftp-client.c
+++ b/usr.bin/ssh/sftp-client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.c,v 1.165 2022/09/19 10:43:12 djm Exp $ */
+/* $OpenBSD: sftp-client.c,v 1.166 2022/12/16 03:40:03 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -55,10 +55,10 @@
 extern volatile sig_atomic_t interrupted;
 extern int showprogress;
 
-/* Default size of buffer for up/download */
+/* Default size of buffer for up/download (fix sftp.1 scp.1 if changed) */
 #define DEFAULT_COPY_BUFLEN    32768
 
-/* Default number of concurrent outstanding requests */
+/* Default number of concurrent xfer requests (fix sftp.1 scp.1 if changed) */
 #define DEFAULT_NUM_REQUESTS   64
 
 /* Minimum amount of data to read at a time */

diff --git a/usr.bin/ssh/sftp.1 b/usr.bin/ssh/sftp.1
index 3b3f2c5..68923ae 100644 (file)
--- a/usr.bin/ssh/sftp.1
+++ b/usr.bin/ssh/sftp.1
@@ -1,4 +1,4 @@
-.\" $OpenBSD: sftp.1,v 1.142 2022/09/19 21:39:16 djm Exp $
+.\" $OpenBSD: sftp.1,v 1.143 2022/12/16 03:40:03 djm Exp $
 .\"
 .\" Copyright (c) 2001 Damien Miller.  All rights reserved.
 .\"
@@ -22,7 +22,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: September 19 2022 $
+.Dd $Mdocdate: December 16 2022 $
 .Dt SFTP 1
 .Os
 .Sh NAME
@@ -44,6 +44,7 @@
 .Op Fl R Ar num_requests
 .Op Fl S Ar program
 .Op Fl s Ar subsystem | sftp_server
+.Op Fl X Ar sftp_option
 .Ar destination
 .Sh DESCRIPTION
 .Nm
@@ -320,6 +321,19 @@ does not have an sftp subsystem configured.
 .It Fl v
 Raise logging level.
 This option is also passed to ssh.
+.It Fl X Ar sftp_option
+Specify an option that controls aspects of SFTP protocol behaviour.
+The valid options are:
+.Bl -tag -width Ds
+.It Cm nrequests Ns = Ns Ar value
+Controls how many concurrent SFTP read or write requests may be in progress
+at any point in time during a download or upload.
+By default 64 requests may be active concurrently.
+.It Cm buffer Ns = Ns Ar value
+Controls the maximum buffer size for a single SFTP read/write operation used
+during download or upload.
+By default a 32KB buffer is used.
+.El
 .El
 .Sh INTERACTIVE COMMANDS
 Once in interactive mode,

diff --git a/usr.bin/ssh/sftp.c b/usr.bin/ssh/sftp.c
index 02547f6..a224685 100644 (file)
--- a/usr.bin/ssh/sftp.c
+++ b/usr.bin/ssh/sftp.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp.c,v 1.222 2022/09/19 10:46:00 djm Exp $ */
+/* $OpenBSD: sftp.c,v 1.223 2022/12/16 03:40:03 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -2383,7 +2383,7 @@ main(int argc, char **argv)
        struct sftp_conn *conn;
        size_t copy_buffer_len = 0;
        size_t num_requests = 0;
-       long long limit_kbps = 0;
+       long long llv, limit_kbps = 0;
 
        /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
        sanitise_stdfd();
@@ -2400,7 +2400,7 @@ main(int argc, char **argv)
        infile = stdin;
 
        while ((ch = getopt(argc, argv,
-           "1246AafhNpqrvCc:D:i:l:o:s:S:b:B:F:J:P:R:")) != -1) {
+           "1246AafhNpqrvCc:D:i:l:o:s:S:b:B:F:J:P:R:X:")) != -1) {
                switch (ch) {
                /* Passed through to ssh(1) */
                case 'A':
@@ -2497,6 +2497,31 @@ main(int argc, char **argv)
                        ssh_program = optarg;
                        replacearg(&args, 0, "%s", ssh_program);
                        break;
+               case 'X':
+                       /* Please keep in sync with ssh.c -X */
+                       if (strncmp(optarg, "buffer=", 7) == 0) {
+                               r = scan_scaled(optarg + 7, &llv);
+                               if (r == 0 && (llv <= 0 || llv > 256 * 1024)) {
+                                       r = -1;
+                                       errno = EINVAL;
+                               }
+                               if (r == -1) {
+                                       fatal("Invalid buffer size \"%s\": %s",
+                                            optarg + 7, strerror(errno));
+                               }
+                               copy_buffer_len = (size_t)llv;
+                       } else if (strncmp(optarg, "nrequests=", 10) == 0) {
+                               llv = strtonum(optarg + 10, 1, 256 * 1024,
+                                   &errstr);
+                               if (errstr != NULL) {
+                                       fatal("Invalid number of requests "
+                                           "\"%s\": %s", optarg + 10, errstr);
+                               }
+                               num_requests = (size_t)llv;
+                       } else {
+                               fatal("Invalid -X option");
+                       }
+                       break;
                case 'h':
                default:
                        usage();