Deprecate --fast-io option

Recent changes to the event loop revealed that
the --fast-io option is now partially broken and
may cause "unroutable control packet" issues.

As agreed during the last hackathon, this patch
turns --fast-io into a no-op and emits a warning
when it is used.

Additionally, the MPP_CONDITIONAL_PRE_SELECT
flag has been removed as it was part of the
same code path and no longer needed.

Change-Id: I2c0a0b55ad56e704d4bd19f1fbc1c30c83fae14c
Signed-off-by: Gianmarco De Gregori <gianmarco@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1425
Message-Id: <20251211105956.22789-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg35024.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst
index 882cf283a0649b5151bfa7426e9cb1a026b55f45..a9232cef42f455bec0f1890de1e40c7ef6997921 100644 (file)
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -211,18 +211,6 @@ which mode OpenVPN is configured as.
   ``--show-engines`` standalone option to list the crypto engines which
   are supported by OpenSSL.
 
---fast-io
-  Optimize TUN/TAP/UDP I/O writes by avoiding a call to
-  poll/epoll/select prior to the write operation. The purpose of such a
-  call would normally be to block until the device or socket is ready to
-  accept the write. Such blocking is unnecessary on some platforms which
-  don't support write blocking on UDP sockets or TUN/TAP devices. In such
-  cases, one can optimize the event loop by avoiding the poll/epoll/select
-  call, improving CPU efficiency by 5% to 10%.
-
-  This option can only be used on non-Windows systems, when ``--proto
-  udp`` is specified, and when ``--shaper`` is *NOT* specified.
-
 --group group
   Similar to the ``--user`` option, this option changes the group ID of
   the OpenVPN process to ``group`` after initialization.

diff --git a/doc/man-sections/unsupported-options.rst b/doc/man-sections/unsupported-options.rst
index b646991ff239e2288765f287fd618c746f9c89b0..f1332f3dbfd5d1e32057418a817a0bd7dc51c9ec 100644 (file)
--- a/doc/man-sections/unsupported-options.rst
+++ b/doc/man-sections/unsupported-options.rst
@@ -9,6 +9,10 @@ longer supported
   Removed in OpenVPN 2.5.  This should be replaced with
   ``--verify-client-cert none``.
 
+--fast-io
+  Ignored since OpenVPN 2.7. This option became broken due to changes
+  to the event loop.
+
 --http-proxy-retry
   Removed in OpenVPN 2.4.  All retries are controlled by ``--max-connect-retry``.
 

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 913fb9202d6c3d6b4ee9a6e82a0a12831f33fc75..492e66716652c41487959e72c9615febe47391f8 100644 (file)
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -2154,13 +2154,12 @@ multi_io_process_flags(struct context *c, struct event_set *es, const unsigned i
 }
 
 /*
- * Wait for I/O events.  Used for both TCP & UDP sockets
- * in point-to-point mode and for UDP sockets in
+ * Wait for I/O events.  Used for UDP sockets in
  * point-to-multipoint mode.
  */
 
 void
-get_io_flags_dowork_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags)
+get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags)
 {
     unsigned int out_socket;
 
@@ -2168,33 +2167,12 @@ get_io_flags_dowork_udp(struct context *c, struct multi_io *multi_io, const unsi
     multi_io->udp_flags = (out_socket << SOCKET_SHIFT);
 }
 
+/*
+ * This is the core I/O wait function, used for all I/O waits except
+ * for the top-level server sockets.
+ */
 void
-get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags)
-{
-    multi_io->udp_flags = ES_ERROR;
-    if (c->c2.fast_io && (flags & (IOW_TO_TUN | IOW_TO_LINK | IOW_MBUF)))
-    {
-        /* fast path -- only for TUN/TAP/UDP writes */
-        unsigned int ret = 0;
-        if (flags & IOW_TO_TUN)
-        {
-            ret |= TUN_WRITE;
-        }
-        if (flags & (IOW_TO_LINK | IOW_MBUF))
-        {
-            ret |= SOCKET_WRITE;
-        }
-        multi_io->udp_flags = ret;
-    }
-    else
-    {
-        /* slow path - delegate to io_wait_dowork_udp to calculate flags */
-        get_io_flags_dowork_udp(c, multi_io, flags);
-    }
-}
-
-void
-io_wait_dowork(struct context *c, const unsigned int flags)
+io_wait(struct context *c, const unsigned int flags)
 {
     unsigned int out_socket;
     unsigned int out_tuntap;

diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index 06808b93b3d774f7908d88798afafac5fd17ac3a..7f6f666f4ded8b7c4d356bdea15c8b84b4dea4df 100644 (file)
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -68,12 +68,9 @@ extern counter_type link_read_bytes_global;
 
 extern counter_type link_write_bytes_global;
 
-void get_io_flags_dowork_udp(struct context *c, struct multi_io *multi_io,
-                             const unsigned int flags);
-
 void get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags);
 
-void io_wait_dowork(struct context *c, const unsigned int flags);
+void io_wait(struct context *c, const unsigned int flags);
 
 void pre_select(struct context *c);
 
@@ -382,34 +379,6 @@ p2p_iow_flags(const struct context *c)
     return flags;
 }
 
-/*
- * This is the core I/O wait function, used for all I/O waits except
- * for the top-level server sockets.
- */
-static inline void
-io_wait(struct context *c, const unsigned int flags)
-{
-    if (proto_is_dgram(c->c2.link_sockets[0]->info.proto) && c->c2.fast_io
-        && (flags & (IOW_TO_TUN | IOW_TO_LINK | IOW_MBUF)))
-    {
-        /* fast path -- only for TUN/TAP/UDP writes */
-        unsigned int ret = 0;
-        if (flags & IOW_TO_TUN)
-        {
-            ret |= TUN_WRITE;
-        }
-        if (flags & (IOW_TO_LINK | IOW_MBUF))
-        {
-            ret |= SOCKET_WRITE;
-        }
-        c->c2.event_set_status = ret;
-    }
-    else
-    {
-        /* slow path */
-        io_wait_dowork(c, flags);
-    }
-}
 
 static inline bool
 connection_established(struct context *c)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index fc079e1124a05294404b4865c05844e7f954b420..cd0152003e810051e0c733ac6458ece135087908 100644 (file)
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -4139,34 +4139,6 @@ do_env_set_destroy(struct context *c)
     }
 }
 
-/*
- * Fast I/O setup.  Fast I/O is an optimization which only works
- * if all of the following are true:
- *
- * (1) The platform is not Windows
- * (2) --proto udp is enabled
- * (3) --shaper is disabled
- */
-static void
-do_setup_fast_io(struct context *c)
-{
-    if (c->options.fast_io)
-    {
-#ifdef _WIN32
-        msg(M_INFO, "NOTE: --fast-io is disabled since we are running on Windows");
-#else
-        if (c->options.shaper)
-        {
-            msg(M_INFO, "NOTE: --fast-io is disabled since we are using --shaper");
-        }
-        else
-        {
-            c->c2.fast_io = true;
-        }
-#endif
-    }
-}
-
 static void
 do_signal_on_tls_errors(struct context *c)
 {
@@ -4513,12 +4485,6 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f
     }
 #endif
 
-    /* should we enable fast I/O? */
-    if (c->mode == CM_P2P || c->mode == CM_TOP)
-    {
-        do_setup_fast_io(c);
-    }
-
     /* should we throw a signal on TLS errors? */
     do_signal_on_tls_errors(c);
 

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 5de3af69707439c6625c5502dbdd470063596f4c..92d4dda028d2402623d983bffaeb8b1137c16a51 100644 (file)
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -339,9 +339,7 @@ void
 multi_process_io_udp(struct multi_context *m, struct link_socket *sock)
 {
     const unsigned int status = m->multi_io->udp_flags;
-    const unsigned int mpp_flags = m->top.c2.fast_io
-                                       ? (MPP_CONDITIONAL_PRE_SELECT | MPP_CLOSE_ON_SIGNAL)
-                                       : (MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL);
+    const unsigned int mpp_flags = (MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL);
 
     /* UDP port ready to accept write */
     if (status & SOCKET_WRITE)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 153695c49b4efcc44582f2145c04ec5d6b944b98..d2d9ba8e162f7d7a8f8f4eb2b8f4c2126c467ea3 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3008,8 +3008,7 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns
     bool ret = true;
 
     if (!IS_SIG(&mi->context)
-        && ((flags & MPP_PRE_SELECT)
-            || ((flags & MPP_CONDITIONAL_PRE_SELECT) && !ANY_OUT(&mi->context))))
+        && ((flags & MPP_PRE_SELECT)))
     {
 #if defined(ENABLE_ASYNC_PUSH)
         bool was_unauthenticated = true;

diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index a44f9f25b0451240cbef9d68fc38f19298ce6b88..1209dfb928fbb22c1cb7d7a0177c33edb2b9ad24 100644 (file)
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -271,10 +271,9 @@ void multi_close_instance(struct multi_context *m, struct multi_instance *mi, bo
 
 bool multi_process_timeout(struct multi_context *m, const unsigned int mpp_flags);
 
-#define MPP_PRE_SELECT             (1 << 0)
-#define MPP_CONDITIONAL_PRE_SELECT (1 << 1)
-#define MPP_CLOSE_ON_SIGNAL        (1 << 2)
-#define MPP_RECORD_TOUCH           (1 << 3)
+#define MPP_PRE_SELECT      (1 << 0)
+#define MPP_CLOSE_ON_SIGNAL (1 << 1)
+#define MPP_RECORD_TOUCH    (1 << 2)
 
 
 /**************************************************************************/

diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index a198fcf6d84e4a1c2739fba819828e69026447ba..3e1ae78d3c4697edcf26a2a01ae3934bb6501a80 100644 (file)
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -420,9 +420,6 @@ struct context_2
     struct env_set *es;
     bool es_owned;
 
-    /* don't wait for TUN/TAP/UDP to be ready to accept write */
-    bool fast_io;
-
     /* --ifconfig endpoints to be pushed to client */
     bool push_request_received;
     bool push_ifconfig_defined;

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2d1f740c51ee93cef66b2fa7578574799aa8b24d..d01ec474d021be1ff7fa1fe4226607267c37d041 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -285,7 +285,6 @@ static const char usage_message[] =
 #if ENABLE_IP_PKTINFO
     "--multihome     : Configure a multi-homed UDP server.\n"
 #endif
-    "--fast-io       : Optimize TUN/TAP/UDP writes.\n"
     "--remap-usr1 s  : On SIGUSR1 signals, remap signal (s='SIGHUP' or 'SIGTERM').\n"
     "--persist-tun   : Keep tun/tap device open across SIGUSR1 or --ping-restart.\n"
     "--persist-remote-ip : Keep remote IP address across SIGUSR1 or --ping-restart.\n"
@@ -1795,8 +1794,6 @@ show_settings(const struct options *o)
 #endif
     SHOW_INT(sockflags);
 
-    SHOW_BOOL(fast_io);
-
     SHOW_INT(comp.alg);
     SHOW_INT(comp.flags);
 
@@ -6592,7 +6589,7 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
     else if (streq(p[0], "fast-io") && !p[1])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
-        options->fast_io = true;
+        msg(M_WARN, "DEPRECATED OPTION: --fast-io option ignored.");
     }
     else if (streq(p[0], "inactive") && p[1] && !p[3])
     {

diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index d3310332c41292df419ff8fc7c5d92fa1ebe1441..555d9dd66bc5aac8b211af0572de9f7b6dbead48 100644 (file)
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -406,9 +406,6 @@ struct options
     int status_file_version;
     int status_file_update_freq;
 
-    /* optimize TUN/TAP/UDP writes */
-    bool fast_io;
-
     struct compress_options comp;
 
     /* buffer sizes */