BIND 10 trac3200, updated. 1bbab8c3c1d12e1a7aaa2e2fc36ab4fc5653c92e [3200] Changes after code review.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 22 12:47:32 UTC 2013
The branch, trac3200 has been updated
via 1bbab8c3c1d12e1a7aaa2e2fc36ab4fc5653c92e (commit)
from 5baa1aeb9435170663bcce936e53fbac6d55eef8 (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 1bbab8c3c1d12e1a7aaa2e2fc36ab4fc5653c92e
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Oct 22 14:47:24 2013 +0200
[3200] Changes after code review.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4_messages.mes | 4 +-
src/bin/dhcp4/tests/dhcp4_test_utils.cc | 124 ++++++++++++++++++++-----------
src/bin/dhcp4/tests/dhcp4_test_utils.h | 35 +++++++--
3 files changed, 114 insertions(+), 49 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 5423a21..1bfce66 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -118,7 +118,7 @@ a lease. It is up to the client to choose one server out of othe advertised
and continue allocation with that server. This is a normal behavior and
indicates successful operation.
-% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id %1, hwaddr %2, yiaddr %3
+% DHCP4_LEASE_ADVERT_FAIL failed to advertise a lease for client client-id %1, hwaddr %2, client sent yiaddr %3
This message indicates that the server has failed to offer a lease to
the specified client after receiving a DISCOVER message from it. There are
many possible reasons for such a failure.
@@ -128,7 +128,7 @@ This debug message indicates that the server successfully granted a lease
in response to client's REQUEST message. This is a normal behavior and
indicates successful operation.
-% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2, yiaddr %3
+% DHCP4_LEASE_ALLOC_FAIL failed to grant a lease for client-id %1, hwaddr %2, client sent yiaddr %3
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. Additional messages will indicate the
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index 81416b6..471e0be 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
@@ -124,51 +124,87 @@ void Dhcpv4SrvTest::messageCheck(const Pkt4Ptr& q, const Pkt4Ptr& a) {
EXPECT_TRUE(q->getLocalHWAddr() == a->getLocalHWAddr());
EXPECT_TRUE(q->getRemoteHWAddr() == a->getRemoteHWAddr());
- // Check that bare minimum of required options are there.
- // We don't check options requested by a client. Those
- // are checked elsewhere.
- EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
- EXPECT_TRUE(a->getOption(DHO_ROUTERS));
+ // Check that the server identifier is present in the response.
+ // Presence (or absence) of other options is checked elsewhere.
EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
- EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
- EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
- EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
- EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
// Check that something is offered
EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
}
-void Dhcpv4SrvTest::optionsCheck(const Pkt4Ptr& pkt) {
- // Check that the requested and configured options are returned
- // in the ACK message.
- EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME))
- << "domain-name not present in the response";
- EXPECT_TRUE(pkt->getOption(DHO_DOMAIN_NAME_SERVERS))
- << "dns-servers not present in the response";
- EXPECT_TRUE(pkt->getOption(DHO_LOG_SERVERS))
- << "log-servers not present in the response";
- EXPECT_TRUE(pkt->getOption(DHO_COOKIE_SERVERS))
- << "cookie-servers not present in the response";
- // Check that the requested but not configured options are not
- // returned in the ACK message.
- EXPECT_FALSE(pkt->getOption(DHO_LPR_SERVERS))
- << "domain-name present in the response but it is"
- << " expected not to be present";
+::testing::AssertionResult
+Dhcpv4SrvTest::basicOptionsPresent(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option missing in the response";
+ if (!pkt->getOption(DHO_DOMAIN_NAME)) {
+ return (::testing::AssertionFailure() << "domain-name " << errmsg);
+
+ } else if (!pkt->getOption(DHO_DOMAIN_NAME_SERVERS)) {
+ return (::testing::AssertionFailure() << "dns-servers " << errmsg);
+
+ } else if (!pkt->getOption(DHO_SUBNET_MASK)) {
+ return (::testing::AssertionFailure() << "subnet-mask " << errmsg);
+
+ } else if (!pkt->getOption(DHO_ROUTERS)) {
+ return (::testing::AssertionFailure() << "routers " << errmsg);
+
+ } else if (!pkt->getOption(DHO_DHCP_LEASE_TIME)) {
+ return (::testing::AssertionFailure() << "dhcp-lease-time " << errmsg);
+
+ }
+ return (::testing::AssertionSuccess());
+
}
-void Dhcpv4SrvTest::noOptionsCheck(const Pkt4Ptr& pkt) {
- // Check that certain options are not returned in the packet.
- // This is the case, when client didn't ask for them or when
- // NAK was returned by the server.
- EXPECT_FALSE(pkt->getOption(DHO_DOMAIN_NAME))
- << "domain-name present in the response";
- EXPECT_FALSE(pkt->getOption(DHO_DOMAIN_NAME_SERVERS))
- << "dns-servers present in the response";
- EXPECT_FALSE(pkt->getOption(DHO_LOG_SERVERS))
- << "log-servers present in the response";
- EXPECT_FALSE(pkt->getOption(DHO_COOKIE_SERVERS))
- << "cookie-servers present in the response";
+::testing::AssertionResult
+Dhcpv4SrvTest::noBasicOptions(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option present in the response";
+ if (pkt->getOption(DHO_DOMAIN_NAME)) {
+ return (::testing::AssertionFailure() << "domain-name " << errmsg);
+
+ } else if (pkt->getOption(DHO_DOMAIN_NAME_SERVERS)) {
+ return (::testing::AssertionFailure() << "dns-servers " << errmsg);
+
+ } else if (pkt->getOption(DHO_SUBNET_MASK)) {
+ return (::testing::AssertionFailure() << "subnet-mask " << errmsg);
+
+ } else if (pkt->getOption(DHO_ROUTERS)) {
+ return (::testing::AssertionFailure() << "routers " << errmsg);
+
+ } else if (pkt->getOption(DHO_DHCP_LEASE_TIME)) {
+ return (::testing::AssertionFailure() << "dhcp-lease-time " << errmsg);
+
+ }
+ return (::testing::AssertionSuccess());
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::requestedOptionsPresent(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option missing in the response";
+ if (!pkt->getOption(DHO_LOG_SERVERS)) {
+ return (::testing::AssertionFailure() << "log-servers " << errmsg);
+
+ } else if (!pkt->getOption(DHO_COOKIE_SERVERS)) {
+ return (::testing::AssertionFailure() << "cookie-servers " << errmsg);
+
+ }
+ return (::testing::AssertionSuccess());
+}
+
+::testing::AssertionResult
+Dhcpv4SrvTest::noRequestedOptions(const Pkt4Ptr& pkt) {
+ std::ostringstream errmsg;
+ errmsg << "option present in the response";
+ if (pkt->getOption(DHO_LOG_SERVERS)) {
+ return (::testing::AssertionFailure() << "log-servers " << errmsg);
+
+ } else if (pkt->getOption(DHO_COOKIE_SERVERS)) {
+ return (::testing::AssertionFailure() << "cookie-servers " << errmsg);
+
+ }
+ return (::testing::AssertionSuccess());
}
OptionPtr Dhcpv4SrvTest::generateClientId(size_t size /*= 4*/) {
@@ -397,11 +433,11 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
messageCheck(received, rsp);
+ // Basic options should be present when we got the lease.
+ EXPECT_TRUE(basicOptionsPresent(rsp));
// We did not request any options so these should not be present
// in the RSP.
- EXPECT_FALSE(rsp->getOption(DHO_LOG_SERVERS));
- EXPECT_FALSE(rsp->getOption(DHO_COOKIE_SERVERS));
- EXPECT_FALSE(rsp->getOption(DHO_LPR_SERVERS));
+ EXPECT_TRUE(noRequestedOptions(rsp));
// Repeat the test but request some options.
// Add 'Parameter Request List' option.
@@ -426,7 +462,8 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
}
// Check that the requested options are returned.
- optionsCheck(rsp);
+ EXPECT_TRUE(basicOptionsPresent(rsp));
+ EXPECT_TRUE(requestedOptionsPresent(rsp));
// The following part of the test will test that the NAK is sent when
// there is no address pool configured. In the same time, we expect
@@ -456,7 +493,10 @@ void Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
ASSERT_EQ("0.0.0.0", rsp->getYiaddr().toText());
// Make sure that none of the requested options is returned in NAK.
- noOptionsCheck(rsp);
+ // Also options such as Routers or Subnet Mask should not be there,
+ // because lease hasn't been acquired.
+ EXPECT_TRUE(noRequestedOptions(rsp));
+ EXPECT_TRUE(noBasicOptions(rsp));
}
/// @brief This function cleans up after the test.
diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index 97eaaff..ee19728 100644
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -109,15 +109,32 @@ public:
/// @param a answer (server's message)
void messageCheck(const Pkt4Ptr& q, const Pkt4Ptr& a);
- /// @brief Check that requested options are present.
+ /// @brief Check that certain basic (always added when lease is acquired)
+ /// are present in a message.
///
- /// @param pkt packet to be checked.
- void optionsCheck(const Pkt4Ptr& pkt);
+ /// @param pkt A message to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult basicOptionsPresent(const Pkt4Ptr& pkt);
- /// @brief Check that certain options are not present.
+ /// @brief Check that certain basic (always added when lease is acquired)
+ /// are not present.
///
/// @param pkt A packet to be checked.
- void noOptionsCheck(const Pkt4Ptr& pkt);
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult noBasicOptions(const Pkt4Ptr& pkt);
+
+ /// @brief Check that certain requested options are present in the message.
+ ///
+ /// @param pkt A message to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult requestedOptionsPresent(const Pkt4Ptr& pkt);
+
+ /// @brief Check that certain options (requested with PRL option)
+ /// are not present.
+ ///
+ /// @param pkt A packet to be checked.
+ /// @return Assertion result which indicates whether test passed or failed.
+ ::testing::AssertionResult noRequestedOptions(const Pkt4Ptr& pkt);
/// @brief generates client-id option
///
@@ -221,6 +238,14 @@ public:
/// @brief Tests if Discover or Request message is processed correctly
///
+ /// This test verifies that the Parameter Request List option is handled
+ /// correctly, i.e. it checks that certain options are present in the
+ /// server's response when they are requested and that they are not present
+ /// when they are not requested or NAK occurs.
+ ///
+ /// @todo We need an additional test for PRL option using real traffic
+ /// capture.
+ ///
/// @param msg_type DHCPDISCOVER or DHCPREQUEST
void testDiscoverRequest(const uint8_t msg_type);
More information about the bind10-changes
mailing list