Browse Source

[2765] Receive and discard data from the fallback socket.

Marcin Siodelski 11 years ago
parent
commit
8cd003e665

+ 12 - 0
src/lib/dhcp/pkt_filter.cc

@@ -16,6 +16,9 @@
 #include <dhcp/iface_mgr.h>
 #include <dhcp/pkt_filter.h>
 
+#include <sys/fcntl.h>
+#include <sys/socket.h>
+
 namespace isc {
 namespace dhcp {
 
@@ -42,6 +45,15 @@ PktFilter::openFallbackSocket(const isc::asiolink::IOAddress& addr,
                   << addr.toText() << ", port " << port << " - is another DHCP "
                   "server running?");
     }
+
+    // Set socket to non-blocking mode. This is to prevent the read from the fallback
+    // socket to block message processing on the primary socket.
+    if (fcntl(sock, F_SETFL, O_NONBLOCK) != 0) {
+        close(sock);
+        isc_throw(SocketConfigError, "failed to set SO_NONBLOCK option on the"
+                  " fallback socket, bound to " << addr.toText() << ", port "
+                  << port);
+    }
     // Successfully created and bound a fallback socket. Return a descriptor.
     return (sock);
 }

+ 13 - 0
src/lib/dhcp/pkt_filter_lpf.cc

@@ -165,6 +165,19 @@ PktFilterLPF::openSocket(const Iface& iface,
 Pkt4Ptr
 PktFilterLPF::receive(const Iface& iface, const SocketInfo& socket_info) {
     uint8_t raw_buf[IfaceMgr::RCVBUFSIZE];
+    // First let's get some data from the fallback socket. The data will be
+    // discarded but we don't want the socket buffer to bloat. We get the
+    // packets from the socket in loop but most of the time the loop will
+    // end after receiving one packet. The call to recv returns immediately
+    // when there is no data left on the socket because the socket is
+    // non-blocking.
+    int datalen;
+    do {
+        datalen = recv(socket_info.fallbackfd_, raw_buf, sizeof(raw_buf), 0);
+    } while (datalen >= 0);
+
+    // Now that we finished getting data from the fallback socket, we
+    // have to get the data from the raw socket too.
     int data_len = read(socket_info.sockfd_, raw_buf, sizeof(raw_buf));
     // If negative value is returned by read(), it indicates that an
     // error occured. If returned value is 0, no data was read from the

+ 10 - 1
src/lib/dhcp/tests/pkt_filter_unittest.cc

@@ -45,16 +45,25 @@ TEST_F(PktFilterBaseClassTest, openFallbackSocket) {
     PktFilterStub pkt_filter;
     ASSERT_NO_THROW(sock_info_.fallbackfd_ =
                     pkt_filter.openFallbackSocket(IOAddress("127.0.0.1"), PORT)
+                    << "Failed to open fallback socket.";
     );
     // Test that the socket has been successfully created.
     testDgramSocket(sock_info_.fallbackfd_);
 
+    // In addition, we should check that the fallback socket is non-blocking.
+    int sock_flags = fcntl(sock_info_.fallbackfd_, F_GETFL);
+    EXPECT_EQ(O_NONBLOCK, sock_flags & O_NONBLOCK)
+        << "Fallback socket is blocking, it should be non-blocking - check"
+        " fallback socket flags (fcntl).";
+
     // Now that we have the socket open, let's try to open another one. This
     // should cause a binding error.
     int another_sock;
     EXPECT_THROW(another_sock =
                  pkt_filter.openFallbackSocket(IOAddress("127.0.0.1"), PORT),
-                 isc::dhcp::SocketConfigError);
+                 isc::dhcp::SocketConfigError)
+        << "it should be not allowed to open and bind two fallback sockets"
+        " to the same address and port. Surprisingly, the socket bound.";
     // Hard to believe we managed to open another socket. But if so, we have
     // to close it to prevent a resource leak.
     if (another_sock >= 0) {