From: Marcin Siodelski Date: Mon, 11 Dec 2017 10:29:27 +0000 (+0100) Subject: [5442] Addressed review comments. X-Git-Tag: trac5443_base~1 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=7d46d8910295e73289eb6f881501faf45a815e47;p=thirdparty%2Fkea.git [5442] Addressed review comments. --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index ca2e58abdc..97ecfa5cf3 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -805,10 +805,11 @@ Dhcpv4Srv::run_one() { } // If the DHCP service has been globally disabled, drop the packet. - if (network_state_.isServiceEnabled()) { + if (!network_state_.isServiceEnabled()) { LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_BASIC, DHCP4_PACKET_DROP_0008) .arg(query->getLabel()); + } else { processPacket(query, rsp); } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index eeadd5e70d..dceb6367fb 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -418,7 +418,7 @@ ControlledDhcpv6Srv::commandDhcpDisableHandler(const std::string&, if (message.tellp() == 0) { network_state_.disableService(); - message << "DHCPv4 service disabled"; + message << "DHCPv6 service disabled"; if (max_period > 0) { message << " for " << max_period << " seconds"; } diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index d895a45493..0d6c9caebe 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -416,7 +416,7 @@ service may be enabled by the "dhcp-enable" control command or automatically after a specified amount of time since receiving "dhcp-disable" command. % DHCP6_PACKET_DROP_PARSE_FAIL failed to parse packet from %1 to %2, received over interface %3, reason: %4 -The DHCPv4 server has received a packet that it is unable to +The DHCPv6 server has received a packet that it is unable to interpret. The reason why the packet is invalid is included in the message. % DHCP6_PACKET_DROP_SERVERID_MISMATCH %1: dropping packet with server identifier: %2, server is using: %3 diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index c39073519e..08c2dc13d2 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -469,10 +469,11 @@ void Dhcpv6Srv::run_one() { } // If the DHCP service has been globally disabled, drop the packet. - if (network_state_.isServiceEnabled()) { + if (!network_state_.isServiceEnabled()) { LOG_DEBUG(bad_packet6_logger, DBG_DHCP6_DETAIL_DATA, DHCP6_PACKET_DROP_DHCP_DISABLED) .arg(query->getLabel()); + } else { processPacket(query, rsp); } diff --git a/src/lib/dhcpsrv/network_state.cc b/src/lib/dhcpsrv/network_state.cc index 8e757607d3..552f7c76d8 100644 --- a/src/lib/dhcpsrv/network_state.cc +++ b/src/lib/dhcpsrv/network_state.cc @@ -52,7 +52,7 @@ public: destroyTimer(); } - /// @brief Creates a timer couting the time when @c enableAll should be + /// @brief Creates a timer counting the time when @c enableAll should be /// automatically called. /// /// If the timer has been already scheduled, it is destroyed and replaced @@ -67,7 +67,7 @@ public: shared_from_this()), seconds * 1000, asiolink::IntervalTimer::ONE_SHOT); - timer_mgr_->setup("network-state-timer"); + timer_mgr_->setup(NETWORK_STATE_TIMER_NAME); } /// @brief Destroys a timer if present. diff --git a/src/lib/dhcpsrv/network_state.h b/src/lib/dhcpsrv/network_state.h index cfd48a3615..137c29bbed 100644 --- a/src/lib/dhcpsrv/network_state.h +++ b/src/lib/dhcpsrv/network_state.h @@ -23,7 +23,7 @@ class NetworkStateImpl; /// When the DHCP server receives a command to disable DHCP service entirely /// or for specific networks, this has to be recorded to allow for re-enabling /// DHCP service for these networks as a result of receiving a command from -/// the administrator or when the tiemout for re-enabling the service occurs. +/// the administrator or when the timeout for re-enabling the service occurs. /// /// In the future, it will be possible to specify "disabled" parameter for /// a subnet (or network) in the configuration file to indicate that this subnet diff --git a/src/lib/dhcpsrv/tests/network_state_unittest.cc b/src/lib/dhcpsrv/tests/network_state_unittest.cc index ab2837b350..b21be026e7 100644 --- a/src/lib/dhcpsrv/tests/network_state_unittest.cc +++ b/src/lib/dhcpsrv/tests/network_state_unittest.cc @@ -49,7 +49,7 @@ public: /// @brief Runs IO service with a timeout. /// - /// @param timeout_ms Timeout for runninr IO service in milliseconds. + /// @param timeout_ms Timeout for running IO service in milliseconds. void runIOService(const long timeout_ms) { test_timer_.setup(boost::bind(&NetworkStateTest::testTimerCallback, this), timeout_ms, IntervalTimer::ONE_SHOT); @@ -63,8 +63,8 @@ public: IntervalTimer test_timer_; }; -// This test verifies that it is possible to disable and then enable service. -TEST_F(NetworkStateTest, disableEnableService) { +// This test verifies that it is possible to disable and then enable DHCPv4 service. +TEST_F(NetworkStateTest, disableEnableService4) { NetworkState state(NetworkState::DHCPv4); state.disableService(); EXPECT_FALSE(state.isServiceEnabled()); @@ -72,6 +72,15 @@ TEST_F(NetworkStateTest, disableEnableService) { 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); + state.disableService(); + EXPECT_FALSE(state.isServiceEnabled()); + state.enableService(); + EXPECT_TRUE(state.isServiceEnabled()); +} + // This test verifies that enableAll() enables the service. This test will be extended // in the future to verify that it also enables disabled scopes. TEST_F(NetworkStateTest, enableAll) {