BIND 10 trac2984, updated. 8525babb4efb54926675d2d1a1398638cbc6676d [2984] Tests implemented for buffer6_receive, buffer6_send now pass
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jul 23 18:09:17 UTC 2013
The branch, trac2984 has been updated
via 8525babb4efb54926675d2d1a1398638cbc6676d (commit)
via 4b0906418973479e742cf831d5acdb291384014e (commit)
from 6f304d9ee4e85db2e3aa708ee31135c4e42ef347 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 8525babb4efb54926675d2d1a1398638cbc6676d
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Jul 23 20:09:01 2013 +0200
[2984] Tests implemented for buffer6_receive, buffer6_send now pass
commit 4b0906418973479e742cf831d5acdb291384014e
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Jul 23 20:08:29 2013 +0200
[2984] skip in buffer6_receive not skips unpack procedure.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/dhcp6_srv.cc | 14 +-
src/bin/dhcp6/tests/dhcp6_test_utils.h | 8 ++
src/bin/dhcp6/tests/hook_unittest.cc | 213 +++++++++++++++++++++++++++++-
src/lib/dhcp/pkt6.h | 24 ++--
src/lib/dhcp/tests/iface_mgr_unittest.cc | 6 +-
src/lib/dhcp/tests/pkt6_unittest.cc | 4 +-
6 files changed, 247 insertions(+), 22 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 30aea8f..c44a2e8 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -197,6 +197,8 @@ bool Dhcpv6Srv::run() {
continue;
}
+ bool skip_unpack = false;
+
// Let's execute all callouts registered for buffer6_receive
if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_buffer6_receive_)) {
CalloutHandlePtr callout_handle = getCalloutHandle(query);
@@ -215,16 +217,18 @@ bool Dhcpv6Srv::run() {
// stage means drop.
if (callout_handle->getSkip()) {
LOG_DEBUG(dhcp6_logger, DBG_DHCP6_HOOKS, DHCP6_HOOK_BUFFER_RCVD_SKIP);
- continue;
+ skip_unpack = true;
}
callout_handle->getArgument("query6", query);
}
- if (!query->unpack()) {
- LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
- DHCP6_PACKET_PARSE_FAIL);
- continue;
+ if (!skip_unpack) {
+ if (!query->unpack()) {
+ LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
+ DHCP6_PACKET_PARSE_FAIL);
+ continue;
+ }
}
LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PACKET_RECEIVED)
.arg(query->getName());
diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h
index d9bf298..6a327fe 100644
--- a/src/bin/dhcp6/tests/dhcp6_test_utils.h
+++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h
@@ -278,6 +278,14 @@ public:
// Let's clean up if there is such a file.
unlink(DUID_FILE);
HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "buffer6_receive");
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "buffer6_send");
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "lease6_renew");
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "lease6_release");
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
"pkt6_receive");
HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
"pkt6_send");
diff --git a/src/bin/dhcp6/tests/hook_unittest.cc b/src/bin/dhcp6/tests/hook_unittest.cc
index 278f5a4..c3eb0bd 100644
--- a/src/bin/dhcp6/tests/hook_unittest.cc
+++ b/src/bin/dhcp6/tests/hook_unittest.cc
@@ -56,11 +56,23 @@ TEST_F(Dhcpv6SrvTest, Hooks) {
NakedDhcpv6Srv srv(0);
// check if appropriate hooks are registered
+ int hook_index_buffer6_receive = -1;
+ int hook_index_buffer6_send = -1;
+ int hook_index_lease6_renew = -1;
+ int hook_index_lease6_release = -1;
int hook_index_pkt6_received = -1;
int hook_index_select_subnet = -1;
int hook_index_pkt6_send = -1;
// check if appropriate indexes are set
+ EXPECT_NO_THROW(hook_index_buffer6_receive = ServerHooks::getServerHooks()
+ .getIndex("buffer6_receive"));
+ EXPECT_NO_THROW(hook_index_buffer6_send = ServerHooks::getServerHooks()
+ .getIndex("buffer6_send"));
+ EXPECT_NO_THROW(hook_index_lease6_renew = ServerHooks::getServerHooks()
+ .getIndex("lease6_renew"));
+ EXPECT_NO_THROW(hook_index_lease6_release = ServerHooks::getServerHooks()
+ .getIndex("lease6_release"));
EXPECT_NO_THROW(hook_index_pkt6_received = ServerHooks::getServerHooks()
.getIndex("pkt6_receive"));
EXPECT_NO_THROW(hook_index_select_subnet = ServerHooks::getServerHooks()
@@ -71,6 +83,10 @@ TEST_F(Dhcpv6SrvTest, Hooks) {
EXPECT_TRUE(hook_index_pkt6_received > 0);
EXPECT_TRUE(hook_index_select_subnet > 0);
EXPECT_TRUE(hook_index_pkt6_send > 0);
+ EXPECT_TRUE(hook_index_buffer6_receive > 0);
+ EXPECT_TRUE(hook_index_buffer6_send > 0);
+ EXPECT_TRUE(hook_index_lease6_renew > 0);
+ EXPECT_TRUE(hook_index_lease6_release > 0);
}
// This function returns buffer for very simple Solicit
@@ -208,6 +224,80 @@ public:
return pkt6_receive_callout(callout_handle);
}
+ /// test callback that stores received callout name and pkt6 value
+ /// @param callout_handle handle passed by the hooks framework
+ /// @return always 0
+ static int
+ buffer6_receive_callout(CalloutHandle& callout_handle) {
+ callback_name_ = string("buffer6_receive");
+
+ callout_handle.getArgument("query6", callback_pkt6_);
+
+ callback_argument_names_ = callout_handle.getArgumentNames();
+ return (0);
+ }
+
+ /// test callback that changes first byte of client-id value
+ /// @param callout_handle handle passed by the hooks framework
+ /// @return always 0
+ static int
+ buffer6_receive_change_clientid(CalloutHandle& callout_handle) {
+
+ Pkt6Ptr pkt;
+ callout_handle.getArgument("query6", pkt);
+
+ // If there is at least one option with data
+ if (pkt->data_.size()>Pkt6::DHCPV6_PKT_HDR_LEN + Option::OPTION6_HDR_LEN) {
+ pkt->data_[8] = 0xff;
+ }
+
+ // carry on as usual
+ return buffer6_receive_callout(callout_handle);
+ }
+
+ /// test callback that deletes client-id
+ /// @param callout_handle handle passed by the hooks framework
+ /// @return always 0
+ static int
+ buffer6_receive_delete_clientid(CalloutHandle& callout_handle) {
+
+ Pkt6Ptr pkt;
+ callout_handle.getArgument("query6", pkt);
+
+ // this is modified SOLICIT (with missing mandatory client-id)
+ uint8_t data[] = {
+ 1, // type 1 = SOLICIT
+ 0xca, 0xfe, 0x01, // trans-id = 0xcafe01
+ 0, 3, // option type 3 (IA_NA)
+ 0, 12, // option length 12
+ 0, 0, 0, 1, // iaid = 1
+ 0, 0, 0, 0, // T1 = 0
+ 0, 0, 0, 0 // T2 = 0
+ };
+
+ OptionBuffer modifiedMsg(data, data + sizeof(data));
+
+ pkt->data_ = modifiedMsg;
+
+ // carry on as usual
+ return buffer6_receive_callout(callout_handle);
+ }
+
+ /// test callback that sets skip flag
+ /// @param callout_handle handle passed by the hooks framework
+ /// @return always 0
+ static int
+ buffer6_receive_skip(CalloutHandle& callout_handle) {
+
+ Pkt6Ptr pkt;
+ callout_handle.getArgument("query6", pkt);
+
+ callout_handle.setSkip(true);
+
+ // carry on as usual
+ return buffer6_receive_callout(callout_handle);
+ }
+
/// Test callback that stores received callout name and pkt6 value
/// @param callout_handle handle passed by the hooks framework
/// @return always 0
@@ -348,7 +438,128 @@ const Subnet6Collection* HooksDhcpv6SrvTest::callback_subnet6collection_;
vector<string> HooksDhcpv6SrvTest::callback_argument_names_;
-// Checks if callouts installed on pkt6_received are indeed called and the
+// Checks if callouts installed on pkt6_receive are indeed called and the
+// all necessary parameters are passed.
+//
+// Note that the test name does not follow test naming convention,
+// but the proper hook name is "buffer6_receive".
+TEST_F(HooksDhcpv6SrvTest, simple_buffer6_receive) {
+
+ // Install pkt6_receive_callout
+ EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+ "buffer6_receive", buffer6_receive_callout));
+
+ // Let's create a simple SOLICIT
+ Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
+
+ // Simulate that we have received that traffic
+ srv_->fakeReceive(sol);
+
+ // Server will now process to run its normal loop, but instead of calling
+ // IfaceMgr::receive6(), it will read all packets from the list set by
+ // fakeReceive()
+ // In particular, it should call registered pkt6_receive callback.
+ srv_->run();
+
+ // check that the callback called is indeed the one we installed
+ EXPECT_EQ("buffer6_receive", callback_name_);
+
+ // check that pkt6 argument passing was successful and returned proper value
+ EXPECT_TRUE(callback_pkt6_.get() == sol.get());
+
+ // Check that all expected parameters are there
+ vector<string> expected_argument_names;
+ expected_argument_names.push_back(string("query6"));
+
+ EXPECT_TRUE(expected_argument_names == callback_argument_names_);
+}
+
+// Checks if callouts installed on pkt6_received is able to change
+// the values and the parameters are indeed used by the server.
+TEST_F(HooksDhcpv6SrvTest, valueChange_buffer6_receive) {
+
+ // Install pkt6_receive_callout
+ EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+ "buffer6_receive", buffer6_receive_change_clientid));
+
+ // Let's create a simple SOLICIT
+ Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
+
+ // Simulate that we have received that traffic
+ srv_->fakeReceive(sol);
+
+ // Server will now process to run its normal loop, but instead of calling
+ // IfaceMgr::receive6(), it will read all packets from the list set by
+ // fakeReceive()
+ // In particular, it should call registered pkt6_receive callback.
+ srv_->run();
+
+ // check that the server did send a reposonce
+ ASSERT_EQ(1, srv_->fake_sent_.size());
+
+ // Make sure that we received a response
+ Pkt6Ptr adv = srv_->fake_sent_.front();
+ ASSERT_TRUE(adv);
+
+ // Get client-id...
+ OptionPtr clientid = adv->getOption(D6O_CLIENTID);
+
+ ASSERT_TRUE(clientid);
+
+ // ... and check if it is the modified value
+ EXPECT_EQ(0xff, clientid->getData()[0]);
+}
+
+// Checks if callouts installed on buffer6_receive is able to delete
+// existing options and that change impacts server processing (mandatory
+// client-id option is deleted, so the packet is expected to be dropped)
+TEST_F(HooksDhcpv6SrvTest, deleteClientId_buffer6_receive) {
+
+ // Install pkt6_receive_callout
+ EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+ "buffer6_receive", buffer6_receive_delete_clientid));
+
+ // Let's create a simple SOLICIT
+ Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
+
+ // Simulate that we have received that traffic
+ srv_->fakeReceive(sol);
+
+ // Server will now process to run its normal loop, but instead of calling
+ // IfaceMgr::receive6(), it will read all packets from the list set by
+ // fakeReceive()
+ // In particular, it should call registered pkt6_receive callback.
+ srv_->run();
+
+ // Check that the server dropped the packet and did not send a response
+ ASSERT_EQ(0, srv_->fake_sent_.size());
+}
+
+// Checks if callouts installed on buffer6_received is able to set skip flag that
+// will cause the server to not process the packet (drop), even though it is valid.
+TEST_F(HooksDhcpv6SrvTest, skip_buffer6_receive) {
+
+ // Install pkt6_receive_callout
+ EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
+ "buffer6_receive", buffer6_receive_skip));
+
+ // Let's create a simple SOLICIT
+ Pkt6Ptr sol = Pkt6Ptr(captureSimpleSolicit());
+
+ // Simulate that we have received that traffic
+ srv_->fakeReceive(sol);
+
+ // Server will now process to run its normal loop, but instead of calling
+ // IfaceMgr::receive6(), it will read all packets from the list set by
+ // fakeReceive()
+ // In particular, it should call registered pkt6_receive callback.
+ srv_->run();
+
+ // check that the server dropped the packet and did not produce any response
+ ASSERT_EQ(0, srv_->fake_sent_.size());
+}
+
+// Checks if callouts installed on pkt6_receive are indeed called and the
// all necessary parameters are passed.
//
// Note that the test name does not follow test naming convention,
diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h
index c65142e..4094708 100644
--- a/src/lib/dhcp/pkt6.h
+++ b/src/lib/dhcp/pkt6.h
@@ -142,7 +142,7 @@ public:
/// @brief Returns reference to input buffer.
///
/// @return reference to input buffer
- const OptionBuffer& getData() const { return(data_); }
+ /// const OptionBuffer& getData() const { return(data_); }
/// @brief Returns protocol of this packet (UDP or TCP).
///
@@ -410,6 +410,18 @@ public:
/// to be impossible). Therefore public field is considered the best
/// (or least bad) solution.
std::vector<RelayInfo> relay_info_;
+
+
+ /// unparsed data (in received packets)
+ ///
+ /// @warning This public member is accessed by derived
+ /// classes directly. One of such derived classes is
+ /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+ /// behavior must be taken into consideration before making
+ /// changes to this member such as access scope restriction or
+ /// data format change etc.
+ OptionBuffer data_;
+
protected:
/// Builds on wire packet for TCP transmission.
///
@@ -492,16 +504,6 @@ protected:
/// DHCPv6 transaction-id
uint32_t transid_;
- /// unparsed data (in received packets)
- ///
- /// @warning This protected member is accessed by derived
- /// classes directly. One of such derived classes is
- /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
- /// behavior must be taken into consideration before making
- /// changes to this member such as access scope restriction or
- /// data format change etc.
- OptionBuffer data_;
-
/// name of the network interface the packet was received/to be sent over
std::string iface_;
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index cdb0159..488ecb3 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -766,9 +766,9 @@ TEST_F(IfaceMgrTest, sendReceive6) {
ASSERT_TRUE(rcvPkt); // received our own packet
// let's check that we received what was sent
- ASSERT_EQ(sendPkt->getData().size(), rcvPkt->getData().size());
- EXPECT_EQ(0, memcmp(&sendPkt->getData()[0], &rcvPkt->getData()[0],
- rcvPkt->getData().size()));
+ ASSERT_EQ(sendPkt->data_.size(), rcvPkt->data_.size());
+ EXPECT_EQ(0, memcmp(&sendPkt->data_[0], &rcvPkt->data_[0],
+ rcvPkt->data_.size()));
EXPECT_EQ(sendPkt->getRemoteAddr().toText(), rcvPkt->getRemoteAddr().toText());
diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc
index d4907d6..123b9e5 100644
--- a/src/lib/dhcp/tests/pkt6_unittest.cc
+++ b/src/lib/dhcp/tests/pkt6_unittest.cc
@@ -64,8 +64,8 @@ TEST_F(Pkt6Test, constructor) {
uint8_t data[] = { 0, 1, 2, 3, 4, 5 };
scoped_ptr<Pkt6> pkt1(new Pkt6(data, sizeof(data)));
- EXPECT_EQ(6, pkt1->getData().size());
- EXPECT_EQ(0, memcmp( &pkt1->getData()[0], data, sizeof(data)));
+ EXPECT_EQ(6, pkt1->data_.size());
+ EXPECT_EQ(0, memcmp( &pkt1->data_[0], data, sizeof(data)));
}
/// @brief returns captured actual SOLICIT packet
More information about the bind10-changes
mailing list