Browse Source

[4552] DHCPv4 server assigns reserved siaddr, sname and file field.

Marcin Siodelski 8 years ago
parent
commit
e3415ecc07

+ 36 - 9
src/bin/dhcp4/dhcp4_srv.cc

@@ -153,6 +153,9 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine,
 
             // Check for static reservations.
             alloc_engine->findReservation(*context_);
+
+            // Set siaddr, sname and file.
+            setReservedMessageFields();
         }
     }
 };
@@ -333,6 +336,27 @@ Dhcpv4Exchange::setHostIdentifiers() {
     }
 }
 
+void
+Dhcpv4Exchange::setReservedMessageFields() {
+    ConstHostPtr host = context_->host_;
+    // Nothing to do if host reservations not specified for this client.
+    if (host) {
+        if (!host->getNextServer().isV4Zero()) {
+            resp_->setSiaddr(host->getNextServer());
+        }
+
+        if (!host->getServerHostname().empty()) {
+            resp_->setSname(reinterpret_cast<
+                            const uint8_t*>(host->getServerHostname().c_str()));
+        }
+
+        if (!host->getBootFileName().empty()) {
+            resp_->setFile(reinterpret_cast<
+                           const uint8_t*>(host->getBootFileName().c_str()));
+        }
+    }
+}
+
 const std::string Dhcpv4Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast,
@@ -1515,12 +1539,6 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
         return;
     }
 
-    // Set up siaddr. Perhaps assignLease is not the best place to call this
-    // as siaddr has nothing to do with a lease, but otherwise we would have
-    // to select subnet twice (performance hit) or update too many functions
-    // at once.
-    /// @todo: move subnet selection to a common code
-    resp->setSiaddr(subnet->getSiaddr());
 
     // Get the server identifier. It will be used to determine the state
     // of the client.
@@ -2635,9 +2653,12 @@ Dhcpv4Srv::vendorClassSpecificProcessing(const Dhcpv4Exchange& ex) {
 
     if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_MODEM)) {
 
-        // Set next-server. This is TFTP server address. Cable modems will
-        // download their configuration from that server.
-        rsp->setSiaddr(subnet->getSiaddr());
+        // Do not override
+        if (rsp->getSiaddr().isV4Zero()) {
+            // Set next-server. This is TFTP server address. Cable modems will
+            // download their configuration from that server.
+            rsp->setSiaddr(subnet->getSiaddr());
+        }
 
         // Now try to set up file field in DHCPv4 packet. We will just copy
         // content of the boot-file option, which contains the same information.
@@ -2664,6 +2685,12 @@ Dhcpv4Srv::vendorClassSpecificProcessing(const Dhcpv4Exchange& ex) {
         rsp->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS());
     }
 
+    // Set up siaddr. Do not override siaddr if host specific value or
+    // vendor class specific value present.
+    if (rsp->getSiaddr().isV4Zero()) {
+        rsp->setSiaddr(subnet->getSiaddr());
+    }
+
     return (true);
 }
 

+ 4 - 0
src/bin/dhcp4/dhcp4_srv.h

@@ -150,6 +150,10 @@ private:
     /// host-reservation-identifiers
     void setHostIdentifiers();
 
+    /// @brief Sets reserved values of siaddr, sname and file in the
+    /// server's response.
+    void setReservedMessageFields();
+
     /// @brief Pointer to the allocation engine used by the server.
     AllocEnginePtr alloc_engine_;
     /// @brief Pointer to the DHCPv4 message sent by the client.

+ 15 - 1
src/bin/dhcp4/tests/dhcp4_client.cc

@@ -23,7 +23,7 @@ namespace test {
 
 Dhcp4Client::Configuration::Configuration()
     : routers_(), dns_servers_(), log_servers_(), quotes_servers_(),
-      serverid_("0.0.0.0") {
+      serverid_("0.0.0.0"), siaddr_(asiolink::IOAddress::IPV4_ZERO_ADDRESS()) {
     reset();
 }
 
@@ -34,6 +34,9 @@ Dhcp4Client::Configuration::reset() {
     log_servers_.clear();
     quotes_servers_.clear();
     serverid_ = asiolink::IOAddress("0.0.0.0");
+    siaddr_ = asiolink::IOAddress::IPV4_ZERO_ADDRESS();
+    sname_.clear();
+    boot_file_name_.clear();
     lease_ = Lease4();
 }
 
@@ -178,6 +181,17 @@ Dhcp4Client::applyConfiguration() {
     if (opt_vendor) {
         config_.vendor_suboptions_ = opt_vendor->getOptions();
     }
+    // siaddr
+    config_.siaddr_ = resp->getSiaddr();
+    // sname
+    OptionBuffer buf = resp->getSname();
+    // sname is a fixed length field holding null terminated string. Use
+    // of c_str() guarantess that only a useful portion (ending with null
+    // character) is assigned.
+    config_.sname_.assign(std::string(buf.begin(), buf.end()).c_str());
+    // (boot)file
+    buf = resp->getFile();
+    config_.boot_file_name_.assign(std::string(buf.begin(), buf.end()).c_str());
     // Server Identifier
     OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast<
         OptionCustom>(resp->getOption(DHO_DHCP_SERVER_IDENTIFIER));

+ 6 - 0
src/bin/dhcp4/tests/dhcp4_client.h

@@ -81,6 +81,12 @@ public:
         /// @brief Holds server id of the server which responded to the client's
         /// request.
         asiolink::IOAddress serverid_;
+        /// @brief Holds returned siaddr.
+        asiolink::IOAddress siaddr_;
+        /// @brief Holds returned sname.
+        std::string sname_;
+        /// @brief Holds returned (boot)file.
+        std::string boot_file_name_;
 
         /// @brief Constructor.
         Configuration();

+ 56 - 1
src/bin/dhcp4/tests/dora_unittest.cc

@@ -71,6 +71,15 @@ namespace {
 ///   - The same as configuration 4, but using the following order of
 ///     host-reservation-identifiers: duid, circuit-id, hw-address.
 ///
+/// - Configuration 6:
+///   - This configuration provides reservations for next-server,
+///     server-name and boot-file-name value.
+///   - 1 subnet: 10.0.0.0/24
+///   - 1 reservation for this subnet:
+///     - Client's HW address: aa:bb:cc:dd:ee:ff
+///     - next-server = 192.0.0.2
+///     - server name = "some-name.example.org"
+///     - boot-file-name = "bootfile.efi"
 const char* DORA_CONFIGS[] = {
 // Configuration 0
     "{ \"interfaces-config\": {"
@@ -233,7 +242,27 @@ const char* DORA_CONFIGS[] = {
         "       }"
         "    ]"
         "} ]"
-    "}"
+    "}",
+
+// Configuration 6
+    "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"valid-lifetime\": 600,"
+        "\"next-server\": \"10.0.0.1\","
+        "\"subnet4\": [ { "
+        "    \"subnet\": \"10.0.0.0/24\", "
+        "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
+        "    \"reservations\": [ "
+        "       {"
+        "         \"hw-address\": \"aa:bb:cc:dd:ee:ff\","
+        "         \"next-server\": \"10.0.0.7\","
+        "         \"server-name\": \"some-name.example.org\","
+        "         \"boot-file-name\": \"bootfile.efi\""
+        "       }"
+        "    ]"
+        "} ]"
+    "}",
 };
 
 /// @brief Test fixture class for testing 4-way (DORA) exchanges.
@@ -989,6 +1018,32 @@ TEST_F(DORATest, changingHWAddress) {
     EXPECT_FALSE(client.config_.lease_.client_id_);
 }
 
+// This test verifies that the server assigns reserved values for the
+// siaddr, sname and file fields carried within DHCPv4 message.
+TEST_F(DORATest, messageFieldsReservations) {
+    // Client has a reservation.
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Set explicit HW address so as it matches the reservation in the
+    // configuration used below.
+    client.setHWAddress("aa:bb:cc:dd:ee:ff");
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[6], *client.getServer());
+    // Client A performs 4-way exchange and should obtain a reserved
+    // address.
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+
+    // Check that the reserved values have been assigned.
+    EXPECT_EQ("10.0.0.7", client.config_.siaddr_.toText());
+    EXPECT_EQ("some-name.example.org", client.config_.sname_);
+    EXPECT_EQ("bootfile.efi", client.config_.boot_file_name_);
+}
+
 // This test checks the following scenario:
 // 1. Client A performs 4-way exchange and obtains a lease from the dynamic pool.
 // 2. Reservation is created for the client A using an address out of the dynamic