[GNUnet-SVN] [libmicrohttpd] 130/154: always set nodelay, except if we cannot cork

[gnunet]

This is an automated email from the git hooks/post-receive script.

ng0 pushed a commit to branch master
in repository libmicrohttpd.

commit c5081fb276b52fc9d48b9ba441686727eafaaad8
Author: Christian Grothoff <address@hidden>
AuthorDate: Thu Aug 1 20:44:41 2019 +0200

    always set nodelay, except if we cannot cork
---
 src/microhttpd/daemon.c      |  14 +++-
 src/microhttpd/mhd_send.c    | 166 ++++++++++++++++---------------------------
 src/microhttpd/mhd_sockets.c |  92 +++++++++++++++++-------
 src/microhttpd/mhd_sockets.h |  38 ++++++++++
 4 files changed, 181 insertions(+), 129 deletions(-)

diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index bf01ba9b..16655f65 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -3224,6 +3224,19 @@ MHD_accept_connection (struct MHD_Daemon *daemon)
         }
       return MHD_NO;
     }
+#if defined(MHD_TCP_CORK_NOPUSH) || defined(HAVE_MSG_MORE)
+  /* We will use TCP_CORK or TCP_NOPUSH or MSG_MORE to control
+     transmission, disable Nagle's algorithm (always) */
+  if (0 != MHD_socket_set_nodelay_ (s,
+                                    true))
+    {
+#ifdef HAVE_MESSAGES
+      MHD_DLOG (daemon,
+                _("Failed to disable TCP Nagle on socket: %s\n"),
+                MHD_socket_last_strerr_());
+    }
+#endif
+#endif
 #if !defined(USE_ACCEPT4) || !defined(HAVE_SOCK_NONBLOCK)
   if (! MHD_socket_nonblocking_ (s))
     {
@@ -6223,7 +6236,6 @@ MHD_start_daemon_va (unsigned int flags,
         }
     }
 #endif /* HAVE_GETSOCKNAME */
-
   if ( (MHD_INVALID_SOCKET != listen_fd) &&
        (! MHD_socket_nonblocking_ (listen_fd)) )
     {
diff --git a/src/microhttpd/mhd_send.c b/src/microhttpd/mhd_send.c
index 7ca2b506..9b07115c 100644
--- a/src/microhttpd/mhd_send.c
+++ b/src/microhttpd/mhd_send.c
@@ -49,7 +49,8 @@ pre_cork_setsockopt (struct MHD_Connection *connection,
                      bool want_cork)
 {
 #if HAVE_MSG_MORE
-#else
+  /* We use the MSG_MORE option for corking, no need for extra syscalls! */
+#elif defined(MHD_TCP_CORK_NOPUSH)
   int ret;
 
   /* If sk_cork_on is already what we pass in, return. */
@@ -58,84 +59,38 @@ pre_cork_setsockopt (struct MHD_Connection *connection,
       /* nothing to do, success! */
       return;
     }
-
-  ret = -1;
-#if TCP_CORK
-  if (want_cork)
-  {
-    const MHD_SCKT_OPT_BOOL_ on_val = 1;
-
-    ret = setsockopt (connection->socket_fd,
-                      IPPROTO_TCP,
-                      TCP_CORK,
-                      (const void *) &on_val,
-                      sizeof (on_val));
-  }
-#elif TCP_NOPUSH
-  if (want_cork)
-  {
-    const MHD_SCKT_OPT_BOOL_ on_val = 1;
-    /* TCP_NOPUSH has the same logic as MSG_MSG_MORE.
-     * The two are more or less equivalent by a source
-     * transformation (ie
-     * send(MSG_MORE) => "set TCP_NOPUSH + send() + clear TCP_NOPUSH".
-     * Both of them are really fairly "local", but TCP_NOPUSH has a
-     * _notion_ of persistency that is entirely lacking in MSG_MORE.
-     * ... with TCP_NOPUSH you basically have to know what your last
-     * write is, and clear the bit _before_ that write if you want
-     * to avoid bad latencies.
-     * https://yarchive.net/comp/linux/sendfile.html A thread from 2001,
-     * take with 18 grains of salt. */
-
-    ret = setsockopt (connection->socket_fd,
-                      IPPROTO_TCP,
-                      TCP_NOPUSH,
-                      (const void *) &on_val,
-                      sizeof (on_val));
-  }
-#elif TCP_NODELAY
-  {
-    const MHD_SCKT_OPT_BOOL_ off_val = 0;
-    const MHD_SCKT_OPT_BOOL_ on_val = 1;
-
-    ret = setsockopt (connection->socket_fd,
-                      IPPROTO_TCP,
-                      TCP_NODELAY,
-                      (const void *) want_cork ? &off_val : &on_val,
-                      sizeof (on_val));
-  }
-#endif
+  if (! want_cork)
+    return; /* nothing to do *pre* syscall! */
+  ret = MHD_socket_cork_ (connection->socket_fd,
+                          true);
   if (0 == ret)
     {
-      connection->sk_cork_on = want_cork;
+      connection->sk_cork_on = true;
+      return;
     }
-  else
+  switch (errno)
     {
-      switch (errno)
-        {
-        case ENOTSOCK:
-          /* FIXME: Could be we are talking to a pipe, maybe remember this
-             and avoid all setsockopt() in the future? */
-          break;
-        case EBADF:
-          /* FIXME: should we die hard here? */
-          break;
-        case EINVAL:
-          /* FIXME: optlen invalid, should at least log this, maybe die */
-          break;
-        case EFAULT:
-          /* FIXME: wopsie, should at leats log this, maybe die */
-          break;
-        case ENOPROTOOPT:
-          /* FIXME: optlen unknown, should at least log this */
-          break; 
-       default:
-          /* any others? man page does not list more... */
-          break;
-        }
+    case ENOTSOCK:
+      /* FIXME: Could be we are talking to a pipe, maybe remember this
+         and avoid all setsockopt() in the future? */
+      break;
+    case EBADF:
+      /* FIXME: should we die hard here? */
+      break;
+    case EINVAL:
+      /* FIXME: optlen invalid, should at least log this, maybe die */
+      break;
+    case EFAULT:
+      /* FIXME: wopsie, should at leats log this, maybe die */
+      break;
+    case ENOPROTOOPT:
+      /* FIXME: optlen unknown, should at least log this */
+      break;
+    default:
+      /* any others? man page does not list more... */
+      break;
     }
-  return;
-#endif /* MSG_MORE */
+#endif
 }
 
 
@@ -150,7 +105,8 @@ post_cork_setsockopt (struct MHD_Connection *connection,
                       bool want_cork)
 {
 #if HAVE_MSG_MORE
-#else
+  /* We use the MSG_MORE option for corking, no need for extra syscalls! */
+#elif defined(MHD_TCP_CORK_NOPUSH)
   int ret;
 
   /* If sk_cork_on is already what we pass in, return. */
@@ -159,38 +115,40 @@ post_cork_setsockopt (struct MHD_Connection *connection,
       /* nothing to do, success! */
       return;
     }
-  ret = -1;
-#if TCP_CORK
-  if (! want_cork)
-  {
-    const MHD_SCKT_OPT_BOOL_ off_val = 0;
-
-    ret = setsockopt (connection->socket_fd,
-                      IPPROTO_TCP,
-                      TCP_CORK,
-                      &off_val,
-                      sizeof (off_val));
-  }
-#elif TCP_NOPUSH
-  if (! want_cork)
-  {
-    const MHD_SCKT_OPT_BOOL_ off_val = 0;
-
-    ret = setsockopt (connection->socket_fd,
-                      IPPROTO_TCP,
-                      TCP_NOPUSH,
-                      (const void *) &off_val,
-                      sizeof (off_val));
-  }
-#elif TCP_NODELAY
-  /* nothing to do */
-#endif
+  if (want_cork)
+    return; /* nothing to do *post* syscall (in fact, we should never
+               get here, as sk_cork_on should have succeeded in the
+               pre-syscall) */
+  ret = MHD_socket_cork_ (connection->socket_fd,
+                          false);
   if (0 == ret)
     {
-      connection->sk_cork_on = want_cork;
+      connection->sk_cork_on = false;
+      return;
+    }
+  switch (errno)
+    {
+    case ENOTSOCK:
+      /* FIXME: Could be we are talking to a pipe, maybe remember this
+         and avoid all setsockopt() in the future? */
+      break;
+    case EBADF:
+      /* FIXME: should we die hard here? */
+      break;
+    case EINVAL:
+      /* FIXME: optlen invalid, should at least log this, maybe die */
+      break;
+    case EFAULT:
+      /* FIXME: wopsie, should at leats log this, maybe die */
+      break;
+    case ENOPROTOOPT:
+      /* FIXME: optlen unknown, should at least log this */
+      break;
+    default:
+      /* any others? man page does not list more... */
+      break;
     }
-  return;
-#endif /* MSG_MORE */
+#endif
 }
 
 
diff --git a/src/microhttpd/mhd_sockets.c b/src/microhttpd/mhd_sockets.c
index 0bfe2517..b4e73480 100644
--- a/src/microhttpd/mhd_sockets.c
+++ b/src/microhttpd/mhd_sockets.c
@@ -461,6 +461,64 @@ MHD_socket_noninheritable_ (MHD_socket sock)
 }
 
 
+/**
+ * Disable Nagle's algorithm on @a sock.  This is what we do by default for
+ * all TCP sockets in MHD, unless the platform does not support the MSG_MORE
+ * or MSG_CORK or MSG_NOPUSH options.
+ *
+ * @param sock socket to manipulate
+ * @param on value to use
+ * @return 0 on success
+ */
+int
+MHD_socket_set_nodelay_ (MHD_socket sock,
+                         bool on)
+{
+#ifdef TCP_NODELAY
+  {
+    const MHD_SCKT_OPT_BOOL_ off_val = 0;
+    const MHD_SCKT_OPT_BOOL_ on_val = 1;
+    /* Disable Nagle's algorithm for normal buffering */
+    return setsockopt (sock,
+                       IPPROTO_TCP,
+                       TCP_NODELAY,
+                       (const void *) (on) ? &on_val : &off_val,
+                       sizeof (on_val));
+  }
+#else
+  (void) sock;
+  return 0;
+#endif /* TCP_NODELAY */
+}
+
+
+/**
+ * Enable/disable the cork option.
+ *
+ * @param sock socket to manipulate
+ * @param on set to true to enable CORK, false to disable
+ * @return non-zero if succeeded, zero otherwise
+ */
+int
+MHD_socket_cork_ (MHD_socket sock,
+                  bool on)
+{
+#if defined(MHD_TCP_CORK_NOPUSH)
+  const MHD_SCKT_OPT_BOOL_ off_val = 0;
+  const MHD_SCKT_OPT_BOOL_ on_val = 1;
+
+  /* Disable extra buffering */
+  return (0 == setsockopt (sock,
+                           IPPROTO_TCP,
+                           MHD_TCP_CORK_NOPUSH,
+                           (const void *) (on ? &on_val : &off_val),
+                           sizeof (off_val)));
+#else
+  return 0;
+#endif /* MHD_TCP_CORK_NOPUSH */
+}
+
+
 /**
  * Change socket buffering mode to default.
  *
@@ -470,33 +528,19 @@ MHD_socket_noninheritable_ (MHD_socket sock)
 int
 MHD_socket_buffering_reset_ (MHD_socket sock)
 {
-  int res = !0;
-#if defined(TCP_NODELAY) || defined(MHD_TCP_CORK_NOPUSH)
-  const MHD_SCKT_OPT_BOOL_ off_val = 0;
 #if defined(MHD_TCP_CORK_NOPUSH)
-  //#if OLD_SOCKOPT
+  int res = ! MHD_socket_set_nodelay_ (sock,
+                                       true);
   /* Disable extra buffering */
-  res = (0 == setsockopt (sock,
-                          IPPROTO_TCP,
-                          MHD_TCP_CORK_NOPUSH,
-                          (const void *) &off_val,
-                          sizeof (off_val))) && res;
-  //#endif
+  return MHD_socket_cork_ (sock,
+                           false) && res;
+#elif defined(HAVE_MSG_MORE)
+  return ! MHD_socket_set_nodelay_ (sock,
+                                    true);
+#else
+  return ! MHD_socket_set_nodelay_ (sock,
+                                    false);
 #endif /* MHD_TCP_CORK_NOPUSH */
-#if defined(TCP_NODELAY)
-  //#if OLD_SOCKOPT
-  /* Enable Nagle's algorithm for normal buffering */
-  res = (0 == setsockopt (sock,
-                          IPPROTO_TCP,
-                          TCP_NODELAY,
-                          (const void *) &off_val,
-                          sizeof (off_val))) && res;
-  //#endif
-#endif /* TCP_NODELAY */
-#else  /* !TCP_NODELAY && !MHD_TCP_CORK_NOPUSH */
-  (void) sock;
-#endif /* !TCP_NODELAY && !MHD_TCP_CORK_NOPUSH */
-  return res;
 }
 
 
diff --git a/src/microhttpd/mhd_sockets.h b/src/microhttpd/mhd_sockets.h
index a684d71d..08b01c20 100644
--- a/src/microhttpd/mhd_sockets.h
+++ b/src/microhttpd/mhd_sockets.h
@@ -35,6 +35,7 @@
 #include "mhd_options.h"
 
 #include <errno.h>
+#include <stdbool.h>
 
 #if !defined(MHD_POSIX_SOCKETS) && !defined(MHD_WINSOCK_SOCKETS)
 #  if !defined(_WIN32) || defined(__CYGWIN__)
@@ -744,6 +745,19 @@ int
 MHD_socket_nonblocking_ (MHD_socket sock);
 
 
+/**
+ * Disable Nagle's algorithm on @a sock.  This is what we do by default for
+ * all TCP sockets in MHD, unless the platform does not support the MSG_MORE
+ * or MSG_CORK or MSG_NOPUSH options.
+ *
+ * @param sock socket to manipulate
+ * @param on value to use
+ * @return 0 on success
+ */
+int
+MHD_socket_set_nodelay_ (MHD_socket sock,
+                         bool on);
+
 /**
  * Change socket options to be non-inheritable.
  *
@@ -755,6 +769,30 @@ int
 MHD_socket_noninheritable_ (MHD_socket sock);
 
 
+/**
+ * Enable/disable the cork option.
+ *
+ * TCP_NOPUSH has the same logic as MSG_MSG_MORE.
+ * The two are more or less equivalent by a source
+ * transformation (ie
+ * send(MSG_MORE) => "set TCP_NOPUSH + send() + clear TCP_NOPUSH".
+ * Both of them are really fairly "local", but TCP_NOPUSH has a
+ * _notion_ of persistency that is entirely lacking in MSG_MORE.
+ * ... with TCP_NOPUSH you basically have to know what your last
+ * write is, and clear the bit _before_ that write if you want
+ * to avoid bad latencies.
+ *
+ * See also: https://yarchive.net/comp/linux/sendfile.html
+ *
+ * @param sock socket to manipulate
+ * @param on set to true to enable CORK, false to disable
+ * @return non-zero if succeeded, zero otherwise
+ */
+int
+MHD_socket_cork_ (MHD_socket sock,
+                  bool on);
+
+
 /**
  * Change socket buffering mode to default.
  *

-- 
To stop receiving notification emails like this one, please contact
address@hidden.