From c5081fb276b52fc9d48b9ba441686727eafaaad8 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Thu, 1 Aug 2019 20:44:41 +0200 Subject: 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 +#include #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. * -- cgit v1.2.3