On 12/12/22 10:15 PM, Damien Miller wrote:
On Mon, 12 Dec 2022, Chris Rapier wrote:
The value tests should probably be 'val <= 0' as opposed to 'val == 0'. Also
you might want to have the buffer size check for a maximum value. With
overhead, 256K triggers an "Outbound buffer too long" error. 255K seems to
work but I haven't done the exact math on that one yet. Lastly, if nrequests
is set to a ridiculous size (on my system more than 150000) it ends up sucking
up a lot of memory and stalls. I think that's a buyer beware sort of thing but
I just wanted to let you know.
Also, not defining the sftp_copy_buflen and sftp_nrequests means that in
do_sftp_connect the line
return do_init(*reminp, *remoutp,
sftp_copy_buflen, sftp_nrequests, limit_kbps);
Ends up with 0's in there for the buflen and requests. What sort of behaviour
does that create on the sftp server? It seems to just pull out the stops.
No, if you look at do_init() then it uses default values of 32K buffer
size and 64 concurrent requests.
Ah, I was missing that it was bringing in those values from sftp-client.h.
A couple of things
> +++ b/scp.c
> @@ -96,6 +96,7 @@
> #include <time.h>
> #include <unistd.h>
> #include <limits.h>
> +#include <util.h>
> #include <vis.h>
Is util.h a BSD library? My linux distro doesn't seem to have it. I did
find it in FreeBSD but it doesn't seem necessary in linux unless it's a
security feature.
+ 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;
It seems that 256 * 1024 is too large here.
"./scp -Xbuffer=262144 ~/50GB kilo:~
scp: Outbound message too long 262169"
I'm guessing control messages or something being appended is making it
exceed SFTP_MAX_MSG_LENGTH. The size seems consistent at 25 bytes. Perhaps:
if (r == 0 && (llv <= 0 || llv > SFTP_MAX_MSG_LENGTH - 1024)) {
r = -1;
errno = EINVAL;
}
That might be more informative as to why you are using that value and
give it some additional headroom.
+ }
+ 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);
In this one I would suggest making that smaller. On my tests 262144
requests leads to an immediate stall lasting for more than 5 minutes (at
which point I got bored and killed the process). I'd suggest something
closer to 8k. This gives a maximum of 256MB of outstanding data with the
default buffer size. That's enough for a 10G path at 200ms RTT.
I'd like to figure out why it stalls for so long with larger values but
that's for another time.
Also, once again, my thanks for this and all the other help you've
provided.
Chris
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
