From 30a20ce29360184cd0cef67a8dc53882a0e1b58d Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 26 Mar 2014 12:40:34 +0100 Subject: [PATCH] [3281] DHCPv4 configuration parser sets the subnet id specified by user. --- src/bin/dhcp4/config_parser.cc | 9 +- src/bin/dhcp4/dhcp4.spec | 6 + src/bin/dhcp4/tests/config_parser_unittest.cc | 137 ++++++++++++++++-- 3 files changed, 137 insertions(+), 15 deletions(-) diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index 1d010f326a..d22fef2aa7 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/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 t1 = getParam("renew-timer"); Triplet t2 = getParam("rebind-timer"); Triplet 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(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 diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 5de65b2099..1eae833d0a 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/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, diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index e16ae1786f..05ae616b96 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/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\" ]," @@ -605,6 +608,106 @@ 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; + // 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, " + "\"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); + + 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); + 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 < 10); +} + +// Check that the configuration with two subnets having the same id is rejected. +TEST_F(Dhcp4ParserTest, multipleSubnetsOverlapingIDs) { + 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, " + "\"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 +719,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 +745,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 +766,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 +813,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 +835,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 }