From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 29 Apr 2013 09:00:22 +0000 (+0200)
Subject: [2902] Direct traffic is optional in the Dhcp4Srv to allow unit testing.
X-Git-Tag: bind10-1.2.0beta1-release~440^2~18
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d20b1f2ce6030846d6de80f6addf2c5ce53d564f;p=thirdparty%2Fkea.git

[2902] Direct traffic is optional in the Dhcp4Srv to allow unit testing.
---

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 1182d06d18..b0908bb754 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -57,7 +57,8 @@ static const char* SERVER_ID_FILE = "b10-dhcp4-serverid";
 // These are hardcoded parameters. Currently this is a skeleton server that only
 // grants those options and a single, fixed, hardcoded lease.
 
-Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast) {
+Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
+                     const bool direct_response_desired) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
         // First call to instance() will create IfaceMgr (it's a singleton)
@@ -67,7 +68,7 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast)
         // to the client which doesn't have address assigned. This capability
         // may be lacking on some OSes, so there is no guarantee that server
         // will be able to respond directly.
-        IfaceMgr::instance().setMatchingPacketFilter(true);
+        IfaceMgr::instance().setMatchingPacketFilter(direct_response_desired);
 
         if (port) {
             // open sockets only if port is non-zero. Port 0 is used
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index bc8851eb0c..32e9f0d6a8 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -61,15 +61,20 @@ class Dhcpv4Srv : public boost::noncopyable {
     /// network interaction. Will instantiate lease manager, and load
     /// old or create new DUID. It is possible to specify alternate
     /// port on which DHCPv4 server will listen on. That is mostly useful
-    /// for testing purposes.
+    /// for testing purposes. The Last two arguments of the constructor
+    /// should be left at default values for normal server operation.
+    /// They should be disabled when creating an instance of this class
+    /// for unit testing as enabling them requires root privilegs.
     ///
     /// @param port specifies port number to listen on
     /// @param dbconfig Lease manager configuration string.  The default
     ///        of the "memfile" manager is used for testing.
     /// @param use_bcast configure sockets to support broadcast messages.
+    /// @param specifies if it is desired to support direct V4 traffic.
     Dhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT,
               const char* dbconfig = "type=memfile",
-              const bool use_bcast = true);
+              const bool use_bcast = true,
+              const bool direct_response_desired = true);
 
     /// @brief Destructor. Used during DHCPv4 service shutdown.
     ~Dhcpv4Srv();
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 18e3ee369b..daa94d72fa 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -50,10 +50,12 @@ public:
     ///
     /// It disables configuration of broadcast options on
     /// sockets that are opened by the Dhcpv4Srv constructor.
-    /// Setting broadcast options requires root privileges
-    /// which is not the case when running unit tests.
+    /// Also, disables the Direct V4 traffic as it requires
+    /// use of raw sockets. Use of broadcast as well as raw
+    /// sockets require root privileges, thus can't be used
+    /// in unit testing.
     NakedDhcpv4Srv(uint16_t port = 0)
-        : Dhcpv4Srv(port, "type=memfile", false) {
+        : Dhcpv4Srv(port, "type=memfile", false, false) {
     }
 
     using Dhcpv4Srv::processDiscover;
diff --git a/src/lib/dhcp/pkt_filter_inet.cc b/src/lib/dhcp/pkt_filter_inet.cc
index a05ea482ff..a80f6b7a26 100644
--- a/src/lib/dhcp/pkt_filter_inet.cc
+++ b/src/lib/dhcp/pkt_filter_inet.cc
@@ -28,25 +28,12 @@ PktFilterInet::PktFilterInet()
 {
 }
 
-// iface is only used when SO_BINDTODEVICE is defined and thus
-// the code section using this variable is compiled.
-#ifdef SO_BINDTODEVICE
 int PktFilterInet::openSocket(const Iface& iface,
                               const isc::asiolink::IOAddress& addr,
                               const uint16_t port,
                               const bool receive_bcast,
                               const bool send_bcast) {
 
-#else
-int PktFilterInet::openSocket(const Iface&,
-                              const isc::asiolink::IOAddress& addr,
-                              const uint16_t port,
-                              const bool receive_bcast,
-                              const bool send_bcast) {
-
-
-#endif
-
     struct sockaddr_in addr4;
     memset(&addr4, 0, sizeof(sockaddr));
     addr4.sin_family = AF_INET;
@@ -54,7 +41,7 @@ int PktFilterInet::openSocket(const Iface&,
 
     // If we are to receive broadcast messages we have to bind
     // to "ANY" address.
-    if (receive_bcast) {
+    if (receive_bcast && iface.flag_broadcast_) {
         addr4.sin_addr.s_addr = INADDR_ANY;
     } else {
         addr4.sin_addr.s_addr = htonl(addr);
@@ -77,7 +64,7 @@ int PktFilterInet::openSocket(const Iface&,
     }
 #endif
 
-    if (send_bcast) {
+    if (send_bcast && iface.flag_broadcast_) {
         // Enable sending to broadcast address.
         int flag = 1;
         if (setsockopt(sock, SOL_SOCKET, SO_BROADCAST, &flag, sizeof(flag)) < 0) {