From: Thomas Markwalder Date: Thu, 20 Feb 2014 20:25:06 +0000 (-0500) Subject: [3222] b10-dhcp6 now sends NCRs to b10-dhcp-ddns X-Git-Tag: bind10-1.2.0beta1-release~19^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e6d57d893c8abb993158ba3621e087e6bd0d1b30;p=thirdparty%2Fkea.git [3222] b10-dhcp6 now sends NCRs to b10-dhcp-ddns Replaced use of internal queue in Dhcpv6Srv with posting NCRs to D2ClientMgr. Upgraded unit tests accordingly. b10-dhcp6 is now fully able to participate in DDNS. --- diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 8879679130..9c9032fd8c 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -494,9 +494,6 @@ bool Dhcpv6Srv::run() { LOG_ERROR(dhcp6_logger, DHCP6_PACKET_SEND_FAIL) .arg(e.what()); } - - // Send NameChangeRequests to the b10-dhcp-ddns module. - sendNameChangeRequests(); } } @@ -1077,18 +1074,18 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer) { // Get the IP address from the lease. Also, use the S flag to determine // if forward change should be performed. This flag will always be // set if server has taken responsibility for the forward update. - NameChangeRequest ncr(isc::dhcp_ddns::CHG_ADD, - opt_fqdn->getFlag(Option6ClientFqdn::FLAG_S), - true, opt_fqdn->getDomainName(), - iaaddr->getAddress().toText(), - dhcid, 0, iaaddr->getValid()); - // Add the request to the queue. This queue will be read elsewhere in - // the code and all requests from this queue will be sent to the - // D2 module. - name_change_reqs_.push(ncr); + NameChangeRequestPtr ncr; + ncr.reset(new NameChangeRequest(isc::dhcp_ddns::CHG_ADD, + opt_fqdn->getFlag(Option6ClientFqdn:: + FLAG_S), + true, opt_fqdn->getDomainName(), + iaaddr->getAddress().toText(), + dhcid, 0, iaaddr->getValid())); + // Post the NCR to the D2ClientMgr. + CfgMgr::instance().getD2ClientMgr().sendRequest(ncr); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, - DHCP6_DDNS_CREATE_ADD_NAME_CHANGE_REQUEST).arg(ncr.toText()); + DHCP6_DDNS_CREATE_ADD_NAME_CHANGE_REQUEST).arg(ncr->toText()); /// @todo Currently we create NCR with the first IPv6 address that /// is carried in one of the IA_NAs. In the future, the NCR API should @@ -1138,31 +1135,20 @@ Dhcpv6Srv::createRemovalNameChangeRequest(const Lease6Ptr& lease) { } isc::dhcp_ddns::D2Dhcid dhcid(*lease->duid_, hostname_wire); - // Create a NameChangeRequest to remove the entry. - NameChangeRequest ncr(isc::dhcp_ddns::CHG_REMOVE, - lease->fqdn_fwd_, lease->fqdn_rev_, - lease->hostname_, - lease->addr_.toText(), - dhcid, 0, lease->valid_lft_); - name_change_reqs_.push(ncr); + NameChangeRequestPtr ncr; + ncr.reset(new NameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, + lease->fqdn_fwd_, lease->fqdn_rev_, + lease->hostname_, + lease->addr_.toText(), + dhcid, 0, lease->valid_lft_)); + CfgMgr::instance().getD2ClientMgr().sendRequest(ncr); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, - DHCP6_DDNS_CREATE_REMOVE_NAME_CHANGE_REQUEST).arg(ncr.toText()); + DHCP6_DDNS_CREATE_REMOVE_NAME_CHANGE_REQUEST).arg(ncr->toText()); } -void -Dhcpv6Srv::sendNameChangeRequests() { - while (!name_change_reqs_.empty()) { - // @todo Once next NameChangeRequest is picked from the queue - // we should send it to the b10-dhcp_ddns module. Currently we - // just drop it. - name_change_reqs_.pop(); - } -} - - OptionPtr Dhcpv6Srv::assignIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid, const Pkt6Ptr& query, const Pkt6Ptr& answer, diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index 91dbd8bc9d..e0d4791996 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -455,17 +455,6 @@ protected: /// records will be performed. void createRemovalNameChangeRequest(const Lease6Ptr& lease); - /// @brief Sends all outstanding NameChangeRequests to bind10-d2 module. - /// - /// The purpose of this function is to pick all outstanding - /// NameChangeRequests from the FIFO queue and send them to b10-dhcp-ddns - /// module. - /// - /// @todo Currently this function simply removes all requests from the - /// queue but doesn't send them anywhere. In the future, the - /// NameChangeSender will be used to deliver requests to the other module. - void sendNameChangeRequests(); - /// @brief Attempts to renew received addresses /// /// It iterates through received IA_NA options and attempts to renew diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc index 7347958976..c11370bf90 100644 --- a/src/bin/dhcp6/tests/fqdn_unittest.cc +++ b/src/bin/dhcp6/tests/fqdn_unittest.cc @@ -43,6 +43,9 @@ namespace { /// @brief A test fixture class for testing DHCPv6 Client FQDN Option handling. class FqdnDhcpv6SrvTest : public Dhcpv6SrvTest { public: + /// Pointer to Dhcpv6Srv that is used in tests + boost::scoped_ptr srv_; + // Reference to D2ClientMgr singleton D2ClientMgr& d2_mgr_; @@ -55,7 +58,8 @@ public: /// @brief Constructor FqdnDhcpv6SrvTest() - : Dhcpv6SrvTest(), d2_mgr_(CfgMgr::instance().getD2ClientMgr()) { + : Dhcpv6SrvTest(), srv_(new NakedDhcpv6Srv(0)), + d2_mgr_(CfgMgr::instance().getD2ClientMgr()) { // generateClientId assigns DUID to duid_. generateClientId(); lease_.reset(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1"), @@ -100,6 +104,7 @@ public: (mask & REPLACE_CLIENT_NAME), "myhost", "example.com"))); ASSERT_NO_THROW(CfgMgr::instance().setD2ClientConfig(cfg)); + ASSERT_NO_THROW(srv_->startD2()); } /// @brief Construct the DHCPv6 Client FQDN option using flags and @@ -285,7 +290,9 @@ public: const Option6ClientFqdn::DomainNameType in_domain_type, const uint8_t exp_flags, const std::string& exp_domain_name) { - NakedDhcpv6Srv srv(0); + +// NakedDhcpv6Srv srv(0); + Pkt6Ptr question = generateMessage(msg_type, in_flags, in_domain_name, @@ -295,7 +302,7 @@ public: Pkt6Ptr answer(new Pkt6(msg_type == DHCPV6_SOLICIT ? DHCPV6_ADVERTISE : DHCPV6_REPLY, question->getTransid())); - ASSERT_NO_THROW(srv.processClientFqdn(question, answer)); + ASSERT_NO_THROW(srv_->processClientFqdn(question, answer)); Option6ClientFqdnPtr answ_fqdn = boost::dynamic_pointer_cast< Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN)); ASSERT_TRUE(answ_fqdn); @@ -437,16 +444,21 @@ public: const std::string& dhcid, const uint16_t expires, const uint16_t len) { - NameChangeRequest ncr = srv.name_change_reqs_.front(); - EXPECT_EQ(type, ncr.getChangeType()); - EXPECT_EQ(forward, ncr.isForwardChange()); - EXPECT_EQ(reverse, ncr.isReverseChange()); - EXPECT_EQ(addr, ncr.getIpAddress()); - EXPECT_EQ(dhcid, ncr.getDhcid().toStr()); - EXPECT_EQ(expires, ncr.getLeaseExpiresOn()); - EXPECT_EQ(len, ncr.getLeaseLength()); - EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr.getStatus()); - srv.name_change_reqs_.pop(); + NameChangeRequestPtr ncr; + ASSERT_NO_THROW(ncr = d2_mgr_.peekAt(0)); + ASSERT_TRUE(ncr); + + EXPECT_EQ(type, ncr->getChangeType()); + EXPECT_EQ(forward, ncr->isForwardChange()); + EXPECT_EQ(reverse, ncr->isReverseChange()); + EXPECT_EQ(addr, ncr->getIpAddress()); + EXPECT_EQ(dhcid, ncr->getDhcid().toStr()); + EXPECT_EQ(expires, ncr->getLeaseExpiresOn()); + EXPECT_EQ(len, ncr->getLeaseLength()); + EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr->getStatus()); + + // Process the message off the queue + ASSERT_NO_THROW(d2_mgr_.runReadyIO()); } // Holds a lease used by a test. @@ -538,7 +550,7 @@ TEST_F(FqdnDhcpv6SrvTest, createNameChangeRequestsNoFQDN) { ASSERT_NO_THROW(srv.createNameChangeRequests(answer)); // There should be no new NameChangeRequests. - EXPECT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } // Test that NameChangeRequests are not generated if an answer message @@ -559,7 +571,7 @@ TEST_F(FqdnDhcpv6SrvTest, createNameChangeRequestsNoAddr) { // We didn't add any IAs, so there should be no NameChangeRequests in th // queue. - ASSERT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } // Test that exactly one NameChangeRequest is created as a result of processing @@ -585,7 +597,7 @@ TEST_F(FqdnDhcpv6SrvTest, createNameChangeRequests) { // Create NameChangeRequest for the first allocated address. ASSERT_NO_THROW(srv.createNameChangeRequests(answer)); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); // Verify that NameChangeRequest is correct. verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, @@ -618,9 +630,8 @@ TEST_F(FqdnDhcpv6SrvTest, noAddRequestsWhenDisabled) { Option6ClientFqdn::FULL); answer->addOption(fqdn); - // Create NameChangeRequest for the first allocated address. + // An attempt to send a NCR would throw. ASSERT_NO_THROW(srv.createNameChangeRequests(answer)); - ASSERT_TRUE(srv.name_change_reqs_.empty()); } @@ -638,8 +649,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestFwdRev) { ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_)); - ASSERT_EQ(1, srv.name_change_reqs_.size()); - + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_REMOVE, true, true, "2001:db8:1::1", "000201415AA33D1187D148275136FA30300478" @@ -663,9 +673,8 @@ TEST_F(FqdnDhcpv6SrvTest, noRemovalsWhenDisabled) { // as if we typed domain-name in lower case. lease_->hostname_ = "MYHOST.example.com."; + // When DDNS is disabled an attempt to send a request will throw. ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_)); - - ASSERT_TRUE(srv.name_change_reqs_.empty()); } @@ -680,7 +689,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestRev) { ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_)); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_REMOVE, true, false, "2001:db8:1::1", @@ -700,7 +709,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestNoUpdate) { ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_)); - EXPECT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } @@ -715,7 +724,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestNoHostname) { ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_)); - EXPECT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } @@ -731,7 +740,7 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestWrongHostname) { ASSERT_NO_THROW(srv.createRemovalNameChangeRequest(lease_)); - EXPECT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } @@ -744,7 +753,7 @@ TEST_F(FqdnDhcpv6SrvTest, processSolicit) { // response using processSolicit function. testProcessMessage(DHCPV6_SOLICIT, "myhost.example.com", "myhost.example.com.", srv); - EXPECT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } // Test that client may send two requests, each carrying FQDN option with @@ -760,7 +769,7 @@ TEST_F(FqdnDhcpv6SrvTest, processTwoRequests) { // to add both reverse and forward mapping to DNS. testProcessMessage(DHCPV6_REQUEST, "myhost.example.com", "myhost.example.com.", srv); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -776,7 +785,7 @@ TEST_F(FqdnDhcpv6SrvTest, processTwoRequests) { // remove the existing entries, one to add new entries. testProcessMessage(DHCPV6_REQUEST, "otherhost.example.com", "otherhost.example.com.", srv); - ASSERT_EQ(2, srv.name_change_reqs_.size()); + ASSERT_EQ(2, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_REMOVE, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -804,7 +813,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestSolicit) { // to add both reverse and forward mapping to DNS. testProcessMessage(DHCPV6_REQUEST, "myhost.example.com", "myhost.example.com.", srv); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -817,7 +826,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestSolicit) { // Request or Renew. testProcessMessage(DHCPV6_SOLICIT, "otherhost.example.com", "otherhost.example.com.", srv); - ASSERT_TRUE(srv.name_change_reqs_.empty()); + ASSERT_EQ(0, d2_mgr_.getQueueSize()); } @@ -836,7 +845,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenew) { // to add both reverse and forward mapping to DNS. testProcessMessage(DHCPV6_REQUEST, "myhost.example.com", "myhost.example.com.", srv); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -852,7 +861,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenew) { // remove the existing entries, one to add new entries. testProcessMessage(DHCPV6_RENEW, "otherhost.example.com", "otherhost.example.com.", srv); - ASSERT_EQ(2, srv.name_change_reqs_.size()); + ASSERT_EQ(2, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_REMOVE, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -875,7 +884,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) { // to add both reverse and forward mapping to DNS. testProcessMessage(DHCPV6_REQUEST, "myhost.example.com", "myhost.example.com.", srv); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -888,7 +897,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) { // remove DNS entries is generated. testProcessMessage(DHCPV6_RELEASE, "otherhost.example.com", "otherhost.example.com.", srv); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_REMOVE, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -907,7 +916,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestWithoutFqdn) { // in the server's response. The testProcessMessage will check that. testProcessMessage(DHCPV6_REQUEST, "myhost.example.com", "myhost.example.com.", srv, false); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -923,7 +932,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestEmptyFqdn) { testProcessMessage(DHCPV6_REQUEST, "", "myhost-2001-db8-1-1--dead-beef.example.com.", srv, false); - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201C905E54BE12DE6AF92ADE72752B9F362" @@ -953,7 +962,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) { testProcessMessage(DHCPV6_REQUEST, "myhost.example.com", "myhost.example.com.", srv); // Test that the appropriate NameChangeRequest has been generated. - ASSERT_EQ(1, srv.name_change_reqs_.size()); + ASSERT_EQ(1, d2_mgr_.getQueueSize()); verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_ADD, true, true, "2001:db8:1:1::dead:beef", "000201415AA33D1187D148275136FA30300478" @@ -985,7 +994,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) { // reuse this lease. testProcessMessage(DHCPV6_REQUEST, "myhost.example.com.", "myhost.example.com.", srv); - ASSERT_EQ(2, srv.name_change_reqs_.size()); + ASSERT_EQ(2, d2_mgr_.getQueueSize()); // The first name change request generated, should remove a DNS // mapping for an expired lease. verifyNameChangeRequest(srv, isc::dhcp_ddns::CHG_REMOVE, true, true,