Attention is currently required from: cron2, flichtenheld, ralf_lici.
Hello cron2, flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/831?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by cron2
Change subject: Handle missing DCO peer by restarting the session
......................................................................
Handle missing DCO peer by restarting the session
Occasionally, CMD_DEL_PEER is not delivered to userspace, preventing the
openvpn process from registering the event. To handle this case, we
check if calls to the Linux DCO module return an error, and, if so, send
a SIGUSR1 signal to reset the session.
Most DCO commands that return an error already trigger a SIGUSR1 signal
or even call _exit(1). This commit extends that behavior to include
dco_get_peer_stats_multi() and dco_get_peer_stats().
Change-Id: Ib118426c5a69256894040c69856a4003d9f4637c
Signed-off-by: Ralf Lici <r...@mandelbit.com>
---
M src/openvpn/dco.h
M src/openvpn/dco_freebsd.c
M src/openvpn/dco_linux.c
M src/openvpn/dco_win.c
M src/openvpn/forward.c
M src/openvpn/manage.c
M src/openvpn/multi.c
M src/openvpn/sig.c
8 files changed, 63 insertions(+), 22 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/31/831/2
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 035474f..97453e3 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -230,17 +230,20 @@
/**
* Update traffic statistics for all peers
*
- * @param dco DCO device context
- * @param m the server context
+ * @param dco DCO device context
+ * @param m the server context
+ * @param raise_sigusr1_on_err whether to raise SIGUSR1 on error
**/
-int dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m);
+int dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m,
+ const bool raise_sigusr1_on_err);
/**
* Update traffic statistics for single peer
*
- * @param c instance context of the peer
+ * @param c instance context of the peer
+ * @param raise_sigusr1_on_err whether to raise SIGUSR1 on error
**/
-int dco_get_peer_stats(struct context *c);
+int dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err);
/**
* Retrieve the list of ciphers supported by the current platform
@@ -363,13 +366,14 @@
}
static inline int
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m)
+dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m,
+ const bool raise_sigusr1_on_err)
{
return 0;
}
static inline int
-dco_get_peer_stats(struct context *c)
+dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err)
{
return 0;
}
diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index f4c3b02..4b8d72f 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -713,7 +713,8 @@
}
int
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m)
+dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m,
+ const bool raise_sigusr1_on_err)
{
struct ifdrv drv;
@@ -781,7 +782,7 @@
}
int
-dco_get_peer_stats(struct context *c)
+dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err)
{
/* Not implemented. */
return 0;
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index b038382..bfe8cc2 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -921,7 +921,8 @@
}
int
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m)
+dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m,
+ const bool raise_sigusr1_on_err)
{
msg(D_DCO_DEBUG, "%s", __func__);
@@ -932,6 +933,14 @@
int ret = ovpn_nl_msg_send(dco, nl_msg, dco_parse_peer_multi, m, __func__);
nlmsg_free(nl_msg);
+
+ if (raise_sigusr1_on_err && ret < 0)
+ {
+ msg(M_WARN, "Error retrieving DCO peer stats: the underlying DCO peer"
+ "may have been deleted from the kernel without notifying "
+ "userspace. Restarting the session");
+ register_signal(m->top.sig, SIGUSR1, "dco peer stats error");
+ }
return ret;
}
@@ -977,9 +986,14 @@
}
int
-dco_get_peer_stats(struct context *c)
+dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err)
{
- uint32_t peer_id = c->c2.tls_multi->dco_peer_id;
+ int peer_id = c->c2.tls_multi->dco_peer_id;
+ if (peer_id == -1)
+ {
+ return 0;
+ }
+
msg(D_DCO_DEBUG, "%s: peer-id %d", __func__, peer_id);
if (!c->c1.tuntap)
@@ -999,6 +1013,14 @@
nla_put_failure:
nlmsg_free(nl_msg);
+
+ if (raise_sigusr1_on_err && ret < 0)
+ {
+ msg(M_WARN, "Error retrieving DCO peer stats: the underlying DCO peer"
+ "may have been deleted from the kernel without notifying "
+ "userspace. Restarting the session");
+ register_signal(c->sig, SIGUSR1, "dco peer stats error");
+ }
return ret;
}
diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
index 46b2d03..5687dda 100644
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -459,14 +459,15 @@
}
int
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m)
+dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m,
+ const bool raise_sigusr1_on_err)
{
/* Not implemented. */
return 0;
}
int
-dco_get_peer_stats(struct context *c)
+dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err)
{
struct tuntap *tt = c->c1.tuntap;
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index f3f3503..80ec43a 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -488,16 +488,14 @@
static void
check_inactivity_timeout(struct context *c)
{
- if (dco_enabled(&c->options) && dco_get_peer_stats(c) == 0)
+ if (dco_enabled(&c->options) && dco_get_peer_stats(c, true) == 0)
{
int64_t tot_bytes = c->c2.tun_read_bytes + c->c2.tun_write_bytes;
int64_t new_bytes = tot_bytes - c->c2.inactivity_bytes;
-
if (new_bytes > c->options.inactivity_minimum_bytes)
{
c->c2.inactivity_bytes = tot_bytes;
event_timeout_reset(&c->c2.inactivity_interval);
-
return;
}
}
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 0c77f85..ccdfd264 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4139,8 +4139,13 @@
counter_type dco_read_bytes = 0;
counter_type dco_write_bytes = 0;
- if (dco_enabled(&c->options) && (dco_get_peer_stats(c) == 0))
+ if (dco_enabled(&c->options))
{
+ if (dco_get_peer_stats(c, true) < 0)
+ {
+ return;
+ }
+
dco_read_bytes = c->c2.dco_read_bytes;
dco_write_bytes = c->c2.dco_write_bytes;
}
@@ -4159,7 +4164,8 @@
void
man_persist_client_stats(struct management *man, struct context *c)
{
- if (dco_enabled(&c->options) && (dco_get_peer_stats(c) == 0))
+ /* no need to raise SIGUSR1 since we are already closing the instance */
+ if (dco_enabled(&c->options) && (dco_get_peer_stats(c, false) == 0))
{
management_bytes_client(man, c->c2.dco_read_bytes,
c->c2.dco_write_bytes);
}
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f426b46..95b5e04 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -549,7 +549,10 @@
{
if (dco_enabled(&m->top.options))
{
- dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m);
+ if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m, false) < 0)
+ {
+ return;
+ }
}
setenv_counter(c->c2.es, "bytes_received", c->c2.link_read_bytes +
c->c2.dco_read_bytes);
@@ -856,7 +859,10 @@
if (dco_enabled(&m->top.options))
{
- dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m);
+ if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m, true) < 0)
+ {
+ return;
+ }
}
if (version == 1)
diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index 8323f0d..b0f8935 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -489,7 +489,10 @@
if (dco_enabled(&c->options))
{
- dco_get_peer_stats(c);
+ if (dco_get_peer_stats(c, true) < 0)
+ {
+ return;
+ }
}
status_printf(so, "OpenVPN STATISTICS");
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/831?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: Ib118426c5a69256894040c69856a4003d9f4637c
Gerrit-Change-Number: 831
Gerrit-PatchSet: 2
Gerrit-Owner: ralf_lici <r...@mandelbit.com>
Gerrit-Reviewer: cron2 <g...@greenie.muc.de>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: cron2 <g...@greenie.muc.de>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-Attention: ralf_lici <r...@mandelbit.com>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
