Re: asynchronous channel write

[g4-lisz@xxxxxxxxxxxx]

On 15.01.20 23:45, Torsten Kuehnel wrote:
> On Wed, 15 Jan 2020 00:53:51 +0100
> g4-lisz@xxxxxxxxxxxx wrote:
>
>> On 13.01.20 22:54, Torsten Kuehnel wrote:
>>
>>> Looks to me your loop does not cope with partial writes correctly as
>>> you pass the initial (channel, buf, len) parameters unmodified in
>>> subsequent calls to ssh_channel_write, shall your innermost do {...}
>>> loop iterate more than once.
>>>
>>> Shouldn't you increment your buf pointer by the bytes already sent, or
>>> does libssh keep track of the write progress and does this for you ?
>> Hi Torsten,
>>
>> yes you are right! After reading the source of ssh_channel_write() I
>> totally agree. The buf argument is a const char* so it can't be modified
>> through ssh_channel_write()...
>>
>> I'm really suprised that we never had an issue with this... We tested
>> the code extensively.
> Hi,
> when you are feeding sockets and there is no network/interface load, every
> os (as far as i know of) tries to send/write the amount requested in whole.
>
> The task of the underlaying kernel is to avoid, when you request non
> blocking i/o, that the kernel *itself* does not has to wait. So when
> the kernel sees there is a small amount of room in the underlaying
> ethernet driver buffers, for example, he fills this space with (part)
> of your data and returns immediately, telling you how much could be send.
>
> Thats my assumption how the underlaying stuff works. Feel free to add
> any knowledge.
>
> The hard thing is to test such edge cases in your test suite. For my
> packet code for example i have tests which *simulate* low bandwidth
> by sending single or a few, half of a logical packet for example to cover
> at least the reading end dealing with partial stuff.
Yes I totally agree with what you write. But this is also a question of
which layer we are talking about.

On TCP level on Linux you can tweak things a lot, so maybe you can
provocate some edge cases on this way. For example by lowering some
socket buffer sizes.

But in this case (ssh_channel_write), from the point of view of
networking, we are on application level. I think the only situation when
a write to a channel can't write all bytes at once is when the needed
windows size was somehow changed or rejected by the peer...

Indeed we never tested this case, but it seems to happen only in such
rare situations that it's not notable in day-to-day operations. This
code is running for half a year now in a multithreaded multisession SSH
server deamon for receiving direct-tcp connections. It's tunneling old
plaintext protocols like telnet, but also SOCKS, so we have all
different kinds of packet sizes and bandwidths.  

Anyway I now corrected the code to this:

    len = recvfrom(...);
    do {
        i = ssh_channel_write(channel, buf + wr, len - wr);
        if (i < 0) {
             .....
        }
        wr += i;
    } while (i > 0 && wr < len);

Thanks a lot for the hint!


> Currently i still stuck on non blocking writing at all using libssh, as
> i have still no clue on how libssh poll code decides to take the POLLOUT
> event out of the session poll ctx handle ?

I never had the need to manipulate ctx handles directly. I only use
ssh_event_add_xxx() and ssh_event_remove_xxx().


> When i call ssh_event_add_fd with my socket and POLLOUT and a callback,
> is it save to call ssh_event_remove_fd afterwards to take it out and not
> break the underlaying rekeying and session/channel working stuff ?

Yes you can do this without affecting the other polling contexts. The
ony thing you should avoid is to do any event modification in a CB
function itslef. This can lead to recursions.

So in our case we use a queue for closing channels and sockets, which is
handled in the session main loop, i.e. synchronized with
ssh_event_dopoll(). While openig sockets is handled by dedicated threads.

If you are writing client code, you should have a look at connectors and
the ssh_client.c example.

Regards,
Till