SFTP: Increase speed and datasize in SFTP read

The function sftp_read never return more then 2000 bytes (as it should
when I asked Daniel). I increased the MAX_SFTP_READ_SIZE to 30000 but
didn't get the same speed as a sftp read in SecureSSH. I analyzed the
code and found that a return always was dona when a chunk has been read.
I changed it to a sliding buffer and worked on all available chunks. I
got an increase in speed and non of the test I have done has failed
(both local net and over Internet). Please review and test. I think
30000 is still not the optimal MAX_SFTP_READ_SIZE, my next goal is to
make an API to enable changing this value (The SecureSSH sftp_read has
more complete filled packages when comparing the network traffic)
This commit is contained in:
LarsNordin-LNdata
2015-05-17 18:29:56 +02:00
committed by Daniel Stenberg
parent 6c14cc003a
commit d754fee2f2
2 changed files with 25 additions and 13 deletions

View File

@@ -204,7 +204,8 @@ sftp_packet_add(LIBSSH2_SFTP *sftp, unsigned char *data,
LIBSSH2_SFTP_PACKET *packet; LIBSSH2_SFTP_PACKET *packet;
uint32_t request_id; uint32_t request_id;
_libssh2_debug(session, LIBSSH2_TRACE_SFTP, "Received packet %d (len %d)", _libssh2_debug(session, LIBSSH2_TRACE_SFTP,
"Received packet type %d (len %d)",
(int) data[0], data_len); (int) data[0], data_len);
/* /*
@@ -250,6 +251,9 @@ sftp_packet_add(LIBSSH2_SFTP *sftp, unsigned char *data,
request_id = _libssh2_ntohu32(&data[1]); request_id = _libssh2_ntohu32(&data[1]);
_libssh2_debug(session, LIBSSH2_TRACE_SFTP, "Received packet id %d",
request_id);
/* Don't add the packet if it answers a request we've given up on. */ /* Don't add the packet if it answers a request we've given up on. */
if((data[0] == SSH_FXP_STATUS || data[0] == SSH_FXP_DATA) if((data[0] == SSH_FXP_STATUS || data[0] == SSH_FXP_DATA)
&& find_zombie_request(sftp, request_id)) { && find_zombie_request(sftp, request_id)) {
@@ -1245,6 +1249,8 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
ssize_t rc; ssize_t rc;
struct _libssh2_sftp_handle_file_data *filep = struct _libssh2_sftp_handle_file_data *filep =
&handle->u.file; &handle->u.file;
size_t bytes_in_buffer = 0;
char *sliding_bufferp = buffer;
/* This function can be interrupted in three different places where it /* This function can be interrupted in three different places where it
might need to wait for data from the network. It returns EAGAIN to might need to wait for data from the network. It returns EAGAIN to
@@ -1305,7 +1311,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
without having been acked - until we reach EOF. */ without having been acked - until we reach EOF. */
if(!filep->eof) { if(!filep->eof) {
/* Number of bytes asked for that haven't been acked yet */ /* Number of bytes asked for that haven't been acked yet */
size_t already = (filep->offset_sent - filep->offset); size_t already = (size_t)(filep->offset_sent - filep->offset);
size_t max_read_ahead = buffer_size*4; size_t max_read_ahead = buffer_size*4;
unsigned long recv_window; unsigned long recv_window;
@@ -1391,6 +1397,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
_libssh2_list_add(&handle->packet_list, &chunk->node); _libssh2_list_add(&handle->packet_list, &chunk->node);
count -= size; /* deduct the size we used, as we might have count -= size; /* deduct the size we used, as we might have
to create more packets */ to create more packets */
_libssh2_debug(session, LIBSSH2_TRACE_SFTP, "read request id %d sent", request_id);
} }
case libssh2_NB_state_sent: case libssh2_NB_state_sent:
@@ -1475,7 +1482,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
if (rc32 == LIBSSH2_FX_EOF) { if (rc32 == LIBSSH2_FX_EOF) {
filep->eof = TRUE; filep->eof = TRUE;
return 0; return bytes_in_buffer;
} }
else { else {
sftp->last_errno = rc32; sftp->last_errno = rc32;
@@ -1505,13 +1512,13 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
filep->offset_sent -= (chunk->len - rc32); filep->offset_sent -= (chunk->len - rc32);
} }
if(rc32 > buffer_size) { if((bytes_in_buffer + rc32) > buffer_size) {
/* figure out the overlap amount */ /* figure out the overlap amount */
filep->data_left = rc32 - buffer_size; filep->data_left = (bytes_in_buffer + rc32) - buffer_size;
/* getting the full packet would overflow the buffer, so /* getting the full packet would overflow the buffer, so
only get the correct amount and keep the remainder */ only get the correct amount and keep the remainder */
rc32 = (uint32_t)buffer_size; rc32 = (uint32_t)buffer_size - bytes_in_buffer;
/* store data to keep for next call */ /* store data to keep for next call */
filep->data = data; filep->data = data;
@@ -1522,7 +1529,7 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
/* copy the received data from the received FXP_DATA packet to /* copy the received data from the received FXP_DATA packet to
the buffer at the correct index */ the buffer at the correct index */
memcpy(buffer, data + 9, rc32); memcpy(sliding_bufferp, data + 9, rc32);
filep->offset += rc32; filep->offset += rc32;
if(filep->data_len == 0) if(filep->data_len == 0)
@@ -1538,8 +1545,10 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
chunk = NULL; chunk = NULL;
if(rc32 > 0) { if(rc32 > 0) {
/* we must return as we wrote some data to the buffer */ /* continue to the next chunk */
return rc32; bytes_in_buffer += rc32;
sliding_bufferp += rc32;
chunk = next;
} else { } else {
/* A zero-byte read is not necessarily EOF so we must not /* A zero-byte read is not necessarily EOF so we must not
* return 0 (that would signal EOF to the caller) so * return 0 (that would signal EOF to the caller) so
@@ -1555,6 +1564,9 @@ static ssize_t sftp_read(LIBSSH2_SFTP_HANDLE * handle, char *buffer,
} }
} }
if (bytes_in_buffer > 0)
return bytes_in_buffer;
break; break;
default: default:
@@ -1827,7 +1839,7 @@ static ssize_t sftp_write(LIBSSH2_SFTP_HANDLE *handle, const char *buffer,
acked but we haven't been able to return as such yet, so we will acked but we haven't been able to return as such yet, so we will
get that data as well passed in here again. get that data as well passed in here again.
*/ */
already = (handle->u.file.offset_sent - handle->u.file.offset)+ already = (size_t) (handle->u.file.offset_sent - handle->u.file.offset)+
handle->u.file.acked; handle->u.file.acked;
if(count >= already) { if(count >= already) {
@@ -2767,7 +2779,7 @@ static int sftp_fstatvfs(LIBSSH2_SFTP_HANDLE *handle, LIBSSH2_SFTP_STATVFS *st)
st->f_ffree = _libssh2_ntohu64(data + 53); st->f_ffree = _libssh2_ntohu64(data + 53);
st->f_favail = _libssh2_ntohu64(data + 61); st->f_favail = _libssh2_ntohu64(data + 61);
st->f_fsid = _libssh2_ntohu64(data + 69); st->f_fsid = _libssh2_ntohu64(data + 69);
flag = _libssh2_ntohu64(data + 77); flag = (unsigned int)_libssh2_ntohu64(data + 77);
st->f_namemax = _libssh2_ntohu64(data + 85); st->f_namemax = _libssh2_ntohu64(data + 85);
st->f_flag = (flag & SSH_FXE_STATVFS_ST_RDONLY) st->f_flag = (flag & SSH_FXE_STATVFS_ST_RDONLY)
@@ -2893,7 +2905,7 @@ static int sftp_statvfs(LIBSSH2_SFTP *sftp, const char *path,
st->f_ffree = _libssh2_ntohu64(data + 53); st->f_ffree = _libssh2_ntohu64(data + 53);
st->f_favail = _libssh2_ntohu64(data + 61); st->f_favail = _libssh2_ntohu64(data + 61);
st->f_fsid = _libssh2_ntohu64(data + 69); st->f_fsid = _libssh2_ntohu64(data + 69);
flag = _libssh2_ntohu64(data + 77); flag = (unsigned int)_libssh2_ntohu64(data + 77);
st->f_namemax = _libssh2_ntohu64(data + 85); st->f_namemax = _libssh2_ntohu64(data + 85);
st->f_flag = (flag & SSH_FXE_STATVFS_ST_RDONLY) st->f_flag = (flag & SSH_FXE_STATVFS_ST_RDONLY)

View File

@@ -48,7 +48,7 @@
/* MAX_SFTP_READ_SIZE is how much data is asked for at max in each FXP_READ /* MAX_SFTP_READ_SIZE is how much data is asked for at max in each FXP_READ
* packets. * packets.
*/ */
#define MAX_SFTP_READ_SIZE 2000 #define MAX_SFTP_READ_SIZE 30000
struct sftp_pipeline_chunk { struct sftp_pipeline_chunk {
struct list_node node; struct list_node node;