Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1797?usp=email
to review the following change.
Change subject: Enable TCP_NODELAY by default and deprecate --tcp-nodelay
......................................................................
Enable TCP_NODELAY by default and deprecate --tcp-nodelay
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".
Set SF_TCP_NODELAY unconditionally on every TCP
socket so the flag is always applied
(socket_set_flags still skips it on dco-win and
non-TCP transports).
With the flag always on, --tcp-nodelay is deprecated.
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]>
---
M doc/man-sections/link-options.rst
M src/openvpn/helper.c
M src/openvpn/helper.h
M src/openvpn/options.c
M src/openvpn/options.h
M src/openvpn/socket.c
6 files changed, 21 insertions(+), 66 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/97/1797/1
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/options.c b/src/openvpn/options.c
index 87218d4..6fcec92 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)
{
@@ -7715,7 +7707,8 @@
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 now the default and "
+ "the option is ignored.");
}
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..871bf0c 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1485,6 +1485,13 @@
static void
phase2_set_socket_flags(struct link_socket *sock)
{
+ /* TCP_NODELAY is enabled by default on every TCP socket (socket_set_flags
+ * skips it on dco-win and non-TCP transports) */
+ if (proto_is_tcp(sock->info.proto))
+ {
+ sock->sockflags |= SF_TCP_NODELAY;
+ }
+
/* set misc socket parameters */
socket_set_flags(sock->sd, sock->sockflags);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1797?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I434a5373f77b0f7570a6a2aafe0eaf970e2eabe7
Gerrit-Change-Number: 1797
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
