| Message ID | 20200725234803.22058-2-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,1/2] Simplify calling logic of check_connection_established_dowork | expand |
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
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 + 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 ========================== 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. */ 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;
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(+)