diff mbox series

[Openvpn-devel,v2] Fix --mtu-disc maybe|yes on Linux.

Message ID 20220222113832.13383-1-gert@greenie.muc.de
State Accepted
Headers
Series [Openvpn-devel,v2] Fix --mtu-disc maybe|yes on Linux. |

Commit Message

Gert Doering Feb. 22, 2022, 12:38 a.m. UTC 
  --mtu-disc (on Linux) needs two components to work:
   - setsockopt() with IP_MTU_DISCOVER or IPV6_MTU_DISCOVER
   - "extended error reporting" (setsockopt(IP_RECVERR) and
     then via mtu.c/format_extended_socket_error()) to react on
     "packet too big" errors on sendto() / sendmsg()

Some configure.ac reorganization broke detection of <linux/errqueue.h>
and "struct sock_extended_err".  Re-add <linux/errqueue.h> to configure.ac,
remove all the other conditionals in syshead.h, and remove the
"struct sock_extended_err" check completely (assumption: if errqueue.h
exists, it contains what we need).

Thus, the "non-helpful" socket error message turns into:

2022-02-22 12:31:42 write UDPv4 [EMSGSIZE Path-MTU=800]: Message too long (fd=3,code=90)
2022-02-22 12:31:42 Note adjusting 'mssfix 1400 mtu' to 'mssfix 800 mtu' according to path MTU discovery
2022-02-22 12:31:42 Note adjusting 'fragment 1400 mtu' to 'fragment 800 mtu' according to path MTU discovery

... while at it, fix extra space in first part of these messages, and
print o->ce.fragment for the "fragment" message...

v2: assume that "if it's linux, and has these two headers, everything
else will be there as well" and get rid of most of the #ifdef checks

Trac: #1452

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 configure.ac          | 8 +-------
 src/openvpn/mss.c     | 6 +++---
 src/openvpn/syshead.h | 2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

Comments

Arne Schwabe Feb. 22, 2022, 1:06 a.m. UTC | #1 
Am 22.02.22 um 12:38 schrieb Gert Doering:
> --mtu-disc (on Linux) needs two components to work:
>     - setsockopt() with IP_MTU_DISCOVER or IPV6_MTU_DISCOVER
>     - "extended error reporting" (setsockopt(IP_RECVERR) and
>       then via mtu.c/format_extended_socket_error()) to react on
>       "packet too big" errors on sendto() / sendmsg()
> 
> Some configure.ac reorganization broke detection of <linux/errqueue.h>
> and "struct sock_extended_err".  Re-add <linux/errqueue.h> to configure.ac,
> remove all the other conditionals in syshead.h, and remove the
> "struct sock_extended_err" check completely (assumption: if errqueue.h
> exists, it contains what we need).
> 
> Thus, the "non-helpful" socket error message turns into:
> 
> 2022-02-22 12:31:42 write UDPv4 [EMSGSIZE Path-MTU=800]: Message too long (fd=3,code=90)
> 2022-02-22 12:31:42 Note adjusting 'mssfix 1400 mtu' to 'mssfix 800 mtu' according to path MTU discovery
> 2022-02-22 12:31:42 Note adjusting 'fragment 1400 mtu' to 'fragment 800 mtu' according to path MTU discovery
> 
> ... while at it, fix extra space in first part of these messages, and
> print o->ce.fragment for the "fragment" message...
> 
> v2: assume that "if it's linux, and has these two headers, everything
> else will be there as well" and get rid of most of the #ifdef checks
> 

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Feb. 22, 2022, 2:31 a.m. UTC | #2 
Patch has been applied to the master branch, backported to release/2.5 
(leaving out the mss.c hunks, and the syshead.h one still has HAVE_IPVEC 
to remove...) and to release/2.4 (sys/poll.h in configure.ac).

Verified that 2.5 and 2.4 show the right socket error message
("write UDPv4 [EMSGSIZE Path-MTU=800]"), which is what this bugfix
is fixing - have not verified that the old frame/MTU code actually
works, but I suspect we never managed to break it.

Note: this fixes --mtu-disc over IPv4 transport.  IPv6 transport returns
"extended error messages" now as well, but now the handler seems to expect
something else, and does not recognize this as "mtu".  Will address this
in the next patch.

commit 4225114b96723bdecd68398f7a89765879b31b5d (master)
commit 3e0c506e5d9135ef4b08547db8679cc5bd2a7582 (release/2.5)
commit 4d63d15ef9e1eb34ffdc4028a96f506decced99c (release/2.4)
Author: Gert Doering
Date:   Tue Feb 22 12:38:32 2022 +0100

     Fix --mtu-disc maybe|yes on Linux.

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


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index d42185d0..9c898718 100644
--- a/configure.ac
+++ b/configure.ac
@@ -425,7 +425,7 @@  AC_CHECK_HEADERS([ \
 	unistd.h libgen.h stropts.h \
 	syslog.h pwd.h grp.h termios.h \
 	sys/sockio.h sys/uio.h linux/sockios.h \
-	linux/types.h poll.h sys/epoll.h err.h \
+	linux/types.h linux/errqueue.h poll.h sys/epoll.h err.h \
 ])
 
 SOCKET_INCLUDES="
@@ -484,12 +484,6 @@  AC_CHECK_TYPE(
 	,
 	[[${SOCKET_INCLUDES}]]
 )
-AC_CHECK_TYPE(
-	[struct sock_extended_err],
-	[AC_DEFINE([HAVE_SOCK_EXTENDED_ERR], [1], [struct sock_extended_err needed for extended socket error support])],
-	,
-	[[${SOCKET_INCLUDES}]]
-)
 AC_CHECK_TYPE(
 	[struct msghdr],
 	[AC_DEFINE([HAVE_MSGHDR], [1], [struct msghdr needed for extended socket error support])],
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 81692e91..25b26405 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -361,7 +361,7 @@  frame_adjust_path_mtu(struct context *c)
         || (o->ce.mssfix_encap && pmtu < o->ce.mssfix + encap_overhead))
     {
         const char* mtustr = o->ce.mssfix_encap ? " mtu" : "";
-        msg(D_MTU_INFO, "Note adjusting 'mssfix %d %s' to 'mssfix %d mtu' "
+        msg(D_MTU_INFO, "Note adjusting 'mssfix %d%s' to 'mssfix %d mtu' "
                         "according to path MTU discovery", o->ce.mssfix,
             mtustr, pmtu);
         o->ce.mssfix = pmtu;
@@ -374,8 +374,8 @@  frame_adjust_path_mtu(struct context *c)
         (o->ce.fragment_encap && pmtu < o->ce.fragment + encap_overhead))
     {
         const char* mtustr = o->ce.fragment_encap ? " mtu" : "";
-        msg(D_MTU_INFO, "Note adjusting 'fragment %d %s' to 'fragment %d mtu' "
-                        "according to path MTU discovery", o->ce.mssfix,
+        msg(D_MTU_INFO, "Note adjusting 'fragment %d%s' to 'fragment %d mtu' "
+                        "according to path MTU discovery", o->ce.fragment,
             mtustr, pmtu);
         o->ce.fragment = pmtu;
         o->ce.fragment_encap = true;
diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h
index d699c608..a638b3dd 100644
--- a/src/openvpn/syshead.h
+++ b/src/openvpn/syshead.h
@@ -371,7 +371,7 @@  typedef int MIB_TCP_STATE;
 /*
  * Do we have the capability to report extended socket errors?
  */
-#if defined(HAVE_LINUX_TYPES_H) && defined(HAVE_LINUX_ERRQUEUE_H) && defined(HAVE_SOCK_EXTENDED_ERR) && defined(HAVE_MSGHDR) && defined(HAVE_CMSGHDR) && defined(CMSG_FIRSTHDR) && defined(CMSG_NXTHDR) && defined(IP_RECVERR) && defined(MSG_ERRQUEUE) && defined(SOL_IP)
+#if defined(HAVE_LINUX_TYPES_H) && defined(HAVE_LINUX_ERRQUEUE_H)
 #define EXTENDED_SOCKET_ERROR_CAPABILITY 1
 #else
 #define EXTENDED_SOCKET_ERROR_CAPABILITY 0