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 <ralf@mandelbit.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Message-Id: <20250305171730.250444-1-frank@lichtenheld.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31022.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 35ceace3aac55b834077884161c13ed3ee760c2f..ed194cc5017cde782065866da12a39340ef6600c 100644 (file)
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -231,17 +231,20 @@ void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi);
 /**
  * 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
@@ -373,13 +376,14 @@ dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi)
 }
 
 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 0e536de80500fd96dbd4d3d85ab7cce88f11d51d..b8816c63bd4362910b6802f15168ee4443bbd1d1 100644 (file)
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -713,7 +713,8 @@ dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *n
 }
 
 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 @@ retry:
 }
 
 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 68c1a8d3271abdd62e3e872bf56ad79f7978853d..b0a85fdaf365865af17b2134b368381ca9eefd8e 100644 (file)
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -952,7 +952,8 @@ dco_parse_peer_multi(struct nl_msg *msg, void *arg)
 }
 
 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__);
 
@@ -963,6 +964,14 @@ dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m)
     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;
 }
 
@@ -1008,9 +1017,14 @@ dco_parse_peer(struct nl_msg *msg, void *arg)
 }
 
 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)
@@ -1030,6 +1044,14 @@ dco_get_peer_stats(struct context *c)
 
 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 45cb919277c6b324f0ff91e7af493319cd3dab07..8b47124631bfd6714f1b9ad69aed64cb5cf4dbd9 100644 (file)
--- a/src/openvpn/dco_win.c
+++ b/src/openvpn/dco_win.c
@@ -712,14 +712,15 @@ dco_do_read(dco_context_t *dco)
 }
 
 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 b0253443aa3a1fe4dde4295731f2e63e8e594740..fce7ac8bb5ee3c964a964cced44b488aa9df309b 100644 (file)
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -488,7 +488,7 @@ check_add_routes(struct context *c)
 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;
@@ -497,7 +497,6 @@ check_inactivity_timeout(struct context *c)
         {
             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 484042ada70eec37c2e2d9fc94b51935b09d1678..0e73942762af259e13594a25a920141eda4fc1cc 100644 (file)
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4146,8 +4146,13 @@ management_check_bytecount(struct context *c, struct management *man, struct tim
         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;
         }
@@ -4166,7 +4171,8 @@ management_check_bytecount(struct context *c, struct management *man, struct tim
 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 85a1712e2334c239684f5e4e5f83739640fca5f3..9d244be6d794a20762032fa6cee3282abddd0d7e 100644 (file)
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -548,7 +548,10 @@ setenv_stats(struct multi_context *m, struct context *c)
 {
     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 @@ multi_print_status(struct multi_context *m, struct status_output *so, const int
 
         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 8323f0d97f2141693b6703ab1b0636ea25eadc84..b0f8935d4302b98f846e2a980b7e828c50b905dd 100644 (file)
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -489,7 +489,10 @@ print_status(struct context *c, struct status_output *so)
 
     if (dco_enabled(&c->options))
     {
-        dco_get_peer_stats(c);
+        if (dco_get_peer_stats(c, true) < 0)
+        {
+            return;
+        }
     }
 
     status_printf(so, "OpenVPN STATISTICS");