[#1601] use internal state to differentiate between actors affecting the network...

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index 96ebb585ef6baf7491b1aa2c61f5166a5770fe43..af15fbbe23d2c861d42f841fd37825e701542079 100644 (file)
--- 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<unsigned>(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 6e32c4a284291060f0d464a3f3ea4d126d19b511..e771b8549ff2e15dd2e9dbb92a9754f0046c7b2e 100644 (file)
--- 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<unsigned>(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 2a5bdf8c2f9cc0aa89eab78a347b477de198499a..6944bbcb89a6fac3958cdf6982714f10cb01ac32 100644 (file)
--- 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 a006c2d4835ad881aae5de18e1a2ae7e4b1f4f56..e92320386d1ca82cd7266c282d7947d9d543c627 100644 (file)
--- 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 bcabaea9e56172ffc94dbde40f6a003b44f1808f..9d067c8114bba25293df2573313157253cfeca0d 100644 (file)
--- 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