[3985] Several minor tweaks, docs updated.

doc/guide/dhcp6-srv.xml

@@ -2676,8 +2676,30 @@ should include options from the isc option space:
       </listitem>
     </itemizedlist>
     </para>
+  </section>
+
+    <section id="dhcp6-decline">
+      <title>Duplicate Addresses (DECLINE support)</title>
+      <note>
+        <para>@todo: Full text will be added as part of #3990.</para>
+      </note>
+
+      <para>
+        The server does not decrease assigned-addresses statistics
+        when Decline message is received and processed successfully. While
+        technically a declined address is no longer assigned, the primary usage
+        of the assigned-addresses statistic is to monitor pool utilization. Most
+        people would forget to include declined-addresses in the calculation,
+        and simply do assigned-addresses/total-addresses. This would have a bias
+        towards under-representing pool utilization. As this has a
+        potential for major issues, we decided not to decrease assigned
+        addresses immediately after receiving Decline, but to do
+        it later when we recover the address back to the available pool.
+      </para>
+
     </section>
 
+
     <section id="dhcp6-stats">
       <title>Statistics in DHCPv6 server</title>
       <note>

src/lib/dhcpsrv/alloc_engine.h

@@ -506,7 +506,7 @@ public:
     /// - updating statistics of assigned and reclaimed leases
     ///
     /// Note: declined leases fall under the same expiration/reclaimation
-    /// processing as normal leases. In principle, it would more elegant
+    /// processing as normal leases. In principle, it would be more elegant
     /// to have a separate processing for declined leases reclaimation. However,
     /// due to performance reasons we decided to use them together. Several
     /// aspects were taken into consideration. First, normal leases are expected
@@ -524,7 +524,7 @@ public:
     /// declined state). Therefore remove_leases parameter is ignored for
     /// declined leases. They are always removed.
     ///
-    /// Also, for delined leases @ref reclaimDeclined is called. It conducts
+    /// Also, for declined leases @ref reclaimDeclined is called. It conducts
     /// several declined specific operation (extra log entry, stats dump,
     /// hooks).
     ///
@@ -552,7 +552,7 @@ public:
     /// - updating statistics of assigned and reclaimed leases
     ///
     /// Note: declined leases fall under the same expiration/reclaimation
-    /// processing as normal leases. In principle, it would more elegant
+    /// processing as normal leases. In principle, it would be more elegant
     /// to have a separate processing for declined leases reclaimation. However,
     /// due to performance reasons we decided to use them together. Several
     /// aspects were taken into consideration. First, normal leases are expected
@@ -570,7 +570,7 @@ public:
     /// declined state). Therefore remove_leases parameter is ignored for
     /// declined leases. They are always removed.
     ///
-    /// Also, for delined leases @ref reclaimDeclined is called. It conducts
+    /// Also, for declined leases @ref reclaimDeclined is called. It conducts
     /// several declined specific operation (extra log entry, stats dump,
     /// hooks).
     ///

src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc

@@ -990,9 +990,9 @@ public:
                            stat_name), int64_t(2000));
 
         stats_mgr.setValue(stats_mgr.generateName("subnet", 1,
-                           "reclaimed-declined-addresses"), int64_t(3000));
+                           "reclaimed-declined-addresses"), int64_t(10000));
         stats_mgr.setValue(stats_mgr.generateName("subnet", 2,
-                           "reclaimed-declined-addresses"), int64_t(4000));
+                           "reclaimed-declined-addresses"), int64_t(20000));
 
         stats_mgr.setValue(stats_mgr.generateName("subnet", 1,
                            "declined-addresses"), int64_t(100));
@@ -1021,11 +1021,11 @@ public:
         testStatistics("subnet[2]." + stat_name, 2000 - subnet2_cnt);
 
         testStatistics("subnet[1].declined-addresses", 100 - subnet1_cnt);
-        testStatistics("subnet[2.declined-addresses", 100 - subnet1_cnt);
+        testStatistics("subnet[2].declined-addresses", 200 - subnet2_cnt);
 
         // subnet[X].reclaimed-declined-addresses should go up in each subnet
-        testStatistics("subnet[1].reclaimed-declined-addresses", 3000 + subnet1_cnt);
-        testStatistics("subnet[2].reclaimed-declined-addresses", 4000 + subnet1_cnt);
+        testStatistics("subnet[1].reclaimed-declined-addresses", 10000 + subnet1_cnt);
+        testStatistics("subnet[2].reclaimed-declined-addresses", 20000 + subnet1_cnt);
     }
 
     /// @brief Collection of leases created at construction time.

src/lib/dhcpsrv/tests/alloc_engine_utils.cc

@@ -59,6 +59,9 @@ bool testStatistics(const std::string& stat_name, const int64_t exp_value) {
                     <<  "doesn't match expected value (" << exp_value << ")";
             }
             return (observation->getInteger().first == exp_value);
+        } else {
+            ADD_FAILURE() << "Expected statistic " << stat_name
+                          << " not found.";
         }
 
     } catch (...) {

src/lib/dhcpsrv/tests/alloc_engine_utils.h

@@ -289,7 +289,7 @@ public:
     ///
     /// This test inserts existing_lease (if specified, may be null) into the
     /// LeaseMgr, then conducts lease allocation (pretends that client
-    /// sent either Discover or Request, depending on fake_allocation).
+    /// sent either Solicit or Request, depending on fake_allocation).
     /// Allocated lease is then returned (using result) for further inspection.
     ///
     /// @param alloc_engine allocation engine
@@ -315,7 +315,7 @@ public:
     ///
     /// @param addr address of the lease
     /// @param probation_period expressed in seconds
-    /// @param expired number of seconds where it will expire
+    /// @param expired number of seconds when the lease will expire
     Lease6Ptr generateDeclinedLease(const std::string& addr,
                                     time_t probation_period,
                                     int32_t expired);
@@ -448,7 +448,7 @@ public:
                          ExpectedResult exp_result,
                          Lease4Ptr& result);
 
-    /// @brief Creates a declined lease with specified expiration time
+    /// @brief Creates a declined IPv4 lease with specified expiration time
     ///
     /// expired parameter controls probation period. Positive value
     /// means that the lease will expire in X seconds. Negative means
@@ -458,7 +458,7 @@ public:
     ///
     /// @param addr address of the lease
     /// @param probation_period expressed in seconds
-    /// @param expired number of seconds where it will expire
+    /// @param expired number of seconds when the lease will expire
     Lease4Ptr generateDeclinedLease(const std::string& addr,
                                     time_t probation_period,
                                     int32_t expired);

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014, 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -2393,7 +2393,6 @@ GenericLeaseMgrTest::testGetDeclinedLeases6() {
     }
 }
 
-
 }; // namespace test
 }; // namespace dhcp
 }; // namespace isc

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h

@@ -250,7 +250,7 @@ public:
     /// Checks that the code is able to update an IPv6 lease in the database.
     void testUpdateLease6();
 
-    /// @brief Check that the DHCPv6 lease can be added, removed and recreated.
+    /// @brief Check that the IPv6 lease can be added, removed and recreated.
     ///
     /// This test creates a lease, removes it and then recreates it with some
     /// of the attributes changed. Next it verifies that the lease in the
@@ -275,7 +275,7 @@ public:
     /// - reclaimed leases are not returned.
     void testGetExpiredLeases4();
 
-    /// @brief Checks that the expired DHCPv6 leases can be retrieved.
+    /// @brief Checks that the expired IPv6 leases can be retrieved.
     ///
     /// This test checks the following:
     /// - all expired and not reclaimed leases are retured
@@ -285,7 +285,7 @@ public:
     /// - reclaimed leases are not returned.
     void testGetExpiredLeases6();
 
-    /// @brief Checks that declined DHCPv4 leases that have expired can be retrieved.
+    /// @brief Checks that declined IPv4 leases that have expired can be retrieved.
     ///
     /// This test checks that the following:
     /// - all expired and not reclaimed leases are returned, regardless if
@@ -295,7 +295,7 @@ public:
     ///   expired
     void testGetDeclinedLeases4();
 
-    /// @brief Checks that declined DHCPv6 leases that have expired can be retrieved.
+    /// @brief Checks that declined IPv6 leases that have expired can be retrieved.
     ///
     /// This test checks that the following:
     /// - all expired and not reclaimed leases are returned, regardless if
@@ -305,7 +305,7 @@ public:
     ///   expired
     void testGetDeclinedLeases6();
 
-    /// @brief Checks that selected expired-reclaimed DHCPv6 leases
+    /// @brief Checks that selected expired-reclaimed IPv6 leases
     /// are removed.
     ///
     /// This creates a number of DHCPv6 leases and marks some of them
@@ -313,7 +313,7 @@ public:
     /// leases can be removed.
     void testDeleteExpiredReclaimedLeases6();
 
-    /// @brief Checks that selected expired-reclaimed DHCPv4 leases
+    /// @brief Checks that selected expired-reclaimed IPv4 leases
     /// are removed.
     ///
     /// This creates a number of DHCPv4 leases and marks some of them

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -994,14 +994,12 @@ TEST_F(MemfileLeaseMgrTest, versionCheck) {
 
 // Checks that declined IPv4 leases can be returned correctly.
 TEST_F(MemfileLeaseMgrTest, getDeclined4) {
-
     startBackend(V4);
     testGetDeclinedLeases4();
 }
 
 // Checks that declined IPv6 leases can be returned correctly.
 TEST_F(MemfileLeaseMgrTest, getDeclined6) {
-
     startBackend(V6);
     testGetDeclinedLeases6();
 }