BIND 10 trac3234, updated. 31e416087685a6dadc3047fdbb0927bbf60095aa [3234] Changes after review:
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 8 14:28:10 UTC 2014
The branch, trac3234 has been updated
via 31e416087685a6dadc3047fdbb0927bbf60095aa (commit)
from 09cfa792bffe8ffa9106c915334ef539577fb62d (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 31e416087685a6dadc3047fdbb0927bbf60095aa
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Wed Jan 8 15:27:44 2014 +0100
[3234] Changes after review:
- ChangeLog updated
- Current reconfiguration limitations described in User's Guide
- Dhcp{4,6}ParserTest.reconfigureRemoveSubnet added
- Subnet::getNextID -> generateNextID
- Various comments improved
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 2 +-
doc/guide/bind10-guide.xml | 31 ++++++
src/bin/dhcp4/tests/config_parser_unittest.cc | 123 ++++++++++++++++++++++++
src/bin/dhcp6/tests/config_parser_unittest.cc | 125 +++++++++++++++++++++++++
src/lib/dhcpsrv/subnet.cc | 2 +-
src/lib/dhcpsrv/subnet.h | 18 +++-
6 files changed, 297 insertions(+), 4 deletions(-)
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index f9e71d5..c27d1ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,5 @@
7XX. [bug] tomek
- b10-dhcp4,b10-dhcp6: Both servers used to unnecessarily increase
+ b10-dhcp4, b10-dhcp6: Both servers used to unnecessarily increase
subnet-id values after reconfiguration. The subnet-ids are now reset
to 1 every time a server is reconfigured.
(Trac #3234, git abcd)
diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml
index d915e27..13d017f 100644
--- a/doc/guide/bind10-guide.xml
+++ b/doc/guide/bind10-guide.xml
@@ -4476,6 +4476,21 @@ Dhcp4/subnet4 [] list (default)
development and should be treated as <quote>not implemented
yet</quote>, rather than actual limitations.</para>
<itemizedlist>
+ <listitem> <!-- see tickets #3234, #3281 -->
+ <para>
+ On-line configuration has some limitations. Adding new subnets or
+ modifying existing ones work, as is removing the last subnet from
+ the list. However, removing non-last (e.g. removing subnet 1,2 or 3 if
+ there are 4 subnets configured) will cause issues. The problem is
+ caused by simplistic subnet-id assignment. The subnets are always
+ numbered, starting from 1. That subnet-id is then used in leases
+ that are stored in the lease database. Removing non-last subnet will
+ cause the configuration information to mismatch data in the lease
+ database. It is possible to manually update subnet-id fields in
+ MySQL database, but it is awkward and error prone process. A better
+ reconfiguration support is planned.
+ </para>
+ </listitem>
<listitem>
<para>
On startup, the DHCPv4 server does not get the full configuration from
@@ -5389,6 +5404,22 @@ should include options from the isc option space:
yet</quote>, rather than actual limitations.</para>
<itemizedlist>
+ <listitem> <!-- see tickets #3234, #3281 -->
+ <para>
+ On-line configuration has some limitations. Adding new subnets or
+ modifying existing ones work, as is removing the last subnet from
+ the list. However, removing non-last (e.g. removing subnet 1,2 or 3 if
+ there are 4 subnets configured) will cause issues. The problem is
+ caused by simplistic subnet-id assignment. The subnets are always
+ numbered, starting from 1. That subnet-id is then used in leases
+ that are stored in the lease database. Removing non-last subnet will
+ cause the configuration information to mismatch data in the lease
+ database. It is possible to manually update subnet-id fields in
+ MySQL database, but it is awkward and error prone process. A better
+ reconfiguration support is planned.
+ </para>
+ </listitem>
+
<listitem>
<para>
On startup, the DHCPv6 server does not get the full configuration from
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index e7a034d..433a612 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -475,6 +475,129 @@ TEST_F(Dhcp4ParserTest, multipleSubnets) {
} while (++cnt < 10);
}
+// Goal of this test is to verify that a previously configured subnet can be
+// deleted in subsequent reconfiguration.
+TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
+ ConstElementPtr x;
+
+ // All four subnets
+ string config4 = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
+ " \"subnet\": \"192.0.3.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
+ " \"subnet\": \"192.0.4.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.5.101 - 192.0.5.150\" ],"
+ " \"subnet\": \"192.0.5.0/24\" "
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ // Three subnets (the last one removed)
+ string config_first3 = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
+ " \"subnet\": \"192.0.3.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
+ " \"subnet\": \"192.0.4.0/24\" "
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ // Second subnet removed
+ string config_second_removed = "{ \"interfaces\": [ \"*\" ],"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet4\": [ { "
+ " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+ " \"subnet\": \"192.0.2.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
+ " \"subnet\": \"192.0.4.0/24\" "
+ " },"
+ " {"
+ " \"pool\": [ \"192.0.5.101 - 192.0.5.150\" ],"
+ " \"subnet\": \"192.0.5.0/24\" "
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ // CASE 1: Configure 4 subnets, then reconfigure and remove the
+ // last one.
+
+ ElementPtr json = Element::fromJSON(config4);
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4();
+ ASSERT_TRUE(subnets);
+ ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
+
+ // Do the reconfiguration (the last subnet is removed)
+ json = Element::fromJSON(config_first3);
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ subnets = CfgMgr::instance().getSubnets4();
+ ASSERT_TRUE(subnets);
+ ASSERT_EQ(3, subnets->size()); // We expect 3 subnets now (4th is removed)
+
+ // Check subnet-ids of each subnet (it should be monotonously increasing)
+ EXPECT_EQ(1, subnets->at(0)->getID());
+ EXPECT_EQ(2, subnets->at(1)->getID());
+ EXPECT_EQ(3, subnets->at(2)->getID());
+
+ /// CASE 2: Configure 4 subnets, then reconfigure and remove one
+ /// from in between (not first, not last)
+
+#if 0
+ /// @todo: Uncomment subnet removal test as part of #3281.
+ json = Element::fromJSON(config4);
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ // Do reconfiguration
+ json = Element::fromJSON(config_second_removed);
+ EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ subnets = CfgMgr::instance().getSubnets4();
+ ASSERT_TRUE(subnets);
+ ASSERT_EQ(3, subnets->size()); // We expect 4 subnets
+
+ EXPECT_EQ(1, subnets->at(0)->getID());
+ // The second subnet (with subnet-id = 2) is no longer there
+ EXPECT_EQ(3, subnets->at(1)->getID());
+ EXPECT_EQ(4, subnets->at(2)->getID());
+#endif
+
+}
+
+/// @todo: implement subnet removal test as part of #3281.
// Checks if the next-server defined as global parameter is taken into
// consideration.
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index b613095..84dd5b7 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -495,6 +495,131 @@ TEST_F(Dhcp6ParserTest, multipleSubnets) {
} while (++cnt < 10);
}
+// Goal of this test is to verify that a previously configured subnet can be
+// deleted in subsequent reconfiguration.
+TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
+ ConstElementPtr x;
+
+ // All four subnets
+ string config4 = "{ \"interfaces\": [ \"*\" ],"
+ "\"preferred-lifetime\": 3000,"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet6\": [ { "
+ " \"pool\": [ \"2001:db8:1::/80\" ],"
+ " \"subnet\": \"2001:db8:1::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:2::/80\" ],"
+ " \"subnet\": \"2001:db8:2::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:3::/80\" ],"
+ " \"subnet\": \"2001:db8:3::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:4::/80\" ],"
+ " \"subnet\": \"2001:db8:4::/64\" "
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ // Three subnets (the last one removed)
+ string config_first3 = "{ \"interfaces\": [ \"*\" ],"
+ "\"preferred-lifetime\": 3000,"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet6\": [ { "
+ " \"pool\": [ \"2001:db8:1::/80\" ],"
+ " \"subnet\": \"2001:db8:1::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:2::/80\" ],"
+ " \"subnet\": \"2001:db8:2::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:3::/80\" ],"
+ " \"subnet\": \"2001:db8:3::/64\" "
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ // Second subnet removed
+ string config_second_removed = "{ \"interfaces\": [ \"*\" ],"
+ "\"preferred-lifetime\": 3000,"
+ "\"rebind-timer\": 2000, "
+ "\"renew-timer\": 1000, "
+ "\"subnet6\": [ { "
+ " \"pool\": [ \"2001:db8:1::/80\" ],"
+ " \"subnet\": \"2001:db8:1::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:3::/80\" ],"
+ " \"subnet\": \"2001:db8:3::/64\" "
+ " },"
+ " {"
+ " \"pool\": [ \"2001:db8:4::/80\" ],"
+ " \"subnet\": \"2001:db8:4::/64\" "
+ " } ],"
+ "\"valid-lifetime\": 4000 }";
+
+ // CASE 1: Configure 4 subnets, then reconfigure and remove the
+ // last one.
+
+ ElementPtr json = Element::fromJSON(config4);
+ EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
+ ASSERT_TRUE(subnets);
+ ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
+
+ // Do the reconfiguration (the last subnet is removed)
+ json = Element::fromJSON(config_first3);
+ EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ subnets = CfgMgr::instance().getSubnets6();
+ ASSERT_TRUE(subnets);
+ ASSERT_EQ(3, subnets->size()); // We expect 3 subnets now (4th is removed)
+
+ EXPECT_EQ(1, subnets->at(0)->getID());
+ EXPECT_EQ(2, subnets->at(1)->getID());
+ EXPECT_EQ(3, subnets->at(2)->getID());
+
+ /// CASE 2: Configure 4 subnets, then reconfigure and remove one
+ /// from in between (not first, not last)
+
+#if 0
+ /// @todo: Uncomment subnet removal test as part of #3281.
+ json = Element::fromJSON(config4);
+ EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ // Do reconfiguration
+ json = Element::fromJSON(config_second_removed);
+ EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
+ ASSERT_TRUE(x);
+ comment_ = parseAnswer(rcode_, x);
+ ASSERT_EQ(0, rcode_);
+
+ subnets = CfgMgr::instance().getSubnets6();
+ ASSERT_TRUE(subnets);
+ ASSERT_EQ(3, subnets->size()); // We expect 4 subnets
+
+ EXPECT_EQ(1, subnets->at(0)->getID());
+ // The second subnet (with subnet-id = 2) is no longer there
+ EXPECT_EQ(3, subnets->at(1)->getID());
+ EXPECT_EQ(4, subnets->at(2)->getID());
+#endif
+}
+
+
+
// This test checks if it is possible to override global values
// on a per subnet basis.
TEST_F(Dhcp6ParserTest, subnetLocal) {
diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc
index a07442e..56ed105 100644
--- a/src/lib/dhcpsrv/subnet.cc
+++ b/src/lib/dhcpsrv/subnet.cc
@@ -31,7 +31,7 @@ Subnet::Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len,
const Triplet<uint32_t>& t1,
const Triplet<uint32_t>& t2,
const Triplet<uint32_t>& valid_lifetime)
- :id_(getNextID()), prefix_(prefix), prefix_len_(len), t1_(t1),
+ :id_(generateNextID()), prefix_(prefix), prefix_len_(len), t1_(t1),
t2_(t2), valid_(valid_lifetime),
last_allocated_ia_(lastAddrInPrefix(prefix, len)),
last_allocated_ta_(lastAddrInPrefix(prefix, len)),
diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h
index ec8c397..ecac6c3 100644
--- a/src/lib/dhcpsrv/subnet.h
+++ b/src/lib/dhcpsrv/subnet.h
@@ -387,7 +387,12 @@ protected:
/// @brief Protected constructor
//
/// By making the constructor protected, we make sure that noone will
- /// ever instantiate that class. Pool4 and Pool6 should be used instead.
+ /// ever instantiate that class. Subnet4 and Subnet6 should be used instead.
+ ///
+ /// This constructor assigns a new subnet-id (see @ref generateNextID).
+ /// This subnet-id has unique value that is strictly monotonously increasing
+ /// for each subnet, until it is explicitly reset back to 1 during
+ /// reconfiguration process.
Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len,
const Triplet<uint32_t>& t1,
const Triplet<uint32_t>& t2,
@@ -409,8 +414,13 @@ protected:
/// @brief returns the next unique Subnet-ID
///
+ /// This method generates and returns the next unique subnet-id.
+ /// It is a strictly monotonously increasing value (1,2,3,...) for
+ /// each new Subnet object created. It can be explicitly reset
+ /// back to 1 during reconfiguration (@ref resetSubnetID).
+ ///
/// @return the next unique Subnet-ID
- static SubnetID getNextID() {
+ static SubnetID generateNextID() {
return (static_id_++);
}
@@ -511,6 +521,8 @@ public:
/// @brief Constructor with all parameters
///
+ /// This constructor calls Subnet::Subnet, where subnet-id is generated.
+ ///
/// @param prefix Subnet4 prefix
/// @param length prefix length
/// @param t1 renewal timer (in seconds)
@@ -575,6 +587,8 @@ public:
/// @brief Constructor with all parameters
///
+ /// This constructor calls Subnet::Subnet, where subnet-id is generated.
+ ///
/// @param prefix Subnet6 prefix
/// @param length prefix length
/// @param t1 renewal timer (in seconds)
More information about the bind10-changes
mailing list