[#225] Addressed review comments

Mostly minor nits and cosmetics:

modified:
doc/sphinx/arm/dhcp4-srv.rst
doc/sphinx/arm/dhcp6-srv.rst
src/bin/dhcp6/tests/fqdn_unittest.cc
src/lib/dhcpsrv/ncr_generator.cc
src/lib/dhcpsrv/parsers/base_network_parser.h
src/lib/dhcpsrv/srv_config.h
src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
src/lib/dhcpsrv/tests/ncr_generator_unittest.cc

diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst
index 895bf21f4c25c747ed6bbc533fbc8fb9dd65545d..2f6c1b9dc15c5cd81f983a5f9505f95388ab7233 100644 (file)
--- a/doc/sphinx/arm/dhcp4-srv.rst
+++ b/doc/sphinx/arm/dhcp4-srv.rst
@@ -3409,7 +3409,6 @@ The default configuration and values would appear as follows:
         "ddns-qualifying-suffix": "",
         "ddns-update-on-renew": false,
         "ddns-use-conflict-resolution": true,
-        "ddns-ttl-percent": 0.75,
         "hostname-char-set": "",
         "hostname-char-replacement": ""
         ...

diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index f506d317e6b09bc554f7dcc76ef3fabc85a629ae..59c7c3eaddf074747ebd375f6fa1d996ce501147 100644 (file)
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -2992,7 +2992,6 @@ The default configuration and values would appear as follows:
         "ddns-qualifying-suffix": "",
         "ddns-update-on-renew": false,
         "ddns-use-conflict-resolution": true,
-        "ddns-ttl-percent": 0.75,
         "hostname-char-set": "",
         "hostname-char-replacement": ""
         ...

diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index f91342fa1d1f98f6f3ba4f3c18b6c528188d02b3..2c7ae7d0c14fe51a53f24191ea6ad3c18e778381 100644 (file)
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -2144,8 +2144,8 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenew) {
     }
 }
 
-// Verify that ddns-ttl-percent can be used to calculate
-// NCR lifetime.
+// Verify that when specified ddns-ttl-percent is used to calculate
+// the lease length in an NCR.
 TEST_F(FqdnDhcpv6SrvTest, ddnsTtlPercent) {
     // Create Reply message with Client Id and Server id.
     Pkt6Ptr answer = generateMessageWithIds(DHCPV6_REPLY);
@@ -2178,5 +2178,4 @@ TEST_F(FqdnDhcpv6SrvTest, ddnsTtlPercent) {
                             0, 500, "", false, subnet_->getDdnsTtlPercent());
 }
 
-
 } // end of anonymous namespace

diff --git a/src/lib/dhcpsrv/ncr_generator.cc b/src/lib/dhcpsrv/ncr_generator.cc
index 42ead6c2eeed7144e406d9a7317da1a7834e32e6..4646ef8b989c28789ef9f6db6e2f67fbe5428cf7 100644 (file)
--- a/src/lib/dhcpsrv/ncr_generator.cc
+++ b/src/lib/dhcpsrv/ncr_generator.cc
@@ -32,7 +32,6 @@ namespace {
 /// @param label Client identification information in the textual format.
 /// This is used for logging purposes.
 /// @param subnet subnet to which the lease belongs.
-/// use conflict resolution.
 ///
 /// @tparam LeasePtrType Pointer to a lease.
 /// @tparam IdentifierType HW Address, Client Identifier or DUID.
@@ -130,7 +129,7 @@ void queueNCR(const NameChangeType& chg_type, const Lease6Ptr& lease) {
 
 uint32_t calculateDdnsTtl(uint32_t lease_lft, const util::Optional<double>& ddns_ttl_percent) {
     //  If we have a configured percentage use it to calculate TTL.
-    if (!ddns_ttl_percent.unspecified() && (ddns_ttl_percent.get() > 0.0000)) {
+    if (!ddns_ttl_percent.unspecified() && (ddns_ttl_percent.get() > 0.0)) {
         uint32_t new_lft = static_cast<uint32_t>(round(ddns_ttl_percent.get() * lease_lft));
         if (new_lft > 0) {
             return (new_lft);

diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.h b/src/lib/dhcpsrv/parsers/base_network_parser.h
index 7ba01f70e746bd96960c774008eec1452364d7cb..19a1e1b3bdf7c0634773aaa74f114923837a31cd 100644 (file)
--- a/src/lib/dhcpsrv/parsers/base_network_parser.h
+++ b/src/lib/dhcpsrv/parsers/base_network_parser.h
@@ -99,6 +99,7 @@ protected:
     /// - ddns-generated-prefix
     /// - ddns-qualifying-suffix
     /// - ddns-use-conflict-resolution
+    /// - ddns-update-on-renew
     /// - ddns-ttl-percent
     ///
     /// @param network_data Data element holding shared network

diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h
index 6bc66e596c8fc799982873cdb66dd1ad61eb916b..c3d1b571f79243f77f2a06e07d94975d3ae93a14 100644 (file)
--- a/src/lib/dhcpsrv/srv_config.h
+++ b/src/lib/dhcpsrv/srv_config.h
@@ -155,7 +155,7 @@ public:
     /// This value, if greater than zero, is used to calculate the lease lifetime
     /// passed to D2 in the NCR.  Otherwise the value is calculated per RFC 4702.
     ///
-    /// @return True if conflict resolution should be used.
+    /// @return TTL percent as an Optional.
     util::Optional<double> getTtlPercent() const;
 
     /// @brief Returns the subnet-id of the subnet associated with these parameters

diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
index 9f157bf2dbf3d86fbb0c7b29dc8566ebfd31fe9b..8915dfbd82a2b0d4e6d716af08a37b0918774e41 100644 (file)
--- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
@@ -2072,7 +2072,7 @@ TEST_F(ClientClassDefParserTest, offerLft) {
 
     auto offer_lft = cclass->getOfferLft();
     ASSERT_FALSE(offer_lft.unspecified());
-    EXPECT_EQ(99, offer_lft);
+    EXPECT_EQ(99, offer_lft.get());
 }
 
 // Test verifies that the parser rejects bogus offer-lifetime value.

diff --git a/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc b/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
index 4cbe1d77074948c06d22891c74aa2d685d9c1efe..acc7f3a713e73e1a8aa5d8cb8c096ffa88298990 100644 (file)
--- a/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
+++ b/src/lib/dhcpsrv/tests/ncr_generator_unittest.cc
@@ -732,6 +732,10 @@ TEST_F(NCRGenerator4Test, withTtlPercent) {
                 "000001E356D43E5F0A496D65BCA24D982D646140813E3"
                 "B03AB370BFF46BFA309AE7BFD", true, ttl_percent);
     }
+
+    double dnum = -1.0;
+    uint32_t inum =  static_cast<uint32_t>(round(dnum * 3400));
+    std::cout << "inums is: " << inum << std::endl;
 }