From: Tomek Mrugalski Date: Fri, 2 Oct 2015 18:35:02 +0000 (+0200) Subject: [3985] Several minor tweaks, docs updated. X-Git-Tag: trac3874_base~17^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=52de9ac39f414bbe4fa6513621d20f89a2c3be2d;p=thirdparty%2Fkea.git [3985] Several minor tweaks, docs updated. --- diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 084cb47a3c..39b5a2742c 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -2676,8 +2676,30 @@ should include options from the isc option space: + + +
+ Duplicate Addresses (DECLINE support) + + @todo: Full text will be added as part of #3990. + + + + 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. + +
+
Statistics in DHCPv6 server diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index 88baa205d8..04e82f1824 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/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). /// diff --git a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc index 04f0ebbb84..dedcad1024 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc +++ b/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. diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 8905326e66..46db622ac7 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/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 (...) { diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index bd4fe16487..2d3f84231d 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h +++ b/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); diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 03bfe56f9b..9a3d8de932 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/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 diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h index 1318e34622..d7c3a407e3 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h +++ b/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 diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc index 6c3e33293a..2f424d1c9f 100644 --- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc +++ b/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(); }