[master] Merge branch 'trac3281'

Conflicts:
	doc/guide/bind10-guide.xml

doc/guide/bind10-guide.xml

@@ -3848,6 +3848,42 @@ Dhcp4/dhcp-ddns/qualifying-suffix	"example.com"	string
       </para>
       </section>
 
+      <section id="ipv4-subnet-id">
+      <title>IPv4 Subnet Identifier</title>
+      <para>
+        Subnet identifier is a unique number associated with a particular subnet.
+        In principle, it is used to associate clients' leases with respective subnets.
+        When subnet identifier is not specified for a subnet being configured, it will
+        be automatically assigned by the configuration mechanism. The identifiers
+        are assigned from 1 and are monotonically increased for each subsequent
+        subnet: 1, 2, 3 ....
+      </para>
+      <para>
+       If there are multiple subnets configured with auto-generated identifiers and
+       one of them is removed, the subnet identifiers may be renumbered. For example:
+       if there are 4 subnets and 3rd is removed the last subnet will be assigned
+       identifier that the 3rd subnet had before removal. As a result, the leases
+       stored in the lease database for subnet 3 are now associated with the
+       subnet 4, which may have unexpected consequences. In the future it is planned
+       to implement the mechanism to preserve auto-generated subnet ids upon removal
+       of one of the subnets. Currently, the only remedy for this issue is to
+       manually specify the unique subnet identifier for each subnet.
+      </para>
+      <para>
+        The following configuration:
+        <screen>
+&gt; <userinput>config add Dhcp4/subnet4</userinput>
+&gt; <userinput>config set Dhcp4/subnet4[0]/subnet "192.0.2.0/24"</userinput>
+&gt; <userinput>config set Dhcp4/subnet4[0]/id 1024</userinput>
+&gt; <userinput>config commit</userinput>
+        </screen>
+        will assign the arbitrary subnet identifier to the newly configured subnet.
+        This identifier will not change for this subnet until "id" parameter is
+        removed or set to 0. The value of 0 forces auto-generation of subnet
+        identifier.
+      </para>
+      </section>
+
       <section id="dhcp4-address-config">
       <title>Configuration of IPv4 Address Pools</title>
       <para>
@@ -5016,19 +5052,11 @@ Dhcp4/dhcp-ddns/qualifying-suffix	"example.com"	string
       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>
+            <simpara>
+              Removal of a subnet during server reconfiguration may cause renumbering
+              of auto-generated subnet identifiers, as described in section
+              <xref linkend="ipv4-subnet-id"/>.
+            </simpara>
           </listitem>
           <listitem>
             <simpara>
@@ -5296,6 +5324,42 @@ Dhcp6/dhcp-ddns/qualifying-suffix   "example.com"   string
       </para>
     </section>
 
+    <section id="ipv6-subnet-id">
+      <title>IPv6 Subnet Identifier</title>
+      <para>
+        Subnet identifier is a unique number associated with a particular subnet.
+        In principle, it is used to associate clients' leases with respective subnets.
+        When subnet identifier is not specified for a subnet being configured, it will
+        be automatically assigned by the configuration mechanism. The identifiers
+        are assigned from 1 and are monotonically increased for each subsequent
+        subnet: 1, 2, 3 ....
+      </para>
+      <para>
+       If there are multiple subnets configured with auto-generated identifiers and
+       one of them is removed, the subnet identifiers may be renumbered. For example:
+       if there are 4 subnets and 3rd is removed the last subnet will be assigned
+       identifier that the 3rd subnet had before removal. As a result, the leases
+       stored in the lease database for subnet 3 are now associated with the
+       subnet 4, which may have unexpected consequences. In the future it is planned
+       to implement the mechanism to preserve auto-generated subnet ids upon removal
+       of one of the subnets. Currently, the only remedy for this issue is to
+       manually specify the unique subnet identifier for each subnet.
+      </para>
+      <para>
+        The following configuration:
+        <screen>
+&gt; <userinput>config add Dhcp6/subnet6</userinput>
+&gt; <userinput>config set Dhcp6/subnet6[0]/subnet "2001:db8:1::/64"</userinput>
+&gt; <userinput>config set Dhcp6/subnet6[0]/id 1024</userinput>
+&gt; <userinput>config commit</userinput>
+        </screen>
+        will assign the arbitrary subnet identifier to the newly configured subnet.
+        This identifier will not change for this subnet until "id" parameter is
+        removed or set to 0. The value of 0 forces auto-generation of subnet
+        identifier.
+      </para>
+    </section>
+
     <section id="dhcp6-unicast">
       <title>Unicast traffic support</title>
       <para>
@@ -6439,19 +6503,11 @@ Dhcp6/dhcp-ddns/qualifying-suffix	"example.com"	string
       <itemizedlist>
 
         <listitem> <!-- see tickets #3234, #3281 -->
-          <simpara>
-            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.
-          </simpara>
+            <simpara>
+              Removal of a subnet during server reconfiguration may cause renumbering
+              of auto-generated subnet identifiers, as described in section
+              <xref linkend="ipv6-subnet-id"/>.
+            </simpara>
         </listitem>
         <listitem>
           <simpara>

src/bin/dhcp4/config_parser.cc

@@ -201,7 +201,8 @@ protected:
         DhcpConfigParser* parser = NULL;
         if ((config_id.compare("valid-lifetime") == 0)  ||
             (config_id.compare("renew-timer") == 0)  ||
-            (config_id.compare("rebind-timer") == 0))  {
+            (config_id.compare("rebind-timer") == 0) ||
+            (config_id.compare("id") == 0)) {
             parser = new Uint32Parser(config_id, uint32_values_);
         } else if ((config_id.compare("subnet") == 0) ||
                    (config_id.compare("interface") == 0) ||
@@ -271,6 +272,10 @@ protected:
         Triplet<uint32_t> t1 = getParam("renew-timer");
         Triplet<uint32_t> t2 = getParam("rebind-timer");
         Triplet<uint32_t> valid = getParam("valid-lifetime");
+        // Subnet ID is optional. If it is not supplied the value of 0 is used,
+        // which means autogenerate.
+        SubnetID subnet_id =
+            static_cast<SubnetID>(uint32_values_->getOptionalParam("id", 0));
 
         stringstream tmp;
         tmp << addr << "/" << (int)len
@@ -278,7 +283,7 @@ protected:
 
         LOG_INFO(dhcp4_logger, DHCP4_CONFIG_NEW_SUBNET).arg(tmp.str());
 
-        Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid));
+        Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid, subnet_id));
         subnet_ = subnet4;
 
         // Try global value first

src/bin/dhcp4/dhcp4.spec

@@ -213,6 +213,12 @@
                   "item_default": ""
                 },
 
+                { "item_name": "id",
+                  "item_type": "integer",
+                  "item_optional": false,
+                  "item_default": 0
+                },
+
                 { "item_name": "renew-timer",
                   "item_type": "integer",
                   "item_optional": false,

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -550,6 +550,8 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
 // multiple times.
 TEST_F(Dhcp4ParserTest, multipleSubnets) {
     ConstElementPtr x;
+    // Collection of four subnets for which subnet ids should be
+    // autogenerated - ids are unspecified or set to 0.
     string config = "{ \"interfaces\": [ \"*\" ],"
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
@@ -559,7 +561,8 @@ TEST_F(Dhcp4ParserTest, multipleSubnets) {
         " },"
         " {"
         "    \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
-        "    \"subnet\": \"192.0.3.0/24\" "
+        "    \"subnet\": \"192.0.3.0/24\", "
+        "    \"id\": 0 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
@@ -573,16 +576,9 @@ TEST_F(Dhcp4ParserTest, multipleSubnets) {
 
     ElementPtr json = Element::fromJSON(config);
 
-    EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
-    ASSERT_TRUE(x);
-    comment_ = parseAnswer(rcode_, x);
-    ASSERT_EQ(0, rcode_);
-
     int cnt = 0; // Number of reconfigurations
 
     do {
-        ElementPtr json = Element::fromJSON(config);
-
         EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
         ASSERT_TRUE(x);
         comment_ = parseAnswer(rcode_, x);
@@ -605,6 +601,96 @@ TEST_F(Dhcp4ParserTest, multipleSubnets) {
     } while (++cnt < 10);
 }
 
+// This test checks that it is possible to assign arbitrary ids for subnets.
+TEST_F(Dhcp4ParserTest, multipleSubnetsExplicitIDs) {
+    ConstElementPtr x;
+    // Four subnets with arbitrary subnet ids.
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"id\": 1024 "
+        " },"
+        " {"
+        "    \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
+        "    \"subnet\": \"192.0.3.0/24\", "
+        "    \"id\": 100 "
+        " },"
+        " {"
+        "    \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
+        "    \"subnet\": \"192.0.4.0/24\", "
+        "    \"id\": 1 "
+        " },"
+        " {"
+        "    \"pool\": [ \"192.0.5.101 - 192.0.5.150\" ],"
+        "    \"subnet\": \"192.0.5.0/24\", "
+        "    \"id\": 34 "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+
+    int cnt = 0; // Number of reconfigurations
+    do {
+        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
+
+        // Verify that subnet ids are as expected.
+        EXPECT_EQ(1024, subnets->at(0)->getID());
+        EXPECT_EQ(100, subnets->at(1)->getID());
+        EXPECT_EQ(1, subnets->at(2)->getID());
+        EXPECT_EQ(34, subnets->at(3)->getID());
+
+        // Repeat reconfiguration process 10 times and check that the subnet-id
+        // is set to the same value.
+    } while (++cnt < 3);
+}
+
+// Check that the configuration with two subnets having the same id is rejected.
+TEST_F(Dhcp4ParserTest, multipleSubnetsOverlapingIDs) {
+    ConstElementPtr x;
+    // Four subnets, two of them having the same id.
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"id\": 1024 "
+        " },"
+        " {"
+        "    \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
+        "    \"subnet\": \"192.0.3.0/24\", "
+        "    \"id\": 100 "
+        " },"
+        " {"
+        "    \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
+        "    \"subnet\": \"192.0.4.0/24\", "
+        "    \"id\": 1024 "
+        " },"
+        " {"
+        "    \"pool\": [ \"192.0.5.101 - 192.0.5.150\" ],"
+        "    \"subnet\": \"192.0.5.0/24\", "
+        "    \"id\": 34 "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(x);
+    comment_ = parseAnswer(rcode_, x);
+    EXPECT_NE(rcode_, 0);
+}
+
 // Goal of this test is to verify that a previously configured subnet can be
 // deleted in subsequent reconfiguration.
 TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
@@ -616,19 +702,23 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
-        "    \"subnet\": \"192.0.2.0/24\" "
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"id\": 1 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
-        "    \"subnet\": \"192.0.3.0/24\" "
+        "    \"subnet\": \"192.0.3.0/24\", "
+        "    \"id\": 2 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
-        "    \"subnet\": \"192.0.4.0/24\" "
+        "    \"subnet\": \"192.0.4.0/24\", "
+        "    \"id\": 3 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.5.101 - 192.0.5.150\" ],"
-        "    \"subnet\": \"192.0.5.0/24\" "
+        "    \"subnet\": \"192.0.5.0/24\", "
+        "    \"id\": 4 "
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -638,15 +728,18 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
-        "    \"subnet\": \"192.0.2.0/24\" "
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"id\": 1 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ],"
-        "    \"subnet\": \"192.0.3.0/24\" "
+        "    \"subnet\": \"192.0.3.0/24\", "
+        "    \"id\": 2 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
-        "    \"subnet\": \"192.0.4.0/24\" "
+        "    \"subnet\": \"192.0.4.0/24\", "
+        "    \"id\": 3 "
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -656,15 +749,18 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
-        "    \"subnet\": \"192.0.2.0/24\" "
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"id\": 1 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.4.101 - 192.0.4.150\" ],"
-        "    \"subnet\": \"192.0.4.0/24\" "
+        "    \"subnet\": \"192.0.4.0/24\", "
+        "    \"id\": 3 "
         " },"
         " {"
         "    \"pool\": [ \"192.0.5.101 - 192.0.5.150\" ],"
-        "    \"subnet\": \"192.0.5.0/24\" "
+        "    \"subnet\": \"192.0.5.0/24\", "
+        "    \"id\": 4 "
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -700,7 +796,6 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
     /// 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));
@@ -723,7 +818,6 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) {
     // 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
 
 }
 

src/bin/dhcp6/config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-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
@@ -406,7 +406,8 @@ protected:
         if ((config_id.compare("preferred-lifetime") == 0)  ||
             (config_id.compare("valid-lifetime") == 0)  ||
             (config_id.compare("renew-timer") == 0)  ||
-            (config_id.compare("rebind-timer") == 0))  {
+            (config_id.compare("rebind-timer") == 0) ||
+            (config_id.compare("id") == 0)) {
             parser = new Uint32Parser(config_id, uint32_values_);
         } else if ((config_id.compare("subnet") == 0) ||
                    (config_id.compare("interface") == 0) ||
@@ -480,6 +481,10 @@ protected:
         Triplet<uint32_t> t2 = getParam("rebind-timer");
         Triplet<uint32_t> pref = getParam("preferred-lifetime");
         Triplet<uint32_t> valid = getParam("valid-lifetime");
+        // Subnet ID is optional. If it is not supplied the value of 0 is used,
+        // which means autogenerate.
+        SubnetID subnet_id =
+            static_cast<SubnetID>(uint32_values_->getOptionalParam("id", 0));
 
         // Get interface-id option content. For now we support string
         // represenation only
@@ -518,7 +523,8 @@ protected:
         LOG_INFO(dhcp6_logger, DHCP6_CONFIG_NEW_SUBNET).arg(tmp.str());
 
         // Create a new subnet.
-        Subnet6* subnet6 = new Subnet6(addr, len, t1, t2, pref, valid);
+        Subnet6* subnet6 = new Subnet6(addr, len, t1, t2, pref, valid,
+                                       subnet_id);
 
         // Configure interface-id for remote interfaces, if defined
         if (!ifaceid.empty()) {

src/bin/dhcp6/dhcp6.spec

@@ -207,6 +207,12 @@
                   "item_default": ""
                 },
 
+                { "item_name": "id",
+                  "item_type": "integer",
+                  "item_optional": false,
+                  "item_default": 0
+                },
+
                 { "item_name": "interface",
                   "item_type": "string",
                   "item_optional": false,

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -569,11 +569,10 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) {
     EXPECT_EQ(1, subnet->getID());
 }
 
-// Goal of this test is to verify that multiple subnets get unique
-// subnet-ids. Also, test checks that it's possible to do reconfiguration
-// multiple times.
 TEST_F(Dhcp6ParserTest, multipleSubnets) {
     ConstElementPtr x;
+    // Collection of four subnets for which ids should be autogenerated
+    // - ids are unspecified or set to 0.
     string config = "{ \"interfaces\": [ \"*\" ],"
         "\"preferred-lifetime\": 3000,"
         "\"rebind-timer\": 2000, "
@@ -584,7 +583,8 @@ TEST_F(Dhcp6ParserTest, multipleSubnets) {
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:2::/80\" ],"
-        "    \"subnet\": \"2001:db8:2::/64\" "
+        "    \"subnet\": \"2001:db8:2::/64\", "
+        "    \"id\": 0"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:3::/80\" ],"
@@ -598,9 +598,9 @@ TEST_F(Dhcp6ParserTest, multipleSubnets) {
 
     int cnt = 0; // Number of reconfigurations
 
-    do {
-        ElementPtr json = Element::fromJSON(config);
+    ElementPtr json = Element::fromJSON(config);
 
+    do {
         EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
         ASSERT_TRUE(x);
         comment_ = parseAnswer(rcode_, x);
@@ -623,6 +623,100 @@ TEST_F(Dhcp6ParserTest, multipleSubnets) {
     } while (++cnt < 10);
 }
 
+// This checks that it is possible to assign arbitrary ids for subnets.
+TEST_F(Dhcp6ParserTest, multipleSubnetsExplicitIDs) {
+    ConstElementPtr x;
+    // Four subnets with arbitrary subnet ids.
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pool\": [ \"2001:db8:1::/80\" ],"
+        "    \"subnet\": \"2001:db8:1::/64\", "
+        "    \"id\": 1024"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:2::/80\" ],"
+        "    \"subnet\": \"2001:db8:2::/64\", "
+        "    \"id\": 100"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:3::/80\" ],"
+        "    \"subnet\": \"2001:db8:3::/64\", "
+        "    \"id\": 1"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:4::/80\" ],"
+        "    \"subnet\": \"2001:db8:4::/64\", "
+        "    \"id\": 34"
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    int cnt = 0; // Number of reconfigurations
+
+    ElementPtr json = Element::fromJSON(config);
+
+    do {
+        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
+
+        // Check that subnet ids are as expected.
+        EXPECT_EQ(1024, subnets->at(0)->getID());
+        EXPECT_EQ(100, subnets->at(1)->getID());
+        EXPECT_EQ(1, subnets->at(2)->getID());
+        EXPECT_EQ(34, subnets->at(3)->getID());
+
+        // Repeat reconfiguration process 10 times and check that the subnet-id
+        // is set to the same value.
+    } while (++cnt < 3);
+}
+
+// CHeck that the configuration with two subnets having the same id is rejected.
+TEST_F(Dhcp6ParserTest, multipleSubnetsOverlapingIDs) {
+    ConstElementPtr x;
+    // Four subnets, two of them have the same id.
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pool\": [ \"2001:db8:1::/80\" ],"
+        "    \"subnet\": \"2001:db8:1::/64\", "
+        "    \"id\": 1024"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:2::/80\" ],"
+        "    \"subnet\": \"2001:db8:2::/64\", "
+        "    \"id\": 100"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:3::/80\" ],"
+        "    \"subnet\": \"2001:db8:3::/64\", "
+        "    \"id\": 1024"
+        " },"
+        " {"
+        "    \"pool\": [ \"2001:db8:4::/80\" ],"
+        "    \"subnet\": \"2001:db8:4::/64\", "
+        "    \"id\": 34"
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(x);
+    comment_ = parseAnswer(rcode_, x);
+    ASSERT_NE(rcode_, 0);
+}
+
+
 // Goal of this test is to verify that a previously configured subnet can be
 // deleted in subsequent reconfiguration.
 TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
@@ -635,19 +729,23 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
         "\"renew-timer\": 1000, "
         "\"subnet6\": [ { "
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
-        "    \"subnet\": \"2001:db8:1::/64\" "
+        "    \"subnet\": \"2001:db8:1::/64\", "
+        "    \"id\": 1"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:2::/80\" ],"
-        "    \"subnet\": \"2001:db8:2::/64\" "
+        "    \"subnet\": \"2001:db8:2::/64\", "
+        "    \"id\": 2"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:3::/80\" ],"
-        "    \"subnet\": \"2001:db8:3::/64\" "
+        "    \"subnet\": \"2001:db8:3::/64\", "
+        "    \"id\": 3"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:4::/80\" ],"
-        "    \"subnet\": \"2001:db8:4::/64\" "
+        "    \"subnet\": \"2001:db8:4::/64\", "
+        "    \"id\": 4"
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -658,15 +756,18 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
         "\"renew-timer\": 1000, "
         "\"subnet6\": [ { "
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
-        "    \"subnet\": \"2001:db8:1::/64\" "
+        "    \"subnet\": \"2001:db8:1::/64\", "
+        "    \"id\": 1"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:2::/80\" ],"
-        "    \"subnet\": \"2001:db8:2::/64\" "
+        "    \"subnet\": \"2001:db8:2::/64\", "
+        "    \"id\": 2"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:3::/80\" ],"
-        "    \"subnet\": \"2001:db8:3::/64\" "
+        "    \"subnet\": \"2001:db8:3::/64\", "
+        "    \"id\": 3"
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -677,15 +778,18 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
         "\"renew-timer\": 1000, "
         "\"subnet6\": [ { "
         "    \"pool\": [ \"2001:db8:1::/80\" ],"
-        "    \"subnet\": \"2001:db8:1::/64\" "
+        "    \"subnet\": \"2001:db8:1::/64\", "
+        "    \"id\": 1"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:3::/80\" ],"
-        "    \"subnet\": \"2001:db8:3::/64\" "
+        "    \"subnet\": \"2001:db8:3::/64\", "
+        "    \"id\": 3"
         " },"
         " {"
         "    \"pool\": [ \"2001:db8:4::/80\" ],"
-        "    \"subnet\": \"2001:db8:4::/64\" "
+        "    \"subnet\": \"2001:db8:4::/64\", "
+        "    \"id\": 4"
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -720,8 +824,6 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
     /// 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);
@@ -743,7 +845,6 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) {
     // 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
 }
 
 

src/lib/dhcpsrv/cfgmgr.cc

@@ -219,6 +219,10 @@ void CfgMgr::addSubnet6(const Subnet6Ptr& subnet) {
     /// @todo: Check that this new subnet does not cross boundaries of any
     /// other already defined subnet.
     /// @todo: Check that there is no subnet with the same interface-id
+    if (isDuplicate(*subnet)) {
+        isc_throw(isc::dhcp::DuplicateSubnetID, "ID of the new IPv6 subnet '"
+                  << subnet->getID() << "' is already in use");
+    }
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_SUBNET6)
               .arg(subnet->toText());
     subnets6_.push_back(subnet);
@@ -282,6 +286,10 @@ CfgMgr::getSubnet4(const std::string& iface_name,
 void CfgMgr::addSubnet4(const Subnet4Ptr& subnet) {
     /// @todo: Check that this new subnet does not cross boundaries of any
     /// other already defined subnet.
+    if (isDuplicate(*subnet)) {
+        isc_throw(isc::dhcp::DuplicateSubnetID, "ID of the new IPv4 subnet '"
+                  << subnet->getID() << "' is already in use");
+    }
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_SUBNET4)
               .arg(subnet->toText());
     subnets4_.push_back(subnet);
@@ -377,6 +385,29 @@ CfgMgr::isIfaceListedActive(const std::string& iface) const {
     return (false);
 }
 
+bool
+CfgMgr::isDuplicate(const Subnet4& subnet) const {
+    for (Subnet4Collection::const_iterator subnet_it = subnets4_.begin();
+         subnet_it != subnets4_.end(); ++subnet_it) {
+        if ((*subnet_it)->getID() == subnet.getID()) {
+            return (true);
+        }
+    }
+    return (false);
+}
+
+bool
+CfgMgr::isDuplicate(const Subnet6& subnet) const {
+    for (Subnet6Collection::const_iterator subnet_it = subnets6_.begin();
+         subnet_it != subnets6_.end(); ++subnet_it) {
+        if ((*subnet_it)->getID() == subnet.getID()) {
+            return (true);
+        }
+    }
+    return (false);
+}
+
+
 const isc::asiolink::IOAddress*
 CfgMgr::getUnicast(const std::string& iface) const {
     UnicastIfacesCollection::const_iterator addr = unicast_addrs_.find(iface);

src/lib/dhcpsrv/cfgmgr.h

@@ -47,6 +47,13 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief Exception thrown upon attempt to add subnet with an ID that belongs
+/// to the subnet that already exists.
+class DuplicateSubnetID : public Exception {
+public:
+    DuplicateSubnetID(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
 
 /// @brief Configuration Manager
 ///
@@ -458,6 +465,20 @@ private:
     /// @c CfgMgr::active_ifaces_.
     bool isIfaceListedActive(const std::string& iface) const;
 
+    /// @brief Checks that the IPv4 subnet with the given id already exists.
+    ///
+    /// @param subnet Subnet for which this function will check if the other
+    /// subnet with equal id already exists.
+    /// @return true if the duplicate subnet exists.
+    bool isDuplicate(const Subnet4& subnet) const;
+
+    /// @brief Checks that the IPv6 subnet with the given id already exists.
+    ///
+    /// @param subnet Subnet for which this function will check if the other
+    /// subnet with equal id already exists.
+    /// @return true if the duplicate subnet exists.
+    bool isDuplicate(const Subnet6& subnet) const;
+
     /// @brief A collection of option definitions.
     ///
     /// A collection of option definitions that can be accessed

src/lib/dhcpsrv/subnet.cc

@@ -31,8 +31,9 @@ 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,
-               const isc::dhcp::Subnet::RelayInfo& relay)
-    :id_(generateNextID()), prefix_(prefix), prefix_len_(len),
+               const isc::dhcp::Subnet::RelayInfo& relay,
+               const SubnetID id)
+    :id_(id == 0 ? generateNextID() : id), prefix_(prefix), prefix_len_(len),
      t1_(t1), t2_(t2), valid_(valid_lifetime),
      last_allocated_ia_(lastAddrInPrefix(prefix, len)),
      last_allocated_ta_(lastAddrInPrefix(prefix, len)),
@@ -211,9 +212,10 @@ void Subnet4::checkType(Lease::Type type) const {
 Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t1,
                  const Triplet<uint32_t>& t2,
-                 const Triplet<uint32_t>& valid_lifetime)
+                 const Triplet<uint32_t>& valid_lifetime,
+                 const SubnetID id)
 :Subnet(prefix, length, t1, t2, valid_lifetime,
-        RelayInfo(IOAddress("0.0.0.0"))), siaddr_(IOAddress("0.0.0.0")) {
+        RelayInfo(IOAddress("0.0.0.0")), id), siaddr_(IOAddress("0.0.0.0")) {
     if (!prefix.isV4()) {
         isc_throw(BadValue, "Non IPv4 prefix " << prefix.toText()
                   << " specified in subnet4");
@@ -363,9 +365,10 @@ Subnet6::Subnet6(const isc::asiolink::IOAddress& prefix, uint8_t length,
                  const Triplet<uint32_t>& t1,
                  const Triplet<uint32_t>& t2,
                  const Triplet<uint32_t>& preferred_lifetime,
-                 const Triplet<uint32_t>& valid_lifetime)
-:Subnet(prefix, length, t1, t2, valid_lifetime, RelayInfo(IOAddress("::"))),
-     preferred_(preferred_lifetime){
+                 const Triplet<uint32_t>& valid_lifetime,
+                 const SubnetID id)
+:Subnet(prefix, length, t1, t2, valid_lifetime, RelayInfo(IOAddress("::")), id),
+     preferred_(preferred_lifetime) {
     if (!prefix.isV6()) {
         isc_throw(BadValue, "Non IPv6 prefix " << prefix
                   << " specified in subnet6");

src/lib/dhcpsrv/subnet.h

@@ -466,7 +466,7 @@ protected:
 
     /// @brief Protected constructor
     //
-    /// By making the constructor protected, we make sure that noone will
+    /// By making the constructor protected, we make sure that no one will
     /// ever instantiate that class. Subnet4 and Subnet6 should be used instead.
     ///
     /// This constructor assigns a new subnet-id (see @ref generateNextID).
@@ -480,11 +480,14 @@ protected:
     /// @param t2 T2 (rebind-time) timer, expressed in seconds
     /// @param valid_lifetime valid lifetime of leases in this subnet (in seconds)
     /// @param relay optional relay information (currently with address only)
+    /// @param id arbitraty subnet id, value of 0 triggers autogeneration
+    /// of subnet id
     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,
-           const isc::dhcp::Subnet::RelayInfo& relay);
+           const isc::dhcp::Subnet::RelayInfo& relay,
+           const SubnetID id);
 
     /// @brief virtual destructor
     ///
@@ -637,10 +640,13 @@ public:
     /// @param t1 renewal timer (in seconds)
     /// @param t2 rebind timer (in seconds)
     /// @param valid_lifetime preferred lifetime of leases (in seconds)
+    /// @param id arbitraty subnet id, default value of 0 triggers
+    /// autogeneration of subnet id
     Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
             const Triplet<uint32_t>& t1,
             const Triplet<uint32_t>& t2,
-            const Triplet<uint32_t>& valid_lifetime);
+            const Triplet<uint32_t>& valid_lifetime,
+            const SubnetID id = 0);
 
     /// @brief Sets siaddr for the Subnet4
     ///
@@ -704,11 +710,14 @@ public:
     /// @param t2 rebind timer (in seconds)
     /// @param preferred_lifetime preferred lifetime of leases (in seconds)
     /// @param valid_lifetime preferred lifetime of leases (in seconds)
+    /// @param id arbitraty subnet id, default value of 0 triggers
+    /// autogeneration of subnet id
     Subnet6(const isc::asiolink::IOAddress& prefix, uint8_t length,
             const Triplet<uint32_t>& t1,
             const Triplet<uint32_t>& t2,
             const Triplet<uint32_t>& preferred_lifetime,
-            const Triplet<uint32_t>& valid_lifetime);
+            const Triplet<uint32_t>& valid_lifetime,
+            const SubnetID id = 0);
 
     /// @brief Returns preverred lifetime (in seconds)
     ///

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -1053,6 +1053,41 @@ TEST_F(CfgMgrTest, getSubnet4ForInterface) {
 
 }
 
+// Checks that detection of duplicated subnet IDs works as expected. It should
+// not be possible to add two IPv4 subnets holding the same ID to the config
+// manager.
+TEST_F(CfgMgrTest, subnet4Duplication) {
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+
+    Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.2.0"), 26, 1, 2, 3, 123));
+    Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.64"), 26, 1, 2, 3, 124));
+    Subnet4Ptr subnet3(new Subnet4(IOAddress("192.0.2.128"), 26, 1, 2, 3, 123));
+
+    ASSERT_NO_THROW(cfg_mgr.addSubnet4(subnet1));
+    EXPECT_NO_THROW(cfg_mgr.addSubnet4(subnet2));
+    // Subnet 3 has the same ID as subnet 1. It shouldn't be able to add it.
+    EXPECT_THROW(cfg_mgr.addSubnet4(subnet3), isc::dhcp::DuplicateSubnetID);
+}
+
+// Checks that detection of duplicated subnet IDs works as expected. It should
+// not be possible to add two IPv6 subnets holding the same ID to the config
+// manager.
+TEST_F(CfgMgrTest, subnet6Duplication) {
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+
+    Subnet6Ptr subnet1(new Subnet6(IOAddress("2001:db8:1::"), 64, 1, 2, 3,
+                                   4, 123));
+    Subnet6Ptr subnet2(new Subnet6(IOAddress("2001:db8:2::"), 64, 1, 2, 3,
+                                   4, 124));
+    Subnet6Ptr subnet3(new Subnet6(IOAddress("2001:db8:3::"), 64, 1, 2, 3,
+                                   4, 123));
+
+    ASSERT_NO_THROW(cfg_mgr.addSubnet6(subnet1));
+    EXPECT_NO_THROW(cfg_mgr.addSubnet6(subnet2));
+    // Subnet 3 has the same ID as subnet 1. It shouldn't be able to add it.
+    EXPECT_THROW(cfg_mgr.addSubnet6(subnet3), isc::dhcp::DuplicateSubnetID);
+}
+
 
 /// @todo Add unit-tests for testing:
 /// - addActiveIface() with invalid interface name

src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -34,7 +34,7 @@ namespace {
 
 TEST(Subnet4Test, constructor) {
     EXPECT_NO_THROW(Subnet4 subnet1(IOAddress("192.0.2.2"), 16,
-                                    1, 2, 3));
+                                    1, 2, 3, 10));
 
     EXPECT_THROW(Subnet4 subnet2(IOAddress("192.0.2.0"), 33, 1, 2, 3),
                 BadValue); // invalid prefix length
@@ -42,7 +42,33 @@ TEST(Subnet4Test, constructor) {
                 BadValue); // IPv6 addresses are not allowed in Subnet4
 }
 
-TEST(Subnet4Test, in_range) {
+// Checks that the subnet id can be either autogenerated or set to an
+// arbitrary value through the constructor.
+TEST(Subnet4Test, subnetID) {
+    // Create subnet and don't specify id, so as it is autogenerated.
+    Subnet4Ptr subnet(new Subnet4(IOAddress("192.0.2.0"), 24, 1000, 2000,
+                                  3000));
+    SubnetID id0 = subnet->getID();
+
+    // Create another subnet and let id be autogenerated.
+    subnet.reset(new Subnet4(IOAddress("192.0.3.0"), 24, 1000, 2000,
+                             3000));
+    SubnetID id1 = subnet->getID();
+
+    // The autogenerated ids must not be equal.
+    EXPECT_NE(id0, id1);
+
+    // Create third subnet but this time select an arbitrary id. The id
+    // we use the one of the second subnet. That way we ensure that the
+    // subnet id we provide via constructor is used and it is not
+    // autogenerated - if it was autogenerated we would get id other
+    // than id1 because id1 has already been used.
+    subnet.reset(new Subnet4(IOAddress("192.0.4.0"), 24, 1000, 2000,
+                             3000, id1));
+    EXPECT_EQ(id1, subnet->getID());
+}
+
+TEST(Subnet4Test, inRange) {
     Subnet4 subnet(IOAddress("192.0.2.1"), 24, 1000, 2000, 3000);
 
     EXPECT_EQ(1000, subnet.getT1());
@@ -362,7 +388,33 @@ TEST(Subnet6Test, constructor) {
                 BadValue); // IPv4 addresses are not allowed in Subnet6
 }
 
-TEST(Subnet6Test, in_range) {
+// Checks that the subnet id can be either autogenerated or set to an
+// arbitrary value through the constructor.
+TEST(Subnet6Test, subnetID) {
+    // Create subnet and don't specify id, so as it is autogenerated.
+    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 64, 1000, 2000,
+                                  3000, 4000));
+    SubnetID id0 = subnet->getID();
+
+    // Create another subnet and let id be autogenerated.
+    subnet.reset(new Subnet6(IOAddress("2001:db8:2::"), 64, 1000, 2000,
+                             3000, 4000));
+    SubnetID id1 = subnet->getID();
+
+    // The autogenerated ids must not be equal.
+    EXPECT_NE(id0, id1);
+
+    // Create third subnet but this time select an arbitrary id. The id
+    // we use us the one of second subnet. That way we ensure that the
+    // subnet id we provide via constructor is used and it is not
+    // autogenerated - if it was autogenerated we would get id other
+    // than id1 because id1 has already been used.
+    subnet.reset(new Subnet6(IOAddress("2001:db8:3::"), 64, 1000, 2000,
+                             3000, 4000, id1));
+    EXPECT_EQ(id1, subnet->getID());
+}
+
+TEST(Subnet6Test, inRange) {
     Subnet6 subnet(IOAddress("2001:db8:1::"), 64, 1000, 2000, 3000, 4000);
 
     EXPECT_EQ(1000, subnet.getT1());