Attention is currently required from: plaisthos, ralf_lici, stipa.

Hello plaisthos, ralf_lici,

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1910?usp=email

to look at the new patch set (#3).

The following approvals got outdated and were removed:
Code-Review+2 by ralf_lici


Change subject: socket: factor out UDP socket creation and setup
......................................................................

socket: factor out UDP socket creation and setup

create_socket() creates the UDP socket and then applies the link
socket's options and local bind in place. Pull that sequence into
create_socket_udp_configured(), taking the address family and the
option values rather than a struct link_socket, and have create_socket()
call it; the TCP path keeps its own sequence through the shared
socket_apply_options().

--server-probe will open its probe sockets through the same function, so
a probe socket is set up and bound exactly like the connection socket it
may later become. No behaviour change here.

Change-Id: I8f2bd694146aacd244a1d385ddfe9bb84ec29334
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/socket.c
M src/openvpn/socket.h
2 files changed, 69 insertions(+), 26 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/10/1910/3

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 7c6217a..c2e7f7b 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -636,12 +636,65 @@
     }
 }

+/* The per-socket options every link socket gets right after creation. */
+static void
+socket_apply_options(socket_descriptor_t sd, const struct socket_buffer_size 
*sbs, int mark,
+                     const char *bind_dev)
+{
+    /* set socket buffers based on --sndbuf and --rcvbuf options */
+    socket_set_buffers(sd, sbs, true);
+
+    /* set socket to --mark packets with given value */
+    socket_set_mark(sd, mark);
+
+#if defined(TARGET_LINUX)
+    if (bind_dev)
+    {
+        msg(M_INFO, "Using bind-dev %s", bind_dev);
+        /* Note: We verify strlen of bind_dev in options parsing */
+        if (setsockopt(sd, SOL_SOCKET, SO_BINDTODEVICE, bind_dev, 
(socklen_t)(strlen(bind_dev) + 1))
+            != 0)
+        {
+            msg(M_WARN | M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s 
failed", bind_dev);
+        }
+    }
+#else
+    (void)bind_dev;
+#endif
+}
+
+socket_descriptor_t
+create_socket_udp_configured(sa_family_t af, unsigned int sockflags, const 
struct socket_buffer_size *sbs,
+                             int mark, const char *bind_dev, struct addrinfo 
*bind_addr,
+                             bool bind_ipv6_only)
+{
+    struct addrinfo ai = { .ai_family = af, .ai_socktype = SOCK_DGRAM, 
.ai_protocol = IPPROTO_UDP };
+    socket_descriptor_t sd = create_socket_udp(&ai, sockflags);
+
+    socket_apply_options(sd, sbs, mark, bind_dev);
+    if (bind_addr)
+    {
+        socket_bind(sd, bind_addr, af, "TCP/UDP", bind_ipv6_only);
+    }
+    return sd;
+}
+
 static void
 create_socket(struct link_socket *sock, struct addrinfo *addr)
 {
+    /* Set af field of sock->info, so it always reflects the address family
+     * of the created socket */
+    sock->info.af = (sa_family_t)addr->ai_family;
+
     if (addr->ai_protocol == IPPROTO_UDP || addr->ai_socktype == SOCK_DGRAM)
     {
-        sock->sd = create_socket_udp(addr, sock->sockflags);
+        /* With a SOCKS proxy the local bind goes on the control socket instead
+         * (see bind_local()), so the UDP socket is created unbound then. */
+        struct addrinfo *bind_addr =
+            (sock->bind_local && !sock->socks_proxy) ? 
sock->info.lsa->bind_local : NULL;
+        sock->sd = create_socket_udp_configured(sock->info.af, 
sock->sockflags, &sock->socket_buffer_sizes,
+                                                sock->mark, sock->bind_dev, 
bind_addr,
+                                                sock->info.bind_ipv6_only);
         sock->sockflags |= SF_GETADDRINFO_DGRAM;

         /* Assume that control socket and data socket to the socks proxy
@@ -655,41 +708,19 @@
             addrinfo_tmp.ai_socktype = SOCK_STREAM;
             addrinfo_tmp.ai_protocol = IPPROTO_TCP;
             sock->ctrl_sd = create_socket_tcp(&addrinfo_tmp);
+            bind_local(sock);
         }
     }
     else if (addr->ai_protocol == IPPROTO_TCP || addr->ai_socktype == 
SOCK_STREAM)
     {
         sock->sd = create_socket_tcp(addr);
+        socket_apply_options(sock->sd, &sock->socket_buffer_sizes, sock->mark, 
sock->bind_dev);
+        bind_local(sock);
     }
     else
     {
         ASSERT(0);
     }
-    /* Set af field of sock->info, so it always reflects the address family
-     * of the created socket */
-    sock->info.af = (sa_family_t)addr->ai_family;
-
-    /* set socket buffers based on --sndbuf and --rcvbuf options */
-    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);
-
-#if defined(TARGET_LINUX)
-    if (sock->bind_dev)
-    {
-        msg(M_INFO, "Using bind-dev %s", sock->bind_dev);
-        /* Note: We verify strlen of bind_dev in options parsing */
-        if (setsockopt(sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev,
-                       (socklen_t)(strlen(sock->bind_dev) + 1))
-            != 0)
-        {
-            msg(M_WARN | M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s 
failed", sock->bind_dev);
-        }
-    }
-#endif
-
-    bind_local(sock);
 }

 #ifdef TARGET_ANDROID
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index 89465bc..905b4fd 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -401,6 +401,18 @@

 socket_descriptor_t create_socket_tcp(struct addrinfo *);

+/**
+ * Create a UDP socket for @p af set up the way a link socket is: 
--sndbuf/--rcvbuf,
+ * --mark, --bind-dev and, when @p bind_addr is given, the local bind 
(IPV6_V6ONLY
+ * per @p bind_ipv6_only). Shared by create_socket() and the --server-probe 
sockets,
+ * so a probe socket is the connection socket it may become. Fatal if the 
socket
+ * cannot be created or bound.
+ */
+socket_descriptor_t create_socket_udp_configured(sa_family_t af, unsigned int 
sockflags,
+                                                 const struct 
socket_buffer_size *sbs, int mark,
+                                                 const char *bind_dev, struct 
addrinfo *bind_addr,
+                                                 bool bind_ipv6_only);
+
 socket_descriptor_t socket_do_accept(socket_descriptor_t sd, struct 
link_socket_actual *act,
                                      const bool nowait);


--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1910?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I8f2bd694146aacd244a1d385ddfe9bb84ec29334
Gerrit-Change-Number: 1910
Gerrit-PatchSet: 3
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-Reviewer: ralf_lici <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: ralf_lici <[email protected]>
Gerrit-Attention: stipa <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to