openssh unix dev - May 2022 - Difference in buffer behaviour between 8.89 and 8.9?

On Thu, 19 May 2022, rapier wrote:
> 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.
I don't understand the problem and I don't understand the fix, sorry.

The sshbuf_read() in packet.c will always reserve PACKET_MAX_SIZE (256k)
in addition to whatever is in the buffer currently and will greedily try to
fill it (up to max_size).

Is there some other buffer that is growing more slowly?

If so, I don't think the answer is to change the buffer code but instead
to sshbuf_reserve() an appropriate size before filling data into it.

-d

On 5/19/22 7:29 PM, Damien Miller wrote:
> On Thu, 19 May 2022, rapier wrote:
> 
>> 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.
> 
> I don't understand the problem and I don't understand the fix,
sorry.
I don't really understand the problem either whcih means the fix really 
is a band aid at this point.
> The sshbuf_read() in packet.c will always reserve PACKET_MAX_SIZE (256k)
> in addition to whatever is in the buffer currently and will greedily try to
> fill it (up to max_size).
The problem is that I'm not seeing that behaviour. It's pretty easy to 
see though - just up the default window and drop that debug into 
sshbuf.c. It's easier to see if you ramp up the delay with netem but 
even with no delay it's there. Either way, I'm only seeing it ramp up by
32K at a time.
> Is there some other buffer that is growing more slowly?
I honestly don't know. No small part of this is guess work on my part 
but there is some buffer that's growing pretty slowly in this situation.
> If so, I don't think the answer is to change the buffer code but
instead
> to sshbuf_reserve() an appropriate size before filling data into it.
I'll look into that but I'm in a place where figuring out where to start
can be tricky (my knowledge of the sshbuf code is, to be sure, weak) so 
I'm mostly working backwards. I got to where I am by profiling via 
vtune. sshbuf_reserve() is in the call stack so I'll start going 
backwards again.


Chris