From: Thomas Markwalder Date: Wed, 11 May 2016 17:12:19 +0000 (-0400) Subject: [4481] Made query4 and query6 callout arguments uniformly available X-Git-Tag: trac4106_update_base~20^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1d7afd1c02db0631dee7b999691aa435e7a180a0;p=thirdparty%2Fkea.git [4481] Made query4 and query6 callout arguments uniformly available The client packet is now uniformly available to all client packet driven callouts for both v4 and v6: Added "query4" to lease4_select and lease4_renew src/bin/dhcp4/dhcp4_hooks.dox Added query4 argument to lease4_select and lease4_renew documentation src/bin/dhcp4/tests/hooks_unittest.cc Revamped to track both query4 and response4 arguments TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) - modified to verify query4 set by lease4_renew callout src/lib/dhcpsrv/alloc_engine.cc - AllocEngine::createLease4() - AllocEngine::reuseExpiredLease4() - modified to add query4 to lease4_select callout arguments - AllocEngine::renewLease4() - modified to add query4 to lease4_renew callout arguments src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc HookAllocEngine4Test() - modified to track query4 argument TEST_F(HookAllocEngine4Test, lease4_select) - modified to verify query4 callout argument Added "query6" to pkt6_send and lease6_select src/bin/dhcp6/dhcp6_hooks.dox Added query6 argument to pkt6_send and lease6_select documentation src/bin/dhcp6/dhcp6_srv.cc Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) - modified to add query6 to the pkt_send callout arguments src/bin/dhcp6/tests/hooks_unittest.cc Revamped to track both query4 and response4 arguments TEST_F(HooksDhcpv6SrvTest, simplePkt6Send) - modified to verify query6 set by pkt6_send callout src/lib/dhcpsrv/alloc_engine.cc - AllocEngine::createLease4() - AllocEngine::reuseExpiredLease6() - modified to add query6 to lease6_select callout arguments callout arguments src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc HookAllocEngine6Test() - modified to track query6 argument TEST_F(HookAllocEngine6Test, lease4_select) - modified to verify query6 argument --- diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index e97a9f09eb..f82308bf40 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -110,6 +110,7 @@ packet processing. Hook points that are not specific to packet processing @subsection dhcpv4HooksLeaseSelect lease4_select - @b Arguments: + - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: in - name: @b subnet4, type: isc::dhcp::Subnet4Ptr, direction: in - name: @b fake_allocation, type: bool, direction: in - name: @b lease4, type: isc::dhcp::Lease4Ptr, direction: in/out @@ -137,6 +138,7 @@ packet processing. Hook points that are not specific to packet processing @subsection dhcpv4HooksLeaseRenew lease4_renew - @b Arguments: + - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: in - name: @b subnet4, type: isc::dhcp::Subnet4Ptr, direction: in - name: @b clientid, type: isc::dhcp::ClientId, direction: in - name: @b hwaddr, type: isc::dhcp::HWAddr, direction: in diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index 173274d3ba..41cf65e54f 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -197,7 +197,7 @@ public: buffer4_receive_callout(CalloutHandle& callout_handle) { callback_name_ = string("buffer4_receive"); - callout_handle.getArgument("query4", callback_pkt4_); + callout_handle.getArgument("query4", callback_qry_pkt4_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -242,7 +242,7 @@ public: pkt4_receive_callout(CalloutHandle& callout_handle) { callback_name_ = string("pkt4_receive"); - callout_handle.getArgument("query4", callback_pkt4_); + callout_handle.getArgument("query4", callback_qry_pkt4_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -307,9 +307,9 @@ public: pkt4_send_callout(CalloutHandle& callout_handle) { callback_name_ = string("pkt4_send"); - callout_handle.getArgument("response4", callback_pkt4_); + callout_handle.getArgument("response4", callback_resp_pkt4_); - callout_handle.getArgument("query4", callback_pkt4_query_); + callout_handle.getArgument("query4", callback_qry_pkt4_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -372,7 +372,7 @@ public: buffer4_send_callout(CalloutHandle& callout_handle) { callback_name_ = string("buffer4_send"); - callout_handle.getArgument("response4", callback_pkt4_); + callout_handle.getArgument("response4", callback_resp_pkt4_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -412,7 +412,7 @@ public: subnet4_select_callout(CalloutHandle& callout_handle) { callback_name_ = string("subnet4_select"); - callout_handle.getArgument("query4", callback_pkt4_); + callout_handle.getArgument("query4", callback_qry_pkt4_); callout_handle.getArgument("subnet4", callback_subnet4_); callout_handle.getArgument("subnet4collection", callback_subnet4collection_); @@ -450,7 +450,7 @@ public: lease4_release_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease4_release"); - callout_handle.getArgument("query4", callback_pkt4_); + callout_handle.getArgument("query4", callback_qry_pkt4_); callout_handle.getArgument("lease4", callback_lease4_); callback_argument_names_ = callout_handle.getArgumentNames(); @@ -464,6 +464,7 @@ public: lease4_renew_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease4_renew"); + callout_handle.getArgument("query4", callback_qry_pkt4_); callout_handle.getArgument("subnet4", callback_subnet4_); callout_handle.getArgument("lease4", callback_lease4_); callout_handle.getArgument("hwaddr", callback_hwaddr_); @@ -480,7 +481,7 @@ public: static int lease4_decline_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease4_decline"); - callout_handle.getArgument("query4", callback_pkt4_); + callout_handle.getArgument("query4", callback_qry_pkt4_); callout_handle.getArgument("lease4", callback_lease4_); return (0); @@ -500,8 +501,8 @@ public: /// resets buffers used to store data received by callouts void resetCalloutBuffers() { callback_name_ = string(""); - callback_pkt4_.reset(); - callback_pkt4_query_.reset(); + callback_qry_pkt4_.reset(); + callback_qry_pkt4_.reset(); callback_lease4_.reset(); callback_hwaddr_.reset(); callback_clientid_.reset(); @@ -518,14 +519,11 @@ public: /// String name of the received callout static string callback_name_; - /// Pkt4 structure returned in the callout - static Pkt4Ptr callback_pkt4_; + /// Client/query Pkt4 structure returned in the callout + static Pkt4Ptr callback_qry_pkt4_; - /// @brief Pkt4 structure returned in the callout - /// - /// pkt4_send hook now passes both the query and the response, - /// so we actually need two fields. - static Pkt4Ptr callback_pkt4_query_; + /// Server/response Pkt4 structure returned in the callout + static Pkt4Ptr callback_resp_pkt4_; /// Lease4 structure returned in the callout static Lease4Ptr callback_lease4_; @@ -549,8 +547,8 @@ public: // The following fields are used in testing pkt4_receive_callout. // See fields description in the class for details string HooksDhcpv4SrvTest::callback_name_; -Pkt4Ptr HooksDhcpv4SrvTest::callback_pkt4_; -Pkt4Ptr HooksDhcpv4SrvTest::callback_pkt4_query_; +Pkt4Ptr HooksDhcpv4SrvTest::callback_qry_pkt4_; +Pkt4Ptr HooksDhcpv4SrvTest::callback_resp_pkt4_; Subnet4Ptr HooksDhcpv4SrvTest::callback_subnet4_; HWAddrPtr HooksDhcpv4SrvTest::callback_hwaddr_; ClientIdPtr HooksDhcpv4SrvTest::callback_clientid_; @@ -618,7 +616,7 @@ TEST_F(HooksDhcpv4SrvTest, Buffer4ReceiveSimple) { EXPECT_EQ("buffer4_receive", callback_name_); // Check that pkt4 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt4_.get() == dis.get()); + EXPECT_TRUE(callback_qry_pkt4_.get() == dis.get()); // Check that all expected parameters are there vector expected_argument_names; @@ -723,7 +721,7 @@ TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSimple) { EXPECT_EQ("pkt4_receive", callback_name_); // check that pkt4 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt4_.get() == sol.get()); + EXPECT_TRUE(callback_qry_pkt4_.get() == sol.get()); // Check that all expected parameters are there vector expected_argument_names; @@ -853,12 +851,12 @@ TEST_F(HooksDhcpv4SrvTest, pkt4SendSimple) { Pkt4Ptr adv = srv_->fake_sent_.front(); // Check that pkt4 argument passing was successful and returned proper value - ASSERT_TRUE(callback_pkt4_); - EXPECT_TRUE(callback_pkt4_.get() == adv.get()); + ASSERT_TRUE(callback_resp_pkt4_); + EXPECT_TRUE(callback_resp_pkt4_.get() == adv.get()); // That that the query4 argument was correctly set to the Discover we sent. - ASSERT_TRUE(callback_pkt4_query_); - EXPECT_TRUE(callback_pkt4_query_.get() == sol.get()); + ASSERT_TRUE(callback_qry_pkt4_); + EXPECT_TRUE(callback_qry_pkt4_.get() == sol.get()); // Check that all expected parameters are there vector expected_argument_names; @@ -1002,7 +1000,7 @@ TEST_F(HooksDhcpv4SrvTest, buffer4SendSimple) { Pkt4Ptr adv = srv_->fake_sent_.front(); // Check that pkt4 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt4_.get() == adv.get()); + EXPECT_TRUE(callback_resp_pkt4_.get() == adv.get()); // Check that all expected parameters are there vector expected_argument_names; @@ -1124,7 +1122,7 @@ TEST_F(HooksDhcpv4SrvTest, subnet4SelectSimple) { EXPECT_EQ("subnet4_select", callback_name_); // Check that pkt4 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt4_.get() == sol.get()); + EXPECT_TRUE(callback_qry_pkt4_.get() == sol.get()); const Subnet4Collection* exp_subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); @@ -1274,6 +1272,10 @@ TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) { // Check that the callback called is indeed the one we installed EXPECT_EQ("lease4_renew", callback_name_); + // Check that query4 argument passing was successful and + // returned proper value + EXPECT_TRUE(callback_qry_pkt4_.get() == req.get()); + // Check that hwaddr parameter is passed properly ASSERT_TRUE(callback_hwaddr_); EXPECT_TRUE(*callback_hwaddr_ == *req->getHWAddr()); @@ -1288,6 +1290,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) { // Check if all expected parameters were really received vector expected_argument_names; + expected_argument_names.push_back("query4"); expected_argument_names.push_back("subnet4"); expected_argument_names.push_back("clientid"); expected_argument_names.push_back("hwaddr"); @@ -1444,7 +1447,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSimple) { EXPECT_EQ("lease4_release", callback_name_); // Check that pkt4 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt4_.get() == rel.get()); + EXPECT_TRUE(callback_qry_pkt4_.get() == rel.get()); // Check if all expected parameters were really received vector expected_argument_names; @@ -1540,15 +1543,15 @@ TEST_F(HooksDhcpv4SrvTest, HooksDecline) { // Verifying DHCPDECLINE is a bit tricky, as it is created somewhere in // acquireAndDecline. We'll just verify that it's really a DECLINE // and that its address is equal to what we have in LeaseMgr. - ASSERT_TRUE(callback_pkt4_); + ASSERT_TRUE(callback_qry_pkt4_); ASSERT_TRUE(callback_lease4_); // Check that it's the proper packet that was reported. - EXPECT_EQ(DHCPDECLINE, callback_pkt4_->getType()); + EXPECT_EQ(DHCPDECLINE, callback_qry_pkt4_->getType()); // Extract the address being declined. OptionCustomPtr opt_declined_addr = boost::dynamic_pointer_cast< - OptionCustom>(callback_pkt4_->getOption(DHO_DHCP_REQUESTED_ADDRESS)); + OptionCustom>(callback_qry_pkt4_->getOption(DHO_DHCP_REQUESTED_ADDRESS)); ASSERT_TRUE(opt_declined_addr); IOAddress addr(opt_declined_addr->readAddress()); @@ -1584,15 +1587,15 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineDrop) { // Verifying DHCPDECLINE is a bit tricky, as it is created somewhere in // acquireAndDecline. We'll just verify that it's really a DECLINE // and that its address is equal to what we have in LeaseMgr. - ASSERT_TRUE(callback_pkt4_); + ASSERT_TRUE(callback_qry_pkt4_); ASSERT_TRUE(callback_lease4_); // Check that it's the proper packet that was reported. - EXPECT_EQ(DHCPDECLINE, callback_pkt4_->getType()); + EXPECT_EQ(DHCPDECLINE, callback_qry_pkt4_->getType()); // Extract the address being declined. OptionCustomPtr opt_declined_addr = boost::dynamic_pointer_cast< - OptionCustom>(callback_pkt4_->getOption(DHO_DHCP_REQUESTED_ADDRESS)); + OptionCustom>(callback_qry_pkt4_->getOption(DHO_DHCP_REQUESTED_ADDRESS)); ASSERT_TRUE(opt_declined_addr); IOAddress addr(opt_declined_addr->readAddress()); diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox index 7e2bcf02ce..d9743d4d1d 100644 --- a/src/bin/dhcp6/dhcp6_hooks.dox +++ b/src/bin/dhcp6/dhcp6_hooks.dox @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -207,6 +207,7 @@ packet processing. Hook points that are not specific to packet processing @subsection dhcpv6HooksPkt6Send pkt6_send - @b Arguments: + - name: @b query6, type: isc::dhcp::Pkt6Ptr, direction: in - name: @b response6, type: isc::dhcp::Pkt6Ptr, direction: in/out - @b Description: This callout is executed when server's response diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index c99d4d322a..e5dad328b4 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -713,6 +713,9 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) { // Delete all previous arguments callout_handle->deleteAllArguments(); + // Pass incoming packet as argument + callout_handle->setArgument("query6", query); + // Set our response callout_handle->setArgument("response6", rsp); diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index ecb987ac8f..aec9c260f0 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/src/bin/dhcp6/tests/hooks_unittest.cc @@ -136,7 +136,7 @@ public: pkt6_receive_callout(CalloutHandle& callout_handle) { callback_name_ = string("pkt6_receive"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -199,7 +199,7 @@ public: buffer6_receive_callout(CalloutHandle& callout_handle) { callback_name_ = string("buffer6_receive"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -271,7 +271,9 @@ public: pkt6_send_callout(CalloutHandle& callout_handle) { callback_name_ = string("pkt6_send"); - callout_handle.getArgument("response6", callback_pkt6_); + callout_handle.getArgument("response6", callback_resp_pkt6_); + + callout_handle.getArgument("query6", callback_qry_pkt6_); callback_argument_names_ = callout_handle.getArgumentNames(); return (0); @@ -334,7 +336,7 @@ public: subnet6_select_callout(CalloutHandle& callout_handle) { callback_name_ = string("subnet6_select"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callout_handle.getArgument("subnet6", callback_subnet6_); callout_handle.getArgument("subnet6collection", callback_subnet6collection_); @@ -372,7 +374,7 @@ public: lease6_renew_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease6_renew"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callout_handle.getArgument("lease6", callback_lease6_); callout_handle.getArgument("ia_na", callback_ia_na_); @@ -380,6 +382,22 @@ public: return (0); } + /// Test callback that stores received callout name and pkt6 value + /// @param callout_handle handle passed by the hooks framework + /// @return always 0 + static int + lease6_rebind_callout(CalloutHandle& callout_handle) { + callback_name_ = string("lease6_rebind"); + + callout_handle.getArgument("query6", callback_qry_pkt6_); + callout_handle.getArgument("lease6", callback_lease6_); + callout_handle.getArgument("ia_na", callback_ia_na_); + + callback_argument_names_ = callout_handle.getArgumentNames(); + return (0); + } + + /// The following values are used by the callout to override /// renewed lease parameters static const uint32_t override_iaid_; @@ -396,7 +414,7 @@ public: lease6_renew_update_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease6_renew"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callout_handle.getArgument("lease6", callback_lease6_); callout_handle.getArgument("ia_na", callback_ia_na_); @@ -435,7 +453,7 @@ public: lease6_release_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease6_release"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callout_handle.getArgument("lease6", callback_lease6_); callback_argument_names_ = callout_handle.getArgumentNames(); @@ -463,7 +481,7 @@ public: static int lease6_decline_callout(CalloutHandle& callout_handle) { callback_name_ = string("lease6_decline"); - callout_handle.getArgument("query6", callback_pkt6_); + callout_handle.getArgument("query6", callback_qry_pkt6_); callout_handle.getArgument("lease6", callback_lease6_); return (0); @@ -494,7 +512,8 @@ public: /// Resets buffers used to store data received by callouts void resetCalloutBuffers() { callback_name_ = string(""); - callback_pkt6_.reset(); + callback_qry_pkt6_.reset(); + callback_resp_pkt6_.reset(); callback_subnet6_.reset(); callback_lease6_.reset(); callback_ia_na_.reset(); @@ -510,8 +529,11 @@ public: /// String name of the received callout static string callback_name_; - /// Pkt6 structure returned in the callout - static Pkt6Ptr callback_pkt6_; + /// Client's query Pkt6 structure returned in the callout + static Pkt6Ptr callback_qry_pkt6_; + + /// Server's response Pkt6 structure returned in the callout + static Pkt6Ptr callback_resp_pkt6_; /// Pointer to lease6 static Lease6Ptr callback_lease6_; @@ -540,7 +562,8 @@ const uint32_t HooksDhcpv6SrvTest::override_valid_ = 1004; // The following fields are used in testing pkt6_receive_callout. // See fields description in the class for details string HooksDhcpv6SrvTest::callback_name_; -Pkt6Ptr HooksDhcpv6SrvTest::callback_pkt6_; +Pkt6Ptr HooksDhcpv6SrvTest::callback_qry_pkt6_; +Pkt6Ptr HooksDhcpv6SrvTest::callback_resp_pkt6_; Subnet6Ptr HooksDhcpv6SrvTest::callback_subnet6_; const Subnet6Collection* HooksDhcpv6SrvTest::callback_subnet6collection_; vector HooksDhcpv6SrvTest::callback_argument_names_; @@ -606,7 +629,7 @@ TEST_F(HooksDhcpv6SrvTest, simpleBuffer6Receive) { EXPECT_EQ("buffer6_receive", callback_name_); // Check that pkt6 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt6_.get() == sol.get()); + EXPECT_TRUE(callback_qry_pkt6_.get() == sol.get()); // Check that all expected parameters are there vector expected_argument_names; @@ -727,7 +750,7 @@ TEST_F(HooksDhcpv6SrvTest, simplePkt6Receive) { EXPECT_EQ("pkt6_receive", callback_name_); // Check that pkt6 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt6_.get() == sol.get()); + EXPECT_TRUE(callback_qry_pkt6_.get() == sol.get()); // Check that all expected parameters are there vector expected_argument_names; @@ -848,11 +871,14 @@ TEST_F(HooksDhcpv6SrvTest, simplePkt6Send) { ASSERT_EQ(1, srv_->fake_sent_.size()); Pkt6Ptr adv = srv_->fake_sent_.front(); - // Check that pkt6 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt6_.get() == adv.get()); + // Check that pkt6 argument passing was successful and returned proper + // values + EXPECT_TRUE(callback_qry_pkt6_.get() == sol.get()); + EXPECT_TRUE(callback_resp_pkt6_.get() == adv.get()); // Check that all expected parameters are there vector expected_argument_names; + expected_argument_names.push_back(string("query6")); expected_argument_names.push_back(string("response6")); EXPECT_TRUE(expected_argument_names == callback_argument_names_); } @@ -1011,7 +1037,7 @@ TEST_F(HooksDhcpv6SrvTest, subnet6Select) { EXPECT_EQ("subnet6_select", callback_name_); // Check that pkt6 argument passing was successful and returned proper value - EXPECT_TRUE(callback_pkt6_.get() == sol.get()); + EXPECT_TRUE(callback_qry_pkt6_.get() == sol.get()); const Subnet6Collection* exp_subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll(); @@ -1163,7 +1189,7 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Renew) { EXPECT_EQ("lease6_renew", callback_name_); // Check that appropriate parameters are passed to the callouts - EXPECT_TRUE(callback_pkt6_); + EXPECT_TRUE(callback_qry_pkt6_); EXPECT_TRUE(callback_lease6_); EXPECT_TRUE(callback_ia_na_); @@ -1425,7 +1451,7 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Release) { EXPECT_EQ("lease6_release", callback_name_); // Check that appropriate parameters are passed to the callouts - EXPECT_TRUE(callback_pkt6_); + EXPECT_TRUE(callback_qry_pkt6_); EXPECT_TRUE(callback_lease6_); // Check if all expected parameters were really received @@ -1537,14 +1563,14 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) { EXPECT_EQ("lease6_decline", callback_name_); // And valid parameters were passed. - ASSERT_TRUE(callback_pkt6_); + ASSERT_TRUE(callback_qry_pkt6_); ASSERT_TRUE(callback_lease6_); // Test sanity check - it was a decline, right? - EXPECT_EQ(DHCPV6_DECLINE, callback_pkt6_->getType()); + EXPECT_EQ(DHCPV6_DECLINE, callback_qry_pkt6_->getType()); // Get the address from this decline. - OptionPtr ia = callback_pkt6_->getOption(D6O_IA_NA); + OptionPtr ia = callback_qry_pkt6_->getOption(D6O_IA_NA); ASSERT_TRUE(ia); boost::shared_ptr addr_opt = boost::dynamic_pointer_cast(ia->getOption(D6O_IAADDR)); @@ -1581,14 +1607,14 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) { EXPECT_EQ("lease6_decline", callback_name_); // And valid parameters were passed. - ASSERT_TRUE(callback_pkt6_); + ASSERT_TRUE(callback_qry_pkt6_); ASSERT_TRUE(callback_lease6_); // Test sanity check - it was a decline, right? - EXPECT_EQ(DHCPV6_DECLINE, callback_pkt6_->getType()); + EXPECT_EQ(DHCPV6_DECLINE, callback_qry_pkt6_->getType()); // Get the address from this decline. - OptionPtr ia = callback_pkt6_->getOption(D6O_IA_NA); + OptionPtr ia = callback_qry_pkt6_->getOption(D6O_IA_NA); ASSERT_TRUE(ia); boost::shared_ptr addr_opt = boost::dynamic_pointer_cast(ia->getOption(D6O_IAADDR)); @@ -1627,14 +1653,14 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) { EXPECT_EQ("lease6_decline", callback_name_); // And valid parameters were passed. - ASSERT_TRUE(callback_pkt6_); + ASSERT_TRUE(callback_qry_pkt6_); ASSERT_TRUE(callback_lease6_); // Test sanity check - it was a decline, right? - EXPECT_EQ(DHCPV6_DECLINE, callback_pkt6_->getType()); + EXPECT_EQ(DHCPV6_DECLINE, callback_qry_pkt6_->getType()); // Get the address from this decline. - OptionPtr ia = callback_pkt6_->getOption(D6O_IA_NA); + OptionPtr ia = callback_qry_pkt6_->getOption(D6O_IA_NA); ASSERT_TRUE(ia); boost::shared_ptr addr_opt = boost::dynamic_pointer_cast(ia->getOption(D6O_IAADDR)); diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 81016c363b..edbefccd45 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -991,6 +991,10 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx, ctx.callout_handle_->deleteAllArguments(); // Pass necessary arguments + + // Pass the original packet + ctx.callout_handle_->setArgument("query6", ctx.query_); + // Subnet from which we do the allocation ctx.callout_handle_->setArgument("subnet6", ctx.subnet_); @@ -1057,6 +1061,9 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx, // Pass necessary arguments + // Pass the original packet + ctx.callout_handle_->setArgument("query6", ctx.query_); + // Subnet from which we do the allocation ctx.callout_handle_->setArgument("subnet6", ctx.subnet_); @@ -2447,6 +2454,8 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) { ctx.callout_handle_->deleteAllArguments(); // Pass necessary arguments + // Pass the original client query + ctx.callout_handle_->setArgument("query4", ctx.query_); // Subnet from which we do the allocation (That's as far as we can go // with using SubnetPtr to point to Subnet4 object. Users should not @@ -2556,6 +2565,7 @@ AllocEngine::renewLease4(const Lease4Ptr& lease, Subnet4Ptr subnet4 = boost::dynamic_pointer_cast(ctx.subnet_); // Pass the parameters + ctx.callout_handle_->setArgument("query4", ctx.query_); ctx.callout_handle_->setArgument("subnet4", subnet4); ctx.callout_handle_->setArgument("clientid", ctx.clientid_); ctx.callout_handle_->setArgument("hwaddr", ctx.hwaddr_); @@ -2626,6 +2636,8 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired, ctx.callout_handle_->deleteAllArguments(); // Pass necessary arguments + // Pass the original client query + ctx.callout_handle_->setArgument("query4", ctx.query_); // Subnet from which we do the allocation. Convert the general subnet // pointer to a pointer to a Subnet4. Note that because we are using diff --git a/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc index d0142c6d24..d38439a418 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_hooks_unittest.cc @@ -44,6 +44,7 @@ public: callback_argument_names_.clear(); callback_addr_original_ = IOAddress("::"); callback_addr_updated_ = IOAddress("::"); + callback_qry_pkt6_.reset(); } /// callback that stores received callout name and received values @@ -52,6 +53,7 @@ public: callback_name_ = string("lease6_select"); + callout_handle.getArgument("query6", callback_qry_pkt6_); callout_handle.getArgument("subnet6", callback_subnet6_); callout_handle.getArgument("fake_allocation", callback_fake_allocation_); callout_handle.getArgument("lease6", callback_lease6_); @@ -99,6 +101,7 @@ public: static Lease6Ptr callback_lease6_; static bool callback_fake_allocation_; static vector callback_argument_names_; + static Pkt6Ptr callback_qry_pkt6_; }; // For some reason intialization within a class makes the linker confused. @@ -118,6 +121,7 @@ Subnet6Ptr HookAllocEngine6Test::callback_subnet6_; Lease6Ptr HookAllocEngine6Test::callback_lease6_; bool HookAllocEngine6Test::callback_fake_allocation_; vector HookAllocEngine6Test::callback_argument_names_; +Pkt6Ptr HookAllocEngine6Test::callback_qry_pkt6_; // This test checks if the lease6_select callout is executed and expected // parameters as passed. @@ -166,6 +170,10 @@ TEST_F(HookAllocEngine6Test, lease6_select) { // Check that callouts were indeed called EXPECT_EQ("lease6_select", callback_name_); + // Check that query6 argument was set correctly + ASSERT_TRUE(callback_qry_pkt6_); + EXPECT_EQ(callback_qry_pkt6_.get(), ctx.query_.get()); + // Now check that the lease in LeaseMgr has the same parameters ASSERT_TRUE(callback_lease6_); detailCompareLease(callback_lease6_, from_mgr); @@ -175,12 +183,13 @@ TEST_F(HookAllocEngine6Test, lease6_select) { EXPECT_FALSE(callback_fake_allocation_); - // Check if all expected parameters are reported. It's a bit tricky, because - // order may be different. If the test starts failing, because someone tweaked - // hooks engine, we'll have to implement proper vector matching (ignoring order) + // Check if all expected parameters are reported. The order needs to be + // alphapbetical to match the order returned by + // CallbackHandle::getAgrumentNames() vector expected_argument_names; expected_argument_names.push_back("fake_allocation"); expected_argument_names.push_back("lease6"); + expected_argument_names.push_back("query6"); expected_argument_names.push_back("subnet6"); sort(callback_argument_names_.begin(), callback_argument_names_.end()); @@ -277,6 +286,7 @@ public: callback_argument_names_.clear(); callback_addr_original_ = IOAddress("::"); callback_addr_updated_ = IOAddress("::"); + callback_qry_pkt4_.reset(); } /// callback that stores received callout name and received values @@ -285,6 +295,7 @@ public: callback_name_ = string("lease4_select"); + callout_handle.getArgument("query4", callback_qry_pkt4_); callout_handle.getArgument("subnet4", callback_subnet4_); callout_handle.getArgument("fake_allocation", callback_fake_allocation_); callout_handle.getArgument("lease4", callback_lease4_); @@ -330,6 +341,7 @@ public: static Lease4Ptr callback_lease4_; static bool callback_fake_allocation_; static vector callback_argument_names_; + static Pkt4Ptr callback_qry_pkt4_; }; // For some reason intialization within a class makes the linker confused. @@ -348,6 +360,7 @@ Subnet4Ptr HookAllocEngine4Test::callback_subnet4_; Lease4Ptr HookAllocEngine4Test::callback_lease4_; bool HookAllocEngine4Test::callback_fake_allocation_; vector HookAllocEngine4Test::callback_argument_names_; +Pkt4Ptr HookAllocEngine4Test::callback_qry_pkt4_; // This test checks if the lease4_select callout is executed and expected // parameters as passed. @@ -398,6 +411,10 @@ TEST_F(HookAllocEngine4Test, lease4_select) { // Check that callouts were indeed called EXPECT_EQ("lease4_select", callback_name_); + // Check that query4 argument was set correctly + ASSERT_TRUE(callback_qry_pkt4_); + EXPECT_EQ(callback_qry_pkt4_.get(), ctx.query_.get()); + // Now check that the lease in LeaseMgr has the same parameters ASSERT_TRUE(callback_lease4_); detailCompareLease(callback_lease4_, from_mgr); @@ -407,12 +424,13 @@ TEST_F(HookAllocEngine4Test, lease4_select) { EXPECT_EQ(callback_fake_allocation_, false); - // Check if all expected parameters are reported. It's a bit tricky, because - // order may be different. If the test starts failing, because someone tweaked - // hooks engine, we'll have to implement proper vector matching (ignoring order) + // Check if all expected parameters are reported. The order needs to be + // alphapbetical to match the order returned by + // CallbackHandle::getAgrumentNames() vector expected_argument_names; expected_argument_names.push_back("fake_allocation"); expected_argument_names.push_back("lease4"); + expected_argument_names.push_back("query4"); expected_argument_names.push_back("subnet4"); EXPECT_TRUE(callback_argument_names_ == expected_argument_names); }