[2765] Open fallback socket when LPF is used.

src/lib/dhcp/pkt_filter_lpf.cc

@@ -107,28 +107,17 @@ PktFilterLPF::openSocket(const Iface& iface,
                          const isc::asiolink::IOAddress& addr,
                          const uint16_t port, const bool,
                          const bool) {
-    // Let's check if a socket is already in use
-    int sock_check = socket(AF_INET, SOCK_DGRAM, 0);
-    if (sock_check < 0) {
-        isc_throw(SocketConfigError, "Failed to create dgram socket");
-    }
 
-    struct sockaddr_in addr4;
-    memset(& addr4, 0, sizeof(addr4));
-    addr4.sin_family = AF_INET;
-    addr4.sin_addr.s_addr = htonl(addr);
-    addr4.sin_port = htons(port);
-
-    if (bind(sock_check, (struct sockaddr *)& addr4, sizeof(addr4)) < 0) {
-        // We return negative, the proper error message will be displayed
-        // by the IfaceMgr ...
-        close(sock_check);
-        return (SocketInfo(addr, port, -1));
-    }
-    close(sock_check);
+    // Open fallback socket first. If it fails, it will give us an indication
+    // that there is another service (perhaps DHCP server) running.
+    // The function will throw an exception and effectivelly cease opening
+    // raw socket below.
+    int fallback = openFallbackSocket(addr, port);
 
+    // The fallback is open, so we are good to open primary socket.
     int sock = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
     if (sock < 0) {
+        close(fallback);
         isc_throw(SocketConfigError, "Failed to create raw LPF socket");
     }
 
@@ -146,6 +135,7 @@ PktFilterLPF::openSocket(const Iface& iface,
     if (setsockopt(sock, SOL_SOCKET, SO_ATTACH_FILTER, &filter_program,
                    sizeof(filter_program)) < 0) {
         close(sock);
+        close(fallback);
         isc_throw(SocketConfigError, "Failed to install packet filtering program"
                   << " on the socket " << sock);
     }
@@ -162,11 +152,12 @@ PktFilterLPF::openSocket(const Iface& iface,
     if (bind(sock, reinterpret_cast<const struct sockaddr*>(&sa),
              sizeof(sa)) < 0) {
         close(sock);
+        close(fallback);
         isc_throw(SocketConfigError, "Failed to bind LPF socket '" << sock
                   << "' to interface '" << iface.getName() << "'");
     }
 
-    SocketInfo sock_desc(addr, port, sock);
+    SocketInfo sock_desc(addr, port, sock, fallback);
     return (sock_desc);
 
 }

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -117,7 +117,13 @@ public:
     IfaceCollection & getIfacesLst() { return ifaces_; }
 };
 
-// Dummy class for now, but this will be expanded when needed
+/// @brief A test fixture class for IfaceMgr.
+///
+/// @todo Sockets being opened by IfaceMgr tests should be managed by
+/// the test fixture. In particular, the class should close sockets after
+/// each test. Current approach where test cases are responsible for
+/// closing sockets is resource leak prone, especially in case of the
+/// test failure path.
 class IfaceMgrTest : public ::testing::Test {
 public:
     // These are empty for now, but let's keep them around
@@ -1007,17 +1013,23 @@ TEST_F(IfaceMgrTest, checkPacketFilterLPFSocket) {
 
     // Then the second use PkFilterLPF mode
     EXPECT_NO_THROW(iface_mgr2->setMatchingPacketFilter(true));
-    // This socket opening attempt should not return positive value
-    // The first socket already opened same port
-    EXPECT_NO_THROW(
+
+    // The socket is open and bound. Another attempt to open socket and
+    // bind to the same address and port should result in an exception.
+    EXPECT_THROW(
         socket2 = iface_mgr2->openSocket(LOOPBACK, loAddr,
-                                         DHCP4_SERVER_PORT + 10000);
+                                         DHCP4_SERVER_PORT + 10000),
+        isc::dhcp::SocketConfigError
     );
+    // Surprisingly we managed to open another socket. We have to close it
+    // to prevent resource leak.
+    if (socket2 >= 0) {
+        close(socket2);
+    }
 
-    EXPECT_LE(socket2, 0);
-
-    close(socket2);
-    close(socket1);
+    if (socket1 >= 0) {
+        close(socket1);
+    }
 }
 
 #else