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
rsbecker at nexbridge.com
2022-May-19 22:04 UTC
Difference in buffer behaviour between 8.89 and 8.9?
WRITEX, yes. And write() and fwrite(). It is a file system limit, but there are replacement procedures available now, just not for sockets.>-----Original Message----- >From: openssh-unix-dev <openssh-unix-dev- >bounces+rsbecker=nexbridge.com at mindrot.org> On Behalf Of Scott Neugroschl >Sent: May 19, 2022 5:21 PM >To: rsbecker at nexbridge.com; '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? > >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 cannotexceed 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%2fmail>man%2flistinfo%2fopenssh-unix- >dev&c=E,1,SmySto8ksZw6pYMAttooee6uwhmQWL12sdiMti7O5hQO3xmxl- >jKai0Ov1ZnkmR_0YbGT2tpOz9XQZRL2do2o_weY7FdhnvvghG7YR7U&typo=1 >_______________________________________________ >openssh-unix-dev mailing list >openssh-unix-dev at mindrot.org >https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
rsbecker at nexbridge.com
2023-Sep-06 16:41 UTC
Difference in buffer behaviour between 8.89 and 8.9?
On Thursday, May 19, 2022 5:21 PM, Scott Neugroschl wrote:>Isn't that a consequence of the limits on the nonstop READX call?Yes it is an OS limit. I have been able to partially get this to work as follows: @@ -281,6 +281,10 @@ sshbuf_read(int fd, struct sshbuf *buf, size_t maxlen, size_t *rlen) if (rlen != NULL) *rlen = 0; +#if defined (__TANDEM) + if (maxlen > SSIZE_MAX) + maxlen = SSIZE_MAX; // About 56Kb from limits.h +#endif But I cannot figure out the best way to deal with the writes or the most acceptable way to the OpenSSH team. Any suggestions? Thanks, Randall> >-----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 cannotexceed 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 >_______________________________________________ >openssh-unix-dev mailing list >openssh-unix-dev at mindrot.org >https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev