From: Razvan Becheriu Date: Wed, 21 Jun 2023 08:17:26 +0000 (+0300) Subject: [#2883] addressed review comments X-Git-Tag: Kea-2.4.0~164 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=029ba4d5484495ca55a003ac0ce61d279887fa6e;p=thirdparty%2Fkea.git [#2883] addressed review comments --- diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index 928d69fb21..23a0505c8a 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/doc/sphinx/arm/dhcp4-srv.rst @@ -5764,9 +5764,9 @@ file and in the host database backend, via :ischooklib:`libdhcp_host_cmds.so`. Setting ``ip-reservations-unique`` to ``false`` when using memfile, MySQL or PostgreSQL is supported. This setting is not supported when using Host Cache (see :ref:`hooks-host-cache`), and the RADIUS backend -(see :ref:`hooks-radius`). These reservation backends simply do not support multiple reservations for the +(see :ref:`hooks-radius`). These reservation backends simply do not support multiple reservations for the same IP. If either of these hooks are loaded and ``ip-reservations-unique`` is set to ``false``, then a -configuration error will be emitted and the server will fail to start. +configuration error will be emitted and the server will fail to start. .. note:: @@ -6881,7 +6881,7 @@ The DHCPv4 server supports the following statistics: | subnet[id].total-addresses | integer | Total number of addresses | | | | available for DHCPv4 management | | | | for a given subnet; in other | - | | | words, this is the sum of all | + | | | words, this is the count of all | | | | addresses in all configured pools. | | | | This statistic changes only during | | | | configuration updates. It does not | @@ -6896,7 +6896,7 @@ The DHCPv4 server supports the following statistics: | subnet[id].pool[pid].total-addresses | integer | Total number of addresses | | | | available for DHCPv4 management | | | | for a given subnet pool; in other | - | | | words, this is the sum of all | + | | | words, this is the count of all | | | | addresses in configured subnet | | | | pool. This statistic changes only | | | | during configuration updates. It | diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst index 81acaf3673..1af7b60297 100644 --- a/doc/sphinx/arm/dhcp6-srv.rst +++ b/doc/sphinx/arm/dhcp6-srv.rst @@ -6589,7 +6589,7 @@ The DHCPv6 server supports the following statistics: | subnet[id].total-nas | big integer | Total number of NA addresses | | | | available for DHCPv6 management | | | | for a given subnet; in other | - | | | words, this is the sum of all | + | | | words, this is the count of all | | | | addresses in all configured pools. | | | | This statistic changes only during | | | | configuration changes. It does not | @@ -6604,7 +6604,7 @@ The DHCPv6 server supports the following statistics: | subnet[id].pool[pid].total-nas | big integer | Total number of NA addresses | | | | available for DHCPv6 management | | | | for a given subnet pool; in other | - | | | words, this is the sum of all | + | | | words, this is the count of all | | | | addresses in configured subnet | | | | pool. This statistic changes only | | | | during configuration changes. It | @@ -6682,7 +6682,7 @@ The DHCPv6 server supports the following statistics: | subnet[id].total-pds | big integer | Total number of PD prefixes | | | | available for DHCPv6 management | | | | for a given subnet; in other | - | | | words, this is the sum of all | + | | | words, this is the count of all | | | | prefixes in all configured pools. | | | | This statistic changes only during | | | | configuration changes. Note it | @@ -6697,7 +6697,7 @@ The DHCPv6 server supports the following statistics: | subnet[id].pd-pool[pid].total-pds | big integer | Total number of PD prefixes | | | | available for DHCPv6 management | | | | for a given subnet pool; in other | - | | | words, this is the sum of all | + | | | words, this is the count of all | | | | prefixes in configured subnet | | | | pd-pool. This statistic changes | | | | only during configuration changes. | diff --git a/doc/sphinx/arm/hooks-stat-cmds.rst b/doc/sphinx/arm/hooks-stat-cmds.rst index 475caf06ad..fce8cfa8e0 100644 --- a/doc/sphinx/arm/hooks-stat-cmds.rst +++ b/doc/sphinx/arm/hooks-stat-cmds.rst @@ -153,18 +153,19 @@ The response to either command will contain three elements: the element "result-set", which is patterned after SQL statement responses: - - ``columns`` - a list of text column labels. The columns returned - for DHCPv4 are: + - ``columns`` - a list of text column labels. + + The columns returned for DHCPv4 are: - ``subnet-id`` - the ID of the subnet. - ``total-addresses`` - the total number of addresses available for DHCPv4 management in the subnet. In other words, this is the - sum of all addresses in all the configured pools in the subnet. + count of all addresses in all the configured pools in the subnet. - - ``cumulative-assigned-addresses`` - the cumulative number of addresses - in the subnet that have been assigned to a client by the server - since it started. + - ``cumulative-assigned-addresses`` - the cumulative number of addresses + in the subnet that have been assigned to a client by the server + since it started. - ``assigned-addresses`` - the number of addresses in the subnet that are currently assigned to a client. @@ -172,18 +173,18 @@ The response to either command will contain three elements: - ``declined-addresses`` - the number of addresses in the subnet that are currently declined and are thus unavailable for assignment. - - The columns returned for DHCPv6 are: + The columns returned for DHCPv6 are: - ``subnet-id`` - the ID of the subnet. - ``total-nas`` - the number of NA addresses available for DHCPv6 - management in the subnet. In other words, this is the sum of + management in the subnet. In other words, this is the count of all the NA addresses in all the configured NA pools in the subnet. - - ``cumulative-assigned-nas`` - the cumulative number of NA addresses - in the subnet that have been assigned to a client by the server - since it started. + - ``cumulative-assigned-nas`` - the cumulative number of NA addresses + in the subnet that have been assigned to a client by the server + since it started. - ``assigned-nas`` - the number of NA addresses in the subnet that are currently assigned to a client. @@ -192,12 +193,12 @@ The response to either command will contain three elements: declined and are thus unavailable for assignment. - ``total-pds`` - the total number of PD prefixes available of DHCPv6 - management in the subnet. In other words, this is the sum of + management in the subnet. In other words, this is the count of all prefixes in all the configured prefix pools in the subnet. - - ``cumulative-assigned-pds`` - the cumulative number of PD prefixes - in the subnet that have been assigned to a client by the server - since it started. + - ``cumulative-assigned-pds`` - the cumulative number of PD prefixes + in the subnet that have been assigned to a client by the server + since it started. - ``assigned-pds`` - the number of PD prefixes in the subnet that are currently assigned to a client. diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index 46f4d8069c..fd927c3b89 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -88,8 +88,10 @@ bool testStatistics(const std::string& stat_name, const int64_t exp_value, } } } + } catch (const std::exception& e) { + ADD_FAILURE() << "Uncaught exception " << e.what(); } catch (...) { - ; + ADD_FAILURE() << "Unknown exception"; } return (false); } @@ -102,8 +104,10 @@ int64_t getStatistics(const std::string& stat_name, const SubnetID subnet_id) { if (observation) { return (observation->getInteger().first); } + } catch (const std::exception& e) { + ADD_FAILURE() << "Uncaught exception " << e.what(); } catch (...) { - ; + ADD_FAILURE() << "Unknown exception"; } return (0); } diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index 9ad9238e0d..756b92c7e5 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h @@ -38,6 +38,10 @@ namespace test { /// This function may be used in many allocation tests and there's no /// single base class for it. @todo consider moving it src/lib/util. /// +/// It checks statistics by name, either the global version when the subnet ID +/// is SUBNET_ID_UNUSED or the subnet statistic, including the pool 0 respective +/// statistic. +/// /// @param stat_name Statistic name. /// @param exp_value Expected value. /// @param subnet_id subnet_id of the desired subnet, if not zero @@ -58,13 +62,16 @@ bool testStatistics(const std::string& stat_name, /// /// @param stat_name Statistic name. /// @param subnet_id subnet_id of the desired subnet, if not zero. +/// /// @return the value held by the statistic manager or zero. -int64_t getStatistics(const std::string& stat_name, const SubnetID subnet_id = SUBNET_ID_UNUSED); +int64_t getStatistics(const std::string& stat_name, + const SubnetID subnet_id = SUBNET_ID_UNUSED); /// @brief IterativeAllocator with internal methods exposed class NakedIterativeAllocator : public IterativeAllocator { public: /// @brief constructor + /// /// @param type pool types that will be iterated through NakedIterativeAllocator(Lease::Type type, const WeakSubnetPtr& subnet) : IterativeAllocator(type, subnet) { @@ -78,6 +85,7 @@ public: class NakedAllocEngine : public AllocEngine { public: /// @brief the sole constructor + /// /// @param attempts number of lease selection attempts before giving up NakedAllocEngine(unsigned int attempts) : AllocEngine(attempts) { @@ -87,8 +95,10 @@ public: using AllocEngine::updateLease4ExtendedInfo; /// @brief Wrapper method for invoking AllocEngine4::updateLease4ExtendedInfo(). + /// /// @param lease lease to update /// @param ctx current packet processing context + /// /// @return true if extended information was changed bool callUpdateLease4ExtendedInfo(const Lease4Ptr& lease, AllocEngine::ClientContext4& ctx) const { @@ -96,13 +106,13 @@ public: } /// @brief Wrapper method for invoking AllocEngine6::updateLease6ExtendedInfo(). + /// /// @param lease lease to update /// @param ctx current packet processing context void callUpdateLease6ExtendedInfo(const Lease6Ptr& lease, AllocEngine::ClientContext6& ctx) const { - return (updateLease6ExtendedInfo(lease, ctx)); + updateLease6ExtendedInfo(lease, ctx); } - }; /// @brief Used in Allocation Engine tests for IPv6 @@ -175,6 +185,7 @@ public: /// /// @param col collection of leases (zero or one leases allowed) /// @throw MultipleRecords if there is more than one lease + /// /// @return Lease6 pointer (or NULL if collection was empty) Lease6Ptr expectOneLease(const Lease6Collection& col) { if (col.size() > 1) { @@ -293,6 +304,7 @@ public: /// @param hint address to be used as a hint /// @param fake true - this is fake allocation (SOLICIT) /// @param in_pool specifies whether the lease is expected to be in pool + /// /// @return allocated lease (or NULL) Lease6Ptr simpleAlloc6Test(const Pool6Ptr& pool, const asiolink::IOAddress& hint, @@ -310,6 +322,7 @@ public: /// @param exp_preferred expected lease preferred lifetime /// @param exp_valid expected lease valid lifetime /// @param class_def class definition to add to the context + /// /// @return allocated lease (or NULL) Lease6Ptr simpleAlloc6Test(const Pool6Ptr& pool, const asiolink::IOAddress& hint, @@ -328,6 +341,7 @@ public: /// @param hint address to be used as a hint /// @param fake true - this is fake allocation (SOLICIT) /// @param in_pool specifies whether the lease is expected to be in pool + /// /// @return allocated lease (or NULL) Lease6Ptr simpleAlloc6Test(const Pool6Ptr& pool, const DuidPtr& duid, @@ -347,6 +361,7 @@ public: /// @param in_pool specifies whether the lease is expected to be in pool /// @param hint_prefix_length The hint prefix length that the client /// provided. + /// /// @return allocated lease(s) (may be empty) Lease6Collection allocateTest(AllocEngine& engine, const Pool6Ptr& pool, @@ -365,6 +380,7 @@ public: /// @param hints address to be used as a hint /// @param in_subnet whether the lease is expected to be in subnet /// @param in_pool specifies whether the lease is expected to be in pool + /// /// @return allocated lease(s) (may be empty) Lease6Collection renewTest(AllocEngine& engine, const Pool6Ptr& pool, @@ -418,6 +434,8 @@ public: /// @param addr address of the lease /// @param probation_period expressed in seconds /// @param expired number of seconds when the lease will expire + /// + /// @return generated lease Lease6Ptr generateDeclinedLease(const std::string& addr, time_t probation_period, int32_t expired); @@ -439,6 +457,7 @@ public: /// @param type specifies reservation type /// @param addr specifies reserved address or prefix /// @param prefix_len prefix length (should be 128 for addresses) + /// /// @return created Host object. HostPtr createHost6(bool add_to_host_mgr, IPv6Resrv::Type type, @@ -476,6 +495,7 @@ public: /// @param hwaddr hardware address to be reserved to /// @param addr specifies reserved address or prefix /// @param prefix_len prefix length (should be 128 for addresses) + /// /// @return created Host object. HostPtr createHost6HWAddr(bool add_to_host_mgr, IPv6Resrv::Type type, @@ -605,6 +625,8 @@ public: /// @param addr address of the lease /// @param probation_period expressed in seconds /// @param expired number of seconds when the lease will expire + /// + /// @return generated lease Lease4Ptr generateDeclinedLease(const std::string& addr, time_t probation_period, int32_t expired); @@ -616,6 +638,7 @@ public: void initSubnet(const asiolink::IOAddress& pool_start, const asiolink::IOAddress& pool_end); + /// @brief Default constructor virtual ~AllocEngine4Test() { factory_.destroy(); }