[3231] Dynamically generate server identifier and append to server response

src/bin/dhcp4/dhcp4_srv.cc

@@ -131,18 +131,20 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
 
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
     try {
-        // First call to instance() will create IfaceMgr (it's a singleton)
-        // it may throw something if things go wrong.
-        // The 'true' value of the call to setMatchingPacketFilter imposes
-        // that IfaceMgr will try to use the mechanism to respond directly
-        // 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(direct_response_desired);
-
-        // Open sockets only if port is non-zero. Port 0 is used
-        // for non-socket related testing.
+        // Open sockets only if port is non-zero. Port 0 is used for testing
+        // purposes in two cases:
+        // - when non-socket related testing is performed
+        // - when the particular test supplies its own packet filtering class.
         if (port) {
+            // First call to instance() will create IfaceMgr (it's a singleton)
+            // it may throw something if things go wrong.
+            // The 'true' value of the call to setMatchingPacketFilter imposes
+            // that IfaceMgr will try to use the mechanism to respond directly
+            // 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(direct_response_desired);
+
             // Create error handler. This handler will be called every time
             // the socket opening operation fails. We use this handler to
             // log a warning.
@@ -389,19 +391,6 @@ Dhcpv4Srv::run() {
             continue;
         }
 
-        adjustRemoteAddr(query, rsp);
-
-        if (!rsp->getHops()) {
-            rsp->setRemotePort(DHCP4_CLIENT_PORT);
-        } else {
-            rsp->setRemotePort(DHCP4_SERVER_PORT);
-        }
-
-        rsp->setLocalAddr(query->getLocalAddr());
-        rsp->setLocalPort(DHCP4_SERVER_PORT);
-        rsp->setIface(query->getIface());
-        rsp->setIndex(query->getIndex());
-
         // Specifies if server should do the packing
         bool skip_pack = false;
 
@@ -688,12 +677,21 @@ Dhcpv4Srv::appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type) {
     // add Message Type Option (type 53)
     msg->setType(msg_type);
 
-    // DHCP Server Identifier (type 54)
-    msg->addOption(getServerID());
-
     // more options will be added here later
 }
 
+void
+Dhcpv4Srv::appendServerID(const Pkt4Ptr& response) {
+    // The source address for the outbound message should have been set already.
+    // This is the address that to the best of the server's knowledge will be
+    // available from the client.
+    // @todo: perhaps we should consider some more sophisticated server id
+    // generation, but for the current use cases, it should be ok.
+    response->addOption(OptionPtr(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
+                                                     response->getLocalAddr()))
+                        );
+}
+
 void
 Dhcpv4Srv::appendRequestedOptions(const Pkt4Ptr& question, Pkt4Ptr& msg) {
 
@@ -1267,7 +1265,34 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
 }
 
 void
-Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg) {
+Dhcpv4Srv::adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response) {
+    adjustRemoteAddr(query, response);
+
+    // For the non-relayed message, the destination port is the client's port.
+    // For the relayed message, the server/relay port is a destination.
+    if (!response->getHops()) {
+        response->setRemotePort(DHCP4_CLIENT_PORT);
+    } else {
+        response->setRemotePort(DHCP4_SERVER_PORT);
+    }
+    // In many cases the query is sent to a broadcast address. This address
+    // appears as a local address in the query message. Therefore we can't
+    // simply copy local address from the query and use it as a source
+    // address for the response. Instead, we have to check what address our
+    // socket is bound to and use it as a source address. This operation
+    // may throw if for some reason the socket is closed.
+    // @todo Consider an optimization that we use local address from
+    // the query if this address is not broadcast.
+    SocketInfo sock_info = IfaceMgr::instance().getSocket(*query);
+    // Set local adddress, port and interface.
+    response->setLocalAddr(sock_info.addr_);
+    response->setLocalPort(DHCP4_SERVER_PORT);
+    response->setIface(query->getIface());
+    response->setIndex(query->getIndex());
+}
+
+void
+Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) {
     // Let's create static objects representing zeroed and broadcast
     // addresses. We will use them further in this function to test
     // other addresses against them. Since they are static, they will
@@ -1277,21 +1302,21 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg) {
 
     // If received relayed message, server responds to the relay address.
     if (question->getGiaddr() != zero_addr) {
-        msg->setRemoteAddr(question->getGiaddr());
+        response->setRemoteAddr(question->getGiaddr());
 
     // If giaddr is 0 but client set ciaddr, server should unicast the
     // response to ciaddr.
     } else if (question->getCiaddr() != zero_addr) {
-        msg->setRemoteAddr(question->getCiaddr());
+        response->setRemoteAddr(question->getCiaddr());
 
     // We can't unicast the response to the client when sending NAK,
     // because we haven't allocated address for him. Therefore,
     // NAK is broadcast.
-    } else if (msg->getType() == DHCPNAK) {
-        msg->setRemoteAddr(bcast_addr);
+    } else if (response->getType() == DHCPNAK) {
+        response->setRemoteAddr(bcast_addr);
 
     // If yiaddr is set it means that we have created a lease for a client.
-    } else if (msg->getYiaddr() != zero_addr) {
+    } else if (response->getYiaddr() != zero_addr) {
         // If the broadcast bit is set in the flags field, we have to
         // send the response to broadcast address. Client may have requested it
         // because it doesn't support reception of messages on the interface
@@ -1300,13 +1325,13 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg) {
         // directly to a client without address assigned.
         const bool bcast_flag = ((question->getFlags() & Pkt4::FLAG_BROADCAST_MASK) != 0);
         if (!IfaceMgr::instance().isDirectResponseSupported() || bcast_flag) {
-            msg->setRemoteAddr(bcast_addr);
+            response->setRemoteAddr(bcast_addr);
 
         // Client cleared the broadcast bit and we support direct responses
         // so we should unicast the response to a newly allocated address -
         // yiaddr.
         } else {
-            msg->setRemoteAddr(msg->getYiaddr());
+            response->setRemoteAddr(response ->getYiaddr());
 
         }
 
@@ -1314,7 +1339,7 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg) {
     // found ourselves at this point, the rational thing to do is to respond
     // to the address we got the query from.
     } else {
-        msg->setRemoteAddr(question->getRemoteAddr());
+        response->setRemoteAddr(question->getRemoteAddr());
 
     }
 }
@@ -1359,6 +1384,12 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
         appendBasicOptions(discover, offer);
     }
 
+    // Set the src/dest IP address, port and interface for the outgoing
+    // packet.
+    adjustIfaceData(discover, offer);
+
+    appendServerID(offer);
+
     return (offer);
 }
 
@@ -1395,6 +1426,12 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
         appendBasicOptions(request, ack);
     }
 
+    // Set the src/dest IP address, port and interface for the outgoing
+    // packet.
+    adjustIfaceData(request, ack);
+
+    appendServerID(ack);
+
     return (ack);
 }
 

src/bin/dhcp4/dhcp4_srv.h

@@ -396,10 +396,62 @@ protected:
 
     /// @brief Appends default options to a message
     ///
+    /// Currently it is only a Message Type option. This function does not add
+    /// the Server Identifier option as this option must be added using
+    /// @c Dhcpv4Srv::appendServerID.
+    ///
     /// @param msg message object (options will be added to it)
     /// @param msg_type specifies message type
     void appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type);
 
+    /// @brief Adds server identifier option to the server's response.
+    ///
+    /// This method adds a server identifier to the DHCPv4 message. It exects
+    /// that the local (source) address is set for this message. If address is
+    /// not set, it will throw an exception. This method also expects that the
+    /// server identifier option is not present in the specified message.
+    /// Otherwise, it will throw an exception on attempt to add a duplicate
+    /// server identifier option.
+    ///
+    /// @note This method doesn't throw exceptions by itself but the underlying
+    /// classes being used my throw. The reason for this method to not sanity
+    /// check the specified message is that it is meant to be called internally
+    /// by the @c Dhcpv4Srv class.
+    ///
+    /// @param [out] response DHCPv4 message to which the server identifier
+    /// option should be added.
+    static void appendServerID(const Pkt4Ptr& response);
+
+    /// @brief Set IP/UDP and interface parameters for the DHCPv4 response.
+    ///
+    /// This method sets the following parameters for the DHCPv4 message being
+    /// sent to a client:
+    /// - client unicast or a broadcast address,
+    /// - client or relay port,
+    /// - server address,
+    /// - server port,
+    /// - name and index of the interface which is to be used to send the
+    /// message.
+    ///
+    /// Internally it calls the @c Dhcpv4Srv::adjustRemoteAddr to figure
+    /// out the destination address (client unicast address or broadcast
+    /// address).
+    ///
+    /// The destination port is always DHCPv4 client (68) or relay (67) port,
+    /// depending if the response will be sent directly to a client (hops = 0),
+    /// or through a relay (hops > 0).
+    ///
+    /// The source port is always set to DHCPv4 server port (67).
+    ///
+    /// The interface selected for the response is always the same as the
+    /// one through which the query has been received.
+    ///
+    /// The source address for the response is the IPv4 address assigned to
+    /// the interface being used to send the response. This function uses
+    /// @c IfaceMgr to get the socket bound to the IPv4 address on the
+    /// particular interface.
+    static void adjustIfaceData(const Pkt4Ptr& query, const Pkt4Ptr& response);
+
     /// @brief Sets remote addresses for outgoing packet.
     ///
     /// This method sets the local and remote addresses on outgoing packet.
@@ -414,8 +466,8 @@ protected:
     /// function.
     ///
     /// @param question instance of a packet received by a server.
-    /// @param [out] msg response packet which addresses are to be adjusted.
-    void adjustRemoteAddr(const Pkt4Ptr& question, Pkt4Ptr& msg);
+    /// @param [out] response response packet which addresses are to be adjusted.
+    static void adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response);
 
     /// @brief Returns server-identifier option
     ///

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -65,19 +65,25 @@ const char* SRVID_FILE = "server-id-test.txt";
 // This test verifies that the destination address of the response
 // message is set to giaddr, when giaddr is set to non-zero address
 // in the received message.
-TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) {
-    boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
-
+TEST_F(Dhcpv4SrvFakeIfaceTest, adjustIfaceDataRelay) {
     // Create the instance of the incoming packet.
     boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
-    // Set the giaddr to non-zero address as if it was relayed.
+    // Set the giaddr to non-zero address and hops to non-zero value
+    // as if it was relayed.
     req->setGiaddr(IOAddress("192.0.2.1"));
+    req->setHops(2);
     // Set ciaddr to zero. This simulates the client which applies
     // for the new lease.
     req->setCiaddr(IOAddress("0.0.0.0"));
     // Clear broadcast flag.
     req->setFlags(0x0000);
 
+    // Set local address, port and interface.
+    req->setLocalAddr(IOAddress("192.0.3.1"));
+    req->setLocalPort(1001);
+    req->setIface("eth0");
+    req->setIndex(1);
+
     // Create a response packet. Assume that the new lease have
     // been created and new address allocated. This address is
     // stored in yiaddr field.
@@ -85,12 +91,24 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) {
     resp->setYiaddr(IOAddress("192.0.2.100"));
     // Clear the remote address.
     resp->setRemoteAddr(IOAddress("0.0.0.0"));
+    // Set hops value for the response.
+    resp->setHops(req->getHops());
 
     // This function never throws.
-    ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+    ASSERT_NO_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp));
 
     // Now the destination address should be relay's address.
     EXPECT_EQ("192.0.2.1", resp->getRemoteAddr().toText());
+    // The query has been relayed, so the response must be sent to the port 67.
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getRemotePort());
+    // Local address should be copied from the query message.
+    EXPECT_EQ("192.0.3.1", resp->getLocalAddr().toText());
+    // The local port is always DHCPv4 server port 67.
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getLocalPort());
+    // We will send response over the same interface which was used to receive
+    // query.
+    EXPECT_EQ("eth0", resp->getIface());
+    EXPECT_EQ(1, resp->getIndex());
 
     // Let's do another test and set other fields: ciaddr and
     // flags. By doing it, we want to make sure that the relay
@@ -103,7 +121,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) {
     // Clear remote address.
     resp->setRemoteAddr(IOAddress("0.0.0.0"));
 
-    ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+    ASSERT_NO_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp));
 
     // Response should be sent back to the relay address.
     EXPECT_EQ("192.0.2.50", resp->getRemoteAddr().toText());
@@ -113,9 +131,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRelay) {
 // is set to ciaddr when giaddr is set to zero and the ciaddr is set to
 // non-zero address in the received message. This is the case when the
 // client is in Renew or Rebind state.
-TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRenewRebind) {
-    boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
-
+TEST_F(Dhcpv4SrvFakeIfaceTest, adjustIfaceDataRenew) {
     // Create instance of the incoming packet.
     boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
 
@@ -132,6 +148,15 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRenewRebind) {
     // whether to unicast the response to the acquired address or
     // broadcast it.
     req->setFlags(Pkt4::FLAG_BROADCAST_MASK);
+    // This is a direct message, so the hops should be cleared.
+    req->setHops(0);
+    // Set local unicast address as if we are renewing a lease.
+    req->setLocalAddr(IOAddress("192.0.3.1"));
+    // Request is received on the DHCPv4 server port.
+    req->setLocalPort(DHCP4_SERVER_PORT);
+    // Set the interface. The response should be sent over the same interface.
+    req->setIface("eth0");
+    req->setIndex(1);
 
     // Create a response.
     boost::shared_ptr<Pkt4> resp(new Pkt4(DHCPOFFER, 1234));
@@ -142,11 +167,25 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRenewRebind) {
     resp->setYiaddr(IOAddress("192.0.2.13"));
     // Clear the remote address.
     resp->setRemoteAddr(IOAddress("0.0.0.0"));
+    // Copy hops value from the query.
+    resp->setHops(req->getHops());
 
-    ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+    ASSERT_NO_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp));
 
     // Check that server responds to ciaddr
     EXPECT_EQ("192.0.2.15", resp->getRemoteAddr().toText());
+    // The query was non-relayed, so the response should be sent to a DHCPv4
+    // client port 68.
+    EXPECT_EQ(DHCP4_CLIENT_PORT, resp->getRemotePort());
+    // The response should be sent from the unicast address on which the
+    // query has been received.
+    EXPECT_EQ("192.0.3.1", resp->getLocalAddr().toText());
+    // The response should be sent from the DHCPv4 server port.
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getLocalPort());
+    // The interface data should match the data in the query.
+    EXPECT_EQ("eth0", resp->getIface());
+    EXPECT_EQ(1, resp->getIndex());
+
 }
 
 // This test verifies that the destination address of the response message
@@ -156,9 +195,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressRenewRebind) {
 // of the response should be set to yiaddr if server supports direct responses
 // to the client which doesn't have an address yet or broadcast if the server
 // doesn't support direct responses.
-TEST_F(Dhcpv4SrvTest, adjustRemoteAddressSelect) {
-    boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
-
+TEST_F(Dhcpv4SrvFakeIfaceTest, adjustIfaceDataSelect) {
     // Create instance of the incoming packet.
     boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
 
@@ -170,13 +207,31 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressSelect) {
     // Let's clear the broadcast flag.
     req->setFlags(0);
 
+    // This is a non-relayed message, so let's clear hops count.
+    req->setHops(0);
+    // The query is sent to the broadcast address in the Select state.
+    req->setLocalAddr(IOAddress("255.255.255.255"));
+    // The query has been received on the DHCPv4 server port 67.
+    req->setLocalPort(DHCP4_SERVER_PORT);
+    // Set the interface. The response should be sent via the same interface.
+    req->setIface("eth0");
+    req->setIndex(1);
+
     // Create a response.
     boost::shared_ptr<Pkt4> resp(new Pkt4(DHCPOFFER, 1234));
     // Assign some new address for this client.
     resp->setYiaddr(IOAddress("192.0.2.13"));
-
     // Clear the remote address.
     resp->setRemoteAddr(IOAddress("0.0.0.0"));
+    // Copy hops count.
+    resp->setHops(req->getHops());
+
+    // We want to test the case, when the server (packet filter) doesn't support
+    // ddirect responses to the client which doesn't have an address yet. In
+    // case, the server should send its response to the broadcast address.
+    // We can control whether the current packet filter returns that its support
+    // direct responses or not.
+    current_pkt_filter_->direct_resp_supported_ = false;
 
     // When running unit tests, the IfaceMgr is using the default Packet
     // Filtering class, PktFilterInet. This class does not support direct
@@ -184,24 +239,36 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressSelect) {
     // are zero and client has just got new lease, the assigned address is
     // carried in yiaddr. In order to send this address to the client,
     // server must broadcast its response.
-    ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+    ASSERT_NO_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp));
 
     // Check that the response is sent to broadcast address as the
     // server doesn't have capability to respond directly.
     EXPECT_EQ("255.255.255.255", resp->getRemoteAddr().toText());
 
+    // Although the query has been sent to the broadcast address, the
+    // server should select a unicast address on the particular interface
+    // as a source address for the response.
+    EXPECT_EQ("192.0.3.1", resp->getLocalAddr().toText());
+
+    // The response should be sent from the DHCPv4 server port.
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getLocalPort());
+
+    // The response should be sent via the same interface through which
+    // query has been received.
+    EXPECT_EQ("eth0", resp->getIface());
+    EXPECT_EQ(1, resp->getIndex());
+
     // We also want to test the case when the server has capability to
     // respond directly to the client which is not configured. Server
     // makes decision whether it responds directly or broadcast its
-    // response based on the capability reported by IfaceMgr. In order
-    // to set this capability we have to provide a dummy Packet Filter
-    // class which would report the support for direct responses.
-    // This class is called PktFilterTest.
-    IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterTest()));
+    // response based on the capability reported by IfaceMgr. We can
+    // control whether the current packet filter returns that it supports
+    // direct responses or not.
+    current_pkt_filter_->direct_resp_supported_ = true;
 
     // Now we expect that the server will send its response to the
     // address assigned for the client.
-    ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+    ASSERT_NO_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp));
 
     EXPECT_EQ("192.0.2.13", resp->getRemoteAddr().toText());
 }
@@ -211,9 +278,7 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressSelect) {
 // query. Client sets this flag to indicate that it can't receive direct
 // responses from the server when it doesn't have its interface configured.
 // Server must respect broadcast flag.
-TEST_F(Dhcpv4SrvTest, adjustRemoteAddressBroadcast) {
-    boost::scoped_ptr<NakedDhcpv4Srv> srv(new NakedDhcpv4Srv(0));
-
+TEST_F(Dhcpv4SrvFakeIfaceTest, adjustIfaceDataBroadcast) {
     // Create instance of the incoming packet.
     boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
 
@@ -221,6 +286,13 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressBroadcast) {
     req->setGiaddr(IOAddress("0.0.0.0"));
     // Clear client address as it hasn't got any address configured yet.
     req->setCiaddr(IOAddress("0.0.0.0"));
+    // The query is sent to the broadcast address in the Select state.
+    req->setLocalAddr(IOAddress("255.255.255.255"));
+    // The query has been received on the DHCPv4 server port 67.
+    req->setLocalPort(DHCP4_SERVER_PORT);
+    // Set the interface. The response should be sent via the same interface.
+    req->setIface("eth0");
+    req->setIndex(1);
 
     // Let's set the broadcast flag.
     req->setFlags(Pkt4::FLAG_BROADCAST_MASK);
@@ -233,25 +305,51 @@ TEST_F(Dhcpv4SrvTest, adjustRemoteAddressBroadcast) {
     // Clear the remote address.
     resp->setRemoteAddr(IOAddress("0.0.0.0"));
 
-    // When running unit tests, the IfaceMgr is using the default Packet
-    // Filtering class, PktFilterInet. This class does not support direct
-    // responses to the clients without address assigned. If giaddr and
-    // ciaddr are zero and client has just got the new lease, the assigned
-    // address is carried in yiaddr. In order to send this address to the
-    // client, server must send the response to the broadcast address when
-    // direct response is not supported. This conflicts with the purpose
-    // of this test which is supposed to verify that responses are sent
-    // to broadcast address only, when broadcast flag is set. Therefore,
-    // in order to simulate that direct responses are supported we have
-    // to replace the default packet filtering class with a dummy class
-    // which reports direct response capability.
-    IfaceMgr::instance().setPacketFilter(PktFilterPtr(new PktFilterTest()));
-
-    ASSERT_NO_THROW(srv->adjustRemoteAddr(req, resp));
+    ASSERT_NO_THROW(NakedDhcpv4Srv::adjustIfaceData(req, resp));
 
     // Server must repond to broadcast address when client desired that
     // by setting the broadcast flag in its request.
     EXPECT_EQ("255.255.255.255", resp->getRemoteAddr().toText());
+
+    // Although the query has been sent to the broadcast address, the
+    // server should select a unicast address on the particular interface
+    // as a source address for the response.
+    EXPECT_EQ("192.0.3.1", resp->getLocalAddr().toText());
+
+    // The response should be sent from the DHCPv4 server port.
+    EXPECT_EQ(DHCP4_SERVER_PORT, resp->getLocalPort());
+
+    // The response should be sent via the same interface through which
+    // query has been received.
+    EXPECT_EQ("eth0", resp->getIface());
+    EXPECT_EQ(1, resp->getIndex());
+
+}
+
+// This test verifies that the server identifier option is appended to
+// a specified DHCPv4 message and the server identifier is correct.
+TEST_F(Dhcpv4SrvTest, appendServerID) {
+    Pkt4Ptr response(new Pkt4(DHCPDISCOVER, 1234));
+    // Set a local address. It is required by the function under test
+    // to create the Server Identifier option.
+    response->setLocalAddr(IOAddress("192.0.3.1"));
+
+    // Append the Server Identifier.
+    ASSERT_NO_THROW(NakedDhcpv4Srv::appendServerID(response));
+
+    // Make sure that the option has been added.
+    OptionPtr opt = response->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+    ASSERT_TRUE(opt);
+    Option4AddrLstPtr opt_server_id =
+        boost::dynamic_pointer_cast<Option4AddrLst>(opt);
+    ASSERT_TRUE(opt_server_id);
+
+    // The option is represented as a list of IPv4 addresses but with
+    // only one address added.
+    Option4AddrLst::AddressContainer addrs = opt_server_id->getAddresses();
+    ASSERT_EQ(1, addrs.size());
+    // This address should match the local address of the packet.
+    EXPECT_EQ("192.0.3.1", addrs[0].toText());
 }
 
 // Sanity check. Verifies that both Dhcpv4Srv and its derived
@@ -856,7 +954,7 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, RenewBasic) {
 // @todo: Implement tests for rejecting renewals
 
 // This test verifies if the sanityCheck() really checks options presence.
-TEST_F(Dhcpv4SrvFakeIfaceTest, sanityCheck) {
+TEST_F(Dhcpv4SrvTest, sanityCheck) {
     boost::scoped_ptr<NakedDhcpv4Srv> srv;
     ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
 

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -399,7 +399,7 @@ void Dhcpv4SrvTest::TearDown() {
 }
 
 Dhcpv4SrvFakeIfaceTest::Dhcpv4SrvFakeIfaceTest()
-: Dhcpv4SrvTest() {
+: Dhcpv4SrvTest(), current_pkt_filter_() {
     // Remove current interface configuration. Instead we want to add
     // a couple of fake interfaces.
     IfaceMgr& ifacemgr = IfaceMgr::instance();
@@ -414,7 +414,8 @@ Dhcpv4SrvFakeIfaceTest::Dhcpv4SrvFakeIfaceTest()
     // 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()));
+    current_pkt_filter_.reset(new PktFilterTest());
+    ifacemgr.setPacketFilter(current_pkt_filter_);
     ifacemgr.openSockets4();
 }
 

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -47,11 +47,18 @@ namespace test {
 class PktFilterTest : public PktFilter {
 public:
 
+    /// @brief Constructor.
+    ///
+    /// Sets the 'direct response' capability to true.
+    PktFilterTest()
+        : direct_resp_supported_(true) {
+    }
+
     /// @brief Reports 'direct response' capability.
     ///
     /// @return always true.
     virtual bool isDirectResponseSupported() const {
-        return (true);
+        return (direct_resp_supported_);
     }
 
     /// Does nothing.
@@ -71,6 +78,10 @@ public:
         return (0);
     }
 
+    /// @brief Holds a boolean value which indicates whether direct response
+    /// capability is supported (true) or not (false).
+    bool direct_resp_supported_;
+
 };
 
 typedef boost::shared_ptr<PktFilterTest> PktFilterTestPtr;
@@ -311,6 +322,10 @@ public:
     /// @param msg_type DHCPDISCOVER or DHCPREQUEST
     void testDiscoverRequest(const uint8_t msg_type);
 
+    /// @brief Holds a pointer to the packet filter object currently used
+    /// by the IfaceMgr.
+    PktFilterTestPtr current_pkt_filter_;
+
 };