From: Razvan Becheriu Date: Thu, 10 Dec 2020 15:39:00 +0000 (+0200) Subject: [#1601] use internal state to differentiate between actors affecting the network... X-Git-Tag: Kea-1.9.4~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d7ea6fcac06c36dda70c87db57db39faab127d23;p=thirdparty%2Fkea.git [#1601] use internal state to differentiate between actors affecting the network state --- diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 96ebb585ef..af15fbbe23 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -259,7 +259,7 @@ ControlledDhcpv4Srv::commandConfigReloadHandler(const string&, } catch (const std::exception& ex) { // Log the unsuccessful reconfiguration. The reason for failure // should be already logged. Don't rethrow an exception so as - // the control channel perhaps keeps working. + // the server keeps working. LOG_FATAL(dhcp4_logger, DHCP4_DYNAMIC_RECONFIGURATION_FAIL) .arg(file); return (createAnswer(CONTROL_RESULT_ERROR, @@ -405,7 +405,7 @@ ControlledDhcpv4Srv::commandConfigSetHandler(const string&, // the logging first in case there's a configuration failure. int rcode = 0; isc::config::parseAnswer(rcode, result); - if (rcode == 0) { + if (rcode == CONTROL_RESULT_SUCCESS) { CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); // Use new configuration. @@ -490,6 +490,9 @@ ControlledDhcpv4Srv::commandDhcpDisableHandler(const std::string&, ConstElementPtr args) { std::ostringstream message; int64_t max_period = 0; + int64_t handle_id = 0; + + NetworkState::ControllerType type = NetworkState::COMMAND; // Parse arguments to see if the 'max-period' parameter has been specified. if (args) { @@ -518,12 +521,28 @@ ControlledDhcpv4Srv::commandDhcpDisableHandler(const std::string&, network_state_->delayedEnableAll(static_cast(max_period)); } } + ConstElementPtr handle_id_element = args->get("handle-id"); + // handle-id is optional. + if (handle_id_element) { + // It must be an integer, if specified. + if (handle_id_element->getType() != Element::integer) { + message << "'handle-id' argument must be a number"; + + } else { + // It must be positive integer. + handle_id = handle_id_element->intValue(); + if (handle_id <= 0) { + message << "'handle-id' must be positive integer"; + } + type = NetworkState::HA; + } + } } } // No error occurred, so let's disable the service. if (message.tellp() == 0) { - network_state_->disableService(NetworkState::COMMAND); + network_state_->disableService(type); message << "DHCPv4 service disabled"; if (max_period > 0) { @@ -538,9 +557,49 @@ ControlledDhcpv4Srv::commandDhcpDisableHandler(const std::string&, } ConstElementPtr -ControlledDhcpv4Srv::commandDhcpEnableHandler(const std::string&, ConstElementPtr) { - network_state_->enableService(NetworkState::COMMAND); - return (config::createAnswer(CONTROL_RESULT_SUCCESS, "DHCP service successfully enabled")); +ControlledDhcpv4Srv::commandDhcpEnableHandler(const std::string&, + ConstElementPtr args) { + std::ostringstream message; + int64_t handle_id = 0; + + NetworkState::ControllerType type = NetworkState::COMMAND; + + // Parse arguments to see if the 'max-period' parameter has been specified. + if (args) { + // Arguments must be a map. + if (args->getType() != Element::map) { + message << "arguments for the 'dhcp-enable' command must be a map"; + + } else { + ConstElementPtr handle_id_element = args->get("handle-id"); + // handle-id is optional. + if (handle_id_element) { + // It must be an integer, if specified. + if (handle_id_element->getType() != Element::integer) { + message << "'handle-id' argument must be a number"; + + } else { + // It must be positive integer. + handle_id = handle_id_element->intValue(); + if (handle_id <= 0) { + message << "'handle-id' must be positive integer"; + } + type = NetworkState::HA; + } + } + } + } + + // No error occurred, so let's disable the service. + if (message.tellp() == 0) { + network_state_->enableService(type); + + // Success. + return (config::createAnswer(CONTROL_RESULT_SUCCESS, "DHCP service successfully enabled")); + } + + // Failure. + return (config::createAnswer(CONTROL_RESULT_ERROR, message.str())); } ConstElementPtr @@ -817,6 +876,8 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { CfgDbAccessPtr cfg_db = CfgMgr::instance().getStagingCfg()->getCfgDbAccess(); cfg_db->setAppendedParameters("universe=4"); cfg_db->createManagers(); + // Reset counters related to connections as all managers have been recreated. + srv->getNetworkState()->resetInternalCounters(); } catch (const std::exception& ex) { err << "Unable to open database: " << ex.what(); return (isc::config::createAnswer(1, err.str())); diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 6e32c4a284..e771b8549f 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -493,6 +493,9 @@ ControlledDhcpv6Srv::commandDhcpDisableHandler(const std::string&, ConstElementPtr args) { std::ostringstream message; int64_t max_period = 0; + int64_t handle_id = 0; + + NetworkState::ControllerType type = NetworkState::COMMAND; // Parse arguments to see if the 'max-period' parameter has been specified. if (args) { @@ -521,12 +524,28 @@ ControlledDhcpv6Srv::commandDhcpDisableHandler(const std::string&, network_state_->delayedEnableAll(static_cast(max_period)); } } + ConstElementPtr handle_id_element = args->get("handle-id"); + // handle-id is optional. + if (handle_id_element) { + // It must be an integer, if specified. + if (handle_id_element->getType() != Element::integer) { + message << "'handle-id' argument must be a number"; + + } else { + // It must be positive integer. + handle_id = handle_id_element->intValue(); + if (handle_id <= 0) { + message << "'handle-id' must be positive integer"; + } + type = NetworkState::HA; + } + } } } // No error occurred, so let's disable the service. if (message.tellp() == 0) { - network_state_->disableService(NetworkState::COMMAND); + network_state_->disableService(type); message << "DHCPv6 service disabled"; if (max_period > 0) { @@ -541,9 +560,49 @@ ControlledDhcpv6Srv::commandDhcpDisableHandler(const std::string&, } ConstElementPtr -ControlledDhcpv6Srv::commandDhcpEnableHandler(const std::string&, ConstElementPtr) { - network_state_->enableService(NetworkState::COMMAND); - return (config::createAnswer(CONTROL_RESULT_SUCCESS, "DHCP service successfully enabled")); +ControlledDhcpv6Srv::commandDhcpEnableHandler(const std::string&, + ConstElementPtr args) { + std::ostringstream message; + int64_t handle_id = 0; + + NetworkState::ControllerType type = NetworkState::COMMAND; + + // Parse arguments to see if the 'max-period' parameter has been specified. + if (args) { + // Arguments must be a map. + if (args->getType() != Element::map) { + message << "arguments for the 'dhcp-enable' command must be a map"; + + } else { + ConstElementPtr handle_id_element = args->get("handle-id"); + // handle-id is optional. + if (handle_id_element) { + // It must be an integer, if specified. + if (handle_id_element->getType() != Element::integer) { + message << "'handle-id' argument must be a number"; + + } else { + // It must be positive integer. + handle_id = handle_id_element->intValue(); + if (handle_id <= 0) { + message << "'handle-id' must be positive integer"; + } + type = NetworkState::HA; + } + } + } + } + + // No error occurred, so let's disable the service. + if (message.tellp() == 0) { + network_state_->enableService(type); + + // Success. + return (config::createAnswer(CONTROL_RESULT_SUCCESS, "DHCP service successfully enabled")); + } + + // Failure. + return (config::createAnswer(CONTROL_RESULT_ERROR, message.str())); } ConstElementPtr @@ -820,6 +879,8 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { CfgDbAccessPtr cfg_db = CfgMgr::instance().getStagingCfg()->getCfgDbAccess(); cfg_db->setAppendedParameters("universe=6"); cfg_db->createManagers(); + // Reset counters related to connections as all managers have been recreated. + srv->getNetworkState()->resetInternalCounters(); } catch (const std::exception& ex) { err << "Unable to open database: " << ex.what(); return (isc::config::createAnswer(1, err.str())); diff --git a/src/lib/dhcpsrv/network_state.cc b/src/lib/dhcpsrv/network_state.cc index 2a5bdf8c2f..6944bbcb89 100644 --- a/src/lib/dhcpsrv/network_state.cc +++ b/src/lib/dhcpsrv/network_state.cc @@ -62,7 +62,9 @@ public: disabled_by_command_ = false; break; case NetworkState::CONNECTION: - --disabled_by_connection_; + if (disabled_by_connection_) { + --disabled_by_connection_; + } break; case NetworkState::HA: disabled_by_ha_ = false; @@ -75,6 +77,15 @@ public: } } + /// @brief Reset internal counters. + void resetInternalCounters() { + disabled_by_connection_ = 0; + if (!disabled_by_command_ && disabled_by_connection_ == 0 && + !disabled_by_ha_) { + globally_disabled_ = false; + } + } + /// @brief Enables DHCP service globally and per scopes. /// /// If delayed enabling DHCP service has been scheduled, it cancels it. @@ -154,6 +165,11 @@ NetworkState::enableService(const ControllerType& type) { impl_->setDisableService(false, type); } +void +NetworkState::resetInternalCounters() { + impl_->resetInternalCounters(); +} + void NetworkState::enableAll() { impl_->enableAll(); diff --git a/src/lib/dhcpsrv/network_state.h b/src/lib/dhcpsrv/network_state.h index a006c2d483..e92320386d 100644 --- a/src/lib/dhcpsrv/network_state.h +++ b/src/lib/dhcpsrv/network_state.h @@ -96,6 +96,11 @@ public: /// @param type Controller type which issued the state transition. void enableService(const ControllerType& type); + /// @brief Reset internal counters. + /// + /// Reset internal counters. + void resetInternalCounters(); + /// @brief Enables DHCP service globally and for scopes which have been /// disabled as a result of control command. void enableAll(); diff --git a/src/lib/dhcpsrv/tests/network_state_unittest.cc b/src/lib/dhcpsrv/tests/network_state_unittest.cc index bcabaea9e5..9d067c8114 100644 --- a/src/lib/dhcpsrv/tests/network_state_unittest.cc +++ b/src/lib/dhcpsrv/tests/network_state_unittest.cc @@ -76,18 +76,188 @@ TEST_F(NetworkStateTest, default) { // This test verifies that it is possible to disable and then enable DHCPv4 service. TEST_F(NetworkStateTest, disableEnableService4) { NetworkState state(NetworkState::DHCPv4); + + // Test that enable/disable command works state.disableService(NetworkState::COMMAND); EXPECT_FALSE(state.isServiceEnabled()); state.enableService(NetworkState::COMMAND); EXPECT_TRUE(state.isServiceEnabled()); + + // Test that command does not use internal counter + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_TRUE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that enable/disable ha works + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that command does not use internal counter + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that enable/disable connection works + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that connection uses internal counter + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that a combination properly affect the state + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); } // This test verifies that it is possible to disable and then enable DHCPv6 service. TEST_F(NetworkStateTest, disableEnableService6) { NetworkState state(NetworkState::DHCPv6); + + // Test that enable/disable command works + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that command does not use internal counter + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_TRUE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that enable/disable ha works + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that command does not use internal counter + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that enable/disable connection works + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that connection uses internal counter + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); + + // Test that a combination properly affect the state + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); state.disableService(NetworkState::COMMAND); EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); state.enableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::CONNECTION); + EXPECT_TRUE(state.isServiceEnabled()); +} + +// This test verifies that resetInternalCounters works, so that internal counter +// is reset after all managers are recreated. +TEST_F(NetworkStateTest, resetInternalCounters) { + NetworkState state(NetworkState::DHCPv4); + state.disableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::HA); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.resetInternalCounters(); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::COMMAND); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(NetworkState::HA); + EXPECT_TRUE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.disableService(NetworkState::CONNECTION); + EXPECT_FALSE(state.isServiceEnabled()); + state.resetInternalCounters(); EXPECT_TRUE(state.isServiceEnabled()); } @@ -152,5 +322,4 @@ TEST_F(NetworkStateTest, multipleDelayedEnableAll) { EXPECT_FALSE(state.isDelayedEnableAll()); } - } // end of anonymous namespace