- Using the libssh2 0.19 function libssh2_session_block_directions(), libcurl

now has an improved ability to do right when the multi interface (both
  "regular" and multi_socket) is used for SCP and SFTP transfers. This should
  result in (much) less busy-loop situations and thus less CPU usage with no
  speed loss.

CHANGES

@@ -6,6 +6,13 @@
 
                                   Changelog
 
+Daniel Stenberg (19 Dec 2008)
+- Using the libssh2 0.19 function libssh2_session_block_directions(), libcurl
+  now has an improved ability to do right when the multi interface (both
+  "regular" and multi_socket) is used for SCP and SFTP transfers. This should
+  result in (much) less busy-loop situations and thus less CPU usage with no
+  speed loss.
+
 Daniel Stenberg (17 Dec 2008)
 - SCP and SFTP with the multi interface had the same flaw: the 'DONE'
   operation didn't complete properly if the EAGAIN equivalent was returned but

RELEASE-NOTES

@@ -31,7 +31,9 @@ This release includes the following bugfixes:
  o curl_easy_duphandle() doesn't try to duplicate the connection cache pointer
  o build failure on OS/400 when enabling IPv6
  o better detection of SFTP failures
- o improved connection re-use for subsequent SCP and SFTP trnasfers 
+ o improved connection re-use for subsequent SCP and SFTP transfers
+ o multi interface does less busy-loops for SCP and SFTP transfers with libssh2
+   0.19 or later
 
 This release includes the following known bugs:
 

lib/dict.c

@@ -106,6 +106,7 @@ const struct Curl_handler Curl_handler_dict = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_DICT,                            /* defport */
   PROT_DICT                             /* protocol */

lib/file.c

@@ -119,6 +119,7 @@ const struct Curl_handler Curl_handler_file = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   0,                                    /* defport */
   PROT_FILE                             /* protocol */

lib/ftp.c

@@ -171,9 +171,10 @@ const struct Curl_handler Curl_handler_ftp = {
   ftp_doing,                       /* doing */
   ftp_getsock,                     /* proto_getsock */
   ftp_getsock,                     /* doing_getsock */
+  ZERO_NULL,                       /* perform_getsock */
   ftp_disconnect,                  /* disconnect */
-  PORT_FTP,                             /* defport */
+  PORT_FTP,                        /* defport */
-  PROT_FTP                              /* protocol */
+  PROT_FTP                         /* protocol */
 };
 
 
@@ -193,9 +194,10 @@ const struct Curl_handler Curl_handler_ftps = {
   ftp_doing,                       /* doing */
   ftp_getsock,                     /* proto_getsock */
   ftp_getsock,                     /* doing_getsock */
+  ZERO_NULL,                       /* perform_getsock */
   ftp_disconnect,                  /* disconnect */
-  PORT_FTPS,                            /* defport */
+  PORT_FTPS,                       /* defport */
-  PROT_FTP | PROT_FTPS | PROT_SSL       /* protocol */
+  PROT_FTP | PROT_FTPS | PROT_SSL  /* protocol */
 };
 #endif
 
@@ -215,6 +217,7 @@ const struct Curl_handler Curl_handler_ftp_proxy = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_FTP,                             /* defport */
   PROT_HTTP                             /* protocol */
@@ -237,6 +240,7 @@ const struct Curl_handler Curl_handler_ftps_proxy = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_FTPS,                            /* defport */
   PROT_HTTP                             /* protocol */

lib/http.c

@@ -137,6 +137,7 @@ const struct Curl_handler Curl_handler_http = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   http_getsock_do,                      /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_HTTP,                            /* defport */
   PROT_HTTP,                            /* protocol */
@@ -157,6 +158,7 @@ const struct Curl_handler Curl_handler_https = {
   ZERO_NULL,                            /* doing */
   https_getsock,                        /* proto_getsock */
   http_getsock_do,                      /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_HTTPS,                           /* defport */
   PROT_HTTP | PROT_HTTPS | PROT_SSL     /* protocol */

lib/ldap.c

@@ -136,6 +136,7 @@ const struct Curl_handler Curl_handler_ldap = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_LDAP,                            /* defport */
   PROT_LDAP                             /* protocol */
@@ -157,6 +158,7 @@ const struct Curl_handler Curl_handler_ldaps = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_LDAPS,                           /* defport */
   PROT_LDAP | PROT_SSL                  /* protocol */

lib/multi.c

@@ -786,11 +786,8 @@ static int multi_getsock(struct Curl_one_easy *easy,
      happen when this is called from curl_multi_remove_handle() =>
      singlesocket() => multi_getsock().
   */
-
+  if(easy->easy_handle->state.pipe_broke || !easy->easy_conn)
-  if(easy->easy_handle->state.pipe_broke ||
-      !easy->easy_conn) {
     return 0;
-  }
 
   if(easy->state > CURLM_STATE_CONNECT &&
       easy->state < CURLM_STATE_COMPLETED) {
@@ -2108,7 +2105,10 @@ static bool isHandleAtHead(struct SessionHandle *handle,
 }
 
 /* given a number of milliseconds from now to use to set the 'act before
-   this'-time for the transfer, to be extracted by curl_multi_timeout() */
+   this'-time for the transfer, to be extracted by curl_multi_timeout()
+
+   Pass zero to clear the timeout value for this handle.
+*/
 void Curl_expire(struct SessionHandle *data, long milli)
 {
   struct Curl_multi *multi = data->multi;

lib/ssh.c

@@ -144,6 +144,17 @@ static
 CURLcode sftp_perform(struct connectdata *conn,
                       bool *connected,
                       bool *dophase_done);
+
+static int ssh_getsock(struct connectdata *conn,
+                       curl_socket_t *sock, /* points to numsocks number
+                                               of sockets */
+                       int numsocks);
+
+static int ssh_perform_getsock(const struct connectdata *conn,
+                               curl_socket_t *sock, /* points to numsocks
+                                                       number of sockets */
+                               int numsocks);
+
 /*
  * SCP protocol handler.
  */
@@ -157,8 +168,9 @@ const struct Curl_handler Curl_handler_scp = {
   ssh_connect,                          /* connect_it */
   ssh_multi_statemach,                  /* connecting */
   scp_doing,                            /* doing */
-  ZERO_NULL,                            /* proto_getsock */
+  ssh_getsock,                          /* proto_getsock */
-  ZERO_NULL,                            /* doing_getsock */
+  ssh_getsock,                          /* doing_getsock */
+  ssh_perform_getsock,                  /* perform_getsock */
   scp_disconnect,                       /* disconnect */
   PORT_SSH,                             /* defport */
   PROT_SCP                              /* protocol */
@@ -178,8 +190,9 @@ const struct Curl_handler Curl_handler_sftp = {
   ssh_connect,                          /* connect_it */
   ssh_multi_statemach,                  /* connecting */
   sftp_doing,                           /* doing */
-  ZERO_NULL,                            /* proto_getsock */
+  ssh_getsock,                          /* proto_getsock */
-  ZERO_NULL,                            /* doing_getsock */
+  ssh_getsock,                          /* doing_getsock */
+  ssh_perform_getsock,                  /* perform_getsock */
   sftp_disconnect,                      /* disconnect */
   PORT_SSH,                             /* defport */
   PROT_SFTP                             /* protocol */
@@ -1345,6 +1358,10 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
       sshc->actualcode = result;
     }
     else {
+      /* store this original bitmask setup to use later on if we can't figure
+         out a "real" bitmask */
+      sshc->orig_waitfor = data->req.keepon;
+
       state(conn, SSH_STOP);
     }
     break;
@@ -2055,17 +2072,89 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
   return result;
 }
 
+/* called by the multi interface to figure out what socket(s) to wait for and
+   for what actions in the DO_DONE, PERFORM and WAITPERFORM states */
+static int ssh_perform_getsock(const struct connectdata *conn,
+                               curl_socket_t *sock, /* points to numsocks
+                                                       number of sockets */
+                               int numsocks)
+{
+#ifdef HAVE_LIBSSH2_SESSION_BLOCK_DIRECTIONS
+  int bitmap = GETSOCK_BLANK;
+  (void)numsocks;
+
+  sock[0] = conn->sock[FIRSTSOCKET];
+
+  if(conn->proto.sshc.waitfor & KEEP_READ)
+    bitmap |= GETSOCK_READSOCK(FIRSTSOCKET);
+
+  if(conn->proto.sshc.waitfor & KEEP_WRITE)
+    bitmap |= GETSOCK_WRITESOCK(FIRSTSOCKET);
+
+  return bitmap;
+#else
+  /* if we don't know the direction we can use the generic *_getsock()
+     function even for the protocol_connect and doing states */
+  return Curl_single_getsock(conn, sock, numsocks);
+#endif
+}
+
+/* Generic function called by the multi interface to figure out what socket(s)
+   to wait for and for what actions during the DOING and PROTOCONNECT states*/
+static int ssh_getsock(struct connectdata *conn,
+                       curl_socket_t *sock, /* points to numsocks number
+                                               of sockets */
+                       int numsocks)
+{
+#ifndef HAVE_LIBSSH2_SESSION_BLOCK_DIRECTIONS
+  /* if we don't know any direction we can just play along as we used to and
+     not provide any sensible info */
+  return GETSOCK_BLANK;
+#else
+  /* if we know the direction we can use the generic *_getsock() function even
+     for the protocol_connect and doing states */
+  return ssh_perform_getsock(conn, sock, numsocks);
+#endif
+}
+
+#ifdef HAVE_LIBSSH2_SESSION_BLOCK_DIRECTIONS
+/*
+ * When one of the libssh2 functions has returned LIBSSH2_ERROR_EAGAIN this
+ * function is used to figure out in what direction and stores this info so
+ * that the multi interface can take advantage of it. Make sure to call this
+ * function in all cases so that when it _doesn't_ return EAGAIN we can
+ * restore the default wait bits.
+ */
+static void ssh_block2waitfor(struct connectdata *conn, bool block)
+{
+  struct ssh_conn *sshc = &conn->proto.sshc;
+  int dir;
+  if(block && (dir = libssh2_session_block_directions(sshc->ssh_session))) {
+    /* translate the libssh2 define bits into our own bit defines */
+    sshc->waitfor = ((dir&LIBSSH2_SESSION_BLOCK_INBOUND)?KEEP_READ:0) |
+      ((dir&LIBSSH2_SESSION_BLOCK_OUTBOUND)?KEEP_WRITE:0);
+  }
+  else
+    /* It didn't block or libssh2 didn't reveal in which direction, put back
+       the original set */
+    sshc->waitfor = sshc->orig_waitfor;
+}
+#else
+  /* no libssh2 directional support so we simply don't know */
+#define ssh_block2waitfor(x,y)
+#endif
+
 /* called repeatedly until done from multi.c */
 static CURLcode ssh_multi_statemach(struct connectdata *conn, bool *done)
 {
   struct ssh_conn *sshc = &conn->proto.sshc;
   CURLcode result = CURLE_OK;
-  bool block_we_ignore; /* we don't care about EAGAIN at this point, but TODO:
+  bool block; /* we store the status and use that to provide a ssh_getsock()
-                           we _should_ store the status and use that to
+                 implementation */
-                           provide a ssh_getsock() implementation */
 
-  result = ssh_statemach_act(conn, &block_we_ignore);
+  result = ssh_statemach_act(conn, &block);
   *done = (bool)(sshc->state == SSH_STOP);
+  ssh_block2waitfor(conn, block);
 
   return result;
 }
@@ -2170,10 +2259,7 @@ static CURLcode ssh_connect(struct connectdata *conn, bool *done)
   }
 
 #ifdef CURL_LIBSSH2_DEBUG
-  libssh2_trace(ssh->ssh_session, LIBSSH2_TRACE_CONN|LIBSSH2_TRACE_TRANS|
+  libssh2_trace(ssh->ssh_session, ~0);
-                LIBSSH2_TRACE_KEX|LIBSSH2_TRACE_AUTH|LIBSSH2_TRACE_SCP|
-                LIBSSH2_TRACE_SFTP|LIBSSH2_TRACE_ERROR|
-                LIBSSH2_TRACE_PUBLICKEY);
   infof(data, "SSH socket: %d\n", sock);
 #endif /* CURL_LIBSSH2_DEBUG */
 
@@ -2328,6 +2414,7 @@ static CURLcode ssh_done(struct connectdata *conn, CURLcode status)
   sftp_scp->path = NULL;
   Curl_pgrsDone(conn);
 
+  conn->data->req.keepon = 0; /* clear all bits */
   return result;
 }
 
@@ -2354,6 +2441,9 @@ ssize_t Curl_scp_send(struct connectdata *conn, int sockindex,
   /* libssh2_channel_write() returns int! */
   nwrite = (ssize_t)
     libssh2_channel_write(conn->proto.sshc.ssh_channel, mem, len);
+
+  ssh_block2waitfor(conn, (nwrite == LIBSSH2_ERROR_EAGAIN)?TRUE:FALSE);
+
   if(nwrite == LIBSSH2_ERROR_EAGAIN)
     return 0;
 
@@ -2373,6 +2463,9 @@ ssize_t Curl_scp_recv(struct connectdata *conn, int sockindex,
   /* libssh2_channel_read() returns int */
   nread = (ssize_t)
     libssh2_channel_read(conn->proto.sshc.ssh_channel, mem, len);
+
+  ssh_block2waitfor(conn, (nread == LIBSSH2_ERROR_EAGAIN)?TRUE:FALSE);
+
   return nread;
 }
 
@@ -2484,6 +2577,12 @@ ssize_t Curl_sftp_send(struct connectdata *conn, int sockindex,
   (void)sockindex;
 
   nwrite = libssh2_sftp_write(conn->proto.sshc.sftp_handle, mem, len);
+
+  infof(conn->data, "libssh2_sftp_write() returned %d (told to send %d)\n",
+        nwrite, (int)len);
+
+  ssh_block2waitfor(conn, (nwrite == LIBSSH2_ERROR_EAGAIN)?TRUE:FALSE);
+
   if(nwrite == LIBSSH2_ERROR_EAGAIN)
     return 0;
 
@@ -2501,6 +2600,8 @@ ssize_t Curl_sftp_recv(struct connectdata *conn, int sockindex,
 
   nread = libssh2_sftp_read(conn->proto.sshc.sftp_handle, mem, len);
 
+  ssh_block2waitfor(conn, (nread == LIBSSH2_ERROR_EAGAIN)?TRUE:FALSE);
+
   return nread;
 }
 

lib/telnet.c

@@ -193,6 +193,7 @@ const struct Curl_handler Curl_handler_telnet = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_TELNET,                          /* defport */
   PROT_TELNET                           /* protocol */

lib/tftp.c

@@ -174,6 +174,7 @@ const struct Curl_handler Curl_handler_tftp = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   PORT_TFTP,                            /* defport */
   PROT_TFTP                             /* protocol */

lib/transfer.c

@@ -1524,6 +1524,7 @@ static CURLcode readwrite_upload(struct SessionHandle *data,
 			data->req.upload_fromhere, /* buffer pointer */
 			data->req.upload_present,  /* buffer size */
 			&bytes_written);       /* actually send away */
+
     if(result)
       return result;
 
@@ -1744,6 +1745,9 @@ int Curl_single_getsock(const struct connectdata *conn,
   int bitmap = GETSOCK_BLANK;
   unsigned sockindex = 0;
 
+  if(conn->handler->perform_getsock)
+    return conn->handler->perform_getsock(conn, sock, numsocks);
+
   if(numsocks < 2)
     /* simple check but we might need two slots */
     return GETSOCK_BLANK;

lib/url.c

@@ -218,6 +218,7 @@ static const struct Curl_handler Curl_handler_dummy = {
   ZERO_NULL,                            /* doing */
   ZERO_NULL,                            /* proto_getsock */
   ZERO_NULL,                            /* doing_getsock */
+  ZERO_NULL,                            /* perform_getsock */
   ZERO_NULL,                            /* disconnect */
   0,                                    /* defport */
   0                                     /* protocol */

lib/urldata.h

@@ -559,6 +559,8 @@ struct ssh_conn {
   LIBSSH2_CHANNEL *ssh_channel; /* Secure Shell channel handle */
   LIBSSH2_SFTP *sftp_session;   /* SFTP handle */
   LIBSSH2_SFTP_HANDLE *sftp_handle;
+  int waitfor;                  /* current READ/WRITE bits to wait for */
+  int orig_waitfor;             /* default READ/WRITE bits wait for */
 #endif /* USE_LIBSSH2 */
 };
 
@@ -850,6 +852,13 @@ struct Curl_handler {
                        curl_socket_t *socks,
                        int numsocks);
 
+  /* Called from the multi interface during the DO_DONE, PERFORM and
+     WAITPERFORM phases, and it should then return a proper fd set. Not setting
+     this will make libcurl use the generic default one. */
+  int (*perform_getsock)(const struct connectdata *conn,
+                         curl_socket_t *socks,
+                         int numsocks);
+
   /* This function *MAY* be set to a protocol-dependent function that is run
    * by the curl_disconnect(), as a step in the disconnection.
    */