[Openvpn-devel,2/2] Avoid sending push request after receving push reply
Commit Message
The introduction of IV_PROTO_REQUEST_PUSH (c290df55) sometimes causes the
server to reply before we setup the push timer. The push reply will then clear
a timer that has not been setup yet. We then start sending push
request after we have gone through the whole initialisation already.
This patch also clears the connestion_established timer that sets up the
push request timer. This lead to the
management_set_state(management, OPENVPN_STATE_GET_CONFIG, ...)
function not being called. But a to display "waiting for configuration..." or
sending a "getting config state" after "initialisation" does not make sense
anyway.
Also add the IV_PROTO_REQUEST_PUSH feature as new feature in Changes.rst
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
Changes.rst | 11 +++++++++++
src/openvpn/forward.c | 3 +++
src/openvpn/push.c | 1 +
3 files changed, 15 insertions(+)
Comments
a little help..
On 26/07/2020 00:48, Arne Schwabe wrote:
> The introduction of IV_PROTO_REQUEST_PUSH (c290df55) sometimes causes the
> server to reply before we setup the push timer. The push reply will then clear
> a timer that has not been setup yet. We then start sending push
> request after we have gone through the whole initialisation already.
>
> This patch also clears the connestion_established timer that sets up the
> push request timer. This lead to the
>
> management_set_state(management, OPENVPN_STATE_GET_CONFIG, ...)
>
> function not being called. But a to display "waiting for configuration..." or
I think this was meant to read as (moved 'a'):
But to display a "waiting for configuration..."
> sending a "getting config state" after "initialisation" does not make sense
> anyway.
>
> Also add the IV_PROTO_REQUEST_PUSH feature as new feature in Changes.rst
>
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
> Changes.rst | 11 +++++++++++
> src/openvpn/forward.c | 3 +++
> src/openvpn/push.c | 1 +
> 3 files changed, 15 insertions(+)
>
> diff --git a/Changes.rst b/Changes.rst
> index 8fbaf419..e377a36c 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -37,6 +37,13 @@ Deferred client-connect
> asynchronous/deferred return of the configuration file in the same way
> as the auth-plugin.
>
> +Faster connection setup
> + A client will signal in the ``IV_PROTO`` variable that is in pull
I think this is meant to read:
variable that is in pull ->
variable that it is in pull
> + mode. This allows the server to push the configuration options to
> + the client without waiting for a ``PULL_REQUEST`` message. The feature
> + is automatically enabled if both client and server support it and
> + reduces the of connection setup time by one round-trip time.
> +
> Deprecated features
> -------------------
> For an up-to-date list of all deprecated options, see this wiki page:
> @@ -72,6 +79,10 @@ User-visible Changes
> - Support for building with OpenSSL 1.0.1 has been removed. The minimum
> supported OpenSSL version is now 1.0.2.
>
> +- The GET_CONFIG management state is ommited if the server pushes
ommited -> ommitted
> + the client configuration almost immediately as result of the
> + faster connection setup feature.
> +
>
> Overview of changes in 2.4
> ==========================
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 30a3fd46..759fdbe1 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -425,6 +425,9 @@ check_push_request_dowork(struct context *c)
> *
> * Options like --up-delay need to be triggered by this function which
> * checks for connection establishment.
> + *
> + * Note: The process_incoming_push_reply currently assumes that this function
> + * only sets ups the pull request timer when pull is enabled.
ups -> up
> */
> void
> check_connection_established(struct context *c)
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 84193afe..9c720b42 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
> @@ -358,6 +358,7 @@ incoming_push_message(struct context *c, const struct buffer *buffer)
> }
> }
> event_timeout_clear(&c->c2.push_request_interval);
> + event_timeout_clear(&c->c2.wait_for_connect);
> }
>
> goto cleanup;
>
Acked-by: Gert Doering <gert@greenie.muc.de>
This is magic :-) - it's just a one liner (plus lots of documentation)
but the description makes sense, staring at the code for a while also
makes sense. Plus, it works :-)
I have not tested the server side, as (to my understanding) this is
basically "client side only" code. I *have* tested p2p with --tls-client
(now! part of my t_client rig :-) ) and that still works - relevant, as
the code flows along the "established? send pull?" path here.
The new code also works correctly with pre-fast-pull servers (the
"reference server" is - because I'm lazy - still running an older
version).
I have merged the typo fixes from Richard as well (note: ommitted has
only one m! I asked google!), and done a bit more bragging in the
Changes.rst wording ("significantly! improves connection time!").
Your patch has been applied to the master branch.
commit a3b21a76b87fedf045c409481f55c34486d8cd27
Author: Arne Schwabe
Date: Sun Jul 26 01:48:03 2020 +0200
Avoid sending push request after receving push reply
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200725234803.22058-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20589.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -37,6 +37,13 @@ Deferred client-connect
asynchronous/deferred return of the configuration file in the same way
as the auth-plugin.
+Faster connection setup
+ A client will signal in the ``IV_PROTO`` variable that is in pull
+ mode. This allows the server to push the configuration options to
+ the client without waiting for a ``PULL_REQUEST`` message. The feature
+ is automatically enabled if both client and server support it and
+ reduces the of connection setup time by one round-trip time.
+
Deprecated features
-------------------
For an up-to-date list of all deprecated options, see this wiki page:
@@ -72,6 +79,10 @@ User-visible Changes
- Support for building with OpenSSL 1.0.1 has been removed. The minimum
supported OpenSSL version is now 1.0.2.
+- The GET_CONFIG management state is ommited if the server pushes
+ the client configuration almost immediately as result of the
+ faster connection setup feature.
+
Overview of changes in 2.4
==========================
@@ -425,6 +425,9 @@ check_push_request_dowork(struct context *c)
*
* Options like --up-delay need to be triggered by this function which
* checks for connection establishment.
+ *
+ * Note: The process_incoming_push_reply currently assumes that this function
+ * only sets ups the pull request timer when pull is enabled.
*/
void
check_connection_established(struct context *c)
@@ -358,6 +358,7 @@ incoming_push_message(struct context *c, const struct buffer *buffer)
}
}
event_timeout_clear(&c->c2.push_request_interval);
+ event_timeout_clear(&c->c2.wait_for_connect);
}
goto cleanup;