From be7d21bba12c4df244d89abcfaf316d4825b2c04 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Tue, 21 Nov 2023 10:41:08 +0200 Subject: [PATCH] [#3084] fixed doxygen and comments --- doc/sphinx/arm/hooks-ping-check.rst | 20 ++++---- src/bin/dhcp4/dhcp4_messages.mes | 15 +++--- src/bin/dhcp4/dhcp4_srv.cc | 72 +++++++++++++++-------------- src/bin/dhcp4/dhcp4_srv.h | 16 +++---- 4 files changed, 62 insertions(+), 61 deletions(-) diff --git a/doc/sphinx/arm/hooks-ping-check.rst b/doc/sphinx/arm/hooks-ping-check.rst index 36019b8231..22b2ca5f5a 100644 --- a/doc/sphinx/arm/hooks-ping-check.rst +++ b/doc/sphinx/arm/hooks-ping-check.rst @@ -37,15 +37,15 @@ by adding it to the ``hooks-libraries`` element of the server's configuration: } When the library is loaded :iscman:`kea-dhcp4` will conduct ping-check prior to -offering a lease to client if all of the following conditions are true +offering a lease to client if all of the following conditions are true: -1. Ping check hook library is loaded +1. Ping check hook library is loaded. -2. Ping checking is enabled +2. Ping checking is enabled. 3. The server is responding to a DHCPDISCOVER. -4. The candidate lease is neither active nor reserved +4. The candidate lease is neither active nor reserved. 5. Any of the following are true: @@ -55,7 +55,7 @@ offering a lease to client if all of the following conditions are true ping checks are done for every DHCPOFFER as the server has no way to know it has made prior offers. - b. The lease is being offered to a client other than its previous owner + b. The lease is being offered to a client other than its previous owner. c. The lease is being offered to its previous owner and more than a configurable number of seconds, `ping-cltt-secs`, have elapsed since @@ -65,10 +65,10 @@ When the ping check library is loaded, in response to a DHCPDISCOVER the :iscman:`kea-dhcp4` will: 1. Select a candidate IPv4 address through normal processes and use it to -construct a DHCPOFFER +construct a DHCPOFFER. 2. Park the DHCPOFFER and request a ping-check from the ping-check hook -library via its `lease4-offer` callout +library via its `lease4_offer` callout. 3. The callout will test conditions described above. If they are not satisfied it will return without conducting a check, and the server @@ -93,9 +93,9 @@ return to step 1. 4. Any of the following occur: a. Receipt of an ICMP DESTINATION UNREACHABLE message - b. ICMP ECHO REQUEST send fails due to a network error (e.g. network is unreachable) - c. ICMP ECHO REQUEST send fails due to a permissions error (e.g. lease address is a broadcast address) - d. ICMP ECHO REQUEST send fails with socket buffer full error + b. Send fail of an ICMP ECHO REQUEST due to a network error (e.g. network is unreachable) + c. Send fail of an ICMP ECHO REQUEST due to a permissions error (e.g. lease address is a broadcast address) + d. Send fail of an ICMP ECHO REQUEST with socket buffer full error In each of these instances the address could not be checked and is treated as available. diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 947eaeb866..ba3392a76f 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -465,7 +465,7 @@ This debug message occurs when the parking lot used to hold client queries while hook library work for them completes has reached or exceeded the limit set by the parked-packet-limit global parameter. This can occur when kea-dhcp4 is using hook libraries (e.g. ping-check) that implement the -"lease4-offer" callout and client queries are arriving faster than +"lease4_offer" callout and client queries are arriving faster than those callouts can fulfill them. % DHCP4_HOOK_LEASE4_RELEASE_SKIP %1: lease was not released because a callout set the next step to SKIP @@ -1082,19 +1082,18 @@ address may be declined again in the future. % DHCP4_SERVER_INITIATED_DECLINE_UPDATE_FAILED %1: error updating lease for address %2 This error message indicates that the server failed to update a lease in the -lease store to the DECLINED state The first argument includes the client and +lease store to the DECLINED state. The first argument includes the client and the transaction identification information. The second argument holds the IPv4 address for which the decline was attempted. % DHCP4_SERVER_INITIATED_DECLINE_ADD_FAILED %1: error adding a lease for address %2 This error message indicates that the server failed to add a DECLINED lease to -the lease store The first argument includes the client and the transaction +the lease store. The first argument includes the client and the transaction identification information. The second argument holds the IPv4 address for which the decline was attempted. % DHCP4_SERVER_INITIATED_DECLINE_RESOURCE_BUSY %1: error declining a lease for address %2 -This error message indicates that the while one server thread was attempting -to mark a lease as DECLINED, it was already locked by another thread. -The first argument includes the client and the transaction identification -information. The second argument holds the IPv4 address for which the decline -was attempted. +This error message indicates that while one server thread was attempting to mark +a lease as DECLINED, it was already locked by another thread. The first argument +includes the client and the transaction identification information. The second +argument holds the IPv4 address for which the decline was attempted. diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index bbf7e9dd2d..1a08ae03f8 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -3951,46 +3951,48 @@ void Dhcpv4Srv::serverDecline(hooks::CalloutHandlePtr& /* callout_handle */, Pkt4Ptr& query, Lease4Ptr lease, bool lease_exists) { - // Check if the resource is busy i.e. can be modified by another thread - // for another client. Highly unlikely. - ResourceHandler4 resource_handler; - if (MultiThreadingMgr::instance().getMode() && !resource_handler.tryLock4(lease->addr_)) { - LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_RESOURCE_BUSY) - .arg(query->getLabel()) - .arg(lease->addr_.toText()); - return; - } - - // We need to disassociate the lease from the client. Once we move a lease - // to declined state, it is no longer associated with the client in any - // way. - lease->decline(CfgMgr::instance().getCurrentCfg()->getDeclinePeriod()); - - // If the lease already exists, update it in the database. - if (lease_exists) { - try { - LeaseMgrFactory::instance().updateLease4(lease); - } catch (const NoSuchLease& ex) { - // We expected the lease to exist but it doesn't so let's add - // try to add it. - lease_exists = false; - } catch (const Exception& ex) { - // Update failed. - LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_UPDATE_FAILED) + { + // Check if the resource is busy i.e. can be modified by another thread + // for another client. Highly unlikely. + ResourceHandler4 resource_handler; + if (MultiThreadingMgr::instance().getMode() && !resource_handler.tryLock4(lease->addr_)) { + LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_RESOURCE_BUSY) .arg(query->getLabel()) .arg(lease->addr_.toText()); return; } - } - if (!lease_exists) { - try { - LeaseMgrFactory::instance().addLease(lease); - } catch (const Exception& ex) { - LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_ADD_FAILED) - .arg(query->getLabel()) - .arg(lease->addr_.toText()); - return; + // We need to disassociate the lease from the client. Once we move a lease + // to declined state, it is no longer associated with the client in any + // way. + lease->decline(CfgMgr::instance().getCurrentCfg()->getDeclinePeriod()); + + // If the lease already exists, update it in the database. + if (lease_exists) { + try { + LeaseMgrFactory::instance().updateLease4(lease); + } catch (const NoSuchLease& ex) { + // We expected the lease to exist but it doesn't so let's try + // to add it. + lease_exists = false; + } catch (const Exception& ex) { + // Update failed. + LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_UPDATE_FAILED) + .arg(query->getLabel()) + .arg(lease->addr_.toText()); + return; + } + } + + if (!lease_exists) { + try { + LeaseMgrFactory::instance().addLease(lease); + } catch (const Exception& ex) { + LOG_ERROR(lease4_logger, DHCP4_SERVER_INITIATED_DECLINE_ADD_FAILED) + .arg(query->getLabel()) + .arg(lease->addr_.toText()); + return; + } } } diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 97e9d7ba52..8321a6a55b 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -814,17 +814,17 @@ class Dhcpv4Srv : public process::Daemon { /// @brief Renders a lease declined after the server has detected, via ping-check /// or other means, that its address is already in-use. /// - /// This function is invoked during the unpark callback for the lease4-offer + /// This function is invoked during the unpark callback for the lease4_offer /// hook point, if a hook callout has set the handle status to NEXT_STEP_DROP. /// It will create/update the lease to DECLINED state in the lease store, /// update the appropriate stats, and @todo implement a new hook point, - /// lease4-server-declined-lease (name subject to change). + /// lease4_server_declined_lease (name subject to change). /// - /// @param callout_handle - current callout handle + /// @param callout_handle - current callout handle. /// @param query - DHCPDISCOVER which instigated the declination. - /// @param lease - lease to decline (i.e lease that would have been offered) + /// @param lease - lease to decline (i.e lease that would have been offered). /// @param lease_exists - true if the lease already exists in the lease store - /// (as is the case when offer-lifetime is > 0) + /// (as is the case when offer-lifetime is > 0). void serverDecline(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, Lease4Ptr lease, bool lease_exists); @@ -833,11 +833,11 @@ class Dhcpv4Srv : public process::Daemon { /// In MT mode this wrapper is used to safely invoke serverDecline() as a /// DHCP worker thread task. /// - /// @param callout_handle - current callout handle + /// @param callout_handle - current callout handle. /// @param query - DHCPDISCOVER which instigated the declination. - /// @param lease - lease to decline (i.e lease that would have been offered) + /// @param lease - lease to decline (i.e lease that would have been offered). /// @param lease_exists - true if the lease already exists in the lease store - /// (as is the case when offer-lifetime is > 0) + /// (as is the case when offer-lifetime is > 0). void serverDeclineNoThrow(hooks::CalloutHandlePtr& callout_handle, Pkt4Ptr& query, Lease4Ptr lease, bool lease_exists);