From fae9245a36d275f5dc8bfd8167cc9de623c8efa7 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Fri, 16 Nov 2018 23:09:31 +0100 Subject: [PATCH] [268-reservation-mode-is-not-global] Updated code for global reservation mode --- src/bin/dhcp4/dhcp4_lexer.ll | 1 + src/bin/dhcp4/dhcp4_parser.yy | 1 + src/bin/dhcp4/json_config_parser.cc | 3 +- src/bin/dhcp4/parser_context.h | 2 +- src/bin/dhcp4/tests/config_parser_unittest.cc | 54 +++++++++++++++ src/bin/dhcp6/dhcp6_lexer.ll | 1 + src/bin/dhcp6/dhcp6_parser.yy | 1 + src/bin/dhcp6/json_config_parser.cc | 3 +- src/bin/dhcp6/parser_context.h | 2 +- src/bin/dhcp6/tests/config_parser_unittest.cc | 66 +++++++++++++++++-- src/lib/dhcpsrv/parsers/simple_parser4.cc | 7 +- src/lib/dhcpsrv/parsers/simple_parser6.cc | 9 ++- 12 files changed, 133 insertions(+), 17 deletions(-) diff --git a/src/bin/dhcp4/dhcp4_lexer.ll b/src/bin/dhcp4/dhcp4_lexer.ll index c4b5c26f83..314ca284ae 100644 --- a/src/bin/dhcp4/dhcp4_lexer.ll +++ b/src/bin/dhcp4/dhcp4_lexer.ll @@ -766,6 +766,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} \"reservation-mode\" { switch(driver.ctx_) { + case isc::dhcp::Parser4Context::DHCP4: case isc::dhcp::Parser4Context::SUBNET4: case isc::dhcp::Parser4Context::SHARED_NETWORK: return isc::dhcp::Dhcp4Parser::make_RESERVATION_MODE(driver.loc_); diff --git a/src/bin/dhcp4/dhcp4_parser.yy b/src/bin/dhcp4/dhcp4_parser.yy index a3add7ff85..0a03e7eba7 100644 --- a/src/bin/dhcp4/dhcp4_parser.yy +++ b/src/bin/dhcp4/dhcp4_parser.yy @@ -464,6 +464,7 @@ global_param: valid_lifetime | reservations | config_control | server_tag + | reservation_mode | unknown_map_entry ; diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 913ff50a23..fcaaa75b00 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -545,7 +545,8 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, (config_pair.first == "next-server") || (config_pair.first == "server-hostname") || (config_pair.first == "boot-file-name") || - (config_pair.first == "server-tag")) { + (config_pair.first == "server-tag") || + (config_pair.first == "reservation-mode")) { continue; } diff --git a/src/bin/dhcp4/parser_context.h b/src/bin/dhcp4/parser_context.h index 85f262e012..a9a35ea2c2 100644 --- a/src/bin/dhcp4/parser_context.h +++ b/src/bin/dhcp4/parser_context.h @@ -244,7 +244,7 @@ public: /// Used while parsing shared-networks structures. SHARED_NETWORK, - /// Used while parsing Dhcp4/Subnet4/reservation-mode. + /// Used while parsing Dhcp4/reservation-mode. RESERVATION_MODE, /// Used while parsing Dhcp4/option-def structures. diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 2b050c0bfd..f3d5e02258 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -4929,6 +4929,60 @@ TEST_F(Dhcp4ParserTest, hostReservationPerSubnet) { EXPECT_EQ(Network::HR_ALL, subnet->getHostReservationMode()); } +/// The goal of this test is to verify that Host Reservation modes can be +/// specified globally. +TEST_F(Dhcp4ParserTest, hostReservationGlobal) { + + /// - Configuration: + /// - only addresses (no prefixes) + /// - 2 subnets with : + /// - 192.0.2.0/24 (all reservations enabled) + /// - 192.0.3.0/24 (reservations not specified) + const char* hr_config = + "{ " + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"reservation-mode\": \"out-of-pool\", " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ]," + " \"subnet\": \"192.0.2.0/24\", " + " \"reservation-mode\": \"all\"" + " }," + " {" + " \"pools\": [ { \"pool\": \"192.0.3.0/24\" } ]," + " \"subnet\": \"192.0.3.0/24\"" + " } ]," + "\"valid-lifetime\": 4000 }"; + + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(hr_config)); + extractConfig(hr_config); + ConstElementPtr result; + EXPECT_NO_THROW(result = configureDhcp4Server(*srv_, json)); + + // returned value should be 0 (success) + checkResult(result, 0); + + // Let's get all subnets and check that there are 4 of them. + ConstCfgSubnets4Ptr subnets = CfgMgr::instance().getStagingCfg()->getCfgSubnets4(); + ASSERT_TRUE(subnets); + const Subnet4Collection* subnet_col = subnets->getAll(); + ASSERT_EQ(2, subnet_col->size()); // We expect 2 subnets + + // Let's check if the parsed subnets have correct HR modes. + + // Subnet 1 + Subnet4Ptr subnet; + subnet = subnets->selectSubnet(IOAddress("192.0.2.1")); + ASSERT_TRUE(subnet); + EXPECT_EQ(Network::HR_ALL, subnet->getHostReservationMode()); + + // Subnet 2 + subnet = subnets->selectSubnet(IOAddress("192.0.3.1")); + ASSERT_TRUE(subnet); + EXPECT_EQ(Network::HR_OUT_OF_POOL, subnet->getHostReservationMode()); +} + /// Check that the decline-probation-period has a default value when not /// specified. TEST_F(Dhcp4ParserTest, declineTimerDefault) { diff --git a/src/bin/dhcp6/dhcp6_lexer.ll b/src/bin/dhcp6/dhcp6_lexer.ll index dd9d789208..0cb1a73fdb 100644 --- a/src/bin/dhcp6/dhcp6_lexer.ll +++ b/src/bin/dhcp6/dhcp6_lexer.ll @@ -1039,6 +1039,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence} \"reservation-mode\" { switch(driver.ctx_) { + case isc::dhcp::Parser6Context::DHCP6: case isc::dhcp::Parser6Context::SUBNET6: case isc::dhcp::Parser6Context::SHARED_NETWORK: return isc::dhcp::Dhcp6Parser::make_RESERVATION_MODE(driver.loc_); diff --git a/src/bin/dhcp6/dhcp6_parser.yy b/src/bin/dhcp6/dhcp6_parser.yy index 1b88046c0a..be817255bf 100644 --- a/src/bin/dhcp6/dhcp6_parser.yy +++ b/src/bin/dhcp6/dhcp6_parser.yy @@ -465,6 +465,7 @@ global_param: preferred_lifetime | reservations | config_control | server_tag + | reservation_mode | unknown_map_entry ; diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 3ff82c89bb..555b3500c9 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -637,7 +637,8 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, (config_pair.first == "decline-probation-period") || (config_pair.first == "dhcp4o6-port") || (config_pair.first == "user-context") || - (config_pair.first == "server-tag")) { + (config_pair.first == "server-tag") || + (config_pair.first == "reservation-mode")) { continue; } diff --git a/src/bin/dhcp6/parser_context.h b/src/bin/dhcp6/parser_context.h index 27953b9f37..9cec79b29a 100644 --- a/src/bin/dhcp6/parser_context.h +++ b/src/bin/dhcp6/parser_context.h @@ -245,7 +245,7 @@ public: /// Used while parsing shared-networks structures. SHARED_NETWORK, - /// Used while parsing Dhcp6/Subnet6/reservation-mode. + /// Used while parsing Dhcp6/reservation-mode. RESERVATION_MODE, /// Used while parsing Dhcp6/option-def structures. diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 4888c627d8..646f5c3975 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -5184,11 +5184,12 @@ TEST_F(Dhcp6ParserTest, hostReservationPerSubnet) { /// - Configuration: /// - only addresses (no prefixes) - /// - 4 subnets with: + /// - 5 subnets with: /// - 2001:db8:1::/64 (all reservations enabled) /// - 2001:db8:2::/64 (out-of-pool reservations) /// - 2001:db8:3::/64 (reservations disabled) - /// - 2001:db8:3::/64 (reservations not specified) + /// - 2001:db8:4::/64 (global reservations) + /// - 2001:db8:5::/64 (reservations not specified) const char* HR_CONFIG = "{" "\"preferred-lifetime\": 3000," @@ -5231,11 +5232,11 @@ TEST_F(Dhcp6ParserTest, hostReservationPerSubnet) { checkResult(status, 0); CfgMgr::instance().commit(); - // Let's get all subnets and check that there are 4 of them. + // Let's get all subnets and check that there are 5 of them. ConstCfgSubnets6Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6(); ASSERT_TRUE(subnets); const Subnet6Collection* subnet_col = subnets->getAll(); - ASSERT_EQ(5, subnet_col->size()); // We expect 4 subnets + ASSERT_EQ(5, subnet_col->size()); // We expect 5 subnets // Let's check if the parsed subnets have correct HR modes. @@ -5266,6 +5267,63 @@ TEST_F(Dhcp6ParserTest, hostReservationPerSubnet) { EXPECT_EQ(Network::HR_ALL, subnet->getHostReservationMode()); } +/// The goal of this test is to verify that Host Reservation modes can be +/// specified globally. +TEST_F(Dhcp6ParserTest, hostReservationGlobal) { + + /// - Configuration: + /// - only addresses (no prefixes) + /// - 2 subnets with: + /// - 2001:db8:1::/64 (all reservations enabled) + /// - 2001:db8:2::/64 (reservations not specified) + const char* HR_CONFIG = + "{" + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"reservation-mode\": \"out-of-pool\", " + "\"subnet6\": [ { " + " \"pools\": [ { \"pool\": \"2001:db8:1::/64\" } ]," + " \"subnet\": \"2001:db8:1::/48\", " + " \"reservation-mode\": \"all\"" + " }," + " {" + " \"pools\": [ { \"pool\": \"2001:db8:2::/64\" } ]," + " \"subnet\": \"2001:db8:2::/48\" " + " } ]," + "\"valid-lifetime\": 4000 }"; + + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP6(HR_CONFIG)); + extractConfig(HR_CONFIG); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + + // returned value should be 0 (success) + checkResult(status, 0); + CfgMgr::instance().commit(); + + // Let's get all subnets and check that there are 2 of them. + ConstCfgSubnets6Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6(); + ASSERT_TRUE(subnets); + const Subnet6Collection* subnet_col = subnets->getAll(); + ASSERT_EQ(2, subnet_col->size()); // We expect 2 subnets + + // Let's check if the parsed subnets have correct HR modes. + + // Subnet 1 + Subnet6Ptr subnet; + subnet = subnets->selectSubnet(IOAddress("2001:db8:1::1")); + ASSERT_TRUE(subnet); + EXPECT_EQ(Network::HR_ALL, subnet->getHostReservationMode()); + + // Subnet 2 + subnet = subnets->selectSubnet(IOAddress("2001:db8:2::1")); + ASSERT_TRUE(subnet); + EXPECT_EQ(Network::HR_OUT_OF_POOL, subnet->getHostReservationMode()); +} + /// The goal of this test is to verify that configuration can include /// Relay Supplied options (specified as numbers). TEST_F(Dhcp6ParserTest, rsooNumbers) { diff --git a/src/lib/dhcpsrv/parsers/simple_parser4.cc b/src/lib/dhcpsrv/parsers/simple_parser4.cc index b6a356397a..eb83fc539d 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser4.cc +++ b/src/lib/dhcpsrv/parsers/simple_parser4.cc @@ -67,7 +67,8 @@ const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { { "next-server", Element::string, "0.0.0.0" }, { "server-hostname", Element::string, "" }, { "boot-file-name", Element::string, "" }, - { "server-tag", Element::string, "" } + { "server-tag", Element::string, "" }, + { "reservation-mode", Element::string, "all" } }; /// @brief This table defines default values for each IPv4 subnet. @@ -81,7 +82,6 @@ const SimpleDefaults SimpleParser4::SUBNET4_DEFAULTS = { { "id", Element::integer, "0" }, // 0 means autogenerate { "interface", Element::string, "" }, { "client-class", Element::string, "" }, - { "reservation-mode", Element::string, "all" }, { "4o6-interface", Element::string, "" }, { "4o6-interface-id", Element::string, "" }, { "4o6-subnet", Element::string, "" }, @@ -103,8 +103,7 @@ const SimpleDefaults SimpleParser4::SHARED_SUBNET4_DEFAULTS = { /// @brief This table defines default values for each IPv4 shared network. const SimpleDefaults SimpleParser4::SHARED_NETWORK4_DEFAULTS = { { "client-class", Element::string, "" }, - { "interface", Element::string, "" }, - { "reservation-mode", Element::string, "all" } + { "interface", Element::string, "" } }; /// @brief This table defines default values for interfaces for DHCPv4. diff --git a/src/lib/dhcpsrv/parsers/simple_parser6.cc b/src/lib/dhcpsrv/parsers/simple_parser6.cc index 8f4c45b1e2..dbb2b463b1 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser6.cc +++ b/src/lib/dhcpsrv/parsers/simple_parser6.cc @@ -63,7 +63,8 @@ const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { { "valid-lifetime", Element::integer, "7200" }, { "decline-probation-period", Element::integer, "86400" }, // 24h { "dhcp4o6-port", Element::integer, "0" }, - { "server-tag", Element::string, "" } + { "server-tag", Element::string, "" }, + { "reservation-mode", Element::string, "all" } }; /// @brief This table defines default values for each IPv6 subnet. @@ -71,9 +72,8 @@ const SimpleDefaults SimpleParser6::SUBNET6_DEFAULTS = { { "id", Element::integer, "0" }, // 0 means autogenerate { "interface", Element::string, "" }, { "client-class", Element::string, "" }, - { "reservation-mode", Element::string, "all" }, { "rapid-commit", Element::boolean, "false" }, // rapid-commit disabled by default - { "interface-id", Element::string, "" }, + { "interface-id", Element::string, "" } }; /// @brief This table defines default values for each IPv6 subnet. @@ -84,9 +84,8 @@ const SimpleDefaults SimpleParser6::SHARED_SUBNET6_DEFAULTS = { /// @brief This table defines default values for each IPv6 shared network. const SimpleDefaults SimpleParser6::SHARED_NETWORK6_DEFAULTS = { { "client-class", Element::string, "" }, - { "interface", Element::string, "" }, + { "interface", Element::string, "" }, { "interface-id", Element::string, "" }, - { "reservation-mode", Element::string, "all" }, { "rapid-commit", Element::boolean, "false" } // rapid-commit disabled by default }; -- 2.47.2