From c41e32f709d01bce4c7fad869b8e55e2537dd305 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Thu, 23 Jan 2020 19:50:54 +0200 Subject: [PATCH] [#1040] handle update and delete --- src/bin/dhcp4/dhcp4_srv.cc | 8 +++---- src/bin/dhcp6/dhcp6_srv.cc | 16 +++++++------- src/lib/dhcpsrv/alloc_engine.cc | 37 +++++++++++++++++++++++++-------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 01773340e8..74f1f01952 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -3039,10 +3039,6 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline, } } - // Remove existing DNS entries for the lease, if any. - // queueNCR will do the necessary checks and will skip the update, if not needed. - queueNCR(CHG_REMOVE, lease); - // @todo: Call hooks. // We need to disassociate the lease from the client. Once we move a lease @@ -3052,6 +3048,10 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline, LeaseMgrFactory::instance().updateLease4(lease); + // Remove existing DNS entries for the lease, if any. + // queueNCR will do the necessary checks and will skip the update, if not needed. + queueNCR(CHG_REMOVE, lease); + // Bump up the statistics. // Per subnet declined addresses counter. diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 9f6f9e192c..68586a7ebd 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -3395,11 +3395,6 @@ Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease, } } - // Check if a lease has flags indicating that the FQDN update has - // been performed. If so, create NameChangeRequest which removes - // the entries. This method does all necessary checks. - queueNCR(CHG_REMOVE, lease); - // @todo: Call hooks. // We need to disassociate the lease from the client. Once we move a lease @@ -3409,6 +3404,11 @@ Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease, LeaseMgrFactory::instance().updateLease6(lease); + // Check if a lease has flags indicating that the FQDN update has + // been performed. If so, create NameChangeRequest which removes + // the entries. This method does all necessary checks. + queueNCR(CHG_REMOVE, lease); + // Bump up the subnet-specific statistic. StatsMgr::instance().addValue( StatsMgr::generateName("subnet", lease->subnet_id_, "declined-addresses"), @@ -3774,9 +3774,9 @@ Dhcpv6Srv::generateFqdn(const Pkt6Ptr& answer, // Set the generated FQDN in the Client FQDN option. fqdn->setDomainName(generated_name, Option6ClientFqdn::FULL); - answer->delOption(D6O_CLIENT_FQDN); - answer->addOption(fqdn); - ctx.hostname_ = generated_name; + answer->delOption(D6O_CLIENT_FQDN); + answer->addOption(fqdn); + ctx.hostname_ = generated_name; } catch (const Exception& ex) { LOG_ERROR(ddns6_logger, DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL) .arg(answer->getLabel()) diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index fe4a6c6d71..8f49fa6c81 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -1474,7 +1474,11 @@ AllocEngine::removeNonmatchingReservedLeases6(ClientContext6& ctx, // Remove this lease from LeaseMgr as it is reserved to someone // else or doesn't belong to a pool. - LeaseMgrFactory::instance().deleteLease(candidate); + bool success = LeaseMgrFactory::instance().deleteLease(candidate); + + if (!success) { + continue; + } // Update DNS if needed. queueNCR(CHG_REMOVE, candidate); @@ -1517,7 +1521,11 @@ AllocEngine::removeNonmatchingReservedNoHostLeases6(ClientContext6& ctx, } // Remove this lease from LeaseMgr as it doesn't belong to a pool. - LeaseMgrFactory::instance().deleteLease(candidate); + bool success = LeaseMgrFactory::instance().deleteLease(candidate); + + if (!success) { + continue; + } // Update DNS if needed. queueNCR(CHG_REMOVE, candidate); @@ -1584,7 +1592,11 @@ AllocEngine::removeNonreservedLeases6(ClientContext6& ctx, // simply remove it from the list. // We have reservations, but not for this lease. Release it. // Remove this lease from LeaseMgr - LeaseMgrFactory::instance().deleteLease(*lease); + bool success = LeaseMgrFactory::instance().deleteLease(*lease); + + if (!success) { + continue; + } // Update DNS if required. queueNCR(CHG_REMOVE, *lease); @@ -1984,7 +1996,11 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) { // Oh dear, the lease is no longer valid. We need to get rid of it. // Remove this lease from LeaseMgr - LeaseMgrFactory::instance().deleteLease(lease); + bool success = LeaseMgrFactory::instance().deleteLease(lease); + + if (!success) { + return; + } // Updated DNS if required. queueNCR(CHG_REMOVE, lease); @@ -3523,12 +3539,15 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { .arg(ctx.query_->getLabel()) .arg(client_lease->addr_.toText()); - lease_mgr.deleteLease(client_lease); + bool success = lease_mgr.deleteLease(client_lease); - // Need to decrease statistic for assigned addresses. - StatsMgr::instance().addValue( - StatsMgr::generateName("subnet", client_lease->subnet_id_, "assigned-addresses"), - static_cast(-1)); + if (success) { + // Need to decrease statistic for assigned addresses. + StatsMgr::instance().addValue( + StatsMgr::generateName("subnet", client_lease->subnet_id_, + "assigned-addresses"), + static_cast(-1)); + } } // Return the allocated lease or NULL pointer if allocation was