Difference in buffer behaviour between 8.89 and 8.9?

[Scott Neugroschl]

Randall, 

Isn't that a consequence of the limits on the nonstop READX call?

-----Original Message-----
From: openssh-unix-dev <openssh-unix-dev-bounces+scott_n=xypro.com at mindrot.org> On Behalf Of rsbecker at nexbridge.com
Sent: Thursday, May 19, 2022 11:33 AM
To: 'rapier' <rapier at psc.edu>; 'Damien Miller' <djm at mindrot.org>
Cc: openssh-unix-dev at mindrot.org
Subject: RE: Difference in buffer behaviour between 8.89 and 8.9?

This may be part of it for me also, but in the other direction. I cannot exceed 56000 bytes for any I/O unless through a buffered stream.

>-----Original Message-----
>From: openssh-unix-dev <openssh-unix-dev-
>bounces+rsbecker=nexbridge.com at mindrot.org> On Behalf Of rapier
>Sent: May 19, 2022 2:22 PM
>To: Damien Miller <djm at mindrot.org>
>Cc: openssh-unix-dev at mindrot.org
>Subject: Re: Difference in buffer behaviour between 8.89 and 8.9?
>
>I think I have a fix, or at least a better band aid, for people in my
situation. If, at
>some point, you decide to increase the size of CHAN_SES_WINDOW_DEFAULT 
>this may be helpful.
>
>Basically, the changes to channels.h increases the size of SSH's 
>receive
buffer,
>which triggers this behaviour. The changes in sshbuf.c tests to see if 
>the
buffer is
>larger than ssh buffer that seems to be handling the incoming packets. 
>The assumption is that if it's larger then it's the receive buffer. I 
>then
aggressively grow
>that buffer (4MB at a time) but no more buf->max_size. The debug 
>statement
is
>so you can see what's going on but that should be removed for production.
>
>If you comment out the section that grows the window aggressively you 
>can
see
>the stall as the buffer grows 32k at a time. I have some concerns about
growing
>the buffer it such large chunks. Also, I'd ideally make it a function 
>of
the size of c-
>>local_window_max but I can't seem to get an extern working.
>
>Chris
>
>This patch is against the head of the master branch of openssh-portable.
>
>diff --git a/channels.h b/channels.h
>index 828c1b61b..ae897680c 100644
>--- a/channels.h
>+++ b/channels.h
>@@ -210,7 +210,7 @@ struct Channel {
>
>  /* default window/packet sizes for tcp/x11-fwd-channel */
>  #define CHAN_SES_PACKET_DEFAULT        (32*1024)
>-#define CHAN_SES_WINDOW_DEFAULT        (64*CHAN_SES_PACKET_DEFAULT)
>+#define CHAN_SES_WINDOW_DEFAULT
>(4096*CHAN_SES_PACKET_DEFAULT)
>  #define CHAN_TCP_PACKET_DEFAULT        (32*1024)
>  #define CHAN_TCP_WINDOW_DEFAULT        (64*CHAN_TCP_PACKET_DEFAULT)
>  #define CHAN_X11_PACKET_DEFAULT        (16*1024)
>diff --git a/sshbuf.c b/sshbuf.c
>index 840b899b1..b45720e1f 100644
>--- a/sshbuf.c
>+++ b/sshbuf.c
>@@ -27,6 +27,7 @@
>  #include "ssherr.h"
>  #include "sshbuf.h"
>  #include "misc.h"
>+#include "log.h"
>
>  static inline int
>  sshbuf_check_sanity(const struct sshbuf *buf) @@ -327,9 +328,27 @@ 
>sshbuf_allocate(struct sshbuf *buf, size_t len)
>          */
>         need = len + buf->size - buf->alloc;
>         rlen = ROUNDUP(buf->alloc + need, SSHBUF_SIZE_INC);
>+       /* I think the receive buffer is the one that is
>+        * growing slowly. This buffer quickly ends up being larger
>+        * than the typical packet buffer (usually 32.25KB) so if
>+        * we explicitly test for growth needs larger than that we
>+        * can accelerate the growth of this buffer and reduce load
>+        * the CPU and improve throughput. Ideally we would use
>+        * (local_window_max - rlen) as need but we don't have access
>+        * to that here */
>+       if (rlen > 34*1024) {
>+               need = 4 * 1024 * 1024;
>+               rlen = ROUNDUP(buf->alloc + need, SSHBUF_SIZE_INC);
>+               if (rlen > buf->max_size)
>+                       rlen = buf->max_size;
>+       }
>         SSHBUF_DBG(("need %zu initial rlen %zu", need, rlen));
>         if (rlen > buf->max_size)
>                 rlen = buf->alloc + need;
>+       /* be sure to include log.h if you uncomment the debug
>+        * this debug helped identify the buffer growth issue in v8.9
>+        * see the git log about it. search for sshbuf_read */
>+       debug("adjusted rlen: %zu, len: %lu for %p", rlen, len, buf);
>         SSHBUF_DBG(("adjusted rlen %zu", rlen));
>         if ((dp = recallocarray(buf->d, buf->alloc, rlen, 1)) == NULL) {
>                 SSHBUF_DBG(("realloc fail")); @@ -401,4 +420,3 @@ 
>sshbuf_consume_end(struct sshbuf *buf, size_t len)
>         SSHBUF_TELL("done");
>         return 0;
>  }
>
>
>On 5/19/22 11:23 AM, rapier wrote:
>> Damien
>>
>>> 8.9 switch from select() to poll() and included a couple of bugs 
>>> that could cause weird problems. IMO you should try to port to 
>>> what's on the top of the V_9_0 git branch, which is 9.0 + one more 
>>> poll()- related fix.
>>
>> I just tried it with the head (commit bedb93415b) of the master branch.
>> Unfortunately, I'm still seeing the issue.
>>
>> debug1: adjusted rlen: 33024, len: 32788 for 0x5609805a3eb0
>> debug1: adjusted rlen: 36438016, len: 32768 for 0x5609805b0ed0
>> debug1: adjusted rlen: 33024, len: 32788 for 0x5609805a3eb0
>> debug1: adjusted rlen: 36470784, len: 32768 for 0x5609805b0ed0
>> debug1: adjusted rlen: 33024, len: 32788 for 0x5609805a3eb0
>> debug1: adjusted rlen: 36503552, len: 32768 for 0x5609805b0ed0
>>
>> Chris
>_______________________________________________
>openssh-unix-dev mailing list
>openssh-unix-dev at mindrot.org
>https://linkprotect.cudasvc.com/url?a=https%3a%2f%2flists.mindrot.org%2
>fmailman%2flistinfo%2fopenssh-unix-dev&c=E,1,50VJZTQXRUdUXTEwgfs20i0gpr
>m86flr2uBmX13I7_aUSLTmp7fTXXTn5CytR1N1UDctCiUGQW-3t_4-SaIEjdYe1kTTsnel3
>UmfvFwkHrU4Sg,,&typo=1

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev at mindrot.org
https://linkprotect.cudasvc.com/url?a=https%3a%2f%2flists.mindrot.org%2fmailman%2flistinfo%2fopenssh-unix-dev&c=E,1,SmySto8ksZw6pYMAttooee6uwhmQWL12sdiMti7O5hQO3xmxl-jKai0Ov1ZnkmR_0YbGT2tpOz9XQZRL2do2o_weY7FdhnvvghG7YR7U&typo=1