From f08a02278d794a2b692444db5d04be3fce4da18d Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 28 Nov 2023 16:00:43 -0500 Subject: [PATCH] [#3110] HA decline updates working Updates are working, need clean up and UTs in HA. src/bin/dhcp4/dhcp4_hooks.dox Updated hooks dev guide src/bin/dhcp4/dhcp4_srv.cc Added callout lease4_server_decline Dhcpv4Srv::serverDecline() - added lease4_server_decline hook point src/bin/dhcp4/tests/hooks_unittest.cc TEST_F(HooksDhcpv4SrvTest, lease4OfferDiscoverDecline) - new test src/hooks/dhcp/high_availability/ha_callouts.cc lease4_server_decline() - new callout function src/hooks/dhcp/high_availability/ha_impl.* HAImpl::lease4ServerDecline() - new callout handler src/hooks/dhcp/high_availability/ha_messages.mes HA_LEASE4_SERVER_DECLINE_FAILED - new log src/hooks/dhcp/high_availability/ha_service.* HAService::asyncSendLeaseUpdate() - new function for a single lease update HAService::asyncSendLeaseUpdates() - check parking lot null --- src/bin/dhcp4/dhcp4_hooks.dox | 28 +++- src/bin/dhcp4/dhcp4_srv.cc | 80 ++++------ src/bin/dhcp4/tests/hooks_unittest.cc | 139 ++++++++++++++++++ .../dhcp/high_availability/ha_callouts.cc | 22 +++ src/hooks/dhcp/high_availability/ha_impl.cc | 35 +++++ src/hooks/dhcp/high_availability/ha_impl.h | 5 + .../dhcp/high_availability/ha_messages.cc | 2 + .../dhcp/high_availability/ha_messages.h | 1 + .../dhcp/high_availability/ha_messages.mes | 7 + .../dhcp/high_availability/ha_service.cc | 57 ++++++- src/hooks/dhcp/high_availability/ha_service.h | 4 + 11 files changed, 327 insertions(+), 53 deletions(-) diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index 6051cee847..4fe12b613f 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -340,8 +340,32 @@ called before "subnet4_select". - Next step status: If any callout installed on the "lease4_offer" hook point sets the next step action to DROP, the server will drop the processed query and discard the prepared DHCPOFFER rather than sending it to the client. - Any further action, such as marking the discarded DHCPOFFER's lease as declined, - is the responsibility of the callout. + If it sets the next step action to PARK, the server will park the processed + packet (hold packet processing) until the hook libraries explicitly unpark + the packet after they are done performing asynchronous operations. + + Prior to unparking the packet, callouts are expected to set the argument, + "offer_address_in_use" to false if the address is available and the offer should + be sent to the client. If the address should be declined, callouts should set + the argument to true. This will cause the server to discard the DHCPOFFER, mark + the lease DECLINED in the lease store, update the relevant statitics, and then + invoke callouts installed for lease4_server_decline. + +@subsection dhcpv4Lease4ServerDecline lease4_server_decline + + - @b Arguments: + - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: in + - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: in + + - @b Description: this callout is executed after a lease4_offer callout has + determined that the lease to be offered is already in use by an unknown + client. At this point in processing, the offer has been discarded, the lease + has been marked DECLINED in the lease store, and the relevant statistics have + been updated. The argument "query4" contains a pointer to an isc::dhcp::Pkt4 + object that contains all information regarding incoming DHCPDISCOVER packet. + The "leases4" object will hold the pointer to the declined lease. + + - Next step status: Not applicable, its value will be ignored. @subsection dhcpv4HooksPkt4Send pkt4_send diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 258066727e..71c385b963 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -89,31 +89,33 @@ namespace { /// Structure that holds registered hook indexes struct Dhcp4Hooks { - int hook_index_buffer4_receive_; ///< index for "buffer4_receive" hook point - int hook_index_pkt4_receive_; ///< index for "pkt4_receive" hook point - int hook_index_subnet4_select_; ///< index for "subnet4_select" hook point - int hook_index_leases4_committed_; ///< index for "leases4_committed" hook point - int hook_index_lease4_release_; ///< index for "lease4_release" hook point - int hook_index_pkt4_send_; ///< index for "pkt4_send" hook point - int hook_index_buffer4_send_; ///< index for "buffer4_send" hook point - int hook_index_lease4_decline_; ///< index for "lease4_decline" hook point - int hook_index_host4_identifier_; ///< index for "host4_identifier" hook point - int hook_index_ddns4_update_; ///< index for "ddns4_update" hook point - int hook_index_lease4_offer_; ///< index for "lease4_offer" hook point + int hook_index_buffer4_receive_; ///< index for "buffer4_receive" hook point + int hook_index_pkt4_receive_; ///< index for "pkt4_receive" hook point + int hook_index_subnet4_select_; ///< index for "subnet4_select" hook point + int hook_index_leases4_committed_; ///< index for "leases4_committed" hook point + int hook_index_lease4_release_; ///< index for "lease4_release" hook point + int hook_index_pkt4_send_; ///< index for "pkt4_send" hook point + int hook_index_buffer4_send_; ///< index for "buffer4_send" hook point + int hook_index_lease4_decline_; ///< index for "lease4_decline" hook point + int hook_index_host4_identifier_; ///< index for "host4_identifier" hook point + int hook_index_ddns4_update_; ///< index for "ddns4_update" hook point + int hook_index_lease4_offer_; ///< index for "lease4_offer" hook point + int hook_index_lease4_server_decline_; ///< index for "lease4_server_decline" hook point /// Constructor that registers hook points for DHCPv4 engine Dhcp4Hooks() { - hook_index_buffer4_receive_ = HooksManager::registerHook("buffer4_receive"); - hook_index_pkt4_receive_ = HooksManager::registerHook("pkt4_receive"); - hook_index_subnet4_select_ = HooksManager::registerHook("subnet4_select"); - hook_index_leases4_committed_ = HooksManager::registerHook("leases4_committed"); - hook_index_lease4_release_ = HooksManager::registerHook("lease4_release"); - hook_index_pkt4_send_ = HooksManager::registerHook("pkt4_send"); - hook_index_buffer4_send_ = HooksManager::registerHook("buffer4_send"); - hook_index_lease4_decline_ = HooksManager::registerHook("lease4_decline"); - hook_index_host4_identifier_ = HooksManager::registerHook("host4_identifier"); - hook_index_ddns4_update_ = HooksManager::registerHook("ddns4_update"); - hook_index_lease4_offer_ = HooksManager::registerHook("lease4_offer"); + hook_index_buffer4_receive_ = HooksManager::registerHook("buffer4_receive"); + hook_index_pkt4_receive_ = HooksManager::registerHook("pkt4_receive"); + hook_index_subnet4_select_ = HooksManager::registerHook("subnet4_select"); + hook_index_leases4_committed_ = HooksManager::registerHook("leases4_committed"); + hook_index_lease4_release_ = HooksManager::registerHook("lease4_release"); + hook_index_pkt4_send_ = HooksManager::registerHook("pkt4_send"); + hook_index_buffer4_send_ = HooksManager::registerHook("buffer4_send"); + hook_index_lease4_decline_ = HooksManager::registerHook("lease4_decline"); + hook_index_host4_identifier_ = HooksManager::registerHook("host4_identifier"); + hook_index_ddns4_update_ = HooksManager::registerHook("ddns4_update"); + hook_index_lease4_offer_ = HooksManager::registerHook("lease4_offer"); + hook_index_lease4_server_decline_ = HooksManager::registerHook("lease4_server_decline"); } }; @@ -3949,8 +3951,11 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline, } void -Dhcpv4Srv::serverDecline(hooks::CalloutHandlePtr& /* callout_handle */, Pkt4Ptr& query, +Dhcpv4Srv::serverDecline(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, Lease4Ptr lease, bool lease_exists) { + LOG_INFO(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE) + .arg(lease->addr_.toText()) + .arg(lease->valid_lft_); { // Check if the resource is busy i.e. can be modified by another thread @@ -4039,47 +4044,25 @@ Dhcpv4Srv::serverDecline(hooks::CalloutHandlePtr& /* callout_handle */, Pkt4Ptr& StatsMgr::instance().addValue("assigned-addresses", static_cast(1)); } - /* (comment it out so picky tools don't flag this as dead code - // - /// @todo #3110, HA will implement a handler for this hook point to - /// propagate an update of the lease to peers. - // // Let's check if there are hooks installed for server decline hook point. - // If they are, let's pass the lease and client's packet. Note this code - // has never been compiled, it is just an initial draft. + // If there are, let's pass the DHCPDISCOVER and the declined lease . if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_server_decline_)) { - CalloutHandlePtr callout_handle = getCalloutHandle(decline); - // Use the RAII wrapper to make sure that the callout handle state is // reset when this object goes out of scope. All hook points must do // it to prevent possible circular dependency between the callout // handle and its arguments. ScopedCalloutHandleState callout_handle_state(callout_handle); - // Pass the original packet + // Pass in the original DHCPDISCOVER callout_handle->setArgument("query4", query); - // Pass the lease to be updated + // Pass in the declined lease. callout_handle->setArgument("lease4", lease); // Call callouts HooksManager::callCallouts(Hooks.hook_index_lease4_server_decline_, *callout_handle); - - // Check if callouts decided to skip the next processing step. - // If any of them did, we will drop the packet. - if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_SKIP) || - (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP)) { - LOG_DEBUG(hooks_logger, DBGLVL_PKT_HANDLING, DHCP4_HOOK_SERVER_DECLINE_SKIP) - .arg(query->getLabel()).arg(lease->addr_.toText()); - return; - } } - */ - - LOG_INFO(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE) - .arg(lease->addr_.toText()) - .arg(lease->valid_lft_); } void @@ -4092,7 +4075,6 @@ Dhcpv4Srv::serverDeclineNoThrow(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr } } - Pkt4Ptr Dhcpv4Srv::processInform(Pkt4Ptr& inform, AllocEngine::ClientContext4Ptr& context) { // server-id is supposed to be forbidden (as is requested address) diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index 27746cb6cf..dbf9d9e7a0 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -58,6 +58,7 @@ TEST_F(Dhcpv4SrvTest, Hooks) { int hook_index_leases4_committed = -1; int hook_index_host4_identifier = -1; int hook_index_lease4_offer = -1; + int hook_index_lease4_server_decline = -1; // check if appropriate indexes are set EXPECT_NO_THROW(hook_index_dhcp4_srv_configured = ServerHooks::getServerHooks() @@ -84,6 +85,8 @@ TEST_F(Dhcpv4SrvTest, Hooks) { .getIndex("host4_identifier")); EXPECT_NO_THROW(hook_index_lease4_offer = ServerHooks::getServerHooks() .getIndex("lease4_offer")); + EXPECT_NO_THROW(hook_index_lease4_server_decline = ServerHooks::getServerHooks() + .getIndex("lease4_server_decline")); EXPECT_TRUE(hook_index_dhcp4_srv_configured > 0); EXPECT_TRUE(hook_index_buffer4_receive > 0); @@ -97,6 +100,7 @@ TEST_F(Dhcpv4SrvTest, Hooks) { EXPECT_TRUE(hook_index_leases4_committed > 0); EXPECT_TRUE(hook_index_host4_identifier > 0); EXPECT_TRUE(hook_index_lease4_offer > 0); + EXPECT_TRUE(hook_index_lease4_server_decline > 0); } // A dummy MAC address, padded with 0s @@ -164,6 +168,7 @@ public: HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_decline"); HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("host4_identifier"); HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_offer"); + HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts("lease4_server_decline"); HooksManager::setTestMode(false); bool status = HooksManager::unloadLibraries(); @@ -836,6 +841,39 @@ public: return (0); } + static int + lease4_offer_park_in_use_callout(CalloutHandle& callout_handle) { + callback_name_ = string("lease4_offer"); + + callout_handle.getArgument("query4", callback_qry_pkt4_); + + callout_handle.setArgument("offer_address_in_use", true); + io_service_->post(std::bind(&HooksDhcpv4SrvTest::pkt4_unpark_callout, + callout_handle.getParkingLotHandlePtr(), + callback_qry_pkt4_)); + + callout_handle.getParkingLotHandlePtr()->reference(callback_qry_pkt4_); + callout_handle.setStatus(CalloutHandle::NEXT_STEP_PARK); + + Lease4CollectionPtr leases4; + callout_handle.getArgument("leases4", leases4); + if (leases4->size() > 0) { + callback_lease4_ = leases4->at(0); + } + + callout_handle.getArgument("offer_lifetime", callback_offer_lft_); + callout_handle.getArgument("old_lease", callback_old_lease_); + callback_argument_names_ = callout_handle.getArgumentNames(); + sort(callback_argument_names_.begin(), callback_argument_names_.end()); + + if (callback_qry_pkt4_) { + callback_qry_options_copy_ = callback_qry_pkt4_->isCopyRetrievedOptions(); + } + + return (0); + } + + /// @brief Test callback that stores callout name and passed parameters. /// /// @param callout_handle handle passed by the hooks framework @@ -868,6 +906,29 @@ public: return (0); } + /// @brief Test callback that stores callout name and passed parameters. + /// + /// @param callout_handle handle passed by the hooks framework + /// @return always 0 + static int + lease4_server_decline_callout(CalloutHandle& callout_handle) { + callback_name_ = string("lease4_server_decline"); + callout_handle.getArgument("query4", callback_qry_pkt4_); + + Lease4Ptr lease4; + callout_handle.getArgument("lease4", lease4); + callback_lease4_ = lease4; + + callback_argument_names_ = callout_handle.getArgumentNames(); + sort(callback_argument_names_.begin(), callback_argument_names_.end()); + + if (callback_qry_pkt4_) { + callback_qry_options_copy_ = callback_qry_pkt4_->isCopyRetrievedOptions(); + } + + return (0); + } + /// @brief Test callback which asks the server to unpark the packet. /// /// This can be used with hook points: leases4_committed, lease4_offer. @@ -3881,4 +3942,82 @@ TEST_F(HooksDhcpv4SrvTest, lease4OfferParkedPacketLimit) { EXPECT_EQ(1, getStatistic("pkt4-receive-drop")); } +// This test verifies that a lease4_offer callout that marks a +// lease as in-use and unparks the query, causes the offer to be +// discarded, and Dhcpv4Srv::serverDecline() to be invoked. This should, +// in turn, cause the lease to be declined in the lease store and the +// callout for lease4_server_decline to be called. +TEST_F(HooksDhcpv4SrvTest, lease4OfferDiscoverDecline) { + IfaceMgrTestConfig test_config(true); + IfaceMgr::instance().openSockets4(); + + // Install lease4_offer callout that will mark lease as in-use and unpark + ASSERT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( + "lease4_offer", lease4_offer_park_in_use_callout)); + + // Install lease4_server_decline callout + ASSERT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( + "lease4_server_decline", lease4_server_decline_callout)); + + // Make sure there's no existing lease. + IOAddress expected_address("192.0.2.100"); + ASSERT_FALSE(LeaseMgrFactory::instance().getLease4(expected_address)); + + // Generate a DISCOVER. + Dhcp4Client client(Dhcp4Client::SELECTING); + client.setIfaceName("eth1"); + client.setIfaceIndex(ETH1_INDEX); + ASSERT_NO_THROW(client.doDiscover()); + + // Check that the callback called is indeed the one we installed + EXPECT_EQ("lease4_offer", callback_name_); + + // Check if all expected parameters were really received + vector expected_argument_names; + expected_argument_names.push_back("query4"); + expected_argument_names.push_back("leases4"); + expected_argument_names.push_back("offer_lifetime"); + expected_argument_names.push_back("old_lease"); + expected_argument_names.push_back("offer_address_in_use"); + + sort(expected_argument_names.begin(), expected_argument_names.end()); + EXPECT_TRUE(callback_argument_names_ == expected_argument_names); + + // Declined lease should be returned. + ASSERT_TRUE(callback_lease4_); + EXPECT_EQ(expected_address, callback_lease4_->addr_); + + // Since the callout set offer_address_in_use flag to true the offer should + // have been discarded. Make sure that we did not receive a repsonse. + ASSERT_FALSE(client.getContext().response_); + + // Clear static buffers + resetCalloutBuffers(); + + // Polling the IOService should unpark the packet invoking the unpark lambda + // which should invoke Dhcp4Srv::serverDecline(). This should decline the + // lease in the db and then invoke lease4_server_decline callback. + ASSERT_NO_THROW(io_service_->poll()); + + // The lease should be in the lease store and in the DECLINED state. + Lease4Ptr declined_lease = LeaseMgrFactory::instance().getLease4(callback_lease4_->addr_); + ASSERT_TRUE(declined_lease); + EXPECT_EQ(declined_lease->state_, Lease::STATE_DECLINED); + + // Check that we called lease4_server_decline callback. + EXPECT_EQ("lease4_server_decline", callback_name_); + + // Check if all expected parameters were really received + expected_argument_names.clear(); + expected_argument_names.push_back("query4"); + expected_argument_names.push_back("lease4"); + + sort(expected_argument_names.begin(), expected_argument_names.end()); + EXPECT_TRUE(callback_argument_names_ == expected_argument_names); + + // Declined lease should be returned. + ASSERT_TRUE(callback_lease4_); + EXPECT_EQ(expected_address, callback_lease4_->addr_); +} + } // namespace diff --git a/src/hooks/dhcp/high_availability/ha_callouts.cc b/src/hooks/dhcp/high_availability/ha_callouts.cc index 6e2bf27b1a..7e7b43ae22 100644 --- a/src/hooks/dhcp/high_availability/ha_callouts.cc +++ b/src/hooks/dhcp/high_availability/ha_callouts.cc @@ -115,6 +115,28 @@ int leases4_committed(CalloutHandle& handle) { return (0); } +/// @brief lease4_server_decline callout implementation. +/// +/// @param handle callout handle. +int lease4_server_decline(CalloutHandle& handle) { + CalloutHandle::CalloutNextStep status = handle.getStatus(); + if (status == CalloutHandle::NEXT_STEP_DROP || + status == CalloutHandle::NEXT_STEP_SKIP) { + return (0); + } + + try { + impl->lease4ServerDecline(handle); + } catch (const std::exception& ex) { + LOG_ERROR(ha_logger, HA_LEASE4_SERVER_DECLINE_FAILED) + .arg(ex.what()); + return (1); + } + + return (0); +} + + /// @brief dhcp6_srv_configured callout implementation. /// /// @param handle callout handle. diff --git a/src/hooks/dhcp/high_availability/ha_impl.cc b/src/hooks/dhcp/high_availability/ha_impl.cc index 192a7ca53a..4138a133b3 100644 --- a/src/hooks/dhcp/high_availability/ha_impl.cc +++ b/src/hooks/dhcp/high_availability/ha_impl.cc @@ -185,6 +185,41 @@ HAImpl::leases4Committed(CalloutHandle& callout_handle) { callout_handle.setStatus(CalloutHandle::NEXT_STEP_PARK); } +void +HAImpl::lease4ServerDecline(CalloutHandle& callout_handle) { + // If the hook library is configured to not send lease updates to the + // partner, there is nothing to do because this whole callout is + // currently about sending lease updates. + if (!config_->amSendingLeaseUpdates()) { + // No need to log it, because it was already logged when configuration + // was applied. + return; + } + + Pkt4Ptr query4; + Lease4Ptr lease4; + + // Get all arguments available for the lease4_server_decline hook point. + // If any of these arguments is not available this is a programmatic + // error. An exception will be thrown which will be caught by the + // caller and logged. + callout_handle.getArgument("query4", query4); + callout_handle.getArgument("lease4", lease4); + + // Get the parking lot for this hook point. We're going to remember this + // pointer until we unpark the packet. + ParkingLotHandlePtr parking_lot = callout_handle.getParkingLotHandlePtr(); + + // Asynchronously send lease updates. In some cases no updates will be sent, + // e.g. when this server is in the partner-down state and there are no backup + // servers. In those cases we simply return without parking the DHCP query. + // The response will be sent to the client immediately. + service_->asyncSendLeaseUpdate(query4, lease4, parking_lot); + + callout_handle.setStatus(CalloutHandle::NEXT_STEP_CONTINUE); +} + + void HAImpl::buffer6Receive(hooks::CalloutHandle& callout_handle) { Pkt6Ptr query6; diff --git a/src/hooks/dhcp/high_availability/ha_impl.h b/src/hooks/dhcp/high_availability/ha_impl.h index 9b26f9dfa2..986c204158 100644 --- a/src/hooks/dhcp/high_availability/ha_impl.h +++ b/src/hooks/dhcp/high_availability/ha_impl.h @@ -106,6 +106,11 @@ public: /// @param callout_handle Callout handle provided to the callout. void leases4Committed(hooks::CalloutHandle& callout_handle); + /// @brief Implementation of the "lease4_server_decline" callout. + /// + /// @param callout_handle Callout handle provided to the callout. + void lease4ServerDecline(hooks::CalloutHandle& callout_handle); + /// @brief Implementation of the "buffer6_receive" callout. /// /// This callout uses HA service to check if the query should be processed diff --git a/src/hooks/dhcp/high_availability/ha_messages.cc b/src/hooks/dhcp/high_availability/ha_messages.cc index a9dfe3fe47..36a223252e 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.cc +++ b/src/hooks/dhcp/high_availability/ha_messages.cc @@ -49,6 +49,7 @@ extern const isc::log::MessageID HA_INIT_OK = "HA_INIT_OK"; extern const isc::log::MessageID HA_INVALID_PARTNER_STATE_COMMUNICATION_RECOVERY = "HA_INVALID_PARTNER_STATE_COMMUNICATION_RECOVERY"; extern const isc::log::MessageID HA_INVALID_PARTNER_STATE_HOT_STANDBY = "HA_INVALID_PARTNER_STATE_HOT_STANDBY"; extern const isc::log::MessageID HA_INVALID_PARTNER_STATE_LOAD_BALANCING = "HA_INVALID_PARTNER_STATE_LOAD_BALANCING"; +extern const isc::log::MessageID HA_LEASE4_SERVER_DECLINE_FAILED = "HA_LEASE4_SERVER_DECLINE_FAILED"; extern const isc::log::MessageID HA_LEASES4_COMMITTED_FAILED = "HA_LEASES4_COMMITTED_FAILED"; extern const isc::log::MessageID HA_LEASES4_COMMITTED_NOTHING_TO_UPDATE = "HA_LEASES4_COMMITTED_NOTHING_TO_UPDATE"; extern const isc::log::MessageID HA_LEASES6_COMMITTED_FAILED = "HA_LEASES6_COMMITTED_FAILED"; @@ -157,6 +158,7 @@ const char* values[] = { "HA_INVALID_PARTNER_STATE_COMMUNICATION_RECOVERY", "%1: partner is in the communication-recovery state unexpectedly", "HA_INVALID_PARTNER_STATE_HOT_STANDBY", "%1: partner is in the hot-standby state unexpectedly", "HA_INVALID_PARTNER_STATE_LOAD_BALANCING", "%1: partner is in the load-balancing state unexpectedly", + "HA_LEASE4_SERVER_DECLINE_FAILED", "lease4_server_decline callout failed: %1", "HA_LEASES4_COMMITTED_FAILED", "leases4_committed callout failed: %1", "HA_LEASES4_COMMITTED_NOTHING_TO_UPDATE", "%1: leases4_committed callout was invoked without any leases", "HA_LEASES6_COMMITTED_FAILED", "leases6_committed callout failed: %1", diff --git a/src/hooks/dhcp/high_availability/ha_messages.h b/src/hooks/dhcp/high_availability/ha_messages.h index 314de4c22b..2aa209cc0f 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.h +++ b/src/hooks/dhcp/high_availability/ha_messages.h @@ -50,6 +50,7 @@ extern const isc::log::MessageID HA_INIT_OK; extern const isc::log::MessageID HA_INVALID_PARTNER_STATE_COMMUNICATION_RECOVERY; extern const isc::log::MessageID HA_INVALID_PARTNER_STATE_HOT_STANDBY; extern const isc::log::MessageID HA_INVALID_PARTNER_STATE_LOAD_BALANCING; +extern const isc::log::MessageID HA_LEASE4_SERVER_DECLINE_FAILED; extern const isc::log::MessageID HA_LEASES4_COMMITTED_FAILED; extern const isc::log::MessageID HA_LEASES4_COMMITTED_NOTHING_TO_UPDATE; extern const isc::log::MessageID HA_LEASES6_COMMITTED_FAILED; diff --git a/src/hooks/dhcp/high_availability/ha_messages.mes b/src/hooks/dhcp/high_availability/ha_messages.mes index 9a6687167e..17994cb64f 100644 --- a/src/hooks/dhcp/high_availability/ha_messages.mes +++ b/src/hooks/dhcp/high_availability/ha_messages.mes @@ -631,3 +631,10 @@ This informational message is issued when the server has been restarted after correcting the clock skew. The partner server is still in the terminated state. The partner must be restarted before the server can synchronize the database and start normal operation. + +% HA_LEASE4_SERVER_DECLINE_FAILED lease4_server_decline callout failed: %1 +This error message is issued when the callout for the lease4_server_declinehook +point failed. This includes unexpected errors like wrong arguments provided to +the callout by the DHCP server (unlikely internal server error). +The argument contains a reason for the error. + diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index 013ea7a2eb..e964e7f340 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -1243,6 +1243,55 @@ HAService::asyncSendLeaseUpdates(const dhcp::Pkt4Ptr& query, return (sent_num); } +size_t +HAService::asyncSendLeaseUpdate(const dhcp::Pkt4Ptr& query, + const dhcp::Lease4Ptr& lease, + const hooks::ParkingLotHandlePtr& parking_lot) { + + // Get configurations of the peers. Exclude this instance. + HAConfig::PeerConfigMap peers_configs = config_->getOtherServersConfig(); + + size_t sent_num = 0; + + // Schedule sending lease updates to each peer. + for (auto p = peers_configs.begin(); p != peers_configs.end(); ++p) { + HAConfig::PeerConfigPtr conf = p->second; + + // Check if the lease updates should be queued. This is the case when the + // server is in the communication-recovery state. Queued lease updates may + // be sent when the communication is re-established. + if (shouldQueueLeaseUpdates(conf)) { + lease_update_backlog_.push(LeaseUpdateBacklog::ADD, lease); + continue; + } + + // Check if the lease update should be sent to the server. If we're in + // the partner-down state we don't send lease updates to the partner. + if (!shouldSendLeaseUpdates(conf)) { + // If we decide to not send the lease updates to an active partner, we + // should make a record of it in the communication state. The partner + // can check if there were any unsent lease updates when he determines + // whether it should synchronize its database or not when it recovers + // from the partner-down state. + if (conf->getRole() != HAConfig::PeerConfig::BACKUP) { + communication_state_->increaseUnsentUpdateCount(); + } + continue; + } + + asyncSendLeaseUpdate(query, conf, CommandCreator::createLease4Update(*lease), parking_lot); + + // If we're contacting a backup server from which we don't expect a + // response prior to responding to the DHCP client we don't count + // it. + if ((config_->amWaitingBackupAck() || (conf->getRole() != HAConfig::PeerConfig::BACKUP))) { + ++sent_num; + } + } + + return (sent_num); +} + size_t HAService::asyncSendLeaseUpdates(const dhcp::Pkt6Ptr& query, const dhcp::Lease6CollectionPtr& leases, @@ -1324,7 +1373,9 @@ HAService::leaseUpdateCompleteInternal(QueryPtrType& query, // If there are no more pending requests for this query, let's unpark // the DHCP packet. if (it == pending_requests_.end() || (--pending_requests_[query] <= 0)) { - parking_lot->unpark(query); + if (parking_lot) { + parking_lot->unpark(query); + } // If we have unparked the packet we can clear pending requests for // this query. @@ -1481,7 +1532,9 @@ HAService::asyncSendLeaseUpdate(const QueryPtrType& query, // a backup server and the lease update was unsuccessful. In such // case the DHCP exchange fails. if (!lease_update_success) { - parking_lot->drop(query); + if (parking_lot) { + parking_lot->drop(query); + } } } else { // This was a response from the backup server and we're configured to diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h index 48b325d4ca..02f6ff3870 100644 --- a/src/hooks/dhcp/high_availability/ha_service.h +++ b/src/hooks/dhcp/high_availability/ha_service.h @@ -580,6 +580,10 @@ public: const dhcp::Lease4CollectionPtr& deleted_leases, const hooks::ParkingLotHandlePtr& parking_lot); + size_t asyncSendLeaseUpdate(const dhcp::Pkt4Ptr& query, + const dhcp::Lease4Ptr& lease, + const hooks::ParkingLotHandlePtr& parking_lot); + /// @brief Schedules asynchronous IPv6 lease updates. /// /// This method schedules asynchronous IPv6 lease updates as a result of the