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
On Wed, 6 Sep 2023, rsbecker at nexbridge.com wrote:> 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?Is this true for all read() calls or just particular ones? If it's for all read() calls, then maybe wrapping/replacing the read() function more generally would be needed. channels.c isn't the only place in OpenSSH that might do a large read... -d