BIND 10 master, updated. ff013364643f9bfa736b2d23fec39ac35872d6ad Merge #1534

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Feb 27 18:29:19 UTC 2012


The branch, master has been updated
       via  ff013364643f9bfa736b2d23fec39ac35872d6ad (commit)
       via  c5ba45e293726f7642f6e081de16bc3b2daff98b (commit)
       via  ffd2c98603eb6db553d4cf18d2fd7ac29a62080f (commit)
       via  bc4fc1865977c94d4ad6eddf0cf96a7881a71536 (commit)
       via  f43af5ae2a1a904d1fa091d227f6d5cb1c580fa3 (commit)
       via  5758984e174f25954af41dcf07979e2dfca12763 (commit)
       via  a0f1ca23cd394e0d94845ecc08c35e56bf313b01 (commit)
       via  e494212d02fa6efcb437f966708992de8b02cb74 (commit)
      from  79e99b7285b93325862f80746d3a4d1b5938ca4d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit ff013364643f9bfa736b2d23fec39ac35872d6ad
Merge: 79e99b7285b93325862f80746d3a4d1b5938ca4d c5ba45e293726f7642f6e081de16bc3b2daff98b
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Feb 27 19:21:02 2012 +0100

    Merge #1534
    
    Conflicts:
    	src/bin/sockcreator/Makefile.am

commit c5ba45e293726f7642f6e081de16bc3b2daff98b
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Feb 27 17:39:28 2012 +0100

    [1534] makefile linker addition

commit ffd2c98603eb6db553d4cf18d2fd7ac29a62080f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon Feb 27 13:57:34 2012 +0100

    [1534] Use the other methods of setting it as fallback
    
    Meaning if it works, don't try the others. Also, a little comment in
    test was added.

commit bc4fc1865977c94d4ad6eddf0cf96a7881a71536
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Feb 23 12:58:52 2012 +0100

    [1534] Close sockets on errors

commit f43af5ae2a1a904d1fa091d227f6d5cb1c580fa3
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Feb 23 12:34:27 2012 +0100

    [1534] Remove the failing test
    
    Because it's the test which is failing, not the code. Unfortunately, it
    can't be reliably tested :-(.

commit 5758984e174f25954af41dcf07979e2dfca12763
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Feb 21 12:43:44 2012 +0100

    [1534] Try connecting before getsockopt
    
    Because it complains it is not supported on sockets not connected.

commit a0f1ca23cd394e0d94845ecc08c35e56bf313b01
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Feb 16 13:01:35 2012 +0100

    [1534] Implement the setting
    
    But, for some reason, it doesn't really work. The tests fail with
    strange error. To be talked about on ML.

commit e494212d02fa6efcb437f966708992de8b02cb74
Merge: 72b239285aa03f9afa5685a4665451d89a941143 d3781127f6d22976c761003fba52df8c9c30436d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Feb 16 10:30:05 2012 +0100

    Merge ongoing #1593 into #1534
    
    To minimize future conflicts, the #1593 (sockcreator cleanup) might have
    severe impact on the code.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/sockcreator/sockcreator.cc             |   61 ++++++++++++++++--
 src/bin/sockcreator/sockcreator.h              |   15 +++--
 src/bin/sockcreator/tests/sockcreator_tests.cc |   81 ++++++++++++++++++++----
 3 files changed, 133 insertions(+), 24 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/sockcreator/sockcreator.cc b/src/bin/sockcreator/sockcreator.cc
index d7fe1f8..167e3f0 100644
--- a/src/bin/sockcreator/sockcreator.cc
+++ b/src/bin/sockcreator/sockcreator.cc
@@ -157,7 +157,7 @@ handleRequest(const int input_fd, const int output_fd,
     }
 
     // Obtain the socket
-    const int result = get_sock(sock_type, addr, addr_len);
+    const int result = get_sock(sock_type, addr, addr_len, close_fun);
     if (result >= 0) {
         // Got the socket, send it to the client.
         writeMessage(output_fd, "S", 1);
@@ -186,6 +186,52 @@ handleRequest(const int input_fd, const int output_fd,
     }
 }
 
+// Sets the MTU related flags for IPv6 UDP sockets.
+// It is borrowed from bind-9 lib/isc/unix/socket.c and modified
+// to compile here.
+//
+// The function returns -2 if it fails or the socket file descriptor
+// on success (for convenience, so the result can be just returned).
+int
+mtu(int fd) {
+#ifdef IPV6_USE_MIN_MTU        /* RFC 3542, not too common yet*/
+    const int on(1);
+    // use minimum MTU
+    if (setsockopt(fd, IPPROTO_IPV6, IPV6_USE_MIN_MTU, &on, sizeof(on)) < 0) {
+        return (-2);
+    }
+#else // Try the following as fallback
+#ifdef IPV6_MTU
+    // Use minimum MTU on systems that don't have the IPV6_USE_MIN_MTU
+    const int mtu = 1280;
+    if (setsockopt(fd, IPPROTO_IPV6, IPV6_MTU, &mtu, sizeof(mtu)) < 0) {
+        return (-2);
+    }
+#endif
+#if defined(IPV6_MTU_DISCOVER) && defined(IPV6_PMTUDISC_DONT)
+    // Turn off Path MTU discovery on IPv6/UDP sockets.
+    const int action = IPV6_PMTUDISC_DONT;
+    if (setsockopt(fd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, &action,
+                   sizeof(action)) < 0) {
+
+        return (-2);
+    }
+#endif
+#endif
+    return (fd);
+}
+
+// This one closes the socket if result is negative. Used not to leak socket
+// on error.
+int maybeClose(const int result, const int socket, const close_t close_fun) {
+    if (result < 0) {
+        if (close_fun(socket) == -1) {
+            isc_throw(InternalError, "Error closing socket");
+        }
+    }
+    return (result);
+}
+
 } // Anonymous namespace
 
 namespace isc {
@@ -193,7 +239,8 @@ namespace socket_creator {
 
 // Get the socket and bind to it.
 int
-getSock(const int type, struct sockaddr* bind_addr, const socklen_t addr_len) {
+getSock(const int type, struct sockaddr* bind_addr, const socklen_t addr_len,
+        const close_t close_fun) {
     const int sock = socket(bind_addr->sa_family, type, 0);
     if (sock == -1) {
         return (-1);
@@ -201,15 +248,19 @@ getSock(const int type, struct sockaddr* bind_addr, const socklen_t addr_len) {
     const int on = 1;
     if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) == -1) {
         // This is part of the binding process, so it's a bind error
-        return (-2);
+        return (maybeClose(-2, sock, close_fun));
     }
     if (bind_addr->sa_family == AF_INET6 &&
         setsockopt(sock, IPPROTO_IPV6, IPV6_V6ONLY, &on, sizeof(on)) == -1) {
         // This is part of the binding process, so it's a bind error
-        return (-2);
+        return (maybeClose(-2, sock, close_fun));
     }
     if (bind(sock, bind_addr, addr_len) == -1) {
-        return (-2);
+        return (maybeClose(-2, sock, close_fun));
+    }
+    if (type == SOCK_DGRAM && bind_addr->sa_family == AF_INET6) {
+        // Set some MTU flags on IPv6 UDP sockets.
+        return (maybeClose(mtu(sock), sock, close_fun));
     }
     return (sock);
 }
diff --git a/src/bin/sockcreator/sockcreator.h b/src/bin/sockcreator/sockcreator.h
index 012d8c3..e5d4783 100644
--- a/src/bin/sockcreator/sockcreator.h
+++ b/src/bin/sockcreator/sockcreator.h
@@ -73,6 +73,9 @@ public:
 };
 
 
+// Type of the close() function, so it can be passed as a parameter.
+// Argument is the same as that for close(2).
+typedef int (*close_t)(int);
 
 /// \short Create a socket and bind it.
 ///
@@ -82,13 +85,16 @@ public:
 /// \param type The type of socket to create (SOCK_STREAM, SOCK_DGRAM, etc).
 /// \param bind_addr The address to bind.
 /// \param addr_len The actual length of bind_addr.
+/// \param close_fun The furction used to close a socket if there's an error
+///     after the creation.
 ///
 /// \return The file descriptor of the newly created socket, if everything
 ///         goes well. A negative number is returned if an error occurs -
 ///         -1 if the socket() call fails or -2 if bind() fails. In case of
 ///         error, errno is set (or left intact from socket() or bind()).
 int
-getSock(const int type, struct sockaddr* bind_addr, const socklen_t addr_len);
+getSock(const int type, struct sockaddr* bind_addr, const socklen_t addr_len,
+        const close_t close_fun);
 
 // Define some types for functions used to perform socket-related operations.
 // These are typedefed so that alternatives can be passed through to the
@@ -96,16 +102,13 @@ getSock(const int type, struct sockaddr* bind_addr, const socklen_t addr_len);
 
 // Type of the function to get a socket and to pass it as parameter.
 // Arguments are those described above for getSock().
-typedef int (*get_sock_t)(const int, struct sockaddr *, const socklen_t);
+typedef int (*get_sock_t)(const int, struct sockaddr *, const socklen_t,
+                          const close_t close_fun);
 
 // Type of the send_fd() function, so it can be passed as a parameter.
 // Arguments are the same as those of the send_fd() function.
 typedef int (*send_fd_t)(const int, const int);
 
-// Type of the close() function, so it can be passed as a parameter.
-// Argument is the same as that for close(2).
-typedef int (*close_t)(int);
-
 
 /// \brief Infinite loop parsing commands and returning the sockets.
 ///
diff --git a/src/bin/sockcreator/tests/sockcreator_tests.cc b/src/bin/sockcreator/tests/sockcreator_tests.cc
index eccc3ed..9bbb789 100644
--- a/src/bin/sockcreator/tests/sockcreator_tests.cc
+++ b/src/bin/sockcreator/tests/sockcreator_tests.cc
@@ -22,6 +22,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
+#include <arpa/inet.h>
 #include <unistd.h>
 
 #include <iostream>
@@ -105,17 +106,47 @@ typedef void (*socket_check_t)(const int);
 // The other argument is the socket descriptor number.
 
 // IPv4 check
-void addressFamilySpecificCheck(const sockaddr_in*, const int) {
+void addressFamilySpecificCheck(const sockaddr_in*, const int, const int) {
 }
 
 // IPv6 check
-void addressFamilySpecificCheck(const sockaddr_in6*, const int socknum) {
+void addressFamilySpecificCheck(const sockaddr_in6*, const int socknum,
+                                const int socket_type)
+{
     int options;
     socklen_t len = sizeof(options);
-    EXPECT_EQ(0, getsockopt(socknum, IPPROTO_IPV6, IPV6_V6ONLY, &options, &len));
+    EXPECT_EQ(0, getsockopt(socknum, IPPROTO_IPV6, IPV6_V6ONLY, &options,
+                            &len));
     EXPECT_NE(0, options);
+    if (socket_type == SOCK_DGRAM) {
+    // Some more checks for UDP - MTU
+#ifdef IPV6_USE_MIN_MTU        /* RFC 3542, not too common yet*/
+        // use minimum MTU
+        EXPECT_EQ(getsockopt(socknum, IPPROTO_IPV6, IPV6_USE_MIN_MTU, &options,
+                             &len)) << strerror(errno);
+        EXPECT_NE(0, options);
+#else
+        // We do not check for the IPV6_MTU, because while setting works (eg.
+        // the packets are fragmented correctly), the getting does not. If
+        // we try to getsockopt it, an error complaining it can't be accessed
+        // on unconnected socket is returned. If we try to connect it, the
+        // MTU of the interface is returned, not the one we set. So we live
+        // in belief it works because we examined the packet dump.
+#if defined(IPV6_MTU_DISCOVER) && defined(IPV6_PMTUDISC_DONT)
+        // Turned off Path MTU discovery on IPv6/UDP sockets?
+        EXPECT_EQ(0, getsockopt(socknum, IPPROTO_IPV6, IPV6_MTU_DISCOVER,
+                                &options, &len)) << strerror(errno);
+        EXPECT_EQ(IPV6_PMTUDISC_DONT, options);
+#endif
+#endif
+    }
 }
 
+// Just ignore the fd and pretend success. We close invalid fds in the tests.
+int
+closeIgnore(int) {
+    return (0);
+}
 
 // Generic version of the socket test.  It creates the socket and checks that
 // it is a valid descriptor.  The family-specific check functions are called
@@ -133,7 +164,8 @@ void testAnyCreate(int socket_type, socket_check_t socket_check) {
     memset(&addr, 0, sizeof(addr));
     setAddressFamilyFields(&addr);
     sockaddr* addr_ptr = reinterpret_cast<sockaddr*>(&addr);
-    const int socket = getSock(socket_type, addr_ptr, sizeof(addr));
+    const int socket = getSock(socket_type, addr_ptr, sizeof(addr),
+                               closeIgnore);
     ASSERT_GE(socket, 0) << "Couldn't create socket: failed with " <<
         "return code " << socket << " and error " << strerror(errno);
 
@@ -147,7 +179,7 @@ void testAnyCreate(int socket_type, socket_check_t socket_check) {
     EXPECT_NE(0, options);
 
     // ...and the address-family specific tests.
-    addressFamilySpecificCheck(&addr, socket);
+    addressFamilySpecificCheck(&addr, socket, socket_type);
 
     // Tidy up and exit.
     EXPECT_EQ(0, close(socket));
@@ -171,12 +203,40 @@ TEST(get_sock, tcp6_create) {
     testAnyCreate<sockaddr_in6>(SOCK_STREAM, tcpCheck);
 }
 
+bool close_called(false);
+
+// You can use it as a close mockup. If you care about checking if it was really
+// called, you can use the close_called variable. But set it to false before the
+// test.
+int closeCall(int socket) {
+    close(socket);
+    close_called = true;
+    return (0);
+}
+
 // Ask the get_sock function for some nonsense and test if it is able to report
 // an error.
 TEST(get_sock, fail_with_nonsense) {
     sockaddr addr;
     memset(&addr, 0, sizeof(addr));
-    ASSERT_LT(getSock(0, &addr, sizeof addr), 0);
+    close_called = false;
+    ASSERT_EQ(-1, getSock(0, &addr, sizeof addr, closeCall));
+    ASSERT_FALSE(close_called); // The "socket" call should have failed already
+}
+
+// Bind should have failed here
+TEST(get_sock, fail_with_bind) {
+    sockaddr_in addr;
+    memset(&addr, 0, sizeof(addr));
+    addr.sin_family = AF_INET;
+    addr.sin_port = 1;
+    // No host should have this address on the interface, so it should not be
+    // possible to bind it.
+    addr.sin_addr.s_addr = inet_addr("192.0.2.1");
+    close_called = false;
+    ASSERT_EQ(-2, getSock(SOCK_STREAM, reinterpret_cast<sockaddr*>(&addr),
+                          sizeof addr, closeCall));
+    ASSERT_TRUE(close_called); // The "socket" call should have failed already
 }
 
 // The main run() function in the socket creator takes three functions to
@@ -198,7 +258,8 @@ TEST(get_sock, fail_with_nonsense) {
 // -1: The simulated bind() call has failed
 // -2: The simulated socket() call has failed
 int
-getSockDummy(const int type, struct sockaddr* addr, const socklen_t) {
+getSockDummy(const int type, struct sockaddr* addr, const socklen_t,
+             const close_t) {
     int result = 0;
     int port = 0;
 
@@ -253,12 +314,6 @@ send_FdDummy(const int destination, const int what) {
     return (status ? 0 : -1);
 }
 
-// Just ignore the fd and pretend success. We close invalid fds in the tests.
-int
-closeIgnore(int) {
-    return (0);
-}
-
 // Generic test that it works, with various inputs and outputs.
 // It uses different functions to create the socket and send it and pass
 // data to it and check it returns correct data back, to see if the run()



More information about the bind10-changes mailing list