Message ID | 20220222113832.13383-1-gert@greenie.muc.de |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel,v2] Fix --mtu-disc maybe|yes on Linux. | expand |
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>
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 --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
--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(-)