BIND 10 trac2324, updated. 9394d67b638a84409c3491fee821f347ca2acbcf [2324] Changes after review.
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Oct 26 13:39:10 UTC 2012
The branch, trac2324 has been updated
via 9394d67b638a84409c3491fee821f347ca2acbcf (commit)
from 6a8228eb8556681c30c7163010f7fb354a9ef66c (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 9394d67b638a84409c3491fee821f347ca2acbcf
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Fri Oct 26 15:37:45 2012 +0200
[2324] Changes after review.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/alloc_engine.cc | 28 ++++++++-----------
src/lib/dhcp/alloc_engine.h | 45 ++++++++++++++----------------
src/lib/dhcp/lease_mgr.cc | 2 +-
src/lib/dhcp/lease_mgr.h | 14 +++++-----
src/lib/dhcp/subnet.cc | 1 -
src/lib/dhcp/subnet.h | 33 +++++++++++++++++++---
src/lib/dhcp/tests/lease_mgr_unittest.cc | 18 ++++++------
src/lib/dhcp/tests/memfile_lease_mgr.cc | 8 +++---
src/lib/dhcp/tests/memfile_lease_mgr.h | 30 +++++++++++++++++---
9 files changed, 109 insertions(+), 70 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/alloc_engine.cc b/src/lib/dhcp/alloc_engine.cc
index 1ab677f..5e8d90d 100644
--- a/src/lib/dhcp/alloc_engine.cc
+++ b/src/lib/dhcp/alloc_engine.cc
@@ -12,7 +12,6 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-#include <asiolink/io_address.h>
#include <alloc_engine.h>
using namespace isc::asiolink;
@@ -41,7 +40,7 @@ AllocEngine::IterativeAllocator::increaseAddress(const isc::asiolink::IOAddress&
}
for (int i = len - 1; i >= 0; --i) {
- packed[i]++;
+ ++packed[i];
if (packed[i] != 0) {
break;
}
@@ -67,8 +66,6 @@ AllocEngine::IterativeAllocator::pickAddress(const Subnet6Ptr& subnet,
isc_throw(AllocFailed, "No pools defined in selected subnet");
}
- Pool6Ptr pool = Pool6Ptr(); // null
-
// first we need to find a pool the last address belongs to.
Pool6Collection::const_iterator it;
for (it = pools.begin(); it != pools.end(); ++it) {
@@ -144,13 +141,13 @@ AllocEngine::AllocEngine(AllocType engine_type, unsigned int attempts)
:attempts_(attempts) {
switch (engine_type) {
case ALLOC_ITERATIVE:
- allocator_ = new IterativeAllocator();
+ allocator_ = boost::shared_ptr<Allocator>(new IterativeAllocator());
break;
case ALLOC_HASHED:
- allocator_ = new HashedAllocator();
+ allocator_ = boost::shared_ptr<Allocator>(new HashedAllocator());
break;
case ALLOC_RANDOM:
- allocator_ = new RandomAllocator();
+ allocator_ = boost::shared_ptr<Allocator>(new RandomAllocator());
break;
default:
@@ -163,7 +160,8 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
const DuidPtr& duid,
uint32_t iaid,
const IOAddress& hint,
- bool fake /* = false */ ) {
+ bool fake_allocation /* = false */ ) {
+
// That check is not necessary. We create allocator in AllocEngine
// constructor
if (!allocator_) {
@@ -186,7 +184,7 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
/// implemented
// the hint is valid and not currently used, let's create a lease for it
- Lease6Ptr lease = createLease(subnet, duid, iaid, hint, fake);
+ Lease6Ptr lease = createLease(subnet, duid, iaid, hint, fake_allocation);
// It can happen that the lease allocation failed (we could have lost
// the race condition. That means that the hint is lo longer usable and
@@ -208,7 +206,8 @@ AllocEngine::allocateAddress6(const Subnet6Ptr& subnet,
// there's no existing lease for selected candidate, so it is
// free. Let's allocate it.
if (!existing) {
- Lease6Ptr lease = createLease(subnet, duid, iaid, candidate, fake);
+ Lease6Ptr lease = createLease(subnet, duid, iaid, candidate,
+ fake_allocation);
if (lease) {
return (lease);
}
@@ -231,13 +230,13 @@ Lease6Ptr AllocEngine::createLease(const Subnet6Ptr& subnet,
const DuidPtr& duid,
uint32_t iaid,
const IOAddress& addr,
- bool fake /*= false */ ) {
+ bool fake_allocation /*= false */ ) {
Lease6Ptr lease(new Lease6(Lease6::LEASE_IA_NA, addr, duid, iaid,
subnet->getPreferred(), subnet->getValid(),
subnet->getT1(), subnet->getT2(), subnet->getID()));
- if (!fake) {
+ if (!fake_allocation) {
// That is a real (REQUEST) allocation
bool status = LeaseMgr::instance().addLease(lease);
@@ -266,10 +265,7 @@ Lease6Ptr AllocEngine::createLease(const Subnet6Ptr& subnet,
}
AllocEngine::~AllocEngine() {
- if (allocator_) {
- delete allocator_;
- allocator_ = NULL;
- }
+ // no need to delete allocator. smart_ptr will do the trick for us
}
}; // end of isc::dhcp namespace
diff --git a/src/lib/dhcp/alloc_engine.h b/src/lib/dhcp/alloc_engine.h
index 5df70cb..74e30dd 100644
--- a/src/lib/dhcp/alloc_engine.h
+++ b/src/lib/dhcp/alloc_engine.h
@@ -15,12 +15,12 @@
#ifndef ALLOC_ENGINE_H
#define ALLOC_ENGINE_H
+#include <boost/shared_ptr.hpp>
#include <boost/noncopyable.hpp>
#include <dhcp/duid.h>
#include <dhcp/subnet.h>
#include <asiolink/io_address.h>
#include <dhcp/lease_mgr.h>
-#include <iostream>
namespace isc {
namespace dhcp {
@@ -30,13 +30,13 @@ namespace dhcp {
class AllocFailed : public isc::Exception {
public:
-/// @brief constructor
-///
-/// @param file name of the file, where exception occurred
-/// @param line line of the file, where exception occurred
-/// @param what text description of the issue that caused exception
-AllocFailed(const char* file, size_t line, const char* what)
- : isc::Exception(file, line, what) {}
+ /// @brief constructor
+ ///
+ /// @param file name of the file, where exception occurred
+ /// @param line line of the file, where exception occurred
+ /// @param what text description of the issue that caused exception
+ AllocFailed(const char* file, size_t line, const char* what)
+ : isc::Exception(file, line, what) {}
};
/// @brief DHCPv4 and DHCPv6 allocation engine
@@ -65,22 +65,17 @@ protected:
/// again if necessary. The number of times this method is called will
/// increase as the number of available leases will decrease.
virtual isc::asiolink::IOAddress
- pickAddress(const Subnet6Ptr& subnet,
- const DuidPtr& duid,
- const isc::asiolink::IOAddress& hint) = 0;
+ pickAddress(const Subnet6Ptr& subnet, const DuidPtr& duid,
+ const isc::asiolink::IOAddress& hint) = 0;
protected:
- /// @brief protected constructor
- ///
- /// Prevents anyone from attempting to instantiate Allocator objects
- /// directly. Derived classes should be used instead.
- Allocator() {
- }
};
/// @brief Address/prefix allocator that iterates over all addresses
///
/// This class implements iterative algorithm that returns all addresses in
- /// a pool iteratively, one after another.
+ /// a pool iteratively, one after another. Once the last address is reached,
+ /// it starts allocating from the beginning of the first pool (i.e. it loops
+ /// over).
class IterativeAllocator : public Allocator {
public:
@@ -184,14 +179,15 @@ protected:
/// @param duid Client'd DUID
/// @param iaid iaid field from the IA_NA container that client sent
/// @param hint a hint that the client provided
- /// @param fake is this real (REQUEST) or fake (SOLICIT) allocation
+ /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
+ /// an address for SOLICIT that is not really allocated (true)
/// @return Allocated IPv6 lease (or NULL if allocation failed)
Lease6Ptr
allocateAddress6(const Subnet6Ptr& subnet,
const DuidPtr& duid,
uint32_t iaid,
const isc::asiolink::IOAddress& hint,
- bool fake);
+ bool fake_allocation);
/// @brief Destructor. Used during DHCPv6 service shutdown.
virtual ~AllocEngine();
@@ -207,15 +203,16 @@ private:
/// @param duid client's DUID
/// @param iaid IAID from the IA_NA container the client sent to us
/// @param addr an address that was selected and is confirmed to be available
- /// @param fake is this SOLICIT (true) or a real/REQUEST allocation (false)?
+ /// @param fake_allocation is this real i.e. REQUEST (false) or just picking
+ /// an address for SOLICIT that is not really allocated (true)
/// @return allocated lease (or NULL in the unlikely case of the lease just
/// becomed unavailable)
Lease6Ptr createLease(const Subnet6Ptr& subnet, const DuidPtr& duid,
uint32_t iaid, const isc::asiolink::IOAddress& addr,
- bool fake = false);
+ bool fake_allocation = false);
/// @brief a pointer to currently used allocator
- Allocator* allocator_;
+ boost::shared_ptr<Allocator> allocator_;
/// @brief number of attempts before we give up lease allocation (0=unlimited)
unsigned int attempts_;
@@ -224,4 +221,4 @@ private:
}; // namespace isc::dhcp
}; // namespace isc
-#endif // DHCP6_SRV_H
+#endif // ALLOC_ENGINE_H
diff --git a/src/lib/dhcp/lease_mgr.cc b/src/lib/dhcp/lease_mgr.cc
index 2e77ca6..59582fb 100644
--- a/src/lib/dhcp/lease_mgr.cc
+++ b/src/lib/dhcp/lease_mgr.cc
@@ -39,7 +39,7 @@ Lease6::Lease6(LeaseType type, const isc::asiolink::IOAddress& addr, DuidPtr dui
preferred_lft_(preferred), valid_lft_(valid), t1_(t1), t2_(t2),
subnet_id_(subnet_id), fixed_(false), fqdn_fwd_(false),
fqdn_rev_(false) {
- if (duid == DuidPtr()) {
+ if (!duid) {
isc_throw(InvalidOperation, "DUID must be specified for a lease");
}
diff --git a/src/lib/dhcp/lease_mgr.h b/src/lib/dhcp/lease_mgr.h
index ec2853c..7541a15 100644
--- a/src/lib/dhcp/lease_mgr.h
+++ b/src/lib/dhcp/lease_mgr.h
@@ -289,9 +289,9 @@ public:
/// @brief returns a single instance of LeaseMgr
///
/// LeaseMgr is a singleton and this method is the only way of
- /// accessing it. LeaseMgr must be create first using
- /// instantiate() method, otherwise instance() will throw
- /// InvalidOperation exception.
+ /// accessing it. LeaseMgr must be created first. See
+ /// isc::dhcp::LeaseMgrFactory class (work of ticket #2342.
+ /// Otherwise instance() will throw InvalidOperation exception.
/// @throw InvalidOperation if LeaseMgr not instantiated
static LeaseMgr& instance();
@@ -305,12 +305,12 @@ public:
/// @brief Adds an IPv4 lease.
///
/// @param lease lease to be added
- virtual bool addLease(Lease4Ptr lease) = 0;
+ virtual bool addLease(const Lease4Ptr& lease) = 0;
/// @brief Adds an IPv6 lease.
///
/// @param lease lease to be added
- virtual bool addLease(Lease6Ptr lease) = 0;
+ virtual bool addLease(const Lease6Ptr& lease) = 0;
/// @brief Returns existing IPv4 lease for specified IPv4 address and subnet_id
///
@@ -429,14 +429,14 @@ public:
/// @param lease4 The lease to be updated.
///
/// If no such lease is present, an exception will be thrown.
- virtual void updateLease4(Lease4Ptr lease4) = 0;
+ virtual void updateLease4(const Lease4Ptr& lease4) = 0;
/// @brief Updates IPv4 lease.
///
/// @param lease4 The lease to be updated.
///
/// If no such lease is present, an exception will be thrown.
- virtual void updateLease6(Lease6Ptr lease6) = 0;
+ virtual void updateLease6(const Lease6Ptr& lease6) = 0;
/// @brief Deletes a lease.
///
diff --git a/src/lib/dhcp/subnet.cc b/src/lib/dhcp/subnet.cc
index 5fb1d43..230a1b9 100644
--- a/src/lib/dhcp/subnet.cc
+++ b/src/lib/dhcp/subnet.cc
@@ -15,7 +15,6 @@
#include <dhcp/addr_utilities.h>
#include <asiolink/io_address.h>
#include <dhcp/subnet.h>
-#include <dhcp/pool.h>
using namespace isc::asiolink;
diff --git a/src/lib/dhcp/subnet.h b/src/lib/dhcp/subnet.h
index 9238240..714ed9e 100644
--- a/src/lib/dhcp/subnet.h
+++ b/src/lib/dhcp/subnet.h
@@ -49,10 +49,11 @@ public:
/// subnet (e.g. 2001::/64) there may be one or more pools defined
/// that may or may not cover entire subnet, e.g. pool 2001::1-2001::10).
/// inPool() returning true implies inSubnet(), but the reverse implication
- /// is not always true. For the given example, 2001::abc would return
+ /// is not always true. For the given example, 2001::1234:abcd would return
/// true for inSubnet(), but false for inPool() check.
///
- /// @param addr this address will be checked if it belongs to any pools in that subnet
+ /// @param addr this address will be checked if it belongs to any pools in
+ /// that subnet
/// @return true if the address is in any of the pools
virtual bool inPool(const isc::asiolink::IOAddress& addr) const = 0;
@@ -71,15 +72,35 @@ public:
return (t2_);
}
- isc::asiolink::IOAddress getLastAllocated() {
+ /// @brief returns the last address that was tried from this pool
+ ///
+ /// This method returns the last address that was attempted to be allocated
+ /// from this subnet. This is used as helper information for the next
+ /// iteration of the allocation algorithm.
+ ///
+ /// @todo: Define map<SubnetID, IOAddress> somewhere in the
+ /// AllocEngine::IterativeAllocator and keep the data there
+ ///
+ /// @return address that was last tried from this pool
+ isc::asiolink::IOAddress getLastAllocated() const {
return (last_allocated_);
}
+ /// @brief sets the last address that was tried from this pool
+ ///
+ /// This method sets the last address that was attempted to be allocated
+ /// from this subnet. This is used as helper information for the next
+ /// iteration of the allocation algorithm.
+ ///
+ /// @todo: Define map<SubnetID, IOAddress> somewhere in the
+ /// AllocEngine::IterativeAllocator and keep the data there
void setLastAllocated(const isc::asiolink::IOAddress& addr) {
last_allocated_ = addr;
}
- SubnetID getID() { return id_; }
+ /// @brief returns unique ID for that subnet
+ /// @return unique ID for that subnet
+ SubnetID getID() const { return (id_); }
protected:
/// @brief protected constructor
@@ -91,6 +112,10 @@ protected:
const Triplet<uint32_t>& t2,
const Triplet<uint32_t>& valid_lifetime);
+ /// @brief virtual destructor
+ virtual ~Subnet() {
+ };
+
/// @brief returns the next unique Subnet-ID
///
/// @return the next unique Subnet-ID
diff --git a/src/lib/dhcp/tests/lease_mgr_unittest.cc b/src/lib/dhcp/tests/lease_mgr_unittest.cc
index 8446160..743b34a 100644
--- a/src/lib/dhcp/tests/lease_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/lease_mgr_unittest.cc
@@ -86,18 +86,18 @@ TEST_F(LeaseMgrTest, addGetDelete) {
x = leaseMgr->getLease6(IOAddress("2001:db8:1::456"));
ASSERT_TRUE(x);
- EXPECT_TRUE(x->addr_ == addr);
- EXPECT_TRUE(*x->duid_ == *duid);
- EXPECT_TRUE(x->iaid_ == iaid);
- EXPECT_TRUE(x->subnet_id_ == subnet_id);
+ EXPECT_EQ(x->addr_.toText(), addr.toText());
+ EXPECT_EQ(*x->duid_, *duid);
+ EXPECT_EQ(x->iaid_, iaid);
+ EXPECT_EQ(x->subnet_id_, subnet_id);
// These are not important from lease management perspective, but
// let's check them anyway.
- EXPECT_TRUE(x->type_ == Lease6::LEASE_IA_NA);
- EXPECT_TRUE(x->preferred_lft_ == 100);
- EXPECT_TRUE(x->valid_lft_ == 200);
- EXPECT_TRUE(x->t1_ == 50);
- EXPECT_TRUE(x->t2_ == 80);
+ EXPECT_EQ(x->type_, Lease6::LEASE_IA_NA);
+ EXPECT_EQ(x->preferred_lft_, 100);
+ EXPECT_EQ(x->valid_lft_, 200);
+ EXPECT_EQ(x->t1_, 50);
+ EXPECT_EQ(x->t2_, 80);
// should return false - there's no such address
EXPECT_FALSE(leaseMgr->deleteLease6(IOAddress("2001:db8:1::789")));
diff --git a/src/lib/dhcp/tests/memfile_lease_mgr.cc b/src/lib/dhcp/tests/memfile_lease_mgr.cc
index d0a8b99..ac3316d 100644
--- a/src/lib/dhcp/tests/memfile_lease_mgr.cc
+++ b/src/lib/dhcp/tests/memfile_lease_mgr.cc
@@ -24,11 +24,11 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const std::string& dbconfig)
Memfile_LeaseMgr::~Memfile_LeaseMgr() {
}
-bool Memfile_LeaseMgr::addLease(Lease4Ptr) {
+bool Memfile_LeaseMgr::addLease(const Lease4Ptr&) {
return (false);
}
-bool Memfile_LeaseMgr::addLease(Lease6Ptr lease) {
+bool Memfile_LeaseMgr::addLease(const Lease6Ptr& lease) {
if (getLease6(lease->addr_)) {
// there is a lease with specified address already
return (false);
@@ -84,10 +84,10 @@ Lease6Ptr Memfile_LeaseMgr::getLease6(const DUID&, uint32_t,
return (Lease6Ptr());
}
-void Memfile_LeaseMgr::updateLease4(Lease4Ptr ) {
+void Memfile_LeaseMgr::updateLease4(const Lease4Ptr& ) {
}
-void Memfile_LeaseMgr::updateLease6(Lease6Ptr ) {
+void Memfile_LeaseMgr::updateLease6(const Lease6Ptr& ) {
}
diff --git a/src/lib/dhcp/tests/memfile_lease_mgr.h b/src/lib/dhcp/tests/memfile_lease_mgr.h
index d39e3eb..666db6a 100644
--- a/src/lib/dhcp/tests/memfile_lease_mgr.h
+++ b/src/lib/dhcp/tests/memfile_lease_mgr.h
@@ -50,22 +50,26 @@ public:
/// @brief Adds an IPv4 lease.
///
+ /// @todo Not implemented yet
/// @param lease lease to be added
- virtual bool addLease(Lease4Ptr lease);
+ virtual bool addLease(const Lease4Ptr& lease);
/// @brief Adds an IPv6 lease.
///
/// @param lease lease to be added
- virtual bool addLease(Lease6Ptr lease);
+ virtual bool addLease(const Lease6Ptr& lease);
/// @brief Returns existing IPv4 lease for specified IPv4 address.
///
+ /// @todo Not implemented yet
/// @param addr address of the searched lease
///
/// @return a collection of leases
virtual Lease4Ptr getLease4(isc::asiolink::IOAddress addr) const;
/// @brief Returns existing IPv4 lease for specific address and subnet
+ ///
+ /// @todo Not implemented yet
/// @param addr address of the searched lease
/// @param subnet_id ID of the subnet the lease must belong to
///
@@ -75,6 +79,8 @@ public:
/// @brief Returns existing IPv4 leases for specified hardware address.
///
+ /// @todo Not implemented yet
+ ///
/// Although in the usual case there will be only one lease, for mobile
/// clients or clients with multiple static/fixed/reserved leases there
/// can be more than one. Thus return type is a container, not a single
@@ -88,6 +94,8 @@ public:
/// @brief Returns existing IPv4 leases for specified hardware address
/// and a subnet
///
+ /// @todo Not implemented yet
+ ///
/// There can be at most one lease for a given HW address in a single
/// pool, so this method with either return a single lease or NULL.
///
@@ -100,6 +108,8 @@ public:
/// @brief Returns existing IPv4 lease for specified client-id
///
+ /// @todo Not implemented yet
+ ///
/// @param clientid client identifier
virtual Lease4Collection getLease4(const ClientId& clientid) const;
@@ -108,6 +118,8 @@ public:
/// There can be at most one lease for a given HW address in a single
/// pool, so this method with either return a single lease or NULL.
///
+ /// @todo Not implemented yet
+ ///
/// @param clientid client identifier
/// @param subnet_id identifier of the subnet that lease must belong to
///
@@ -124,6 +136,8 @@ public:
/// @brief Returns existing IPv6 lease for a given DUID+IA combination
///
+ /// @todo Not implemented yet
+ ///
/// @param duid client DUID
/// @param iaid IA identifier
///
@@ -132,6 +146,8 @@ public:
/// @brief Returns existing IPv6 lease for a given DUID+IA combination
///
+ /// @todo Not implemented yet
+ ///
/// @param duid client DUID
/// @param iaid IA identifier
/// @param subnet_id identifier of the subnet the lease must belong to
@@ -141,20 +157,26 @@ public:
/// @brief Updates IPv4 lease.
///
+ /// @todo Not implemented yet
+ ///
/// @param lease4 The lease to be updated.
///
/// If no such lease is present, an exception will be thrown.
- void updateLease4(Lease4Ptr lease4);
+ void updateLease4(const Lease4Ptr& lease4);
/// @brief Updates IPv4 lease.
///
+ /// @todo Not implemented yet
+ ///
/// @param lease4 The lease to be updated.
///
/// If no such lease is present, an exception will be thrown.
- void updateLease6(Lease6Ptr lease6);
+ void updateLease6(const Lease6Ptr& lease6);
/// @brief Deletes a lease.
///
+ /// @todo Not implemented yet
+ ///
/// @param addr IPv4 address of the lease to be deleted.
///
/// @return true if deletion was successful, false if no such lease exists
More information about the bind10-changes
mailing list