Re: [Openvpn-devel] [PATCH v3] Notify clients about server's exit/restart

[Lev Stipakov]

Hi,


Since the new server side code does not actually *do* OCC any more we
are just #ifdef'ing it to access options->ce.explicit_exit_notify
because that one is only compiled in #ifdef ENABLE_OCC ... so we're
coupling this new functionality to an #ifdef which is not really
relevant.

No good suggestion yet, just expressing thoughts...



Since connection_entry->explicit_exit_notification is now used also 
outside of OCC context, I suggest to:


1) Remove #ifdef ENABLE_OCC around it's definition.

2) Fix comment there:

/* Explicitly tell peer when we are exiting via OCC_EXIT or control 
channel [RESTART] message */


3) Remove (unneeded anymore) #ifdef in multi_process_signal around 
accessing explicit_exit_notification.


What do you think?

-Lev

Re: [Openvpn-devel] [PATCH v3] Notify clients about server's exit/restart

[Gert Doering]

Hi,

On Tue, Oct 13, 2015 at 05:45:58PM +0300, Lev Stipakov wrote:
> When server exits / restarts (gets SIGUSR1, SIGTERM, SIGHUP, SIGINT) and
> explicit-exit-notify is set, server sends RESTART control channel command to
> all clients and reschedules received signal in 2 secs.
> 
> When client receives RESTART command, it either reconnects to the same server 
> or
> advances to the new one, depends on parameter comes with RESTART
> command - behavior is controlled by explicit-exit-notify in the server config.
> 
> v3:
>  - Use control channel "RESTART" command instead of new OCC code to
> notify clients
>  - Configure on the server side (by value of explicit-exit-notify) if
> client should reconnect to the same server or advance to the next one
>  - Fix compilation when OCC is disabled (--enable-small)
>  - Update man page

Thanks for this.  A few observations...

 - it does not apply anymore, due to new code in multi.h
   (someone committed #ifdef ENABLE_ASYNC_PUSH in the meantime :-) ),
   so I'd appreciate a rebase

 - I'm not totally happy about the #ifdef ENABLE_OCC and the fact that
   multi_push_restart_schedule_exit() is not #ifdef'ed as well (a smart
   compiler will optimize it away, as it is static and not called, but 
   I'm not sure whether it is considered good style).  

   Since the new server side code does not actually *do* OCC any more we 
   are just #ifdef'ing it to access options->ce.explicit_exit_notify 
   because that one is only compiled in #ifdef ENABLE_OCC ... so we're
   coupling this new functionality to an #ifdef which is not really 
   relevant.

   No good suggestion yet, just expressing thoughts...


gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH v3] Notify clients about server's exit/restart

[Arne Schwabe]

Am 13.10.15 um 16:45 schrieb Lev Stipakov:
> When server exits / restarts (gets SIGUSR1, SIGTERM, SIGHUP, SIGINT) and
> explicit-exit-notify is set, server sends RESTART control channel command to
> all clients and reschedules received signal in 2 secs.
>
> When client receives RESTART command, it either reconnects to the same server 
> or
> advances to the new one, depends on parameter comes with RESTART
> command - behavior is controlled by explicit-exit-notify in the server config.
>
> v3:
>  - Use control channel "RESTART" command instead of new OCC code to
> notify clients
>  - Configure on the server side (by value of explicit-exit-notify) if
> client should reconnect to the same server or advance to the next one
>  - Fix compilation when OCC is disabled (--enable-small)
>  - Update man page
>
>
This gets an ACK from me. As additional note, we should add a follow up
that a server shutting down should not accept new clients. My client
reconnects in the 2s shutdown interval.

Arne

[Openvpn-devel] [PATCH v3] Notify clients about server's exit/restart

[Lev Stipakov]

When server exits / restarts (gets SIGUSR1, SIGTERM, SIGHUP, SIGINT) and
explicit-exit-notify is set, server sends RESTART control channel command to
all clients and reschedules received signal in 2 secs.

When client receives RESTART command, it either reconnects to the same server or
advances to the new one, depends on parameter comes with RESTART
command - behavior is controlled by explicit-exit-notify in the server config.

v3:
 - Use control channel "RESTART" command instead of new OCC code to
notify clients
 - Configure on the server side (by value of explicit-exit-notify) if
client should reconnect to the same server or advance to the next one
 - Fix compilation when OCC is disabled (--enable-small)
 - Update man page

v2:
 - Take into use explicit-exit-notify on the server side
 - OCC_SHUTTING_DOWN renamed to OCC_SERVER_EXIT
 - Code prettifying

Signed-off-by: Lev Stipakov 
---
 doc/openvpn.8 | 15 ++--
 src/openvpn/multi.c   | 68 ---
 src/openvpn/multi.h   |  9 +++
 src/openvpn/options.c |  9 +++
 src/openvpn/push.c|  6 +
 5 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index e213f5a..0a00dec 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -3829,8 +3829,19 @@ option will tell the server to immediately close its 
client instance object
 rather than waiting for a timeout.  The
 .B n
 parameter (default=1) controls the maximum number of attempts that the client
-will try to resend the exit notification message.  OpenVPN will not send any 
exit
-notifications unless this option is enabled.
+will try to resend the exit notification message. 
+
+In UDP server mode, send RESTART control channel command to connected clients. 
The
+.B n
+parameter (default=1) controls client behavior. With
+.B n
+= 1 client will attempt to reconnect
+to the same server, with
+.B n
+= 2 client will advance to the next server.
+
+OpenVPN will not send any exit
+notifications unless this option is enabled. 
 .\"*
 .SS Data Channel Encryption Options:
 These options are meaningful for both Static & TLS-negotiated key modes
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 902c4dc..d16b145 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -396,6 +396,8 @@ multi_init (struct multi_context *m, struct context *t, 
bool tcp_mode, int threa
 t->options.stale_routes_check_interval, 
t->options.stale_routes_ageing_time);
   event_timeout_init (>stale_routes_check_et, 
t->options.stale_routes_check_interval, 0);
 }
+
+  m->deferred_shutdown_signal.signal_received = 0;
 }

 const char *
@@ -2572,10 +2574,18 @@ multi_process_timeout (struct multi_context *m, const 
unsigned int mpp_flags)
   /* instance marked for wakeup? */
   if (m->earliest_wakeup)
 {
-  set_prefix (m->earliest_wakeup);
-  ret = multi_process_post (m, m->earliest_wakeup, mpp_flags);
+  if (m->earliest_wakeup == (struct 
multi_instance*)>deferred_shutdown_signal)
+   {
+ schedule_remove_entry(m->schedule, (struct schedule_entry*) 
>deferred_shutdown_signal);
+ throw_signal(m->deferred_shutdown_signal.signal_received);
+   }
+  else
+   {
+ set_prefix (m->earliest_wakeup);
+ ret = multi_process_post (m, m->earliest_wakeup, mpp_flags);
+ clear_prefix ();
+   }
   m->earliest_wakeup = NULL;
-  clear_prefix ();
 }
   return ret;
 }
@@ -2700,6 +2710,48 @@ multi_top_free (struct multi_context *m)
   free_context_buffers (m->top.c2.buffers);
 }

+static bool
+is_exit_restart(int sig)
+{
+  return (sig == SIGUSR1 || sig == SIGTERM || sig == SIGHUP || sig == SIGINT);
+}
+
+static void
+multi_push_restart_schedule_exit(struct multi_context *m, bool next_server)
+{
+  struct hash_iterator hi;
+  struct hash_element *he;
+  struct timeval tv;
+
+  /* tell all clients to restart */
+  hash_iterator_init (m->iter, );
+  while ((he = hash_iterator_next ()))
+{
+  struct multi_instance *mi = (struct multi_instance *) he->value;
+  if (!mi->halt)
+{
+ send_control_channel_string (>context, next_server ? 
"RESTART,[N]" : "RESTART", D_PUSH);
+ multi_schedule_context_wakeup(m, mi);
+}
+}
+  hash_iterator_free ();
+
+  /* reschedule signal */
+  ASSERT (!openvpn_gettimeofday (>deferred_shutdown_signal.wakeup, NULL));
+  tv.tv_sec = 2;
+  tv.tv_usec = 0;
+  tv_add (>deferred_shutdown_signal.wakeup, );
+
+  m->deferred_shutdown_signal.signal_received = m->top.sig->signal_received;
+
+  schedule_add_entry (m->schedule,
+ (struct schedule_entry *) >deferred_shutdown_signal,
+ >deferred_shutdown_signal.wakeup,
+ compute_wakeup_sigma 
(>deferred_shutdown_signal.wakeup));
+
+  m->top.sig->signal_received = 0;
+}
+
 /*
  * Return true if event loop should break,