diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index e2bd4d0b5e..4184b16d9b 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -3504,7 +3504,7 @@ const std::string HARDCODED_SERVER_ID = "192.0.2.1";
RFC2131: Supported messages are DISCOVER, OFFER,
- REQUEST, and ACK.
+ REQUEST, ACK, NACK, RELEASE.
RFC2132: Supported options are: PAD (0),
@@ -3512,6 +3512,10 @@ const std::string HARDCODED_SERVER_ID = "192.0.2.1";
Domain Name (15), DNS Servers (6), IP Address Lease Time
(51), Subnet mask (1), and Routers (3).
+
+ RFC6842: Server responses include client-id option
+ if client sent it in its message.
+
diff --git a/src/bin/dhcp4/tests/dhcp4_unittests.cc b/src/bin/dhcp4/tests/dhcp4_unittests.cc
index 2cd915b5fa..93e836b166 100644
--- a/src/bin/dhcp4/tests/dhcp4_unittests.cc
+++ b/src/bin/dhcp4/tests/dhcp4_unittests.cc
@@ -21,16 +21,10 @@ main(int argc, char* argv[]) {
::testing::InitGoogleTest(&argc, argv);
+ // See the documentation of the B10_* environment variables in
+ // src/lib/log/README for info on how to tweak logging
isc::log::initLogger();
- // Uncomment those to get much more verbose tests
- /*
- isc::log::initLogger("b10-dhcp4",
- isc::log::DEBUG,
- isc::log::MAX_DEBUG_LEVEL, NULL, false);
- isc::dhcp::dhcp4_logger.setSeverity(isc::log::DEBUG, 99);
- */
-
int result = RUN_ALL_TESTS();
return (result);
diff --git a/src/lib/dhcp/hwaddr.h b/src/lib/dhcp/hwaddr.h
index c8be2787b0..77cd6bd4ef 100644
--- a/src/lib/dhcp/hwaddr.h
+++ b/src/lib/dhcp/hwaddr.h
@@ -23,10 +23,22 @@
namespace isc {
namespace dhcp {
+/// @brief Hardware type that represents information from DHCPv4 packet
struct HWAddr {
public:
+
+ /// @brief default constructor
HWAddr();
+
+ /// @brief constructor, based on C-style pointer and length
+ /// @param hwaddr pointer to hardware address
+ /// @param len length of the address pointed by hwaddr
+ /// @param htype hardware type
HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype);
+
+ /// @brief constructor, based on C++ vector
+ /// @param hwaddr const reference to hardware address
+ /// @param htype hardware type
HWAddr(const std::vector& hwaddr, uint8_t htype);
// Vector that keeps the actual hardware address
@@ -48,7 +60,6 @@ public:
/// @brief Shared pointer to a hardware address structure
typedef boost::shared_ptr HWAddrPtr;
-
}; // end of isc::dhcp namespace
}; // end of isc namespace
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 3c97d619eb..d3b22deea4 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -112,7 +112,7 @@ Pkt4::pack() {
bufferOut_.writeUint8(op_);
bufferOut_.writeUint8(hwaddr_->htype_);
- bufferOut_.writeUint8(hw_len < 16 ? hw_len : 16);
+ bufferOut_.writeUint8(hw_len < MAX_CHADDR_LEN ? hw_len : MAX_CHADDR_LEN);
bufferOut_.writeUint8(hops_);
bufferOut_.writeUint32(transid_);
bufferOut_.writeUint16(secs_);
@@ -123,13 +123,14 @@ Pkt4::pack() {
bufferOut_.writeUint32(giaddr_);
- if (hw_len <=16) {
+ if (hw_len <= MAX_CHADDR_LEN) {
// write up to 16 bytes of the hardware address (CHADDR field is 16
// bytes long in DHCPv4 message).
- bufferOut_.writeData(&hwaddr_->hwaddr_[0], (hw_len<16?hw_len:16) );
- hw_len = 16 - hw_len;
+ bufferOut_.writeData(&hwaddr_->hwaddr_[0],
+ (hw_len < MAX_CHADDR_LEN ? hw_len : MAX_CHADDR_LEN) );
+ hw_len = MAX_CHADDR_LEN - hw_len;
} else {
- hw_len = 16;
+ hw_len = MAX_CHADDR_LEN;
}
// write (len) bytes of padding
@@ -288,7 +289,7 @@ Pkt4::setHWAddr(uint8_t hType, uint8_t hlen,
isc_throw(OutOfRange, "Invalid HW Address specified");
}
- hwaddr_ = HWAddrPtr(new HWAddr(mac_addr, hType));
+ hwaddr_.reset(new HWAddr(mac_addr, hType));
}
void
@@ -370,7 +371,7 @@ Pkt4::getHlen() const {
isc_throw(InvalidOperation, "Can't get HType. HWAddr not defined");
}
uint8_t len = hwaddr_->hwaddr_.size();
- return (len <= 16 ? len : 16);
+ return (len <= MAX_CHADDR_LEN ? len : MAX_CHADDR_LEN);
}
void
@@ -395,7 +396,7 @@ Pkt4::getOption(uint8_t type) const {
bool
Pkt4::delOption(uint8_t type) {
isc::dhcp::Option::OptionCollection::iterator x = options_.find(type);
- if (x!=options_.end()) {
+ if (x != options_.end()) {
options_.erase(x);
return (true); // delete successful
}
diff --git a/src/lib/dhcp/tests/hwaddr_unittest.cc b/src/lib/dhcp/tests/hwaddr_unittest.cc
index 1173b996c7..b4ee55eea7 100644
--- a/src/lib/dhcp/tests/hwaddr_unittest.cc
+++ b/src/lib/dhcp/tests/hwaddr_unittest.cc
@@ -90,8 +90,9 @@ TEST(HWAddrTest, operators) {
EXPECT_TRUE(*hw4 != *hw5);
}
+// Checks that toText() method produces appropriate text representation
TEST(HWAddrTest, toText) {
- uint8_t data[] = {0, 1, 2, 3, 4, 5}; // last digit different
+ uint8_t data[] = {0, 1, 2, 3, 4, 5};
uint8_t htype = 15;
HWAddrPtr hw(new HWAddr(data, sizeof(data), htype));
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index a3dfa683ed..49588e1de8 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -222,6 +222,7 @@ TEST(Pkt4Test, fixedFields) {
// Chaddr contains link-layer addr (MAC). It is no longer always 16 bytes
// long and its length depends on hlen value (it is up to 16 bytes now).
+ ASSERT_EQ(pkt->getHWAddr()->hwaddr_.size(), dummyHlen);
EXPECT_EQ(0, memcmp(dummyChaddr, &pkt->getHWAddr()->hwaddr_[0], dummyHlen));
EXPECT_EQ(0, memcmp(dummySname, &pkt->getSname()[0], 64));
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index f039fa0686..7a64dac889 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -267,36 +267,32 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
const IOAddress& hint,
bool fake_allocation /* = false */ ) {
- // That check is not necessary. We create allocator in AllocEngine
- // constructor
+ // Allocator is always created in AllocEngine constructor and there is
+ // currently no other way to set it, so that check is not really necessary.
if (!allocator_) {
isc_throw(InvalidOperation, "No allocator selected");
}
- // check if there's existing lease for that subnet/clientid/hwaddr combination.
+ // Check if there's existing lease for that subnet/clientid/hwaddr combination.
Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(hwaddr->hwaddr_, subnet->getID());
if (existing) {
- // we have a lease already. This is a returning client, probably after
- // his reboot.
-
+ // We have a lease already. This is a returning client, probably after
+ // its reboot.
existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
-
if (existing) {
return (existing);
}
// If renewal failed (e.g. the lease no longer matches current configuration)
- // let's continue allocation process
+ // let's continue the allocation process
}
if (clientid) {
existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
if (existing) {
// we have a lease already. This is a returning client, probably after
- // his reboot.
-
+ // its reboot.
existing = renewLease4(subnet, clientid, hwaddr, existing, fake_allocation);
-
// @todo: produce a warning. We haven't found him using MAC address, but
// we found him using client-id
if (existing) {
@@ -309,10 +305,10 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
if (subnet->inPool(hint)) {
existing = LeaseMgrFactory::instance().getLease4(hint);
if (!existing) {
- /// @todo: check if the hint is reserved once we have host support
+ /// @todo: Check if the hint is reserved once we have host support
/// implemented
- // the hint is valid and not currently used, let's create a lease for it
+ // The hint is valid and not currently used, let's create a lease for it
Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint, fake_allocation);
// It can happen that the lease allocation failed (we could have lost
@@ -334,7 +330,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
// the following occurs:
// - we find a free address
// - we find an address for which the lease has expired
- // - we exhaust number of tries
+ // - we exhaust the number of tries
//
// @todo: Current code does not handle pool exhaustion well. It will be
// improved. Current problems:
@@ -373,7 +369,7 @@ AllocEngine::allocateAddress4(const SubnetPtr& subnet,
}
}
- // continue trying allocation until we run out of attempts
+ // Continue trying allocation until we run out of attempts
// (or attempts are set to 0, which means infinite)
--i;
} while ( i || !attempts_);
@@ -545,7 +541,6 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
if (!fake_allocation) {
// That is a real (REQUEST) allocation
bool status = LeaseMgrFactory::instance().addLease(lease);
-
if (status) {
return (lease);
} else {
@@ -569,7 +564,6 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
}
}
-
AllocEngine::~AllocEngine() {
// no need to delete allocator. smart_ptr will do the trick for us
}
diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h
index 76e9acd33d..c6cbc35685 100644
--- a/src/lib/dhcpsrv/alloc_engine.h
+++ b/src/lib/dhcpsrv/alloc_engine.h
@@ -66,6 +66,12 @@ protected:
/// reserved - AllocEngine will check that and will call pickAddress
/// again if necessary. The number of times this method is called will
/// increase as the number of available leases will decrease.
+ ///
+ /// @param subnet next address will be returned from pool of that subnet
+ /// @param duid Client's DUID
+ /// @param hint client's hint
+ ///
+ /// @return the next address
virtual isc::asiolink::IOAddress
pickAddress(const SubnetPtr& subnet, const DuidPtr& duid,
const isc::asiolink::IOAddress& hint) = 0;
@@ -197,12 +203,12 @@ protected:
/// @brief Renews a IPv4 lease
///
- /// Since both request and renew are implemented in DHCPv4 as sending
- /// REQUEST packet, it is difficult to easily distinguish between those
- /// cases. Therefore renew for DHCPv4 is done in allocation engine.
+ /// Since both request and renew are implemented in DHCPv4 as the sending of
+ /// a REQUEST packet, it is difficult to easily distinguish between those
+ /// cases. Therefore renew for DHCPv4 is done in the allocation engine.
/// This method is also used when client crashed/rebooted and tries
/// to get a new lease. It thinks that it gets a new lease, but in fact
- /// we are only renewing still valid lease for that client.
+ /// we are only renewing the still valid lease for that client.
///
/// @param subnet subnet the client is attached to
/// @param clientid client identifier
@@ -241,10 +247,10 @@ protected:
virtual ~AllocEngine();
private:
- /// @brief creates a lease and inserts it in LeaseMgr if necessary
+ /// @brief Creates a lease and inserts it in LeaseMgr if necessary
///
/// Creates a lease based on specified parameters and tries to insert it
- /// into the database. That may fail in some cases, i.e. when there is another
+ /// into the database. That may fail in some cases, e.g. when there is another
/// allocation process and we lost a race to a specific lease.
///
/// @param subnet subnet the lease is allocated from
@@ -278,7 +284,7 @@ private:
uint32_t iaid, const isc::asiolink::IOAddress& addr,
bool fake_allocation = false);
- /// @brief reuses expired IPv4 lease
+ /// @brief Reuses expired IPv4 lease
///
/// Updates existing expired lease with new information. Lease database
/// is updated if this is real (i.e. REQUEST, fake_allocation = false), not
@@ -297,7 +303,7 @@ private:
const HWAddrPtr& hwaddr,
bool fake_allocation = false);
- /// @brief reuses expired IPv6 lease
+ /// @brief Reuses expired IPv6 lease
///
/// Updates existing expired lease with new information. Lease database
/// is updated if this is real (i.e. REQUEST, fake_allocation = false), not
diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc
index 8109745128..07d4d689f6 100644
--- a/src/lib/dhcpsrv/subnet.cc
+++ b/src/lib/dhcpsrv/subnet.cc
@@ -93,7 +93,7 @@ PoolPtr Subnet::getPool(isc::asiolink::IOAddress hint) {
// getPool() as pure virtual and have Subnet4 and Subnet6 provide their
// own methods. Those two implementation would only differ by a default
// value, so it would just include duplicate code.
- if (dynamic_cast(this) && hint.toText() == "::") {
+ if (dynamic_cast(this) && hint.toText() == "::") {
hint = IOAddress("0.0.0.0");
}
diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
index 8030abc862..078998d940 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
@@ -31,7 +31,7 @@
#include
#include
-#include