The default channel window size used until now was 256KB. This value is
too small and results on a bottleneck on real-life networks where
round-trip delays can easily reach 300ms.
The issue was not visible because the configured channel window size
was being ignored and a hard-coded value of ~22MB being used instead,
but that was fixed on a previous commit.
This patch just changes the default window size
(LIBSSH2_CHANNEL_WINDOW_DEFAULT) to 2MB. It is the same value used by
OpenSSH and in our opinion represents a good compromise between memory
used and transfer speed.
Performance tests were run to determine the optimum value. The details
and related discussion are available from the following thread on the
libssh2 mailing-list:
http://www.libssh2.org/mail/libssh2-devel-archive-2013-10/0018.shtmlhttp://article.gmane.org/gmane.network.ssh.libssh2.devel/6543
An excerpt follows:
"I have been running some transfer test and measuring their speed.
My setup was composed of a quad-core Linux machine running Ubuntu 13.10
x86_64 with a LXC container inside. The data transfers were performed
from the container to the host (never crossing through a physical
network device).
Network delays were simulated using the tc tool. And ping was used to
verify that they worked as intended during the tests.
The operation performed was the equivalent to the following ssh command:
$ ssh container "dd bs=16K count=8K if=/dev/zero" >/dev/null
Though, establishment and closing of the SSH connection was excluded
from the timings.
I run the tests several times transferring files of sizes up to 128MB
and the results were consistent between runs.
The results corresponding to the 128MB transfer are available here:
https://docs.google.com/spreadsheet/ccc?key=0Ao1yRmX6PQQzdG5wSFlrZl9HRWNET3ZyN0hnaGo5ZFE&usp=sharing
It clearly shows that 256KB is too small as the default window size.
Moving to a 512MB generates a great improvement and after the 1MB mark
the returns rapidly diminish. Other factors (TCP window size, probably)
become more limiting than the channel window size
For comparison I also performed the same transfers using OpenSSH. Its
speed is usually on par with that of libssh2 using a window size of 1MB
(even if it uses a 2MB window, maybe it is less aggressive sending the
window adjust msgs)."
Signed-off-by: Salvador Fandino <sfandino@yahoo.com>
_libssh2_channel_read was using an arbitrary hard-coded limit to trigger
the window adjusting code. The adjustment used was also hard-coded and
arbitrary, 15MB actually, which would limit the usability of libssh2 on
systems with little RAM.
This patch, uses the window_size parameter passed to
libssh2_channel_open_ex (stored as remote.window_size_initial) plus the
buflen as the base for the trigger and the adjustment calculation.
The memory usage when using the default window size is reduced from 22MB
to 256KB per channel (actually, if compression is used, these numbers
should be incremented by ~50% to account for the errors between the
decompressed packet sizes and the predicted sizes).
My tests indicate that this change does not impact the performance of
transfers across localhost or a LAN, being it on par with that of
OpenSSH. On the other hand, it will probably slow down transfers on
networks with high bandwidth*delay when the default window size
(LIBSSH2_CHANNEL_WINDOW_DEFAULT=256KB) is used.
Signed-off-by: Salvador Fandino <sfandino@yahoo.com>
Store but don't use keys of unsupported types on the known_hosts file.
Currently, when libssh2 parses a known_host file containing keys of some
type it doesn't natively support, it stops reading the file and returns
an error.
That means, that the known_host file can not be safely shared with other
software supporting other key types (i.e. OpenSSH).
This patch adds support for handling keys of unknown type. It can read
and write them, even if they are never going to be matched.
At the source level the patch does the following things:
- add a new unknown key type LIBSSH2_KNOWNHOST_KEY_UNKNOWN
- add a new slot (key_type_name) on the known_host struct that is
used to store the key type in ascii form when it is not supported
- parse correctly known_hosts entries with unknown key types and
populate the key_type_name slot
- print correctly known_hosts entries of unknown type
- when checking a host key ignore keys that do not match the key
Fixes#276
Deflate may return Z_OK even when not all data has been compressed
if the output buffer becomes full.
In practice this is very unlikely to happen because the output buffer
size is always some KBs larger than the size of the data passed for
compression from the upper layers and I think that zlib never expands
the data so much, even on the worst cases.
Anyway, this patch plays on the safe side checking that the output
buffer is not exhausted.
Signed-off-by: Salvador <sfandino@yahoo.com>
The old algorithm was O(N^2), causing lots and lots of reallocations
when highly compressed data was transferred.
This patch implements a simpler one that just doubles the buffer size
everytime it is exhausted. It results in O(N) complexity.
Also a smaller inflate ratio is used to calculate the initial size (x4).
Signed-off-by: Salvador <sfandino@yahoo.com>
Data may remain in zlib internal buffers when inflate() returns Z_OK
and avail_out == 0. In that case, inflate has to be called again.
Also, once all the data has been inflated, it returns Z_BUF_ERROR to
signal that the input buffer has been exhausted.
Until now, the way to detect that a packet payload had been completely
decompressed was to check that no data remained on the input buffer
but that didn't account for the case where data remained on the internal
zlib buffers.
That resulted in packets not being completely decompressed and the
missing data reappearing on the next packet, though the bug was masked
by the buffer allocation algorithm most of the time and only manifested
when transferring highly compressible data.
This patch fixes the zlib usage.
Signed-off-by: Salvador <sfandino@yahoo.com>
After filling the read buffer with data from the read queue, when the
window size was too small, "libssh2_channel_receive_window_adjust" was
called to increase it. In non-blocking mode that function could return
EAGAIN and, in that case, the EAGAIN was propagated upwards and the data
already read on the buffer lost.
The function was also moving between the two read states
"libssh2_NB_state_idle" and "libssh2_NB_state_created" both of which
behave in the same way (excepting a debug statment).
This commit modifies "_libssh2_channel_read" so that the
"libssh2_channel_receive_window_adjust" call is performed first (when
required) and if everything goes well, then it reads the data from the
queued packets into the read buffer.
It also removes the useless "libssh2_NB_state_created" read state.
Some rotted comments have also been updated.
Signed-off-by: Salvador <sfandino@yahoo.com>
Until now, the window size (channel->remote.window_size) was being
updated just after receiving the packet from the transport layer.
That behaviour is wrong because the channel queue may grow uncontrolled
when data arrives from the network faster that the upper layer consumes
it.
This patch adds a new counter, read_avail, which keeps a count of the
bytes available from the packet queue for reading. Also, now the window
size is adjusted when the data is actually read by an upper layer.
That way, if the upper layer stops reading data, the window will
eventually fill and the remote host will stop sending data. When the
upper layers reads enough data, a window adjust packet is delivered and
the transfer resumes.
The read_avail counter is used to detect the situation when the remote
server tries to send data surpassing the window size. In that case, the
extra data is discarded.
Signed-off-by: Salvador <sfandino@yahoo.com>
libssh2 used to explicitly check for libgcrypt and default to OpenSSL.
Now all possible crypto libraries are checked for explicitly, making
the addition of further crypto libraries both simpler and cleaner.
Fixes issue arising when server does not support statfvs and or fstatvfs
extensions. sftp_statvfs() and sftp_fstatvfs() after this patch will
handle the case when SSH_FXP_STATUS is returned from server.
This partially reverts commit 03ca902075
in order to fix extreme slowdown when uploading to localhost via SFTP.
I was able to repeat the issue on RHEL-7 on localhost only. It did not
occur when uploading via network and it did not occur on a RHEL-6 box
with the same version of libssh2.
The problem was that sftp_read() used a read-ahead logic to figure out
the window_size, but sftp_packet_read() called indirectly from
sftp_write() did not use any read-ahead logic.
When there's no window to "write to", there's no point in waiting for
the socket to become writable since it most likely just will continue to
be.
Patch-by: ncm
Fixes#258
In _libssh2_packet_add, called by _libssh2_packet_read, a call to
_libssh2_packet_send that is supposed to send a one-byte message
SSH_MSG_REQUEST_FAILURE would send an uninitialized byte upon re-entry
if its call to _send returns _EAGAIN.
Fixes#259
The new libssh2_sftp_fsync API causes data and metadata in the
currently open file to be committed to disk at the server.
This is an OpenSSH extension to the SFTP protocol. See:
https://bugzilla.mindrot.org/show_bug.cgi?id=1798
... in macro parameters to avoid compiler warnings about lost precision.
Several macros in libssh2.h call strlen and pass the result directly to
unsigned int parameters of other functions, which warns about precision
loss because strlen returns size_t which is unsigned long on at least
some platforms (such as OS X). The fix is to simply typecast the
strlen() result to unsigned int.
