[Openvpn-devel,v4,4/8] Update fragment and mssfix related warnings

Message ID 20220210162632.3309974-4-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v4,1/8] Replace TUN_MTU_SIZE with frame->tun_mtu | expand

Commit Message

Arne Schwabe Feb. 10, 2022, 4:26 p.m. UTC
The warning that fragment/mssfix needs also tun-mtu set to 1500 makes
little sense. Remove it completely. Instead warn if there are incosistencies
between --fragment and mssfix.

Patch v2: clarify the mssfix and fragment mtu warning message
Patch v4: Rebase

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/init.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Gert Doering Feb. 13, 2022, 9:45 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Have not actually tried triggering these warnings, but the logic is
sane, and the code looks reasonable.

Client-side tested "just for completeness".

For reference, this was 15/21 in v1+v2 of this series and 06/14 in v3,
and 15/21 got an ACK from Frank Lichtenfeld with a comment about the
wording of the "the mtu" warning, which was addressed in v2.

Your patch has been applied to the master branch.

commit f1e06301c71e8c8d4eb9c93939e775596fc2f998
Author: Arne Schwabe
Date:   Thu Feb 10 17:26:28 2022 +0100

     Update fragment and mssfix related warnings

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20220210162632.3309974-4-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23753.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b9c3e166..b1952f46 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3184,12 +3184,17 @@  do_init_frame(struct context *c)
 #endif
 
 #ifdef ENABLE_FRAGMENT
-    if ((c->options.ce.mssfix || c->options.ce.fragment)
-        && c->c2.frame.tun_mtu != ETHERNET_MTU)
+    if (c->options.ce.fragment > 0 && c->options.ce.mssfix > c->options.ce.fragment)
     {
-        msg(M_WARN,
-            "WARNING: normally if you use --mssfix and/or --fragment, you should also set --tun-mtu %d (currently it is %d)",
-            ETHERNET_MTU, c->c2.frame.tun_mtu);
+        msg(M_WARN, "WARNING: if you use --mssfix and --fragment, you should "
+                    "set --fragment (%d) larger or equal than --mssfix (%d)",
+                    c->options.ce.fragment, c->options.ce.mssfix);
+    }
+    if (c->options.ce.fragment > 0 && c->options.ce.mssfix > 0
+        && c->options.ce.fragment_encap != c->options.ce.mssfix_encap)
+    {
+        msg(M_WARN, "WARNING: if you use --mssfix and --fragment, you should "
+                    "use the \"mtu\" flag for both or none of of them.");
     }
 #endif
 }