On Sat, 28 Nov 2009, Peter Stuge wrote:
I agree. But then there's nothing that prevents the lower layer functions
to just use as much data it can and not to cram in more than it should into
the outgoing packages.
There is for SFTP, since it is a packet based protocol. Each FXP packet
starts with a length, and another FXP packet can not be started until the
full packet has been sent. sftp.c must know and adjust the packet length
(first 4 bytes in the SFTP packet) if lower layers can not reliably send the
full packet.
I know that's how SFTP works and I still don't understand how that
protocol design prevents our API/ABI to work.
App wants to send file with SFTP and calls libssh2_sftp_open().
App sends first 32k of file, expecting 32768 bytes to go out. sftp.c adds
FXP protocol header and writes to the channel. channel.c adds channel
protocol header and sends to transport. transport.c again adds some bytes
and must now send a total of 32820 bytes onto the wire.
Yes, that's pretty much equals how an app that sends TCP uses send() but the
100 bytes the app sends equals many more IP bytes and even more ethernet
bytes.
The fact that send() and recv() return such values, and all SSL libraries I've
used return such numbers for their SSL_send()/SSL_recv() equivalents tell me
that's a well used paradigm that I think we should keep.
The current API without sync between upper and lower layer numbers is
bad since information gets lost.
No, it's not lost information. It's a simplified world view, and I don't see
how that's bad. An app that uses SFTP doesn't necessarily know about the lower
layers of SSH/TCP/IP/ethernet anyway.
In order to silently work, *each* layer would require *another* state
machine which would be used to split an upper layer buffer into
multiple manageable chunks - which trickles down through all lower
layers whenever an application happens to hit the buffer size limit
for a particular layer.
I disagree. Your description is one approach to rework this to function
optimally. The current way of doing it still works. It's just a bit lame with
buffer sizes.
I'd like to work towards less state within libssh2, not more. :)
We will always have lots of state within libssh2. The challenge is to make the
states readable and understandable.
The ABI could remain, but the API semantics need to change, and it's not so
nice to make an incompatible API change without also changing the ABI.
(soname I guess, but is there a big difference?)
If API semantics change, then I claim the ABI is changed as well. ABI is not
just about being able to still link IMHO.
But again I don't see why this is a MUST to fix our problems.
I hope the above explains it - upper layers measure progress by how many of
"their" bytes that have been sent, but each lower layer adds however many
bytes it needs and there's no way for the top layer to know what is actually
going out on the wire - yet "bytes sent" is being used to measure progress
and track state.
Sent bytes can be used to measure state within the protocol layer, but it can
of course not tell anything about the state in other layers. And it _can_ be
used to measure progress, again in exactly the same way we can track a TCP
transfer by counting how many bytes send() tells us it has transfered.
AFAIU libssh2 write functions returning 0 are not really well defined nor
consistent, but one possibility might be to "hide" the extra bytes that way;
lower layer returns 0 until all it's extra bytes have been sent.
I agree that we have some internal confusions about a return value 0, but the
fact is that we MUST never treat reading or writing zero bytes as an error
exactly because of the reasons you mention: it simply means that we didn't
read or send any payload data but lots of protocol bits may still have flown
over the wire. 0 is progress, just very little progress ;-)
Any other dealing with 0 is a bug in my eyes.
Another solution is that upper layer learns about lower layer needs; instead
of counting up to 32768 in the app, it gets the 32820 number to measure
progress on the wire. This is mildly confusing for app writers, but it would
work well.
It would be completely crazy in my mind. What exactly would be included in
that count? The SFTP_write() function is for sending N bytes to the peer. We
want to know how many of those N bytes that were sent - and thus how many more
we need to send before we are done. It is of no value to the app to know that
when we wanted to send N bytes, M bytes were actually sent.
A better solution would be great! In any case I don't think the API can
remain as is.
I disagree. The problems are not in the API or ABI, the problems are in the
implementation of the functions we have.
--
/ daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
