diff mbox series

[Openvpn-devel,v2] Ensure that the AF_UNIX socket pair has at least 65k of buffer space

Message ID 20240925063016.22532-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v2] Ensure that the AF_UNIX socket pair has at least 65k of buffer space | expand

Commit Message

Gert Doering Sept. 25, 2024, 6:30 a.m. UTC 
From: Arne Schwabe <arne@rfc2549.org>

Without this change, pinging a lwipovpn client with something like a
3000 byte payload on macOS often fails as the default buffer sizes on
macOS are 2048 for send and 4096 for receive.

Change-Id: Ice015df81543c01094479929f0cb3075ca4f3813
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/754
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Sept. 25, 2024, 7:06 a.m. UTC | #1 
The interesting part in this is the change to socket_set_buffers(),
introducing an "increase only" flag to avoid accidentially reducing
socket buffers (which happens when people use --sndbuf with a smaller
buffer size than the socket already has...) - sndbuf/rcvbuf behaviour
is not change, but for the newly created socketpair(), this is "increase
to 64k, or leave at already-larger value"...

Sanity check via a local FreeBSD test run (with lwipovpn) and a GHA run.

Your patch has been applied to the master branch.

commit bae48c1d58d5592ffecca8a490c2440ec3afe959
Author: Arne Schwabe
Date:   Wed Sep 25 08:30:16 2024 +0200

     Ensure that the AF_UNIX socket pair has at least 65k of buffer space

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


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 6c790a0..7b1e603 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -890,20 +890,23 @@ 
 #endif
 }
 
-static void
-socket_set_buffers(socket_descriptor_t fd, const struct socket_buffer_size *sbs)
+void
+socket_set_buffers(socket_descriptor_t fd, const struct socket_buffer_size *sbs,
+                   bool reduce_size)
 {
     if (sbs)
     {
         const int sndbuf_old = socket_get_sndbuf(fd);
         const int rcvbuf_old = socket_get_rcvbuf(fd);
 
-        if (sbs->sndbuf)
+        if (sbs->sndbuf
+            && (reduce_size || sndbuf_old < sbs->sndbuf))
         {
             socket_set_sndbuf(fd, sbs->sndbuf);
         }
 
-        if (sbs->rcvbuf)
+        if (sbs->rcvbuf
+            && (reduce_size || rcvbuf_old < sbs->rcvbuf))
         {
             socket_set_rcvbuf(fd, sbs->rcvbuf);
         }
@@ -986,7 +989,7 @@ 
     {
         ls->socket_buffer_sizes.sndbuf = sndbuf;
         ls->socket_buffer_sizes.rcvbuf = rcvbuf;
-        socket_set_buffers(ls->sd, &ls->socket_buffer_sizes);
+        socket_set_buffers(ls->sd, &ls->socket_buffer_sizes, true);
     }
 }
 
@@ -1136,7 +1139,7 @@ 
     sock->info.af = addr->ai_family;
 
     /* set socket buffers based on --sndbuf and --rcvbuf options */
-    socket_set_buffers(sock->sd, &sock->socket_buffer_sizes);
+    socket_set_buffers(sock->sd, &sock->socket_buffer_sizes, true);
 
     /* set socket to --mark packets with given value */
     socket_set_mark(sock->sd, sock->mark);
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index bbdabfb..2e583af 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -157,6 +157,18 @@ 
     int sndbuf;
 };
 
+/**
+ * Sets the receive and send buffer sizes of a socket descriptor.
+ *
+ * @param fd            The socket to modify
+ * @param sbs           new sizes.
+ * @param reduce_size   apply the new size even if smaller than current one
+ */
+void
+socket_set_buffers(socket_descriptor_t fd,
+                   const struct socket_buffer_size *sbs,
+                   bool reduce_size);
+
 /*
  * This is the main socket structure used by OpenVPN.  The SOCKET_
  * defines try to abstract away our implementation differences between
diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index f4ce4b7..6b6c159 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -35,6 +35,7 @@ 
 #include "wfp_block.h"
 #include "argv.h"
 #include "options.h"
+#include "socket.h"
 
 #ifndef WIN32
 /* Windows does implement some AF_UNIX functionality but key features
@@ -80,6 +81,13 @@ 
         return;
     }
 
+
+    /* Ensure that the buffer sizes are decently sized. Otherwise macOS will
+     * just have 2048 */
+    struct socket_buffer_size newsizes = {65536, 65536 };
+    socket_set_buffers(fds[0], &newsizes, false);
+    socket_set_buffers(fds[1], &newsizes, false);
+
     /* Use the first file descriptor for our side and avoid passing it
      * to the child */
     tt->fd = fds[1];