From b82db9d608868c11c68d692458769e9afb47f871 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 11 Apr 2013 14:05:25 +0200 Subject: [PATCH 1/8] [2898] Support for v6 relayed traffic added. --- src/bin/dhcp6/config_parser.cc | 28 +++- src/bin/dhcp6/dhcp6.spec | 6 + src/bin/dhcp6/dhcp6_srv.cc | 50 +++++-- src/bin/dhcp6/tests/config_parser_unittest.cc | 104 +++++++++++++++ src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 124 ++++++++++++++++++ src/lib/dhcp/pkt6.cc | 81 ++++++++++++ src/lib/dhcp/pkt6.h | 47 ++++++- src/lib/dhcp/tests/pkt6_unittest.cc | 103 +++++++++++++++ src/lib/dhcpsrv/cfgmgr.cc | 21 ++- src/lib/dhcpsrv/cfgmgr.h | 1 - src/lib/dhcpsrv/dhcpsrv_messages.mes | 7 + src/lib/dhcpsrv/subnet.h | 16 +++ src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 116 ++++++++++++++-- src/lib/dhcpsrv/tests/subnet_unittest.cc | 16 +++ 14 files changed, 692 insertions(+), 28 deletions(-) diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index 0c093619d8..243d217605 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -1481,11 +1481,27 @@ private: std::string iface; try { iface = string_values_.getParam("interface"); - } catch (DhcpConfigError) { + } catch (const DhcpConfigError&) { // iface not mandatory so swallow the exception } - /// @todo: Convert this to logger once the parser is working reliably + // Get interface-id option content. For now we support string + // represenation only + std::string ifaceid; + try { + ifaceid = string_values_.getParam("interface-id"); + } catch (DhcpConfigError) { + // interface-id is not mandatory + } + + if (!iface.empty() && !ifaceid.empty()) { + isc_throw(isc::dhcp::DhcpConfigError, + "parser error: interface (defined for locally reachable " + "subnets) and interface-id (defined for subnets reachable" + " via relays) cannot be defined at the same time for " + "subnet " << addr.toText() << "/" << (int)len); + } + stringstream tmp; tmp << addr.toText() << "/" << (int)len << " with params t1=" << t1 << ", t2=" << t2 << ", pref=" @@ -1512,6 +1528,13 @@ private: subnet_->setIface(iface); } + // Configure interface-id for remote interfaces, if defined + if (!ifaceid.empty()) { + OptionBuffer tmp(ifaceid.begin(), ifaceid.end()); + OptionPtr opt(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); + subnet_->setInterfaceId(opt); + } + // We are going to move configured options to the Subnet object. // Configured options reside in the container where options // are grouped by space names. Thus we need to get all space names @@ -1591,6 +1614,7 @@ private: factories["pool"] = PoolParser::factory; factories["option-data"] = OptionDataListParser::factory; factories["interface"] = StringParser::factory; + factories["interface-id"] = StringParser::factory; FactoryMap::iterator f = factories.find(config_id); if (f == factories.end()) { diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index 1129aec717..bb5de0a086 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -199,6 +199,12 @@ "item_default": "" }, + { "item_name": "interface-id", + "item_type": "string", + "item_optional": false, + "item_default": "" + }, + { "item_name": "renew-timer", "item_type": "integer", "item_optional": false, diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 75a5337a17..ec74bb1810 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -403,8 +403,13 @@ Dhcpv6Srv::copyDefaultOptions(const Pkt6Ptr& question, Pkt6Ptr& answer) { if (clientid) { answer->addOption(clientid); } + /// @todo: Should throw if there is no client-id (except anonymous INF-REQUEST) + + // if this is a relayed message, we need to copy relay information + if (!question->relay_info_.empty()) { + answer->copyRelayInfo(question); + } - // TODO: Should throw if there is no client-id (except anonymous INF-REQUEST) } void @@ -523,18 +528,43 @@ Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt, RequirementLevel clientid, Subnet6Ptr Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) { - /// @todo: pass interface information only if received direct (non-relayed) message + Subnet6Ptr subnet; - // Try to find a subnet if received packet from a directly connected client - Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(question->getIface()); - if (subnet) { + if (question->relay_info_.empty()) { + // This is a direct (non-relayed) message + + // Try to find a subnet if received packet from a directly connected client + Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(question->getIface()); + if (subnet) { + return (subnet); + } + + // If no subnet was found, try to find it based on remote address + subnet = CfgMgr::instance().getSubnet6(question->getRemoteAddr()); + return (subnet); + } else { + + // This is a relayed message + OptionPtr interface_id = question->getAnyRelayOption(D6O_INTERFACE_ID, + Pkt6::RELAY_SEARCH_FIRST); + if (interface_id) { + subnet = CfgMgr::instance().getSubnet6(interface_id); + } + + if (subnet) { + return (subnet); + } + + // If no interface-id was specified (or not configured on server), let's + // try address matching + IOAddress link_addr = question->relay_info_.back().linkaddr_; + + // if relay filled in link_addr field, then let's use it + if (link_addr != IOAddress("::")) { + subnet = CfgMgr::instance().getSubnet6(link_addr); + } return (subnet); } - - // If no subnet was found, try to find it based on remote address - subnet = CfgMgr::instance().getSubnet6(question->getRemoteAddr()); - - return (subnet); } void diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index f4ebf0c848..37dd783cc9 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -500,6 +500,110 @@ TEST_F(Dhcp6ParserTest, interfaceGlobal) { EXPECT_EQ(1, rcode_); } + +// This test checks if it is possible to define a subnet with an +// interface-id option defined. +TEST_F(Dhcp6ParserTest, subnetInterfaceId) { + + const string valid_interface_id = "foobar"; + const string bogus_interface_id = "blah"; + + ConstElementPtr status; + + // There should be at least one interface + + string config = "{ " + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet6\": [ { " + " \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ]," + " \"interface-id\": \"" + valid_interface_id + "\"," + " \"subnet\": \"2001:db8:1::/64\" } ]," + "\"valid-lifetime\": 4000 }"; + cout << config << endl; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + + // returned value should be 0 (configuration success) + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + EXPECT_EQ(0, rcode_); + + // try to get a subnet based on bogus interface-id option + OptionBuffer tmp(bogus_interface_id.begin(), bogus_interface_id.end()); + OptionPtr ifaceid(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); + Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(ifaceid); + EXPECT_FALSE(subnet); + + // now try to get subnet for valid interface-id value + tmp = OptionBuffer(valid_interface_id.begin(), valid_interface_id.end()); + ifaceid.reset(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); + subnet = CfgMgr::instance().getSubnet6(ifaceid); + ASSERT_TRUE(subnet); + EXPECT_TRUE(ifaceid->equal(subnet->getInterfaceId())); +} + + +// This test checks if it is not allowed to define global interface +// parameter. +TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { + + ConstElementPtr status; + + string config = "{ \"interface\": [ \"all\" ]," + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"interface-id\": \"foobar\"," // Not valid. Can be defined in subnet only + "\"subnet6\": [ { " + " \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ]," + " \"subnet\": \"2001:db8:1::/64\" } ]," + "\"valid-lifetime\": 4000 }"; + cout << config << endl; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + + // returned value should be 1 (parse error) + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + EXPECT_EQ(1, rcode_); +} + +// This test checks if it is not possible to define a subnet with an +// interface (i.e. local subnet) and interface-id (remote subnet) defined. +TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) { + + ConstElementPtr status; + + string config = "{ \"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet6\": [ { " + " \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ]," + " \"interface\": \"" + valid_iface_ + "\"," + " \"interface-id\": \"foobar\"," + " \"subnet\": \"2001:db8:1::/64\" } ]," + "\"valid-lifetime\": 4000 }"; + cout << config << endl; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + + // returned value should be 0 (configuration success) + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + EXPECT_EQ(1, rcode_); + +} + + + // Test verifies that a subnet with pool values that do not belong to that // pool are rejected. TEST_F(Dhcp6ParserTest, poolOutOfSubnet) { diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 03b2c0cdab..d63d518b39 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -98,6 +98,16 @@ public: return (ia); } + /// @brief generates interface-id option, based on text + /// + /// @param iface_id textual representation of the interface-id content + /// + /// @return pointer to the option object + OptionPtr generateInterfaceId(const string& iface_id) { + OptionBuffer tmp(iface_id.begin(), iface_id.end()); + return OptionPtr(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); + } + // Generate client-id option OptionPtr generateClientId(size_t duid_size = 32) { @@ -678,6 +688,7 @@ TEST_F(Dhcpv6SrvTest, SolicitBasic) { // check that IA_NA was returned and that there's an address included boost::shared_ptr addr = checkIA_NA(reply, 234, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr); // Check that the assigned address is indeed from the configured pool checkIAAddr(addr, addr->getAddress(), subnet_->getPreferred(), subnet_->getValid()); @@ -731,6 +742,7 @@ TEST_F(Dhcpv6SrvTest, SolicitHint) { // check that IA_NA was returned and that there's an address included boost::shared_ptr addr = checkIA_NA(reply, 234, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr); // check that we've got the address we requested checkIAAddr(addr, hint, subnet_->getPreferred(), subnet_->getValid()); @@ -779,6 +791,7 @@ TEST_F(Dhcpv6SrvTest, SolicitInvalidHint) { // check that IA_NA was returned and that there's an address included boost::shared_ptr addr = checkIA_NA(reply, 234, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr); // Check that the assigned address is indeed from the configured pool checkIAAddr(addr, addr->getAddress(), subnet_->getPreferred(), subnet_->getValid()); @@ -840,6 +853,9 @@ TEST_F(Dhcpv6SrvTest, ManySolicits) { subnet_->getT2()); boost::shared_ptr addr3 = checkIA_NA(reply3, 3, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr1); + ASSERT_TRUE(addr2); + ASSERT_TRUE(addr3); // Check that the assigned address is indeed from the configured pool checkIAAddr(addr1, addr1->getAddress(), subnet_->getPreferred(), subnet_->getValid()); @@ -910,6 +926,7 @@ TEST_F(Dhcpv6SrvTest, RequestBasic) { // check that IA_NA was returned and that there's an address included boost::shared_ptr addr = checkIA_NA(reply, 234, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr); // check that we've got the address we requested checkIAAddr(addr, hint, subnet_->getPreferred(), subnet_->getValid()); @@ -1592,6 +1609,113 @@ TEST_F(Dhcpv6SrvTest, selectSubnetIface) { EXPECT_EQ(subnet3, srv.selectSubnet(pkt)); } +// This test verifies if selectSubnet() selects proper subnet for a given +// linkaddr in RELAY-FORW message +TEST_F(Dhcpv6SrvTest, selectSubnetRelayLinkaddr) { + NakedDhcpv6Srv srv(0); + + Subnet6Ptr subnet1(new Subnet6(IOAddress("2001:db8:1::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet2(new Subnet6(IOAddress("2001:db8:2::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet3(new Subnet6(IOAddress("2001:db8:3::"), 48, 1, 2, 3, 4)); + + Pkt6::RelayInfo relay; + relay.linkaddr_ = IOAddress("2001:db8:2::1234"); + relay.peeraddr_ = IOAddress("fe80::1"); + + // CASE 1: We have only one subnet defined and we received relayed traffic. + // The only available subnet should NOT be selected. + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet1); // just a single subnet + + Pkt6Ptr pkt = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)); + pkt->relay_info_.push_back(relay); + + Subnet6Ptr selected = srv.selectSubnet(pkt); + EXPECT_FALSE(selected); + + // CASE 2: We have three subnets defined and we received relayed traffic. + // Nothing should be selected. + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet1); + CfgMgr::instance().addSubnet6(subnet2); + CfgMgr::instance().addSubnet6(subnet3); + selected = srv.selectSubnet(pkt); + EXPECT_EQ(selected, subnet2); + + // CASE 3: We have three subnets defined and we received relayed traffic + // that came out of subnet 2. We should select subnet2 then + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet1); + CfgMgr::instance().addSubnet6(subnet2); + CfgMgr::instance().addSubnet6(subnet3); + + // source of the packet should have no meaning. Selection is based + // on linkaddr field in the relay + pkt->setRemoteAddr(IOAddress("2001:db8:1::baca")); + selected = srv.selectSubnet(pkt); + EXPECT_EQ(selected, subnet2); + + // CASE 4: We have three subnets defined and we received relayed traffic + // that came out of undefined subnet. We should select nothing + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet1); + CfgMgr::instance().addSubnet6(subnet2); + CfgMgr::instance().addSubnet6(subnet3); + pkt->relay_info_.clear(); + relay.linkaddr_ = IOAddress("2001:db8:4::1234"); + pkt->relay_info_.push_back(relay); + selected = srv.selectSubnet(pkt); + EXPECT_FALSE(selected); + +} + +// This test verifies if selectSubnet() selects proper subnet for a given +// interface-id option +TEST_F(Dhcpv6SrvTest, selectSubnetRelayInterfaceId) { + NakedDhcpv6Srv srv(0); + + Subnet6Ptr subnet1(new Subnet6(IOAddress("2001:db8:1::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet2(new Subnet6(IOAddress("2001:db8:2::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet3(new Subnet6(IOAddress("2001:db8:3::"), 48, 1, 2, 3, 4)); + + subnet1->setInterfaceId(generateInterfaceId("relay1")); + subnet2->setInterfaceId(generateInterfaceId("relay2")); + + // CASE 1: We have only one subnet defined and it is for interface-id "relay1" + // Packet came with interface-id "relay2". We should not select subnet1 + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet1); // just a single subnet + + Pkt6Ptr pkt = Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)); + Pkt6::RelayInfo relay; + relay.linkaddr_ = IOAddress("2001:db8:2::1234"); + relay.peeraddr_ = IOAddress("fe80::1"); + OptionPtr opt = generateInterfaceId("relay2"); + relay.options_.insert(pair(opt->getType(), opt)); + pkt->relay_info_.push_back(relay); + + // There is only one subnet configured and we are outside of that subnet + Subnet6Ptr selected = srv.selectSubnet(pkt); + EXPECT_FALSE(selected); + + // CASE 2: We have only one subnet defined and it is for interface-id "relay2" + // Packet came with interface-id "relay2". We should select it + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet2); // just a single subnet + selected = srv.selectSubnet(pkt); + EXPECT_EQ(selected, subnet2); + + // CASE 3: We have only 3 subnets defined: one remote for interface-id "relay1", + // one remote for interface-id "relay2" and third local + // packet comes with interface-id "relay2". We should select subnet2 + CfgMgr::instance().deleteSubnets6(); + CfgMgr::instance().addSubnet6(subnet1); + CfgMgr::instance().addSubnet6(subnet2); + CfgMgr::instance().addSubnet6(subnet3); + + EXPECT_EQ(subnet2, srv.selectSubnet(pkt)); +} + // This test verifies if the server-id disk operations (read, write) are // working properly. TEST_F(Dhcpv6SrvTest, ServerID) { diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index c97281e9ce..921b8e6ea0 100644 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -72,6 +72,60 @@ uint16_t Pkt6::len() { } } +OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) { + int start = 0; + int end = 0; + int direction = 0; + + if (relay_info_.empty()) { + // there's no relay info, this is a direct message + return (OptionPtr()); + } + + switch (order) { + case RELAY_SEARCH_FROM_CLIENT: + // search backwards + start = relay_info_.size() - 1; + end = 0; + direction = -1; + break; + case RELAY_SEARCH_FROM_SERVER: + // search forward + start = 0; + end = relay_info_.size() - 1; + direction = 1; + break; + case RELAY_SEARCH_FIRST: + // look at the innermost relay only + start = relay_info_.size() - 1; + end = start; + direction = 0; + break; + case RELAY_SEARCH_LAST: + // look at the outermost relay only + start = 0; + end = 0; + direction = 0; + } + + // this is a tricky loop. It must go from start to end, but it must work in + // both directions (start > end; or start < end). We can't use regular + // exit condition, because we don't know whether to use i <= end or i >= end + for (int i = start; ; i += direction) { + OptionPtr opt = getRelayOption(opt_type, i); + if (opt) { + return (opt); + } + + if (i == end) { + break; + } + } + + return getRelayOption(opt_type, end); +} + + OptionPtr Pkt6::getRelayOption(uint16_t opt_type, uint8_t relay_level) { if (relay_level >= relay_info_.size()) { isc_throw(OutOfRange, "This message was relayed " << relay_info_.size() << " time(s)." @@ -483,5 +537,32 @@ const char* Pkt6::getName() const { return (getName(getType())); } +void Pkt6::copyRelayInfo(const Pkt6Ptr& question) { + + // we use index rather than iterator, because we need that as a parameter + // passed to getRelayOption() + for (int i = 0; i < question->relay_info_.size(); ++i) { + RelayInfo x; + x.msg_type_ = DHCPV6_RELAY_REPL; + x.hop_count_ = question->relay_info_[i].hop_count_; + x.linkaddr_ = question->relay_info_[i].linkaddr_; + x.peeraddr_ = question->relay_info_[i].peeraddr_; + + // Is there interface-id option in this nesting level if there is, + // we need to echo it back + OptionPtr opt = question->getRelayOption(D6O_INTERFACE_ID, i); + // taken from question->RelayInfo_[i].options_ + if (opt) { + x.options_.insert(pair >(opt->getType(), opt)); + } + + /// @todo: implement support for ERO (Echo Request Option, RFC4994) + + // add this relay-repl info to our message + relay_info_.push_back(x); + } +} + + } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index 0bf4192dd4..09050c5666 100644 --- a/src/lib/dhcp/pkt6.h +++ b/src/lib/dhcp/pkt6.h @@ -30,6 +30,9 @@ namespace isc { namespace dhcp { +class Pkt6; +typedef boost::shared_ptr Pkt6Ptr; + class Pkt6 { public: /// specifies non-relayed DHCPv6 packet header length (over UDP) @@ -44,6 +47,28 @@ public: TCP = 1 // there are TCP DHCPv6 packets (bulk leasequery, failover) }; + /// @brief defines relay search pattern + /// + /// Defines order in which options are searched in a message that + /// passed through mulitple relays. RELAY_SEACH_FROM_CLIENT will + /// start search from the relay that was the closest to the client + /// (i.e. innermost in the encapsulated message, which also means + /// this was the first relay that forwarded packet received by the + /// server and this will be the last relay that will handle the + /// response that server sent towards the client.). + /// RELAY_SEARCH_FROM_SERVER is the opposite. This will be the + /// relay closest to the server (i.e. outermost in the encapsulated + /// message, which also means it was the last relay that relayed + /// the received message and will be the first one to process + /// server's response). RELAY_SEARCH_FIRST is option from the first + /// relay (closest to the client), RELAY_SEARCH_LAST is the + /// last relay (closest to the server). + enum RelaySearchOrder { + RELAY_SEARCH_FROM_CLIENT = 1, + RELAY_SEARCH_FROM_SERVER = 2, + RELAY_SEARCH_FIRST = 3, + RELAY_SEARCH_LAST = 4 + }; /// @brief structure that describes a single relay information /// @@ -201,6 +226,18 @@ public: /// @return pointer to the option (or NULL if there is no such option) OptionPtr getRelayOption(uint16_t option_code, uint8_t nesting_level); + /// @brief returns first instance of a specified option + /// + /// When client's packet traverses multiple relays, each passing relay + /// may insert extra options. This method allows getting specific instance + /// of a given option (closest to the client, closest to the server, etc.) + /// See @ref RelaySearchOrder for detailed description. + /// + /// @param option_code searched option + /// @param order option search order (see @ref RelaySearchOrder) + /// @return option pointer (or NULL if no option matches specified criteria) + OptionPtr getAnyRelayOption(uint16_t option_code, RelaySearchOrder order); + /// @brief Returns all instances of specified type. /// /// Returns all instances of options of the specified type. DHCPv6 protocol @@ -356,6 +393,14 @@ public: /// be freed by the caller. const char* getName() const; + /// @brief copies relay information from client's packet to server's response + /// + /// This information is not simply copied over. Some parameter are + /// removed, msg_type_is updated (RELAY-FORW => RELAY-REPL), etc. + /// + /// @param question client's packet + void copyRelayInfo(const Pkt6Ptr& question); + /// relay information /// /// this is a public field. Otherwise we hit one of the two problems: @@ -494,8 +539,6 @@ protected: boost::posix_time::ptime timestamp_; }; // Pkt6 class -typedef boost::shared_ptr Pkt6Ptr; - } // isc::dhcp namespace } // isc namespace diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index b5a745e77f..5dee36c3d5 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -44,6 +45,18 @@ class Pkt6Test : public ::testing::Test { public: Pkt6Test() { } + + /// @brief generates an option with given code (and length) and random content + /// + /// @param code option code + /// @param len data length (data will be randomized) + /// + /// @return pointer to the new option + OptionPtr generateRandomOption(uint16_t code, size_t len = 10) { + OptionBuffer data(len); + util::fillRandom(data.begin(), data.end()); + return OptionPtr(new Option(Option::V6, code, data)); + } }; TEST_F(Pkt6Test, constructor) { @@ -552,4 +565,94 @@ TEST_F(Pkt6Test, relayPack) { EXPECT_EQ(0, memcmp(relay_opt_data, relay_opt_data, sizeof(relay_opt_data))); } + +// This test verified that options added by relays to the message can be +// accessed and retrieved properly +TEST_F(Pkt6Test, getAnyRelayOption) { + + boost::scoped_ptr msg(new Pkt6(DHCPV6_ADVERTISE, 0x020304)); + msg->addOption(generateRandomOption(300)); + + // generate options for relay1 + Pkt6::RelayInfo relay1; + + // generate 3 options with code 200,201,202 and random content + OptionPtr relay1_opt1(generateRandomOption(200)); + OptionPtr relay1_opt2(generateRandomOption(201)); + OptionPtr relay1_opt3(generateRandomOption(202)); + + relay1.options_.insert(pair >(200, relay1_opt1)); + relay1.options_.insert(pair >(201, relay1_opt2)); + relay1.options_.insert(pair >(202, relay1_opt3)); + msg->addRelayInfo(relay1); + + // generate options for relay2 + Pkt6::RelayInfo relay2; + OptionPtr relay2_opt1(new Option(Option::V6, 100)); + OptionPtr relay2_opt2(new Option(Option::V6, 101)); + OptionPtr relay2_opt3(new Option(Option::V6, 102)); + OptionPtr relay2_opt4(new Option(Option::V6, 200)); // the same code as relay1_opt3 + relay2.options_.insert(pair >(100, relay2_opt1)); + relay2.options_.insert(pair >(101, relay2_opt2)); + relay2.options_.insert(pair >(102, relay2_opt3)); + relay2.options_.insert(pair >(200, relay2_opt4)); + msg->addRelayInfo(relay2); + + // generate options for relay3 + Pkt6::RelayInfo relay3; + OptionPtr relay3_opt1(generateRandomOption(200, 7)); + relay3.options_.insert(pair >(200, relay3_opt1)); + msg->addRelayInfo(relay3); + + // Ok, so we now have a packet that traversed the following network: + // client---relay3---relay2---relay1---server + + // First check that the getAnyRelayOption does not confuse client options + // and relay options + OptionPtr opt = msg->getAnyRelayOption(300, Pkt6::RELAY_SEARCH_FROM_CLIENT); + // 300 is a client option, present in the message itself. + EXPECT_FALSE(opt); + + // Option 200 is added in every relay. + + // We want to get that one inserted by relay3 (first match, starting from + // closest to the client. + opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_CLIENT); + ASSERT_TRUE(opt); + EXPECT_TRUE(opt->equal(relay3_opt1)); + + // We want to ge that one inserted by relay1 (first match, starting from + // closest to the server. + opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_SERVER); + ASSERT_TRUE(opt); + EXPECT_TRUE(opt->equal(relay1_opt1)); + + // We just want option from the first relay (closest to the client) + opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FIRST); + ASSERT_TRUE(opt); + EXPECT_TRUE(opt->equal(relay3_opt1)); + + // We just want option from the last relay (closest to the server) + opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_LAST); + ASSERT_TRUE(opt); + EXPECT_TRUE(opt->equal(relay1_opt1)); + + // Let's try to ask for something that is inserted by the middle relay + // only. + opt = msg->getAnyRelayOption(100, Pkt6::RELAY_SEARCH_FROM_SERVER); + ASSERT_TRUE(opt); + EXPECT_TRUE(opt->equal(relay2_opt1)); + + opt = msg->getAnyRelayOption(100, Pkt6::RELAY_SEARCH_FROM_CLIENT); + ASSERT_TRUE(opt); + EXPECT_TRUE(opt->equal(relay2_opt1)); + + opt = msg->getAnyRelayOption(100, Pkt6::RELAY_SEARCH_FIRST); + EXPECT_FALSE(opt); + + opt = msg->getAnyRelayOption(100, Pkt6::RELAY_SEARCH_LAST); + EXPECT_FALSE(opt); + +} + } diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index b5e83e34ad..8c0742c136 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -178,14 +178,29 @@ CfgMgr::getSubnet6(const isc::asiolink::IOAddress& hint) { return (Subnet6Ptr()); } -Subnet6Ptr CfgMgr::getSubnet6(OptionPtr /*interfaceId*/) { - /// @todo: Implement get subnet6 by interface-id (for relayed traffic) - isc_throw(NotImplemented, "Relayed DHCPv6 traffic is not supported yet."); +Subnet6Ptr CfgMgr::getSubnet6(OptionPtr iface_id_option) { + if (!iface_id_option) { + return (Subnet6Ptr()); + } + + // If there is more than one, we need to choose the proper one + for (Subnet6Collection::iterator subnet = subnets6_.begin(); + subnet != subnets6_.end(); ++subnet) { + if ( (*subnet)->getInterfaceId() && + ((*subnet)->getInterfaceId()->equal(iface_id_option))) { + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, + DHCPSRV_CFGMGR_SUBNET6_IFACE_ID) + .arg((*subnet)->toText()); + return (*subnet); + } + } + return (Subnet6Ptr()); } 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 LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_SUBNET6) .arg(subnet->toText()); subnets6_.push_back(subnet); diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 00f7e251df..05c1752944 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -174,7 +174,6 @@ public: /// @param interface_id content of interface-id option returned by a relay /// /// @return a subnet object - /// @todo This method is not currently supported. Subnet6Ptr getSubnet6(OptionPtr interface_id); /// @brief adds an IPv6 subnet diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 9b8a0ce9a0..ff8c36e13c 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -107,6 +107,13 @@ given interface. This particular subnet was selected, because it was specified as being directly reachable over given interface. (see 'interface' parameter in subnet6 definition). +% DHCPSRV_CFGMGR_SUBNET6_IFACE_ID selected subnet %1 (interface-id match) for incoming packet +This is a debug message reporting that the DHCP configuration manager +has returned the specified IPv6 subnet for a received packet. This particular +subnet was selected, because value of interface-id option matched what was +configured in server's interface-id option for that selected subnet6. +(see 'interface-id' parameter in subnet6 definition). + % DHCPSRV_CLOSE_DB closing currently open %1 database This is a debug message, issued when the DHCP server closes the currently open lease database. It is issued at program shutdown and whenever diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 4c7cbccbf8..107ff1d418 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -463,6 +463,19 @@ public: /// @return network interface name for directly attached subnets or "" std::string getIface() const; + /// @brief sets interface-id option (if defined) + /// + /// @param ifaceid pointer to interface-id option + void setInterfaceId(const OptionPtr& ifaceid) { + interface_id_ = ifaceid; + } + + /// @brief returns interface-id value (if specified) + /// @return interface-id option (if defined) + OptionPtr getInterfaceId() const { + return interface_id_; + } + protected: /// @brief Check if option is valid and can be added to a subnet. @@ -478,6 +491,9 @@ protected: return (isc::asiolink::IOAddress("::")); } + /// @brief specifies optional interface-id + OptionPtr interface_id_; + /// @brief collection of pools in that list Pool6Collection pools_; diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index be31bab840..2ffe5da68b 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -37,7 +38,7 @@ using boost::scoped_ptr; namespace { -// This test verifies that BooleanStorage functions properly. +// This test verifies that BooleanStorage functions properly. TEST(ValueStorageTest, BooleanTesting) { BooleanStorage testStore; @@ -48,7 +49,7 @@ TEST(ValueStorageTest, BooleanTesting) { EXPECT_FALSE(testStore.getParam("firstBool")); EXPECT_TRUE(testStore.getParam("secondBool")); - // Verify that we can update paramaters. + // Verify that we can update paramaters. testStore.setParam("firstBool", true); testStore.setParam("secondBool", false); @@ -65,7 +66,7 @@ TEST(ValueStorageTest, BooleanTesting) { // Verify that looking for a parameter that never existed throws. ASSERT_THROW(testStore.getParam("bogusBool"), isc::dhcp::DhcpConfigError); - // Verify that attempting to delete a parameter that never existed does not throw. + // Verify that attempting to delete a parameter that never existed does not throw. EXPECT_NO_THROW(testStore.delParam("bogusBool")); // Verify that we can empty the list. @@ -74,21 +75,21 @@ TEST(ValueStorageTest, BooleanTesting) { } -// This test verifies that Uint32Storage functions properly. +// This test verifies that Uint32Storage functions properly. TEST(ValueStorageTest, Uint32Testing) { Uint32Storage testStore; uint32_t intOne = 77; uint32_t intTwo = 33; - // Verify that we can add and retrieve parameters. + // Verify that we can add and retrieve parameters. testStore.setParam("firstInt", intOne); testStore.setParam("secondInt", intTwo); EXPECT_EQ(testStore.getParam("firstInt"), intOne); EXPECT_EQ(testStore.getParam("secondInt"), intTwo); - // Verify that we can update parameters. + // Verify that we can update parameters. testStore.setParam("firstInt", --intOne); testStore.setParam("secondInt", ++intTwo); @@ -105,7 +106,7 @@ TEST(ValueStorageTest, Uint32Testing) { // Verify that looking for a parameter that never existed throws. ASSERT_THROW(testStore.getParam("bogusInt"), isc::dhcp::DhcpConfigError); - // Verify that attempting to delete a parameter that never existed does not throw. + // Verify that attempting to delete a parameter that never existed does not throw. EXPECT_NO_THROW(testStore.delParam("bogusInt")); // Verify that we can empty the list. @@ -113,7 +114,7 @@ TEST(ValueStorageTest, Uint32Testing) { EXPECT_THROW(testStore.getParam("secondInt"), isc::dhcp::DhcpConfigError); } -// This test verifies that StringStorage functions properly. +// This test verifies that StringStorage functions properly. TEST(ValueStorageTest, StringTesting) { StringStorage testStore; @@ -127,7 +128,7 @@ TEST(ValueStorageTest, StringTesting) { EXPECT_EQ(testStore.getParam("firstString"), stringOne); EXPECT_EQ(testStore.getParam("secondString"), stringTwo); - // Verify that we can update parameters. + // Verify that we can update parameters. stringOne.append("-boo"); stringTwo.append("-boo"); @@ -147,7 +148,7 @@ TEST(ValueStorageTest, StringTesting) { // Verify that looking for a parameter that never existed throws. ASSERT_THROW(testStore.getParam("bogusString"), isc::dhcp::DhcpConfigError); - // Verify that attempting to delete a parameter that never existed does not throw. + // Verify that attempting to delete a parameter that never existed does not throw. EXPECT_NO_THROW(testStore.delParam("bogusString")); // Verify that we can empty the list. @@ -165,6 +166,16 @@ public: CfgMgr::instance().deleteSubnets6(); } + /// @brief generates interface-id option based on provided text + /// + /// @param text content of the option to be created + /// + /// @return pointer to the option object created + OptionPtr generateInterfaceId(const string& text) { + OptionBuffer buffer(text.begin(), text.end()); + return OptionPtr(new Option(Option::V6, D6O_INTERFACE_ID, buffer)); + } + ~CfgMgrTest() { // clean up after the test CfgMgr::instance().deleteSubnets4(); @@ -406,6 +417,91 @@ TEST_F(CfgMgrTest, subnet6) { EXPECT_FALSE(cfg_mgr.getSubnet6(IOAddress("4000::123"))); } +// This test verifies if the configuration manager is able to hold, select +// and return valid subnets, based on interface names. +TEST_F(CfgMgrTest, subnet6Interface) { + CfgMgr& cfg_mgr = CfgMgr::instance(); + + Subnet6Ptr subnet1(new Subnet6(IOAddress("2000::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet2(new Subnet6(IOAddress("3000::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet3(new Subnet6(IOAddress("4000::"), 48, 1, 2, 3, 4)); + subnet1->setIface("foo"); + subnet2->setIface("bar"); + subnet3->setIface("foobar"); + + // There shouldn't be any subnet configured at this stage + EXPECT_FALSE(cfg_mgr.getSubnet6("foo")); + + cfg_mgr.addSubnet6(subnet1); + + // Now we have only one subnet, any request will be served from it + EXPECT_EQ(subnet1, cfg_mgr.getSubnet6("foo")); + + // If we have only a single subnet and the request came from a local + // address, let's use that subnet + EXPECT_EQ(subnet1, cfg_mgr.getSubnet6(IOAddress("fe80::dead:beef"))); + + cfg_mgr.addSubnet6(subnet2); + cfg_mgr.addSubnet6(subnet3); + + EXPECT_EQ(subnet3, cfg_mgr.getSubnet6("foobar")); + EXPECT_EQ(subnet2, cfg_mgr.getSubnet6("bar")); + EXPECT_FALSE(cfg_mgr.getSubnet6("xyzzy")); // no such interface + + // Check that deletion of the subnets works. + cfg_mgr.deleteSubnets6(); + EXPECT_FALSE(cfg_mgr.getSubnet6("foo")); + EXPECT_FALSE(cfg_mgr.getSubnet6("bar")); + EXPECT_FALSE(cfg_mgr.getSubnet6("foobar")); +} + +// This test verifies if the configuration manager is able to hold, select +// and return valid leases, based on interface-id option values +TEST_F(CfgMgrTest, subnet6InterfaceId) { + CfgMgr& cfg_mgr = CfgMgr::instance(); + + Subnet6Ptr subnet1(new Subnet6(IOAddress("2000::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet2(new Subnet6(IOAddress("3000::"), 48, 1, 2, 3, 4)); + Subnet6Ptr subnet3(new Subnet6(IOAddress("4000::"), 48, 1, 2, 3, 4)); + + // interface-id options used in subnets 1,2, and 3 + OptionPtr ifaceid1 = generateInterfaceId("relay1.eth0"); + OptionPtr ifaceid2 = generateInterfaceId("VL32"); + // That's a strange interface-id, but this is a real life example + OptionPtr ifaceid3 = generateInterfaceId("ISAM144|299|ipv6|nt:vp:1:110"); + + // bogus interface-id + OptionPtr ifaceid_bogus = generateInterfaceId("non-existent"); + + subnet1->setInterfaceId(ifaceid1); + subnet2->setInterfaceId(ifaceid2); + subnet3->setInterfaceId(ifaceid3); + + // There shouldn't be any subnet configured at this stage + EXPECT_FALSE(cfg_mgr.getSubnet6(ifaceid1)); + + cfg_mgr.addSubnet6(subnet1); + + // If we have only a single subnet and the request came from a local + // address, let's use that subnet + EXPECT_EQ(subnet1, cfg_mgr.getSubnet6(ifaceid1)); + EXPECT_FALSE(cfg_mgr.getSubnet6(ifaceid2)); + + cfg_mgr.addSubnet6(subnet2); + cfg_mgr.addSubnet6(subnet3); + + EXPECT_EQ(subnet3, cfg_mgr.getSubnet6(ifaceid3)); + EXPECT_EQ(subnet2, cfg_mgr.getSubnet6(ifaceid2)); + EXPECT_FALSE(cfg_mgr.getSubnet6(ifaceid_bogus)); + + // Check that deletion of the subnets works. + cfg_mgr.deleteSubnets6(); + EXPECT_FALSE(cfg_mgr.getSubnet6(ifaceid1)); + EXPECT_FALSE(cfg_mgr.getSubnet6(ifaceid2)); + EXPECT_FALSE(cfg_mgr.getSubnet6(ifaceid3)); +} + + // This test verifies that new DHCPv4 option spaces can be added to // the configuration manager and that duplicated option space is // rejected. diff --git a/src/lib/dhcpsrv/tests/subnet_unittest.cc b/src/lib/dhcpsrv/tests/subnet_unittest.cc index 1f0ef8b3cd..fbcebcb621 100644 --- a/src/lib/dhcpsrv/tests/subnet_unittest.cc +++ b/src/lib/dhcpsrv/tests/subnet_unittest.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -516,4 +517,19 @@ TEST(Subnet6Test, iface) { EXPECT_EQ("en1", subnet.getIface()); } +// This trivial test checks if the interface-id option can be set and +// later retrieved for a subnet6 object. +TEST(Subnet6Test, interfaceId) { + // Create as subnet to add options to it. + Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4)); + + EXPECT_FALSE(subnet->getInterfaceId()); + + OptionPtr option(new Option(Option::V6, D6O_INTERFACE_ID, OptionBuffer(10, 0xFF))); + subnet->setInterfaceId(option); + + EXPECT_EQ(option, subnet->getInterfaceId()); + +} + }; From df3ee4b9e04459e2ba6dbc50702b3c7f29c0ceb7 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 11 Apr 2013 14:52:21 +0200 Subject: [PATCH 2/8] [2898] Documentation for v6 relays written --- doc/devel/mainpage.dox | 1 + doc/guide/bind10-guide.xml | 81 ++++++++++++++++++++++++++++---------- src/lib/dhcp/libdhcp++.dox | 47 ++++++++++++++++++++++ 3 files changed, 108 insertions(+), 21 deletions(-) diff --git a/doc/devel/mainpage.dox b/doc/devel/mainpage.dox index 295fd03986..92f86eb765 100644 --- a/doc/devel/mainpage.dox +++ b/doc/devel/mainpage.dox @@ -30,6 +30,7 @@ * - @subpage dhcpv6ConfigInherit * - @subpage libdhcp * - @subpage libdhcpIntro + * - @subpage libdhcpRelay * - @subpage libdhcpIfaceMgr * - @subpage libdhcpsrv * - @subpage leasemgr diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index d0d1d4c3f6..e76839bfcc 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -4842,35 +4842,77 @@ should include options from the isc option space:
Subnet Selection - The DHCPv6 server may receive requests from local (connected - to the same subnet as the server) and remote (connecting via - relays) clients. - - - Currently relayed DHCPv6 traffic is not supported. The server will - only respond to local DHCPv6 requests - see - - - As it may have many subnet configurations defined, it - must select appropriate subnet for a given request. To do this, the server first + The DHCPv6 server may receive requests from local (connected to the + same subnet as the server) and remote (connecting via relays) clients. + As server may have many subnet configurations defined, it must select + appropriate subnet for a given request. To do this, the server first checks if there is only one subnet defined and source of the packet is - link-local. If this is the case, the server assumes that the only subnet - defined is local and client is indeed connected to it. This check - simplifies small deployments. + link-local. If this is the case, the server assumes that the only + subnet defined is local and client is indeed connected to it. This + check simplifies small deployments. If there are two or more subnets defined, the server can not assume which of those (if any) subnets are local. Therefore an optional - "interface" parameter is available within a subnet definition to designate that a given subnet - is local, i.e. reachable directly over specified interface. For example - the server that is intended to serve a local subnet over eth0 may be configured - as follows: + "interface" parameter is available within a subnet definition to + designate that a given subnet is local, i.e. reachable directly over + specified interface. For example the server that is intended to serve + a local subnet over eth0 may be configured as follows: > config add Dhcp6/subnet6 > config set Dhcp6/subnet6[1]/subnet "2001:db8:beef::/48" > config set Dhcp6/subnet6[1]/pool [ "2001:db8:beef::/48" ] > config set Dhcp6/subnet6[1]/interface "eth0" > config commit + + +
+ +
+ DHCPv6 Relays + + DHCPv6 server supports remote clients connected via relays. Server + that has many subnets defined and receives a request from client, must + select appropriate subnet for it. Remote clients there are two + mechanisms that can be used here. The first one is based on + interface-id options. While forwarding client's message, relays may + insert interface-id option that identifies the interface on which the + client message was received on. Some relays allow configuration of + that parameter, but it is sometimes hardcoded. This may range from + very simple (e.g. "vlan100") to very cryptic. One example used by real + hardware was "ISAM144|299|ipv6|nt:vp:1:110"). This may seem + meaningless, but that information is sufficient for its + purpose. Server may use it to select appropriate subnet and the relay + will know which interface to use for response transmission when it + gets that value back. This value must be unique in the whole + network. Server configuration must match whatever values are inserted + by the relays. + + + The second way in which server may pick the proper subnet is by using + linkaddr field in the RELAY_FORW message. Name of this field is somewhat + misleading. It does not contain link-layer address, but an address that + is used to identify a link. This typically is a global address. Kea + server checks if that address belongs to any defined subnet6. If it does, + that subnet is selected for client's request. + + + It should be noted that "interface" that defines which local network + interface can be used to access a given subnet and "interface-id" that + specify the content of the interface-id option used by relays are mutually + exclusive. A subnet cannot be both reachable locally (direct traffic) and + via relays (remote traffic). Specifying both is a configuration error and + Kea server will refuse such configuration. + + + To specify interface-id with value "vlan123", the following commands can + be used: + +> config add Dhcp6/subnet6 +> config set Dhcp6/subnet6[0]/subnet "2001:db8:beef::/48" +> config set Dhcp6/subnet6[0]/pool [ "2001:db8:beef::/48" ] +> config set Dhcp6/subnet6[0]/interface-id "vland123" +> config commit
@@ -4940,9 +4982,6 @@ Dhcp6/renew-timer 1000 integer (default) > config commit - - Relayed traffic is not supported. - Temporary addresses are not supported. diff --git a/src/lib/dhcp/libdhcp++.dox b/src/lib/dhcp/libdhcp++.dox index 194175aa10..eabc3926c7 100644 --- a/src/lib/dhcp/libdhcp++.dox +++ b/src/lib/dhcp/libdhcp++.dox @@ -57,6 +57,53 @@ DHCPv6, but is rarely used in DHCPv4. isc::dhcp::Option::addOption(), isc::dhcp::Option::delOption(), isc::dhcp::Option::getOption() can be used for that purpose. +@section libdhcpRelay Relay v6 support in Pkt6 + +DHCPv6 clients that are not connected to the same link as DHCPv6 +servers need relays to reach the server. Each relay receives a message +on a client facing interface, encapsulates it into RELAY_MSG option +and sends as RELAY_FORW message towards the server (or the next relay, +which is closer to the server). This procedure can be repeated up to +32 times. Kea is able to support up to 32 relays. Each traversed relay +may add certain options. The most obvious example is interface-id +option, but there may be other options as well. Each relay may add such +an option, regardless of whether other relays added it before. Thanks +to encapsulation, those options are separated and it is possible to +differentiate which relay inserted specific instance of an option. + +Interface-id is used to identify a subnet (or interface) the original message +came from and is used for that purpose on two occasions. First, the server +uses the interface-id included by the first relay (the one closest to +the client) to select appropriate subnet for a given request. Server includes +that interface-id in its copy, when sending data back to the client. +This will be used by the relay to choose proper interface when forwarding +response towards the client. + +Pkt6 class has a public Pkt6::relay_info_ field, which is of type Pkt6::RelayInfo. +This is a simple structure that represents the information in each RELAY_FORW +or RELAY_REPL message. It is important to understand the order in which +the data appear here. Consider the following network: + +\verbatim +client-------relay1-----relay2-----relay3----server +\endverbatim + +Client will transmit SOLICIT message. Relay1 will forward it as +RELAY_FORW with SOLICIT in it. Relay2 forward it as RELAY_FORW with +RELAY_FORW with SOLICIT in it. Finally the third relay will add yet +another RELAY_FORW around it. The server will parse the packet and +create Pkt6 object for it. Its relay_info_ will have 3 +elements. Packet parsing is done in reverse order, compare to the +order the packet traversed in the network. The first element +(relay_info_[0]) will represent relay3 information (the "last" relay or +in other words the one closest to the server). The second element +will represent relay2. The third element (relay_info_[2]) will represent +the first relay (relay1) or in other words the one closest to the client. + +Packets sent by the server must maintain the same encapsulation order. +This is easy to do - just copy data from client's message object into +server's response object. See Pkt6::coyRelayInfo for details. + @section libdhcpIfaceMgr Interface Manager Interface Manager (or IfaceMgr) is an abstraction layer about low-level From a19cdcff7accbce09eab6bf7ea359a830564c1f8 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Tue, 7 May 2013 19:46:57 +0200 Subject: [PATCH 3/8] [2898] First set of changes after review --- src/bin/dhcp6/config_parser.cc | 4 +- src/bin/dhcp6/dhcp6_srv.cc | 34 +++++++-------- src/bin/dhcp6/tests/config_parser_unittest.cc | 38 +++++++---------- src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 42 +++++++++++-------- src/lib/dhcpsrv/cfgmgr.cc | 2 +- 5 files changed, 59 insertions(+), 61 deletions(-) diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index 243d217605..f386243940 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -1490,7 +1490,7 @@ private: std::string ifaceid; try { ifaceid = string_values_.getParam("interface-id"); - } catch (DhcpConfigError) { + } catch (const DhcpConfigError&) { // interface-id is not mandatory } @@ -1503,7 +1503,7 @@ private: } stringstream tmp; - tmp << addr.toText() << "/" << (int)len + tmp << addr.toText() << "/" << static_cast(len) << " with params t1=" << t1 << ", t2=" << t2 << ", pref=" << pref << ", valid=" << valid; diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index ec74bb1810..0252c5199f 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -405,7 +405,7 @@ Dhcpv6Srv::copyDefaultOptions(const Pkt6Ptr& question, Pkt6Ptr& answer) { } /// @todo: Should throw if there is no client-id (except anonymous INF-REQUEST) - // if this is a relayed message, we need to copy relay information + // If this is a relayed message, we need to copy relay information if (!question->relay_info_.empty()) { answer->copyRelayInfo(question); } @@ -534,14 +534,11 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) { // This is a direct (non-relayed) message // Try to find a subnet if received packet from a directly connected client - Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(question->getIface()); - if (subnet) { - return (subnet); + subnet = CfgMgr::instance().getSubnet6(question->getIface()); + if (!subnet) { + // If no subnet was found, try to find it based on remote address + subnet = CfgMgr::instance().getSubnet6(question->getRemoteAddr()); } - - // If no subnet was found, try to find it based on remote address - subnet = CfgMgr::instance().getSubnet6(question->getRemoteAddr()); - return (subnet); } else { // This is a relayed message @@ -551,20 +548,19 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) { subnet = CfgMgr::instance().getSubnet6(interface_id); } - if (subnet) { - return (subnet); - } + if (!subnet) { + // If no interface-id was specified (or not configured on server), let's + // try address matching + IOAddress link_addr = question->relay_info_.back().linkaddr_; - // If no interface-id was specified (or not configured on server), let's - // try address matching - IOAddress link_addr = question->relay_info_.back().linkaddr_; - - // if relay filled in link_addr field, then let's use it - if (link_addr != IOAddress("::")) { - subnet = CfgMgr::instance().getSubnet6(link_addr); + // if relay filled in link_addr field, then let's use it + if (link_addr != IOAddress("::")) { + subnet = CfgMgr::instance().getSubnet6(link_addr); + } } - return (subnet); } + + return (subnet); } void diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 37dd783cc9..be3e3be3ca 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -277,13 +277,13 @@ public: expected_data_len)); } - int rcode_; - Dhcpv6Srv srv_; + int rcode_; ///< return core (see @ref isc::config::parseAnswer) + Dhcpv6Srv srv_; ///< instance of the Dhcp6Srv used during tests - ConstElementPtr comment_; + ConstElementPtr comment_; ///< comment (see @ref isc::config::parseAnswer) - string valid_iface_; - string bogus_iface_; + string valid_iface_; ///< name of a valid network interface (present in system) + string bogus_iface_; ///< name of a invalid network interface (not present in system) }; // Goal of this test is a verification if a very simple config update @@ -508,11 +508,9 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) { const string valid_interface_id = "foobar"; const string bogus_interface_id = "blah"; - ConstElementPtr status; - // There should be at least one interface - string config = "{ " + const string config = "{ " "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -521,24 +519,24 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) { " \"interface-id\": \"" + valid_interface_id + "\"," " \"subnet\": \"2001:db8:1::/64\" } ]," "\"valid-lifetime\": 4000 }"; - cout << config << endl; ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - // returned value should be 0 (configuration success) + // Returned value should be 0 (configuration success) ASSERT_TRUE(status); comment_ = parseAnswer(rcode_, status); EXPECT_EQ(0, rcode_); - // try to get a subnet based on bogus interface-id option + // Try to get a subnet based on bogus interface-id option OptionBuffer tmp(bogus_interface_id.begin(), bogus_interface_id.end()); OptionPtr ifaceid(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(ifaceid); EXPECT_FALSE(subnet); - // now try to get subnet for valid interface-id value + // Now try to get subnet for valid interface-id value tmp = OptionBuffer(valid_interface_id.begin(), valid_interface_id.end()); ifaceid.reset(new Option(Option::V6, D6O_INTERFACE_ID, tmp)); subnet = CfgMgr::instance().getSubnet6(ifaceid); @@ -551,9 +549,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) { // parameter. TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { - ConstElementPtr status; - - string config = "{ \"interface\": [ \"all\" ]," + const string config = "{ \"interface\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -562,13 +558,13 @@ TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { " \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ]," " \"subnet\": \"2001:db8:1::/64\" } ]," "\"valid-lifetime\": 4000 }"; - cout << config << endl; ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - // returned value should be 1 (parse error) + // Returned value should be 1 (parse error) ASSERT_TRUE(status); comment_ = parseAnswer(rcode_, status); EXPECT_EQ(1, rcode_); @@ -578,9 +574,7 @@ TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { // interface (i.e. local subnet) and interface-id (remote subnet) defined. TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) { - ConstElementPtr status; - - string config = "{ \"preferred-lifetime\": 3000," + const string config = "{ \"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet6\": [ { " @@ -589,13 +583,13 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) { " \"interface-id\": \"foobar\"," " \"subnet\": \"2001:db8:1::/64\" } ]," "\"valid-lifetime\": 4000 }"; - cout << config << endl; ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - // returned value should be 0 (configuration success) + // Returned value should be 0 (configuration success) ASSERT_TRUE(status); comment_ = parseAnswer(rcode_, status); EXPECT_EQ(1, rcode_); diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index d63d518b39..53b1daf47d 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -80,7 +80,7 @@ public: static const char* DUID_FILE = "server-id-test.txt"; // test fixture for any tests requiring blank/empty configuration -// serves as base class for additional tests +// serves as base class for additional tests class NakedDhcpv6SrvTest : public ::testing::Test { public: @@ -146,12 +146,12 @@ public: // Checks if server response is a NAK void checkNakResponse(const Pkt6Ptr& rsp, uint8_t expected_message_type, - uint32_t expected_transid, + uint32_t expected_transid, uint16_t expected_status_code) { // Check if we get response at all checkResponse(rsp, expected_message_type, expected_transid); - // Check that IA_NA was returned + // Check that IA_NA was returned OptionPtr option_ia_na = rsp->getOption(D6O_IA_NA); ASSERT_TRUE(option_ia_na); @@ -237,7 +237,7 @@ public: ConstElementPtr comment_; }; -// Provides suport for tests against a preconfigured subnet6 +// Provides suport for tests against a preconfigured subnet6 // extends upon NakedDhcp6SrvTest class Dhcpv6SrvTest : public NakedDhcpv6SrvTest { public: @@ -264,7 +264,7 @@ public: ADD_FAILURE() << "IA_NA option not present in response"; return (boost::shared_ptr()); } - + boost::shared_ptr ia = boost::dynamic_pointer_cast(tmp); if (!ia) { ADD_FAILURE() << "IA_NA cannot convert option ptr to Option6"; @@ -274,7 +274,7 @@ public: EXPECT_EQ(expected_iaid, ia->getIAID()); EXPECT_EQ(expected_t1, ia->getT1()); EXPECT_EQ(expected_t2, ia->getT2()); - + tmp = ia->getOption(D6O_IAADDR); boost::shared_ptr addr = boost::dynamic_pointer_cast(tmp); return (addr); @@ -330,10 +330,10 @@ public: }; // This test verifies that incoming SOLICIT can be handled properly when -// there are no subnets configured. +// there are no subnets configured. // -// This test sends a SOLICIT and the expected response -// is an ADVERTISE with STATUS_NoAddrsAvail and no address provided in the +// This test sends a SOLICIT and the expected response +// is an ADVERTISE with STATUS_NoAddrsAvail and no address provided in the // response TEST_F(NakedDhcpv6SrvTest, SolicitNoSubnet) { NakedDhcpv6Srv srv(0); @@ -352,10 +352,10 @@ TEST_F(NakedDhcpv6SrvTest, SolicitNoSubnet) { } // This test verifies that incoming REQUEST can be handled properly when -// there are no subnets configured. +// there are no subnets configured. // -// This test sends a REQUEST and the expected response -// is an REPLY with STATUS_NoAddrsAvail and no address provided in the +// This test sends a REQUEST and the expected response +// is an REPLY with STATUS_NoAddrsAvail and no address provided in the // response TEST_F(NakedDhcpv6SrvTest, RequestNoSubnet) { NakedDhcpv6Srv srv(0); @@ -386,8 +386,8 @@ TEST_F(NakedDhcpv6SrvTest, RequestNoSubnet) { // This test verifies that incoming RENEW can be handled properly, even when // no subnets are configured. // -// This test sends a RENEW and the expected response -// is an REPLY with STATUS_NoBinding and no address provided in the +// This test sends a RENEW and the expected response +// is an REPLY with STATUS_NoBinding and no address provided in the // response TEST_F(NakedDhcpv6SrvTest, RenewNoSubnet) { NakedDhcpv6Srv srv(0); @@ -421,8 +421,8 @@ TEST_F(NakedDhcpv6SrvTest, RenewNoSubnet) { // This test verifies that incoming RELEASE can be handled properly, even when // no subnets are configured. // -// This test sends a RELEASE and the expected response -// is an REPLY with STATUS_NoBinding and no address provided in the +// This test sends a RELEASE and the expected response +// is an REPLY with STATUS_NoBinding and no address provided in the // response TEST_F(NakedDhcpv6SrvTest, ReleaseNoSubnet) { NakedDhcpv6Srv srv(0); @@ -951,6 +951,8 @@ TEST_F(Dhcpv6SrvTest, RequestBasic) { TEST_F(Dhcpv6SrvTest, ManyRequests) { NakedDhcpv6Srv srv(0); + ASSERT_TRUE(subnet_); + Pkt6Ptr req1 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)); Pkt6Ptr req2 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 2345)); Pkt6Ptr req3 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 3456)); @@ -995,6 +997,10 @@ TEST_F(Dhcpv6SrvTest, ManyRequests) { boost::shared_ptr addr3 = checkIA_NA(reply3, 3, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr1); + ASSERT_TRUE(addr2); + ASSERT_TRUE(addr3); + // Check that the assigned address is indeed from the configured pool checkIAAddr(addr1, addr1->getAddress(), subnet_->getPreferred(), subnet_->getValid()); checkIAAddr(addr2, addr2->getAddress(), subnet_->getPreferred(), subnet_->getValid()); @@ -1083,6 +1089,8 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) { boost::shared_ptr addr_opt = checkIA_NA(reply, 234, subnet_->getT1(), subnet_->getT2()); + ASSERT_TRUE(addr_opt); + // Check that we've got the address we requested checkIAAddr(addr_opt, addr, subnet_->getPreferred(), subnet_->getValid()); @@ -1649,7 +1657,7 @@ TEST_F(Dhcpv6SrvTest, selectSubnetRelayLinkaddr) { CfgMgr::instance().addSubnet6(subnet2); CfgMgr::instance().addSubnet6(subnet3); - // source of the packet should have no meaning. Selection is based + // Source of the packet should have no meaning. Selection is based // on linkaddr field in the relay pkt->setRemoteAddr(IOAddress("2001:db8:1::baca")); selected = srv.selectSubnet(pkt); diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 8c0742c136..745322ecd2 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -147,7 +147,7 @@ CfgMgr::getSubnet6(const isc::asiolink::IOAddress& hint) { // If there's only one subnet configured, let's just use it // The idea is to keep small deployments easy. In a small network - one - // router that also runs DHCPv6 server. Users specifies a single pool and + // router that also runs DHCPv6 server. User specifies a single pool and // expects it to just work. Without this, the server would complain that it // doesn't have IP address on its interfaces that matches that // configuration. Such requirement makes sense in IPv4, but not in IPv6. From 468687262b865e264f2bdc235ec4aff45fffaf29 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Wed, 8 May 2013 11:21:59 +0200 Subject: [PATCH 4/8] [2898] Second part of changes after review --- doc/guide/bind10-guide.xml | 69 ++++++++++++++---------- src/lib/dhcp/pkt6.cc | 67 ++++++++++++----------- src/lib/dhcp/pkt6.h | 20 +++---- src/lib/dhcp/tests/pkt6_unittest.cc | 28 ++++++++-- src/lib/dhcpsrv/cfgmgr.cc | 3 +- src/lib/dhcpsrv/dhcpsrv_messages.mes | 4 +- src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 6 ++- 7 files changed, 117 insertions(+), 80 deletions(-) diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index e76839bfcc..5e08648cea 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -4871,43 +4871,54 @@ should include options from the isc option space:
DHCPv6 Relays - DHCPv6 server supports remote clients connected via relays. Server - that has many subnets defined and receives a request from client, must - select appropriate subnet for it. Remote clients there are two - mechanisms that can be used here. The first one is based on - interface-id options. While forwarding client's message, relays may - insert interface-id option that identifies the interface on which the - client message was received on. Some relays allow configuration of - that parameter, but it is sometimes hardcoded. This may range from - very simple (e.g. "vlan100") to very cryptic. One example used by real - hardware was "ISAM144|299|ipv6|nt:vp:1:110"). This may seem - meaningless, but that information is sufficient for its - purpose. Server may use it to select appropriate subnet and the relay - will know which interface to use for response transmission when it - gets that value back. This value must be unique in the whole - network. Server configuration must match whatever values are inserted - by the relays. + A DHCPv6 server with multiple subnets defined must select the + appropriate subnet when it receives a request from client. For clients + connected via relays, two mechanisms are used. - The second way in which server may pick the proper subnet is by using - linkaddr field in the RELAY_FORW message. Name of this field is somewhat - misleading. It does not contain link-layer address, but an address that - is used to identify a link. This typically is a global address. Kea - server checks if that address belongs to any defined subnet6. If it does, - that subnet is selected for client's request. + The first uses the linkaddr field in the RELAY_FORW message. The name + of this field is somewhat misleading in that it does not contain link-layer + address: instead, it holds an address (typically a global address) that is + used to identify a link. The DHCPv6 server checks if the address belongs + to a defined subnet and, if it does, that subnet is selected for the client's + request. - It should be noted that "interface" that defines which local network - interface can be used to access a given subnet and "interface-id" that - specify the content of the interface-id option used by relays are mutually - exclusive. A subnet cannot be both reachable locally (direct traffic) and - via relays (remote traffic). Specifying both is a configuration error and - Kea server will refuse such configuration. + The second mechanism is based on interface-id options. While forwarding client's + message, relays may insert an interface-id option into the message that + identifies the interface on the relay that received client message. (Some + relays allow configuration of that parameter, but it is sometimes + hardcoded and may range from very simple (e.g. "vlan100") to very cryptic: + one example seen on real hardware was "ISAM144|299|ipv6|nt:vp:1:110"). The + server can use this information to select the appropriate subnet. + The information is also returned to the relay which then knows which + interface to use to transmit the response to the client. In order to + use this successfully, the relay interface IDs must be unique within + the network, and the server configuration must match those values. + + When configuring the DHCPv6 server, it should be noted that two + similarly-named parameters can be configured for a subnet: + + + "interface" defines which local network interface can be used + to access a given subnet. + + + "interface-id" specifies the content of the interface-id option + used by relays to identify the interface on the relay to which + the response packet is sent. + + + The two are mutually exclusive: a subnet cannot be both reachable locally + (direct traffic) and via relays (remote traffic). Specifying both is a + configuration error and the DHCPv6 server will refuse such a configuration. + + To specify interface-id with value "vlan123", the following commands can be used: - + > config add Dhcp6/subnet6 > config set Dhcp6/subnet6[0]/subnet "2001:db8:beef::/48" > config set Dhcp6/subnet6[0]/pool [ "2001:db8:beef::/48" ] diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index 921b8e6ea0..27c8ca67ca 100644 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -73,56 +73,58 @@ uint16_t Pkt6::len() { } OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) { - int start = 0; - int end = 0; - int direction = 0; if (relay_info_.empty()) { - // there's no relay info, this is a direct message + // There's no relay info, this is a direct message return (OptionPtr()); } + int start = 0; // First relay to check + int end = 0; // Last relay to check + int direction = 0; // How we going to iterate: forward or backward? + switch (order) { case RELAY_SEARCH_FROM_CLIENT: - // search backwards + // Search backwards start = relay_info_.size() - 1; end = 0; direction = -1; break; case RELAY_SEARCH_FROM_SERVER: - // search forward + // Search forward start = 0; end = relay_info_.size() - 1; direction = 1; break; - case RELAY_SEARCH_FIRST: - // look at the innermost relay only + case RELAY_GET_FIRST: + // Look at the innermost relay only start = relay_info_.size() - 1; end = start; - direction = 0; + direction = 1; break; - case RELAY_SEARCH_LAST: - // look at the outermost relay only + case RELAY_GET_LAST: + // Look at the outermost relay only start = 0; end = 0; - direction = 0; + direction = 1; } - // this is a tricky loop. It must go from start to end, but it must work in + // This is a tricky loop. It must go from start to end, but it must work in // both directions (start > end; or start < end). We can't use regular - // exit condition, because we don't know whether to use i <= end or i >= end - for (int i = start; ; i += direction) { + // exit condition, because we don't know whether to use i <= end or i >= end. + // That's why we check if in the next iteration we would go past the + // list (end + direction). It is similar to STL concept of end pointing + // to a place after the last element + for (int i = start; i != end + direction; i += direction) { OptionPtr opt = getRelayOption(opt_type, i); if (opt) { return (opt); } - - if (i == end) { - break; - } } - return getRelayOption(opt_type, end); + // We iterated over specified relays and haven't found what we were + // looking for + return (OptionPtr()); } @@ -539,27 +541,28 @@ const char* Pkt6::getName() const { void Pkt6::copyRelayInfo(const Pkt6Ptr& question) { - // we use index rather than iterator, because we need that as a parameter + // We use index rather than iterator, because we need that as a parameter // passed to getRelayOption() for (int i = 0; i < question->relay_info_.size(); ++i) { - RelayInfo x; - x.msg_type_ = DHCPV6_RELAY_REPL; - x.hop_count_ = question->relay_info_[i].hop_count_; - x.linkaddr_ = question->relay_info_[i].linkaddr_; - x.peeraddr_ = question->relay_info_[i].peeraddr_; + RelayInfo info; + info.msg_type_ = DHCPV6_RELAY_REPL; + info.hop_count_ = question->relay_info_[i].hop_count_; + info.linkaddr_ = question->relay_info_[i].linkaddr_; + info.peeraddr_ = question->relay_info_[i].peeraddr_; - // Is there interface-id option in this nesting level if there is, - // we need to echo it back + // Is there an interface-id option in this nesting level? + // If there is, we need to echo it back OptionPtr opt = question->getRelayOption(D6O_INTERFACE_ID, i); // taken from question->RelayInfo_[i].options_ if (opt) { - x.options_.insert(pair >(opt->getType(), opt)); + info.options_.insert(make_pair(opt->getType(), opt)); } - /// @todo: implement support for ERO (Echo Request Option, RFC4994) + /// @todo: Implement support for ERO (Echo Request Option, RFC4994) - // add this relay-repl info to our message - relay_info_.push_back(x); + // Add this relay-forw info (client's message) to our relay-repl + // message (server's response) + relay_info_.push_back(info); } } diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index 09050c5666..c65142ef52 100644 --- a/src/lib/dhcp/pkt6.h +++ b/src/lib/dhcp/pkt6.h @@ -60,14 +60,14 @@ public: /// relay closest to the server (i.e. outermost in the encapsulated /// message, which also means it was the last relay that relayed /// the received message and will be the first one to process - /// server's response). RELAY_SEARCH_FIRST is option from the first - /// relay (closest to the client), RELAY_SEARCH_LAST is the - /// last relay (closest to the server). + /// server's response). RELAY_GET_FIRST will try to get option from + /// the first relay only (closest to the client), RELAY_GET_LAST will + /// try to get option form the the last relay (closest to the server). enum RelaySearchOrder { RELAY_SEARCH_FROM_CLIENT = 1, RELAY_SEARCH_FROM_SERVER = 2, - RELAY_SEARCH_FIRST = 3, - RELAY_SEARCH_LAST = 4 + RELAY_GET_FIRST = 3, + RELAY_GET_LAST = 4 }; /// @brief structure that describes a single relay information @@ -226,12 +226,12 @@ public: /// @return pointer to the option (or NULL if there is no such option) OptionPtr getRelayOption(uint16_t option_code, uint8_t nesting_level); - /// @brief returns first instance of a specified option + /// @brief Return first instance of a specified option /// - /// When client's packet traverses multiple relays, each passing relay - /// may insert extra options. This method allows getting specific instance - /// of a given option (closest to the client, closest to the server, etc.) - /// See @ref RelaySearchOrder for detailed description. + /// When a client's packet traverses multiple relays, each passing relay may + /// insert extra options. This method allows the specific instance of a given + /// option to be obtained (e.g. closest to the client, closest to the server, + /// etc.) See @ref RelaySearchOrder for a detailed description. /// /// @param option_code searched option /// @param order option search order (see @ref RelaySearchOrder) diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index 5dee36c3d5..a7a366dc10 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -609,8 +609,14 @@ TEST_F(Pkt6Test, getAnyRelayOption) { // First check that the getAnyRelayOption does not confuse client options // and relay options - OptionPtr opt = msg->getAnyRelayOption(300, Pkt6::RELAY_SEARCH_FROM_CLIENT); // 300 is a client option, present in the message itself. + OptionPtr opt = msg->getAnyRelayOption(300, Pkt6::RELAY_SEARCH_FROM_CLIENT); + EXPECT_FALSE(opt); + opt = msg->getAnyRelayOption(300, Pkt6::RELAY_SEARCH_FROM_SERVER); + EXPECT_FALSE(opt); + opt = msg->getAnyRelayOption(300, Pkt6::RELAY_GET_FIRST); + EXPECT_FALSE(opt); + opt = msg->getAnyRelayOption(300, Pkt6::RELAY_GET_LAST); EXPECT_FALSE(opt); // Option 200 is added in every relay. @@ -628,12 +634,12 @@ TEST_F(Pkt6Test, getAnyRelayOption) { EXPECT_TRUE(opt->equal(relay1_opt1)); // We just want option from the first relay (closest to the client) - opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FIRST); + opt = msg->getAnyRelayOption(200, Pkt6::RELAY_GET_FIRST); ASSERT_TRUE(opt); EXPECT_TRUE(opt->equal(relay3_opt1)); // We just want option from the last relay (closest to the server) - opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_LAST); + opt = msg->getAnyRelayOption(200, Pkt6::RELAY_GET_LAST); ASSERT_TRUE(opt); EXPECT_TRUE(opt->equal(relay1_opt1)); @@ -647,12 +653,24 @@ TEST_F(Pkt6Test, getAnyRelayOption) { ASSERT_TRUE(opt); EXPECT_TRUE(opt->equal(relay2_opt1)); - opt = msg->getAnyRelayOption(100, Pkt6::RELAY_SEARCH_FIRST); + opt = msg->getAnyRelayOption(100, Pkt6::RELAY_GET_FIRST); EXPECT_FALSE(opt); - opt = msg->getAnyRelayOption(100, Pkt6::RELAY_SEARCH_LAST); + opt = msg->getAnyRelayOption(100, Pkt6::RELAY_GET_LAST); EXPECT_FALSE(opt); + // Finally, try to get an option that does not exist + opt = msg->getAnyRelayOption(500, Pkt6::RELAY_GET_FIRST); + EXPECT_FALSE(opt); + + opt = msg->getAnyRelayOption(500, Pkt6::RELAY_GET_LAST); + EXPECT_FALSE(opt); + + opt = msg->getAnyRelayOption(500, Pkt6::RELAY_SEARCH_FROM_SERVER); + EXPECT_FALSE(opt); + + opt = msg->getAnyRelayOption(500, Pkt6::RELAY_SEARCH_FROM_CLIENT); + EXPECT_FALSE(opt); } } diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 745322ecd2..f1181ddcb9 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -183,7 +183,8 @@ Subnet6Ptr CfgMgr::getSubnet6(OptionPtr iface_id_option) { return (Subnet6Ptr()); } - // If there is more than one, we need to choose the proper one + // Let's iterate over all subnets and for those that have interface-id + // defined, check if the interface-id is equal to what we are looking for for (Subnet6Collection::iterator subnet = subnets6_.begin(); subnet != subnets6_.end(); ++subnet) { if ( (*subnet)->getInterfaceId() && diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index ff8c36e13c..b2a780706a 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -105,14 +105,14 @@ This is a debug message reporting that the DHCP configuration manager has returned the specified IPv6 subnet for a packet received over given interface. This particular subnet was selected, because it was specified as being directly reachable over given interface. (see -'interface' parameter in subnet6 definition). +'interface' parameter in the subnet6 definition). % DHCPSRV_CFGMGR_SUBNET6_IFACE_ID selected subnet %1 (interface-id match) for incoming packet This is a debug message reporting that the DHCP configuration manager has returned the specified IPv6 subnet for a received packet. This particular subnet was selected, because value of interface-id option matched what was configured in server's interface-id option for that selected subnet6. -(see 'interface-id' parameter in subnet6 definition). +(see 'interface-id' parameter in the subnet6 definition). % DHCPSRV_CLOSE_DB closing currently open %1 database This is a debug message, issued when the DHCP server closes the currently diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 2ffe5da68b..ba2f290270 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -49,7 +49,7 @@ TEST(ValueStorageTest, BooleanTesting) { EXPECT_FALSE(testStore.getParam("firstBool")); EXPECT_TRUE(testStore.getParam("secondBool")); - // Verify that we can update paramaters. + // Verify that we can update parameters. testStore.setParam("firstBool", true); testStore.setParam("secondBool", false); @@ -437,6 +437,10 @@ TEST_F(CfgMgrTest, subnet6Interface) { // Now we have only one subnet, any request will be served from it EXPECT_EQ(subnet1, cfg_mgr.getSubnet6("foo")); + // Check that the interface name is checked even when there is + // only one subnet defined. + EXPECT_FALSE(cfg_mgr.getSubnet6("bar")); + // If we have only a single subnet and the request came from a local // address, let's use that subnet EXPECT_EQ(subnet1, cfg_mgr.getSubnet6(IOAddress("fe80::dead:beef"))); From bc6af598cfc62d68cc4a80c0aa10d8573d63cfc0 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Wed, 8 May 2013 11:22:40 +0200 Subject: [PATCH 5/8] [2898] pair replaced with make_pair --- src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 2 +- src/lib/dhcp/tests/libdhcp++_unittest.cc | 10 +++++----- src/lib/dhcp/tests/pkt6_unittest.cc | 18 +++++++++--------- src/lib/dhcpsrv/cfgmgr.cc | 6 ++---- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 53b1daf47d..8184adfa1e 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -1699,7 +1699,7 @@ TEST_F(Dhcpv6SrvTest, selectSubnetRelayInterfaceId) { relay.linkaddr_ = IOAddress("2001:db8:2::1234"); relay.peeraddr_ = IOAddress("fe80::1"); OptionPtr opt = generateInterfaceId("relay2"); - relay.options_.insert(pair(opt->getType(), opt)); + relay.options_.insert(make_pair(opt->getType(), opt)); pkt->relay_info_.push_back(relay); // There is only one subnet configured and we are outside of that subnet diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc index dbab49290d..1567829edb 100644 --- a/src/lib/dhcp/tests/libdhcp++_unittest.cc +++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc @@ -265,11 +265,11 @@ TEST_F(LibDhcpTest, packOptions6) { OptionPtr opt4(new Option(Option::V6, 6, buf.begin() + 8, buf.begin() + 12)); OptionPtr opt5(new Option(Option::V6, 8, buf.begin() + 12, buf.begin() + 14)); - opts.insert(pair(opt1->getType(), opt1)); - opts.insert(pair(opt1->getType(), opt2)); - opts.insert(pair(opt1->getType(), opt3)); - opts.insert(pair(opt1->getType(), opt4)); - opts.insert(pair(opt1->getType(), opt5)); + opts.insert(make_pair(opt1->getType(), opt1)); + opts.insert(make_pair(opt1->getType(), opt2)); + opts.insert(make_pair(opt1->getType(), opt3)); + opts.insert(make_pair(opt1->getType(), opt4)); + opts.insert(make_pair(opt1->getType(), opt5)); OutputBuffer assembled(512); diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index a7a366dc10..b713d2bb98 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -507,7 +507,7 @@ TEST_F(Pkt6Test, relayPack) { OptionPtr optRelay1(new Option(Option::V6, 200, relay_data)); - relay1.options_.insert(pair >(optRelay1->getType(), optRelay1)); + relay1.options_.insert(make_pair(optRelay1->getType(), optRelay1)); OptionPtr opt1(new Option(Option::V6, 100)); OptionPtr opt2(new Option(Option::V6, 101)); @@ -581,9 +581,9 @@ TEST_F(Pkt6Test, getAnyRelayOption) { OptionPtr relay1_opt2(generateRandomOption(201)); OptionPtr relay1_opt3(generateRandomOption(202)); - relay1.options_.insert(pair >(200, relay1_opt1)); - relay1.options_.insert(pair >(201, relay1_opt2)); - relay1.options_.insert(pair >(202, relay1_opt3)); + relay1.options_.insert(make_pair(200, relay1_opt1)); + relay1.options_.insert(make_pair(201, relay1_opt2)); + relay1.options_.insert(make_pair(202, relay1_opt3)); msg->addRelayInfo(relay1); // generate options for relay2 @@ -592,16 +592,16 @@ TEST_F(Pkt6Test, getAnyRelayOption) { OptionPtr relay2_opt2(new Option(Option::V6, 101)); OptionPtr relay2_opt3(new Option(Option::V6, 102)); OptionPtr relay2_opt4(new Option(Option::V6, 200)); // the same code as relay1_opt3 - relay2.options_.insert(pair >(100, relay2_opt1)); - relay2.options_.insert(pair >(101, relay2_opt2)); - relay2.options_.insert(pair >(102, relay2_opt3)); - relay2.options_.insert(pair >(200, relay2_opt4)); + relay2.options_.insert(make_pair(100, relay2_opt1)); + relay2.options_.insert(make_pair(101, relay2_opt2)); + relay2.options_.insert(make_pair(102, relay2_opt3)); + relay2.options_.insert(make_pair(200, relay2_opt4)); msg->addRelayInfo(relay2); // generate options for relay3 Pkt6::RelayInfo relay3; OptionPtr relay3_opt1(generateRandomOption(200, 7)); - relay3.options_.insert(pair >(200, relay3_opt1)); + relay3.options_.insert(make_pair(200, relay3_opt1)); msg->addRelayInfo(relay3); // Ok, so we now have a packet that traversed the following network: diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index f1181ddcb9..592efb7942 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -40,8 +40,7 @@ CfgMgr::addOptionSpace4(const OptionSpacePtr& space) { isc_throw(InvalidOptionSpace, "option space " << space->getName() << " already added."); } - spaces4_.insert(std::pair(space->getName(), space)); + spaces4_.insert(make_pair(space->getName(), space)); } void @@ -55,8 +54,7 @@ CfgMgr::addOptionSpace6(const OptionSpacePtr& space) { isc_throw(InvalidOptionSpace, "option space " << space->getName() << " already added."); } - spaces6_.insert(std::pair(space->getName(), space)); + spaces6_.insert(make_pair(space->getName(), space)); } void From 131a89175e97714ac87f14f95703cd76df7a965c Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Wed, 8 May 2013 11:30:50 +0200 Subject: [PATCH 6/8] [2898] Compilation fix after recent changes. --- src/bin/dhcp6/dhcp6_srv.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 0252c5199f..86f4d8f48a 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -543,7 +543,7 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) { // This is a relayed message OptionPtr interface_id = question->getAnyRelayOption(D6O_INTERFACE_ID, - Pkt6::RELAY_SEARCH_FIRST); + Pkt6::RELAY_GET_FIRST); if (interface_id) { subnet = CfgMgr::instance().getSubnet6(interface_id); } From be918f9a8b657005605fc8856ed037fafc2374e9 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Wed, 8 May 2013 18:33:25 +0100 Subject: [PATCH 7/8] [2898] Minor editing changes to DHCPv6 relay text --- doc/guide/bind10-guide.xml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index 5e08648cea..568bbea0c5 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -4873,28 +4873,28 @@ should include options from the isc option space: A DHCPv6 server with multiple subnets defined must select the appropriate subnet when it receives a request from client. For clients - connected via relays, two mechanisms are used. + connected via relays, two mechanisms are used: The first uses the linkaddr field in the RELAY_FORW message. The name - of this field is somewhat misleading in that it does not contain link-layer + of this field is somewhat misleading in that it does not contain a link-layer address: instead, it holds an address (typically a global address) that is used to identify a link. The DHCPv6 server checks if the address belongs to a defined subnet and, if it does, that subnet is selected for the client's request. - The second mechanism is based on interface-id options. While forwarding client's + The second mechanism is based on interface-id options. While forwarding a client's message, relays may insert an interface-id option into the message that - identifies the interface on the relay that received client message. (Some + identifies the interface on the relay that received the message. (Some relays allow configuration of that parameter, but it is sometimes - hardcoded and may range from very simple (e.g. "vlan100") to very cryptic: + hardcoded and may range from the very simple (e.g. "vlan100") to the very cryptic: one example seen on real hardware was "ISAM144|299|ipv6|nt:vp:1:110"). The server can use this information to select the appropriate subnet. - The information is also returned to the relay which then knows which - interface to use to transmit the response to the client. In order to - use this successfully, the relay interface IDs must be unique within - the network, and the server configuration must match those values. + The information is also returned to the relay which then knows the + interface to use to transmit the response to the client. In order for + this to work successfully, the relay interface IDs must be unique within + the network and the server configuration must match those values. When configuring the DHCPv6 server, it should be noted that two From d5a472119f28ad0f7f15756997c741ba25208ab9 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 9 May 2013 15:43:02 +0200 Subject: [PATCH 8/8] [2898] comment updated in one dhcp6/config_parser test --- src/bin/dhcp6/tests/config_parser_unittest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index be3e3be3ca..d9c5859a64 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -589,7 +589,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - // Returned value should be 0 (configuration success) + // Returned value should be 1 (configuration error) ASSERT_TRUE(status); comment_ = parseAnswer(rcode_, status); EXPECT_EQ(1, rcode_);