[3231] Set fake interface for the incoming packet in DHCPv4 unit tests.

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -55,16 +55,6 @@ Dhcpv4SrvTest::Dhcpv4SrvTest()
 
     // it's ok if that fails. There should not be such a file anyway
     unlink(SRVID_FILE);
-
-    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
-
-    // There must be some interface detected
-    if (ifaces.empty()) {
-        // We can't use ASSERT in constructor
-        ADD_FAILURE() << "No interfaces detected.";
-    }
-
-    valid_iface_ = ifaces.begin()->getName();
 }
 
 void Dhcpv4SrvTest::addPrlOption(Pkt4Ptr& pkt) {
@@ -319,9 +309,6 @@ Lease4Ptr Dhcpv4SrvTest::checkLease(const Pkt4Ptr& rsp,
     return (lease);
 }
 
-/// @brief Checks if server response (OFFER, ACK, NAK) includes proper server-id
-/// @param rsp response packet to be validated
-/// @param expected_srvid expected value of server-id
 void Dhcpv4SrvTest::checkServerId(const Pkt4Ptr& rsp, const OptionPtr& expected_srvid) {
     // Check that server included its server-id
     OptionPtr opt = rsp->getOption(DHO_DHCP_SERVER_IDENTIFIER);
@@ -381,8 +368,87 @@ Dhcpv4SrvTest::createPacketFromBuffer(const Pkt4Ptr& src_pkt,
     return (::testing::AssertionSuccess());
 }
 
+void Dhcpv4SrvTest::TearDown() {
+
+    CfgMgr::instance().deleteSubnets4();
+
+    // Let's clean up if there is such a file.
+    unlink(SRVID_FILE);
+
+    // Close all open sockets.
+    IfaceMgr::instance().closeSockets();
+
+    // Some unit tests override the default packet filtering class, used
+    // by the IfaceMgr. The dummy class, called PktFilterTest, reports the
+    // capability to directly respond to the clients without IP address
+    // assigned. This capability is not supported by the default packet
+    // filtering class: PktFilterInet. Therefore setting the dummy class
+    // allows to test scenarios, when server responds to the broadcast address
+    // on client's request, despite having support for direct response.
+    // The following call restores the use of original packet filtering class
+    // after the test.
+    try {
+        IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterInet()));
+
+    } catch (const Exception& ex) {
+        FAIL() << "Failed to restore the default (PktFilterInet) packet filtering"
+               << " class after the test. Exception has been caught: "
+               << ex.what();
+    }
+
+}
+
+Dhcpv4SrvFakeIfaceTest::Dhcpv4SrvFakeIfaceTest()
+: Dhcpv4SrvTest() {
+    // Remove current interface configuration. Instead we want to add
+    // a couple of fake interfaces.
+    IfaceMgr& ifacemgr = IfaceMgr::instance();
+    ifacemgr.closeSockets();
+    ifacemgr.clearIfaces();
+
+    // Add fake interfaces.
+    ifacemgr.addInterface(createIface("lo", 0, "127.0.0.1"));
+    ifacemgr.addInterface(createIface("eth0", 1, "192.0.3.1"));
+    ifacemgr.addInterface(createIface("eth1", 2, "10.0.0.1"));
+
+    // In order to use fake interfaces we have to supply the custom
+    // packet filtering class, which can mimic opening sockets on
+    // fake interafaces.
+    ifacemgr.setPacketFilter(PktFilterPtr(new PktFilterTest()));
+    ifacemgr.openSockets4();
+}
+
+void
+Dhcpv4SrvFakeIfaceTest::TearDown() {
+    // The base class function restores the original packet filtering class.
+    Dhcpv4SrvTest::TearDown();
+    // The base class however, doesn't re-detect real interfaces.
+    try {
+        IfaceMgr::instance().clearIfaces();
+        IfaceMgr::instance().detectIfaces();
+
+    } catch (const Exception& ex) {
+        FAIL() << "Failed to restore interface configuration after using"
+            " fake interfaces";
+    }
+}
+
+Iface
+Dhcpv4SrvFakeIfaceTest::createIface(const std::string& name, const int ifindex,
+                                    const std::string& addr) {
+    Iface iface(name, ifindex);
+    iface.addAddress(IOAddress(addr));
+    if (name == "lo") {
+        iface.flag_loopback_ = true;
+    }
+    iface.flag_up_ = true;
+    iface.flag_running_ = true;
+    iface.inactive4_ = false;
+    return (iface);
+}
 
-void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
+void
+Dhcpv4SrvFakeIfaceTest::testDiscoverRequest(const uint8_t msg_type) {
     // Create an instance of the tested class.
     boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
 
@@ -431,6 +497,8 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     // which was parsed from its wire format.
     Pkt4Ptr received;
     ASSERT_TRUE(createPacketFromBuffer(req, received));
+    // Set interface. It is required for the server to generate server id.
+    received->setIface("eth0");
     if (msg_type == DHCPDISCOVER) {
         ASSERT_NO_THROW(
             rsp = srv->processDiscover(received);
@@ -464,6 +532,9 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     ASSERT_TRUE(createPacketFromBuffer(req, received));
     ASSERT_TRUE(received->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
 
+    // Set interface. It is required for the server to generate server id.
+    received->setIface("eth0");
+
     if (msg_type == DHCPDISCOVER) {
         ASSERT_NO_THROW(rsp = srv->processDiscover(received));
 
@@ -497,6 +568,9 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     ASSERT_TRUE(createPacketFromBuffer(req, received));
     ASSERT_TRUE(received->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
 
+    // Set interface. It is required for the server to generate server id.
+    received->setIface("eth0");
+
     if (msg_type == DHCPDISCOVER) {
         ASSERT_NO_THROW(rsp = srv->processDiscover(received));
         // Should return non-NULL packet.
@@ -517,66 +591,6 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
     EXPECT_TRUE(noBasicOptions(rsp));
 }
 
-/// @brief This function cleans up after the test.
-void Dhcpv4SrvTest::TearDown() {
-
-    CfgMgr::instance().deleteSubnets4();
-
-    // Let's clean up if there is such a file.
-    unlink(SRVID_FILE);
-
-    // Close all open sockets.
-    IfaceMgr::instance().closeSockets();
-
-    // Some unit tests override the default packet filtering class, used
-    // by the IfaceMgr. The dummy class, called PktFilterTest, reports the
-    // capability to directly respond to the clients without IP address
-    // assigned. This capability is not supported by the default packet
-    // filtering class: PktFilterInet. Therefore setting the dummy class
-    // allows to test scenarios, when server responds to the broadcast address
-    // on client's request, despite having support for direct response.
-    // The following call restores the use of original packet filtering class
-    // after the test.
-    try {
-        IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterInet()));
-
-    } catch (const Exception& ex) {
-        FAIL() << "Failed to restore the default (PktFilterInet) packet filtering"
-               << " class after the test. Exception has been caught: "
-               << ex.what();
-    }
-}
-
-Dhcpv4SrvFakeIfaceTest::Dhcpv4SrvFakeIfaceTest() : Dhcpv4SrvTest() {
-    IfaceMgr& ifacemgr = IfaceMgr::instance();
-    ifacemgr.clearIfaces();
-
-    ifacemgr.addInterface(createIface("lo", 0, "127.0.0.1"));
-    ifacemgr.addInterface(createIface("eth0", 0, "192.0.3.1"));
-    ifacemgr.addInterface(createIface("eth1", 0, "10.0.0.1"));
-}
-
-Dhcpv4SrvFakeIfaceTest::~Dhcpv4SrvFakeIfaceTest() {
-    IfaceMgr& ifacemgr = IfaceMgr::instance();
-    ifacemgr.setPacketFilter(PktFilterPtr(new PktFilterInet()));
-    ifacemgr.clearIfaces();
-    ifacemgr.detectIfaces();
-}
-
-Iface
-Dhcpv4SrvFakeIfaceTest::createIface(const std::string& name, const int ifindex,
-                                    const std::string& addr) {
-    Iface iface(name, ifindex);
-    iface.addAddress(IOAddress(addr));
-    if (name == "lo") {
-        iface.flag_loopback_ = true;
-    }
-    iface.flag_up_ = true;
-    iface.flag_running_ = true;
-    iface.inactive4_ = false;
-    return (iface);
-}
-
 }; // end of isc::dhcp::test namespace
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -237,24 +237,12 @@ public:
     createPacketFromBuffer(const Pkt4Ptr& src_pkt,
                            Pkt4Ptr& dst_pkt);
 
+
     /// @brief generates a DHCPv4 packet based on provided hex string
     ///
     /// @return created packet
     Pkt4Ptr packetFromCapture(const std::string& hex_string);
 
-    /// @brief Tests if Discover or Request message is processed correctly
-    ///
-    /// This test verifies that the Parameter Request List option is handled
-    /// correctly, i.e. it checks that certain options are present in the
-    /// server's response when they are requested and that they are not present
-    /// when they are not requested or NAK occurs.
-    ///
-    /// @todo We need an additional test for PRL option using real traffic
-    /// capture.
-    ///
-    /// @param msg_type DHCPDISCOVER or DHCPREQUEST
-    void testDiscoverRequest(const uint8_t msg_type);
-
     /// @brief This function cleans up after the test.
     virtual void TearDown();
 
@@ -271,8 +259,6 @@ public:
 
     isc::data::ConstElementPtr comment_;
 
-    // Name of a valid network interface
-    std::string valid_iface_;
 };
 
 /// @brief Test fixture class to be used for tests which require fake
@@ -299,11 +285,8 @@ public:
     /// These interfaces replace the real interfaces detected by the IfaceMgr.
     Dhcpv4SrvFakeIfaceTest();
 
-    /// @brief Destructor.
-    ///
-    /// Re-detects the network interfaces. Also, sets the default packet filter
-    /// class, in case the test has changed it.
-    virtual ~Dhcpv4SrvFakeIfaceTest();
+    /// @brief Restores the original interface configuration.
+    virtual void TearDown();
 
     /// @brief Creates an instance of the interface.
     ///
@@ -314,6 +297,20 @@ public:
     /// @return Iface Instance of the interface.
     static Iface createIface(const std::string& name, const int ifindex,
                              const std::string& addr);
+
+    /// @brief Tests if Discover or Request message is processed correctly
+    ///
+    /// This test verifies that the Parameter Request List option is handled
+    /// correctly, i.e. it checks that certain options are present in the
+    /// server's response when they are requested and that they are not present
+    /// when they are not requested or NAK occurs.
+    ///
+    /// @todo We need an additional test for PRL option using real traffic
+    /// capture.
+    ///
+    /// @param msg_type DHCPDISCOVER or DHCPREQUEST
+    void testDiscoverRequest(const uint8_t msg_type);
+
 };
 
 /// @brief "Naked" DHCPv4 server, exposes internal fields
@@ -382,6 +379,7 @@ public:
     ///
     /// See fake_received_ field for description
     void fakeReceive(const Pkt4Ptr& pkt) {
+        pkt->setIface("eth0");
         fake_received_.push_back(pkt);
     }
 

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -30,9 +30,9 @@ using namespace isc::dhcp_ddns;
 
 namespace {
 
-class NameDhcpv4SrvTest : public Dhcpv4SrvTest {
+class NameDhcpv4SrvTest : public Dhcpv4SrvFakeIfaceTest {
 public:
-    NameDhcpv4SrvTest() : Dhcpv4SrvTest() {
+    NameDhcpv4SrvTest() : Dhcpv4SrvFakeIfaceTest() {
         srv_ = new NakedDhcpv4Srv(0);
     }
     virtual ~NameDhcpv4SrvTest() {
@@ -110,6 +110,7 @@ public:
                                 const bool include_clientid = true) {
         Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(msg_type, 1234));
         pkt->setRemoteAddr(IOAddress("192.0.2.3"));
+        pkt->setIface("eth0");
         // For DISCOVER we don't include server id, because client broadcasts
         // the message to all servers.
         if (msg_type != DHCPDISCOVER) {
@@ -580,7 +581,6 @@ TEST_F(NameDhcpv4SrvTest, processDiscover) {
 
     Pkt4Ptr reply;
     ASSERT_NO_THROW(reply = srv_->processDiscover(req));
-
     checkResponse(reply, DHCPOFFER, 1234);
 
     EXPECT_TRUE(srv_->name_change_reqs_.empty());
@@ -615,6 +615,9 @@ TEST_F(NameDhcpv4SrvTest, processRequestFqdnEmptyDomainName) {
 // to it when Hostname option carries the top level domain-name.
 TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
     Pkt4Ptr req = generatePktWithHostname(DHCPREQUEST, ".");
+    // Set interface for the incoming packet. The server requires it to
+    // generate client id.
+    req->setIface("eth0");
 
     Pkt4Ptr reply;
     ASSERT_NO_THROW(reply = srv_->processRequest(req));
@@ -691,6 +694,11 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsFqdn) {
 TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
     Pkt4Ptr req1 = generatePktWithHostname(DHCPREQUEST, "myhost.example.com.");
 
+    // Set interface for the incoming packet. The server requires it to
+    // generate client id.
+    req1->setIface("eth0");
+
+
     Pkt4Ptr reply;
     ASSERT_NO_THROW(reply = srv_->processRequest(req1));
 
@@ -709,6 +717,10 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
     // another one to add new entry with updated domain-name.
     Pkt4Ptr req2 = generatePktWithHostname(DHCPREQUEST, "otherhost");
 
+    // Set interface for the incoming packet. The server requires it to
+    // generate client id.
+    req2->setIface("eth0");
+
     ASSERT_NO_THROW(reply = srv_->processRequest(req2));
 
     checkResponse(reply, DHCPACK, 1234);

src/lib/dhcp/iface_mgr.cc

@@ -293,6 +293,8 @@ void IfaceMgr::stubDetectIfaces() {
     addInterface(iface);
 }
 
+
+
 bool
 IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
                        IfaceMgrErrorMsgCallback error_handler) {
@@ -554,6 +556,11 @@ IfaceMgr::getIface(const std::string& ifname) {
     return (NULL); // not found
 }
 
+void
+IfaceMgr::clearIfaces() {
+    ifaces_.clear();
+}
+
 int IfaceMgr::openSocket(const std::string& ifname, const IOAddress& addr,
                          const uint16_t port, const bool receive_bcast,
                          const bool send_bcast) {
@@ -925,7 +932,7 @@ IfaceMgr::send(const Pkt4Ptr& pkt) {
 
     // Skip checking if packet filter is non-NULL because it has been
     // already checked when packet filter was set.
-    return (packet_filter_->send(*iface, getSocket(*pkt), pkt));
+    return (packet_filter_->send(*iface, getSocket(*pkt).sockfd_, pkt));
 }
 
 
@@ -1220,8 +1227,7 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
 uint16_t IfaceMgr::getSocket(const isc::dhcp::Pkt6& pkt) {
     Iface* iface = getIface(pkt.getIface());
     if (iface == NULL) {
-        isc_throw(BadValue, "Tried to find socket for non-existent interface "
-                  << pkt.getIface());
+        isc_throw(BadValue, "Tried to find socket for non-existent interface");
     }
 
 
@@ -1274,18 +1280,18 @@ uint16_t IfaceMgr::getSocket(const isc::dhcp::Pkt6& pkt) {
               << " does not have any suitable IPv6 sockets open.");
 }
 
-uint16_t IfaceMgr::getSocket(isc::dhcp::Pkt4 const& pkt) {
+SocketInfo
+IfaceMgr::getSocket(isc::dhcp::Pkt4 const& pkt) {
     Iface* iface = getIface(pkt.getIface());
     if (iface == NULL) {
-        isc_throw(BadValue, "Tried to find socket for non-existent interface "
-                  << pkt.getIface());
+        isc_throw(BadValue, "Tried to find socket for non-existent interface");
     }
 
     const Iface::SocketCollection& socket_collection = iface->getSockets();
     Iface::SocketCollection::const_iterator s;
     for (s = socket_collection.begin(); s != socket_collection.end(); ++s) {
         if (s->family_ == AF_INET) {
-            return (s->sockfd_);
+            return (*s);
         }
         /// TODO: Add more checks here later. If remote address is
         /// not link-local, we can't use link local bound socket

src/lib/dhcp/iface_mgr.h

@@ -443,8 +443,7 @@ public:
     /// @return interface with requested name (or NULL if no such
     ///         interface is present)
     ///
-    Iface*
-    getIface(const std::string& ifname);
+    Iface* getIface(const std::string& ifname);
 
     /// @brief Returns container with all interfaces.
     ///
@@ -453,7 +452,22 @@ public:
     /// main() function completes, you should not worry much about this.
     ///
     /// @return container with all interfaces.
-    const IfaceCollection& getIfaces() { return ifaces_; }
+    const IfaceCollection& getIfaces() { return (ifaces_); }
+
+    /// @brief Removes detected interfaces.
+    ///
+    /// This method removes all detected interfaces. This method should be
+    /// used by unit tests to supply a custom set of interfaces. For example:
+    /// a unit test may create a pool of fake interfaces and use the custom
+    /// @c PktFilter class to mimic socket operation on these interfaces.
+    void clearIfaces();
+
+    /// @brief Detects network interfaces.
+    ///
+    /// This method will eventually detect available interfaces. For now
+    /// it offers stub implementation. First interface name and link-local
+    /// IPv6 address is read from interfaces.txt file.
+    void detectIfaces();
 
     /// @brief Return most suitable socket for transmitting specified IPv6 packet.
     ///
@@ -468,7 +482,7 @@ public:
     /// @return a socket descriptor
     uint16_t getSocket(const isc::dhcp::Pkt6& pkt);
 
-    /// @brief Return most suitable socket for transmitting specified IPv6 packet.
+    /// @brief Return most suitable socket for transmitting specified IPv4 packet.
     ///
     /// This method takes Pkt4 (see overloaded implementation that takes
     /// Pkt6) and chooses appropriate socket to send it. This method
@@ -478,8 +492,8 @@ public:
     ///
     /// @param pkt a packet to be transmitted
     ///
-    /// @return a socket descriptor
-    uint16_t getSocket(const isc::dhcp::Pkt4& pkt);
+    /// @return A structure describing a socket.
+    SocketInfo getSocket(const isc::dhcp::Pkt4& pkt);
 
     /// Debugging method that prints out all available interfaces.
     ///
@@ -836,14 +850,6 @@ protected:
     /// @return socket descriptor
     int openSocket6(Iface& iface, const isc::asiolink::IOAddress& addr, uint16_t port);
 
-    /// @brief Detects network interfaces.
-    ///
-    /// This method will eventually detect available interfaces. For now
-    /// it offers stub implementation. First interface name and link-local
-    /// IPv6 address is read from interfaces.txt file.
-    void
-    detectIfaces();
-
     /// @brief Stub implementation of network interface detection.
     ///
     /// This implementations reads a single line from interfaces.txt file

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -496,6 +496,25 @@ TEST_F(IfaceMgrTest, getIface) {
     EXPECT_EQ(static_cast<void*>(NULL), ifacemgr->getIface("wifi15") );
 }
 
+TEST_F(IfaceMgrTest, clearIfaces) {
+    NakedIfaceMgr ifacemgr;
+    // Create a set of fake interfaces. At the same time, remove the actual
+    // interfaces that have been detected by the IfaceMgr.
+    ifacemgr.createIfaces();
+
+    ASSERT_GT(ifacemgr.countIfaces(), 0);
+
+    boost::shared_ptr<TestPktFilter> custom_packet_filter(new TestPktFilter());
+    ASSERT_TRUE(custom_packet_filter);
+    ASSERT_NO_THROW(ifacemgr.setPacketFilter(custom_packet_filter));
+
+    ASSERT_NO_THROW(ifacemgr.openSockets4());
+
+    ifacemgr.clearIfaces();
+
+    EXPECT_EQ(0, ifacemgr.countIfaces());
+}
+
 TEST_F(IfaceMgrTest, receiveTimeout6) {
     using namespace boost::posix_time;
     std::cout << "Testing DHCPv6 packet reception timeouts."
@@ -1204,7 +1223,7 @@ TEST_F(IfaceMgrTest, socket4) {
     pkt.setIface(LOOPBACK);
 
     // Expect that we get the socket that we just opened.
-    EXPECT_EQ(socket1, ifacemgr->getSocket(pkt));
+    EXPECT_EQ(socket1, ifacemgr->getSocket(pkt).sockfd_);
 
     close(socket1);
 }
@@ -1513,7 +1532,7 @@ TEST_F(IfaceMgrTest, socketInfo) {
 
     // Socket info is set, packet has well defined interface. It should work.
     pkt4.setIface(LOOPBACK);
-    EXPECT_EQ(7, ifacemgr->getSocket(pkt4));
+    EXPECT_EQ(7, ifacemgr->getSocket(pkt4).sockfd_);
 
     EXPECT_NO_THROW(
         ifacemgr->getIface(LOOPBACK)->delSocket(7);