[Libguestfs] [nbdkit PATCH] crypto: Tweak handling of SEND_MORE

[Eric Blake]

In the recent commit 3842a080 to add SEND_MORE support, I blindly
implemented the tls code as:
 if (SEND_MORE) {
   cork
   send
 } else {
   send
   uncork
 }
because it showed improvements for my test case of aio-parallel-load
from libnbd. But that test sticks to 64k I/O requests.

But with further investigation, I've learned that even though gnutls
corking works great for smaller NBD_CMD_READ replies, it can actually
pessimize behavior for larger requests (such as a client sending lots
of 2M read requests). This is because gnutls loses time both to
realloc()s to copy the entire packet into memory, as well as CPU time
spent encrypting the entire payload before anything is sent, not to
mention that it still ends up fragmenting the message to fit TCP
segment sizing.

So, let's further tweak the logic to make a decision based on a
heuristic: if we're going to split the reply for exceeding a TCP
segment anyway, then uncork before the data (this sends the header
out early as a partial packet, but that happens while the CPU is
encrypting the large payload); otherwise, there's still hope that we
can merge the previous request and this one into a single TCP segment
(which may or may not happen, based on how much overhead TLS adds,
given that 64k is the maximum TCP segment).  It may turn out that we
can tune the limit for slightly better performance (maybe 32k is
smarter than 64k), but since the previous commit saw improvement with
the benchmark using 64k packets, that's the value picked for now.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 server/crypto.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/server/crypto.c b/server/crypto.c
index 3f3d96b..356f04f 100644
--- a/server/crypto.c
+++ b/server/crypto.c
@@ -345,6 +345,12 @@ crypto_recv (struct connection *conn, void *vbuf, size_t len)
   return 1;
 }

+/* If this send()'s length is so large that it is going to require
+ * multiple TCP segments anyway, there's no need to try and merge it
+ * with any corked data from a previous send that used SEND_MORE.
+ */
+#define MAX_SEND_MORE_LEN (64 * 1024)
+
 /* Write buffer to GnuTLS and either succeed completely
  * (returns 0) or fail (returns -1). flags is ignored for now.
  */
@@ -357,7 +363,11 @@ crypto_send (struct connection *conn, const void *vbuf, size_t len, int flags)

   assert (session != NULL);

-  if (flags & SEND_MORE)
+  if (len >= MAX_SEND_MORE_LEN) {
+    if (gnutls_record_uncork (session, GNUTLS_RECORD_WAIT) < 0)
+      return -1;
+  }
+  else if (flags & SEND_MORE)
     gnutls_record_cork (session);

   while (len > 0) {
-- 
2.20.1