BIND 10 master, updated. b1a0f405463723d539b2e6ed2dcdd692d7796b88 [master] Merge branch 'trac3352'

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Mar 13 17:33:51 UTC 2014


The branch, master has been updated
       via  b1a0f405463723d539b2e6ed2dcdd692d7796b88 (commit)
       via  45b5ff67532605280e2af903263097745c2f5804 (commit)
       via  4fc2c2cb250f93809d3287bdf9d50f5cbbc0fd17 (commit)
       via  614d16ac5d403ff87883c2881f01e1dda3214e75 (commit)
       via  4003a67bb649d47e9e39ba1bded31f8919614e1e (commit)
       via  5351f5e3d01499fb643aeb0146e29bb1d153f333 (commit)
       via  9e1339296c3864779691099231d36fe287c35e24 (commit)
      from  e603c7267edd3793d005c11975885bd893404a74 (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 b1a0f405463723d539b2e6ed2dcdd692d7796b88
Merge: e603c72 45b5ff6
Author: Thomas Markwalder <tmark at isc.org>
Date:   Thu Mar 13 13:33:15 2014 -0400

    [master] Merge branch 'trac3352'
    
    Resolved Conflicts:
    	src/bin/dhcp6/dhcp6_srv.cc

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

Summary of changes:
 src/bin/dhcp4/dhcp4_srv.cc                  |    5 +-
 src/bin/dhcp4/tests/fqdn_unittest.cc        |   16 ++--
 src/bin/dhcp6/dhcp6_srv.cc                  |   43 +++++-----
 src/bin/dhcp6/tests/fqdn_unittest.cc        |  113 +++++++++++++++++++++++----
 src/lib/dhcpsrv/d2_client_mgr.cc            |   26 +++---
 src/lib/dhcpsrv/d2_client_mgr.h             |   61 ++++++++++++++-
 src/lib/dhcpsrv/tests/d2_client_unittest.cc |   85 ++++++++++++++++++--
 7 files changed, 279 insertions(+), 70 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index dee4e41..0ad2319 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -966,8 +966,9 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         Option4ClientFqdn>(answer->getOption(DHO_FQDN));
     if (fqdn) {
         hostname = fqdn->getDomainName();
-        fqdn_fwd = fqdn->getFlag(Option4ClientFqdn::FLAG_S);
-        fqdn_rev = !fqdn->getFlag(Option4ClientFqdn::FLAG_N);
+        CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*fqdn,
+                                                                fqdn_fwd,
+                                                                fqdn_rev);
     } else {
         opt_hostname = boost::dynamic_pointer_cast<OptionString>
             (answer->getOption(DHO_HOST_NAME));
diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 9c80e13..90f95f5 100644
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -387,15 +387,16 @@ public:
 
         // NCRs cannot be sent to the d2_mgr unless updates are enabled.
         if (d2_mgr_.ddnsEnabled()) {
-            // There should be an NCR only if response S flag is 1.
-            /// @todo This logic will need to change if forward and reverse
-            /// updates are ever controlled independently.
-            if ((response_flags & Option4ClientFqdn::FLAG_S) == 0) {
+            // There should be an NCR if response S flag is 1 or N flag is 0.
+            bool exp_fwd = (response_flags & Option4ClientFqdn::FLAG_S);
+            bool exp_rev = (!(response_flags & Option4ClientFqdn::FLAG_N));
+            if (!exp_fwd && !exp_rev) {
                 ASSERT_EQ(0, d2_mgr_.getQueueSize());
             } else {
                 // Verify that there is one NameChangeRequest as expected.
                 ASSERT_EQ(1, d2_mgr_.getQueueSize());
-                verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+                verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD,
+                                        exp_rev, exp_fwd,
                                         reply->getYiaddr().toText(),
                                         "myhost.example.com.",
                                         "", // empty DHCID means don't check it
@@ -537,13 +538,12 @@ TEST_F(NameDhcpv4SrvTest, overrideNoUpdate) {
 //
 // Server should respect client's delegation request and NOT do updates:
 
-// - Response flags should be  N = 1, S = 0, O = 0
+// - Response flags should be  N = 0, S = 0, O = 0
 // - Should not queue any NCRs
 TEST_F(NameDhcpv4SrvTest, respectClientDelegation) {
 
     flagVsConfigScenario(Option4ClientFqdn::FLAG_E,
-                         (Option4ClientFqdn::FLAG_E |
-                          Option4ClientFqdn::FLAG_N));
+                         Option4ClientFqdn::FLAG_E);
 }
 
 // Tests the following scenario:
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index aac0ff6..cc99ab3 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1060,6 +1060,19 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer) {
         return;
     }
 
+    // Get the update directions that should be performed based on our
+    // response FQDN flags.
+    bool do_fwd = false;
+    bool do_rev = false;
+    CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*opt_fqdn,
+                                                             do_fwd, do_rev);
+    if (!do_fwd && !do_rev) {
+        // Flags indicate there is Nothing to do, get out now.
+        // The Most likely scenario is that we are honoring the client's
+        // request that no updates be done.
+        return;
+    }
+
     // Get the Client Id. It is mandatory and a function creating a response
     // would have thrown an exception if it was missing. Thus throwning
     // Unexpected if it is missing as it is a programming error.
@@ -1099,14 +1112,11 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer) {
         // Create new NameChangeRequest. Use the domain name from the FQDN.
         // This is an FQDN included in the response to the client, so it
         // holds a fully qualified domain-name already (not partial).
-        // 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.
+        // Get the IP address from the lease.
         NameChangeRequestPtr ncr;
         ncr.reset(new NameChangeRequest(isc::dhcp_ddns::CHG_ADD,
-                                        opt_fqdn->getFlag(Option6ClientFqdn::
-                                                          FLAG_S),
-                                        true, opt_fqdn->getDomainName(),
+                                        do_fwd, do_rev,
+                                        opt_fqdn->getDomainName(),
                                         iaaddr->getAddress().toText(),
                                         dhcid, 0, iaaddr->getValid()));
 
@@ -1239,12 +1249,8 @@ Dhcpv6Srv::assignIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
     Option6ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
         Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
     if (fqdn) {
-        /// @todo For now, we assert that if we are doing forward we are also
-        /// doing reverse.
-        if (fqdn->getFlag(Option6ClientFqdn::FLAG_S)) {
-            do_fwd = true;
-            do_rev = true;
-        }
+        CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*fqdn,
+                                                                do_fwd, do_rev);
     }
     // Set hostname only in case any of the updates is being performed.
     std::string hostname;
@@ -1513,8 +1519,8 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
 
     } else {
 
-        // At this point, we have to make make some decisions with respect
-        // to the FQDN option that we have generated as a result of receiving
+        // At this point, we have to make make some decisions with respect to
+        // the FQDN option that we have generated as a result of receiving
         // client's FQDN. In particular, we have to get to know if the DNS
         // update will be performed or not. It is possible that option is NULL,
         // which is valid condition if client didn't request DNS updates and
@@ -1524,12 +1530,9 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         Option6ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
             Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
         if (fqdn) {
-        // For now, we assert that if we are doing forward we are also
-        // doing reverse.
-            if (fqdn->getFlag(Option6ClientFqdn::FLAG_S)) {
-                do_fwd = true;
-                do_rev = true;
-            }
+            CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*fqdn,
+                                                                    do_fwd,
+                                                                    do_rev);
         }
 
         std::string hostname;
diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index 43c0958..b57aa4c 100644
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -40,6 +40,7 @@ using namespace std;
 
 namespace {
 
+
 /// @brief A test fixture class for testing DHCPv6 Client FQDN Option handling.
 class FqdnDhcpv6SrvTest : public Dhcpv6SrvTest {
 public:
@@ -56,6 +57,18 @@ public:
     static const uint16_t OVERRIDE_CLIENT_UPDATE = 4;
     static const uint16_t REPLACE_CLIENT_NAME = 8;
 
+    // Type used to indicate whether or not forward updates are expected
+    struct ExpFwd {
+        ExpFwd(bool exp_fwd) : value_(exp_fwd){};
+        bool value_;
+    };
+
+    // Type used to indicate whether or not reverse updates are expected
+    struct ExpRev {
+        ExpRev(bool exp_rev) : value_(exp_rev){};
+        bool value_;
+    };
+
     /// @brief Constructor
     FqdnDhcpv6SrvTest()
         : Dhcpv6SrvTest(), srv_(new NakedDhcpv6Srv(0)),
@@ -264,13 +277,16 @@ public:
     }
 
     /// @brief Verifies if the DHCPv6 server processes DHCPv6 Client FQDN option
-    /// as expected.
+    /// as expected and subsequent interpretation of this processing when
+    /// creating NCRs, if any should be created.
     ///
     /// This function simulates generation of the client's message holding FQDN.
     /// It then calls the server's @c Dhcpv6Srv::processClientFqdn option to
     /// generate server's response to the FQDN. This function returns the FQDN
     /// which should be appended to the server's response to the client.
-    /// This function verifies that the FQDN option returned is correct.
+    /// This function verifies that the FQDN option returned is correct
+    /// Optionally, this function then proceeds to call createNameChangeRequests
+    /// to verify if that method interprets the FQDN information properly.
     ///
     /// @param msg_type A type of the client's message.
     /// @param in_flags A value of flags field to be set for the FQDN carried
@@ -282,22 +298,35 @@ public:
     /// @param exp_flags A value of flags expected in the FQDN sent by a server.
     /// @param exp_domain_name A domain name expected in the FQDN sent by a
     /// server.
+    /// @param create_ncr_check if true, calls createNameChangeRequests method
+    /// and tests the outcome.
+    /// @param exp_fwd indicates whether or not a forward change is expected
+    /// @param exp_fwd indicates whether or not a reverse change is expected
     void testFqdn(const uint16_t msg_type,
                   const uint8_t in_flags,
                   const std::string& in_domain_name,
                   const Option6ClientFqdn::DomainNameType in_domain_type,
                   const uint8_t exp_flags,
-                  const std::string& exp_domain_name) {
+                  const std::string& exp_domain_name,
+                  const bool create_ncr_check,
+                  const ExpFwd& exp_fwd = ExpFwd(true),
+                  const ExpRev& exp_rev = ExpRev(true)) {
 
         Pkt6Ptr question = generateMessage(msg_type,
                                            in_flags,
                                            in_domain_name,
                                            in_domain_type,
                                            true);
+
         ASSERT_TRUE(getClientFqdnOption(question));
 
-        Pkt6Ptr answer(new Pkt6(msg_type == DHCPV6_SOLICIT ? DHCPV6_ADVERTISE :
-                                DHCPV6_REPLY, question->getTransid()));
+        Pkt6Ptr answer = generateMessageWithIds(msg_type == DHCPV6_SOLICIT
+                                                ? DHCPV6_ADVERTISE :
+                                                DHCPV6_REPLY);
+
+        // Create three IAs, each having different address.
+        addIA(1234, IOAddress("2001:db8:1::1"), answer);
+
         ASSERT_NO_THROW(srv_->processClientFqdn(question, answer));
         Option6ClientFqdnPtr answ_fqdn = boost::dynamic_pointer_cast<
             Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
@@ -325,6 +354,25 @@ public:
             EXPECT_EQ(Option6ClientFqdn::FULL, answ_fqdn->getDomainNameType());
 
         }
+
+        if (create_ncr_check) {
+            // Call createNameChangeRequests
+            ASSERT_NO_THROW(srv_->createNameChangeRequests(answer));
+            if (exp_fwd.value_ || exp_rev.value_) {
+                // Should have created 1 NCR.
+                NameChangeRequestPtr ncr;
+                ASSERT_EQ(1, d2_mgr_.getQueueSize());
+                ASSERT_NO_THROW(ncr = d2_mgr_.peekAt(0));
+                ASSERT_TRUE(ncr);
+                EXPECT_EQ(dhcp_ddns::CHG_ADD, ncr->getChangeType());
+                EXPECT_EQ(exp_fwd.value_, ncr->isForwardChange());
+                EXPECT_EQ(exp_rev.value_, ncr->isReverseChange());
+                ASSERT_NO_THROW(d2_mgr_.runReadyIO());
+            } else {
+                // Should not have created any NCRs.
+                EXPECT_EQ(0, d2_mgr_.getQueueSize());
+            }
+        }
     }
 
     /// @brief Tests that the client's message holding an FQDN is processed
@@ -332,6 +380,7 @@ public:
     ///
     /// @param msg_type A type of the client's message.
     /// @param hostname A domain name in the client's FQDN.
+    /// @param client_flags A bitmask of the client FQDN flags
     /// @param include_oro A boolean value which indicates whether the ORO
     /// option should be included in the client's message (if true) or not
     /// (if false). In the former case, the function will expect that server
@@ -340,6 +389,8 @@ public:
     void testProcessMessage(const uint8_t msg_type,
                             const std::string& hostname,
                             const std::string& exp_hostname,
+                            const uint8_t client_flags =
+                                Option6ClientFqdn::FLAG_S,
                             const bool include_oro = true) {
         // Create a message of a specified type, add server id and
         // FQDN option.
@@ -348,8 +399,7 @@ public:
         // empty.
         Option6ClientFqdn::DomainNameType fqdn_type = (hostname.empty() ?
             Option6ClientFqdn::PARTIAL : Option6ClientFqdn::FULL);
-
-        Pkt6Ptr req = generateMessage(msg_type, Option6ClientFqdn::FLAG_S,
+        Pkt6Ptr req = generateMessage(msg_type, client_flags,
                                       hostname, fqdn_type, include_oro, srvid);
 
         // For different client's message types we have to invoke different
@@ -460,15 +510,13 @@ public:
 
 // A set of tests verifying server's behaviour when it receives the DHCPv6
 // Client Fqdn Option.
-// @todo: Extend these tests once appropriate configuration parameters are
-// implemented (ticket #3034).
 
 // Test server's response when client requests that server performs AAAA update.
 TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdate) {
     testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S,
              "myhost.example.com",
              Option6ClientFqdn::FULL, Option6ClientFqdn::FLAG_S,
-             "myhost.example.com.");
+             "myhost.example.com.", true, ExpFwd(true), ExpRev(true));
 }
 
 // Test server's response when client provides partial domain-name and requests
@@ -476,14 +524,14 @@ TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdate) {
 TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdatePartialName) {
     testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S, "myhost",
              Option6ClientFqdn::PARTIAL, Option6ClientFqdn::FLAG_S,
-             "myhost.example.com.");
+             "myhost.example.com.", true, ExpFwd(true), ExpRev(true));
 }
 
 // Test server's response when client provides empty domain-name and requests
 // that server performs AAAA update.
 TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdateNoName) {
     testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S, "",
-             Option6ClientFqdn::PARTIAL, Option6ClientFqdn::FLAG_S, "");
+             Option6ClientFqdn::PARTIAL, Option6ClientFqdn::FLAG_S, "", false);
 }
 
 // Test server's response when client requests no DNS update.
@@ -491,7 +539,27 @@ TEST_F(FqdnDhcpv6SrvTest, noUpdate) {
     testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_N,
              "myhost.example.com",
              Option6ClientFqdn::FULL, Option6ClientFqdn::FLAG_N,
-             "myhost.example.com.");
+             "myhost.example.com.", true, ExpFwd(false), ExpRev(false));
+}
+
+// Test server's response when client requests no DNS update and
+// override-no-updates is true.
+TEST_F(FqdnDhcpv6SrvTest, overrideNoUpdate) {
+    enableD2(OVERRIDE_NO_UPDATE);
+    testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_N,
+             "myhost.example.com",
+             Option6ClientFqdn::FULL,
+             (Option6ClientFqdn::FLAG_S | Option6ClientFqdn::FLAG_O),
+             "myhost.example.com.", true, ExpFwd(true), ExpRev(true));
+}
+
+// Test server's response when client requests that server delegates the AAAA
+// update to the client
+TEST_F(FqdnDhcpv6SrvTest, clientAAAAUpdate) {
+    testFqdn(DHCPV6_SOLICIT, 0, "myhost.example.com.",
+             Option6ClientFqdn::FULL,
+             0,
+             "myhost.example.com.", true, ExpFwd(false), ExpRev(true));
 }
 
 // Test server's response when client requests that server delegates the AAAA
@@ -501,7 +569,7 @@ TEST_F(FqdnDhcpv6SrvTest, clientAAAAUpdateNotAllowed) {
     testFqdn(DHCPV6_SOLICIT, 0, "myhost.example.com.",
              Option6ClientFqdn::FULL,
              Option6ClientFqdn::FLAG_S | Option6ClientFqdn::FLAG_O,
-             "myhost.example.com.");
+             "myhost.example.com.", true, ExpFwd(true), ExpRev(true));
 }
 
 // Test that exception is thrown if supplied NULL answer packet when
@@ -871,7 +939,7 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestWithoutFqdn) {
     // In this case, we expect that the FQDN option will not be included
     // in the server's response. The testProcessMessage will check that.
     testProcessMessage(DHCPV6_REQUEST, "myhost.example.com",
-                       "myhost.example.com.", false);
+                       "myhost.example.com.", Option6ClientFqdn::FLAG_S, false);
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
@@ -884,7 +952,8 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestWithoutFqdn) {
 // FQDN.
 TEST_F(FqdnDhcpv6SrvTest, processRequestEmptyFqdn) {
     testProcessMessage(DHCPV6_REQUEST, "",
-                       "myhost-2001-db8-1-1--dead-beef.example.com.", false);
+                       "myhost-2001-db8-1-1--dead-beef.example.com.",
+                       Option6ClientFqdn::FLAG_S, false);
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
                             "2001:db8:1:1::dead:beef",
@@ -965,4 +1034,16 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestReuseExpiredLease) {
 
 }
 
+TEST_F(FqdnDhcpv6SrvTest, processClientDelegation) {
+    testProcessMessage(DHCPV6_REQUEST, "myhost.example.com",
+                       "myhost.example.com.", 0);
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, false,
+                            "2001:db8:1:1::dead:beef",
+                            "000201415AA33D1187D148275136FA30300478"
+                            "FAAAA3EBD29826B5C907B2C9268A6F52",
+                            0, 4000);
+}
+
+
 }   // end of anonymous namespace
diff --git a/src/lib/dhcpsrv/d2_client_mgr.cc b/src/lib/dhcpsrv/d2_client_mgr.cc
index c806bc4..1e03d76 100644
--- a/src/lib/dhcpsrv/d2_client_mgr.cc
+++ b/src/lib/dhcpsrv/d2_client_mgr.cc
@@ -144,14 +144,20 @@ D2ClientMgr::analyzeFqdn(const bool client_s, const bool client_n,
 
     switch (mask) {
     case 0:
-        // If updates are enabled and we are overriding client delegation
-        // then S flag should be true.
-        server_s = (d2_client_config_->getEnableUpdates() &&
-                    d2_client_config_->getOverrideClientUpdate());
+        if (!d2_client_config_->getEnableUpdates()) {
+            server_s = false;
+            server_n = true;
+        } else {
+            // If updates are enabled and we are overriding client delegation
+            // then S flag should be true.  N-flag should be false.
+            server_s = d2_client_config_->getOverrideClientUpdate();
+            server_n = false;
+        }
         break;
 
     case 1:
         server_s = d2_client_config_->getEnableUpdates();
+        server_n = !server_s;
         break;
 
     case 2:
@@ -159,6 +165,7 @@ D2ClientMgr::analyzeFqdn(const bool client_s, const bool client_n,
         // S flag should be true.
         server_s = (d2_client_config_->getEnableUpdates() &&
                     d2_client_config_->getOverrideNoUpdate());
+        server_n = !server_s;
         break;
 
     default:
@@ -167,17 +174,6 @@ D2ClientMgr::analyzeFqdn(const bool client_s, const bool client_n,
                   "Invalid client FQDN - N and S cannot both be 1");
         break;
     }
-
-    /// @todo Currently we are operating under the premise that N should be 1
-    /// if the server is not doing updates nor do we have configuration
-    /// controls to govern forward and reverse updates independently.
-    /// In addition, the client FQDN flags cannot explicitly suggest what to
-    /// do with reverse updates. They request either forward updates or no
-    /// updates.  In other words, the client cannot request the server do or
-    /// not do reverse updates.  For now, we are either going to do updates in
-    /// both directions or none at all.  If and when additional configuration
-    /// parameters are added this logic will have to be reassessed.
-    server_n = !server_s;
 }
 
 std::string
diff --git a/src/lib/dhcpsrv/d2_client_mgr.h b/src/lib/dhcpsrv/d2_client_mgr.h
index 6222e6b..3086430 100644
--- a/src/lib/dhcpsrv/d2_client_mgr.h
+++ b/src/lib/dhcpsrv/d2_client_mgr.h
@@ -115,7 +115,39 @@ public:
     /// conjunction with the configuration parameters updates-enabled, override-
     /// no-updates, and override-client-updates to determine the values that
     /// should be used for the server's FQDN S and N flags.
-    /// The logic in this method is based upon RFCs 4702 and 4704.
+    /// The logic in this method is based upon RFCs 4702 and 4704, and is
+    /// shown in the following truth table:
+    ///
+    /// @code
+    ///
+    /// When Updates are enabled:
+    ///
+    ///  ON = Override No Updates, OC = Override Client Updates
+    ///
+    ///  | Client |--------   Server Response Flags   ------------|
+    ///  | Flags  | ON=F,OC=F | ON=F,OC=T | ON=T,OC=F | ON=T,OC=T |
+    ///  |  N-S   |  N-S-O    |   N-S-O   |   N-S-O   |   N-S-O   |
+    ///  ----------------------------------------------------------
+    ///  |  0-0   |  0-0-0    |   0-1-1   |   0-0-0   |   0-1-1   |
+    ///  |  0-1   |  0-1-0    |   0-1-0   |   0-1-0   |   0-1-0   |
+    ///  |  1-0   |  1-0-0    |   1-0-0   |   0-1-1   |   0-1-1   |
+    ///
+    /// One can then use the server response flags to know when forward and
+    /// reverse updates should be performed:
+    ///
+    ///  - Forward updates should be done when the Server S-Flag is true.
+    ///  - Reverse updates should be done when the Server N-Flag is false.
+    ///
+    /// When Updates are disabled:
+    ///
+    /// | Client  | Server |
+    /// |  N-S    |  N-S-O |
+    /// --------------------
+    /// |  0-0    | 1-0-0  |
+    /// |  0-1    | 1-0-1  |
+    /// |  1-0    | 1-0-0  |
+    ///
+    /// @endcode
     ///
     /// @param client_s  S Flag from the client's FQDN
     /// @param client_n  N Flag from the client's FQDN
@@ -172,6 +204,26 @@ public:
     template <class T>
     void adjustFqdnFlags(const T& fqdn, T& fqdn_resp);
 
+    /// @brief Get direcional update flags based on server FQDN flags
+    ///
+    /// Templated convenience method which determines whether forward and
+    /// reverse updates should be performed based on a server response version
+    /// of the FQDN flags. The logic is straight forward and currently not
+    /// dependent upon configuration specific values:
+    ///
+    /// * forward will be true if S_FLAG is true
+    /// * reverse will be true if N_FLAG is false
+    ///
+    /// @param fqdn FQDN option from which to read server (outbound) flags
+    /// @param [out] forward bool value will be set to true if forward udpates
+    /// should be done, false if not.
+    /// @param [out] reverse bool value will be set to true if reverse udpates
+    /// should be done, false if not.
+    /// @tparam T FQDN Option class containing the FQDN data such as
+    /// dhcp::Option4ClientFqdn or dhcp::Option6ClientFqdn
+    template <class T>
+    void getUpdateDirections(const T& fqdn_resp, bool& forward, bool& reverse);
+
     /// @brief Set server FQDN name based on configuration and a given FQDN
     ///
     /// Templated method which adjusts the domain name value and type in
@@ -394,6 +446,13 @@ D2ClientMgr::adjustFqdnFlags(const T& fqdn, T& fqdn_resp) {
     fqdn_resp.setFlag(T::FLAG_O, (fqdn.getFlag(T::FLAG_S) != server_s));
 }
 
+template <class T>
+void
+D2ClientMgr::getUpdateDirections(const T& fqdn_resp,
+                                 bool& forward, bool& reverse) {
+    forward = fqdn_resp.getFlag(T::FLAG_S);
+    reverse = !(fqdn_resp.getFlag(T::FLAG_N));
+}
 
 template <class T>
 void
diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc
index b935a0b..68c0e5f 100644
--- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc
+++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc
@@ -337,10 +337,10 @@ TEST(D2ClientMgr, analyzeFqdnEnabledNoOverrides) {
 
     // client S=0 N=0 means client wants to do forward update.
     // server S should be 0 (server is not doing forward updates)
-    // and server N should be 1 (server doing no updates)
+    // and server N should be 0 (server doing reverse updates)
     mgr.analyzeFqdn(false, false, server_s, server_n);
     EXPECT_FALSE(server_s);
-    EXPECT_TRUE(server_n);
+    EXPECT_FALSE(server_n);
 
     // client S=1 N=0 means client wants server to do forward update.
     // server S should be 1 (server is doing forward updates)
@@ -379,10 +379,10 @@ TEST(D2ClientMgr, analyzeFqdnEnabledOverrideNoUpdate) {
 
     // client S=0 N=0 means client wants to do forward update.
     // server S should be 0 (server is not doing forward updates)
-    // and server N should be 1 (server is not doing any updates)
+    // and server N should be 0 (server is doing reverse updates)
     mgr.analyzeFqdn(false, false, server_s, server_n);
     EXPECT_FALSE(server_s);
-    EXPECT_TRUE(server_n);
+    EXPECT_FALSE(server_n);
 
     // client S=1 N=0 means client wants server to do forward update.
     // server S should be 1 (server is doing forward updates)
@@ -462,7 +462,7 @@ TEST(D2ClientMgr, adjustFqdnFlagsV4) {
 
     // client S=0 N=0 means client wants to do forward update.
     // server S should be 0 (server is not doing forward updates)
-    // and server N should be 1 (server doing no updates)
+    // and server N should be 0 (server is doing reverse updates)
     // and server O should be 0
     request.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
                                         "", Option4ClientFqdn::PARTIAL));
@@ -471,7 +471,7 @@ TEST(D2ClientMgr, adjustFqdnFlagsV4) {
 
     mgr.adjustFqdnFlags<Option4ClientFqdn>(*request, *response);
     EXPECT_FALSE(response->getFlag(Option4ClientFqdn::FLAG_S));
-    EXPECT_TRUE(response->getFlag(Option4ClientFqdn::FLAG_N));
+    EXPECT_FALSE(response->getFlag(Option4ClientFqdn::FLAG_N));
     EXPECT_FALSE(response->getFlag(Option4ClientFqdn::FLAG_O));
 
     // client S=1 N=0 means client wants server to do forward update.
@@ -505,6 +505,42 @@ TEST(D2ClientMgr, adjustFqdnFlagsV4) {
     EXPECT_TRUE(response->getFlag(Option4ClientFqdn::FLAG_O));
 }
 
+/// @brief Verified the getUpdateDirections template method with
+/// Option4ClientFqdn objects.
+TEST(D2ClientMgr, updateDirectionsV4) {
+    D2ClientMgr mgr;
+    Option4ClientFqdnPtr response;
+
+    bool do_forward = false;
+    bool do_reverse = false;
+
+    // Response S=0, N=0 should mean do reverse only.
+    response.reset(new Option4ClientFqdn(0,
+                                         Option4ClientFqdn::RCODE_CLIENT(),
+                                         "", Option4ClientFqdn::PARTIAL));
+    mgr.getUpdateDirections(*response, do_forward, do_reverse);
+    EXPECT_FALSE(do_forward);
+    EXPECT_TRUE(do_reverse);
+
+    // Response S=0, N=1 should mean don't do either.
+    response.reset(new Option4ClientFqdn(Option4ClientFqdn::FLAG_N,
+                                         Option4ClientFqdn::RCODE_CLIENT(),
+                                         "", Option4ClientFqdn::PARTIAL));
+    mgr.getUpdateDirections(*response, do_forward, do_reverse);
+    EXPECT_FALSE(do_forward);
+    EXPECT_FALSE(do_reverse);
+
+    // Response S=1, N=0 should mean do both.
+    response.reset(new Option4ClientFqdn(Option4ClientFqdn::FLAG_S,
+                                         Option4ClientFqdn::RCODE_CLIENT(),
+                                         "", Option4ClientFqdn::PARTIAL));
+    mgr.getUpdateDirections(*response, do_forward, do_reverse);
+    EXPECT_TRUE(do_forward);
+    EXPECT_TRUE(do_reverse);
+
+    // Response S=1, N=1 isn't possible.
+}
+
 /// @brief Tests the qualifyName method's ability to construct FQDNs
 TEST(D2ClientMgr, qualifyName) {
     D2ClientMgr mgr;
@@ -761,7 +797,7 @@ TEST(D2ClientMgr, adjustFqdnFlagsV6) {
 
     // client S=0 N=0 means client wants to do forward update.
     // server S should be 0 (server is not doing forward updates)
-    // and server N should be 1 (server doing no updates)
+    // and server N should be 0 (server doing reverse updates)
     // and server O should be 0
     request.reset(new Option6ClientFqdn(0, "", Option6ClientFqdn::PARTIAL));
     response.reset(new Option6ClientFqdn(*request));
@@ -769,7 +805,7 @@ TEST(D2ClientMgr, adjustFqdnFlagsV6) {
 
     mgr.adjustFqdnFlags<Option6ClientFqdn>(*request, *response);
     EXPECT_FALSE(response->getFlag(Option6ClientFqdn::FLAG_S));
-    EXPECT_TRUE(response->getFlag(Option6ClientFqdn::FLAG_N));
+    EXPECT_FALSE(response->getFlag(Option6ClientFqdn::FLAG_N));
     EXPECT_FALSE(response->getFlag(Option6ClientFqdn::FLAG_O));
 
     // client S=1 N=0 means client wants server to do forward update.
@@ -801,4 +837,37 @@ TEST(D2ClientMgr, adjustFqdnFlagsV6) {
     EXPECT_TRUE(response->getFlag(Option6ClientFqdn::FLAG_O));
 }
 
+/// @brief Verified the getUpdateDirections template method with
+/// Option6ClientFqdn objects.
+TEST(D2ClientMgr, updateDirectionsV6) {
+    D2ClientMgr mgr;
+    Option6ClientFqdnPtr response;
+
+    bool do_forward = false;
+    bool do_reverse = false;
+
+    // Response S=0, N=0 should mean do reverse only.
+    response.reset(new Option6ClientFqdn(0,
+                                         "", Option6ClientFqdn::PARTIAL));
+    mgr.getUpdateDirections(*response, do_forward, do_reverse);
+    EXPECT_FALSE(do_forward);
+    EXPECT_TRUE(do_reverse);
+
+    // Response S=0, N=1 should mean don't do either.
+    response.reset(new Option6ClientFqdn(Option6ClientFqdn::FLAG_N,
+                                         "", Option6ClientFqdn::PARTIAL));
+    mgr.getUpdateDirections(*response, do_forward, do_reverse);
+    EXPECT_FALSE(do_forward);
+    EXPECT_FALSE(do_reverse);
+
+    // Response S=1, N=0 should mean do both.
+    response.reset(new Option6ClientFqdn(Option6ClientFqdn::FLAG_S,
+                                         "", Option6ClientFqdn::PARTIAL));
+    mgr.getUpdateDirections(*response, do_forward, do_reverse);
+    EXPECT_TRUE(do_forward);
+    EXPECT_TRUE(do_reverse);
+
+    // Response S=1, N=1 isn't possible.
+}
+
 } // end of anonymous namespace



More information about the bind10-changes mailing list