From: Antonio Quartulli <[email protected]> TCP_NODELAY disables Nagle's algorithm, which is beneficial for tunnelled traffic and has long been recommended for OpenVPN over TCP. Until now it had to be requested explicitly via --tcp-nodelay or "socket-flags TCP_NODELAY".
Enable it unconditionally on every TCP socket (dco-win is skipped as it manages its own socket) and drop the socket_set_flags()/link_socket_update_flags() plumbing that only ever applied it; the SF_TCP_NODELAY sockflag is now unused but kept reserved. --tcp-nodelay and "socket-flags TCP_NODELAY" become no-ops, kept for backwards compatibility; the --tcp-nodelay helper, its SF_TCP_NODELAY_HELPER flag and the client-side warning are dropped as well. Measured over a p2p TCP tunnel: bulk throughput is unchanged (~1.7 Gbit/s, within 0.4%), while small-packet interactive latency improves sharply where Nagle bites: a request/response turn that spans several packets drops from ~82 ms to ~0.03 ms round-trip. A turn of a single packet stays fast with or without TCP_NODELAY, as Nagle cannot delay a lone packet. Change-Id: I434a5373f77b0f7570a6a2aafe0eaf970e2eabe7 Signed-off-by: Antonio Quartulli <[email protected]> Acked-by: Arne Schwabe <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1797 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1797 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <[email protected]> diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst index df8c917..ce7200f 100644 --- a/doc/man-sections/link-options.rst +++ b/doc/man-sections/link-options.rst @@ -465,24 +465,19 @@ trying to group several smaller packets into a larger packet. This can result in a considerably improvement in latency. - This option is pushable from server to client, and should be used on - both client and server for maximum effect. + Since OpenVPN 2.7 :code:`TCP_NODELAY` is enabled by default on every TCP + socket, so specifying it here is no longer necessary. --tcp-nodelay - This macro sets the :code:`TCP_NODELAY` socket flag on the server as well - as pushes it to connecting clients. The :code:`TCP_NODELAY` flag disables - the Nagle algorithm on TCP sockets causing packets to be transmitted - immediately with low latency, rather than waiting a short period of time - in order to aggregate several packets into a larger containing packet. - In VPN applications over TCP, :code:`TCP_NODELAY` is generally a good - latency optimization. + (DEPRECATED) The :code:`TCP_NODELAY` socket flag is now enabled by default + on every TCP socket, so this option no longer has any effect and is + ignored. - The macro expands as follows: - :: - - if mode server: - socket-flags TCP_NODELAY - push "socket-flags TCP_NODELAY" + :code:`TCP_NODELAY` disables the Nagle algorithm on TCP sockets, causing + packets to be transmitted immediately with low latency, rather than + waiting a short period of time in order to aggregate several packets into + a larger containing packet. In VPN applications over TCP this is generally + a good latency optimization. --max-packet-size size This option will instruct OpenVPN to try to limit the maximum on-write packet diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c index 4c540a6..be2e246 100644 --- a/src/openvpn/helper.c +++ b/src/openvpn/helper.c @@ -99,14 +99,6 @@ return BSTR(&out); } -static const char * -print_str(const char *str, struct gc_arena *gc) -{ - struct buffer out = alloc_buf_gc(128, gc); - buf_printf(&out, "%s", str); - return BSTR(&out); -} - static void helper_add_route(const in_addr_t network, const in_addr_t netmask, struct options *o) { @@ -587,32 +579,3 @@ } } } - -/* - * - * HELPER DIRECTIVE: - * - * tcp-nodelay - * - * EXPANDS TO: - * - * if mode server: - * socket-flags TCP_NODELAY - * push "socket-flags TCP_NODELAY" - */ -void -helper_tcp_nodelay(struct options *o) -{ - if (o->server_flags & SF_TCP_NODELAY_HELPER) - { - if (o->mode == MODE_SERVER) - { - o->sockflags |= SF_TCP_NODELAY; - push_option(o, print_str("socket-flags TCP_NODELAY", &o->gc), M_USAGE); - } - else - { - o->sockflags |= SF_TCP_NODELAY; - } - } -} diff --git a/src/openvpn/helper.h b/src/openvpn/helper.h index d29257c..d4e7015 100644 --- a/src/openvpn/helper.h +++ b/src/openvpn/helper.h @@ -35,6 +35,4 @@ void helper_client_server(struct options *o); -void helper_tcp_nodelay(struct options *o); - #endif diff --git a/src/openvpn/init.c b/src/openvpn/init.c index caaa769..914d191 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2650,15 +2650,6 @@ } } - if (found & OPT_P_SOCKFLAGS) - { - msg(D_PUSH, "OPTIONS IMPORT: --socket-flags option modified"); - for (int i = 0; i < c->c1.link_sockets_num; i++) - { - link_socket_update_flags(c->c2.link_sockets[i], c->options.sockflags); - } - } - if (found & OPT_P_PERSIST) { msg(D_PUSH, "OPTIONS IMPORT: --persist options modified"); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 87218d4..f3af379 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -486,8 +486,7 @@ " virtual address table to v.\n" "--bcast-buffers n : Allocate n broadcast buffers.\n" "--tcp-queue-limit n : Maximum number of queued TCP output packets.\n" - "--tcp-nodelay : Macro that sets TCP_NODELAY socket flag on the server\n" - " as well as pushes it to connecting clients.\n" + "--tcp-nodelay : (DEPRECATED) TCP_NODELAY is now enabled by default.\n" "--learn-address cmd : Run command cmd to validate client virtual addresses.\n" "--connect-freq n s : Allow a maximum of n new connections per s seconds.\n" "--connect-freq-initial n s : Allow a maximum of n replies for initial connections attempts per s seconds.\n" @@ -2644,12 +2643,6 @@ "verify-client-cert"); MUST_BE_FALSE(options->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME, "username-as-common-name"); MUST_BE_FALSE(options->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL, "auth-user-pass-optional"); - if (options->server_flags & SF_TCP_NODELAY_HELPER) - { - msg(M_WARN, "WARNING: setting tcp-nodelay on the client side will not " - "affect the server. To have TCP_NODELAY in both direction use " - "tcp-nodelay in the server configuration instead."); - } MUST_BE_UNDEF(auth_user_pass_verify_script, "auth-user-pass-verify"); MUST_BE_UNDEF(auth_token_generate, "auth-gen-token"); #if PORT_SHARE @@ -3769,7 +3762,6 @@ /* must be called after helpers that might set --mode */ helper_setdefault_topology(o); helper_keepalive(o); - helper_tcp_nodelay(o); if (o->mode == MODE_SERVER) { @@ -6535,11 +6527,9 @@ VERIFY_PERMISSION(OPT_P_SOCKFLAGS); for (j = 1; j < MAX_PARMS && p[j]; ++j) { - if (streq(p[j], "TCP_NODELAY")) - { - options->sockflags |= SF_TCP_NODELAY; - } - else + /* TCP_NODELAY is enabled by default; the flag is still accepted + * for backwards compatibility but no longer has any effect */ + if (!streq(p[j], "TCP_NODELAY")) { msg(msglevel, "unknown socket flag: %s", p[j]); } @@ -7715,7 +7705,7 @@ else if (streq(p[0], "tcp-nodelay") && !p[1]) { VERIFY_PERMISSION(OPT_P_GENERAL); - options->server_flags |= SF_TCP_NODELAY_HELPER; + msg(M_WARN, "DEPRECATED OPTION: --tcp-nodelay is ignored, it is now enabled by default."); } else if (streq(p[0], "stale-routes-check") && p[1] && !p[3]) { diff --git a/src/openvpn/options.h b/src/openvpn/options.h index a111cf8..7aa7928 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -470,8 +470,7 @@ unsigned int server_netbits_ipv6; /* IPv6 */ #define SF_NOPOOL (1 << 0) -#define SF_TCP_NODELAY_HELPER (1 << 1) -#define SF_NO_PUSH_ROUTE_GATEWAY (1 << 2) +#define SF_NO_PUSH_ROUTE_GATEWAY (1 << 1) unsigned int server_flags; bool server_bridge_proxy_dhcp; diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index df2cc9e..b73a5df 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -516,34 +516,6 @@ #endif } -static bool -socket_set_flags(socket_descriptor_t sd, unsigned int sockflags) -{ - /* SF_TCP_NODELAY doesn't make sense for dco-win */ - if ((sockflags & SF_TCP_NODELAY) && (!(sockflags & SF_DCO_WIN))) - { - return socket_set_tcp_nodelay(sd, 1); - } - else - { - return true; - } -} - -bool -link_socket_update_flags(struct link_socket *sock, unsigned int sockflags) -{ - if (sock && socket_defined(sock->sd)) - { - sock->sockflags |= sockflags; - return socket_set_flags(sock->sd, sock->sockflags); - } - else - { - return false; - } -} - void link_socket_update_buffer_sizes(struct link_socket *sock, int rcvbuf, int sndbuf) { @@ -1485,8 +1457,12 @@ static void phase2_set_socket_flags(struct link_socket *sock) { - /* set misc socket parameters */ - socket_set_flags(sock->sd, sock->sockflags); + /* TCP_NODELAY is enabled by default on every TCP socket; dco-win is + * skipped as it manages its own socket */ + if (proto_is_tcp(sock->info.proto) && !(sock->sockflags & SF_DCO_WIN)) + { + socket_set_tcp_nodelay(sock->sd, 1); + } /* set socket to non-blocking mode */ set_nonblock(sock->sd); diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h index cd4e8ed..5883592 100644 --- a/src/openvpn/socket.h +++ b/src/openvpn/socket.h @@ -207,7 +207,7 @@ int mtu; /* OS discovered MTU, or 0 if unknown */ #define SF_USE_IP_PKTINFO (1 << 0) -#define SF_TCP_NODELAY (1 << 1) +#define SF_TCP_NODELAY (1 << 1) /* unused: flag always enabled */ #define SF_PORT_SHARE (1 << 2) #define SF_HOST_RANDOMIZE (1 << 3) #define SF_GETADDRINFO_DGRAM (1 << 4) @@ -390,8 +390,6 @@ void setenv_trusted(struct env_set *es, const struct link_socket_info *info); -bool link_socket_update_flags(struct link_socket *sock, unsigned int sockflags); - void link_socket_update_buffer_sizes(struct link_socket *sock, int rcvbuf, int sndbuf); /* _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
