BIND 10 trac2320, updated. 21794c5df650ecf28f55ed95cb12d278945ab3a5 [2320] Changes after review.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jan 8 17:11:07 UTC 2013
The branch, trac2320 has been updated
via 21794c5df650ecf28f55ed95cb12d278945ab3a5 (commit)
via d5310a9cec9a3ece6b0cb323ea2a933caca127b9 (commit)
via 564f603d2056a823b3436acf0183a9abeec35ecb (commit)
from d509593229c0424fc430766f56d5e5947cb3dda8 (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 21794c5df650ecf28f55ed95cb12d278945ab3a5
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Jan 8 18:10:50 2013 +0100
[2320] Changes after review.
commit d5310a9cec9a3ece6b0cb323ea2a933caca127b9
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Jan 8 13:53:34 2013 +0100
[2320] Changes after review, part 1
commit 564f603d2056a823b3436acf0183a9abeec35ecb
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Jan 8 12:07:07 2013 +0100
[2320] Pkt4::setType() is now implemented properly.
-----------------------------------------------------------------------
Summary of changes:
doc/guide/bind10-guide.xml | 6 +-
src/bin/dhcp4/dhcp4_messages.mes | 9 +-
src/bin/dhcp4/dhcp4_srv.cc | 138 ++++++------
src/bin/dhcp4/dhcp4_srv.h | 9 +-
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 282 +++++++++++++-----------
src/bin/dhcp4/tests/dhcp4_unittests.cc | 10 +-
src/lib/dhcp/hwaddr.h | 13 +-
src/lib/dhcp/pkt4.cc | 68 ++++--
src/lib/dhcp/pkt4.h | 16 +-
src/lib/dhcp/tests/hwaddr_unittest.cc | 3 +-
src/lib/dhcp/tests/iface_mgr_unittest.cc | 9 +-
src/lib/dhcp/tests/pkt4_unittest.cc | 20 +-
src/lib/dhcpsrv/alloc_engine.cc | 28 +--
src/lib/dhcpsrv/alloc_engine.h | 22 +-
src/lib/dhcpsrv/subnet.cc | 2 +-
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc | 107 +++++----
16 files changed, 417 insertions(+), 325 deletions(-)
-----------------------------------------------------------------------
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index e2bd4d0..4184b16 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";</screen>
<itemizedlist>
<listitem>
<simpara>RFC2131: Supported messages are DISCOVER, OFFER,
- REQUEST, and ACK.</simpara>
+ REQUEST, ACK, NACK, RELEASE.</simpara>
</listitem>
<listitem>
<simpara>RFC2132: Supported options are: PAD (0),
@@ -3512,6 +3512,10 @@ const std::string HARDCODED_SERVER_ID = "192.0.2.1";</screen>
Domain Name (15), DNS Servers (6), IP Address Lease Time
(51), Subnet mask (1), and Routers (3).</simpara>
</listitem>
+ <listitem>
+ <simpara>RFC6842: Server responses include client-id option
+ if client sent it in its message.</simpara>
+ </listitem>
</itemizedlist>
</section>
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index dedbdb0..0ee5139 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -79,7 +79,8 @@ incicates successful operation.
% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2
This message indicates that the server failed to grant a lease to the
specified client after receiving a REQUEST message from it. There are many
-possible reasons for such a failure
+possible reasons for such a failure. Additional messages will indicate the
+reason.
% DHCP4_NOT_RUNNING IPv4 DHCP server is not running
A warning message is issued when an attempt is made to shut down the
@@ -121,6 +122,12 @@ A debug message listing the data received from the client.
This debug message indicates that an address was released properly. It
is a normal operation during client shutdown.
+% DHCP4_RELEASE_EXCEPTION exception %1 while trying to release address %2
+This error message indicates that an exception was raised during an attempt
+to process RELEASE message. This does not affect the client as it does not expect
+any response for RELEASE message. Depending on the nature of exception it may
+affect future server operation.
+
% DHCP4_RELEASE_FAIL failed to remove lease for address %1 for duid %2, hwaddr %3
This error message indicates that the software failed to remove a
lease from the lease database. It is probably due to an error during a
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 7a5d11e..88ccfea 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -213,6 +213,7 @@ void Dhcpv4Srv::copyDefaultFields(const Pkt4Ptr& question, Pkt4Ptr& answer) {
answer->setRemoteAddr(question->getRemoteAddr());
}
+ // Let's copy client-id to response. See RFC6842.
OptionPtr client_id = question->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
if (client_id) {
answer->addOption(client_id);
@@ -223,10 +224,7 @@ void Dhcpv4Srv::appendDefaultOptions(Pkt4Ptr& msg, uint8_t msg_type) {
OptionPtr opt;
// add Message Type Option (type 53)
- std::vector<uint8_t> tmp;
- tmp.push_back(static_cast<uint8_t>(msg_type));
- opt = OptionPtr(new Option(Option::V4, DHO_DHCP_MESSAGE_TYPE, tmp));
- msg->addOption(opt);
+ msg->setType(msg_type);
// DHCP Server Identifier (type 54)
msg->addOption(getServerID());
@@ -267,14 +265,14 @@ void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
LOG_ERROR(dhcp4_logger, DHCP4_SUBNET_SELECTION_FAILED)
.arg(question->getRemoteAddr().toText())
.arg(serverReceivedPacketName(question->getType()));
- setMsgType(answer, DHCPNAK);
+ answer->setType(DHCPNAK);
answer->setYiaddr(IOAddress("0.0.0.0"));
return;
- } else {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED)
- .arg(subnet->toText());
}
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED)
+ .arg(subnet->toText());
+
// Get client-id option
ClientIdPtr client_id;
OptionPtr opt = question->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
@@ -293,10 +291,7 @@ void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
// the user selects this server to do actual allocation (i.e. sends REQUEST)
// it should include this hint. That will help us during the actual lease
// allocation.
- bool fake_allocation = false;
- if (question->getType() == DHCPDISCOVER) {
- fake_allocation = true;
- }
+ bool fake_allocation = (question->getType() == DHCPDISCOVER);
// Use allocation engine to pick a lease for this client. Allocation engine
// will try to honour the hint, but it is just a hint - some other address
@@ -306,8 +301,8 @@ void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
hint, fake_allocation);
if (lease) {
- // We have a lease! Let's wrap its content into IA_NA option
- // with IAADDR suboption.
+ // We have a lease! Let's set it in the packet and send it back to
+ // the client.
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, fake_allocation?
DHCP4_LEASE_ADVERT:DHCP4_LEASE_ALLOC)
.arg(lease->addr_.toText())
@@ -343,7 +338,7 @@ void Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
.arg(hwaddr?hwaddr->toText():"(no hwaddr info)")
.arg(hint.toText());
- setMsgType(answer, DHCPNAK);
+ answer->setType(DHCPNAK);
answer->setYiaddr(IOAddress("0.0.0.0"));
}
}
@@ -357,19 +352,6 @@ OptionPtr Dhcpv4Srv::getNetmaskOption(const Subnet4Ptr& subnet) {
return (opt);
}
-void Dhcpv4Srv::setMsgType(Pkt4Ptr& pkt, uint8_t dhcp_type) {
- OptionPtr opt = pkt->getOption(DHO_DHCP_MESSAGE_TYPE);
- if (opt) {
- // There is message type option already, update it
- opt->setUint8(dhcp_type);
- } else {
- // There is no message type option yet, add it
- std::vector<uint8_t> tmp(1, dhcp_type);
- opt = OptionPtr(new Option(Option::V4, DHO_DHCP_MESSAGE_TYPE, tmp));
- pkt->addOption(opt);
- }
-}
-
Pkt4Ptr Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
Pkt4Ptr offer = Pkt4Ptr
(new Pkt4(DHCPOFFER, discover->getTransid()));
@@ -397,50 +379,71 @@ Pkt4Ptr Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
}
void Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
+
+ // Try to find client-id
ClientIdPtr client_id;
OptionPtr opt = release->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
if (opt) {
client_id = ClientIdPtr(new ClientId(opt->getData()));
}
- Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(release->getYiaddr());
+ try {
+ // Do we have a lease for that particular address?
+ Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(release->getYiaddr());
+
+ if (!lease) {
+ // No such lease - bogus release
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_NO_LEASE)
+ .arg(release->getYiaddr().toText())
+ .arg(release->getHWAddr()->toText())
+ .arg(client_id ? client_id->toText() : "(no client-id)");
+ return;
+ }
- if (!lease) {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_NO_LEASE)
- .arg(release->getYiaddr().toText())
- .arg(release->getHWAddr()->toText())
- .arg(client_id ? client_id->toText() : "(no client-id)");
- return;
- }
+ // Does the hardware address match? We don't want one client releasing
+ // second client's leases.
+ if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) {
+ // @todo: Print hwaddr from lease as part of ticket #2589
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
+ .arg(release->getYiaddr().toText())
+ .arg(client_id ? client_id->toText() : "(no client-id)")
+ .arg(release->getHWAddr()->toText());
+ return;
+ }
- if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) {
- // @todo: Print hwaddr from lease as part of ticket #2589
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
- .arg(release->getYiaddr().toText())
- .arg(client_id ? client_id->toText() : "(no client-id)")
- .arg(release->getHWAddr()->toText());
- return;
- }
+ // Does the lease have client-id info? If it has, then check it with what
+ // the client sent us.
+ if (lease->client_id_ && client_id && *lease->client_id_ != *client_id) {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID)
+ .arg(release->getYiaddr().toText())
+ .arg(client_id->toText())
+ .arg(lease->client_id_->toText());
+ return;
+ }
- if (lease->client_id_ && client_id && *lease->client_id_ != *client_id) {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_CLIENT_ID)
- .arg(release->getYiaddr().toText())
- .arg(client_id->toText())
- .arg(lease->client_id_->toText());
- return;
+ // Ok, hw and client-id match - let's release the lease.
+ if (LeaseMgrFactory::instance().deleteLease(lease->addr_)) {
+
+ // Release successful - we're done here
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE)
+ .arg(lease->addr_.toText())
+ .arg(client_id ? client_id->toText() : "(no client-id)")
+ .arg(release->getHWAddr()->toText());
+ } else {
+
+ // Release failed -
+ LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
+ .arg(lease->addr_.toText())
+ .arg(client_id ? client_id->toText() : "(no client-id)")
+ .arg(release->getHWAddr()->toText());
+ }
+ } catch (const isc::Exception& ex) {
+ // Rethrow the exception with a bit more data.
+ LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_EXCEPTION)
+ .arg(ex.what())
+ .arg(release->getYiaddr());
}
- if (LeaseMgrFactory::instance().deleteLease(lease->addr_)) {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE)
- .arg(lease->addr_.toText())
- .arg(client_id ? client_id->toText() : "(no client-id)")
- .arg(release->getHWAddr()->toText());
- } else {
- LOG_ERROR(dhcp4_logger, DHCP4_RELEASE_FAIL)
- .arg(lease->addr_.toText())
- .arg(client_id ? client_id->toText() : "(no client-id)")
- .arg(release->getHWAddr()->toText());
- }
}
void Dhcpv4Srv::processDecline(Pkt4Ptr& decline) {
@@ -484,9 +487,18 @@ Dhcpv4Srv::serverReceivedPacketName(uint8_t type) {
}
Subnet4Ptr Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) {
- Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(question->getRemoteAddr());
- return (subnet);
+ // Is this relayed message?
+ IOAddress relay = question->getGiaddr();
+ if (relay.toText() == "0.0.0.0") {
+
+ // Yes: Use relay address to select subnet
+ return (CfgMgr::instance().getSubnet4(relay));
+ } else {
+
+ // No: Use client's address to select subnet
+ return (CfgMgr::instance().getSubnet4(question->getRemoteAddr()));
+ }
}
void Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) {
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 9d33441..53401c5 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -207,14 +207,6 @@ protected:
/// @return server-id option
OptionPtr getServerID() { return serverid_; }
-
- /// @brief Sets DHCPv4 message type
- ///
- /// It tries to find existing DHCP TYPE (53) option and update
- /// it to specified value. If there is no such option, it is
- /// added.
- static void setMsgType(Pkt4Ptr& pkt, uint8_t dhcp_type);
-
/// @brief Sets server-identifier.
///
/// This method attempts to set server-identifier DUID. It tries to
@@ -242,6 +234,7 @@ protected:
private:
/// @brief Constructs netmask option based on subnet4
+ /// @param subnet subnet for which the netmask will be calculated
///
/// @return Option that contains netmask information
static OptionPtr getNetmaskOption(const Subnet4Ptr& subnet);
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 810cbdc..144616e 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -39,7 +39,7 @@ using namespace isc::asiolink;
namespace {
class NakedDhcpv4Srv: public Dhcpv4Srv {
- // "naked" DHCPv4 server, exposes internal fields
+ // "Naked" DHCPv4 server, exposes internal fields
public:
NakedDhcpv4Srv(uint16_t port = 0):Dhcpv4Srv(port) { }
@@ -54,6 +54,11 @@ public:
class Dhcpv4SrvTest : public ::testing::Test {
public:
+
+ /// @brief Constructor
+ ///
+ /// Initializes common objects used in many tests.
+ /// Also sets up initial configuration in CfgMgr.
Dhcpv4SrvTest() {
subnet_ = Subnet4Ptr(new Subnet4(IOAddress("192.0.2.0"), 24, 1000,
2000, 3000));
@@ -64,6 +69,9 @@ public:
CfgMgr::instance().addSubnet4(subnet_);
}
+ /// @brief checks that the response matches request
+ /// @param q query (client's message)
+ /// @param a answer (server's message)
void MessageCheck(const boost::shared_ptr<Pkt4>& q,
const boost::shared_ptr<Pkt4>& a) {
ASSERT_TRUE(q);
@@ -74,7 +82,7 @@ public:
EXPECT_EQ(q->getIndex(), a->getIndex());
EXPECT_EQ(q->getGiaddr(), a->getGiaddr());
- // check that bare minimum of required options are there
+ // Check that bare minimum of required options are there
EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
EXPECT_TRUE(a->getOption(DHO_ROUTERS));
EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
@@ -84,15 +92,18 @@ public:
EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
- // check that something is offered
+ // Check that something is offered
EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
}
- // Generate client-id option of specified length
- // Ids with different lengths are sufficent to generate
- // unique ids. If more fine grained control is required,
- // tests generate client-ids on their own.
- // Sets client_id_ field.
+ /// @brief generates client-id option
+ ///
+ /// Generate client-id option of specified length
+ /// Ids with different lengths are sufficent to generate
+ /// unique ids. If more fine grained control is required,
+ /// tests generate client-ids on their own.
+ /// Sets client_id_ field.
+ /// @param size size of the client-id to be generated
OptionPtr generateClientId(size_t size = 4) {
OptionBuffer clnt_id(size);
@@ -107,8 +118,12 @@ public:
clnt_id.begin() + size)));
}
+ /// @brief generate hardware address
+ ///
+ /// @param size size of the generated MAC address
+ /// @param pointer to Hardware Address object
HWAddrPtr generateHWAddr(size_t size = 6) {
- const uint8_t hw_type = 123; // just a fake number (typically 6=HTYPE_ETHER, see dhcp4.h)
+ const uint8_t hw_type = 123; // Just a fake number (typically 6=HTYPE_ETHER, see dhcp4.h)
OptionBuffer mac(size);
for (int i = 0; i < size; ++i) {
mac[i] = 50 + i;
@@ -116,8 +131,12 @@ public:
return (HWAddrPtr(new HWAddr(mac, hw_type)));
}
- // Check that address was returned from proper range, that its lease
- // lifetime is correct, that T1 and T2 are returned properly
+ /// Check that address was returned from proper range, that its lease
+ /// lifetime is correct, that T1 and T2 are returned properly
+ /// @param rsp response to be checked
+ /// @param subnet subnet that should be used to verify assigned address and options
+ /// @param t1_mandatory is T1 mandatory?
+ /// @param t2_mandatory is T2 mandatory?
void checkAddressParams(const Pkt4Ptr& rsp, const SubnetPtr subnet,
bool t1_mandatory = false, bool t2_mandatory = false) {
@@ -127,17 +146,17 @@ public:
EXPECT_TRUE(subnet->inPool(rsp->getYiaddr()));
// Check lease time
- OptionPtr tmp = rsp->getOption(DHO_DHCP_LEASE_TIME);
- if (!tmp) {
+ OptionPtr opt = rsp->getOption(DHO_DHCP_LEASE_TIME);
+ if (!opt) {
ADD_FAILURE() << "Lease time option missing in response";
} else {
- EXPECT_EQ(tmp->getUint32(), subnet->getValid());
+ EXPECT_EQ(opt->getUint32(), subnet->getValid());
}
// Check T1 timer
- tmp = rsp->getOption(DHO_DHCP_RENEWAL_TIME);
- if (tmp) {
- EXPECT_EQ(tmp->getUint32(), subnet->getT1());
+ opt = rsp->getOption(DHO_DHCP_RENEWAL_TIME);
+ if (opt) {
+ EXPECT_EQ(opt->getUint32(), subnet->getT1());
} else {
if (t1_mandatory) {
ADD_FAILURE() << "Required T1 option missing";
@@ -145,9 +164,9 @@ public:
}
// Check T2 timer
- tmp = rsp->getOption(DHO_DHCP_REBINDING_TIME);
- if (tmp) {
- EXPECT_EQ(tmp->getUint32(), subnet->getT2());
+ opt = rsp->getOption(DHO_DHCP_REBINDING_TIME);
+ if (opt) {
+ EXPECT_EQ(opt->getUint32(), subnet->getT2());
} else {
if (t1_mandatory) {
ADD_FAILURE() << "Required T2 option missing";
@@ -155,7 +174,11 @@ public:
}
}
- // Basic checks for generated response (message type and transaction-id).
+ /// @brief Basic checks for generated response (message type and trans-id).
+ ///
+ /// @param rsp response packet to be validated
+ /// @param expected_message_type expected message type
+ /// @param expected_transid expected transaction-id
void checkResponse(const Pkt4Ptr& rsp, uint8_t expected_message_type,
uint32_t expected_transid) {
ASSERT_TRUE(rsp);
@@ -163,9 +186,14 @@ public:
EXPECT_EQ(expected_transid, rsp->getTransid());
}
- // Checks if the lease sent to client is present in the database
+ /// @brief Checks if the lease sent to client is present in the database
+ ///
+ /// @param rsp response packet to be validated
+ /// @param client_id expected client-identifier (or NULL)
+ /// @param HWAddr expected hardware address (not used now)
+ /// @param expected_addr expected address
Lease4Ptr checkLease(const Pkt4Ptr& rsp, const OptionPtr& client_id,
- const HWAddrPtr& hwaddr, const IOAddress& expected_addr) {
+ const HWAddrPtr&, const IOAddress& expected_addr) {
ClientIdPtr id;
if (client_id) {
@@ -180,6 +208,8 @@ public:
return (Lease4Ptr());
}
+ EXPECT_EQ(rsp->getYiaddr().toText(), expected_addr.toText());
+
EXPECT_EQ(expected_addr.toText(), lease->addr_.toText());
if (client_id) {
EXPECT_TRUE(*lease->client_id_ == *id);
@@ -189,52 +219,56 @@ public:
return (lease);
}
- // Checks if server response (OFFER, ACK, NAK) includes proper server-id.
+ /// @brief Checks if server response (OFFER, ACK, NAK) includes proper server-id
+ /// @param rsp response packet to be validated
+ /// @param expected_srvid expected value of server-id
void checkServerId(const Pkt4Ptr& rsp, const OptionPtr& expected_srvid) {
- // check that server included its server-id
- OptionPtr tmp = rsp->getOption(DHO_DHCP_SERVER_IDENTIFIER);
- ASSERT_TRUE(tmp);
- EXPECT_EQ(tmp->getType(), expected_srvid->getType() );
- EXPECT_EQ(tmp->len(), expected_srvid->len() );
- EXPECT_TRUE(tmp->getData() == expected_srvid->getData());
+ // Check that server included its server-id
+ OptionPtr opt = rsp->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+ ASSERT_TRUE(opt);
+ EXPECT_EQ(opt->getType(), expected_srvid->getType() );
+ EXPECT_EQ(opt->len(), expected_srvid->len() );
+ EXPECT_TRUE(opt->getData() == expected_srvid->getData());
}
- // Checks if server response (OFFER, ACK, NAK) includes proper client-id.
+ /// @brief Checks if server response (OFFER, ACK, NAK) includes proper client-id
+ /// @param rsp response packet to be validated
+ /// @param expected_clientid expected value of client-id
void checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_clientid) {
// check that server included our own client-id
- OptionPtr tmp = rsp->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
- ASSERT_TRUE(tmp);
- EXPECT_EQ(expected_clientid->getType(), tmp->getType());
- EXPECT_EQ(expected_clientid->len(), tmp->len());
- EXPECT_TRUE(expected_clientid->getData() == tmp->getData());
+ OptionPtr opt = rsp->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+ ASSERT_TRUE(opt);
+ EXPECT_EQ(expected_clientid->getType(), opt->getType());
+ EXPECT_EQ(expected_clientid->len(), opt->len());
+ EXPECT_TRUE(expected_clientid->getData() == opt->getData());
}
~Dhcpv4SrvTest() {
CfgMgr::instance().deleteSubnets4();
};
- // A subnet used in most tests
+ /// @brief A subnet used in most tests
Subnet4Ptr subnet_;
- // A pool used in most tests
+ /// @brief A pool used in most tests
Pool4Ptr pool_;
- // A client-id used in most tests
+ /// @brief A client-id used in most tests
ClientIdPtr client_id_;
};
// Sanity check. Verifies that both Dhcpv4Srv and its derived
// class NakedDhcpv4Srv can be instantiated and destroyed.
TEST_F(Dhcpv4SrvTest, basic) {
- // nothing to test. DHCPv4_srv instance is created
- // in test fixture. It is destroyed in destructor
+ // Check that the base class can be instantiated
Dhcpv4Srv* srv = NULL;
ASSERT_NO_THROW({
srv = new Dhcpv4Srv(DHCP4_SERVER_PORT + 10000);
});
delete srv;
+ // Check that the derived class can be instantiated
NakedDhcpv4Srv* naked_srv = NULL;
ASSERT_NO_THROW({
naked_srv = new NakedDhcpv4Srv(DHCP4_SERVER_PORT + 10000);
@@ -338,22 +372,22 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
req->setRemoteAddr(IOAddress("192.0.2.56"));
req->setGiaddr(IOAddress("192.0.2.67"));
- // should not throw
+ // Should not throw
ASSERT_NO_THROW(
ack = srv->processRequest(req);
);
- // should return something
+ // Should return something
ASSERT_TRUE(ack);
EXPECT_EQ(DHCPACK, ack->getType());
- // this is relayed message. It should be sent back to relay address.
+ // This is relayed message. It should be sent back to relay address.
EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
MessageCheck(req, ack);
- // now repeat the test for directly sent message
+ // Now repeat the test for directly sent message
req->setHops(0);
req->setGiaddr(IOAddress("0.0.0.0"));
req->setRemotePort(DHCP4_CLIENT_PORT);
@@ -362,12 +396,12 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
ack = srv->processDiscover(req);
);
- // should return something
+ // Should return something
ASSERT_TRUE(ack);
EXPECT_EQ(DHCPOFFER, ack->getType());
- // this is direct message. It should be sent back to origin, not
+ // This is direct message. It should be sent back to origin, not
// to relay.
EXPECT_EQ(ack->getRemoteAddr(), req->getRemoteAddr());
@@ -381,14 +415,11 @@ TEST_F(Dhcpv4SrvTest, processRelease) {
boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPRELEASE, 1234));
- // should not throw
+ // Should not throw
EXPECT_NO_THROW(
srv->processRelease(pkt);
);
- // TODO: Implement more reasonable tests before starting
- // work on processSomething() method.
-
delete srv;
}
@@ -397,13 +428,11 @@ TEST_F(Dhcpv4SrvTest, processDecline) {
boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPDECLINE, 1234));
- // should not throw
+ // Should not throw
EXPECT_NO_THROW(
srv->processDecline(pkt);
);
- // TODO: Implement more reasonable tests before starting
- // work on processSomething() method.
delete srv;
}
@@ -412,15 +441,15 @@ TEST_F(Dhcpv4SrvTest, processInform) {
boost::shared_ptr<Pkt4> pkt(new Pkt4(DHCPINFORM, 1234));
- // should not throw
+ // Should not throw
EXPECT_NO_THROW(
srv->processInform(pkt);
);
- // should return something
+ // Should return something
EXPECT_TRUE(srv->processInform(pkt));
- // TODO: Implement more reasonable tests before starting
+ // @todo Implement more reasonable tests before starting
// work on processSomething() method.
delete srv;
@@ -471,24 +500,24 @@ TEST_F(Dhcpv4SrvTest, serverReceivedPacketName) {
// - offered address
TEST_F(Dhcpv4SrvTest, DiscoverBasic) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
dis->setRemoteAddr(IOAddress("192.0.2.1"));
OptionPtr clientid = generateClientId();
dis->addOption(clientid);
- // Pass it to the server and get an advertise
+ // Pass it to the server and get an offer
Pkt4Ptr offer = srv->processDiscover(dis);
- // check if we get response at all
+ // Check if we get response at all
checkResponse(offer, DHCPOFFER, 1234);
- // check that address was returned from proper range, that its lease
+ // Check that address was returned from proper range, that its lease
// lifetime is correct, that T1 and T2 are returned properly
checkAddressParams(offer, subnet_);
- // check identifiers
+ // Check identifiers
checkServerId(offer, srv->getServerID());
checkClientId(offer, clientid);
}
@@ -508,7 +537,7 @@ TEST_F(Dhcpv4SrvTest, DiscoverBasic) {
// - offered address
TEST_F(Dhcpv4SrvTest, DiscoverHint) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
IOAddress hint("192.0.2.107");
Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
@@ -517,19 +546,19 @@ TEST_F(Dhcpv4SrvTest, DiscoverHint) {
dis->addOption(clientid);
dis->setYiaddr(hint);
- // Pass it to the server and get an advertise
+ // Pass it to the server and get an offer
Pkt4Ptr offer = srv->processDiscover(dis);
- // check if we get response at all
+ // Check if we get response at all
checkResponse(offer, DHCPOFFER, 1234);
- // check that address was returned from proper range, that its lease
+ // Check that address was returned from proper range, that its lease
// lifetime is correct, that T1 and T2 are returned properly
checkAddressParams(offer, subnet_);
EXPECT_EQ(offer->getYiaddr().toText(), hint.toText());
- // check identifiers
+ // Check identifiers
checkServerId(offer, srv->getServerID());
checkClientId(offer, clientid);
}
@@ -548,26 +577,26 @@ TEST_F(Dhcpv4SrvTest, DiscoverHint) {
// - offered address
TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
IOAddress hint("192.0.2.107");
Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
dis->setRemoteAddr(IOAddress("192.0.2.1"));
dis->setYiaddr(hint);
- // Pass it to the server and get an advertise
+ // Pass it to the server and get an offer
Pkt4Ptr offer = srv->processDiscover(dis);
- // check if we get response at all
+ // Check if we get response at all
checkResponse(offer, DHCPOFFER, 1234);
- // check that address was returned from proper range, that its lease
+ // Check that address was returned from proper range, that its lease
// lifetime is correct, that T1 and T2 are returned properly
checkAddressParams(offer, subnet_);
EXPECT_EQ(offer->getYiaddr().toText(), hint.toText());
- // check identifiers
+ // Check identifiers
checkServerId(offer, srv->getServerID());
}
@@ -585,7 +614,7 @@ TEST_F(Dhcpv4SrvTest, DiscoverNoClientId) {
// - offered address (!= hint)
TEST_F(Dhcpv4SrvTest, DiscoverInvalidHint) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
IOAddress hint("10.1.2.3");
Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
@@ -594,19 +623,19 @@ TEST_F(Dhcpv4SrvTest, DiscoverInvalidHint) {
dis->addOption(clientid);
dis->setYiaddr(hint);
- // Pass it to the server and get an advertise
+ // Pass it to the server and get an offer
Pkt4Ptr offer = srv->processDiscover(dis);
- // check if we get response at all
+ // Check if we get response at all
checkResponse(offer, DHCPOFFER, 1234);
- // check that address was returned from proper range, that its lease
+ // Check that address was returned from proper range, that its lease
// lifetime is correct, that T1 and T2 are returned properly
checkAddressParams(offer, subnet_);
EXPECT_NE(offer->getYiaddr().toText(), hint.toText());
- // check identifiers
+ // Check identifiers
checkServerId(offer, srv->getServerID());
checkClientId(offer, clientid);
}
@@ -617,13 +646,13 @@ TEST_F(Dhcpv4SrvTest, DiscoverInvalidHint) {
// This test checks that the server is offering different addresses to different
// clients in OFFERs. Please note that OFFER is not a guarantee that such
// an address will be assigned. Had the pool was very small and contained only
-// 2 addresses, the third client would get the same advertise as the first one
+// 2 addresses, the third client would get the same offer as the first one
// and this is a correct behavior. It is REQUEST that will fail for the third
// client. OFFER is basically saying "if you send me a request, you will
// probably get an address like this" (there are no guarantees).
TEST_F(Dhcpv4SrvTest, ManyDiscovers) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
Pkt4Ptr dis1 = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
Pkt4Ptr dis2 = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 2345));
@@ -633,7 +662,7 @@ TEST_F(Dhcpv4SrvTest, ManyDiscovers) {
dis2->setRemoteAddr(IOAddress("192.0.2.2"));
dis3->setRemoteAddr(IOAddress("192.0.2.3"));
- // different client-id sizes
+ // Different client-id sizes
OptionPtr clientid1 = generateClientId(4); // length 4
OptionPtr clientid2 = generateClientId(5); // length 5
OptionPtr clientid3 = generateClientId(6); // length 6
@@ -642,32 +671,32 @@ TEST_F(Dhcpv4SrvTest, ManyDiscovers) {
dis2->addOption(clientid2);
dis3->addOption(clientid3);
- // Pass it to the server and get an advertise
- Pkt4Ptr reply1 = srv->processDiscover(dis1);
- Pkt4Ptr reply2 = srv->processDiscover(dis2);
- Pkt4Ptr reply3 = srv->processDiscover(dis3);
+ // Pass it to the server and get an offer
+ Pkt4Ptr offer1 = srv->processDiscover(dis1);
+ Pkt4Ptr offer2 = srv->processDiscover(dis2);
+ Pkt4Ptr offer3 = srv->processDiscover(dis3);
- // check if we get response at all
- checkResponse(reply1, DHCPOFFER, 1234);
- checkResponse(reply2, DHCPOFFER, 2345);
- checkResponse(reply3, DHCPOFFER, 3456);
+ // Check if we get response at all
+ checkResponse(offer1, DHCPOFFER, 1234);
+ checkResponse(offer2, DHCPOFFER, 2345);
+ checkResponse(offer3, DHCPOFFER, 3456);
- IOAddress addr1 = reply1->getYiaddr();
- IOAddress addr2 = reply2->getYiaddr();
- IOAddress addr3 = reply3->getYiaddr();
+ IOAddress addr1 = offer1->getYiaddr();
+ IOAddress addr2 = offer2->getYiaddr();
+ IOAddress addr3 = offer3->getYiaddr();
// Check that the assigned address is indeed from the configured pool
- checkAddressParams(reply1, subnet_);
- checkAddressParams(reply2, subnet_);
- checkAddressParams(reply3, subnet_);
-
- // check DUIDs
- checkServerId(reply1, srv->getServerID());
- checkServerId(reply2, srv->getServerID());
- checkServerId(reply3, srv->getServerID());
- checkClientId(reply1, clientid1);
- checkClientId(reply2, clientid2);
- checkClientId(reply3, clientid3);
+ checkAddressParams(offer1, subnet_);
+ checkAddressParams(offer2, subnet_);
+ checkAddressParams(offer3, subnet_);
+
+ // Check DUIDs
+ checkServerId(offer1, srv->getServerID());
+ checkServerId(offer2, srv->getServerID());
+ checkServerId(offer3, srv->getServerID());
+ checkClientId(offer1, clientid1);
+ checkClientId(offer2, clientid2);
+ checkClientId(offer3, clientid3);
// Finally check that the addresses offered are different
EXPECT_NE(addr1.toText(), addr2.toText());
@@ -695,7 +724,7 @@ TEST_F(Dhcpv4SrvTest, ManyDiscovers) {
// Test verifies that the lease is actually in the database.
TEST_F(Dhcpv4SrvTest, RequestBasic) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
IOAddress hint("192.0.2.107");
Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234));
@@ -707,19 +736,19 @@ TEST_F(Dhcpv4SrvTest, RequestBasic) {
// Pass it to the server and get an advertise
Pkt4Ptr ack = srv->processRequest(req);
- // check if we get response at all
+ // Check if we get response at all
checkResponse(ack, DHCPACK, 1234);
EXPECT_EQ(hint.toText(), ack->getYiaddr().toText());
- // check that address was returned from proper range, that its lease
+ // Check that address was returned from proper range, that its lease
// lifetime is correct, that T1 and T2 are returned properly
checkAddressParams(ack, subnet_);
- // check identifiers
+ // Check identifiers
checkServerId(ack, srv->getServerID());
checkClientId(ack, clientid);
- // check that the lease is really in the database
+ // Check that the lease is really in the database
Lease4Ptr l = checkLease(ack, clientid, req->getHWAddr(), hint);
ASSERT_TRUE(l);
@@ -741,7 +770,7 @@ TEST_F(Dhcpv4SrvTest, RequestBasic) {
TEST_F(Dhcpv4SrvTest, ManyRequests) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
const IOAddress req_addr1("192.0.2.105");
const IOAddress req_addr2("192.0.2.101");
@@ -764,7 +793,7 @@ TEST_F(Dhcpv4SrvTest, ManyRequests) {
req2->setHWAddr(generateHWAddr(7));
req3->setHWAddr(generateHWAddr(8));
- // different client-id sizes
+ // Different client-id sizes
OptionPtr clientid1 = generateClientId(4); // length 4
OptionPtr clientid2 = generateClientId(5); // length 5
OptionPtr clientid3 = generateClientId(6); // length 6
@@ -778,7 +807,7 @@ TEST_F(Dhcpv4SrvTest, ManyRequests) {
Pkt4Ptr ack2 = srv->processRequest(req2);
Pkt4Ptr ack3 = srv->processRequest(req3);
- // check if we get response at all
+ // Check if we get response at all
checkResponse(ack1, DHCPACK, 1234);
checkResponse(ack2, DHCPACK, 2345);
checkResponse(ack3, DHCPACK, 3456);
@@ -797,7 +826,7 @@ TEST_F(Dhcpv4SrvTest, ManyRequests) {
checkAddressParams(ack2, subnet_);
checkAddressParams(ack3, subnet_);
- // check DUIDs
+ // Check DUIDs
checkServerId(ack1, srv->getServerID());
checkServerId(ack2, srv->getServerID());
checkServerId(ack3, srv->getServerID());
@@ -832,7 +861,7 @@ TEST_F(Dhcpv4SrvTest, ManyRequests) {
// - lease is actually renewed in LeaseMgr
TEST_F(Dhcpv4SrvTest, RenewBasic) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
const IOAddress addr("192.0.2.106");
const uint32_t temp_t1 = 50;
@@ -840,7 +869,7 @@ TEST_F(Dhcpv4SrvTest, RenewBasic) {
const uint32_t temp_valid = 100;
const time_t temp_timestamp = time(NULL) - 10;
- // Generate client-id also duid_
+ // Generate client-id also sets client_id_ member
OptionPtr clientid = generateClientId();
// Check that the address we are about to use is indeed in pool
@@ -882,7 +911,7 @@ TEST_F(Dhcpv4SrvTest, RenewBasic) {
checkResponse(ack, DHCPACK, 1234);
EXPECT_EQ(addr.toText(), ack->getYiaddr().toText());
- // check that address was returned from proper range, that its lease
+ // Check that address was returned from proper range, that its lease
// lifetime is correct, that T1 and T2 are returned properly
checkAddressParams(ack, subnet_);
@@ -902,7 +931,7 @@ TEST_F(Dhcpv4SrvTest, RenewBasic) {
// Checking for CLTT is a bit tricky if we want to avoid off by 1 errors
int32_t cltt = static_cast<int32_t>(l->cltt_);
int32_t expected = static_cast<int32_t>(time(NULL));
- // equality or difference by 1 between cltt and expected is ok.
+ // Equality or difference by 1 between cltt and expected is ok.
EXPECT_GE(1, abs(cltt - expected));
EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr));
@@ -913,25 +942,22 @@ TEST_F(Dhcpv4SrvTest, RenewBasic) {
// This test verifies if the sanityCheck() really checks options presence.
TEST_F(Dhcpv4SrvTest, sanityCheck) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
- // check that the packets originating from local addresses can be
- pkt->setRemoteAddr(IOAddress("192.0.2.1"));
-
- // client-id is optional for information-request, so
+ // Client-id is optional for information-request, so
EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::OPTIONAL));
- // empty packet, no server-id
+ // Empty packet, no server-id
EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY), RFCViolation);
pkt->addOption(srv->getServerID());
- // server-id is mandatory and present = no exception
+ // Server-id is mandatory and present = no exception
EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY));
- // server-id is forbidden, but present => exception
+ // Server-id is forbidden, but present => exception
EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::FORBIDDEN),
RFCViolation);
}
@@ -941,7 +967,7 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
// the lease is indeed removed from the database.
TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
const IOAddress addr("192.0.2.106");
const uint32_t temp_t1 = 50;
@@ -955,7 +981,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
// Check that the address we are about to use is indeed in pool
ASSERT_TRUE(subnet_->inPool(addr));
- // let's create a lease and put it in the LeaseMgr
+ // Let's create a lease and put it in the LeaseMgr
uint8_t mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
@@ -990,7 +1016,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
// Lease4Collection leases = LeaseMgrFactory::instance().getLease4(hw->hwaddr_);
// EXPECT_EQ(leases.size(), 0);
- // Try to get it by hw/subnet_id compination
+ // Try to get it by hw/subnet_id combination
l = LeaseMgrFactory::instance().getLease4(hw->hwaddr_, subnet_->getID());
EXPECT_FALSE(l);
@@ -1014,7 +1040,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
// 3. there is such a lease, but it belongs to a different client
TEST_F(Dhcpv4SrvTest, ReleaseReject) {
boost::scoped_ptr<NakedDhcpv4Srv> srv;
- ASSERT_NO_THROW( srv.reset(new NakedDhcpv4Srv(0)) );
+ ASSERT_NO_THROW(srv.reset(new NakedDhcpv4Srv(0)));
const IOAddress addr("192.0.2.106");
const uint32_t t1 = 50;
@@ -1022,7 +1048,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseReject) {
const uint32_t valid = 100;
const time_t timestamp = time(NULL) - 10;
- // let's create a lease and put it in the LeaseMgr
+ // Let's create a lease and put it in the LeaseMgr
uint8_t bogus_mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
HWAddrPtr bogus_hw(new HWAddr(bogus_mac_addr, sizeof(bogus_mac_addr), HTYPE_ETHER));
OptionPtr bogus_clientid = generateClientId(7); // different length
diff --git a/src/bin/dhcp4/tests/dhcp4_unittests.cc b/src/bin/dhcp4/tests/dhcp4_unittests.cc
index 2cd915b..93e836b 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 c8be278..77cd6bd 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<uint8_t>
+ /// @param hwaddr const reference to hardware address
+ /// @param htype hardware type
HWAddr(const std::vector<uint8_t>& 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<HWAddr> 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 411c346..d3b22de 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -49,11 +49,12 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
yiaddr_(DEFAULT_ADDRESS),
siaddr_(DEFAULT_ADDRESS),
giaddr_(DEFAULT_ADDRESS),
- bufferOut_(DHCPV4_PKT_HDR_LEN),
- msg_type_(msg_type)
+ bufferOut_(DHCPV4_PKT_HDR_LEN)
{
memset(sname_, 0, MAX_SNAME_LEN);
memset(file_, 0, MAX_FILE_LEN);
+
+ setType(msg_type);
}
Pkt4::Pkt4(const uint8_t* data, size_t len)
@@ -72,8 +73,7 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
yiaddr_(DEFAULT_ADDRESS),
siaddr_(DEFAULT_ADDRESS),
giaddr_(DEFAULT_ADDRESS),
- bufferOut_(0), // not used, this is RX packet
- msg_type_(DHCPDISCOVER)
+ bufferOut_(0) // not used, this is RX packet
{
if (len < DHCPV4_PKT_HDR_LEN) {
isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
@@ -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
@@ -215,21 +216,44 @@ Pkt4::unpack() {
}
void Pkt4::check() {
+ uint8_t msg_type = getType();
+ if (msg_type > DHCPLEASEACTIVE) {
+ isc_throw(BadValue, "Invalid DHCP message type received: "
+ << msg_type);
+ }
+}
+
+uint8_t Pkt4::getType() const {
+ OptionPtr generic = getOption(DHO_DHCP_MESSAGE_TYPE);
+ if (!generic) {
+ isc_throw(Unexpected, "Missing DHCP Message Type option");
+ }
+
+ // Check if Message Type is specified as OptionInt<uint8_t>
boost::shared_ptr<OptionInt<uint8_t> > typeOpt =
- boost::dynamic_pointer_cast<OptionInt<uint8_t> >(getOption(DHO_DHCP_MESSAGE_TYPE));
+ boost::dynamic_pointer_cast<OptionInt<uint8_t> >(generic);
if (typeOpt) {
- uint8_t msg_type = typeOpt->getValue();
- if (msg_type > DHCPLEASEACTIVE) {
- isc_throw(BadValue, "Invalid DHCP message type received: "
- << msg_type);
- }
- msg_type_ = msg_type;
+ return (typeOpt->getValue());
+ }
+
+ // Try to use it as generic option
+ return (generic->getUint8());
+}
+void Pkt4::setType(uint8_t dhcp_type) {
+ OptionPtr opt = getOption(DHO_DHCP_MESSAGE_TYPE);
+ if (opt) {
+ // There is message type option already, update it
+ opt->setUint8(dhcp_type);
} else {
- isc_throw(Unexpected, "Missing DHCP Message Type option");
+ // There is no message type option yet, add it
+ std::vector<uint8_t> tmp(1, dhcp_type);
+ opt = OptionPtr(new Option(Option::V4, DHO_DHCP_MESSAGE_TYPE, tmp));
+ addOption(opt);
}
}
+
void Pkt4::repack() {
bufferOut_.writeData(&data_[0], data_.size());
}
@@ -239,7 +263,7 @@ Pkt4::toText() {
stringstream tmp;
tmp << "localAddr=" << local_addr_.toText() << ":" << local_port_
<< " remoteAddr=" << remote_addr_.toText()
- << ":" << remote_port_ << ", msgtype=" << int(msg_type_)
+ << ":" << remote_port_ << ", msgtype=" << getType()
<< ", transid=0x" << hex << transid_ << dec << endl;
for (isc::dhcp::Option::OptionCollection::iterator opt=options_.begin();
@@ -265,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
@@ -347,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
@@ -361,7 +385,7 @@ Pkt4::addOption(boost::shared_ptr<Option> opt) {
}
boost::shared_ptr<isc::dhcp::Option>
-Pkt4::getOption(uint8_t type) {
+Pkt4::getOption(uint8_t type) const {
Option::OptionCollection::const_iterator x = options_.find(type);
if (x != options_.end()) {
return (*x).second;
@@ -372,7 +396,7 @@ Pkt4::getOption(uint8_t type) {
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/pkt4.h b/src/lib/dhcp/pkt4.h
index 6674bb1..664686b 100644
--- a/src/lib/dhcp/pkt4.h
+++ b/src/lib/dhcp/pkt4.h
@@ -218,16 +218,15 @@ public:
/// @return transaction-id
uint32_t getTransid() const { return (transid_); };
- /// @brief Returns message type (e.g. 1 = DHCPDISCOVER).
+ /// @brief Returns DHCP message type (e.g. 1 = DHCPDISCOVER).
///
/// @return message type
- uint8_t
- getType() const { return (msg_type_); }
+ uint8_t getType() const;
- /// @brief Sets message type (e.g. 1 = DHCPDISCOVER).
+ /// @brief Sets DHCP message type (e.g. 1 = DHCPDISCOVER).
///
/// @param type message type to be set
- void setType(uint8_t type) { msg_type_=type; };
+ void setType(uint8_t type);
/// @brief Returns sname field
///
@@ -323,7 +322,7 @@ public:
/// @return returns option of requested type (or NULL)
/// if no such option is present
boost::shared_ptr<Option>
- getOption(uint8_t opt_type);
+ getOption(uint8_t opt_type) const;
/// @brief Deletes specified option
/// @param type option type to be deleted
@@ -522,11 +521,6 @@ protected:
/// data format change etc.
std::vector<uint8_t> data_;
- /// message type (e.g. 1=DHCPDISCOVER)
- /// @todo this will eventually be replaced with DHCP Message Type
- /// option (option 53)
- uint8_t msg_type_;
-
/// collection of options present in this message
///
/// @warning This protected member is accessed by derived
diff --git a/src/lib/dhcp/tests/hwaddr_unittest.cc b/src/lib/dhcp/tests/hwaddr_unittest.cc
index 1173b99..b4ee55e 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/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 11cbf34..d72ef9e 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -702,11 +702,9 @@ TEST_F(IfaceMgrTest, sendReceive4) {
sendPkt->setYiaddr(IOAddress("192.0.2.3"));
sendPkt->setGiaddr(IOAddress("192.0.2.4"));
- // unpack() now checks if mandatory DHCP_MESSAGE_TYPE is present
- boost::shared_ptr<Option> msgType(new Option(Option::V4,
- static_cast<uint16_t>(DHO_DHCP_MESSAGE_TYPE)));
- msgType->setUint8(static_cast<uint8_t>(DHCPDISCOVER));
- sendPkt->addOption(msgType);
+ // Unpack() now checks if mandatory DHCP_MESSAGE_TYPE is present.
+ // Workarounds (creating DHCP Message Type Option by hand) are no longer
+ // needed as setDhcpType() is called in constructor.
uint8_t sname[] = "That's just a string that will act as SNAME";
sendPkt->setSname(sname, strlen((const char*)sname));
@@ -744,7 +742,6 @@ TEST_F(IfaceMgrTest, sendReceive4) {
EXPECT_EQ(sendPkt->getYiaddr(), rcvPkt->getYiaddr());
EXPECT_EQ(sendPkt->getGiaddr(), rcvPkt->getGiaddr());
EXPECT_EQ(sendPkt->getTransid(), rcvPkt->getTransid());
- EXPECT_EQ(sendPkt->getType(), rcvPkt->getType());
EXPECT_TRUE(sendPkt->getSname() == rcvPkt->getSname());
EXPECT_TRUE(sendPkt->getFile() == rcvPkt->getFile());
EXPECT_EQ(sendPkt->getHtype(), rcvPkt->getHtype());
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 6ca73b1..49588e1 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -69,8 +69,9 @@ TEST(Pkt4Test, constructor) {
pkt = new Pkt4(DHCPDISCOVER, 0xffffffff);
);
- // DHCPv4 packet must be at least 236 bytes long
- EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
+ // DHCPv4 packet must be at least 236 bytes long, with Message Type
+ // Option taking extra 3 bytes it is 239
+ EXPECT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + 3, pkt->len());
EXPECT_EQ(DHCPDISCOVER, pkt->getType());
EXPECT_EQ(0xffffffff, pkt->getTransid());
EXPECT_NO_THROW(
@@ -219,8 +220,10 @@ TEST(Pkt4Test, fixedFields) {
EXPECT_EQ(dummySiaddr.toText(), pkt->getSiaddr().toText());
EXPECT_EQ(dummyGiaddr.toText(), pkt->getGiaddr().toText());
- // chaddr is always 16 bytes long and contains link-layer addr (MAC)
- EXPECT_EQ(0, memcmp(dummyChaddr, &pkt->getHWAddr()->hwaddr_[0], 16));
+ // 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));
@@ -237,7 +240,9 @@ TEST(Pkt4Test, fixedFieldsPack) {
pkt->pack();
);
- ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN), pkt->len());
+ // Minimum packet size is 236 bytes + 3 bytes of mandatory
+ // DHCP Message Type Option
+ ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + 3, pkt->len());
// redundant but MUCH easier for debug in gdb
const uint8_t* exp = &expectedFormat[0];
@@ -469,7 +474,7 @@ TEST(Pkt4Test, file) {
static uint8_t v4Opts[] = {
12, 3, 0, 1, 2, // Hostname
14, 3, 10, 11, 12, // Merit Dump File
- 53, 1, 1, // Message Type (required to not throw exception during unpack)
+ 53, 1, 2, // Message Type (required to not throw exception during unpack)
60, 3, 20, 21, 22, // Class Id
128, 3, 30, 31, 32, // Vendor specific
254, 3, 40, 41, 42, // Reserved
@@ -487,18 +492,15 @@ TEST(Pkt4Test, options) {
boost::shared_ptr<Option> opt1(new Option(Option::V4, 12, payload[0]));
boost::shared_ptr<Option> opt3(new Option(Option::V4, 14, payload[1]));
- boost::shared_ptr<Option> optMsgType(new Option(Option::V4, DHO_DHCP_MESSAGE_TYPE));
boost::shared_ptr<Option> opt2(new Option(Option::V4, 60, payload[2]));
boost::shared_ptr<Option> opt5(new Option(Option::V4,128, payload[3]));
boost::shared_ptr<Option> opt4(new Option(Option::V4,254, payload[4]));
- optMsgType->setUint8(static_cast<uint8_t>(DHCPDISCOVER));
pkt->addOption(opt1);
pkt->addOption(opt2);
pkt->addOption(opt3);
pkt->addOption(opt4);
pkt->addOption(opt5);
- pkt->addOption(optMsgType);
EXPECT_TRUE(pkt->getOption(12));
EXPECT_TRUE(pkt->getOption(60));
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index f039fa0..7a64dac 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 76e9acd..c6cbc35 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 8109745..07d4d68 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<Subnet6*>(this) && hint.toText() == "::") {
+ if (dynamic_cast<Subnet4*>(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 8030abc..078998d 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
@@ -31,7 +31,7 @@
#include <iostream>
#include <sstream>
-#include <map>
+#include <set>
#include <time.h>
using namespace std;
@@ -42,17 +42,31 @@ using namespace isc::dhcp::test;
namespace {
+/// @brief Allocation engine with some internal methods exposed
class NakedAllocEngine : public AllocEngine {
public:
+
+ /// @brief the sole constructor
+ /// @param engine_type specifies engine type (e.g. iterative)
+ /// @param attempts number of lease selection attempts before giving up
NakedAllocEngine(AllocEngine::AllocType engine_type, unsigned int attempts)
:AllocEngine(engine_type, attempts) {
}
+
+ // Expose internal classes for testing purposes
using AllocEngine::Allocator;
using AllocEngine::IterativeAllocator;
};
+/// @brief Used in Allocation Engine tests for IPv6
class AllocEngine6Test : public ::testing::Test {
public:
+
+ /// @brief Default constructor
+ ///
+ /// Sets duid_, iaid_, subnet_, pool_ fields to example values used
+ /// in many tests, initializes cfg_mgr configuration and creates
+ /// lease database.
AllocEngine6Test() {
duid_ = DuidPtr(new DUID(vector<uint8_t>(8, 0x42)));
iaid_ = 42;
@@ -69,6 +83,9 @@ public:
factory_.create("type=memfile");
}
+ /// @brief checks if Lease6 matches expected configuration
+ ///
+ /// @param lease lease to be checked
void checkLease6(const Lease6Ptr& lease) {
// that is belongs to the right subnet
EXPECT_EQ(lease->subnet_id_, subnet_->getID());
@@ -92,15 +109,22 @@ public:
factory_.destroy();
}
- DuidPtr duid_;
- uint32_t iaid_;
- Subnet6Ptr subnet_;
- Pool6Ptr pool_;
- LeaseMgrFactory factory_;
+ DuidPtr duid_; ///< client-identifier (value used in tests)
+ uint32_t iaid_; ///< IA identifier (value used in tests)
+ Subnet6Ptr subnet_; ///< subnet6 (used in tests)
+ Pool6Ptr pool_; ///< pool belonging to subnet_
+ LeaseMgrFactory factory_; ///< pointer to LeaseMgr factory
};
+/// @brief Used in Allocation Engine tests for IPv4
class AllocEngine4Test : public ::testing::Test {
public:
+
+ /// @brief Default constructor
+ ///
+ /// Sets clientid_, hwaddr_, subnet_, pool_ fields to example values
+ /// used in many tests, initializes cfg_mgr configuration and creates
+ /// lease database.
AllocEngine4Test() {
clientid_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x44)));
static uint8_t mac[] = { 0, 1, 22, 33, 44, 55};
@@ -121,6 +145,9 @@ public:
factory_.create("type=memfile");
}
+ /// @brief checks if Lease4 matches expected configuration
+ ///
+ /// @param lease lease to be checked
void checkLease4(const Lease4Ptr& lease) {
// that is belongs to the right subnet
EXPECT_EQ(lease->subnet_id_, subnet_->getID());
@@ -142,11 +169,11 @@ public:
factory_.destroy();
}
- ClientIdPtr clientid_;
- HWAddrPtr hwaddr_;
- Subnet4Ptr subnet_;
- Pool4Ptr pool_;
- LeaseMgrFactory factory_;
+ ClientIdPtr clientid_; ///< client-identifier (value used in tests)
+ HWAddrPtr hwaddr_; ///< hardware address (value used in tests)
+ Subnet4Ptr subnet_; ///< subnet4 (used in tests)
+ Pool4Ptr pool_; ///< pool belonging to subnet_
+ LeaseMgrFactory factory_; ///< pointer to LeaseMgr factory
};
// This test checks if the Allocation Engine can be instantiated and that it
@@ -338,7 +365,7 @@ TEST_F(AllocEngine6Test, IterativeAllocator_manyPools6) {
// there are 8 extra pools with 9 addresses in each.
// Let's keep picked addresses here and check their uniqueness.
- std::map<IOAddress, int> generated_addrs;
+ std::set<IOAddress> generated_addrs;
int cnt = 0;
while (++cnt) {
IOAddress candidate = alloc->pickAddress(subnet_, duid_, IOAddress("::"));
@@ -351,7 +378,7 @@ TEST_F(AllocEngine6Test, IterativeAllocator_manyPools6) {
if (generated_addrs.find(candidate) == generated_addrs.end()) {
// we haven't had this
- generated_addrs[candidate] = 0;
+ generated_addrs.insert(candidate);
} else {
// we have seen this address before. That should mean that we
// iterated over all addresses.
@@ -538,10 +565,10 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
IOAddress("0.0.0.0"), false);
- // check that we got a lease
+ // Check that we got a lease
ASSERT_TRUE(lease);
- // do all checks on the lease
+ // Do all checks on the lease
checkLease4(lease);
// Check that the lease is indeed in LeaseMgr
@@ -561,10 +588,10 @@ TEST_F(AllocEngine4Test, fakeAlloc4) {
Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
IOAddress("0.0.0.0"), true);
- // check that we got a lease
+ // Check that we got a lease
ASSERT_TRUE(lease);
- // do all checks on the lease
+ // Do all checks on the lease
checkLease4(lease);
// Check that the lease is NOT in LeaseMgr
@@ -584,13 +611,13 @@ TEST_F(AllocEngine4Test, allocWithValidHint4) {
IOAddress("192.0.2.105"),
false);
- // check that we got a lease
+ // Check that we got a lease
ASSERT_TRUE(lease);
- // we should get what we asked for
+ // We should get what we asked for
EXPECT_EQ(lease->addr_.toText(), "192.0.2.105");
- // do all checks on the lease
+ // Do all checks on the lease
checkLease4(lease);
// Check that the lease is indeed in LeaseMgr
@@ -609,7 +636,7 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
ASSERT_TRUE(engine);
- // let's create a lease and put it in the LeaseMgr
+ // Let's create a lease and put it in the LeaseMgr
uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
time_t now = time(NULL);
@@ -617,22 +644,22 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
clientid2, sizeof(clientid2), 1, 2, 3, now, subnet_->getID()));
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
- // another client comes in and request an address that is in pool, but
+ // Another client comes in and request an address that is in pool, but
// unfortunately it is used already. The same address must not be allocated
// twice.
Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
IOAddress("192.0.2.106"),
false);
- // check that we got a lease
+ // Check that we got a lease
ASSERT_TRUE(lease);
- // allocated address must be different
+ // Allocated address must be different
EXPECT_TRUE(used->addr_.toText() != lease->addr_.toText());
- // we should NOT get what we asked for, because it is used already
+ // We should NOT get what we asked for, because it is used already
EXPECT_TRUE(lease->addr_.toText() != "192.0.2.106");
- // do all checks on the lease
+ // Do all checks on the lease
checkLease4(lease);
// Check that the lease is indeed in LeaseMgr
@@ -657,13 +684,13 @@ TEST_F(AllocEngine4Test, allocBogusHint4) {
Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_,
IOAddress("10.1.1.1"),
false);
- // check that we got a lease
+ // Check that we got a lease
ASSERT_TRUE(lease);
- // we should NOT get what we asked for, because it is used already
+ // We should NOT get what we asked for, because it is used already
EXPECT_TRUE(lease->addr_.toText() != "10.1.1.1");
- // do all checks on the lease
+ // Do all checks on the lease
checkLease4(lease);
// Check that the lease is indeed in LeaseMgr
@@ -695,12 +722,12 @@ TEST_F(AllocEngine4Test, IterativeAllocator) {
TEST_F(AllocEngine4Test, IterativeAllocator_manyPools4) {
NakedAllocEngine::IterativeAllocator* alloc = new NakedAllocEngine::IterativeAllocator();
- // let's start from 2, as there is 2001:db8:1::10 - 2001:db8:1::20 pool already.
+ // Let's start from 2, as there is 2001:db8:1::10 - 2001:db8:1::20 pool already.
for (int i = 2; i < 10; ++i) {
stringstream min, max;
- min << "192.0.2." << i*10 + 1;
- max << "192.0.2." << i*10 + 9;
+ min << "192.0.2." << i * 10 + 1;
+ max << "192.0.2." << i * 10 + 9;
Pool4Ptr pool(new Pool4(IOAddress(min.str()),
IOAddress(max.str())));
@@ -708,11 +735,11 @@ TEST_F(AllocEngine4Test, IterativeAllocator_manyPools4) {
subnet_->addPool(pool);
}
- int total = 10 + 8*9; // first pool (.100 - .109) has 10 addresses in it,
- // there are 8 extra pools with 9 addresses in each.
+ int total = 10 + 8 * 9; // first pool (.100 - .109) has 10 addresses in it,
+ // there are 8 extra pools with 9 addresses in each.
// Let's keep picked addresses here and check their uniqueness.
- std::map<IOAddress, int> generated_addrs;
+ std::set<IOAddress> generated_addrs;
int cnt = 0;
while (++cnt) {
IOAddress candidate = alloc->pickAddress(subnet_, clientid_, IOAddress("0.0.0.0"));
@@ -724,8 +751,8 @@ TEST_F(AllocEngine4Test, IterativeAllocator_manyPools4) {
// cout << candidate.toText() << endl;
if (generated_addrs.find(candidate) == generated_addrs.end()) {
- // we haven't had this
- generated_addrs[candidate] = 0;
+ // We haven't had this
+ generated_addrs.insert(candidate);
} else {
// we have seen this address before. That should mean that we
// iterated over all addresses.
@@ -771,7 +798,7 @@ TEST_F(AllocEngine4Test, smallPool4) {
EXPECT_EQ("192.0.2.17", lease->addr_.toText());
- // do all checks on the lease
+ // Do all checks on the lease
checkLease4(lease);
// Check that the lease is indeed in LeaseMgr
@@ -837,7 +864,7 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
time_t now = time(NULL) - 500; // Allocated 500 seconds ago
Lease4Ptr lease(new Lease4(addr, clientid2, sizeof(clientid2), hwaddr2, sizeof(hwaddr2),
495, 100, 200, now, subnet_->getID()));
- // lease was assigned 500 seconds ago, but its valid lifetime is 495, so it
+ // Lease was assigned 500 seconds ago, but its valid lifetime is 495, so it
// is expired already
ASSERT_TRUE(lease->expired());
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
@@ -916,7 +943,7 @@ TEST_F(AllocEngine4Test, renewLease4) {
old_timestamp, subnet_->getID()));
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
- // lease was assigned 45 seconds ago and is valid for 100 seconds. Let's
+ // Lease was assigned 45 seconds ago and is valid for 100 seconds. Let's
// renew it.
ASSERT_FALSE(lease->expired());
lease = engine->renewLease4(subnet_, clientid_, hwaddr_, lease, false);
More information about the bind10-changes
mailing list