BIND 10 trac2591, updated. 933430e40bb35135d741df5b9c8b944c5bf9be83 [2591] Code cleanup in Dhcp4SrvTest.processRequest and .processDiscover.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 16 10:38:39 UTC 2013
The branch, trac2591 has been updated
via 933430e40bb35135d741df5b9c8b944c5bf9be83 (commit)
from a0ac798d243153455ce1332a8ef3e1363b33273c (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 933430e40bb35135d741df5b9c8b944c5bf9be83
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Jan 16 11:38:25 2013 +0100
[2591] Code cleanup in Dhcp4SrvTest.processRequest and .processDiscover.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 213 ++++++++++++++++++++++-------
src/lib/dhcpsrv/subnet.cc | 2 +-
src/lib/dhcpsrv/subnet.h | 4 +-
3 files changed, 165 insertions(+), 54 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index b4fca01..2fe4c33 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -72,6 +72,73 @@ public:
CfgMgr::instance().addSubnet4(subnet_);
}
+ /// @brief Add 'Paramter Request List' option to the packet.
+ ///
+ /// This function PRL option comprising the following
+ /// option codes:
+ /// - 5 - Name Server
+ /// - 15 - Domain Name
+ /// - 7 - Log Server
+ /// - 8 - Quotes Server
+ /// - 9 - LPR Server
+ ///
+ /// @param pkt packet to add PRL option to.
+ void addPrlOption(Pkt4Ptr& pkt) {
+
+ OptionUint8ArrayPtr option_prl =
+ OptionUint8ArrayPtr(new OptionUint8Array(Option::V4,
+ DHO_DHCP_PARAMETER_REQUEST_LIST));
+
+ std::vector<uint8_t> opts;
+ // Let's request options that have been configured for the subnet.
+ opts.push_back(DHO_DOMAIN_NAME_SERVERS);
+ opts.push_back(DHO_DOMAIN_NAME);
+ opts.push_back(DHO_LOG_SERVERS);
+ opts.push_back(DHO_COOKIE_SERVERS);
+ // Let's also request the option that hasn't been configured. In such
+ // case server should ignore request for this particular option.
+ opts.push_back(DHO_LPR_SERVERS);
+ // Put the requested option codes into the 'Parameter Request List'.
+ option_prl->setValues(opts);
+ // And add 'Parameter Request List' option into the DISCOVER packet.
+ pkt->addOption(option_prl);
+ }
+
+ /// @brief Configures options being requested in the PRL option.
+ ///
+ /// The lpr-servers option is NOT configured here altough it is
+ /// added to the 'Parameter Request List' option in the
+ /// \ref addPrlOption. When requested option is not configured
+ /// the server should not return it in its rensponse. The goal
+ /// of not configuring the requested option is to verify that
+ /// the server will not return it.
+ void configureRequestedOptions() {
+ // dns-servers
+ Option4AddrLstPtr
+ option_dns_servers(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS));
+ option_dns_servers->addAddress(IOAddress("192.0.2.1"));
+ option_dns_servers->addAddress(IOAddress("192.0.2.100"));
+ ASSERT_NO_THROW(subnet_->addOption(option_dns_servers, false, "dhcp4"));
+
+ // domain-name
+ OptionDefinition def("domain-name", DHO_DOMAIN_NAME, OPT_FQDN_TYPE);
+ boost::shared_ptr<OptionCustom>
+ option_domain_name(new OptionCustom(def, Option::V4));
+ option_domain_name->writeFqdn("example.com");
+ subnet_->addOption(option_domain_name, false, "dhcp4");
+
+ // log-servers
+ Option4AddrLstPtr option_log_servers(new Option4AddrLst(DHO_LOG_SERVERS));
+ option_log_servers->addAddress(IOAddress("192.0.2.2"));
+ option_log_servers->addAddress(IOAddress("192.0.2.10"));
+ ASSERT_NO_THROW(subnet_->addOption(option_log_servers, false, "dhcp4"));
+
+ // cookie-servers
+ Option4AddrLstPtr option_cookie_servers(new Option4AddrLst(DHO_COOKIE_SERVERS));
+ option_cookie_servers->addAddress(IOAddress("192.0.2.1"));
+ ASSERT_NO_THROW(subnet_->addOption(option_cookie_servers, false, "dhcp4"));
+ }
+
/// @brief checks that the response matches request
/// @param q query (client's message)
/// @param a answer (server's message)
@@ -85,20 +152,41 @@ 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.
+ // 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));
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_ROUTERS));
- 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");
}
+ /// @brief Check that requested options are present.
+ ///
+ /// @param pkt packet to be checked.
+ void 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";
+ }
+
/// @brief generates client-id option
///
/// Generate client-id option of specified length
@@ -316,47 +404,10 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
pkt->setRemotePort(DHCP4_SERVER_PORT);
// We are going to test that certain options are returned
- // in the OFFER message when requested using 'Parameter
- // Request List' option. Let's configure those options that
- // are returned when requested.
-
- // dns-servers
- OptionPtr option_dns_servers(new Option4AddrLst(DHO_DOMAIN_NAME_SERVERS));
- ASSERT_NO_THROW(subnet_->addOption(option_dns_servers, false, "dhcp4"));
- // domain-name
- OptionDefinition def("domain-name", DHO_DOMAIN_NAME, OPT_FQDN_TYPE);
- boost::shared_ptr<OptionCustom>
- option_domain_name(new OptionCustom(def, Option::V4));
- option_domain_name->writeFqdn("example.com");
- OptionPtr opt_domain = boost::dynamic_pointer_cast<Option>(option_domain_name);
- subnet_->addOption(opt_domain, false, "dhcp4");
- // log-servers
- OptionPtr option_log_servers(new Option4AddrLst(DHO_LOG_SERVERS));
- ASSERT_NO_THROW(subnet_->addOption(option_log_servers, false, "dhcp4"));
- // cookie-servers
- OptionPtr option_cookie_servers(new Option4AddrLst(DHO_COOKIE_SERVERS));
- ASSERT_NO_THROW(subnet_->addOption(option_cookie_servers, false, "dhcp4"));
-
- // Add 'Parameter Request List' option. In this option we are going
- // specify which options we request to be retured in the OFFER
- // message.
- OptionUint8ArrayPtr option_prl =
- OptionUint8ArrayPtr(new OptionUint8Array(Option::V4,
- DHO_DHCP_PARAMETER_REQUEST_LIST));
-
- std::vector<uint8_t> opts;
- // Let's request options that have been configured for the subnet.
- opts.push_back(DHO_DOMAIN_NAME_SERVERS);
- opts.push_back(DHO_DOMAIN_NAME);
- opts.push_back(DHO_LOG_SERVERS);
- opts.push_back(DHO_COOKIE_SERVERS);
- // Let's also request the option that hasn't been configured. In such
- // case server should ignore request for this particular option.
- opts.push_back(DHO_LPR_SERVERS);
- // Put the requested option codes into the 'Parameter Request List'.
- option_prl->setValues(opts);
- // And add 'Parameter Request List' option into the DISCOVER packet.
- pkt->addOption(option_prl);
+ // (or not returned) in the OFFER message when requested
+ // using 'Parameter Request List' option. Let's configure
+ // those options that are returned when requested.
+ configureRequestedOptions();
// Should not throw
EXPECT_NO_THROW(
@@ -373,6 +424,35 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
messageCheck(pkt, offer);
+ // We did not request any options so they should not be present
+ // in the OFFER.
+ EXPECT_FALSE(offer->getOption(DHO_DOMAIN_NAME));
+ EXPECT_FALSE(offer->getOption(DHO_DOMAIN_NAME_SERVERS));
+ EXPECT_FALSE(offer->getOption(DHO_LOG_SERVERS));
+ EXPECT_FALSE(offer->getOption(DHO_COOKIE_SERVERS));
+ EXPECT_FALSE(offer->getOption(DHO_LPR_SERVERS));
+
+ // Add 'Parameter Request List' option.
+ addPrlOption(pkt);
+
+ // Now repeat the test but request some options.
+ EXPECT_NO_THROW(
+ offer = srv->processDiscover(pkt);
+ );
+
+ // Should return something
+ ASSERT_TRUE(offer);
+
+ EXPECT_EQ(DHCPOFFER, offer->getType());
+
+ // This is relayed message. It should be sent back to relay address.
+ EXPECT_EQ(pkt->getGiaddr(), offer->getRemoteAddr());
+
+ messageCheck(pkt, offer);
+
+ // Check that the requested options are returned.
+ optionsCheck(offer);
+
// Now repeat the test for directly sent message
pkt->setHops(0);
pkt->setGiaddr(IOAddress("0.0.0.0"));
@@ -393,13 +473,8 @@ TEST_F(Dhcpv4SrvTest, processDiscover) {
messageCheck(pkt, offer);
- // Check that the requested and configured options are returned
- // in the OFFER message.
- EXPECT_TRUE(offer->getOption(DHO_LOG_SERVERS));
- EXPECT_TRUE(offer->getOption(DHO_COOKIE_SERVERS));
- // Check that the requested but not configured options are not
- // returned in the OFFER message.
- EXPECT_FALSE(offer->getOption(DHO_LPR_SERVERS));
+ // Check that the requested options are returned.
+ optionsCheck(offer);
delete srv;
}
@@ -427,6 +502,12 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
req->setRemoteAddr(IOAddress("192.0.2.56"));
req->setGiaddr(IOAddress("192.0.2.67"));
+ // We are going to test that certain options are returned
+ // in the ACK message when requested using 'Parameter
+ // Request List' option. Let's configure those options that
+ // are returned when requested.
+ configureRequestedOptions();
+
// Should not throw
ASSERT_NO_THROW(
ack = srv->processRequest(req);
@@ -442,6 +523,33 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
messageCheck(req, ack);
+ // We did not request any options so they should not be present
+ // in the ACK.
+ EXPECT_FALSE(ack->getOption(DHO_DOMAIN_NAME));
+ EXPECT_FALSE(ack->getOption(DHO_DOMAIN_NAME_SERVERS));
+ EXPECT_FALSE(ack->getOption(DHO_LOG_SERVERS));
+ EXPECT_FALSE(ack->getOption(DHO_COOKIE_SERVERS));
+ EXPECT_FALSE(ack->getOption(DHO_LPR_SERVERS));
+
+ // Add 'Parameter Request List' option.
+ addPrlOption(req);
+
+ // Repeat the test but request some options.
+ ASSERT_NO_THROW(
+ ack = srv->processRequest(req);
+ );
+
+ // Should return something
+ ASSERT_TRUE(ack);
+
+ EXPECT_EQ(DHCPACK, ack->getType());
+
+ // This is relayed message. It should be sent back to relay address.
+ EXPECT_EQ(req->getGiaddr(), ack->getRemoteAddr());
+
+ // Check that the requested options are returned.
+ optionsCheck(ack);
+
// Now repeat the test for directly sent message
req->setHops(0);
req->setGiaddr(IOAddress("0.0.0.0"));
@@ -462,6 +570,9 @@ TEST_F(Dhcpv4SrvTest, processRequest) {
messageCheck(req, ack);
+ // Check that the requested options are returned.
+ optionsCheck(ack);
+
delete srv;
}
diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc
index 6414f88..436fd49 100644
--- a/src/lib/dhcpsrv/subnet.cc
+++ b/src/lib/dhcpsrv/subnet.cc
@@ -44,7 +44,7 @@ bool Subnet::inRange(const isc::asiolink::IOAddress& addr) const {
}
void
-Subnet::addOption(OptionPtr& option, bool persistent,
+Subnet::addOption(const OptionPtr& option, bool persistent,
const std::string& option_space) {
// @todo Once the #2313 is merged we need to use the OptionSpace object to
// validate the option space name here. For now, let's check that the name
diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h
index b864321..e1bea05 100644
--- a/src/lib/dhcpsrv/subnet.h
+++ b/src/lib/dhcpsrv/subnet.h
@@ -69,7 +69,7 @@ public:
///
/// @param opt option
/// @param persist if true option is always sent.
- OptionDescriptor(OptionPtr& opt, bool persist)
+ OptionDescriptor(const OptionPtr& opt, bool persist)
: option(opt), persistent(persist) {};
/// @brief Constructor
@@ -225,7 +225,7 @@ public:
/// @param option_space name of the option space to add an option to.
///
/// @throw isc::BadValue if invalid option provided.
- void addOption(OptionPtr& option, bool persistent,
+ void addOption(const OptionPtr& option, bool persistent,
const std::string& option_space);
/// @brief Delete all options configured for the subnet.
More information about the bind10-changes
mailing list