diff mbox series

[Openvpn-devel] Ensure that dco keepalive and mssfix options are also set in pure p2p mode

Message ID 20221219174027.2567505-1-arne@rfc2549.org
State Accepted
Headers
Series [Openvpn-devel] Ensure that dco keepalive and mssfix options are also set in pure p2p mode |

Commit Message

Arne Schwabe Dec. 19, 2022, 5:40 p.m. UTC 
  Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/dco_freebsd.c |  3 +++
 src/openvpn/init.c        | 42 ++++++++++++++++++++++++---------------
 2 files changed, 29 insertions(+), 16 deletions(-)

Comments

Gert Doering Dec. 20, 2022, 12:13 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

This indeed is good news - as in, the p2p --tls-server test case that
used to reliably break when the client disconnected because --ping wasn't
working is now working.  Tested on Linux/DCO, FreeBSD/DCO and "Linux no DCO",
with the full set of client/server tests - it fixes some bits, and breaks
nothing else, so good :-)

Stare-at-code and thinking about the flow of control also confirms that
this is a good fix - do_up() is executed always (after PUSH_REPLY for
--client, and "on init" for --tls-client / --tls-server) so setting
up DCO ping/mss is needed there.

(And thanks for adding the logging entry to dco_freebsd.c - I was 
staring at my logs and wondering why I didn't see anything, even in
p2mp mode...)

(... and, as an afterthought, I re-checked to be sure we've not lost
multipoint keepalive on the way - we haven't, I can see proper
"ping interval..." messages for peer-id 0, 1, 2 in my udp-p2mp
instance)


Your patch has been applied to the master and release/2.6 branch.

commit 7c66a6dab54d8efcde57c8fb562f95d95f9b18d4 (master)
commit c6ba846384b6a7ab610e5d7de689868fa0a48395 (release/2.6)
Author: Arne Schwabe
Date:   Mon Dec 19 18:40:27 2022 +0100

     Ensure that dco keepalive and mssfix options are also set in pure p2p mode

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20221219174027.2567505-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/search?l=mid&q=20221219174027.2567505-1-arne@rfc2549.org
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c
index 7f5e69e3e..cd4083c49 100644
--- a/src/openvpn/dco_freebsd.c
+++ b/src/openvpn/dco_freebsd.c
@@ -461,6 +461,9 @@  dco_set_peer(dco_context_t *dco, unsigned int peerid,
     nvlist_t *nvl;
     int ret;
 
+    msg(D_DCO_DEBUG, "%s: peer-id %d, ping interval %d, ping timeout %d",
+        __func__, peerid, keepalive_interval, keepalive_timeout);
+
     nvl = nvlist_create(0);
     nvlist_add_number(nvl, "peerid", peerid);
     nvlist_add_number(nvl, "interval", keepalive_interval);
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 88f0747f9..71d0804fa 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2119,6 +2119,26 @@  options_hash_changed_or_zero(const struct sha256_digest *a,
            || !memcmp(a, &zero, sizeof(struct sha256_digest));
 }
 
+static bool
+p2p_set_dco_keepalive(struct context *c)
+{
+    if (dco_enabled(&c->options)
+        && (c->options.ping_send_timeout || c->c2.frame.mss_fix))
+    {
+        int ret = dco_set_peer(&c->c1.tuntap->dco,
+                               c->c2.tls_multi->dco_peer_id,
+                               c->options.ping_send_timeout,
+                               c->options.ping_rec_timeout,
+                               c->c2.frame.mss_fix);
+        if (ret < 0)
+        {
+            msg(D_DCO, "Cannot set parameters for DCO peer (id=%u): %s",
+                c->c2.tls_multi->dco_peer_id, strerror(-ret));
+            return false;
+        }
+    }
+    return true;
+}
 /**
  * This function is expected to be invoked after open_tun() was performed.
  *
@@ -2147,22 +2167,6 @@  do_deferred_options_part2(struct context *c)
         return false;
     }
 
-    if (dco_enabled(&c->options)
-        && (c->options.ping_send_timeout || c->c2.frame.mss_fix))
-    {
-        int ret = dco_set_peer(&c->c1.tuntap->dco,
-                               c->c2.tls_multi->dco_peer_id,
-                               c->options.ping_send_timeout,
-                               c->options.ping_rec_timeout,
-                               c->c2.frame.mss_fix);
-        if (ret < 0)
-        {
-            msg(D_DCO, "Cannot set parameters for DCO peer (id=%u): %s",
-                c->c2.tls_multi->dco_peer_id, strerror(-ret));
-            return false;
-        }
-    }
-
     return true;
 }
 
@@ -2265,6 +2269,12 @@  do_up(struct context *c, bool pulled_options, unsigned int option_types_found)
             }
         }
 
+        if (c->mode == MODE_POINT_TO_POINT && !p2p_set_dco_keepalive(c))
+        {
+            msg(D_TLS_ERRORS, "ERROR: Failed to apply DCO keepalive or MSS fix parameters");
+            return false;
+        }
+
         if (c->c2.did_open_tun)
         {
             c->c1.pulled_options_digest_save = c->c2.pulled_options_digest;