rsbecker at nexbridge.com
2022-May-19 18:33 UTC
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 mysituation. 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 receivebuffer,>which triggers this behaviour. The changes in sshbuf.c tests to see if thebuffer 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 thenaggressively grow>that buffer (4MB at a time) but no more buf->max_size. The debug statementis>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 cansee>the stall as the buffer grows 32k at a time. I have some concerns aboutgrowing>the buffer it such large chunks. Also, I'd ideally make it a function ofthe 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://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Scott Neugroschl
2022-May-19 21:21 UTC
Difference in buffer behaviour between 8.89 and 8.9?
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 mysituation. 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 >receivebuffer,>which triggers this behaviour. The changes in sshbuf.c tests to see if >thebuffer 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 >thenaggressively grow>that buffer (4MB at a time) but no more buf->max_size. The debug >statementis>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 >cansee>the stall as the buffer grows 32k at a time. I have some concerns aboutgrowing>the buffer it such large chunks. Also, I'd ideally make it a function >ofthe 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