From: Tomek Mrugalski Date: Mon, 9 Oct 2017 12:13:54 +0000 (+0200) Subject: [5364] The same value of rapid-commit is now enforced on all subnets X-Git-Tag: trac5297_base~8^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aec0d6b5250d0b3ea04465409383ba23e954f65b;p=thirdparty%2Fkea.git [5364] The same value of rapid-commit is now enforced on all subnets --- diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 75073dadd5..fff616eea0 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -251,8 +251,33 @@ public: const Subnet6Collection* subnets = (*net)->getAllSubnets(); if (subnets) { + + bool rapid_commit; + // For each subnet, add it to a list of regular subnets. for (auto subnet = subnets->begin(); subnet != subnets->end(); ++subnet) { + + // Rapid commit must either be enabled or disabled in all subnets + // in the shared network. + if (subnet == subnets->begin()) { + // If this is the first subnet, remember the value. + rapid_commit = (*subnet)->getRapidCommit(); + } else { + // Ok, this is the second or following subnets. The value + // must match what was set in the first subnet. + if (rapid_commit != (*subnet)->getRapidCommit()) { + isc_throw(DhcpConfigError, "All subnets in a shared network " + "must have the same rapid-commit value. Subnet " + << (*subnet)->toText() + << " has specified rapid-commit " + << ( (*subnet)->getRapidCommit() ? "true" : "false") + << ", but earlier subnet in the same shared-network" + << " or the shared-network itself used rapid-commit " + << (rapid_commit ? "true" : "false")); + } + } + + if (iface.empty()) { iface = (*subnet)->getIface(); continue; @@ -290,8 +315,8 @@ public: } } - - + + }; } // anonymous namespace @@ -568,7 +593,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set, // it checks that there is no conflict between plain subnets and those // defined as part of shared networks. global_parser.sanityChecks(srv_config, mutable_cfg); - + } catch (const isc::Exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL) .arg(config_pair.first).arg(ex.what()); @@ -603,7 +628,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set, // Setup the command channel. configureCommandChannel(); - + // No need to commit interface names as this is handled by the // CfgMgr::commit() function. diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 6196c197d6..0c966dacfd 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -5617,7 +5617,7 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDerive) { " },\n" " \"valid-lifetime\": 400, \n" " \"interface-id\": \"twotwo\",\n" - " \"rapid-commit\": false,\n" + " \"rapid-commit\": true,\n" " \"reservation-mode\": \"out-of-pool\"\n" " }\n" " ]\n" @@ -5673,7 +5673,7 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDerive) { ASSERT_TRUE(s->getInterfaceId()); EXPECT_TRUE(iface_id2.equals(s->getInterfaceId())); EXPECT_EQ(IOAddress("2222::2"), s->getRelayInfo().addr_); - EXPECT_EQ(false, s->getRapidCommit()); + EXPECT_EQ(true, s->getRapidCommit()); EXPECT_EQ(Network::HR_OUT_OF_POOL, s->getHostReservationMode()); // Ok, now check the second shared subnet. @@ -5778,6 +5778,38 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDeriveInterfaces) { EXPECT_EQ("", s->getIface()); } + +// It is not allowed to have different values for interfaces names is subnets +// in the same shared network. +TEST_F(Dhcp6ParserTest, sharedNetworksInterfacesMixed) { + + // We need to fake the interfaces present, because we want to test + // interface names inheritance. However, there are sanity checks + // on subnet level that would refuse the value if the interface + // is not present. + IfaceMgrTestConfig iface_config(true); + + string config = "{\n" + "\"shared-networks\": [ {\n" + " \"name\": \"foo\"\n," + " \"subnet6\": [\n" + " { \n" + " \"subnet\": \"2001:db1::/48\",\n" + " \"interface\": \"eth0\"\n" + " },\n" + " { \n" + " \"subnet\": \"2001:db2::/48\",\n" + " \"interface\": \"eth1\"\n" + " }\n" + " ]\n" + " } ]\n" + "} \n"; + + configure(config, CONTROL_RESULT_ERROR, "Subnet 2001:db2::/48 has specified " + "interface eth1, but earlier subnet in the same shared-network " + "or the shared-network itself used eth0"); +} + // This test checks if client-class is derived properly. TEST_F(Dhcp6ParserTest, sharedNetworksDeriveClientClass) { @@ -5862,4 +5894,100 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDeriveClientClass) { EXPECT_TRUE(classes.empty()); } +// Tests if rapid-commit is derived properly. +TEST_F(Dhcp6ParserTest, sharedNetworksRapidCommit) { + + string config = "{\n" + "\"shared-networks\": [ {\n" + " \"name\": \"frog\"\n," + " \"rapid-commit\": true,\n" + " \"subnet6\": [\n" + " { \n" + " \"subnet\": \"2001:db1::/48\",\n" + " \"pools\": [ { \"pool\": \"2001:db1::/64\" } ]\n" + " },\n" + " { \n" + " \"subnet\": \"2001:db2::/48\",\n" + " \"pools\": [ { \"pool\": \"2001:db2::/64\" } ],\n" + " \"client-class\": \"beta\"\n" + " }\n" + " ]\n" + " },\n" + "{ // second shared-network starts here\n" + " \"name\": \"bar\",\n" + " \"rapid-commit\": false,\n" + " \"subnet6\": [\n" + " {\n" + " \"subnet\": \"2001:db3::/48\",\n" + " \"pools\": [ { \"pool\": \"2001:db3::/64\" } ]\n" + " }\n" + " ]\n" + "} ]\n" + "} \n"; + + configure(config, CONTROL_RESULT_SUCCESS, ""); + + // Now verify that the shared network was indeed configured. + CfgSharedNetworks6Ptr cfg_net = CfgMgr::instance().getStagingCfg() + ->getCfgSharedNetworks6(); + + // Two shared networks are expeced. + ASSERT_TRUE(cfg_net); + const SharedNetwork6Collection* nets = cfg_net->getAll(); + ASSERT_TRUE(nets); + ASSERT_EQ(2, nets->size()); + + // Let's check the first one. + SharedNetwork6Ptr net = nets->at(0); + ASSERT_TRUE(net); + + const Subnet6Collection * subs = net->getAllSubnets(); + ASSERT_TRUE(subs); + ASSERT_EQ(2, subs->size()); + EXPECT_TRUE(subs->at(0)->getRapidCommit()); + EXPECT_TRUE(subs->at(1)->getRapidCommit()); + + // Ok, now check the second shared network. It doesn't have + // anything defined on shared-network or subnet level, so + // everything should have default values. + net = nets->at(1); + ASSERT_TRUE(net); + + subs = net->getAllSubnets(); + ASSERT_TRUE(subs); + EXPECT_EQ(1, subs->size()); + + // This subnet should derive its renew-timer from global scope. + EXPECT_FALSE(subs->at(0)->getRapidCommit()); +} + +// Tests if rapid-commit is derived properly. +TEST_F(Dhcp6ParserTest, sharedNetworksRapidCommitMix) { + + string config = "{\n" + "\"shared-networks\": [ {\n" + " \"name\": \"frog\"\n," + " \"subnet6\": [\n" + " { \n" + " \"subnet\": \"2001:db1::/48\",\n" + " \"rapid-commit\": true,\n" + " \"pools\": [ { \"pool\": \"2001:db1::/64\" } ]\n" + " },\n" + " { \n" + " \"subnet\": \"2001:db2::/48\",\n" + " \"rapid-commit\": false,\n" + " \"pools\": [ { \"pool\": \"2001:db2::/64\" } ],\n" + " \"client-class\": \"beta\"\n" + " }\n" + " ]\n" + " } ]\n" + "} \n"; + + configure(config, CONTROL_RESULT_ERROR, "All subnets in a shared network " + "must have the same rapid-commit value. Subnet 2001:db2::/48 has " + "specified rapid-commit false, but earlier subnet in the same " + "shared-network or the shared-network itself used rapid-commit true"); +} + + }; diff --git a/src/bin/dhcp6/tests/dhcp6_client.cc b/src/bin/dhcp6/tests/dhcp6_client.cc index ec3f204be1..9682b2492d 100644 --- a/src/bin/dhcp6/tests/dhcp6_client.cc +++ b/src/bin/dhcp6/tests/dhcp6_client.cc @@ -964,6 +964,22 @@ Dhcp6Client::clearExtraOptions() { extra_options_.clear(); } +void +Dhcp6Client::printConfiguration() const { + + // Print DUID + std::cout << "Client " << (duid_ ? duid_->toText() : "(without duid)") + << " got " << getLeaseNum() << " lease(s): "; + + // Print leases + for (int i = 0; i < getLeaseNum(); i++) { + Lease6 lease = getLease(i); + std::cout << lease.addr_.toText() << " "; + } + + /// @todo: Print many other parameters. + std::cout << std::endl; +} } // end of namespace isc::dhcp::test } // end of namespace isc::dhcp diff --git a/src/bin/dhcp6/tests/dhcp6_client.h b/src/bin/dhcp6/tests/dhcp6_client.h index b3c41123cf..49c5245d12 100644 --- a/src/bin/dhcp6/tests/dhcp6_client.h +++ b/src/bin/dhcp6/tests/dhcp6_client.h @@ -749,6 +749,12 @@ public: /// @brief Configures the client to not send any extra options. void clearExtraOptions(); + /// @brief Debugging method the prints currently held configuration + /// + /// @todo: This is mostly useful when debugging tests. This method + /// is incomplete. Please extend it when needed. + void printConfiguration() const; + private: /// @brief Applies the new leases for the client. diff --git a/src/bin/dhcp6/tests/shared_network_unittest.cc b/src/bin/dhcp6/tests/shared_network_unittest.cc index adca100c6c..db20ccb44c 100644 --- a/src/bin/dhcp6/tests/shared_network_unittest.cc +++ b/src/bin/dhcp6/tests/shared_network_unittest.cc @@ -832,9 +832,7 @@ const char* NETWORKS_CONFIG[] = { "}", // Configuration #16 -// - one shared network with two subnets -// - first subnet has the rapid commit enabled -// - second subnet has the rapid commit disabled +// - one shared network with two subnets, both have rapid-commit enabled "{" " \"shared-networks\": [" " {" @@ -854,7 +852,41 @@ const char* NETWORKS_CONFIG[] = { " {" " \"subnet\": \"2001:db8:2::/64\"," " \"id\": 100," - " \"rapid-commit\": false," + " \"rapid-commit\": true," + " \"pools\": [" + " {" + " \"pool\": \"2001:db8:2::20 - 2001:db8:2::20\"" + " }" + " ]" + " }" + " ]" + " }" + " ]" + "}", + + +// Configuration #17: +// - one shared network with rapid-commit enabled +// - two subnets (which should derive the rapid-commit setting) + "{" + " \"shared-networks\": [" + " {" + " \"name\": \"frog\"," + " \"interface\": \"eth1\"," + " \"rapid-commit\": true," + " \"subnet6\": [" + " {" + " \"subnet\": \"2001:db8:1::/64\"," + " \"id\": 10," + " \"pools\": [" + " {" + " \"pool\": \"2001:db8:1::20 - 2001:db8:1::20\"" + " }" + " ]" + " }," + " {" + " \"subnet\": \"2001:db8:2::/64\"," + " \"id\": 100," " \"pools\": [" " {" " \"pool\": \"2001:db8:2::20 - 2001:db8:2::20\"" @@ -1120,6 +1152,98 @@ public: } + /// @brief Tests that for a given configuration the rapid-commit works (or not) + /// + /// The provided configuration is expected to be able to handle two clients. + /// The second parameter governs whether rapid-commit is expected to be enabled + /// or disabled. Third and fourth parameters are text representations of expected + /// leases to be assigned (if rapid-commit is enabled) + /// + /// @param config - text version of the configuration to be tested + /// @param enabled - true = rapid-commit is expected to work + /// @param exp_addr1 - an eddress the first client is expected to get (if + /// rapid-commit is enabled). + /// @param exp_addr2 - an eddress the second client is expected to get (if + /// rapid-commit is enabled). + void testRapidCommit(const std::string& config, bool enabled, + const std::string& exp_addr1, + const std::string& exp_addr2) { + + // Create client #1. This clients wants to use rapid-commit. + Dhcp6Client client1; + client1.setInterface("eth1"); + client1.useRapidCommit(true); + + Dhcp6Client client2; + client2.setInterface("eth1"); + client2.useRapidCommit(true); + + // Configure the server with a shared network. + ASSERT_NO_FATAL_FAILURE(configure(config, *client1.getServer())); + + // Ok, client should have one + EXPECT_EQ(0, client1.getLeaseNum()); + + // Client #1 should be assigned an address from shared network. The first + // subnet has rapid-commit enabled, so the address should be assigned. + ASSERT_NO_THROW(client1.requestAddress(0xabca0)); + testAssigned([this, &client1] { + ASSERT_NO_THROW(client1.doSolicit()); + }); + + // Make sure something was sent back. + ASSERT_TRUE(client1.getContext().response_); + + if (enabled) { + // rapid-commit enabled. + + // Make sure that REPLY was sent back. + EXPECT_EQ(DHCPV6_REPLY, client1.getContext().response_->getType()); + + // Just make sure the client didn't get an address. + EXPECT_TRUE(hasLeaseForAddress(client1, IOAddress(exp_addr1), + LeaseOnServer::MUST_EXIST)); + } else { + // rapid-commit disabled. + + // Make sure that ADVERTISE was sent back. + EXPECT_EQ(DHCPV6_ADVERTISE, client1.getContext().response_->getType()); + + // And that it doesn't have any leases. + EXPECT_EQ(0, client1.getLeaseNum()); + } + + // Create client #2. This client behaves the same as the first one, but the + // first subnet is already full (it's a really small subnet) and the second + // subnet does not allow rapid-commit. + ASSERT_NO_THROW(client2.requestAddress(0xabca0)); + testAssigned([this, &client2] { + ASSERT_NO_THROW(client2.doSolicit()); + }); + + // Make sure something was sent back. + ASSERT_TRUE(client2.getContext().response_); + + if (enabled) { + // rapid-commit enabled. + + // Make sure that REPLY was sent back. + EXPECT_EQ(DHCPV6_REPLY, client2.getContext().response_->getType()); + + // Just make sure the client didn't get an address. + EXPECT_TRUE(hasLeaseForAddress(client2, IOAddress(exp_addr2), + LeaseOnServer::MUST_EXIST)); + } else { + // rapid-commit disabled. + + // Make sure that ADVERTISE was sent back. + EXPECT_EQ(DHCPV6_ADVERTISE, client1.getContext().response_->getType()); + + // And that it doesn't have any leases. + EXPECT_EQ(0, client1.getLeaseNum()); + } + } + /// @brief Destructor. virtual ~Dhcpv6SharedNetworkTest() { StatsMgr::instance().removeAll(); @@ -1895,67 +2019,22 @@ TEST_F(Dhcpv6SharedNetworkTest, sharedNetworkSelectedByInterfaceIdInSubnet) { ASSERT_TRUE(hasLeaseForAddress(client2, IOAddress("2001:db8:2::20"))); } -// Check that the rapid-commit works with shared networks: -// - that it can be defined on per subnet basis -// - that its value can be mixed (some subnets have enabled, others disabled) -TEST_F(Dhcpv6SharedNetworkTest, sharedNetworkRapidCommit) { - - // Create client #1. This clients wants to use rapid-commit. - Dhcp6Client client1; - client1.setInterface("eth1"); - client1.useRapidCommit(true); - - Dhcp6Client client2; - client2.setInterface("eth1"); - client2.useRapidCommit(true); - - // Configure the server with a shared network. - ASSERT_NO_FATAL_FAILURE(configure(NETWORKS_CONFIG[16], *client1.getServer())); - - // Ok, client should have one - EXPECT_EQ(0, client1.getLeaseNum()); - - std::cout << "Client 1 got " << client1.getLeaseNum() << " lease(s): "; - for (int i = 0; i < client1.getLeaseNum(); i++) { - Lease6 lease = client1.getLease(i); - std::cout << lease.addr_.toText() << " "; - } - std::cout << std::endl; - - // Client #1 should be assigned an address from shared network. The first - // subnet has rapid-commit enabled, so the address should be assigned. - ASSERT_NO_THROW(client1.requestAddress(0xabca0)); - testAssigned([this, &client1] { - ASSERT_NO_THROW(client1.doSolicit()); - }); - - std::cout << "Client 1 got " << client1.getLeaseNum() << " lease(s): "; - for (int i = 0; i < client1.getLeaseNum(); i++) { - Lease6 lease = client1.getLease(i); - std::cout << lease.addr_.toText() << " "; - } - std::cout << std::endl; - - // Make sure that REPLY was sent back. - ASSERT_TRUE(client1.getContext().response_); - EXPECT_EQ(DHCPV6_REPLY, client1.getContext().response_->getType()); - - // Create client #2. This client behaves the same as the first one, but the - // first subnet is already full (it's a really small subnet) and the second - // subnet does not allow rapid-commit. - testAssigned([this, &client2] { - ASSERT_NO_THROW(client2.doSolicit()); - }); - - EXPECT_EQ(0, client2.getLeaseNum()); +// Check that the rapid-commit works with shared networks. Rapid-commit +// enabled on each subnet separately. +TEST_F(Dhcpv6SharedNetworkTest, sharedNetworkRapidCommit1) { + testRapidCommit(NETWORKS_CONFIG[16], true, "2001:db8:1::20", "2001:db8:2::20"); +} - // Make sure that ADVERTISE was sent back. - ASSERT_TRUE(client2.getContext().response_); - EXPECT_EQ(DHCPV6_ADVERTISE, client2.getContext().response_->getType()); +// Check that the rapid-commit works with shared networks. Rapid-commit +// enabled for the whole shared network. This should be applied to both +// subnets. +TEST_F(Dhcpv6SharedNetworkTest, sharedNetworkRapidCommit2) { + testRapidCommit(NETWORKS_CONFIG[17], true, "2001:db8:1::20", "2001:db8:2::20"); +} - // Just make sure the client didn't get an address. - EXPECT_FALSE(hasLeaseForAddress(client2, IOAddress("2001:db8:2::20"), - LeaseOnServer::MUST_NOT_EXIST)); +// Check that the rapid-commit is disabled by default. +TEST_F(Dhcpv6SharedNetworkTest, sharedNetworkRapidCommit3) { + testRapidCommit(NETWORKS_CONFIG[1], false, "", ""); } } // end of anonymous namespace