diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index 4fe12b613f..3b24ad5bda 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -370,11 +370,12 @@ called before "subnet4_select". @subsection dhcpv4HooksPkt4Send pkt4_send - @b Arguments: - - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: in/out - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: in + - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: in/out + - name: @b subnet4, type: isc::dhcp::Subnet4Ptr, direction: in - @b Description: this callout is executed when server's response - is about to be sent back to the client. The sole argument "response4" + is about to be sent back to the client. The argument "response4" contains a pointer to an isc::dhcp::Pkt4 object carrying the packet, with source and destination addresses set, interface over which it will be sent, and a list of all options and relay information. All fields @@ -385,6 +386,8 @@ called before "subnet4_select". The argument query4 contains a pointer to the corresponding query packet (allowing to perform correlation between response and query). This object cannot be modified. + The argument subnet4 contains a pointer to the selected subnet (if one). + This object cannot be modified. - Next step action: if any callout installed on the "pkt4_send" hook sets the next step action to SKIP, the server will not construct the raw diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index b422286034..c0ae4630ce 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1100,6 +1100,7 @@ Dhcpv4Srv::runOne() { return; } else { if (MultiThreadingMgr::instance().getMode()) { + query->addPktEvent("mt_queued"); typedef function CallBack; boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv4Srv::processPacketAndSendResponseNoThrow, @@ -1134,11 +1135,14 @@ Dhcpv4Srv::processPacketAndSendResponse(Pkt4Ptr& query) { } CalloutHandlePtr callout_handle = getCalloutHandle(query); + processPacketBufferSend(callout_handle, rsp); } void Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) { + query->addPktEvent("process_started"); + // All packets belong to ALL. query->addClass("ALL"); @@ -1550,12 +1554,12 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, typedef function CallBack; boost::shared_ptr call_back = boost::make_shared( std::bind(&Dhcpv4Srv::sendResponseNoThrow, this, callout_handle, - query, rsp)); + query, rsp, ctx->subnet_)); callout_handle_state->on_completion_ = [call_back]() { MultiThreadingMgr::instance().getThreadPool().add(call_back); }; } else { - processPacketPktSend(callout_handle, query, rsp); + processPacketPktSend(callout_handle, query, rsp, ctx->subnet_); processPacketBufferSend(callout_handle, rsp); } }); @@ -1596,15 +1600,16 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, // If we have a response prep it for shipment. if (rsp) { - processPacketPktSend(callout_handle, query, rsp); + Subnet4Ptr subnet = (ctx ? ctx->subnet_ : Subnet4Ptr()); + processPacketPktSend(callout_handle, query, rsp, subnet); } } void Dhcpv4Srv::sendResponseNoThrow(hooks::CalloutHandlePtr& callout_handle, - Pkt4Ptr& query, Pkt4Ptr& rsp) { + Pkt4Ptr& query, Pkt4Ptr& rsp, Subnet4Ptr& subnet) { try { - processPacketPktSend(callout_handle, query, rsp); + processPacketPktSend(callout_handle, query, rsp, subnet); processPacketBufferSend(callout_handle, rsp); } catch (const std::exception& e) { LOG_ERROR(packet4_logger, DHCP4_PACKET_PROCESS_STD_EXCEPTION) @@ -1616,7 +1621,8 @@ Dhcpv4Srv::sendResponseNoThrow(hooks::CalloutHandlePtr& callout_handle, void Dhcpv4Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle, - Pkt4Ptr& query, Pkt4Ptr& rsp) { + Pkt4Ptr& query, Pkt4Ptr& rsp, Subnet4Ptr& subnet) { + query->addPktEvent("process_completed"); if (!rsp) { return; } @@ -1643,6 +1649,9 @@ Dhcpv4Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle, // Set our response callout_handle->setArgument("response4", rsp); + // Pass in the selected subnet. + callout_handle->setArgument("subnet4", subnet); + // Call all installed callouts HooksManager::callCallouts(Hooks.hook_index_pkt4_send_, *callout_handle); diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 0ed55adfe5..b2c100e769 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -359,8 +359,9 @@ public: /// @param callout_handle pointer to the callout handle. /// @param query A pointer to the packet to be processed. /// @param rsp A pointer to the response. + /// @param subnet A pointer to selected subnet. void sendResponseNoThrow(hooks::CalloutHandlePtr& callout_handle, - Pkt4Ptr& query, Pkt4Ptr& rsp); + Pkt4Ptr& query, Pkt4Ptr& rsp, Subnet4Ptr& subnet); /// @brief Process a single incoming DHCPv4 packet. /// @@ -1115,8 +1116,9 @@ protected: /// @param callout_handle pointer to the callout handle. /// @param query Pointer to a query. /// @param rsp Pointer to a response. + /// @param subnet A pointer to selected subnet. void processPacketPktSend(hooks::CalloutHandlePtr& callout_handle, - Pkt4Ptr& query, Pkt4Ptr& rsp); + Pkt4Ptr& query, Pkt4Ptr& rsp, Subnet4Ptr& subnet); /// @brief Executes buffer4_send callout and sends the response. /// diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc index c3aa7c44c8..798babfdb5 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc @@ -33,6 +33,7 @@ using namespace std; using namespace isc::asiolink; using namespace isc::data; using namespace isc::util; +using namespace boost::posix_time; namespace isc { namespace dhcp { @@ -57,7 +58,7 @@ BaseServerTest::~BaseServerTest() { } Dhcpv4SrvTest::Dhcpv4SrvTest() - : rcode_(-1), srv_(0), multi_threading_(false) { + : rcode_(-1), srv_(0), multi_threading_(false), start_time_(PktEvent::now()) { // Wipe any existing statistics isc::stats::StatsMgr::instance().removeAll(); @@ -1006,6 +1007,21 @@ Dhcpv4SrvTest::pretendReceivingPkt(NakedDhcpv4Srv& srv, const std::string& confi EXPECT_EQ(1, tested_stat->getInteger().first); } +void +Dhcpv4SrvTest::checkPktEvents(const PktPtr& msg, + std::list expected_events) { + ASSERT_NE(start_time_, PktEvent::EMPTY_TIME()); + auto events = msg->getPktEvents(); + ASSERT_EQ(events.size(), expected_events.size()); + ptime prev_time = start_time_; + auto expected_event = expected_events.begin(); + for (const auto& event : events) { + ASSERT_EQ(event.label_, *expected_event); + EXPECT_GE(event.timestamp_, prev_time); + ++expected_event; + } +} + } // end of isc::dhcp::test namespace } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index 794cbfa86c..d220f04386 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@ -192,6 +192,8 @@ public: /// /// See fake_received_ field for description void fakeReceive(const Pkt4Ptr& pkt) { + pkt->addPktEvent(PktEvent::SOCKET_RECEIVED); + pkt->addPktEvent(PktEvent::BUFFER_READ); fake_received_.push_back(pkt); } @@ -679,6 +681,16 @@ public: multi_threading_ = enabled; } + /// @brief Checks the contents of a packet's event stack agains a list + /// of expected events. + /// + /// @param msg pointer to the packet under test. + /// @param start_time system time prior to or equal to the timestamp + /// of the stack's first event (i.e. before packet was sent or received) + /// @param expected_events a list of the event labels in the order they + /// are expected to occur in the stack. + void checkPktEvents(const PktPtr& msg, std::list expected_events); + /// @brief A subnet used in most tests. Subnet4Ptr subnet_; @@ -699,6 +711,9 @@ public: /// @brief The multi-threading flag. bool multi_threading_; + + /// @brief Time the test started (UTC/microseconds) + boost::posix_time::ptime start_time_; }; /// @brief Patch the server config to add interface-config/re-detect=false diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index aa94f8eba7..95fa3d386c 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -1536,6 +1536,7 @@ TEST_F(HooksDhcpv4SrvTest, pkt4SendSimple) { vector expected_argument_names; expected_argument_names.push_back(string("query4")); expected_argument_names.push_back(string("response4")); + expected_argument_names.push_back(string("subnet4")); sort(callback_argument_names_.begin(), callback_argument_names_.end()); sort(expected_argument_names.begin(), expected_argument_names.end()); EXPECT_TRUE(expected_argument_names == callback_argument_names_); @@ -1544,6 +1545,14 @@ TEST_F(HooksDhcpv4SrvTest, pkt4SendSimple) { EXPECT_TRUE(callback_qry_options_copy_); EXPECT_TRUE(callback_resp_options_copy_); + // Verify that packet sent to callout had the expected packet events. + std::list expected_events; + expected_events.push_back(PktEvent::SOCKET_RECEIVED); + expected_events.push_back(PktEvent::BUFFER_READ); + expected_events.push_back("process_started"); + expected_events.push_back("process_completed"); + checkPktEvents(callback_qry_pkt4_, expected_events); + // Check if the callout handle state was reset after the callout. checkCalloutHandleReset(discover); } diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox index 4c5af6ff8b..29fc2a0ec4 100644 --- a/src/bin/dhcp6/dhcp6_hooks.dox +++ b/src/bin/dhcp6/dhcp6_hooks.dox @@ -378,9 +378,10 @@ called before "subnet6_select". - @b Arguments: - name: @b query6, type: isc::dhcp::Pkt6Ptr, direction: in - name: @b response6, type: isc::dhcp::Pkt6Ptr, direction: in/out + - name: @b subnet6, type: isc::dhcp::Subnet6Ptr, direction: in/out - @b Description: This callout is executed when server's response - is about to be send back to the client. The sole argument "response6" + is about to be send back to the client. The argument "response6" contains a pointer to an @c isc::dhcp::Pkt6 object that contains the packet, with set source and destination addresses, interface over which it will be send, list of all options and relay information. All fields @@ -388,6 +389,12 @@ called before "subnet6_select". noted that unless the callout sets the skip flag (see below), anything placed in the @c buffer_out_ field will be overwritten when the callout returns. (buffer_out_ is scratch space used for constructing the packet.) + The argument query6 contains a pointer to the corresponding query packet + (allowing to perform correlation between response and query). This object + cannot be modified. + The argument subnet6 contains a pointer to the selected subnet (if one). + This object cannot be modified. + - Next step status: If any callout sets the status to SKIP, the server will assume that the callout did pack the "transaction-id", "message type" diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 6f5574a1c6..fae5e93429 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -693,6 +693,7 @@ Dhcpv6Srv::runOne() { return; } else { if (MultiThreadingMgr::instance().getMode()) { + query->addPktEvent("mt_queued"); typedef function CallBack; boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv6Srv::processPacketAndSendResponseNoThrow, @@ -731,6 +732,8 @@ Dhcpv6Srv::processPacketAndSendResponse(Pkt6Ptr query) { Pkt6Ptr Dhcpv6Srv::processPacket(Pkt6Ptr query) { + query->addPktEvent("process_started"); + // All packets belong to ALL. query->addClass("ALL"); @@ -1228,18 +1231,19 @@ Dhcpv6Srv::processLocalizedQuery6(AllocEngine::ClientContext6& ctx) { // the callback (i.e. drop) unless the callout status is set to // NEXT_STEP_PARK. Otherwise the callback we bind here will be // executed when the hook library unparks the packet. + Subnet6Ptr subnet = ctx.subnet_; HooksManager::park("leases6_committed", query, - [this, callout_handle, query, rsp, callout_handle_state]() mutable { + [this, callout_handle, query, rsp, callout_handle_state, subnet]() mutable { if (MultiThreadingMgr::instance().getMode()) { typedef function CallBack; boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv6Srv::sendResponseNoThrow, - this, callout_handle, query, rsp)); + this, callout_handle, query, rsp, subnet)); callout_handle_state->on_completion_ = [call_back]() { MultiThreadingMgr::instance().getThreadPool().add(call_back); }; } else { - processPacketPktSend(callout_handle, query, rsp); + processPacketPktSend(callout_handle, query, rsp, subnet); processPacketBufferSend(callout_handle, rsp); } }); @@ -1275,7 +1279,7 @@ Dhcpv6Srv::processLocalizedQuery6(AllocEngine::ClientContext6& ctx) { // If we have a response prep it for shipment. if (rsp) { - processPacketPktSend(callout_handle, query, rsp); + processPacketPktSend(callout_handle, query, rsp, ctx.subnet_); } return (rsp); @@ -1283,9 +1287,9 @@ Dhcpv6Srv::processLocalizedQuery6(AllocEngine::ClientContext6& ctx) { void Dhcpv6Srv::sendResponseNoThrow(hooks::CalloutHandlePtr& callout_handle, - Pkt6Ptr query, Pkt6Ptr& rsp) { + Pkt6Ptr query, Pkt6Ptr& rsp, Subnet6Ptr& subnet) { try { - processPacketPktSend(callout_handle, query, rsp); + processPacketPktSend(callout_handle, query, rsp, subnet); processPacketBufferSend(callout_handle, rsp); } catch (const std::exception& e) { LOG_ERROR(packet6_logger, DHCP6_PACKET_PROCESS_STD_EXCEPTION) @@ -1297,7 +1301,8 @@ Dhcpv6Srv::sendResponseNoThrow(hooks::CalloutHandlePtr& callout_handle, void Dhcpv6Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle, - Pkt6Ptr& query, Pkt6Ptr& rsp) { + Pkt6Ptr& query, Pkt6Ptr& rsp, Subnet6Ptr& subnet) { + query->addPktEvent("process_completed"); if (!rsp) { return; } @@ -1326,6 +1331,9 @@ Dhcpv6Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle, // Set our response callout_handle->setArgument("response6", rsp); + // Pass the selected subnet as an argument. + callout_handle->setArgument("subnet6", subnet); + // Call all installed callouts HooksManager::callCallouts(Hooks.hook_index_pkt6_send_, *callout_handle); diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index 1a124151af..a7608e34b2 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -170,8 +170,9 @@ public: /// @param callout_handle pointer to the callout handle. /// @param query A pointer to the packet to be processed. /// @param rsp A pointer to the response. + /// @param subnet A pointer to the selected subnet. void sendResponseNoThrow(hooks::CalloutHandlePtr& callout_handle, - Pkt6Ptr query, Pkt6Ptr& rsp); + Pkt6Ptr query, Pkt6Ptr& rsp, Subnet6Ptr& subnet); /// @brief Process a single incoming DHCPv6 packet. /// @@ -1201,8 +1202,9 @@ protected: /// @param callout_handle pointer to the callout handle. /// @param query Pointer to a query. /// @param rsp Pointer to a response. + /// @param subnet A pointer to the selected subnet. void processPacketPktSend(hooks::CalloutHandlePtr& callout_handle, - Pkt6Ptr& query, Pkt6Ptr& rsp); + Pkt6Ptr& query, Pkt6Ptr& rsp, Subnet6Ptr& subnet); /// @brief Allocation Engine. /// Pointer to the allocation engine that we are currently using diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index a10900db13..2293cbacb3 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc @@ -24,6 +24,7 @@ using namespace isc::dhcp; using namespace isc::asiolink; using namespace isc::stats; using namespace isc::util; +using namespace boost::posix_time; namespace isc { namespace dhcp { @@ -1054,7 +1055,7 @@ Dhcpv6SrvTest::configure(const std::string& config, } NakedDhcpv6SrvTest::NakedDhcpv6SrvTest() - : rcode_(-1) { + : rcode_(-1), start_time_(PktEvent::now()) { // it's ok if that fails. There should not be such a file anyway static_cast(remove(DUID_FILE)); @@ -1106,6 +1107,21 @@ NakedDhcpv6SrvTest::generateIA(uint16_t type, uint32_t iaid, uint32_t t1, return (ia); } +void +NakedDhcpv6SrvTest::checkPktEvents(const PktPtr& msg, + std::list expected_events) { + ASSERT_NE(start_time_, PktEvent::EMPTY_TIME()); + auto events = msg->getPktEvents(); + ASSERT_EQ(events.size(), expected_events.size()); + ptime prev_time = start_time_; + auto expected_event = expected_events.begin(); + for (const auto& event : events) { + ASSERT_EQ(event.label_, *expected_event); + EXPECT_GE(event.timestamp_, prev_time); + ++expected_event; + } +} + bool Dhcpv6SrvTest::compareOptions(const isc::dhcp::OptionPtr& option1, const isc::dhcp::OptionPtr& option2) { diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h index 68b6645623..591a69d94b 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -200,6 +200,9 @@ public: /// /// See fake_received_ field for description void fakeReceive(const isc::dhcp::Pkt6Ptr& pkt) { + // Add packet events normally set by PktFilter. + pkt->addPktEvent(PktEvent::SOCKET_RECEIVED); + pkt->addPktEvent(PktEvent::BUFFER_READ); fake_received_.push_back(pkt); } @@ -549,6 +552,16 @@ public: EXPECT_EQ(expected_transid, rsp->getTransid()); } + /// @brief Checks the contents of a packet's event stack agains a list + /// of expected events. + /// + /// @param msg pointer to the packet under test. + /// @param start_time system time prior to or equal to the timestamp + /// of the stack's first event (i.e. before packet was sent or received) + /// @param expected_events a list of the event labels in the order they + /// are expected to occur in the stack. + void checkPktEvents(const PktPtr& msg, std::list expected_events); + virtual ~NakedDhcpv6SrvTest(); // A DUID used in most tests (typically as client-id) @@ -562,6 +575,9 @@ public: // Index of a valid network interface unsigned int valid_ifindex_; + + /// @brief Time the test started (UTC/microseconds) + boost::posix_time::ptime start_time_; }; // We need to pass one reference to the Dhcp6Client, which is defined in diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index 20ef2abcb8..288ae8bd56 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -402,6 +402,8 @@ public: callout_handle.getArgument("query6", callback_qry_pkt6_); + callout_handle.getArgument("subnet6", callback_subnet6_); + callback_argument_names_ = callout_handle.getArgumentNames(); if (callback_qry_pkt6_) { @@ -1444,12 +1446,21 @@ TEST_F(HooksDhcpv6SrvTest, pkt6SendSimple) { vector expected_argument_names; expected_argument_names.push_back(string("query6")); expected_argument_names.push_back(string("response6")); + expected_argument_names.push_back(string("subnet6")); EXPECT_TRUE(expected_argument_names == callback_argument_names_); // Pkt passed to a callout must be configured to copy retrieved options. EXPECT_TRUE(callback_qry_options_copy_); EXPECT_TRUE(callback_resp_options_copy_); + // Verify that packet sent to callout has the expected packet events. + std::list expected_events; + expected_events.push_back(PktEvent::SOCKET_RECEIVED); + expected_events.push_back(PktEvent::BUFFER_READ); + expected_events.push_back("process_started"); + expected_events.push_back("process_completed"); + checkPktEvents(callback_qry_pkt6_, expected_events); + // Check if the callout handle state was reset after the callout. checkCalloutHandleReset(sol); }