BIND 10 trac3242, updated. f22c490bd4a194ad693e29a8e9f65a0480405e5c [3242] Addressed review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Feb 6 17:10:19 UTC 2014
The branch, trac3242 has been updated
via f22c490bd4a194ad693e29a8e9f65a0480405e5c (commit)
from 1b522d63a842654c680602d3ee89eac324333d48 (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 f22c490bd4a194ad693e29a8e9f65a0480405e5c
Author: Marcin Siodelski <marcin at isc.org>
Date: Thu Feb 6 18:10:07 2014 +0100
[3242] Addressed review comments.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 5 +
src/bin/dhcp4/dhcp4_srv.cc | 59 +++++--
src/bin/dhcp4/dhcp4_srv.h | 32 +++-
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 42 +++++
src/bin/dhcp4/tests/dhcp4_test_utils.cc | 22 ++-
src/bin/dhcp4/tests/dhcp4_test_utils.h | 233 +++++++++++++------------
src/bin/dhcp4/tests/direct_client_unittest.cc | 33 +---
src/lib/dhcp/dhcp4.h | 6 +-
src/lib/dhcp/pkt4.cc | 2 +
src/lib/dhcp/tests/Makefile.am | 10 +-
src/lib/dhcp/tests/iface_mgr_test_config.cc | 2 +-
src/lib/dhcp/tests/iface_mgr_test_config.h | 51 +++++-
src/lib/dhcp/tests/pkt_filter_test_stub.h | 4 +-
src/lib/dhcp/tests/pkt_filter_test_utils.h | 6 +-
src/lib/dhcpsrv/cfgmgr.cc | 2 +-
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 4 +-
16 files changed, 348 insertions(+), 165 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index b2a8adb..b9185d8 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -135,6 +135,11 @@ A "libreload" command was issued to reload the hooks libraries but for
some reason the reload failed. Other error messages issued from the
hooks framework will indicate the nature of the problem.
+% DHCP4_UNRECOGNIZED_RCVD_PACKET_TYPE received message (transaction id %1) has unregonized type %2
+This debug message indicates that the message type carried in DHCPv4 option
+53 is unrecognized by the server. The valid message types are available
+on the IANA website. The message will not be processed by the server.
+
% DHCP4_LEASE_ADVERT lease %1 advertised (client client-id %2, hwaddr %3)
This debug message indicates that the server successfully advertised
a lease. It is up to the client to choose one server out of othe advertised
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index abd9405..e6cb252 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -266,7 +266,7 @@ Dhcpv4Srv::run() {
continue;
}
- // We have sanity checked (in accept()) that the Message Type option
+ // We have sanity checked (in accept() that the Message Type option
// exists, so we can safely get it here.
int type = query->getType();
LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_RECEIVED)
@@ -1573,15 +1573,12 @@ Dhcpv4Srv::accept(const Pkt4Ptr& query) const {
return (false);
}
- // When receiving a packet without message type option, getType() will
- // throw.
- try {
- query->getType();
- } catch (...) {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_NO_TYPE)
- .arg(query->getIface());
+ // Check that the message type is accepted by the server. We rely on the
+ // function called to log a message if needed.
+ if (!acceptMessageType(query)) {
return (false);
}
+
return (true);
}
@@ -1595,11 +1592,51 @@ Dhcpv4Srv::acceptDirectRequest(const Pkt4Ptr& pkt) const {
return (false);
}
static const IOAddress bcast("255.255.255.255");
- return ((pkt->getLocalAddr() == bcast && !selectSubnet(pkt)) ? false : true);
+ return ((pkt->getLocalAddr() != bcast || selectSubnet(pkt)));
+}
+
+bool
+Dhcpv4Srv::acceptMessageType(const Pkt4Ptr& query) const {
+ // When receiving a packet without message type option, getType() will
+ // throw.
+ int type;
+ try {
+ type = query->getType();
+
+ } catch (...) {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PACKET_DROP_NO_TYPE)
+ .arg(query->getIface());
+ return (false);
+ }
+
+ // If we receive a message with a non-existing type, we are logging it.
+ if (type > DHCPLEASEQUERYDONE) {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL,
+ DHCP4_UNRECOGNIZED_RCVD_PACKET_TYPE)
+ .arg(type)
+ .arg(query->getTransid());
+ return (false);
+ }
+
+ // Once we know that the message type is within a range of defined DHCPv4
+ // messages, we do a detailed check to make sure that the received message
+ // is targeted at server. Note that we could have received some Offer
+ // message broadcasted by the other server to a relay. Even though, the
+ // server would rather unicast its response to a relay, let's be on the
+ // safe side. Also, we want to drop other messages which we don't support.
+ // All these valid messages that we are not going to process are dropped
+ // silently.
+ if ((type != DHCPDISCOVER) && (type != DHCPREQUEST) &&
+ (type != DHCPRELEASE) && (type != DHCPDECLINE) &&
+ (type != DHCPINFORM)) {
+ return (false);
+ }
+
+ return (true);
}
bool
-Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
+Dhcpv4Srv::acceptServerId(const Pkt4Ptr& query) const {
// This function is meant to be called internally by the server class, so
// we rely on the caller to sanity check the pointer and we don't check
// it here.
@@ -1609,7 +1646,7 @@ Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
// Note that we don't check cases that server identifier is mandatory
// but not present. This is meant to be sanity checked in other
// functions.
- OptionPtr option = pkt->getOption(DHO_DHCP_SERVER_IDENTIFIER);
+ OptionPtr option = query->getOption(DHO_DHCP_SERVER_IDENTIFIER);
if (!option) {
return (true);
}
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 3649ce1..087120c 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -167,6 +167,13 @@ public:
protected:
+ /// @name Functions filtering and sanity-checking received messages.
+ ///
+ /// @todo These functions are supposed to be moved to a new class which
+ /// will manage different rules for accepting and rejecting messages.
+ /// Perhaps ticket #3116 is a good opportunity to do it.
+ ///
+ //@{
/// @brief Checks whether received message should be processed or discarded.
///
/// This function checks whether received message should be processed or
@@ -211,11 +218,31 @@ protected:
/// - all broadcast messages received on the interface for which the
/// suitable subnet exists (is configured).
///
- /// @param pkt Message sent by a client.
+ /// @param query Message sent by a client.
///
/// @return true if message is accepted for further processing, false
/// otherwise.
- bool acceptDirectRequest(const Pkt4Ptr& pkt) const;
+ bool acceptDirectRequest(const Pkt4Ptr& query) const;
+
+ /// @brief Check if received message type is valid for the server to
+ /// process.
+ ///
+ /// This function checks that the received message type belongs to the range
+ /// of types regonized by the server and that the message of this type
+ /// should be processed by the server.
+ ///
+ /// The messages types accepted for processing are:
+ /// - Discover
+ /// - Request
+ /// - Release
+ /// - Decline
+ /// - Inform
+ ///
+ /// @param query Message sent by a client.
+ ///
+ /// @return true if message is accepted for further processing, false
+ /// otherwise.
+ bool acceptMessageType(const Pkt4Ptr& query) const;
/// @brief Verifies if the server id belongs to our server.
///
@@ -231,6 +258,7 @@ protected:
/// @return true, if the server identifier is absent or matches one of the
/// server identifiers that the server is using; false otherwise.
bool acceptServerId(const Pkt4Ptr& pkt) const;
+ //@}
/// @brief verifies if specified packet meets RFC requirements
///
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 2b00c24..b8f6a24 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -3361,4 +3361,46 @@ TEST_F(Dhcpv4SrvTest, acceptDirectRequest) {
}
+// This test checks that the server rejects a message with invalid type.
+TEST_F(Dhcpv4SrvTest, acceptMessageType) {
+ IfaceMgrTestConfig test_config(true);
+ IfaceMgr::instance().openSockets4();
+
+ NakedDhcpv4Srv srv(0);
+
+ // Specify messages to be accepted by the server.
+ int allowed[] = {
+ DHCPDISCOVER,
+ DHCPREQUEST,
+ DHCPRELEASE,
+ DHCPDECLINE,
+ DHCPINFORM
+ };
+ size_t allowed_size = sizeof(allowed) / sizeof(allowed[0]);
+ // Check that the server actually accepts these message types.
+ for (int i = 0; i < allowed_size; ++i) {
+ EXPECT_TRUE(srv.acceptMessageType(Pkt4Ptr(new Pkt4(allowed[i], 1234))))
+ << "Test failed for message type " << i;
+ }
+ // Specify messages which server is supposed to drop.
+ int not_allowed[] = {
+ DHCPOFFER,
+ DHCPACK,
+ DHCPNAK,
+ DHCPLEASEQUERY,
+ DHCPLEASEUNASSIGNED,
+ DHCPLEASEUNKNOWN,
+ DHCPLEASEACTIVE,
+ DHCPBULKLEASEQUERY,
+ DHCPLEASEQUERYDONE
+ };
+ size_t not_allowed_size = sizeof(not_allowed) / sizeof(not_allowed[0]);
+ // Actually check that the server will drop these messages.
+ for (int i = 0; i < not_allowed_size; ++i) {
+ EXPECT_FALSE(srv.acceptMessageType(Pkt4Ptr(new Pkt4(not_allowed[i],
+ 1234))))
+ << "Test failed for message type " << i;
+ }
+}
+
}; // end of anonymous namespace
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index 9d0f344..ab1afc6 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -15,7 +15,9 @@
#include <config.h>
#include <asiolink/io_address.h>
+#include <cc/data.h>
#include <config/ccsession.h>
+#include <dhcp4/config_parser.h>
#include <dhcp4/tests/dhcp4_test_utils.h>
#include <dhcp/option4_addrlst.h>
#include <dhcp/option_int_array.h>
@@ -29,14 +31,14 @@
using namespace std;
using namespace isc::asiolink;
-
+using namespace isc::data;
namespace isc {
namespace dhcp {
namespace test {
Dhcpv4SrvTest::Dhcpv4SrvTest()
-:rcode_(-1) {
+:rcode_(-1), srv_(0) {
subnet_ = Subnet4Ptr(new Subnet4(IOAddress("192.0.2.0"), 24, 1000,
2000, 3000));
pool_ = Pool4Ptr(new Pool4(IOAddress("192.0.2.100"), IOAddress("192.0.2.110")));
@@ -553,6 +555,20 @@ Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
EXPECT_TRUE(noBasicOptions(rsp));
}
+void
+Dhcpv4SrvTest::configure(const std::string& config) {
+ ElementPtr json = Element::fromJSON(config);
+ ConstElementPtr status;
+
+ // Configure the server and make sure the config is accepted
+ EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json));
+ ASSERT_TRUE(status);
+ int rcode;
+ ConstElementPtr comment = config::parseAnswer(rcode, status);
+ ASSERT_EQ(0, rcode);
+}
+
+
}; // end of isc::dhcp::test namespace
}; // end of isc::dhcp namespace
}; // end of isc namespace
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index 69e13f0..3f7c326 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -87,6 +87,120 @@ public:
typedef boost::shared_ptr<PktFilterTest> PktFilterTestPtr;
+/// @brief "Naked" DHCPv4 server, exposes internal fields
+class NakedDhcpv4Srv: public Dhcpv4Srv {
+public:
+
+ /// @brief Constructor.
+ ///
+ /// This constructor disables default modes of operation used by the
+ /// Dhcpv4Srv class:
+ /// - Send/receive broadcast messages through sockets on interfaces
+ /// which support broadcast traffic.
+ /// - Direct DHCPv4 traffic - communication with clients which do not
+ /// have IP address assigned yet.
+ ///
+ /// Enabling these modes requires root privilges so they must be
+ /// disabled for unit testing.
+ ///
+ /// Note, that disabling broadcast options on sockets does not impact
+ /// the operation of these tests because they use local loopback
+ /// interface which doesn't have broadcast capability anyway. It rather
+ /// prevents setting broadcast options on other (broadcast capable)
+ /// sockets which are opened on other interfaces in Dhcpv4Srv constructor.
+ ///
+ /// The Direct DHCPv4 Traffic capability can be disabled here because
+ /// it is tested with PktFilterLPFTest unittest. The tests which belong
+ /// to PktFilterLPFTest can be enabled on demand when root privileges can
+ /// be guaranteed.
+ ///
+ /// @param port port number to listen on; the default value 0 indicates
+ /// that sockets should not be opened.
+ NakedDhcpv4Srv(uint16_t port = 0)
+ : Dhcpv4Srv(port, "type=memfile", false, false) {
+ // Create fixed server id.
+ server_id_.reset(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
+ asiolink::IOAddress("192.0.3.1")));
+ }
+
+ /// @brief Returns fixed server identifier assigned to the naked server
+ /// instance.
+ OptionPtr getServerID() const {
+ return (server_id_);
+ }
+
+ /// @brief fakes packet reception
+ /// @param timeout ignored
+ ///
+ /// The method receives all packets queued in receive queue, one after
+ /// another. Once the queue is empty, it initiates the shutdown procedure.
+ ///
+ /// See fake_received_ field for description
+ virtual Pkt4Ptr receivePacket(int /*timeout*/) {
+
+ // If there is anything prepared as fake incoming traffic, use it
+ if (!fake_received_.empty()) {
+ Pkt4Ptr pkt = fake_received_.front();
+ fake_received_.pop_front();
+ return (pkt);
+ }
+
+ // If not, just trigger shutdown and return immediately
+ shutdown();
+ return (Pkt4Ptr());
+ }
+
+ /// @brief fake packet sending
+ ///
+ /// Pretend to send a packet, but instead just store it in fake_send_ list
+ /// where test can later inspect server's response.
+ virtual void sendPacket(const Pkt4Ptr& pkt) {
+ fake_sent_.push_back(pkt);
+ }
+
+ /// @brief adds a packet to fake receive queue
+ ///
+ /// See fake_received_ field for description
+ void fakeReceive(const Pkt4Ptr& pkt) {
+ fake_received_.push_back(pkt);
+ }
+
+ virtual ~NakedDhcpv4Srv() {
+ }
+
+ /// @brief Dummy server identifier option used by various tests.
+ OptionPtr server_id_;
+
+ /// @brief packets we pretend to receive
+ ///
+ /// Instead of setting up sockets on interfaces that change between OSes, it
+ /// is much easier to fake packet reception. This is a list of packets that
+ /// we pretend to have received. You can schedule new packets to be received
+ /// using fakeReceive() and NakedDhcpv4Srv::receivePacket() methods.
+ std::list<Pkt4Ptr> fake_received_;
+
+ std::list<Pkt4Ptr> fake_sent_;
+
+ using Dhcpv4Srv::adjustIfaceData;
+ using Dhcpv4Srv::appendServerID;
+ using Dhcpv4Srv::processDiscover;
+ using Dhcpv4Srv::processRequest;
+ using Dhcpv4Srv::processRelease;
+ using Dhcpv4Srv::processDecline;
+ using Dhcpv4Srv::processInform;
+ using Dhcpv4Srv::processClientName;
+ using Dhcpv4Srv::computeDhcid;
+ using Dhcpv4Srv::createNameChangeRequests;
+ using Dhcpv4Srv::acceptServerId;
+ using Dhcpv4Srv::sanityCheck;
+ using Dhcpv4Srv::srvidToString;
+ using Dhcpv4Srv::unpackOptions;
+ using Dhcpv4Srv::name_change_reqs_;
+ using Dhcpv4Srv::classifyPacket;
+ using Dhcpv4Srv::accept;
+ using Dhcpv4Srv::acceptMessageType;
+};
+
class Dhcpv4SrvTest : public ::testing::Test {
public:
@@ -280,6 +394,11 @@ public:
/// @param msg_type DHCPDISCOVER or DHCPREQUEST
void testDiscoverRequest(const uint8_t msg_type);
+ /// @brief Runs DHCPv4 configuration from the JSON string.
+ ///
+ /// @param config String holding server configuration in JSON format.
+ void configure(const std::string& config);
+
/// @brief This function cleans up after the test.
virtual void TearDown();
@@ -296,119 +415,9 @@ public:
isc::data::ConstElementPtr comment_;
-};
-
-/// @brief "Naked" DHCPv4 server, exposes internal fields
-class NakedDhcpv4Srv: public Dhcpv4Srv {
-public:
+ /// @brief Server object under test.
+ NakedDhcpv4Srv srv_;
- /// @brief Constructor.
- ///
- /// This constructor disables default modes of operation used by the
- /// Dhcpv4Srv class:
- /// - Send/receive broadcast messages through sockets on interfaces
- /// which support broadcast traffic.
- /// - Direct DHCPv4 traffic - communication with clients which do not
- /// have IP address assigned yet.
- ///
- /// Enabling these modes requires root privilges so they must be
- /// disabled for unit testing.
- ///
- /// Note, that disabling broadcast options on sockets does not impact
- /// the operation of these tests because they use local loopback
- /// interface which doesn't have broadcast capability anyway. It rather
- /// prevents setting broadcast options on other (broadcast capable)
- /// sockets which are opened on other interfaces in Dhcpv4Srv constructor.
- ///
- /// The Direct DHCPv4 Traffic capability can be disabled here because
- /// it is tested with PktFilterLPFTest unittest. The tests which belong
- /// to PktFilterLPFTest can be enabled on demand when root privileges can
- /// be guaranteed.
- ///
- /// @param port port number to listen on; the default value 0 indicates
- /// that sockets should not be opened.
- NakedDhcpv4Srv(uint16_t port = 0)
- : Dhcpv4Srv(port, "type=memfile", false, false) {
- // Create fixed server id.
- server_id_.reset(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
- asiolink::IOAddress("192.0.3.1")));
- }
-
- /// @brief Returns fixed server identifier assigned to the naked server
- /// instance.
- OptionPtr getServerID() const {
- return (server_id_);
- }
-
- /// @brief fakes packet reception
- /// @param timeout ignored
- ///
- /// The method receives all packets queued in receive queue, one after
- /// another. Once the queue is empty, it initiates the shutdown procedure.
- ///
- /// See fake_received_ field for description
- virtual Pkt4Ptr receivePacket(int /*timeout*/) {
-
- // If there is anything prepared as fake incoming traffic, use it
- if (!fake_received_.empty()) {
- Pkt4Ptr pkt = fake_received_.front();
- fake_received_.pop_front();
- return (pkt);
- }
-
- // If not, just trigger shutdown and return immediately
- shutdown();
- return (Pkt4Ptr());
- }
-
- /// @brief fake packet sending
- ///
- /// Pretend to send a packet, but instead just store it in fake_send_ list
- /// where test can later inspect server's response.
- virtual void sendPacket(const Pkt4Ptr& pkt) {
- fake_sent_.push_back(pkt);
- }
-
- /// @brief adds a packet to fake receive queue
- ///
- /// See fake_received_ field for description
- void fakeReceive(const Pkt4Ptr& pkt) {
- fake_received_.push_back(pkt);
- }
-
- virtual ~NakedDhcpv4Srv() {
- }
-
- /// @brief Dummy server identifier option used by various tests.
- OptionPtr server_id_;
-
- /// @brief packets we pretend to receive
- ///
- /// Instead of setting up sockets on interfaces that change between OSes, it
- /// is much easier to fake packet reception. This is a list of packets that
- /// we pretend to have received. You can schedule new packets to be received
- /// using fakeReceive() and NakedDhcpv4Srv::receivePacket() methods.
- std::list<Pkt4Ptr> fake_received_;
-
- std::list<Pkt4Ptr> fake_sent_;
-
- using Dhcpv4Srv::adjustIfaceData;
- using Dhcpv4Srv::appendServerID;
- using Dhcpv4Srv::processDiscover;
- using Dhcpv4Srv::processRequest;
- using Dhcpv4Srv::processRelease;
- using Dhcpv4Srv::processDecline;
- using Dhcpv4Srv::processInform;
- using Dhcpv4Srv::processClientName;
- using Dhcpv4Srv::computeDhcid;
- using Dhcpv4Srv::createNameChangeRequests;
- using Dhcpv4Srv::acceptServerId;
- using Dhcpv4Srv::sanityCheck;
- using Dhcpv4Srv::srvidToString;
- using Dhcpv4Srv::unpackOptions;
- using Dhcpv4Srv::name_change_reqs_;
- using Dhcpv4Srv::classifyPacket;
- using Dhcpv4Srv::accept;
};
}; // end of isc::dhcp::test namespace
diff --git a/src/bin/dhcp4/tests/direct_client_unittest.cc b/src/bin/dhcp4/tests/direct_client_unittest.cc
index 694c848..9a9dbae 100644
--- a/src/bin/dhcp4/tests/direct_client_unittest.cc
+++ b/src/bin/dhcp4/tests/direct_client_unittest.cc
@@ -12,7 +12,6 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-#include <cc/data.h>
#include <dhcp/iface_mgr.h>
#include <dhcp/pkt4.h>
#include <dhcp/tests/iface_mgr_test_config.h>
@@ -50,7 +49,9 @@ public:
/// a specified prefix.
///
/// The subnet parameters (such as options, timers etc.) are aribitrarily
- /// selected. The subnet and pool mask is always /24.
+ /// selected. The subnet and pool mask is always /24. The real configuration
+ /// would exclude .0 (network address) and .255 (broadcast address), but we
+ /// ignore that fact for the sake of test simplicity.
///
/// @param prefix Prefix for a subnet.
void configureSubnet(const std::string& prefix);
@@ -59,7 +60,9 @@ public:
///
/// This function configures DHCPv4 server with two different subnets.
/// The subnet parameters (such as options, timers etc.) are aribitrarily
- /// selected. The subnet and pool mask is /24.
+ /// selected. The subnet and pool mask is /24. The real configuration
+ /// would exclude .0 (network address) and .255 (broadcast address), but we
+ /// ignore that fact for the sake of test simplicity.
///
/// @param prefix1 Prefix of the first subnet to be configured.
/// @param prefix2 Prefix of the second subnet to be configured.
@@ -98,18 +101,9 @@ public:
/// @return Configured and parsed message.
Pkt4Ptr createClientMessage(const Pkt4Ptr &msg, const std::string& iface);
- /// @brief Runs DHCPv4 configuration from the JSON string.
- ///
- /// @param config String holding server configuration in JSON format.
- void configure(const std::string& config);
-
- /// @brief Server object to be unit tested.
- NakedDhcpv4Srv srv_;
-
};
-DirectClientTest::DirectClientTest()
- : srv_(0) {
+DirectClientTest::DirectClientTest() : Dhcpv4SrvTest() {
}
void
@@ -187,19 +181,6 @@ DirectClientTest::createClientMessage(const Pkt4Ptr& msg,
return (received);
}
-void
-DirectClientTest::configure(const std::string& config) {
- ElementPtr json = Element::fromJSON(config);
- ConstElementPtr status;
-
- // Configure the server and make sure the config is accepted
- EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json));
- ASSERT_TRUE(status);
- int rcode;
- ConstElementPtr comment = config::parseAnswer(rcode, status);
- ASSERT_EQ(0, rcode);
-}
-
// This test checks that the message from directly connected client
// is processed and that client is offered IPv4 address from the subnet which
// is suitable for the local interface on which the client's message is
diff --git a/src/lib/dhcp/dhcp4.h b/src/lib/dhcp/dhcp4.h
index 392478a..2b24405 100644
--- a/src/lib/dhcp/dhcp4.h
+++ b/src/lib/dhcp/dhcp4.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2004-2011 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2011, 2014 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 1995-2003 by Internet Software Consortium
*
* Permission to use, copy, modify, and distribute this software for any
@@ -154,7 +154,9 @@ enum DHCPMessageType {
DHCPLEASEQUERY = 10,
DHCPLEASEUNASSIGNED = 11,
DHCPLEASEUNKNOWN = 12,
- DHCPLEASEACTIVE = 13
+ DHCPLEASEACTIVE = 13,
+ DHCPBULKLEASEQUERY = 14,
+ DHCPLEASEQUERYDONE = 15
};
static const uint16_t DHCP4_CLIENT_PORT = 68;
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index b641a03..fa0e0f9 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -396,6 +396,7 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
case DHCPRELEASE:
case DHCPINFORM:
case DHCPLEASEQUERY:
+ case DHCPBULKLEASEQUERY:
return (BOOTREQUEST);
case DHCPACK:
@@ -404,6 +405,7 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
case DHCPLEASEUNASSIGNED:
case DHCPLEASEUNKNOWN:
case DHCPLEASEACTIVE:
+ case DHCPLEASEQUERYDONE:
return (BOOTREPLY);
default:
diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am
index 0f2ea1b..d41246a 100644
--- a/src/lib/dhcp/tests/Makefile.am
+++ b/src/lib/dhcp/tests/Makefile.am
@@ -25,6 +25,13 @@ TESTS_ENVIRONMENT = \
TESTS =
if HAVE_GTEST
+# Creates a library which is shared by various unit tests which require
+# configuration of fake interfaces.
+# The libdhcp++ does not link with this library because this would cause
+# build failures being a result of concurrency between build of this
+# library and the unit tests when make -j option was used, as they
+# are built out of the same makefile. Instead, the libdhcp++ tests link to
+# files belonging to this library, directly.
noinst_LTLIBRARIES = libdhcptest.la
libdhcptest_la_SOURCES = iface_mgr_test_config.cc iface_mgr_test_config.h
@@ -39,6 +46,7 @@ TESTS += libdhcp++_unittests
libdhcp___unittests_SOURCES = run_unittests.cc
libdhcp___unittests_SOURCES += hwaddr_unittest.cc
libdhcp___unittests_SOURCES += iface_mgr_unittest.cc
+libdhcp___unittests_SOURCES += iface_mgr_test_config.cc iface_mgr_test_config.h
libdhcp___unittests_SOURCES += libdhcp++_unittest.cc
libdhcp___unittests_SOURCES += option4_addrlst_unittest.cc
libdhcp___unittests_SOURCES += option4_client_fqdn_unittest.cc
@@ -61,6 +69,7 @@ libdhcp___unittests_SOURCES += pkt6_unittest.cc
libdhcp___unittests_SOURCES += pkt_filter_unittest.cc
libdhcp___unittests_SOURCES += pkt_filter_inet_unittest.cc
libdhcp___unittests_SOURCES += pkt_filter_inet6_unittest.cc
+libdhcp___unittests_SOURCES += pkt_filter_test_stub.cc pkt_filter_test_stub.h
libdhcp___unittests_SOURCES += pkt_filter_test_utils.h pkt_filter_test_utils.cc
libdhcp___unittests_SOURCES += pkt_filter6_test_utils.h pkt_filter6_test_utils.cc
@@ -84,7 +93,6 @@ libdhcp___unittests_CXXFLAGS += -Wno-unused-variable -Wno-unused-parameter
endif
libdhcp___unittests_LDADD = $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
-libdhcp___unittests_LDADD += $(top_builddir)/src/lib/dhcp/tests/libdhcptest.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
diff --git a/src/lib/dhcp/tests/iface_mgr_test_config.cc b/src/lib/dhcp/tests/iface_mgr_test_config.cc
index ca219ed..8da7e6e 100644
--- a/src/lib/dhcp/tests/iface_mgr_test_config.cc
+++ b/src/lib/dhcp/tests/iface_mgr_test_config.cc
@@ -98,7 +98,7 @@ IfaceMgrTestConfig::createIfaces() {
// eth1
addIface("eth1", 2);
addAddress("eth1", IOAddress("192.0.2.3"));
- addAddress("eth1", IOAddress("fe80::3a60:77ff:fed5:cdef"));
+ addAddress("eth1", IOAddress("fe80::3a60:77ff:fed5:abcd"));
}
diff --git a/src/lib/dhcp/tests/iface_mgr_test_config.h b/src/lib/dhcp/tests/iface_mgr_test_config.h
index bbf7046..a2ceba5 100644
--- a/src/lib/dhcp/tests/iface_mgr_test_config.h
+++ b/src/lib/dhcp/tests/iface_mgr_test_config.h
@@ -26,6 +26,35 @@ namespace test {
///
/// @name Set of structures describing interface flags.
///
+/// These flags encapsulate the boolean type to pass the flags values
+/// to @c IfaceMgrTestConfig methods. If the values passed to these methods
+/// were not encapsulated by the types defined here, the API would become
+/// prone to errors like swapping parameters being passed to specific functions.
+/// For example, in the call to @c IfaceMgrTestConfig::setIfaceFlags:
+/// @code
+/// IfaceMgrTestConfig test_config(true);
+/// test_config.setIfaceFlags("eth1", false, false, true, false, false);
+/// @endcode
+///
+/// it is quite likely that the developer by mistake swaps the values and
+/// assigns them to wrong flags. When the flags are encapsulated with dedicated
+/// structs, the compiler will return an error if values are swapped. For
+/// example:
+/// @code
+/// IfaceMgrTestConfig test_config(true);
+/// test_config.setIfaceFlags("eth1", FlagLoopback(false), FlagUp(false),
+/// FlagRunning(true), FlagInactive4(false),
+/// FlagInactive6(false));
+/// @endcode
+/// will succeed, but the following code will result in the compilation error
+/// and thus protect a developer from making an error:
+/// @code
+/// IfaceMgrTestConfig test_config(true);
+/// test_config.setIfaceFlags("eth1", FlagLoopback(false),
+/// FlagRunning(true), FlagUp(false),
+/// FlagInactive4(false), FlagInactive6(false));
+/// @endcode
+///
//@{
/// @brief Structure describing the loopback interface flag.
struct FlagLoopback {
@@ -84,6 +113,25 @@ struct FlagInactive6 {
///
/// This class provides a set of convenience functions that should be called
/// by unit tests to configure the @c IfaceMgr with fake interfaces.
+///
+/// The class allows the caller to create custom fake interfaces (with custom
+/// IPv4 and IPv6 addresses, flags etc.), but it also provides a default
+/// test configuration for interfaces as follows:
+/// - lo
+/// - 127.0.0.1
+/// - ::1
+/// - eth0
+/// - 10.0.0.1
+/// - fe80::3a60:77ff:fed5:cdef
+/// - 2001:db8:1::1
+/// - eth1
+/// - 192.0.2.3
+/// - fe80::3a60:77ff:fed5:abcd
+///
+/// For all interfaces the following flags are set:
+/// - multicast
+/// - up
+/// - running
class IfaceMgrTestConfig : public boost::noncopyable {
public:
@@ -106,7 +154,7 @@ public:
///
/// @param iface_name Name of the interface on which new address should
/// be configured.
- /// @param IPv4 or IPv6 address to be configured on the interface.
+ /// @param address IPv4 or IPv6 address to be configured on the interface.
void addAddress(const std::string& iface_name,
const asiolink::IOAddress& address);
@@ -165,6 +213,7 @@ public:
///
/// This function configures interface with new values for flags.
///
+ /// @param name Interface name.
/// @param loopback Specifies if interface is a loopback interface.
/// @param up Specifies if the interface is up.
/// @param running Specifies if the interface is running.
diff --git a/src/lib/dhcp/tests/pkt_filter_test_stub.h b/src/lib/dhcp/tests/pkt_filter_test_stub.h
index db42b07..f8c6130 100644
--- a/src/lib/dhcp/tests/pkt_filter_test_stub.h
+++ b/src/lib/dhcp/tests/pkt_filter_test_stub.h
@@ -65,7 +65,9 @@ public:
/// fallback socket descriptor is set to a negative value.
virtual SocketInfo openSocket(const Iface& iface,
const isc::asiolink::IOAddress& addr,
- const uint16_t port, const bool, const bool);
+ const uint16_t port,
+ const bool receive_bcast,
+ const bool send_bcast);
/// @brief Simulate reception of the DHCPv4 message.
///
diff --git a/src/lib/dhcp/tests/pkt_filter_test_utils.h b/src/lib/dhcp/tests/pkt_filter_test_utils.h
index b2320cf..2ce9dd5 100644
--- a/src/lib/dhcp/tests/pkt_filter_test_utils.h
+++ b/src/lib/dhcp/tests/pkt_filter_test_utils.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -129,7 +129,9 @@ public:
/// fallback socket descriptor is set to a negative value.
virtual SocketInfo openSocket(const Iface& iface,
const isc::asiolink::IOAddress& addr,
- const uint16_t port, const bool, const bool);
+ const uint16_t port,
+ const bool receive_bcast,
+ const bool send_bcast);
/// @brief Simulate reception of the DHCPv4 message.
///
diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc
index b6959db..d846658 100644
--- a/src/lib/dhcpsrv/cfgmgr.cc
+++ b/src/lib/dhcpsrv/cfgmgr.cc
@@ -236,7 +236,7 @@ CfgMgr::getSubnet4(const std::string& iface_name) const {
" doesn't exist and therefore it is impossible"
" to find a suitable subnet for its IPv4 address");
}
- IOAddress addr("::1");
+ IOAddress addr("0.0.0.0");
// If IPv4 address assigned to the interface exists, find a suitable
// subnet for it, else return NULL pointer to indicate that no subnet
// could be found.
diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
index a6223f7..59ef85b 100644
--- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
@@ -718,8 +718,8 @@ TEST_F(CfgMgrTest, getSubnet4ForInterface) {
// Initially, there are no subnets configured, so none of the IPv4
// addresses assigned to eth0 and eth1 can match with any subnet.
- ASSERT_FALSE(CfgMgr::instance().getSubnet4("eth0"));
- ASSERT_FALSE(CfgMgr::instance().getSubnet4("eth1"));
+ EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth0"));
+ EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth1"));
// Configure first subnet which address on eth0 corresponds to.
Subnet4Ptr subnet1(new Subnet4(IOAddress("10.0.0.1"), 24, 1, 2, 3));
More information about the bind10-changes
mailing list