Merge branch 'dcb_ssl_refactoring' into develop

server/core/dcb.c

@@ -109,6 +109,8 @@ static inline void dcb_write_when_already_queued(DCB *dcb, GWBUF *queue);
 static void dcb_log_write_failure(DCB *dcb, GWBUF *queue, int eno);
 static inline void dcb_write_tidy_up(DCB *dcb, bool below_water);
 static void dcb_write_SSL_error_report (DCB *dcb, int ret, int ssl_errno);
+int dcb_bytes_readable_SSL (DCB *dcb, int nread);
+void dcb_log_ssl_read_error(DCB *dcb, int ssl_errno, int rc);
 
 size_t dcb_get_session_id(
 	DCB *dcb)
@@ -909,91 +911,25 @@ int dcb_read(
  * @return	-1 on error, otherwise the number of read bytes on the last
  * iteration of while loop. 0 is returned if no data available.
  */
-int dcb_read_SSL(
+int dcb_read_SSL(DCB *dcb,
-        DCB   *dcb,
                  GWBUF **head)
 {
     GWBUF *buffer = NULL;
-        int   b,pending;
+    int b, n, nread = 0;
-        int   rc;
-        int   n;
-        int   nread = 0;
-	int ssl_errno = 0;
     CHK_DCB(dcb);
 
     if (dcb->fd <= 0)
     {
-		LOGIF(LE, (skygw_log_write_flush(
+        LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,
-			LOGFILE_ERROR,
                                          "Error : Read failed, dcb is %s.",
                                          dcb->fd == DCBFD_CLOSED ? "closed" : "cloned, not readable")));
-		n = 0;
+        return 0;
-		goto return_n;
     }
 
-	while (true)
+    while ((b = dcb_bytes_readable_SSL(dcb, nread)) > 0)
     {
-                int bufsize;
-		ssl_errno = 0;
-                rc = ioctl(dcb->fd, FIONREAD, &b);
-		pending = SSL_pending(dcb->ssl);
-                if (rc == -1)
-                {
-                        char errbuf[STRERROR_BUFLEN];
-                        LOGIF(LE, (skygw_log_write_flush(
-                                LOGFILE_ERROR,
-                                "Error : ioctl FIONREAD for dcb %p in "
-                                "state %s fd %d failed due error %d, %s.",
-                                dcb,
-                                STRDCBSTATE(dcb->state),
-                                dcb->fd,
-                                errno,
-                                strerror_r(errno, errbuf, sizeof(errbuf)))));
-                        n = -1;
-                        goto return_n;
-                }
-
-                if (b == 0 && pending == 0 && nread == 0)
-                {
-                        /** Handle closed client socket */
-                        if (dcb_isclient(dcb))
-                        {
-                                char c = 0;
-                                int r = -1;
-
-                                /* try to read 1 byte, without consuming the socket buffer */
-                                r = SSL_peek(dcb->ssl, &c, sizeof(char));
-                                if (r <= 0)
-                                {
-				    ssl_errno = SSL_get_error(dcb->ssl,r);
-				    if(ssl_errno != SSL_ERROR_WANT_READ &&
-				     ssl_errno != SSL_ERROR_WANT_WRITE &&
-				     ssl_errno != SSL_ERROR_NONE)
-                                        n = -1;
-				    else
-					n = 0;
-				    goto return_n;
-                                }
-                        }
-                        n = 0;
-                        goto return_n;
-                }
-                else if (b == 0 && pending == 0)
-                {
-                        n = 0;
-                        goto return_n;
-                }
-#ifdef SS_DEBUG
-		else
-		{
-		    skygw_log_write_flush(LD,"Total: %d Socket: %d Pending: %d",
-				     nread,b,pending);
-		}
-#endif
-
         dcb->last_read = hkheartbeat;
-
+        int bufsize = MIN(b, MAX_BUFFER_SIZE);
-                bufsize = MIN(b, MAX_BUFFER_SIZE);
 
         if ((buffer = gwbuf_alloc(bufsize)) == NULL)
         {
@@ -1002,102 +938,55 @@ int dcb_read_SSL(
              * Todo shutdown if memory allocation fails.
              */
             char errbuf[STRERROR_BUFLEN];
-                        LOGIF(LE, (skygw_log_write_flush(
+            LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,
-                                LOGFILE_ERROR,
                                              "Error : Failed to allocate read buffer "
                                              "for dcb %p fd %d, due %d, %s.",
                                              dcb,
                                              dcb->fd,
                                              errno,
-                                strerror_r(errno, errbuf, sizeof(errbuf)))));
+                                             strerror_r(errno, errbuf, sizeof (errbuf)))));
 
-                        n = -1;
+            return -1;
-                        goto return_n;
         }
 
         n = SSL_read(dcb->ssl, GWBUF_DATA(buffer), bufsize);
         dcb->stats.n_reads++;
 
-		    if (n < 0)
+        if (n <= 0)
         {
-			char errbuf[200];
+            int ssl_errno = SSL_get_error(dcb->ssl, n);
-			ssl_errno = SSL_get_error(dcb->ssl,n);
+            dcb_log_ssl_read_error(dcb, ssl_errno, n);
-#ifdef SS_DEBUG
-			if(ssl_errno == SSL_ERROR_SSL ||
-			   ssl_errno == SSL_ERROR_SYSCALL)
-			  {
-			    int eno;
-			    while((eno = ERR_get_error()) != 0)
-			      {
-				ERR_error_string_n(eno,errbuf,200);
-				skygw_log_write(LE,
-						"%s",
-						errbuf);
-			      }
-			  }
-#endif
-			if(ssl_errno == SSL_ERROR_WANT_READ ||
-			   ssl_errno == SSL_ERROR_WANT_WRITE ||
-			   ssl_errno == SSL_ERROR_NONE)
-			{
-			    n = 0;
-			}
-			else
-			{
-                            char errbuf[STRERROR_BUFLEN];
-			    LOGIF(LE, (skygw_log_write_flush(
-				    LOGFILE_ERROR,
-				"Error : Read failed, dcb %p in state "
-				"%s fd %d, SSL error %d: %s.",
-				dcb,
-				STRDCBSTATE(dcb->state),
-				dcb->fd,
-				ssl_errno,
-                                    strerror_r(errno, errbuf, sizeof(errbuf)))));
 
-			    if(ssl_errno == SSL_ERROR_SSL ||
+            if (ssl_errno != SSL_ERROR_WANT_READ &&
-			     ssl_errno == SSL_ERROR_SYSCALL)
+                ssl_errno != SSL_ERROR_WANT_WRITE &&
+                ssl_errno != SSL_ERROR_NONE)
             {
-				while((ssl_errno = ERR_get_error()) != 0)
+                nread = -1;
-				{
-				    ERR_error_string_n(ssl_errno,errbuf,200);
-				    skygw_log_write(LE,
-					     "%s",
-					     errbuf);
-				}
-			    }
-			}
-			n = -1;
                 gwbuf_free(buffer);
-			goto return_n;
             }
-		    else if(n == 0)
+            return nread;
-		    {
-			gwbuf_free(buffer);
-			goto return_n;
         }
 
-		    buffer = gwbuf_rtrim(buffer,bufsize - n);
+        buffer = gwbuf_rtrim(buffer, bufsize - n);
-		    if(buffer == NULL)
+
+        if (buffer)
         {
-			goto return_n;
-		    }
 #ifdef SS_DEBUG
-		    skygw_log_write(LD,"%lu SSL: Truncated buffer from %d to %d bytes. "
+            skygw_log_write(LD, "%lu SSL: Truncated buffer from %d to %d bytes. "
-	            "Read %d bytes, %d bytes waiting.\n",pthread_self(),
+                            "Read %d bytes, %d bytes waiting.\n", pthread_self(),
-		     bufsize,GWBUF_LENGTH(buffer),n,b);
+                            bufsize, GWBUF_LENGTH(buffer), n, b);
 
-		    if(GWBUF_LENGTH(buffer) != n){
+            if (GWBUF_LENGTH(buffer) != n)
+            {
                 skygw_log_sync_all();
             }
 
-		    ss_info_dassert((buffer->start <= buffer->end),"Buffer start has passed end.");
+            ss_info_dassert((buffer->start <= buffer->end), "Buffer start has passed end.");
-		    ss_info_dassert(GWBUF_LENGTH(buffer) == n,"Buffer size not equal to read bytes.");
+            ss_info_dassert(GWBUF_LENGTH(buffer) == n, "Buffer size not equal to read bytes.");
 #endif
             nread += n;
 
-                LOGIF(LD, (skygw_log_write(
+            LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG,
-                        LOGFILE_DEBUG,
                                        "%lu [dcb_read_SSL] Read %d bytes from dcb %p in state %s "
                                        "fd %d.",
                                        pthread_self(),
@@ -1108,8 +997,17 @@ int dcb_read_SSL(
 
             /*< Append read data to the gwbuf */
             *head = gwbuf_append(*head, buffer);
-        } /*< while (true) */
+        }
-return_n:
+    }
+
+    ss_dassert(gwbuf_length(*head) == nread);
+    LOGIF(LD, skygw_log_write(LD, "%lu Read a total of %d bytes from dcb %p in state %s fd %d.",
+                              pthread_self(),
+                              nread,
+                              dcb,
+                              STRDCBSTATE(dcb->state),
+                              dcb->fd));
+
     return nread;
 }
 
@@ -3143,3 +3041,104 @@ char	*name = NULL;
         }
 	return name;
 }
+
+/**
+ * Check how much data is readable from an SSL enabled DCB.
+ * @param dcb DCB to check
+ * @param nread Number of bytes we have already read
+ * @return Number of bytes readable or -1 on error
+ */
+int dcb_bytes_readable_SSL(DCB *dcb, int nread)
+{
+    int rval = 0;
+    int nbytes;
+    int rc = ioctl(dcb->fd, FIONREAD, &nbytes);
+
+    if (rc == -1)
+    {
+        char errbuf[STRERROR_BUFLEN];
+        LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,
+                                         "Error : ioctl FIONREAD for dcb %p in "
+                                         "state %s fd %d failed due error %d, %s.",
+                                         dcb,
+                                         STRDCBSTATE(dcb->state),
+                                         dcb->fd,
+                                         errno,
+                                         strerror_r(errno, errbuf, sizeof (errbuf)))));
+        rval = -1;
+    }
+    else
+    {
+        int pending = SSL_pending(dcb->ssl);
+        rval = nbytes + pending;
+        if (rval == 0 && nread == 0)
+        {
+            /** Handle closed client socket */
+            if (dcb_isclient(dcb))
+            {
+                char c = 0;
+                int r = -1;
+
+                /* try to read 1 byte, without consuming the socket buffer */
+                r = SSL_peek(dcb->ssl, &c, sizeof (char));
+                if (r <= 0)
+                {
+                    int ssl_errno = SSL_get_error(dcb->ssl, r);
+                    if (ssl_errno != SSL_ERROR_WANT_READ &&
+                        ssl_errno != SSL_ERROR_WANT_WRITE &&
+                        ssl_errno != SSL_ERROR_NONE)
+                        rval = -1;
+                }
+            }
+        }
+#ifdef SS_DEBUG
+        else if (nbytes != 0 || pending != 0)
+        {
+            skygw_log_write_flush(LD, "Total: %d Socket: %d Pending: %d",
+                                  nread, nbytes, pending);
+        }
+        else
+        {
+            skygw_log_write(LD, "Tried to read from socket, no data left. %d bytes read in total.", nread);
+        }
+#endif
+    }
+    return rval;
+}
+
+/**
+ * Log SSL read error messages
+ * @param dcb DCB from which the SSL_read was attempted
+ * @param ssl_errno SSL error number
+ * @param rc Return value of SSL_read
+ */
+void dcb_log_ssl_read_error(DCB *dcb, int ssl_errno, int rc)
+{
+    if (ssl_errno != SSL_ERROR_WANT_READ &&
+        ssl_errno != SSL_ERROR_WANT_WRITE &&
+        ssl_errno != SSL_ERROR_NONE)
+    {
+
+        char errbuf[STRERROR_BUFLEN];
+        LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR,
+                                         "Error : Read failed, dcb %p in state "
+                                         "%s fd %d, SSL error %d: %s.",
+                                         dcb,
+                                         STRDCBSTATE(dcb->state),
+                                         dcb->fd,
+                                         ssl_errno,
+                                         strerror_r(errno, errbuf, sizeof (errbuf)))));
+
+        if (ssl_errno == SSL_ERROR_SSL ||
+            ssl_errno == SSL_ERROR_SYSCALL)
+        {
+            while ((ssl_errno = ERR_get_error()) != 0)
+            {
+                ERR_error_string_n(ssl_errno, errbuf, STRERROR_BUFLEN);
+                skygw_log_write(LE,
+                                "%s",
+                                errbuf);
+            }
+        }
+    }
+}