[master] Merge branch 'trac3352'

Resolved Conflicts:
	src/bin/dhcp6/dhcp6_srv.cc

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));

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:

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;

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

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

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

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