BIND 10 trac3191, updated. d8d20c9781e6a644c85339b966c5c6a31ad24a59 [3191] Changes after review
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Oct 22 13:16:29 UTC 2013
The branch, trac3191 has been updated
via d8d20c9781e6a644c85339b966c5c6a31ad24a59 (commit)
from 28c612e504efd4f340fa1d9929ea17cd75565a67 (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 d8d20c9781e6a644c85339b966c5c6a31ad24a59
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue Oct 22 15:16:17 2013 +0200
[3191] Changes after review
- default value is no longer 0.0.0.0, so global value is no longer
overwritten with subnet-specific values
- added 2 new unit-tests to check if the value is really set in
server responses
- guide updated to explain that
- fix in Dhcp4ParserTest.nextServerNegative
-----------------------------------------------------------------------
Summary of changes:
doc/guide/bind10-guide.xml | 3 +
src/bin/dhcp4/config_parser.cc | 8 ++-
src/bin/dhcp4/dhcp4.spec | 2 +-
src/bin/dhcp4/tests/config_parser_unittest.cc | 10 ++-
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 84 +++++++++++++++++++++++++
5 files changed, 102 insertions(+), 5 deletions(-)
-----------------------------------------------------------------------
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index fe166ee..99b2e0c 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -4395,6 +4395,9 @@ Dhcp4/subnet4 [] list (default)
in the DHCPv4 packet for that purpose. That specific field can be configured
using next-server directive. It is possible to define it in global scope or
for a given subnet only. If both are defined, subnet value takes precedence.
+ The value in subnet can be set to 0.0.0.0, which means that next-server should
+ not be sent. It may also be set to empty string, which means the same as if
+ it was not defined at all - use global value.
</para>
<screen>
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index 9c7ca51..3c8ac26 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -270,7 +270,9 @@ protected:
// Try global value first
try {
string next_server = globalContext()->string_values_->getParam("next-server");
- subnet4->setSiaddr(IOAddress(next_server));
+ if (!next_server.empty()) {
+ subnet4->setSiaddr(IOAddress(next_server));
+ }
} catch (const DhcpConfigError&) {
// Don't care. next_server is optional. We can live without it
}
@@ -278,7 +280,9 @@ protected:
// Try subnet specific value if it's available
try {
string next_server = string_values_->getParam("next-server");
- subnet4->setSiaddr(IOAddress(next_server));
+ if (!next_server.empty()) {
+ subnet4->setSiaddr(IOAddress(next_server));
+ }
} catch (const DhcpConfigError&) {
// Don't care. next_server is optional. We can live without it
}
diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec
index 6fe26d6..b507159 100644
--- a/src/bin/dhcp4/dhcp4.spec
+++ b/src/bin/dhcp4/dhcp4.spec
@@ -51,7 +51,7 @@
{ "item_name": "next-server",
"item_type": "string",
"item_optional": true,
- "item_default": "0.0.0.0"
+ "item_default": ""
},
{ "item_name": "option-def",
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 162c837..d5721c4 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -481,6 +481,8 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) {
"\"renew-timer\": 1000, "
"\"subnet4\": [ { "
" \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"rebind-timer\": 2000, "
+ " \"renew-timer\": 1000, "
" \"next-server\": \"a.b.c.d\", "
" \"subnet\": \"192.0.2.0/24\" } ],"
"\"valid-lifetime\": 4000 }";
@@ -491,6 +493,8 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) {
"\"renew-timer\": 1000, "
"\"subnet4\": [ { "
" \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"rebind-timer\": 2000, "
+ " \"renew-timer\": 1000, "
" \"next-server\": \"2001:db8::1\", "
" \"subnet\": \"192.0.2.0/24\" } ],"
"\"valid-lifetime\": 4000 }";
@@ -501,13 +505,15 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) {
"\"renew-timer\": 1000, "
"\"subnet4\": [ { "
" \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"rebind-timer\": 2000, "
+ " \"renew-timer\": 1000, "
" \"next-server\": \"\", "
" \"subnet\": \"192.0.2.0/24\" } ],"
"\"valid-lifetime\": 4000 }";
ElementPtr json1 = Element::fromJSON(config_bogus1);
ElementPtr json2 = Element::fromJSON(config_bogus2);
- ElementPtr json3 = Element::fromJSON(config_bogus2);
+ ElementPtr json3 = Element::fromJSON(config_bogus3);
// check if returned status is always a failure
EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json1));
@@ -517,7 +523,7 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) {
checkResult(status, 1);
EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json3));
- checkResult(status, 1);
+ checkResult(status, 0);
}
// Checks if the next-server defined as global value is overridden by subnet
diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 409509b..e482a1c 100644
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -1339,6 +1339,90 @@ TEST_F(Dhcpv4SrvTest, siaddr) {
EXPECT_EQ("192.0.2.123", offer->getSiaddr().toText());
}
+// Checks if the next-server defined as global value is overridden by subnet
+// specific value and returned in server messagey. There's also similar test for
+// checking parser only configuration, see Dhcp4ParserTest.nextServerOverride in
+// config_parser_unittest.cc.
+TEST_F(Dhcpv4SrvTest, nextServerOverride) {
+
+ NakedDhcpv4Srv srv(0);
+
+ ConstElementPtr status;
+
+ string config = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"next-server\": \"192.0.0.1\", "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"next-server\": \"1.2.3.4\", "
+ " \"subnet\": \"192.0.2.0/24\" } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ ElementPtr json = Element::fromJSON(config);
+
+ EXPECT_NO_THROW(status = configureDhcp4Server(srv, json));
+
+ // check if returned status is OK
+ ASSERT_TRUE(status);
+ comment_ = config::parseAnswer(rcode_, status);
+ ASSERT_EQ(0, rcode_);
+
+ 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 offer
+ Pkt4Ptr offer = srv.processDiscover(dis);
+ ASSERT_TRUE(offer);
+ EXPECT_EQ(DHCPOFFER, offer->getType());
+
+ EXPECT_EQ("1.2.3.4", offer->getSiaddr().toText());
+}
+
+// Checks if the next-server defined as global value is used in responses
+// when there is no specific value defined in subnet and returned to the client
+// properly. There's also similar test for checking parser only configuration,
+// see Dhcp4ParserTest.nextServerGlobal in config_parser_unittest.cc.
+TEST_F(Dhcpv4SrvTest, nextServerGlobal) {
+
+ NakedDhcpv4Srv srv(0);
+
+ ConstElementPtr status;
+
+ string config = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"next-server\": \"192.0.0.1\", "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\" } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ ElementPtr json = Element::fromJSON(config);
+
+ EXPECT_NO_THROW(status = configureDhcp4Server(srv, json));
+
+ // check if returned status is OK
+ ASSERT_TRUE(status);
+ comment_ = config::parseAnswer(rcode_, status);
+ ASSERT_EQ(0, rcode_);
+
+ 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 offer
+ Pkt4Ptr offer = srv.processDiscover(dis);
+ ASSERT_TRUE(offer);
+ EXPECT_EQ(DHCPOFFER, offer->getType());
+
+ EXPECT_EQ("192.0.0.1", offer->getSiaddr().toText());
+}
+
+
// a dummy MAC address
const uint8_t dummyMacAddr[] = {0, 1, 2, 3, 4, 5};
More information about the bind10-changes
mailing list