[3390] Addressed review comments.

doc/guide/dhcp4-srv.xml

@@ -1078,8 +1078,8 @@ temporarily override a list of interface names and listen on all interfaces.
         {
             "subnet": "192.0.2.0/24"
             "option-data": [ {"
-                "name": "routers",
-                "code": 3,
+                "name": "domain-name-servers",
+                "code": 6,
                 "data": "192.0.2.200,192.0.2.201",
                 "csv-format": true,
                 "space": "dhcp4"
@@ -1760,8 +1760,8 @@ temporarily override a list of interface names and listen on all interfaces.
       <para>The following standards and draft standards are currently supported:</para>
       <itemizedlist>
           <listitem>
-            <simpara><ulink url="http://tools.ietf.org/html/rfc2131">RFC 2131</ulink>: Supported messages are DISCOVER, OFFER,
-            REQUEST, RELEASE, INFORM, ACK, and NAK.</simpara>
+            <simpara><ulink url="http://tools.ietf.org/html/rfc2131">RFC 2131</ulink>: Supported messages are DISCOVER (1), OFFER (2),
+            REQUEST (3), RELEASE (7), INFORM (8), ACK (5), and NAK(6).</simpara>
           </listitem>
           <listitem>
             <simpara><ulink url="http://tools.ietf.org/html/rfc2132">RFC 2132</ulink>:

src/bin/dhcp4/dhcp4_srv.cc

@@ -1164,6 +1164,13 @@ Dhcpv4Srv::adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response) {
 
     // If received relayed message, server responds to the relay address.
     if (question->isRelayed()) {
+        // The client should set the ciaddr when sending the DHCPINFORM
+        // but in case he didn't, the relay may not be able to determine the
+        // address of the client, because yiaddr is not set when responding
+        // to Confirm and the only address available was the source address
+        // of the client. The source address is however not used here because
+        // the message is relayed. Therefore, we set the BROADCAST flag so
+        // as the relay can broadcast the packet.
         if ((question->getType() == DHCPINFORM) &&
             (question->getCiaddr() == zero_addr)) {
             response->setFlags(BOOTP_BROADCAST);
@@ -1418,6 +1425,9 @@ Dhcpv4Srv::processDecline(Pkt4Ptr& /* decline */) {
 
 Pkt4Ptr
 Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
+    // DHCPINFORM MUST not include server identifier.
+    sanityCheck(inform, FORBIDDEN);
+
     Pkt4Ptr ack = Pkt4Ptr(new Pkt4(DHCPACK, inform->getTransid()));
     copyDefaultFields(inform, ack);
     appendRequestedOptions(inform, ack);

src/bin/dhcp4/tests/dhcp4_client.cc

@@ -17,6 +17,7 @@
 #include <dhcp/option_int_array.h>
 #include <dhcpsrv/lease.h>
 #include <dhcp4/tests/dhcp4_client.h>
+#include <util/range_utilities.h>
 #include <boost/pointer_cast.hpp>
 #include <cstdlib>
 
@@ -165,11 +166,9 @@ Dhcp4Client::generateHWAddr(const uint8_t htype) const {
                   "The harware address type " << static_cast<int>(htype)
                   << " is currently not supported");
     }
-    std::vector<uint8_t> hwaddr;
+    std::vector<uint8_t> hwaddr(HWAddr::ETHERNET_HWADDR_LEN);
     // Generate ethernet hardware address by assigning random byte values.
-    for (int i = 0; i < HWAddr::ETHERNET_HWADDR_LEN; ++i) {
-        hwaddr.push_back(static_cast<uint8_t>(rand() % 255));
-    }
+    isc::util::fillRandom(hwaddr.begin(), hwaddr.end());
     return (HWAddrPtr(new HWAddr(hwaddr, htype)));
 }
 

src/bin/dhcp4/tests/dhcp4_client.h

@@ -57,15 +57,24 @@ public:
     /// @brief Holds the configuration of the client received from the
     /// DHCP server.
     struct Configuration {
+        /// @brief Holds IP addresses received in the Routers option.
         Option4AddrLst::AddressContainer routers_;
+        /// @brief Holds IP addresses received in the DNS Servers option.
         Option4AddrLst::AddressContainer dns_servers_;
+        /// @brief Holds IP addresses received in the Log Servers option.
         Option4AddrLst::AddressContainer log_servers_;
+        /// @brief Holds IP addresses received in the Quotes Servers option.
         Option4AddrLst::AddressContainer quotes_servers_;
+        /// @brief Holds a lease obtained by the client.
         Lease4 lease_;
+        /// @brief Holds server id of the server which responded to the client's
+        /// request.
         asiolink::IOAddress serverid_;
 
+        /// @brief Constructor.
         Configuration();
 
+        /// @brief Sets configuration values to defaults.
         void reset();
     };
 
@@ -95,7 +104,12 @@ public:
     /// @brief Sends DHCPINFORM message to the server and receives response.
     ///
     /// This function simulates sending the DHCPINFORM message to the server
-    /// and receiving server's response (if any).
+    /// and receiving server's response (if any). The server's response and the
+    /// message sent to the server is stored in the context structure and can
+    /// be accessed using @c getContext function.
+    ///
+    /// The configuration returned by the server is stored in the
+    /// @c config_ public member and can be accessed directly.
     ///
     /// @param set_ciaddr Indicates if the ciaddr should be set for an
     /// outgoing message and defaults to true. Note, that the RFC2131 mandates
@@ -186,6 +200,7 @@ public:
     /// @param use Parameter which 'true' value indicates that client should
     /// simulate sending messages via relay.
     /// @param relay_addr Relay address
+    /// @param sf_relay_addr Server facing relay address.
     void useRelay(const bool use = true,
                   const asiolink::IOAddress& relay_addr =
                   asiolink::IOAddress("192.0.2.2"),

src/bin/dhcp4/tests/inform_unittest.cc

@@ -149,9 +149,10 @@ TEST_F(InformTest, directClientBroadcast) {
     client.requestOptions(DHO_LOG_SERVERS, DHO_COOKIE_SERVERS);
     // Preconfigure the client with the IP address.
     client.createLease(IOAddress("10.0.0.56"), 600);
+
     // Send DHCPINFORM message to the server.
-    client.doInform();
-    //    ASSERT_NO_THROW(client.doInform());
+    ASSERT_NO_THROW(client.doInform());
+
     // Make sure that the server responded.
     ASSERT_TRUE(client.getContext().response_);
     Pkt4Ptr resp = client.getContext().response_;
@@ -163,6 +164,7 @@ TEST_F(InformTest, directClientBroadcast) {
     EXPECT_FALSE(resp->isRelayed());
     // Make sure that the server id is present.
     EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText());
+
     // Make sure that the Routers option has been received.
     ASSERT_EQ(2, client.config_.routers_.size());
     EXPECT_EQ("10.0.0.200", client.config_.routers_[0].toText());
@@ -186,16 +188,20 @@ TEST_F(InformTest, directClientBroadcast) {
     // This time do not request Quotes Servers option and it should not
     // be returned.
     client.requestOptions(DHO_LOG_SERVERS);
+
     // Send DHCPINFORM.
     ASSERT_NO_THROW(client.doInform());
-    ASSERT_TRUE(client.getContext().response_);
+
+    // Make sure that the server responded.
     resp = client.getContext().response_;
+    ASSERT_TRUE(resp);
     // Make sure that the server has responded with DHCPACK.
     ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
     // Response should have been unicast to the ciaddr.
     EXPECT_EQ(IOAddress("10.0.0.12"), resp->getLocalAddr());
     // Response must not be relayed.
     EXPECT_FALSE(resp->isRelayed());
+
     // Make sure that the server id is present.
     EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText());
     // Make sure that the Routers option has been received.