BIND 10 trac3222, updated. e6d57d893c8abb993158ba3621e087e6bd0d1b30 [3222] b10-dhcp6 now sends NCRs to b10-dhcp-ddns

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Feb 20 20:26:48 UTC 2014


The branch, trac3222 has been updated
       via  e6d57d893c8abb993158ba3621e087e6bd0d1b30 (commit)
      from  b30b29613ed5906aa5909e951232f4cfa865609e (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit e6d57d893c8abb993158ba3621e087e6bd0d1b30
Author: Thomas Markwalder <tmark at isc.org>
Date:   Thu Feb 20 15:25:06 2014 -0500

    [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.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp6/dhcp6_srv.cc           |   50 +++++++------------
 src/bin/dhcp6/dhcp6_srv.h            |   11 -----
 src/bin/dhcp6/tests/fqdn_unittest.cc |   87 +++++++++++++++++++---------------
 3 files changed, 66 insertions(+), 82 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 8879679..9c9032f 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 91dbd8b..e0d4791 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 7347958..c11370b 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<NakedDhcpv6Srv> 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,



More information about the bind10-changes mailing list