Attention is currently required from: ordex, plaisthos.
Hello cron2, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1797?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+2 by plaisthos
Change subject: Enable TCP_NODELAY by default and push it to clients
......................................................................
Enable TCP_NODELAY by default and push it to clients
TCP_NODELAY had to be requested explicitly via
--tcp-nodelay or "socket-flags TCP_NODELAY". Enable
it unconditionally on every TCP socket instead
(dco-win is skipped as it manages its own socket).
The socket_set_flags() and
link_socket_update_flags() plumbing that only ever
applied it is dropped; the SF_TCP_NODELAY sockflag
and "socket-flags TCP_NODELAY" become no-ops, the
latter kept for backwards compatibility.
--tcp-nodelay no longer touches the local socket
but, in --mode server, still pushes "socket-flags
TCP_NODELAY" to clients, for the benefit of clients
older than 2.7.6 that do not enable it by default.
On a client the option is deprecated and inert. It
can be dropped once such clients are gone.
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/init.c
M src/openvpn/options.c
M src/openvpn/socket.c
M src/openvpn/socket.h
6 files changed, 29 insertions(+), 81 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/97/1797/3
diff --git a/doc/man-sections/link-options.rst
b/doc/man-sections/link-options.rst
index df8c917..7c61b67 100644
--- a/doc/man-sections/link-options.rst
+++ b/doc/man-sections/link-options.rst
@@ -465,23 +465,20 @@
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.6 :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.
+ :code:`TCP_NODELAY` is enabled by default on every TCP socket. In
+ ``--mode server`` this option additionally pushes
+ ``socket-flags TCP_NODELAY`` to connecting clients, so that clients older
+ than 2.7.6 get it too. The option can be removed once such clients are no
+ longer in use.
The macro expands as follows:
::
if mode server:
- socket-flags TCP_NODELAY
push "socket-flags TCP_NODELAY"
--max-packet-size size
diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 4c540a6..7c186c5 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)
{
@@ -597,22 +589,13 @@
* 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->server_flags & SF_TCP_NODELAY_HELPER) && o->mode == MODE_SERVER)
{
- 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;
- }
+ push_option(o, "socket-flags TCP_NODELAY", M_USAGE);
}
}
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..49b604b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -486,8 +486,8 @@
" 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 : In server mode, push TCP_NODELAY to clients (it is\n"
+ " enabled by default on the local socket).\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"
@@ -2616,6 +2616,13 @@
MUST_BE_UNDEF(vlan_accept, "vlan-accept");
MUST_BE_UNDEF(vlan_pvid, "vlan-pvid");
}
+
+ if (options->server_flags & SF_TCP_NODELAY_HELPER)
+ {
+ msg(M_INFO, "NOTE: TCP_NODELAY is always enabled locally; "
+ "--tcp-nodelay is now only useful to push the flag to "
+ "clients older than 2.7.6.");
+ }
}
else
{
@@ -2646,9 +2653,7 @@
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.");
+ msg(M_WARN, "DEPRECATED OPTION: --tcp-nodelay is always enabled on
clients");
}
MUST_BE_UNDEF(auth_user_pass_verify_script, "auth-user-pass-verify");
MUST_BE_UNDEF(auth_token_generate, "auth-gen-token");
@@ -6535,11 +6540,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]);
}
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);
/*
--
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: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I434a5373f77b0f7570a6a2aafe0eaf970e2eabe7
Gerrit-Change-Number: 1797
Gerrit-PatchSet: 3
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: ordex <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
