Browse Source

[3947] Improved status code placement for address allocation.

Marcin Siodelski 9 years ago
parent
commit
89993d59a9

+ 24 - 22
src/bin/dhcp6/dhcp6_srv.cc

@@ -1743,10 +1743,6 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
         ctx.hints_.push_back(make_pair(iaaddr->getAddress(), 128));
     }
 
-    // We need to remember it as we'll be removing hints from this list as
-    // we extend, cancel or otherwise deal with the leases.
-    bool hints_present = !ctx.hints_.empty();
-
     Lease6Collection leases = alloc_engine_->renewLeases6(ctx);
 
     // Ok, now we have the leases extended. We have:
@@ -1805,26 +1801,35 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
 
     // All is left is to insert the status code.
     if (leases.empty()) {
-        // We did not assign anything. If client has sent something, then
-        // the status code is NoBinding, if he sent an empty IA_NA, then it's
-        // NoAddrsAvailable
-        if (hints_present) {
-            // Insert status code NoBinding to indicate that the lease does not
-            // exist for this client.
-            ia_rsp->addOption(createStatusCode(*query, *ia_rsp, STATUS_NoBinding,
-                              "Sorry, no known leases for this duid/iaid/subnet."));
 
+        // We did not assign any address to the client. Depending on whether the
+        // server is configured to allocate new leases during the Renew or
+        // Rebind we will have to send a different status code. If the server
+        // is configured to allocate new leases for the Renew and Rebind, the
+        // status code will be NoAddressAvail. If the server is not configured
+        // to allocate prefixes for the renewing client, the status code will
+        // be NoBinding, or perhaps the message will be dropped if this is the
+        // Rebind case.
+        if (!subnet->getAllocLeasesOnRenew()) {
+            ia_rsp->addOption(createStatusCode(*query, *ia_rsp,
+                                               STATUS_NoBinding,
+                                               "Sorry, no known NA leases for"
+                                               " this duid/iaid/subnet."));
             LOG_DEBUG(lease_logger, DBG_DHCP6_DETAIL, DHCP6_EXTEND_NA_UNKNOWN)
                 .arg(query->getLabel())
                 .arg(ia->getIAID())
                 .arg(subnet->toText());
+
+
         } else {
-            ia_rsp->addOption(createStatusCode(*query, *ia_rsp, STATUS_NoAddrsAvail,
-                              "Sorry, no addresses could be assigned at this time."));
+            // The server is configured to allocate new leases for the
+            // renewing client, but it could not allocate anything at this
+            // time. The status code should be NoAddrsAvail, per RFC7550.
+            ia_rsp->addOption(createStatusCode(*query, *ia_rsp,
+                                               STATUS_NoAddrsAvail,
+                                               "Sorry, no addresses could be"
+                                               " assigned at this time."));
         }
-    } else {
-        // Yay, the client still has something. For now, let's not insert
-        // status-code=success to conserve bandwidth.
     }
 
     return (ia_rsp);
@@ -1945,7 +1950,7 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
     // already, inform the client that he can't have them.
     for (AllocEngine::HintContainer::const_iterator prefix = ctx.hints_.begin();
          prefix != ctx.hints_.end(); ++prefix) {
-        // Send the prefix hint with the zero lifetimes only if the prefix
+        // Send the prefix with the zero lifetimes only if the prefix
         // contains non-zero value. A zero value indicates that the hint was
         // for the prefix length.
         if (!prefix->first.isV6Zero()) {
@@ -1957,11 +1962,8 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
 
     // All is left is to insert the status code.
     if (leases.empty()) {
-        // We did not assign anything. If client has sent something, then
-        // the status code is NoBinding, if he sent an empty IA_PD, then it's
-        // NoAddrsAvailable
 
-        // We did not assign any prefix to the client. Dependin on whether the
+        // We did not assign any prefix to the client. Depending on whether the
         // server is configured to allocate new leases during the Renew or
         // Rebind we will have to send a different status code. If the server
         // is configured to allocate new leases for the Renew and Rebind, the

+ 14 - 1
src/bin/dhcp6/tests/dhcp6_client.cc

@@ -92,6 +92,7 @@ Dhcp6Client::Dhcp6Client() :
     use_oro_(false),
     use_client_id_(true),
     use_rapid_commit_(false),
+    address_hint_(),
     prefix_hint_(),
     fqdn_() {
 }
@@ -110,6 +111,7 @@ Dhcp6Client::Dhcp6Client(boost::shared_ptr<NakedDhcpv6Srv>& srv) :
     use_oro_(false),
     use_client_id_(true),
     use_rapid_commit_(false),
+    address_hint_(),
     prefix_hint_(),
     fqdn_() {
 }
@@ -278,7 +280,10 @@ Dhcp6Client::conditionallyAppendRequestedIA(const Pkt6Ptr& query,
     // Add prefix hint if specified.
     if (prefix_hint_ && (ia_type == D6O_IA_PD)) {
         requested_ia->addOption(prefix_hint_);
-    }
+
+    } else if (address_hint_ && (ia_type == D6O_IA_NA)) {
+        requested_ia->addOption(address_hint_);
+    } 
 
     query->addOption(requested_ia);
 }
@@ -680,6 +685,14 @@ Dhcp6Client::sendMsg(const Pkt6Ptr& msg) {
 
 void
 Dhcp6Client::useHint(const uint32_t pref_lft, const uint32_t valid_lft,
+                     const std::string& address) {
+    address_hint_.reset(new Option6IAAddr(D6O_IAADDR,
+                                          asiolink::IOAddress(address),
+                                          pref_lft, valid_lft));
+}
+
+void
+Dhcp6Client::useHint(const uint32_t pref_lft, const uint32_t valid_lft,
                      const uint8_t len, const std::string& prefix) {
     prefix_hint_.reset(new Option6IAPrefix(D6O_IAPREFIX,
                                            asiolink::IOAddress(prefix),

+ 11 - 0
src/bin/dhcp6/tests/dhcp6_client.h

@@ -415,6 +415,14 @@ public:
         iface_name_ = iface_name;
     }
 
+    /// @brief Set an address hint to be sent to a server.
+    ///
+    /// @param pref_lft Preferred lifetime.
+    /// @param valid_lft Valid lifetime.
+    /// @param address Address for which the client has a preference.
+    void useHint(const uint32_t pref_lft, const uint32_t valid_lft,
+                 const std::string& address);
+
     /// @brief Sets a prefix hint to be sent to a server.
     ///
     /// @param pref_lft Preferred lifetime.
@@ -681,6 +689,9 @@ private:
     bool use_client_id_;
     bool use_rapid_commit_;
 
+    /// @brief Pointer to the option holding an address hint.
+    Option6IAAddrPtr address_hint_;
+
     /// @brief Pointer to the option holding a prefix hint.
     Option6IAPrefixPtr prefix_hint_;
 

+ 67 - 0
src/bin/dhcp6/tests/renew_unittest.cc

@@ -267,6 +267,73 @@ TEST_F(RenewTest, requestAddressInRenew) {
     ASSERT_EQ(0, leases_client_na.size());
     ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
 
+    // Send Renew message to the server, including IA_PD and requesting IA_NA.
+    // The server should return NoAddrsAvail status code in this case.
+    ASSERT_NO_THROW(client.doRenew());
+    leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(0, leases_client_na.size());
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+
+    // Reconfigure the server to use both NA and PD pools.
+    configure(RENEW_CONFIGS[2], *client.getServer());
+
+    // Send Renew message to the server, including IA_PD and requesting IA_NA.
+    ASSERT_NO_THROW(client.doRenew());
+
+    // Make sure that the client has renewed PD lease.
+    std::vector<Lease6> leases_client_pd_renewed =
+        client.getLeasesByType(Lease::TYPE_PD);
+    ASSERT_EQ(1, leases_client_pd_renewed.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+    EXPECT_GE(leases_client_pd_renewed[0].cltt_ - leases_client_pd[0].cltt_, 1000);
+
+    // The client should now also acquire a NA lease.
+    leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(1, leases_client_na.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(1234));
+}
+
+// This test verifies that the client can request address assignment
+// while it is renewing an address lease, with a hint.
+TEST_F(RenewTest, requestAddressInRenewHint) {
+    Dhcp6Client client;
+
+    // Configure client to request IA_NA and IA_PD.
+    client.useNA();
+    client.usePD();
+
+    // Configure the server with PD pools only.
+    ASSERT_NO_THROW(configure(RENEW_CONFIGS[1], *client.getServer()));
+
+    // Perform 4-way exchange.
+    ASSERT_NO_THROW(client.doSARR());
+
+    // Simulate aging of leases.
+    client.fastFwdTime(1000);
+
+    // Make sure that the client has acquired PD lease.
+    std::vector<Lease6> leases_client_pd = client.getLeasesByType(Lease::TYPE_PD);
+    ASSERT_EQ(1, leases_client_pd.size());
+    EXPECT_EQ(STATUS_Success, client.getStatusCode(5678));
+
+    // The client should not acquire a NA lease.
+    std::vector<Lease6> leases_client_na =
+        client.getLeasesByType(Lease::TYPE_NA);
+    ASSERT_EQ(0, leases_client_na.size());
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+
+    client.useHint(0, 0, "2001:db8:1::100");
+
+    // Send Renew message to the server, including IA_PD and requesting IA_NA.
+    // The server should return NoAddrsAvail status code in this case.
+    ASSERT_NO_THROW(client.doRenew());
+    leases_client_na = client.getLeasesByType(Lease::TYPE_NA);
+    // The server should return the hint with the zero lifetimes.
+    ASSERT_EQ(1, leases_client_na.size());
+    EXPECT_EQ(0, leases_client_na[0].preferred_lft_);
+    EXPECT_EQ(0, leases_client_na[0].valid_lft_);
+    ASSERT_EQ(STATUS_NoAddrsAvail, client.getStatusCode(1234));
+
     // Reconfigure the server to use both NA and PD pools.
     configure(RENEW_CONFIGS[2], *client.getServer());