BIND 10 trac2995, updated. d6de376f97313ba40fef989e4a437d184fdf70cc [2995] Subnet6Collection is now passed as pointer to const object
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Jul 12 12:48:08 UTC 2013
The branch, trac2995 has been updated
via d6de376f97313ba40fef989e4a437d184fdf70cc (commit)
via 14018a4e7e240d9770dfd565284abdf2f82f9b30 (commit)
from 78fdf6daaf77f5e341e921bb1cb3ac7084060c75 (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 d6de376f97313ba40fef989e4a437d184fdf70cc
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Fri Jul 12 14:47:51 2013 +0200
[2995] Subnet6Collection is now passed as pointer to const object
commit 14018a4e7e240d9770dfd565284abdf2f82f9b30
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Fri Jul 12 14:28:01 2013 +0200
[2995] Changes after review:
- callouts called using HooksManager::callCallouts
- removed Dhcp6Srv::getServerHooks() and getHooksManager() declarations.
- updated sendPacket comment (reception->transmission)
- documented getCalloutHandle()
- indentation fixed for getSubnets6()
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/dhcp6_hooks.dox | 2 +-
src/bin/dhcp6/dhcp6_srv.cc | 18 +++++--------
src/bin/dhcp6/dhcp6_srv.h | 23 +++++-----------
src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 41 +++++++++++++++++------------
src/lib/dhcpsrv/alloc_engine.cc | 6 ++---
src/lib/dhcpsrv/cfgmgr.h | 8 +++---
6 files changed, 45 insertions(+), 53 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox
index cf03e50..c9379c6 100644
--- a/src/bin/dhcp6/dhcp6_hooks.dox
+++ b/src/bin/dhcp6/dhcp6_hooks.dox
@@ -75,7 +75,7 @@ packet processing. Hook points that are not specific to packet processing
- @b Arguments:
- name: @b query6, type: isc::dhcp::Pkt6Ptr, direction: <b>in/out</b>
- name: @b subnet6, type: isc::dhcp::Subnet6Ptr, direction: <b>in/out</b>
- - name: @b subnet6collection, type: const isc::dhcp::Subnet6Collection&, direction: <b>in</b>
+ - name: @b subnet6collection, type: const isc::dhcp::Subnet6Collection *, direction: <b>in</b>
- @b Description: this callout is executed when a subnet is being
selected for the incoming packet. All parameters, addresses and
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 3389837..1ea6965 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -141,8 +141,6 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
hook_index_pkt6_send_ = Hooks.hook_index_pkt6_send_;
/// @todo call loadLibraries() when handling configuration changes
- vector<string> libraries; // no libraries at this time
- HooksManager::loadLibraries(libraries);
} catch (const std::exception &e) {
LOG_ERROR(dhcp6_logger, DHCP6_SRV_CONSTRUCT_ERROR).arg(e.what());
@@ -217,8 +215,7 @@ bool Dhcpv6Srv::run() {
callout_handle->setArgument("query6", query);
// Call callouts
- HooksManager::getHooksManager().callCallouts(hook_index_pkt6_receive_,
- *callout_handle);
+ HooksManager::callCallouts(hook_index_pkt6_receive_, *callout_handle);
// Callouts decided to skip the next processing step. The next
// processing step would to process the packet, so skip at this
@@ -308,15 +305,11 @@ bool Dhcpv6Srv::run() {
// Delete all previous arguments
callout_handle->deleteAllArguments();
- // Clear skip flag if it was set in previous callouts
- callout_handle->setSkip(false);
-
// Set our response
callout_handle->setArgument("response6", rsp);
// Call all installed callouts
- HooksManager::getHooksManager().callCallouts(hook_index_pkt6_send_,
- *callout_handle);
+ HooksManager::callCallouts(hook_index_pkt6_send_, *callout_handle);
// Callouts decided to skip the next processing step. The next
// processing step would to send the packet, so skip at this
@@ -667,11 +660,14 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
// Set new arguments
callout_handle->setArgument("query6", question);
callout_handle->setArgument("subnet6", subnet);
+
+ // We pass pointer to const collection for performance reasons.
+ // Otherwise we would get a non-trivial performance penalty each
+ // time subnet6_select is called.
callout_handle->setArgument("subnet6collection", CfgMgr::instance().getSubnets6());
// Call user (and server-side) callouts
- HooksManager::getHooksManager().callCallouts(hook_index_subnet6_select_,
- *callout_handle);
+ HooksManager::callCallouts(hook_index_subnet6_select_, *callout_handle);
// Callouts decided to skip this step. This means that no subnet will be
// selected. Packet processing will continue, but it will be severly limited
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index d1c7bb6..3954581 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -23,7 +23,7 @@
#include <dhcp/pkt6.h>
#include <dhcpsrv/alloc_engine.h>
#include <dhcpsrv/subnet.h>
-#include <hooks/hooks_manager.h>
+#include <hooks/callout_handle.h>
#include <boost/noncopyable.hpp>
@@ -88,20 +88,6 @@ public:
/// @brief Instructs the server to shut down.
void shutdown();
- /// @brief returns ServerHooks object
- /// @todo: remove this as soon as ServerHooks object is converted
- /// to a signleton.
- //static boost::shared_ptr<isc::util::ServerHooks> getServerHooks();
-
- /// @brief returns Callout Manager object
- ///
- /// This manager is used to manage callouts registered on various hook
- /// points. @todo exact access method for HooksManager manager will change
- /// when it will be converted to a singleton.
- ///
- /// @return CalloutManager instance
- //static boost::shared_ptr<isc::util::HooksManager> getHooksManager();
-
protected:
/// @brief verifies if specified packet meets RFC requirements
@@ -347,7 +333,7 @@ protected:
/// @brief dummy wrapper around IfaceMgr::send()
///
/// This method is useful for testing purposes, where its replacement
- /// simulates reception of a packet. For that purpose it is protected.
+ /// simulates transmission of a packet. For that purpose it is protected.
virtual void sendPacket(const Pkt6Ptr& pkt);
private:
@@ -364,6 +350,11 @@ private:
/// initiate server shutdown procedure.
volatile bool shutdown_;
+ /// @brief returns callout handle for specified packet
+ ///
+ /// @param pkt packet for which the handle should be returned
+ ///
+ /// @return a callout handle to be used in hooks related to said packet
isc::hooks::CalloutHandlePtr getCalloutHandle(const Pkt6Ptr& pkt);
/// Indexes for registered hook points
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 5c7f5b6..7957ed4 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -295,6 +295,13 @@ public:
virtual ~NakedDhcpv6SrvTest() {
// Let's clean up if there is such a file.
unlink(DUID_FILE);
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "pkt6_receive");
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "pkt6_send");
+ HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts(
+ "subnet6_select");
+
};
// A DUID used in most tests (typically as client-id)
@@ -1897,8 +1904,8 @@ Pkt6* captureSimpleSolicit() {
/// @brief a class dedicated to Hooks testing in DHCPv6 server
///
-/// This class has a some static members, because each non-static
-/// method has implicit this parameter, so it does not match callout
+/// This class has a number of static members, because each non-static
+/// method has implicit 'this' parameter, so it does not match callout
/// signature and couldn't be registered. Furthermore, static methods
/// can't modify non-static members (for obvious reasons), so many
/// fields are declared static. It is still better to keep them as
@@ -2090,14 +2097,14 @@ public:
// Call the basic calllout to record all passed values
subnet6_select_callout(callout_handle);
- Subnet6Collection subnets;
+ const Subnet6Collection* subnets;
Subnet6Ptr subnet;
callout_handle.getArgument("subnet6", subnet);
callout_handle.getArgument("subnet6collection", subnets);
// Let's change to a different subnet
- if (subnets.size() > 1) {
- subnet = subnets[1]; // Let's pick the other subnet
+ if (subnets->size() > 1) {
+ subnet = (*subnets)[1]; // Let's pick the other subnet
callout_handle.setArgument("subnet6", subnet);
}
@@ -2109,7 +2116,7 @@ public:
callback_name_ = string("");
callback_pkt6_.reset();
callback_subnet6_.reset();
- callback_subnet6collection_.clear();
+ callback_subnet6collection_ = NULL;
callback_argument_names_.clear();
}
@@ -2128,7 +2135,7 @@ public:
static Subnet6Ptr callback_subnet6_;
/// A list of all available subnets (received by callout)
- static Subnet6Collection callback_subnet6collection_;
+ static const Subnet6Collection* callback_subnet6collection_;
/// A list of all received arguments
static vector<string> callback_argument_names_;
@@ -2139,7 +2146,7 @@ public:
string HooksDhcpv6SrvTest::callback_name_;
Pkt6Ptr HooksDhcpv6SrvTest::callback_pkt6_;
Subnet6Ptr HooksDhcpv6SrvTest::callback_subnet6_;
-Subnet6Collection HooksDhcpv6SrvTest::callback_subnet6collection_;
+const Subnet6Collection* HooksDhcpv6SrvTest::callback_subnet6collection_;
vector<string> HooksDhcpv6SrvTest::callback_argument_names_;
@@ -2445,19 +2452,19 @@ TEST_F(HooksDhcpv6SrvTest, subnet6_select) {
// Check that pkt6 argument passing was successful and returned proper value
EXPECT_TRUE(callback_pkt6_.get() == sol.get());
- Subnet6Collection exp_subnets = CfgMgr::instance().getSubnets6();
+ const Subnet6Collection* exp_subnets = CfgMgr::instance().getSubnets6();
// The server is supposed to pick the first subnet, because of matching
// interface. Check that the value is reported properly.
ASSERT_TRUE(callback_subnet6_);
- EXPECT_EQ(callback_subnet6_.get(), exp_subnets.front().get());
+ EXPECT_EQ(callback_subnet6_.get(), exp_subnets->front().get());
// Server is supposed to report two subnets
- ASSERT_EQ(exp_subnets.size(), callback_subnet6collection_.size());
+ ASSERT_EQ(exp_subnets->size(), callback_subnet6collection_->size());
// Compare that the available subnets are reported as expected
- EXPECT_TRUE(exp_subnets[0].get() == callback_subnet6collection_[0].get());
- EXPECT_TRUE(exp_subnets[1].get() == callback_subnet6collection_[1].get());
+ EXPECT_TRUE((*exp_subnets)[0].get() == (*callback_subnet6collection_)[0].get());
+ EXPECT_TRUE((*exp_subnets)[1].get() == (*callback_subnet6collection_)[1].get());
}
// This test checks if callout installed on subnet6_select hook point can pick
@@ -2519,13 +2526,13 @@ TEST_F(HooksDhcpv6SrvTest, subnet_select_change) {
ASSERT_TRUE(addr_opt);
// Get all subnets and use second subnet for verification
- Subnet6Collection subnets = CfgMgr::instance().getSubnets6();
- ASSERT_EQ(2, subnets.size());
+ const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
+ ASSERT_EQ(2, subnets->size());
// Advertised address must belong to the second pool (in subnet's range,
// in dynamic pool)
- EXPECT_TRUE(subnets[1]->inRange(addr_opt->getAddress()));
- EXPECT_TRUE(subnets[1]->inPool(addr_opt->getAddress()));
+ EXPECT_TRUE((*subnets)[1]->inRange(addr_opt->getAddress()));
+ EXPECT_TRUE((*subnets)[1]->inPool(addr_opt->getAddress()));
}
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 9967ac6..e94201d 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -509,8 +509,7 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
callout_handle->setArgument("lease6", expired);
// Call the callouts
- HooksManager::getHooksManager().callCallouts(hook_index_lease6_select_,
- *callout_handle);
+ HooksManager::callCallouts(hook_index_lease6_select_, *callout_handle);
// Callouts decided to skip the action. This means that the lease is not
// assigned, so the client will get NoAddrAvail as a result. The lease
@@ -608,8 +607,7 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
callout_handle->setArgument("lease6", lease);
// This is the first callout, so no need to clear any arguments
- HooksManager::getHooksManager().callCallouts(hook_index_lease6_select_,
- *callout_handle);
+ HooksManager::callCallouts(hook_index_lease6_select_, *callout_handle);
// Callouts decided to skip the action. This means that the lease is not
// assigned, so the client will get NoAddrAvail as a result. The lease
diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h
index 9d4edce..28cfc76 100644
--- a/src/lib/dhcpsrv/cfgmgr.h
+++ b/src/lib/dhcpsrv/cfgmgr.h
@@ -207,10 +207,10 @@ public:
/// This is used in a hook (subnet6_select), where the hook is able
/// to choose a different subnet. Server code has to offer a list
/// of possible choices (i.e. all subnets).
- /// @return const reference to Subnet6 collection
- inline const Subnet6Collection& getSubnets6() {
- return (subnets6_);
-}
+ /// @return a pointer to const Subnet6 collection
+ const Subnet6Collection* getSubnets6() {
+ return (&subnets6_);
+ }
/// @brief get IPv4 subnet by address
More information about the bind10-changes
mailing list