From: Razvan Becheriu Date: Wed, 8 Apr 2020 21:42:32 +0000 (+0300) Subject: [#893] addressed review comments X-Git-Tag: Kea-1.7.7~64 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ea9a9fa460de8619a38c11bbde68aeec271a4494;p=thirdparty%2Fkea.git [#893] addressed review comments --- diff --git a/doc/examples/kea4/all-keys.json b/doc/examples/kea4/all-keys.json index dc60e33605..38f394436c 100644 --- a/doc/examples/kea4/all-keys.json +++ b/doc/examples/kea4/all-keys.json @@ -523,21 +523,25 @@ // When the maximum count is 0 the maximum age (in seconds) applies. "statistic-default-sample-age": 60, - // By default Kea processes packets on a single thread (default 'false' - // value for this option). To enable multi-threading, this option can be - // set ('true' value). - "enable-multi-threading": false, - - // When multi-threading is enabled, Kea will process packets on a number of - // multiple threads configurable through this option. The value must be a - // positive integer (0 means auto detect). - "packet-thread-pool-size": 0, - - // When multi-threading is enabled, Kea will read packets from the interface - // and append a working item to the thread pool. This option configures the - // maximum number of items that can be queued for each processing thread. - // The value must be a positive integer (0 means unlimited). - "packet-thread-queue-size": 0, + // Multi-threading parameters. + "multi-threading": { + // By default Kea processes packets on a single thread (default + // 'false' value for this option). To enable multi-threading, this + // option can be set ('true' value). + "enable-multi-threading": false, + + // When multi-threading is enabled, Kea will process packets on a + // number of multiple threads configurable through this option. The + // value must be a positive integer (0 means auto detect). + "thread-pool-size": 0, + + // When multi-threading is enabled, Kea will read packets from the + // interface and append a working item to the thread pool. This + // option configures the maximum number of items that can be queued + // for each processing thread. The value must be a positive integer + // (0 means unlimited). + "packet-queue-size": 0 + }, // Governs how the Kea DHCPv4 server should deal with the invalid // data received from the client. diff --git a/doc/examples/kea6/all-keys.json b/doc/examples/kea6/all-keys.json index 5f395fe0f9..a4d6ac35f3 100644 --- a/doc/examples/kea6/all-keys.json +++ b/doc/examples/kea6/all-keys.json @@ -464,21 +464,25 @@ // When the maximum count is 0 the maximum age (in seconds) applies. "statistic-default-sample-age": 60, - // By default Kea processes packets on a single thread (default 'false' - // value for this option). To enable multi-threading, this option can be - // set ('true' value). - "enable-multi-threading": false, - - // When multi-threading is enabled, Kea will process packets on a number of - // multiple threads configurable through this option. The value must be a - // positive integer (0 means auto detect). - "packet-thread-pool-size": 0, - - // When multi-threading is enabled, Kea will read packets from the interface - // and append a working item to the thread pool. This option configures the - // maximum number of items that can be queued for each processing thread. - // The value must be a positive integer (0 means unlimited). - "packet-thread-queue-size": 0, + // Multi-threading parameters. + "multi-threading": { + // By default Kea processes packets on a single thread (default + // 'false' value for this option). To enable multi-threading, this + // option can be set ('true' value). + "enable-multi-threading": false, + + // When multi-threading is enabled, Kea will process packets on a + // number of multiple threads configurable through this option. The + // value must be a positive integer (0 means auto detect). + "thread-pool-size": 0, + + // When multi-threading is enabled, Kea will read packets from the + // interface and append a working item to the thread pool. This + // option configures the maximum number of items that can be queued + // for each processing thread. The value must be a positive integer + // (0 means unlimited). + "packet-queue-size": 0 + }, // Governs how the Kea DHCPv6 server should deal with the invalid // data received from the client. diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index fc2ded57cc..8dd405b935 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/doc/sphinx/arm/dhcp4-srv.rst @@ -3743,27 +3743,30 @@ Multi-threading settings ------------------------ The Kea server can be configured to process packets in parallel using multiple -threads. Related setting for this feature are: +threads. These settings can be found under ``multi-threading`` structure and are +represented by: - ``enable-multi-threading`` - use multiple threads to process packets in parallel -- ``packet-thread-pool-size`` - specify the number of threads to process - packets in parallel. Supported values are: 0 (autodetect), any positive - number sets number of threads explicitly. +- ``thread-pool-size`` - specify the number of threads to process packets in + parallel. Supported values are: 0 (auto detect), any positive number sets + number of threads explicitly. -- ``packet-thread-queue-size`` - specify the size of the queue used by each - thread to process packets. Supported values are: 0 (unlimited), any positive - number sets size explicitly. +- ``packet-queue-size`` - specify the size of the queue used by each thread to + process packets. Supported values are: 0 (unlimited), any positive number + sets size explicitly. An example configuration that sets these parameter looks as follows: :: "Dhcp4": { - "enable-multi-threading": true, - "packet-thread-pool-size": 4, - "packet-thread-queue-size": 16, + "multi-threading": { + "enable-multi-threading": true, + "thread-pool-size": 4, + "packet-queue-size": 16 + } ... } diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst index f2db5d9d90..69d34db768 100644 --- a/doc/sphinx/arm/dhcp6-srv.rst +++ b/doc/sphinx/arm/dhcp6-srv.rst @@ -3255,27 +3255,30 @@ Multi-threading settings ------------------------ The Kea server can be configured to process packets in parallel using multiple -threads. Related setting for this feature are: +threads. These settings can be found under ``multi-threading`` structure and are +represented by: - ``enable-multi-threading`` - use multiple threads to process packets in parallel -- ``packet-thread-pool-size`` - specify the number of threads to process - packets in parallel. Supported values are: 0 (autodetect), any positive - number sets number of threads explicitly. +- ``thread-pool-size`` - specify the number of threads to process packets in + parallel. Supported values are: 0 (auto detect), any positive number sets + number of threads explicitly. -- ``packet-thread-queue-size`` - specify the size of the queue used by each - thread to process packets. Supported values are: 0 (unlimited), any positive - number sets size explicitly. +- ``packet-queue-size`` - specify the size of the queue used by each thread to + process packets. Supported values are: 0 (unlimited), any positive number + sets size explicitly. An example configuration that sets these parameter looks as follows: :: "Dhcp6": { - "enable-multi-threading": true, - "packet-thread-pool-size": 4, - "packet-thread-queue-size": 16, + "multi-threading": { + "enable-multi-threading": true, + "thread-pool-size": 4, + "packet-queue-size": 16 + } ... } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 25554103a6..055ed5024c 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -169,21 +169,6 @@ ControlledDhcpv4Srv::loadConfigFile(const std::string& file_name) { "processCommand(\"config-set\", json)"); } - // command line parameters overwrite file and database configuration - bool enabled = false; - if (Dhcpv4Srv::srv_thread_count_ >= 0) { - enabled = true; - } - if (enabled) { - CfgMgr::instance().getCurrentCfg()->setPktThreadPoolSize(Dhcpv4Srv::srv_thread_count_); - CfgMgr::instance().getCurrentCfg()->setPktThreadQueueSize(0); - LOG_FATAL(dhcp4_logger, DHCP4_MULTI_THREADING_WARNING); - } else { - enabled = CfgMgr::instance().getCurrentCfg()->getEnableMultiThreading(); - } - MultiThreadingMgr::instance().apply(enabled, - CfgMgr::instance().getCurrentCfg()->getPktThreadPoolSize()); - // Now check is the returned result is successful (rcode=0) or not // (see @ref isc::config::parseAnswer). int rcode; @@ -206,8 +191,8 @@ ControlledDhcpv4Srv::loadConfigFile(const std::string& file_name) { LOG_WARN(dhcp4_logger, DHCP4_MULTI_THREADING_INFO) .arg(MultiThreadingMgr::instance().getMode() ? "yes" : "no") - .arg(MultiThreadingMgr::instance().getPktThreadPoolSize()) - .arg(CfgMgr::instance().getCurrentCfg()->getPktThreadQueueSize()); + .arg(MultiThreadingMgr::instance().getThreadPoolSize()) + .arg(MultiThreadingMgr::instance().getThreadQueueSize()); return (result); } @@ -891,6 +876,32 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { // operation. } + // Configure multi threading + try { + data::ConstElementPtr mt; + // command line parameters overwrite file and database configuration + bool enabled = false; + uint32_t thread_pool_size = 0; + uint32_t thread_queue_size = 0; + if (Dhcpv4Srv::srv_thread_count_ >= 0) { + enabled = true; + } + if (enabled) { + thread_pool_size = Dhcpv4Srv::srv_thread_count_; + LOG_FATAL(dhcp4_logger, DHCP4_MULTI_THREADING_WARNING); + } else { + enabled = false; // todo parse + thread_pool_size = 0; // todo parse + thread_queue_size = 0; // todo parse + } + MultiThreadingMgr::instance().apply(enabled, thread_pool_size, + thread_queue_size); + } catch (const std::exception& ex) { + err << "Error applying multi threading settings: " + << ex.what(); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); + } + return (answer); } diff --git a/src/bin/dhcp4/dhcp4_lexer.ll b/src/bin/dhcp4/dhcp4_lexer.ll index 688325daff..75f8c633e7 100644 --- a/src/bin/dhcp4/dhcp4_lexer.ll +++ b/src/bin/dhcp4/dhcp4_lexer.ll @@ -866,6 +866,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} case isc::dhcp::Parser4Context::CLIENT_CLASSES: case isc::dhcp::Parser4Context::CONTROL_SOCKET: case isc::dhcp::Parser4Context::DHCP_QUEUE_CONTROL: + case isc::dhcp::Parser4Context::DHCP_MULTI_THREADING: case isc::dhcp::Parser4Context::LOGGERS: case isc::dhcp::Parser4Context::DHCP_DDNS: return isc::dhcp::Dhcp4Parser::make_USER_CONTEXT(driver.loc_); @@ -887,6 +888,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} case isc::dhcp::Parser4Context::CLIENT_CLASSES: case isc::dhcp::Parser4Context::CONTROL_SOCKET: case isc::dhcp::Parser4Context::DHCP_QUEUE_CONTROL: + case isc::dhcp::Parser4Context::DHCP_MULTI_THREADING: case isc::dhcp::Parser4Context::LOGGERS: case isc::dhcp::Parser4Context::DHCP_DDNS: return isc::dhcp::Dhcp4Parser::make_COMMENT(driver.loc_); @@ -1418,30 +1420,39 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} } } -\"enable-multi-threading\" { +\"multi-threading\" { switch(driver.ctx_) { case isc::dhcp::Parser4Context::DHCP4: + return isc::dhcp::Dhcp4Parser::make_DHCP_MULTI_THREADING(driver.loc_); + default: + return isc::dhcp::Dhcp4Parser::make_STRING("multi-threading", driver.loc_); + } +} + +\"enable-multi-threading\" { + switch(driver.ctx_) { + case isc::dhcp::Parser4Context::DHCP_MULTI_THREADING: return isc::dhcp::Dhcp4Parser::make_ENABLE_MULTI_THREADING(driver.loc_); default: return isc::dhcp::Dhcp4Parser::make_STRING("enable-multi-threading", driver.loc_); } } -\"packet-thread-pool-size\" { +\"thread-pool-size\" { switch(driver.ctx_) { - case isc::dhcp::Parser4Context::DHCP4: - return isc::dhcp::Dhcp4Parser::make_PACKET_THREAD_POOL_SIZE(driver.loc_); + case isc::dhcp::Parser4Context::DHCP_MULTI_THREADING: + return isc::dhcp::Dhcp4Parser::make_THREAD_POOL_SIZE(driver.loc_); default: - return isc::dhcp::Dhcp4Parser::make_STRING("packet-thread-pool-size", driver.loc_); + return isc::dhcp::Dhcp4Parser::make_STRING("thread-pool-size", driver.loc_); } } -\"packet-thread-queue-size\" { +\"packet-queue-size\" { switch(driver.ctx_) { - case isc::dhcp::Parser4Context::DHCP4: - return isc::dhcp::Dhcp4Parser::make_PACKET_THREAD_QUEUE_SIZE(driver.loc_); + case isc::dhcp::Parser4Context::DHCP_MULTI_THREADING: + return isc::dhcp::Dhcp4Parser::make_PACKET_QUEUE_SIZE(driver.loc_); default: - return isc::dhcp::Dhcp4Parser::make_STRING("packet-thread-queue-size", driver.loc_); + return isc::dhcp::Dhcp4Parser::make_STRING("packet-queue-size", driver.loc_); } } diff --git a/src/bin/dhcp4/dhcp4_parser.yy b/src/bin/dhcp4/dhcp4_parser.yy index fbc2e1a52a..ba6ab092ae 100644 --- a/src/bin/dhcp4/dhcp4_parser.yy +++ b/src/bin/dhcp4/dhcp4_parser.yy @@ -187,9 +187,10 @@ using namespace std; DHCP4O6_PORT "dhcp4o6-port" + DHCP_MULTI_THREADING "multi-threading" ENABLE_MULTI_THREADING "enable-multi-threading" - PACKET_THREAD_POOL_SIZE "packet-thread-pool-size" - PACKET_THREAD_QUEUE_SIZE "packet-thread-queue-size" + THREAD_POOL_SIZE "thread-pool-size" + PACKET_QUEUE_SIZE "packet-queue-size" CONTROL_SOCKET "control-socket" SOCKET_TYPE "socket-type" @@ -510,9 +511,6 @@ global_param: valid_lifetime | store_extended_info | statistic_default_sample_count | statistic_default_sample_age - | enable_multi_threading - | packet_thread_pool_size - | packet_thread_queue_size | unknown_map_entry ; @@ -1036,19 +1034,44 @@ flex_id: FLEX_ID { ctx.stack_.back()->add(flex_id); }; +// --- multi-threading ------------------------------------------------ + +dhcp_multi_threading: DHCP_MULTI_THREADING { + ElementPtr qc(new MapElement(ctx.loc2pos(@1))); + ctx.stack_.back()->set("multi-threading", qc); + ctx.stack_.push_back(qc); + ctx.enter(ctx.DHCP_MULTI_THREADING); +} COLON LCURLY_BRACKET multi_threading_params RCURLY_BRACKET { + // The enable queue parameter is required. + ctx.require("enable-multi-threading", ctx.loc2pos(@4), ctx.loc2pos(@6)); + ctx.stack_.pop_back(); + ctx.leave(); +}; + +multi_threading_params: multi_threading_param + | multi_threading_param COMMA multi_threading_param + ; + +multi_threading_param: enable_multi_threading + | thread_pool_size + | packet_queue_size + | user_context + | comment + ; + enable_multi_threading: ENABLE_MULTI_THREADING COLON BOOLEAN { ElementPtr b(new BoolElement($3, ctx.loc2pos(@3))); ctx.stack_.back()->set("enable-multi-threading", b); }; -packet_thread_pool_size: PACKET_THREAD_POOL_SIZE COLON INTEGER { +thread_pool_size: THREAD_POOL_SIZE COLON INTEGER { ElementPtr prf(new IntElement($3, ctx.loc2pos(@3))); - ctx.stack_.back()->set("packet-thread-pool-size", prf); + ctx.stack_.back()->set("thread-pool-size", prf); }; -packet_thread_queue_size: PACKET_THREAD_QUEUE_SIZE COLON INTEGER { +packet_queue_size: PACKET_QUEUE_SIZE COLON INTEGER { ElementPtr prf(new IntElement($3, ctx.loc2pos(@3))); - ctx.stack_.back()->set("packet-thread-queue-size", prf); + ctx.stack_.back()->set("packet-queue-size", prf); }; hooks_libraries: HOOKS_LIBRARIES { diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 9e8e1d5201..7e2a8b1c48 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -925,7 +925,7 @@ Dhcpv4Srv::run() { } // destroying the thread pool - MultiThreadingMgr::instance().apply(false, 0); + MultiThreadingMgr::instance().apply(false, 0, 0); return (getExitValue()); } @@ -940,11 +940,11 @@ Dhcpv4Srv::run_one() { bool read_pkt = true; // Do not read more packets from socket if there are enough packets to - // be processed in the packet thread pool queue + // be processed in the dhcp thread pool queue // max_queue_size = 0 means no limit - const int max_queue_size = CfgMgr::instance().getCurrentCfg()->getPktThreadQueueSize(); - const int thread_count = MultiThreadingMgr::instance().getPktThreadPoolSize(); - size_t pkt_queue_size = MultiThreadingMgr::instance().getPktThreadPool().count(); + const int max_queue_size = MultiThreadingMgr::instance().getThreadQueueSize(); + const int thread_count = MultiThreadingMgr::instance().getThreadPoolSize(); + size_t pkt_queue_size = MultiThreadingMgr::instance().getThreadPool().count(); if (thread_count && max_queue_size && (pkt_queue_size >= thread_count * max_queue_size)) { read_pkt = false; } @@ -1029,7 +1029,7 @@ Dhcpv4Srv::run_one() { boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv4Srv::processPacketAndSendResponseNoThrow, this, query, rsp)); - MultiThreadingMgr::instance().getPktThreadPool().add(call_back); + MultiThreadingMgr::instance().getThreadPool().add(call_back); } else { processPacketAndSendResponse(query, rsp); } @@ -1347,7 +1347,7 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) { boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv4Srv::sendResponseNoThrow, this, callout_handle, query, rsp)); - MultiThreadingMgr::instance().getPktThreadPool().add(call_back); + MultiThreadingMgr::instance().getThreadPool().add(call_back); } else { processPacketPktSend(callout_handle, query, rsp); processPacketBufferSend(callout_handle, rsp); diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 6f89b9f3c0..ebb8a8e021 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -102,18 +102,6 @@ public: uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port"); cfg->setDhcp4o6Port(dhcp4o6_port); - // Set enable multi threading flag. - bool enable_multi_threading = getBoolean(global, "enable-multi-threading"); - cfg->setEnableMultiThreading(enable_multi_threading); - - // Set packet thread pool size. - uint32_t packet_thread_pool_size = getUint32(global, "packet-thread-pool-size"); - cfg->setPktThreadPoolSize(packet_thread_pool_size); - - // Set packet thread queue size. - uint32_t packet_thread_queue_size = getUint32(global, "packet-thread-queue-size"); - cfg->setPktThreadQueueSize(packet_thread_queue_size); - // Set the global user context. ConstElementPtr user_context = global->get("user-context"); if (user_context) { @@ -420,6 +408,12 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, continue; } + if (config_pair.first == "multi-threading") { + //DHCPMultiThreadingParser parser; + //srv_cfg->setDHCPMultiThreading(parser.parse(config_pair.second)); + continue; + } + if (config_pair.first == "host-reservation-identifiers") { HostReservationIdsParser4 parser; parser.parse(config_pair.second); @@ -587,10 +581,7 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, (config_pair.first == "ddns-qualifying-suffix") || (config_pair.first == "store-extended-info") || (config_pair.first == "statistic-default-sample-count") || - (config_pair.first == "statistic-default-sample-age") || - (config_pair.first == "enable-multi-threading") || - (config_pair.first == "packet-thread-pool-size") || - (config_pair.first == "packet-thread-queue-size")) { + (config_pair.first == "statistic-default-sample-age")) { CfgMgr::instance().getStagingCfg()->addConfiguredGlobal(config_pair.first, config_pair.second); continue; diff --git a/src/bin/dhcp4/parser_context.cc b/src/bin/dhcp4/parser_context.cc index 75cc71e0f1..3541649a04 100644 --- a/src/bin/dhcp4/parser_context.cc +++ b/src/bin/dhcp4/parser_context.cc @@ -176,6 +176,8 @@ Parser4Context::contextName() return ("control-socket"); case DHCP_QUEUE_CONTROL: return ("dhcp-queue-control"); + case DHCP_MULTI_THREADING: + return ("multi-threading"); case POOLS: return ("pools"); case RESERVATIONS: @@ -207,5 +209,5 @@ Parser4Context::contextName() } } -}; -}; +} // namespace dhcp +} // namespace isc diff --git a/src/bin/dhcp4/parser_context.h b/src/bin/dhcp4/parser_context.h index a36b311d1e..251283e763 100644 --- a/src/bin/dhcp4/parser_context.h +++ b/src/bin/dhcp4/parser_context.h @@ -273,6 +273,9 @@ public: /// Used while parsing Dhcp4/dhcp-queue-control structures. DHCP_QUEUE_CONTROL, + /// Used while parsing Dhcp4/multi-threading structures. + DHCP_MULTI_THREADING, + /// Used while parsing Dhcp4/subnet4/pools structures. POOLS, diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 3492c24564..a7d1bf8a13 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -5314,41 +5314,6 @@ TEST_F(Dhcp4ParserTest, hostReservationGlobal) { EXPECT_EQ(Network::HR_OUT_OF_POOL, subnet->getHostReservationMode()); } -/// Check that the multi-threading settings have a default value when not -/// specified. -TEST_F(Dhcp4ParserTest, multiThreadingDefaultSettings) { - ConstElementPtr status; - - string config = "{ " + genIfaceConfig() + "," + - "\"subnet4\": [ ]" - "}"; - - ConstElementPtr json; - ASSERT_NO_THROW(json = parseDHCP4(config)); - extractConfig(config); - - EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); - - // returned value should be 0 (success) - checkResult(status, 0); - - // The value of enable-multi-threading must be equal to the default value - // (false). The default value is defined in GLOBAL4_DEFAULTS in - // simple_parser4.cc. - EXPECT_EQ(false, - CfgMgr::instance().getStagingCfg()->getEnableMultiThreading()); - - // The value of packet-thread-pool-size must be equal to the default value - // (0). The default value is defined in GLOBAL4_DEFAULTS in - // simple_parser4.cc. - EXPECT_EQ(0, CfgMgr::instance().getStagingCfg()->getPktThreadPoolSize()); - - // The value of packet-thread-queue-size must be equal to the default value - // (4). The default value is defined in GLOBAL4_DEFAULTS in - // simple_parser4.cc. - EXPECT_EQ(4, CfgMgr::instance().getStagingCfg()->getPktThreadQueueSize()); -} - /// Check that the decline-probation-period has a default value when not /// specified. TEST_F(Dhcp4ParserTest, declineTimerDefault) { @@ -7195,34 +7160,4 @@ TEST_F(Dhcp4ParserTest, statsDefaultLimits) { util::durationToText(stats_mgr.getMaxSampleAgeDefault(), 0)); } -/// Check that the multi threading settings can be set properly. -TEST_F(Dhcp4ParserTest, multiThreadingSettings) { - ConstElementPtr status; - - string config = "{ " + genIfaceConfig() + "," + - "\"enable-multi-threading\": true," - "\"packet-thread-pool-size\": 256," - "\"packet-thread-queue-size\": 256," - "\"subnet4\": [ ]" - "}"; - - ConstElementPtr json; - ASSERT_NO_THROW(json = parseDHCP4(config)); - extractConfig(config); - - EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); - - // returned value should be 0 (success) - checkResult(status, 0); - - // The value of multi-threading settings must be equal to the specified - // values - EXPECT_EQ(true, - CfgMgr::instance().getStagingCfg()->getEnableMultiThreading()); - EXPECT_EQ(256, - CfgMgr::instance().getStagingCfg()->getPktThreadPoolSize()); - EXPECT_EQ(256, - CfgMgr::instance().getStagingCfg()->getPktThreadQueueSize()); -} - } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 0aed882902..3cbbf85cee 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -140,21 +140,6 @@ ControlledDhcpv6Srv::loadConfigFile(const std::string& file_name) { "processCommand(\"config-set\", json)"); } - // command line parameters overwrite file and database configuration - bool enabled = false; - if (Dhcpv6Srv::srv_thread_count_ >= 0) { - enabled = true; - } - if (enabled) { - CfgMgr::instance().getCurrentCfg()->setPktThreadPoolSize(Dhcpv6Srv::srv_thread_count_); - CfgMgr::instance().getCurrentCfg()->setPktThreadQueueSize(0); - LOG_FATAL(dhcp6_logger, DHCP6_MULTI_THREADING_WARNING); - } else { - enabled = CfgMgr::instance().getCurrentCfg()->getEnableMultiThreading(); - } - MultiThreadingMgr::instance().apply(enabled, - CfgMgr::instance().getCurrentCfg()->getPktThreadPoolSize()); - // Now check is the returned result is successful (rcode=0) or not // (see @ref isc::config::parseAnswer). int rcode; @@ -177,8 +162,8 @@ ControlledDhcpv6Srv::loadConfigFile(const std::string& file_name) { LOG_WARN(dhcp6_logger, DHCP6_MULTI_THREADING_INFO) .arg(MultiThreadingMgr::instance().getMode() ? "yes" : "no") - .arg(MultiThreadingMgr::instance().getPktThreadPoolSize()) - .arg(CfgMgr::instance().getCurrentCfg()->getPktThreadQueueSize()); + .arg(MultiThreadingMgr::instance().getThreadPoolSize()) + .arg(MultiThreadingMgr::instance().getThreadQueueSize()); return (result); } @@ -749,11 +734,12 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance(); + // Single stream instance used in all error clauses + std::ostringstream err; + if (!srv) { - ConstElementPtr no_srv = isc::config::createAnswer( - CONTROL_RESULT_ERROR, - "Server object not initialized, can't process config."); - return (no_srv); + err << "Server object not initialized, can't process config."; + return (isc::config::createAnswer(1, err.str())); } ConstElementPtr answer = configureDhcp6Server(*srv, config); @@ -767,8 +753,8 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { return (answer); } } catch (const std::exception& ex) { - return (isc::config::createAnswer(1, "Failed to process configuration:" - + string(ex.what()))); + err << "Failed to process configuration:" << ex.what(); + return (isc::config::createAnswer(1, err.str())); } // Re-open lease and host database with new parameters. @@ -779,8 +765,8 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { cfg_db->setAppendedParameters("universe=6"); cfg_db->createManagers(); } catch (const std::exception& ex) { - return (isc::config::createAnswer(1, "Unable to open database: " - + std::string(ex.what()))); + err << "Unable to open database: " << ex.what(); + return (isc::config::createAnswer(1, err.str())); } // Regenerate server identifier if needed. @@ -806,9 +792,8 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { try { srv->startD2(); } catch (const std::exception& ex) { - std::ostringstream err; - err << "error starting DHCP_DDNS client " - " after server reconfiguration: " << ex.what(); + err << "Error starting DHCP_DDNS client after server reconfiguration: " + << ex.what(); return (isc::config::createAnswer(1, err.str())); } @@ -832,7 +817,6 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { } } catch (const std::exception& ex) { - std::ostringstream err; err << "Error setting packet queue controls after server reconfiguration: " << ex.what(); return (isc::config::createAnswer(1, err.str())); @@ -856,7 +840,6 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { server_); } catch (const std::exception& ex) { - std::ostringstream err; err << "unable to setup timers for periodically running the" " reclamation of the expired leases: " << ex.what() << "."; @@ -915,6 +898,32 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { // operation. } + // Configure multi threading + try { + data::ConstElementPtr mt; + // command line parameters overwrite file and database configuration + bool enabled = false; + uint32_t thread_pool_size = 0; + uint32_t thread_queue_size = 0; + if (Dhcpv6Srv::srv_thread_count_ >= 0) { + enabled = true; + } + if (enabled) { + thread_pool_size = Dhcpv6Srv::srv_thread_count_; + LOG_FATAL(dhcp6_logger, DHCP6_MULTI_THREADING_WARNING); + } else { + enabled = false; // todo parse + thread_pool_size = 0; // todo parse + thread_queue_size = 0; // todo parse + } + MultiThreadingMgr::instance().apply(enabled, thread_pool_size, + thread_queue_size); + } catch (const std::exception& ex) { + err << "Error applying multi threading settings: " + << ex.what(); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); + } + return (answer); } diff --git a/src/bin/dhcp6/dhcp6_lexer.ll b/src/bin/dhcp6/dhcp6_lexer.ll index a084532801..36d8c32f1b 100644 --- a/src/bin/dhcp6/dhcp6_lexer.ll +++ b/src/bin/dhcp6/dhcp6_lexer.ll @@ -1145,16 +1145,17 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} case isc::dhcp::Parser6Context::DHCP6: case isc::dhcp::Parser6Context::INTERFACES_CONFIG: case isc::dhcp::Parser6Context::SUBNET6: + case isc::dhcp::Parser6Context::POOLS: + case isc::dhcp::Parser6Context::PD_POOLS: case isc::dhcp::Parser6Context::SHARED_NETWORK: case isc::dhcp::Parser6Context::OPTION_DEF: case isc::dhcp::Parser6Context::OPTION_DATA: + case isc::dhcp::Parser6Context::RESERVATIONS: case isc::dhcp::Parser6Context::CLIENT_CLASSES: case isc::dhcp::Parser6Context::SERVER_ID: case isc::dhcp::Parser6Context::CONTROL_SOCKET: - case isc::dhcp::Parser6Context::POOLS: - case isc::dhcp::Parser6Context::PD_POOLS: - case isc::dhcp::Parser6Context::RESERVATIONS: case isc::dhcp::Parser6Context::DHCP_QUEUE_CONTROL: + case isc::dhcp::Parser6Context::DHCP_MULTI_THREADING: case isc::dhcp::Parser6Context::LOGGERS: case isc::dhcp::Parser6Context::DHCP_DDNS: return isc::dhcp::Dhcp6Parser::make_USER_CONTEXT(driver.loc_); @@ -1168,16 +1169,17 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} case isc::dhcp::Parser6Context::DHCP6: case isc::dhcp::Parser6Context::INTERFACES_CONFIG: case isc::dhcp::Parser6Context::SUBNET6: + case isc::dhcp::Parser6Context::POOLS: + case isc::dhcp::Parser6Context::PD_POOLS: case isc::dhcp::Parser6Context::SHARED_NETWORK: case isc::dhcp::Parser6Context::OPTION_DEF: case isc::dhcp::Parser6Context::OPTION_DATA: + case isc::dhcp::Parser6Context::RESERVATIONS: case isc::dhcp::Parser6Context::CLIENT_CLASSES: case isc::dhcp::Parser6Context::SERVER_ID: case isc::dhcp::Parser6Context::CONTROL_SOCKET: - case isc::dhcp::Parser6Context::POOLS: - case isc::dhcp::Parser6Context::PD_POOLS: case isc::dhcp::Parser6Context::DHCP_QUEUE_CONTROL: - case isc::dhcp::Parser6Context::RESERVATIONS: + case isc::dhcp::Parser6Context::DHCP_MULTI_THREADING: case isc::dhcp::Parser6Context::LOGGERS: case isc::dhcp::Parser6Context::DHCP_DDNS: return isc::dhcp::Dhcp6Parser::make_COMMENT(driver.loc_); @@ -1809,30 +1811,39 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} } } -\"enable-multi-threading\" { +\"multi-threading\" { switch(driver.ctx_) { case isc::dhcp::Parser6Context::DHCP6: + return isc::dhcp::Dhcp6Parser::make_DHCP_MULTI_THREADING(driver.loc_); + default: + return isc::dhcp::Dhcp6Parser::make_STRING("multi-threading", driver.loc_); + } +} + +\"enable-multi-threading\" { + switch(driver.ctx_) { + case isc::dhcp::Parser6Context::DHCP_MULTI_THREADING: return isc::dhcp::Dhcp6Parser::make_ENABLE_MULTI_THREADING(driver.loc_); default: return isc::dhcp::Dhcp6Parser::make_STRING("enable-multi-threading", driver.loc_); } } -\"packet-thread-pool-size\" { +\"thread-pool-size\" { switch(driver.ctx_) { - case isc::dhcp::Parser6Context::DHCP6: - return isc::dhcp::Dhcp6Parser::make_PACKET_THREAD_POOL_SIZE(driver.loc_); + case isc::dhcp::Parser6Context::DHCP_MULTI_THREADING: + return isc::dhcp::Dhcp6Parser::make_THREAD_POOL_SIZE(driver.loc_); default: - return isc::dhcp::Dhcp6Parser::make_STRING("packet-thread-pool-size", driver.loc_); + return isc::dhcp::Dhcp6Parser::make_STRING("thread-pool-size", driver.loc_); } } -\"packet-thread-queue-size\" { +\"packet-queue-size\" { switch(driver.ctx_) { - case isc::dhcp::Parser6Context::DHCP6: - return isc::dhcp::Dhcp6Parser::make_PACKET_THREAD_QUEUE_SIZE(driver.loc_); + case isc::dhcp::Parser6Context::DHCP_MULTI_THREADING: + return isc::dhcp::Dhcp6Parser::make_PACKET_QUEUE_SIZE(driver.loc_); default: - return isc::dhcp::Dhcp6Parser::make_STRING("packet-thread-queue-size", driver.loc_); + return isc::dhcp::Dhcp6Parser::make_STRING("packet-queue-size", driver.loc_); } } diff --git a/src/bin/dhcp6/dhcp6_parser.yy b/src/bin/dhcp6/dhcp6_parser.yy index 7ea3be4a8c..abab801491 100644 --- a/src/bin/dhcp6/dhcp6_parser.yy +++ b/src/bin/dhcp6/dhcp6_parser.yy @@ -192,9 +192,10 @@ using namespace std; DHCP4O6_PORT "dhcp4o6-port" + DHCP_MULTI_THREADING "multi-threading" ENABLE_MULTI_THREADING "enable-multi-threading" - PACKET_THREAD_POOL_SIZE "packet-thread-pool-size" - PACKET_THREAD_QUEUE_SIZE "packet-thread-queue-size" + THREAD_POOL_SIZE "thread-pool-size" + PACKET_QUEUE_SIZE "packet-queue-size" CONTROL_SOCKET "control-socket" SOCKET_TYPE "socket-type" @@ -518,9 +519,6 @@ global_param: data_directory | store_extended_info | statistic_default_sample_count | statistic_default_sample_age - | enable_multi_threading - | packet_thread_pool_size - | packet_thread_queue_size | unknown_map_entry ; @@ -1047,19 +1045,44 @@ relay_supplied_options: RELAY_SUPPLIED_OPTIONS { ctx.leave(); }; +// --- multi-threading ------------------------------------------------ + +dhcp_multi_threading: DHCP_MULTI_THREADING { + ElementPtr qc(new MapElement(ctx.loc2pos(@1))); + ctx.stack_.back()->set("multi-threading", qc); + ctx.stack_.push_back(qc); + ctx.enter(ctx.DHCP_MULTI_THREADING); +} COLON LCURLY_BRACKET multi_threading_params RCURLY_BRACKET { + // The enable queue parameter is required. + ctx.require("enable-multi-threading", ctx.loc2pos(@4), ctx.loc2pos(@6)); + ctx.stack_.pop_back(); + ctx.leave(); +}; + +multi_threading_params: multi_threading_param + | multi_threading_param COMMA multi_threading_param + ; + +multi_threading_param: enable_multi_threading + | thread_pool_size + | packet_queue_size + | user_context + | comment + ; + enable_multi_threading: ENABLE_MULTI_THREADING COLON BOOLEAN { ElementPtr b(new BoolElement($3, ctx.loc2pos(@3))); ctx.stack_.back()->set("enable-multi-threading", b); }; -packet_thread_pool_size: PACKET_THREAD_POOL_SIZE COLON INTEGER { +thread_pool_size: THREAD_POOL_SIZE COLON INTEGER { ElementPtr prf(new IntElement($3, ctx.loc2pos(@3))); - ctx.stack_.back()->set("packet-thread-pool-size", prf); + ctx.stack_.back()->set("thread-pool-size", prf); }; -packet_thread_queue_size: PACKET_THREAD_QUEUE_SIZE COLON INTEGER { +packet_queue_size: PACKET_QUEUE_SIZE COLON INTEGER { ElementPtr prf(new IntElement($3, ctx.loc2pos(@3))); - ctx.stack_.back()->set("packet-thread-queue-size", prf); + ctx.stack_.back()->set("packet-queue-size", prf); }; hooks_libraries: HOOKS_LIBRARIES { diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 6d0be0cfbe..538b8e5927 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -512,7 +512,7 @@ int Dhcpv6Srv::run() { } // destroying the thread pool - MultiThreadingMgr::instance().apply(false, 0); + MultiThreadingMgr::instance().apply(false, 0, 0); return (getExitValue()); } @@ -526,11 +526,11 @@ void Dhcpv6Srv::run_one() { bool read_pkt = true; // Do not read more packets from socket if there are enough packets to - // be processed in the packet thread pool queue + // be processed in the dhcp thread pool queue // max_queue_size = 0 means no limit - const int max_queue_size = CfgMgr::instance().getCurrentCfg()->getPktThreadQueueSize(); - const int thread_count = MultiThreadingMgr::instance().getPktThreadPoolSize(); - size_t pkt_queue_size = MultiThreadingMgr::instance().getPktThreadPool().count(); + const int max_queue_size = MultiThreadingMgr::instance().getThreadQueueSize(); + const int thread_count = MultiThreadingMgr::instance().getThreadPoolSize(); + size_t pkt_queue_size = MultiThreadingMgr::instance().getThreadPool().count(); if (thread_count && max_queue_size && (pkt_queue_size >= thread_count * max_queue_size)) { read_pkt = false; } @@ -619,7 +619,7 @@ void Dhcpv6Srv::run_one() { boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv6Srv::processPacketAndSendResponseNoThrow, this, query, rsp)); - MultiThreadingMgr::instance().getPktThreadPool().add(call_back); + MultiThreadingMgr::instance().getThreadPool().add(call_back); } else { processPacketAndSendResponse(query, rsp); } @@ -1023,7 +1023,7 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) { boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv6Srv::sendResponseNoThrow, this, callout_handle, query, rsp)); - MultiThreadingMgr::instance().getPktThreadPool().add(call_back); + MultiThreadingMgr::instance().getThreadPool().add(call_back); } else { processPacketPktSend(callout_handle, query, rsp); processPacketBufferSend(callout_handle, rsp); diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index fbe5a10c01..bde013ac18 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -188,18 +188,6 @@ public: uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port"); srv_config->setDhcp4o6Port(dhcp4o6_port); - // Set enable multi threading flag. - bool enable_multi_threading = getBoolean(global, "enable-multi-threading"); - srv_config->setEnableMultiThreading(enable_multi_threading); - - // Set packet thread pool size. - uint32_t packet_thread_pool_size = getUint32(global, "packet-thread-pool-size"); - srv_config->setPktThreadPoolSize(packet_thread_pool_size); - - // Set packet thread queue size. - uint32_t packet_thread_queue_size = getUint32(global, "packet-thread-queue-size"); - srv_config->setPktThreadQueueSize(packet_thread_queue_size); - // Set the global user context. ConstElementPtr user_context = global->get("user-context"); if (user_context) { @@ -535,6 +523,12 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, continue; } + if (config_pair.first == "multi-threading") { + //DHCPMultiThreadingParser parser; + //srv_config->setDHCPMultiThreading(parser.parse(config_pair.second)); + continue; + } + if (config_pair.first == "host-reservation-identifiers") { HostReservationIdsParser6 parser; parser.parse(config_pair.second); @@ -705,10 +699,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, (config_pair.first == "ddns-qualifying-suffix") || (config_pair.first == "store-extended-info") || (config_pair.first == "statistic-default-sample-count") || - (config_pair.first == "statistic-default-sample-age") || - (config_pair.first == "enable-multi-threading") || - (config_pair.first == "packet-thread-pool-size") || - (config_pair.first == "packet-thread-queue-size")) { + (config_pair.first == "statistic-default-sample-age")) { CfgMgr::instance().getStagingCfg()->addConfiguredGlobal(config_pair.first, config_pair.second); continue; diff --git a/src/bin/dhcp6/parser_context.cc b/src/bin/dhcp6/parser_context.cc index 6323a261b6..de69a76596 100644 --- a/src/bin/dhcp6/parser_context.cc +++ b/src/bin/dhcp6/parser_context.cc @@ -176,6 +176,8 @@ Parser6Context::contextName() return ("control-socket"); case DHCP_QUEUE_CONTROL: return ("dhcp-queue-control"); + case DHCP_MULTI_THREADING: + return ("multi-threading"); case POOLS: return ("pools"); case PD_POOLS: @@ -209,5 +211,5 @@ Parser6Context::contextName() } } -}; -}; +} // namespace dhcp +} // namespace isc diff --git a/src/bin/dhcp6/parser_context.h b/src/bin/dhcp6/parser_context.h index 513aa02daa..53c4800177 100644 --- a/src/bin/dhcp6/parser_context.h +++ b/src/bin/dhcp6/parser_context.h @@ -274,9 +274,12 @@ public: /// Used while parsing Dhcp6/control-socket structures. CONTROL_SOCKET, - /// Used while parsing Dhcp4/dhcp-queue-control structures. + /// Used while parsing Dhcp6/dhcp-queue-control structures. DHCP_QUEUE_CONTROL, + /// Used while parsing Dhcp6/multi-threading structures. + DHCP_MULTI_THREADING, + /// Used while parsing Dhcp6/subnet6/pools structures. POOLS, @@ -307,7 +310,7 @@ public: /// Used while parsing Dhcp6/dhcp-ddns/replace-client-name. REPLACE_CLIENT_NAME, - /// Used while parsing Dhcp4/config-control + /// Used while parsing Dhcp6/config-control CONFIG_CONTROL, /// Used while parsing config-control/config-databases diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 1a3421e072..aa70d2d408 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -5889,41 +5889,6 @@ TEST_F(Dhcp6ParserTest, testDataDir) { EXPECT_NE(original_datadir, string(CfgMgr::instance().getDataDir())); } -/// Check that the multi-threading settings have a default value when not -/// specified. -TEST_F(Dhcp6ParserTest, multiThreadingDefaultSettings) { - ConstElementPtr status; - - string config = "{ " + genIfaceConfig() + "," + - "\"subnet6\": [ ]" - "}"; - - ConstElementPtr json; - ASSERT_NO_THROW(json = parseDHCP6(config)); - extractConfig(config); - - EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - - // returned value should be 0 (success) - checkResult(status, 0); - - // The value of enable-multi-threading must be equal to the default value - // (false). The default value is defined in GLOBAL6_DEFAULTS in - // simple_parser6.cc. - EXPECT_EQ(false, - CfgMgr::instance().getStagingCfg()->getEnableMultiThreading()); - - // The value of packet-thread-pool-size must be equal to the default value - // (0). The default value is defined in GLOBAL6_DEFAULTS in - // simple_parser6.cc. - EXPECT_EQ(0, CfgMgr::instance().getStagingCfg()->getPktThreadPoolSize()); - - // The value of packet-thread-queue-size must be equal to the default value - // (4). The default value is defined in GLOBAL6_DEFAULTS in - // simple_parser6.cc. - EXPECT_EQ(4, CfgMgr::instance().getStagingCfg()->getPktThreadQueueSize()); -} - /// Check that the decline-probation-period value has a default value if not /// specified explicitly. TEST_F(Dhcp6ParserTest, declineTimerDefault) { @@ -7763,34 +7728,4 @@ TEST_F(Dhcp6ParserTest, statsDefaultLimits) { util::durationToText(stats_mgr.getMaxSampleAgeDefault(), 0)); } -/// Check that the multi threading settings can be set properly. -TEST_F(Dhcp6ParserTest, multiThreadingSettings) { - ConstElementPtr status; - - string config = "{ " + genIfaceConfig() + "," + - "\"enable-multi-threading\": true," - "\"packet-thread-pool-size\": 256," - "\"packet-thread-queue-size\": 256," - "\"subnet6\": [ ]" - "}"; - - ConstElementPtr json; - ASSERT_NO_THROW(json = parseDHCP6(config)); - extractConfig(config); - - EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - - // returned value should be 0 (success) - checkResult(status, 0); - - // The value of multi-threading settings must be equal to the specified - // values - EXPECT_EQ(true, - CfgMgr::instance().getStagingCfg()->getEnableMultiThreading()); - EXPECT_EQ(256, - CfgMgr::instance().getStagingCfg()->getPktThreadPoolSize()); - EXPECT_EQ(256, - CfgMgr::instance().getStagingCfg()->getPktThreadQueueSize()); -} - } diff --git a/src/lib/dhcpsrv/parsers/simple_parser4.cc b/src/lib/dhcpsrv/parsers/simple_parser4.cc index b86087e607..a4bd347cfd 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser4.cc +++ b/src/lib/dhcpsrv/parsers/simple_parser4.cc @@ -6,10 +6,10 @@ #include -#include #include +#include + #include -#include using namespace isc::data; @@ -85,10 +85,7 @@ const SimpleKeywords SimpleParser4::GLOBAL4_PARAMETERS = { { "ddns-qualifying-suffix", Element::string }, { "store-extended-info", Element::boolean }, { "statistic-default-sample-count", Element::integer }, - { "statistic-default-sample-age", Element::integer }, - { "enable-multi-threading", Element::boolean }, - { "packet-thread-pool-size", Element::integer }, - { "packet-thread-queue-size", Element::integer } + { "statistic-default-sample-age", Element::integer } }; /// @brief This table defines default global values for DHCPv4 @@ -121,10 +118,7 @@ const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { { "hostname-char-replacement", Element::string, "" }, { "store-extended-info", Element::boolean, "false" }, { "statistic-default-sample-count", Element::integer, "20" }, - { "statistic-default-sample-age", Element::integer, "0" }, - { "enable-multi-threading", Element::boolean, "false" }, - { "packet-thread-pool-size", Element::integer, "0" }, - { "packet-thread-queue-size", Element::integer, "4" } + { "statistic-default-sample-age", Element::integer, "0" } }; /// @brief This table defines all option definition parameters. @@ -249,7 +243,7 @@ const SimpleDefaults SimpleParser4::SUBNET4_DEFAULTS = { /// @brief This table defines default values for each IPv4 subnet that is /// part of a shared network /// -/// This is mostly the same as @ref SUBNET4_DEFAULTS, except two parameters +/// This is mostly the same as @ref SUBNET4_DEFAULTS, except the parameters /// that can be derived from shared-network, but cannot from global scope. /// Those are: interface and reservation-mode. const SimpleDefaults SimpleParser4::SHARED_SUBNET4_DEFAULTS = { @@ -259,7 +253,7 @@ const SimpleDefaults SimpleParser4::SHARED_SUBNET4_DEFAULTS = { { "4o6-subnet", Element::string, "" }, }; -/// @brief List of parameters that can be inherited to subnet4 scope. +/// @brief List of parameters that can be inherited from the global to subnet4 scope. /// /// Some parameters may be defined on both global (directly in Dhcp4) and /// subnet (Dhcp4/subnet4/...) scope. If not defined in the subnet scope, @@ -355,6 +349,13 @@ const SimpleDefaults SimpleParser4::DHCP_QUEUE_CONTROL4_DEFAULTS = { { "capacity", Element::integer, "500"} }; +/// @brief This table defines default values for multi-threading in DHCPv4. +const SimpleDefaults SimpleParser4::DHCP_MULTI_THREADING4_DEFAULTS = { + { "enable-multi-threading", Element::boolean, "false" }, + { "packet-thread-pool-size", Element::integer, "0" }, + { "packet-thread-queue-size", Element::integer, "4" } +}; + /// @brief This defines default values for sanity checking for DHCPv4. const SimpleDefaults SimpleParser4::SANITY_CHECKS4_DEFAULTS = { { "lease-checks", Element::string, "warn" } @@ -372,12 +373,10 @@ size_t SimpleParser4::setAllDefaults(ElementPtr global) { // Set global defaults first. cnt = setDefaults(global, GLOBAL4_DEFAULTS); - // Now set option definition defaults for each specified option definition + // Now set the defaults for each specified option definition ConstElementPtr option_defs = global->get("option-def"); if (option_defs) { - BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) { - cnt += SimpleParser::setDefaults(option_def, OPTION4_DEF_DEFAULTS); - } + cnt += setListDefaults(option_defs, OPTION4_DEF_DEFAULTS); } // Set the defaults for option data @@ -413,8 +412,8 @@ size_t SimpleParser4::setAllDefaults(ElementPtr global) { } } - // Set the defaults for dhcp-queue-control. If the element isn't - // there we'll add it. + // Set the defaults for dhcp-queue-control. If the element isn't there + // we'll add it. ConstElementPtr queue_control = global->get("dhcp-queue-control"); ElementPtr mutable_cfg; if (queue_control) { @@ -426,6 +425,18 @@ size_t SimpleParser4::setAllDefaults(ElementPtr global) { cnt += setDefaults(mutable_cfg, DHCP_QUEUE_CONTROL4_DEFAULTS); + // Set the defaults for multi-threading. If the element isn't there + // we'll add it. + ConstElementPtr multi_threading = global->get("multi-threading"); + if (queue_control) { + mutable_cfg = boost::const_pointer_cast(queue_control); + } else { + mutable_cfg = Element::createMap(); + global->set("multi-threading", mutable_cfg); + } + + cnt += setDefaults(mutable_cfg, DHCP_MULTI_THREADING4_DEFAULTS); + // Set the defaults for sanity-checks. If the element isn't // there we'll add it. ConstElementPtr sanity_checks = global->get("sanity-checks"); @@ -473,7 +484,6 @@ size_t SimpleParser4::deriveParameters(ElementPtr global) { INHERIT_TO_SUBNET4); } } - } } diff --git a/src/lib/dhcpsrv/parsers/simple_parser4.h b/src/lib/dhcpsrv/parsers/simple_parser4.h index 063c637766..74189f63d3 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser4.h +++ b/src/lib/dhcpsrv/parsers/simple_parser4.h @@ -20,6 +20,7 @@ namespace dhcp { /// For the actual values, see @file simple_parser4.cc class SimpleParser4 : public isc::data::SimpleParser { public: + /// @brief Sets all defaults for DHCPv4 configuration /// /// This method sets global, option data and option definitions defaults. @@ -58,9 +59,11 @@ public: static const isc::data::SimpleDefaults IFACE4_DEFAULTS; static const isc::data::SimpleDefaults DHCP_QUEUE_CONTROL4_DEFAULTS; + static const isc::data::SimpleDefaults DHCP_MULTI_THREADING4_DEFAULTS; static const isc::data::SimpleDefaults SANITY_CHECKS4_DEFAULTS; }; -}; -}; +} // namespace dhcp +} // namespace isc + #endif diff --git a/src/lib/dhcpsrv/parsers/simple_parser6.cc b/src/lib/dhcpsrv/parsers/simple_parser6.cc index 96375bf4cb..f798fafdd9 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser6.cc +++ b/src/lib/dhcpsrv/parsers/simple_parser6.cc @@ -8,6 +8,7 @@ #include #include + #include using namespace isc::data; @@ -85,10 +86,7 @@ const SimpleKeywords SimpleParser6::GLOBAL6_PARAMETERS = { { "ddns-qualifying-suffix", Element::string }, { "store-extended-info", Element::boolean }, { "statistic-default-sample-count", Element::integer }, - { "statistic-default-sample-age", Element::integer }, - { "enable-multi-threading", Element::boolean }, - { "packet-thread-pool-size", Element::integer }, - { "packet-thread-queue-size", Element::integer } + { "statistic-default-sample-age", Element::integer } }; /// @brief This table defines default global values for DHCPv6 @@ -116,10 +114,7 @@ const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { { "hostname-char-replacement", Element::string, "" }, { "store-extended-info", Element::boolean, "false" }, { "statistic-default-sample-count", Element::integer, "20" }, - { "statistic-default-sample-age", Element::integer, "0" }, - { "enable-multi-threading", Element::boolean, "false" }, - { "packet-thread-pool-size", Element::integer, "0" }, - { "packet-thread-queue-size", Element::integer, "4" } + { "statistic-default-sample-age", Element::integer, "0" } }; /// @brief This table defines all option definition parameters. @@ -224,6 +219,11 @@ const SimpleKeywords SimpleParser6::SUBNET6_PARAMETERS = { }; /// @brief This table defines default values for each IPv6 subnet. +/// +/// Note: When updating this array, please also update SHARED_SUBNET6_DEFAULTS +/// below. In most cases, those two should be kept in sync, except cases +/// where a parameter can be derived from shared-networks, but is not +/// defined on global level. const SimpleDefaults SimpleParser6::SUBNET6_DEFAULTS = { { "id", Element::integer, "0" }, // 0 means autogenerate { "interface", Element::string, "" }, @@ -232,7 +232,11 @@ const SimpleDefaults SimpleParser6::SUBNET6_DEFAULTS = { { "interface-id", Element::string, "" } }; -/// @brief This table defines default values for each IPv6 shared network. +/// @brief This table defines default values for each IPv6 subnet that is +/// part of a shared network +/// +/// This is mostly the same as @ref SUBNET6_DEFAULTS, except the parameters +/// that can be derived from shared-network, but cannot from global scope. const SimpleDefaults SimpleParser6::SHARED_NETWORK6_DEFAULTS = { { "client-class", Element::string, "" }, { "interface", Element::string, "" }, @@ -350,13 +354,20 @@ const SimpleDefaults SimpleParser6::IFACE6_DEFAULTS = { { "re-detect", Element::boolean, "true" } }; -/// @brief This table defines default values for dhcp-queue-control in DHCPv4. +/// @brief This table defines default values for dhcp-queue-control in DHCPv6. const SimpleDefaults SimpleParser6::DHCP_QUEUE_CONTROL6_DEFAULTS = { { "enable-queue", Element::boolean, "false"}, { "queue-type", Element::string, "kea-ring6"}, { "capacity", Element::integer, "500"} }; +/// @brief This table defines default values for multi-threading in DHCPv6. +const SimpleDefaults SimpleParser6::DHCP_MULTI_THREADING6_DEFAULTS = { + { "enable-multi-threading", Element::boolean, "false" }, + { "packet-thread-pool-size", Element::integer, "0" }, + { "packet-thread-queue-size", Element::integer, "4" } +}; + /// @brief This defines default values for sanity checking for DHCPv6. const SimpleDefaults SimpleParser6::SANITY_CHECKS6_DEFAULTS = { { "lease-checks", Element::string, "warn" } @@ -377,17 +388,13 @@ size_t SimpleParser6::setAllDefaults(ElementPtr global) { // Now set the defaults for each specified option definition ConstElementPtr option_defs = global->get("option-def"); if (option_defs) { - BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) { - cnt += SimpleParser::setDefaults(option_def, OPTION6_DEF_DEFAULTS); - } + cnt += setListDefaults(option_defs, OPTION6_DEF_DEFAULTS); } // Set the defaults for option data ConstElementPtr options = global->get("option-data"); if (options) { - BOOST_FOREACH(ElementPtr single_option, options->listValue()) { - cnt += SimpleParser::setDefaults(single_option, OPTION6_DEFAULTS); - } + cnt += setListDefaults(options, OPTION6_DEFAULTS); } // Now set the defaults for defined subnets @@ -430,6 +437,18 @@ size_t SimpleParser6::setAllDefaults(ElementPtr global) { cnt += setDefaults(mutable_cfg, DHCP_QUEUE_CONTROL6_DEFAULTS); + // Set the defaults for multi-threading. If the element isn't there + // we'll add it. + ConstElementPtr multi_threading = global->get("multi-threading"); + if (queue_control) { + mutable_cfg = boost::const_pointer_cast(queue_control); + } else { + mutable_cfg = Element::createMap(); + global->set("multi-threading", mutable_cfg); + } + + cnt += setDefaults(mutable_cfg, DHCP_MULTI_THREADING6_DEFAULTS); + // Set the defaults for sanity-checks. If the element isn't // there we'll add it. ConstElementPtr sanity_checks = global->get("sanity-checks"); @@ -447,6 +466,7 @@ size_t SimpleParser6::setAllDefaults(ElementPtr global) { size_t SimpleParser6::deriveParameters(ElementPtr global) { size_t cnt = 0; + // Now derive global parameters into subnets. ConstElementPtr subnets = global->get("subnet6"); if (subnets) { diff --git a/src/lib/dhcpsrv/parsers/simple_parser6.h b/src/lib/dhcpsrv/parsers/simple_parser6.h index 5ab2cd972a..c48545d868 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser6.h +++ b/src/lib/dhcpsrv/parsers/simple_parser6.h @@ -33,7 +33,7 @@ public: /// /// This method currently does the following: /// - derives global parameters to subnets (lifetimes for now) - /// @param global scope to be modified if needed (subnet4 will be extracted) + /// @param global scope to be modified if needed (subnet6 will be extracted) /// @return number of default values derived static size_t deriveParameters(isc::data::ElementPtr global); @@ -60,10 +60,11 @@ public: static const isc::data::SimpleDefaults IFACE6_DEFAULTS; static const isc::data::SimpleDefaults DHCP_QUEUE_CONTROL6_DEFAULTS; + static const isc::data::SimpleDefaults DHCP_MULTI_THREADING6_DEFAULTS; static const isc::data::SimpleDefaults SANITY_CHECKS6_DEFAULTS; }; -}; -}; +} // namespace dhcp +} // namespace isc #endif diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 06969169ad..c24030c1c6 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -42,8 +42,6 @@ SrvConfig::SrvConfig() cfg_host_operations6_(CfgHostOperations::createConfig6()), class_dictionary_(new ClientClassDictionary()), decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0), - enable_multi_threading_(false), - pkt_thread_pool_size_(0), pkt_thread_queue_size_(0), d2_client_config_(new D2ClientConfig()), configured_globals_(Element::createMap()), cfg_consist_(new CfgConsistency()) { @@ -62,8 +60,6 @@ SrvConfig::SrvConfig(const uint32_t sequence) cfg_host_operations6_(CfgHostOperations::createConfig6()), class_dictionary_(new ClientClassDictionary()), decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0), - enable_multi_threading_(false), - pkt_thread_pool_size_(0), pkt_thread_queue_size_(0), d2_client_config_(new D2ClientConfig()), configured_globals_(Element::createMap()), cfg_consist_(new CfgConsistency()) { @@ -443,15 +439,6 @@ SrvConfig::toElement() const { // Set dhcp4o6-port dhcp->set("dhcp4o6-port", Element::create(static_cast(dhcp4o6_port_))); - // Set enable-multi-threading - dhcp->set("enable-multi-threading", - Element::create(enable_multi_threading_)); - // Set packet-thread-pool-size - dhcp->set("packet-thread-pool-size", - Element::create(static_cast(pkt_thread_pool_size_))); - // Set packet-thread-queue-size - dhcp->set("packet-thread-queue-size", - Element::create(static_cast(pkt_thread_queue_size_))); // Set dhcp-ddns dhcp->set("dhcp-ddns", d2_client_config_->toElement()); // Set interfaces-config @@ -628,6 +615,12 @@ SrvConfig::toElement() const { dhcp->set("dhcp-queue-control", dhcp_queue_control); } + // Set multi-threading (if it exists) + data::ConstElementPtr dhcp_multi_threading = getDHCPMultiThreading(); + if (dhcp_multi_threading) { + dhcp->set("multi-threading", dhcp_multi_threading); + } + return (result); } diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 81308f495c..bda3d0a55b 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -467,6 +467,18 @@ public: dhcp_queue_control_ = dhcp_queue_control; } + /// @brief Returns DHCP multi threading information + /// @return pointer to the DHCP multi threading information + const isc::data::ConstElementPtr getDHCPMultiThreading() const { + return (dhcp_multi_threading_); + } + + /// @brief Sets information about the dhcp multi threading + /// @param dhcp_multi_threading new dhcp multi threading information + void setDHCPMultiThreading(const isc::data::ConstElementPtr dhcp_multi_threading) { + dhcp_multi_threading_ = dhcp_multi_threading; + } + /// @brief Returns pointer to the dictionary of global client /// class definitions ClientClassDictionaryPtr getClientClassDictionary() { @@ -705,48 +717,6 @@ public: return (dhcp4o6_port_); } - /// @brief Sets the enable multi threading flag. - /// - /// @param size value of the enable multi threading flag - void setEnableMultiThreading(bool enabled) { - enable_multi_threading_ = enabled; - } - - /// @brief Retrieves the enable multi threading flag. - /// - /// @return value of the enable multi threading flag - uint32_t getEnableMultiThreading() const { - return (enable_multi_threading_); - } - - /// @brief Sets the packet thread pool size. - /// - /// @param size value of the packet thread pool size - void setPktThreadPoolSize(uint32_t size) { - pkt_thread_pool_size_ = size; - } - - /// @brief Retrieves the packet thread pool size. - /// - /// @return value of the packet thread pool size - uint32_t getPktThreadPoolSize() const { - return (pkt_thread_pool_size_); - } - - /// @brief Sets the packet thread queue size. - /// - /// @param size value of the packet thread queue size - void setPktThreadQueueSize(uint32_t size) { - pkt_thread_queue_size_ = size; - } - - /// @brief Retrieves the packet thread queue size. - /// - /// @return value of the packet thread queue size - uint32_t getPktThreadQueueSize() const { - return (pkt_thread_queue_size_); - } - /// @brief Returns pointer to the D2 client configuration D2ClientConfigPtr getD2ClientConfig() { return (d2_client_config_); @@ -944,6 +914,9 @@ private: /// @brief Pointer to the dhcp-queue-control information isc::data::ConstElementPtr dhcp_queue_control_; + /// @brief Pointer to the multi-threading information + isc::data::ConstElementPtr dhcp_multi_threading_; + /// @brief Pointer to the dictionary of global client class definitions ClientClassDictionaryPtr class_dictionary_; @@ -965,15 +938,6 @@ private: /// this socket is bound and connected to this port and port + 1 uint16_t dhcp4o6_port_; - /// @brief The enable multi threading flag. - bool enable_multi_threading_; - - /// @brief The packet thread pool size. - uint32_t pkt_thread_pool_size_; - - /// @brief The packet thread queue size. - uint32_t pkt_thread_queue_size_; - /// @brief Stores D2 client configuration D2ClientConfigPtr d2_client_config_; diff --git a/src/lib/dhcpsrv/tests/srv_config_unittest.cc b/src/lib/dhcpsrv/tests/srv_config_unittest.cc index 7da0f6c16a..6b4b5690f1 100644 --- a/src/lib/dhcpsrv/tests/srv_config_unittest.cc +++ b/src/lib/dhcpsrv/tests/srv_config_unittest.cc @@ -1561,23 +1561,4 @@ TEST_F(SrvConfigTest, getDdnsParamsNoSubnetTest6) { EXPECT_TRUE(params->getHostnameCharReplacement().empty()); } -// Multi-threading settings -TEST_F(SrvConfigTest, multiThreadingSettings) { - SrvConfig conf(32); - - // Upon construction multi-threading should be disabled, thread pool size - // and packet queue size should be 0 - ASSERT_FALSE(conf.getEnableMultiThreading()); - ASSERT_EQ(0, conf.getPktThreadPoolSize()); - ASSERT_EQ(0, conf.getPktThreadQueueSize()); - - // Verify we can change default settings. - ASSERT_NO_THROW(conf.setEnableMultiThreading(true)); - ASSERT_NO_THROW(conf.setPktThreadPoolSize(4)); - ASSERT_NO_THROW(conf.setPktThreadQueueSize(64)); - ASSERT_TRUE(conf.getEnableMultiThreading()); - ASSERT_EQ(4, conf.getPktThreadPoolSize()); - ASSERT_EQ(64, conf.getPktThreadQueueSize()); -} - } // end of anonymous namespace diff --git a/src/lib/util/multi_threading_mgr.cc b/src/lib/util/multi_threading_mgr.cc index 5ebfb110e6..3362d55f13 100644 --- a/src/lib/util/multi_threading_mgr.cc +++ b/src/lib/util/multi_threading_mgr.cc @@ -10,7 +10,8 @@ namespace isc { namespace util { MultiThreadingMgr::MultiThreadingMgr() - : enabled_(false), critical_section_count_(0) { + : enabled_(false), critical_section_count_(0), thread_pool_size_(0), + thread_queue_size_(0) { } MultiThreadingMgr::~MultiThreadingMgr() { @@ -53,18 +54,28 @@ MultiThreadingMgr::isInCriticalSection() { } ThreadPool>& -MultiThreadingMgr::getPktThreadPool() { - return pkt_thread_pool_; +MultiThreadingMgr::getThreadPool() { + return thread_pool_; } uint32_t -MultiThreadingMgr::getPktThreadPoolSize() const { - return (pkt_thread_pool_size_); +MultiThreadingMgr::getThreadPoolSize() const { + return (thread_pool_size_); } void -MultiThreadingMgr::setPktThreadPoolSize(uint32_t size) { - pkt_thread_pool_size_ = size; +MultiThreadingMgr::setThreadPoolSize(uint32_t size) { + thread_pool_size_ = size; +} + +uint32_t +MultiThreadingMgr::getThreadQueueSize() const { + return (thread_queue_size_); +} + +void +MultiThreadingMgr::setThreadQueueSize(uint32_t size) { + thread_queue_size_ = size; } uint32_t @@ -73,7 +84,7 @@ MultiThreadingMgr::supportedThreadCount() { } void -MultiThreadingMgr::apply(bool enabled, uint32_t thread_count) { +MultiThreadingMgr::apply(bool enabled, uint32_t thread_count, uint32_t queue_size) { // check the enabled flag if (enabled) { // check for auto scaling (enabled flag true but thread_count 0) @@ -83,36 +94,39 @@ MultiThreadingMgr::apply(bool enabled, uint32_t thread_count) { } } else { thread_count = 0; + queue_size = 0; } // check enabled flag and explicit number of threads or system supports // hardware concurrency if (thread_count) { - if (pkt_thread_pool_.size()) { - pkt_thread_pool_.stop(); + if (thread_pool_.size()) { + thread_pool_.stop(); } - setPktThreadPoolSize(thread_count); + setThreadPoolSize(thread_count); + setThreadQueueSize(queue_size); setMode(true); if (!isInCriticalSection()) { - pkt_thread_pool_.start(thread_count); + thread_pool_.start(thread_count); } } else { - pkt_thread_pool_.reset(); + thread_pool_.reset(); setMode(false); - setPktThreadPoolSize(thread_count); + setThreadPoolSize(thread_count); + setThreadQueueSize(queue_size); } } void MultiThreadingMgr::stopPktProcessing() { - if (getMode() && getPktThreadPoolSize() && !isInCriticalSection()) { - pkt_thread_pool_.stop(); + if (getMode() && getThreadPoolSize() && !isInCriticalSection()) { + thread_pool_.stop(); } } void MultiThreadingMgr::startPktProcessing() { - if (getMode() && getPktThreadPoolSize() && !isInCriticalSection()) { - pkt_thread_pool_.start(getPktThreadPoolSize()); + if (getMode() && getThreadPoolSize() && !isInCriticalSection()) { + thread_pool_.start(getThreadPoolSize()); } } diff --git a/src/lib/util/multi_threading_mgr.h b/src/lib/util/multi_threading_mgr.h index a130907949..48e0f9b528 100644 --- a/src/lib/util/multi_threading_mgr.h +++ b/src/lib/util/multi_threading_mgr.h @@ -77,7 +77,7 @@ public: /// @brief Exit critical section. /// /// When exiting @ref MultiThreadingCriticalSection, decrement internal - /// counter so that the packet thread pool can be started according to the + /// counter so that the dhcp thread pool can be started according to the /// new configuration. /// If the internal counter is 0, then start the thread pool. void exitCriticalSection(); @@ -87,20 +87,30 @@ public: /// @return The critical section flag. bool isInCriticalSection(); - /// @brief Get the packet thread pool. + /// @brief Get the dhcp thread pool. /// - /// @return The packet thread pool of this binary instance. - ThreadPool>& getPktThreadPool(); + /// @return The dhcp thread pool of this binary instance. + ThreadPool>& getThreadPool(); - /// @brief Get the configured packet thread pool size. + /// @brief Get the configured dhcp thread pool size. /// - /// @return The packet thread pool size of this binary instance. - uint32_t getPktThreadPoolSize() const; + /// @return The dhcp thread pool size of this binary instance. + uint32_t getThreadPoolSize() const; - /// @brief Set the configured packet thread pool size. + /// @brief Set the configured dhcp thread pool size. /// - /// @param size The packet thread pool size of this binary instance. - void setPktThreadPoolSize(uint32_t size); + /// @param size The dhcp thread pool size of this binary instance. + void setThreadPoolSize(uint32_t size); + + /// @brief Get the configured dhcp thread queue size. + /// + /// @return The dhcp thread queue size of this binary instance. + uint32_t getThreadQueueSize() const; + + /// @brief Set the configured dhcp thread queue size. + /// + /// @param size The dhcp thread queue size of this binary instance. + void setThreadQueueSize(uint32_t size); /// @brief The system current supported hardware concurrency thread count. /// @@ -113,9 +123,11 @@ public: /// /// @param enabled The enabled flag: true if multi-threading is enabled, /// false otherwise. - /// @param thread_count The number of desired threads: non 0 if explicitly + /// @param thread_count The desired number of threads: non 0 if explicitly /// configured, 0 if auto scaling is desired - void apply(bool enabled, uint32_t thread_count); + /// @param queue_size The desired thread queue size: non 0 if explicitly + /// configured, 0 for unlimited size + void apply(bool enabled, uint32_t thread_count, uint32_t queue_size); protected: @@ -129,12 +141,12 @@ private: /// @brief Class method stopping and joining all threads of the pool. /// - /// Stop the packet thread pool if running. + /// Stop the dhcp thread pool if running. void stopPktProcessing(); /// @brief Class method (re)starting threads of the pool. /// - /// Start the packet thread pool according to current configuration. + /// Start the dhcp thread pool according to current configuration. void startPktProcessing(); /// @brief The current multi-threading mode. @@ -151,11 +163,14 @@ private: /// This handles multiple interleaved sections. uint32_t critical_section_count_; - /// @brief The configured size of the packet thread pool. - uint32_t pkt_thread_pool_size_; + /// @brief The configured size of the dhcp thread pool. + uint32_t thread_pool_size_; + + /// @brief The configured size of the dhcp thread queue. + uint32_t thread_queue_size_; /// @brief Packet processing thread pool. - ThreadPool> pkt_thread_pool_; + ThreadPool> thread_pool_; }; /// @note: everything here MUST be used ONLY from the main thread. @@ -165,7 +180,7 @@ private: /// /// @note: the multi-threading mode MUST NOT be changed in the RAII /// @c MultiThreadingCriticalSection body. -/// @note: starting and stopping the packet thread pool should be handled +/// @note: starting and stopping the dhcp thread pool should be handled /// in the main thread, if done on one of the processing threads will cause a /// deadlock. /// This is mainly useful in hook commands which handle configuration @@ -175,13 +190,13 @@ public: /// @brief Constructor. /// - /// Entering the critical section. The packet thread pool instance will be + /// Entering the critical section. The dhcp thread pool instance will be /// stopped so that all configuration changes can be safely applied. MultiThreadingCriticalSection(); /// @brief Destructor. /// - /// Leaving the critical section. The packet thread pool instance will be + /// Leaving the critical section. The dhcp thread pool instance will be /// started according to the new configuration. virtual ~MultiThreadingCriticalSection(); }; diff --git a/src/lib/util/tests/multi_threading_mgr_unittest.cc b/src/lib/util/tests/multi_threading_mgr_unittest.cc index 4e2846e9e7..27fe185fe2 100644 --- a/src/lib/util/tests/multi_threading_mgr_unittest.cc +++ b/src/lib/util/tests/multi_threading_mgr_unittest.cc @@ -33,17 +33,31 @@ TEST(MultiThreadingMgrTest, setMode) { } /// @brief Verifies that the thread pool size setter works. -TEST(MultiThreadingMgrTest, pktThreadPoolSize) { +TEST(MultiThreadingMgrTest, threadPoolSize) { // default thread count is 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); // set thread count to 16 - EXPECT_NO_THROW(MultiThreadingMgr::instance().setPktThreadPoolSize(16)); + EXPECT_NO_THROW(MultiThreadingMgr::instance().setThreadPoolSize(16)); // thread count should be 16 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 16); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); // set thread count to 0 - EXPECT_NO_THROW(MultiThreadingMgr::instance().setPktThreadPoolSize(0)); + EXPECT_NO_THROW(MultiThreadingMgr::instance().setThreadPoolSize(0)); // thread count should be 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); +} + +/// @brief Verifies that the thread queue size setter works. +TEST(MultiThreadingMgrTest, threadQueueSize) { + // default queue size is 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); + // set queue size to 16 + EXPECT_NO_THROW(MultiThreadingMgr::instance().setThreadQueueSize(16)); + // queue size should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 16); + // set queue size to 0 + EXPECT_NO_THROW(MultiThreadingMgr::instance().setThreadQueueSize(0)); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); } /// @brief Verifies that determining supported thread count works. @@ -53,51 +67,55 @@ TEST(MultiThreadingMgrTest, supportedThreadCount) { } /// @brief Verifies that accessing the thread pool works. -TEST(MultiThreadingMgrTest, pktThreadPool) { +TEST(MultiThreadingMgrTest, threadPool) { // get the thread pool - EXPECT_NO_THROW(MultiThreadingMgr::instance().getPktThreadPool()); + EXPECT_NO_THROW(MultiThreadingMgr::instance().getThreadPool()); } /// @brief Verifies that apply settings works. TEST(MultiThreadingMgrTest, applyConfig) { // get the thread pool - auto& thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + auto& thread_pool = MultiThreadingMgr::instance().getThreadPool(); // MT should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); // default thread count is 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); - // enable MT with 16 threads - EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 16)); + // enable MT with 16 threads and queue size 256 + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 16, 256)); // MT should be enabled EXPECT_TRUE(MultiThreadingMgr::instance().getMode()); // thread count should be 16 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 16); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); // thread pool should be started EXPECT_EQ(thread_pool.size(), 16); // disable MT - EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 16)); + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 16, 256)); // MT should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); // thread count should be 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); // enable MT with auto scaling - EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 0)); + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 0, 0)); // MT should be enabled EXPECT_TRUE(MultiThreadingMgr::instance().getMode()); // thread count should be maximum - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), MultiThreadingMgr::supportedThreadCount()); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), MultiThreadingMgr::supportedThreadCount()); // thread pool should be started EXPECT_EQ(thread_pool.size(), MultiThreadingMgr::supportedThreadCount()); // disable MT - EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 0)); + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 0, 0)); // MT should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); // thread count should be 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); } @@ -105,13 +123,13 @@ TEST(MultiThreadingMgrTest, applyConfig) { /// @brief Verifies that the critical section flag works. TEST(MultiThreadingMgrTest, criticalSectionFlag) { // get the thread pool - auto& thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + auto& thread_pool = MultiThreadingMgr::instance().getThreadPool(); // MT should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); // critical section should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().isInCriticalSection()); // thread count should be 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); // exit critical section @@ -122,12 +140,14 @@ TEST(MultiThreadingMgrTest, criticalSectionFlag) { EXPECT_NO_THROW(MultiThreadingMgr::instance().enterCriticalSection()); // critical section should be enabled EXPECT_TRUE(MultiThreadingMgr::instance().isInCriticalSection()); - // enable MT with 16 threads - EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 16)); + // enable MT with 16 threads and queue size 256 + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(true, 16, 256)); // MT should be enabled EXPECT_TRUE(MultiThreadingMgr::instance().getMode()); // thread count should be 16 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 16); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); // exit critical section @@ -139,11 +159,13 @@ TEST(MultiThreadingMgrTest, criticalSectionFlag) { // critical section should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().isInCriticalSection()); // disable MT - EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 0)); + EXPECT_NO_THROW(MultiThreadingMgr::instance().apply(false, 0, 0)); // MT should be disabled EXPECT_FALSE(MultiThreadingMgr::instance().getMode()); // thread count should be 0 - EXPECT_EQ(MultiThreadingMgr::instance().getPktThreadPoolSize(), 0); + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); } @@ -151,81 +173,149 @@ TEST(MultiThreadingMgrTest, criticalSectionFlag) { /// @brief Verifies that the critical section works. TEST(MultiThreadingMgrTest, criticalSection) { // get the thread pool instance - auto& thread_pool = MultiThreadingMgr::instance().getPktThreadPool(); + auto& thread_pool = MultiThreadingMgr::instance().getThreadPool(); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); - // apply multi-threading configuration with 16 threads - MultiThreadingMgr::instance().apply(true, 16); + // apply multi-threading configuration with 16 threads and queue size 256 + MultiThreadingMgr::instance().apply(true, 16, 256); // thread count should match EXPECT_EQ(thread_pool.size(), 16); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); // use scope to test constructor and destructor { MultiThreadingCriticalSection cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); // use scope to test constructor and destructor { MultiThreadingCriticalSection inner_cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); } // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); } // thread count should match EXPECT_EQ(thread_pool.size(), 16); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); // use scope to test constructor and destructor { MultiThreadingCriticalSection cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); - // apply multi-threading configuration with 64 threads - MultiThreadingMgr::instance().apply(true, 64); + // thread count should be 16 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 16); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); + // apply multi-threading configuration with 64 threads and queue size 4 + MultiThreadingMgr::instance().apply(true, 64, 4); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 64 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 64); + // queue size should be 4 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 4); } // thread count should match EXPECT_EQ(thread_pool.size(), 64); + // thread count should be 64 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 64); + // queue size should be 4 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 4); // use scope to test constructor and destructor { MultiThreadingCriticalSection cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 64 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 64); + // queue size should be 4 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 4); // apply multi-threading configuration with 0 threads - MultiThreadingMgr::instance().apply(false, 64); + MultiThreadingMgr::instance().apply(false, 64, 256); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); } // thread count should match EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); // use scope to test constructor and destructor { MultiThreadingCriticalSection cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); // use scope to test constructor and destructor { MultiThreadingCriticalSection inner_cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); } // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); } // thread count should match EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 0); + // queue size should be 0 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 0); // use scope to test constructor and destructor { MultiThreadingCriticalSection cs; // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); // apply multi-threading configuration with 64 threads - MultiThreadingMgr::instance().apply(true, 64); + MultiThreadingMgr::instance().apply(true, 64, 256); // thread pool should be stopped EXPECT_EQ(thread_pool.size(), 0); + // thread count should be 64 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 64); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); } // thread count should match EXPECT_EQ(thread_pool.size(), 64); + // thread count should be 64 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadPoolSize(), 64); + // queue size should be 256 + EXPECT_EQ(MultiThreadingMgr::instance().getThreadQueueSize(), 256); // apply multi-threading configuration with 0 threads - MultiThreadingMgr::instance().apply(false, 0); + MultiThreadingMgr::instance().apply(false, 0, 0); }