| Message ID | 20210317160038.25828-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v2,1/3] Move restoring pre pull options to initialising of c2 context | expand |
Hi all, On 17/03/2021 17:00, Arne Schwabe wrote: > We currently delay restoring these options until we actually must > restore them. Since there is no reason to do so apart from the very > minor saving to not have to execute that code when a connection fails, > move them it into the general context_2 initialisation. > > Patch V2: rebase on master. > > Signed-off-by: Arne Schwabe <arne@rfc2549.org> This patch is simply about moving when the pre-pull settings have to be restored. Aims at simplifying the code and make it easier to follow (thanks to the patches that will come later). This patch was reviewed with the Arne's guidance. Acked-by: Antonio Quartulli <antonio@openvpn.net>
Your patch has been applied to the master branch.
I do not really have a test rig to test this, currently. I think it would
need a long-running client instance that connects to a server, receives
a set of options, disconnects (server kicking it out?), reconnects, and
receives *different* options. Or possibly "not receives options that
were set before", so verify "fall back to default/config file settings".
OTOH staring at the code looks reasonable - we only restore if something
was saved before (o->pre_pull initialized) and if we succeed connecting,
it's the same amount of energy spent.
I have run this through the enhanced client side tests (including the
"must fail!") tests, but since these all only do single-shot connections,
this is not really excercising the code change.
I do wonder if connection *failures* would cause an increased memory
consumption now... is "&c->c2.gc" cleared eventually? (Because we
allocate new copies of route-lists in that GC)... *looking*... ah, yes,
c2.gc lives only for "one instance" (init_instance() to close_instance()),
and since that one talks about "advance connection entry", it looks
like "this dance is done per connection attempt"...
commit 528a78fb144ff6a3d5865c871a402ba98fdfe21e
Author: Arne Schwabe
Date: Wed Mar 17 17:00:36 2021 +0100
Move restoring pre pull options to initialising of c2 context
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210317160038.25828-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21676.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/init.c b/src/openvpn/init.c index d234729c..81aaa6c9 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -4165,6 +4165,11 @@ init_instance(struct context *c, const struct env_set *env, const unsigned int f } } + if (c->options.pull) + { + pre_pull_restore(&c->options, &c->c2.gc); + } + /* map in current connection entry */ next_connection_entry(c); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index e9bc7dad..436c10ee 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -463,7 +463,6 @@ struct context_2 struct event_timeout push_request_interval; time_t push_request_timeout; - bool did_pre_pull_restore; /* hash of pulled options, so we can compare when options change */ bool pulled_options_digest_init_done; diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 320ad737..580c16bd 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -929,11 +929,6 @@ process_incoming_push_reply(struct context *c, md_ctx_init(c->c2.pulled_options_state, md_kt_get("SHA256")); c->c2.pulled_options_digest_init_done = true; } - if (!c->c2.did_pre_pull_restore) - { - pre_pull_restore(&c->options, &c->c2.gc); - c->c2.did_pre_pull_restore = true; - } if (apply_push_options(&c->options, buf, permission_mask,
We currently delay restoring these options until we actually must restore them. Since there is no reason to do so apart from the very minor saving to not have to execute that code when a connection fails, move them it into the general context_2 initialisation. Patch V2: rebase on master. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/init.c | 5 +++++ src/openvpn/openvpn.h | 1 - src/openvpn/push.c | 5 ----- 3 files changed, 5 insertions(+), 6 deletions(-)