From: Marcin Siodelski Date: Sun, 19 Jul 2015 14:42:16 +0000 (+0200) Subject: [3947] Improved status code placement for address allocation. X-Git-Tag: trac3238_base~6^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=89993d59a9232102cdc9b7075bf9f83efa950bf8;p=thirdparty%2Fkea.git [3947] Improved status code placement for address allocation. --- diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 77c294a42f..2f214ee09a 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/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 diff --git a/src/bin/dhcp6/tests/dhcp6_client.cc b/src/bin/dhcp6/tests/dhcp6_client.cc index 918af76a59..442689ded6 100644 --- a/src/bin/dhcp6/tests/dhcp6_client.cc +++ b/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& 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); } @@ -678,6 +683,14 @@ Dhcp6Client::sendMsg(const Pkt6Ptr& msg) { srv_->run(); } +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) { diff --git a/src/bin/dhcp6/tests/dhcp6_client.h b/src/bin/dhcp6/tests/dhcp6_client.h index 722afaa778..68dffe0dec 100644 --- a/src/bin/dhcp6/tests/dhcp6_client.h +++ b/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_; diff --git a/src/bin/dhcp6/tests/renew_unittest.cc b/src/bin/dhcp6/tests/renew_unittest.cc index 329e0983db..cef2b7ef2c 100644 --- a/src/bin/dhcp6/tests/renew_unittest.cc +++ b/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 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 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 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());