Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/557?usp=email
to review the following change.
Change subject: Address schedule_exit() review comments
......................................................................
Address schedule_exit() review comments
schedule_exit() now only takes the context as argument. The signal is
hard coded to SIGTERM, and the interval is read directly from the
context options.
Furthermore, schedule_exit() now returns a bool signifying whether an
exit was scheduled; false if exit is already scheduled. The call sites
are updated accordingly. A notable difference is that management is only
notified *once* when an exit is scheduled - we no longer notify
management on redundant exit.
Change-Id: I1cf589ecad82eecf167fe5cf28cda0fe4d42d42f
---
M src/openvpn/forward.c
M src/openvpn/forward.h
M src/openvpn/push.c
3 files changed, 14 insertions(+), 14 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/57/557/1
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 70f8e9d..937fae4 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -514,22 +514,23 @@
}
/*
- * Schedule a signal n_seconds from now.
+ * Schedule a SIGTERM signal c->options.scheduled_exit_interval seconds from
now.
*/
-void
-schedule_exit(struct context *c, const int n_seconds, const int signal)
+bool
+schedule_exit(struct context *c)
{
- /* don't reschedule if already scheduled; we drop the new signal, but it
- * only ever seems to be SIGTERM anyway. */
+ const int n_seconds = c->options.scheduled_exit_interval;
+ /* don't reschedule if already scheduled. */
if (event_timeout_defined(&c->c2.scheduled_exit)) {
- return;
+ return false;
}
tls_set_single_session(c->c2.tls_multi);
update_time();
reset_coarse_timers(c);
event_timeout_init(&c->c2.scheduled_exit, n_seconds, now);
- c->c2.scheduled_exit_signal = signal;
+ c->c2.scheduled_exit_signal = SIGTERM;
msg(D_SCHED_EXIT, "Delayed exit in %d seconds", n_seconds);
+ return true;
}
/*
diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index 6fb5a18..422c591 100644
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -303,7 +303,7 @@
void process_ip_header(struct context *c, unsigned int flags, struct buffer
*buf);
-void schedule_exit(struct context *c, const int n_seconds, const int signal);
+bool schedule_exit(struct context *c);
static inline struct link_socket_info *
get_link_socket_info(struct context *c)
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 1b406b9..db1fd2e 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -191,6 +191,7 @@
receive_exit_message(struct context *c)
{
dmsg(D_STREAM_ERRORS, "CC-EEN exit message received by peer");
+ bool notify_management = true;
/* With control channel exit notification, we want to give the session
* enough time to handle retransmits and acknowledgment, so that eventual
* retries from the client to resend the exit or ACKs will not trigger
@@ -204,14 +205,14 @@
* */
if (c->options.mode == MODE_SERVER)
{
- schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM);
+ notify_management = schedule_exit(c);
}
else
{
register_signal(c->sig, SIGUSR1, "remote-exit");
}
#ifdef ENABLE_MANAGEMENT
- if (management)
+ if (management && notify_management)
{
management_notify(management, "info", "remote-exit", "EXIT");
}
@@ -391,7 +392,7 @@
void
send_auth_failed(struct context *c, const char *client_reason)
{
- if (event_timeout_defined(&c->c2.scheduled_exit))
+ if (!schedule_exit(c))
{
msg(D_TLS_DEBUG, "exit already scheduled for context");
return;
@@ -401,8 +402,6 @@
static const char auth_failed[] = "AUTH_FAILED";
size_t len;
- schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM);
-
len = (client_reason ? strlen(client_reason)+1 : 0) + sizeof(auth_failed);
if (len > PUSH_BUNDLE_SIZE)
{
@@ -492,7 +491,7 @@
void
send_restart(struct context *c, const char *kill_msg)
{
- schedule_exit(c, c->options.scheduled_exit_interval, SIGTERM);
+ schedule_exit(c);
send_control_channel_string(c, kill_msg ? kill_msg : "RESTART", D_PUSH);
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/557?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1cf589ecad82eecf167fe5cf28cda0fe4d42d42f
Gerrit-Change-Number: 557
Gerrit-PatchSet: 1
Gerrit-Owner: reynir <rey...@reynir.dk>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
