From: Thomas Markwalder Date: Tue, 13 Apr 2021 18:25:18 +0000 (-0400) Subject: [#1735] Addressed review comments. X-Git-Tag: Kea-1.9.7~71 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b3872cb5302bc1e48e90f0b2ef713224be0b5ebd;p=thirdparty%2Fkea.git [#1735] Addressed review comments. Cosmetics, typos etc... modified: doc/sphinx/arm/hooks-ha.rst src/hooks/dhcp/high_availability/ha_config.h src/hooks/dhcp/high_availability/ha_messages.mes src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc src/hooks/dhcp/high_availability/tests/ha_test.cc src/hooks/dhcp/high_availability/tests/ha_test.h src/lib/dhcpsrv/cfg_multi_threading.cc --- diff --git a/doc/sphinx/arm/hooks-ha.rst b/doc/sphinx/arm/hooks-ha.rst index dd60c86d5e..daf8c9657d 100644 --- a/doc/sphinx/arm/hooks-ha.rst +++ b/doc/sphinx/arm/hooks-ha.rst @@ -1453,7 +1453,7 @@ capability: - ``enable-multi-threading`` - enables or disables HA+MT -- ``http-dedicated-listener`` - enables or disabled the creation of an dedicated HTTP +- ``http-dedicated-listener`` - enables or disabled the creation of a dedicated HTTP listener the server will use to receive HA messages from its peers. - ``http-listener-threads`` - maximum number of threads the dedicated listener diff --git a/src/hooks/dhcp/high_availability/ha_config.h b/src/hooks/dhcp/high_availability/ha_config.h index a78355f849..fede5a90f6 100644 --- a/src/hooks/dhcp/high_availability/ha_config.h +++ b/src/hooks/dhcp/high_availability/ha_config.h @@ -543,7 +543,7 @@ public: return (http_dedicated_listener_); } - /// @brief Sets whether or not the server is configured use its one HTTP + /// @brief Sets whether or not the server is configured to use its own HTTP /// listener. /// /// @param http_dedicated_listener flag that enables the use of a dedicated @@ -554,28 +554,28 @@ public: /// @brief Fetches the number of threads the HTTP listener should use. /// - /// @return number of the listener is configured to use. + /// @return number of threads the listener is configured to use. uint32_t getHttpListenerThreads() { return (http_listener_threads_); } /// @brief Sets the number of threads the HTTP listener should use. /// - /// @return number of the listener should use. + /// @return number of threads the listener should use. void setHttpListenerThreads(uint32_t http_listener_threads) { http_listener_threads_ = http_listener_threads; } /// @brief Fetches the number of threads the HTTP Client should use. /// - /// @return number of the client is configured to use. + /// @return number of threads the client is configured to use. uint32_t getHttpClientThreads() { return (http_client_threads_); } /// @brief Sets the number of threads the HTTP client should use. /// - /// @return number of the client should use. + /// @return number of threads the client should use. void setHttpClientThreads(uint32_t http_client_threads) { http_client_threads_ = http_client_threads; } @@ -630,9 +630,9 @@ public: /// @brief Validates configuration. /// /// In addition to sanity checking the configuration, it will - /// check HA+MTi configuration against Core multi-threading + /// check HA+MT configuration against Core multi-threading /// configuration add adjust HA+MT values as follows: - /// 1. if DHCP multi-threading is disabled, HA+MT will be disabled. + /// 1. If DHCP multi-threading is disabled, HA+MT will be disabled. /// 2. If http-listener-threads is 0, it will be replaced with /// the number of DHCP threads /// 3. If http-client-threads is 0, it will be replaced with diff --git a/src/hooks/dhcp/high_availability/ha_messages.mes b/src/hooks/dhcp/high_availability/ha_messages.mes index 67cb4a7b8d..69514d22d8 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.mes +++ b/src/hooks/dhcp/high_availability/ha_messages.mes @@ -161,7 +161,7 @@ multi-threading while Kea global configuration has multi-threading disabled. % HA_CONFIG_SYSTEM_MT_UNSUPPORTED HA multi-threading has been disabled, auto-detection of thread support reports 0 This informational message is issued when HA multi-threading configuration has specified auto-detection for the number of threads to use and the system -reports the number of concurrent threads as 0. If you now your system can +reports the number of concurrent threads as 0. If you know your system can support multiple threads, then you may override this condition by specifying explicit values for http-listener-threads and http-client-threads. diff --git a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc index 42c363d310..4a95c9e363 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_config_unittest.cc @@ -1379,7 +1379,7 @@ TEST_F(HAConfigTest, pausingToString) { } -// Verifies permuations of HA+MT configuration. +// Verifies permutaions of HA+MT configuration. TEST_F(HAConfigTest, multiThreadingPermutations) { // Structure describing a test scenario. @@ -1389,7 +1389,7 @@ TEST_F(HAConfigTest, multiThreadingPermutations) { bool dhcp_mt_enabled_; // True if DHCP multi-threading is enabled. uint32_t dhcp_threads_; // Value of DHCP thread-pool-size. bool exp_ha_mt_enabled_; // If HA+MT should be enabled - bool exp_listener_; // if HA+MT should use dedicated listener. + bool exp_listener_; // If HA+MT should use dedicated listener. uint32_t exp_listener_threads_; // Expected number of listener threads. uint32_t exp_client_threads_; // Expected number of client threads. }; @@ -1416,22 +1416,22 @@ TEST_F(HAConfigTest, multiThreadingPermutations) { !ha_mt, !listener, 0, 0 }, { - "3 dhcp mt disabled, mt enabled", + "3 dhcp mt disabled, ha mt enabled", makeHAMtJson(ha_mt, listener, 0, 0), !dhcp_mt, 4, !ha_mt, !listener, 0, 0 }, { - "4 dhcp mt enabled, mt enabled, listener disabled", + "4 dhcp mt enabled, ha mt enabled, listener disabled", makeHAMtJson(ha_mt, !listener, 0, 0), dhcp_mt, 4, ha_mt, !listener, 4, 4 }, { - "5 dhcp mt enabled, mt enabled, listener enabled", - makeHAMtJson(ha_mt, !listener, 0, 0), + "5 dhcp mt enabled, ha mt enabled, listener enabled", + makeHAMtJson(ha_mt, listener, 0, 0), dhcp_mt, 4, - ha_mt, !listener, 4, 4 + ha_mt, listener, 4, 4 }, { "6 explicit DHCP threads, explicit thread values", diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.cc b/src/hooks/dhcp/high_availability/tests/ha_test.cc index 76871c4381..7dd35bbe33 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_test.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_test.cc @@ -340,7 +340,7 @@ HATest::createQuery6(const std::string& duid_text) const { return (query6); } -void +void HATest::setDHCPMultiThreadingConfig(bool enabled, uint32_t thread_pool_size, uint32_t packet_queue_size) { ElementPtr mt_config = Element::createMap(); @@ -357,17 +357,15 @@ HATest::makeHAMtJson(bool enable_multi_threading, bool http_dedicated_listener, uint32_t http_listener_threads, uint32_t http_client_threads) { std::stringstream ss; ss << "\"multi-threading\": {" - << " \"enable-multi-threading\": " + << " \"enable-multi-threading\": " << (enable_multi_threading ? "true" : "false") << "," - << " \"http-dedicated-listener\": " + << " \"http-dedicated-listener\": " << (http_dedicated_listener ? "true" : "false") << "," << " \"http-listener-threads\": " << http_listener_threads << "," << " \"http-client-threads\": " << http_client_threads << "}"; return (ss.str()); } - - } // end of namespace isc::ha::test } // end of namespace isc::ha } // end of namespace isc diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.h b/src/hooks/dhcp/high_availability/tests/ha_test.h index 83d889a30f..8c244c8af1 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_test.h +++ b/src/hooks/dhcp/high_availability/tests/ha_test.h @@ -225,9 +225,9 @@ public: const uint8_t duid_seed, const uint16_t elapsed_time) const; - /// @brief Sets the DHCP mulit-threading configuration in staging SrvConfig. + /// @brief Sets the DHCP multi-threading configuration in staging SrvConfig. /// - /// @param enable_multi_threading value that maps to enable-multi-threading. + /// @param enable_multi_threading value that maps to enable-multi-threading. /// @param thread_pool_size value that maps to thread-pool-size. /// @param queue_size value that maps to queue-size. void setDHCPMultiThreadingConfig(bool enable_multi_threading, @@ -241,7 +241,7 @@ public: /// ``` /// "multi-threading" { /// "enable-multi-threading": , - /// "dedicated-http-listener": , + /// "dedicated-http-listener": , /// "http-listener-threads": , /// "http-client-threads": /// }" diff --git a/src/lib/dhcpsrv/cfg_multi_threading.cc b/src/lib/dhcpsrv/cfg_multi_threading.cc index e74a29875d..c31e6a720f 100644 --- a/src/lib/dhcpsrv/cfg_multi_threading.cc +++ b/src/lib/dhcpsrv/cfg_multi_threading.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -19,30 +19,32 @@ namespace dhcp { void CfgMultiThreading::apply(ConstElementPtr value) { - bool enabled = false; - uint32_t thread_count = 0; - uint32_t queue_size = 0; - CfgMultiThreading::extract(value, enabled, thread_count, queue_size); - MultiThreadingMgr::instance().apply(enabled, thread_count, queue_size); + bool enabled = false; + uint32_t thread_count = 0; + uint32_t queue_size = 0; + CfgMultiThreading::extract(value, enabled, thread_count, queue_size); + MultiThreadingMgr::instance().apply(enabled, thread_count, queue_size); } void CfgMultiThreading::extract(ConstElementPtr value, bool& enabled, uint32_t& thread_count, uint32_t& queue_size) { - enabled = false; - thread_count = 0; - queue_size = 0; - if (value) { - if (value->get("enable-multi-threading")) { - enabled = SimpleParser::getBoolean(value, "enable-multi-threading"); - } - if (value->get("thread-pool-size")) { - thread_count = SimpleParser::getInteger(value, "thread-pool-size"); - } - if (value->get("packet-queue-size")) { - queue_size = SimpleParser::getInteger(value, "packet-queue-size"); - } + enabled = false; + thread_count = 0; + queue_size = 0; + if (value) { + if (value->get("enable-multi-threading")) { + enabled = SimpleParser::getBoolean(value, "enable-multi-threading"); } + + if (value->get("thread-pool-size")) { + thread_count = SimpleParser::getInteger(value, "thread-pool-size"); + } + + if (value->get("packet-queue-size")) { + queue_size = SimpleParser::getInteger(value, "packet-queue-size"); + } + } } } // namespace dhcp