diff mbox series

[Openvpn-devel,M] Change in openvpn[master]: Handle missing DCO peer by restarting the session

Message ID 60542e3cbcd523a4391cc3a40577d474b02c4499-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: Handle missing DCO peer by restarting the session | expand

Commit Message

ralf_lici (Code Review) Dec. 10, 2024, 4:53 p.m. UTC 
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/+/831?usp=email

to review the following change.


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 DCO module return an error, and, if so, send a
SIGUSR1 signal to reset the session.

Change-Id: Ib118426c5a69256894040c69856a4003d9f4637c
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
---
M src/openvpn/dco_linux.c
M src/openvpn/forward.c
M src/openvpn/manage.c
M src/openvpn/multi.c
M src/openvpn/sig.c
5 files changed, 61 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/31/831/1
diff mbox series

Patch

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index b038382..3f8e206 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -979,7 +979,12 @@ 
 int
 dco_get_peer_stats(struct context *c)
 {
-    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)
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index d50b24c..4c4f3f7 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -483,16 +483,27 @@ 
 static void
 check_inactivity_timeout(struct context *c)
 {
-    if (dco_enabled(&c->options) && dco_get_peer_stats(c) == 0)
+    if (dco_enabled(&c->options))
     {
-        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)
+        const int stats_request = dco_get_peer_stats(c);
+        if (stats_request == 0)
         {
-            c->c2.inactivity_bytes = tot_bytes;
-            event_timeout_reset(&c->c2.inactivity_interval);
+            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;
+            }
+        }
+        else if (stats_request < 0)
+        {
+            msg(M_WARN, "Error requesting peer %d DCO stats (%s). Restarting the session",
+                c->c2.tls_multi->dco_peer_id, strerror(-stats_request));
+            register_signal(c->sig, SIGUSR1, "dco peer stats error");
             return;
         }
     }
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index e79a118..be2da4a 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4138,10 +4138,21 @@ 
         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) && c->c2.tls_multi->peer_id != -1)
         {
-            dco_read_bytes = c->c2.dco_read_bytes;
-            dco_write_bytes = c->c2.dco_write_bytes;
+            const int stats_request = dco_get_peer_stats(c);
+            if (stats_request == 0)
+            {
+                dco_read_bytes = c->c2.dco_read_bytes;
+                dco_write_bytes = c->c2.dco_write_bytes;
+            }
+            else if (stats_request < 0)
+            {
+                msg(D_MANAGEMENT, "MANAGEMENT: Error requesting peer %d DCO stats (%s). Restarting the session",
+                    c->c2.tls_multi->dco_peer_id, strerror(-stats_request));
+                register_signal(c->sig, SIGUSR1, "dco peer stats error");
+                return;
+            }
         }
 
         if (!(man->persist.callback.flags & MCF_SERVER))
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 45b3cfa..20724a0 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -549,7 +549,13 @@ 
 {
     if (dco_enabled(&m->top.options))
     {
-        dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m);
+        const int stats_request = dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m);
+        if (stats_request < 0)
+        {
+            msg(D_DCO, "Error requesting peer %d DCO stats (%s)",
+                c->c2.tls_multi->dco_peer_id, strerror(-stats_request));
+            return;
+        }
     }
 
     setenv_counter(c->c2.es, "bytes_received", c->c2.link_read_bytes + c->c2.dco_read_bytes);
@@ -855,7 +861,14 @@ 
 
         if (dco_enabled(&m->top.options))
         {
-            dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m);
+            const int stats_request = dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m);
+            if (stats_request == -ENOENT)
+            {
+                msg(M_WARN, "Error requesting peer DCO stats (%s)",
+                    strerror(-stats_request));
+                register_signal(m->top.sig, SIGUSR1, "dco peer stats error");
+                return;
+            }
         }
 
         if (version == 1)
diff --git a/src/openvpn/sig.c b/src/openvpn/sig.c
index 8323f0d..3654033 100644
--- a/src/openvpn/sig.c
+++ b/src/openvpn/sig.c
@@ -489,7 +489,14 @@ 
 
     if (dco_enabled(&c->options))
     {
-        dco_get_peer_stats(c);
+        const int stats_request = dco_get_peer_stats(c);
+        if (stats_request < 0)
+        {
+            msg(M_WARN, "Error requesting peer %d DCO stats (%s). Restarting the session",
+                c->c2.tls_multi->dco_peer_id, strerror(-stats_request));
+            register_signal(c->sig, SIGUSR1, "dco peer stats error");
+            return;
+        }
     }
 
     status_printf(so, "OpenVPN STATISTICS");