diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 201f8d6568..0870f9a8a5 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1317,11 +1317,8 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, static_cast(1)); } - bool packet_park = false; - + CalloutHandlePtr callout_handle = getCalloutHandle(query); if (ctx && HooksManager::calloutsPresent(Hooks.hook_index_leases4_committed_)) { - CalloutHandlePtr callout_handle = getCalloutHandle(query); - // 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 @@ -1348,55 +1345,61 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, } callout_handle->setArgument("deleted_leases4", deleted_leases); - // Call all installed callouts - HooksManager::callCallouts(Hooks.hook_index_leases4_committed_, - *callout_handle); + if (allow_packet_park) { + // We proactively park the packet. We'll unpark it without invoking + // the callback (i.e. drop) unless the callout status is set to + // is NEXT_STEP_PARK. Otherwise the callback we bind here will be + // executed when the hook library unparks the packet. + HooksManager::park("leases4_committed", query, + [this, callout_handle, query, rsp]() mutable { + if (MultiThreadingMgr::instance().getMode()) { + typedef function CallBack; + boost::shared_ptr call_back = + boost::make_shared(std::bind(&Dhcpv4Srv::sendResponseNoThrow, + this, callout_handle, query, rsp)); + MultiThreadingMgr::instance().getThreadPool().add(call_back); + } else { + processPacketPktSend(callout_handle, query, rsp); + processPacketBufferSend(callout_handle, rsp); + } + }, false); + } - if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) { - LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, - DHCP4_HOOK_LEASES4_COMMITTED_DROP) - .arg(query->getLabel()); + try { + // Call all installed callouts + HooksManager::callCallouts(Hooks.hook_index_leases4_committed_, + *callout_handle); + } catch(...) { + // Make sure we don't orphan a parked packet. + if (allow_packet_park) { + HooksManager::drop("leases4_committed", query); + } + + throw; + } + + if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_PARK) + && allow_packet_park) { + LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_LEASES4_COMMITTED_PARK) + .arg(query->getLabel()); + // Since the hook library(ies) are going to do the unparking, then + // reset the pointer to the response to indicate to the caller that + // it should return, as the packet processing will continue via + // the callback. rsp.reset(); - - } else if ((callout_handle->getStatus() == CalloutHandle::NEXT_STEP_PARK) - && allow_packet_park) { - packet_park = true; + } else { + // Drop the park job on the packet, it isn't needed. + HooksManager::drop("leases4_committed", query); + if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) { + LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, DHCP4_HOOK_LEASES4_COMMITTED_DROP) + .arg(query->getLabel()); + rsp.reset(); + } } } - if (!rsp) { - return; - } - - // PARKING SPOT after leases4_committed hook point. - CalloutHandlePtr callout_handle = getCalloutHandle(query); - if (packet_park) { - LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS, - DHCP4_HOOK_LEASES4_COMMITTED_PARK) - .arg(query->getLabel()); - - // Park the packet. The function we bind here will be executed when the hook - // library unparks the packet. - HooksManager::park("leases4_committed", query, - [this, callout_handle, query, rsp]() mutable { - if (MultiThreadingMgr::instance().getMode()) { - typedef function CallBack; - boost::shared_ptr call_back = - boost::make_shared(std::bind(&Dhcpv4Srv::sendResponseNoThrow, - this, callout_handle, query, rsp)); - MultiThreadingMgr::instance().getThreadPool().add(call_back); - } else { - processPacketPktSend(callout_handle, query, rsp); - processPacketBufferSend(callout_handle, rsp); - } - }); - - // If we have parked the packet, let's reset the pointer to the - // response to indicate to the caller that it should return, as - // the packet processing will continue via the callback. - rsp.reset(); - - } else { + // If we have a response prep it for shipment. + if (rsp) { processPacketPktSend(callout_handle, query, rsp); } } diff --git a/src/lib/hooks/hooks_manager.h b/src/lib/hooks/hooks_manager.h index 7cda194bbb..1ef37b31fe 100644 --- a/src/lib/hooks/hooks_manager.h +++ b/src/lib/hooks/hooks_manager.h @@ -289,9 +289,11 @@ public: /// @param unpark_callback callback invoked when the packet is unparked. template static void park(const std::string& hook_name, T parked_object, - std::function unpark_callback) { + std::function unpark_callback, + bool require_reference = true) { ServerHooks::getServerHooks(). - getParkingLotPtr(hook_name)->park(parked_object, unpark_callback); + getParkingLotPtr(hook_name)->park(parked_object, unpark_callback, + require_reference); } /// @brief Forces unparking the object (packet). diff --git a/src/lib/hooks/parking_lots.h b/src/lib/hooks/parking_lots.h index e50c7b8bdd..50c64fe6fc 100644 --- a/src/lib/hooks/parking_lots.h +++ b/src/lib/hooks/parking_lots.h @@ -86,8 +86,10 @@ public: " reference does not exist."); } - // Not there add it. + // Not there add it. We reset the refcount to zero since + // techinically there are no references yet. ParkingInfo pinfo(parked_object, unpark_callback); + pinfo.refcount_ = 0; parking_[makeKey(parked_object)] = pinfo; return; }