From b1899f97cd7f7f93629c03f27c1632161059e497 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 20 May 2016 19:00:00 +0200 Subject: [PATCH 1/5] [3572] DHCPv4 server sends host specific options. --- src/bin/dhcp4/dhcp4_srv.cc | 10 +- src/bin/dhcp4/tests/Makefile.am | 1 + src/bin/dhcp4/tests/dhcp4_client.cc | 4 +- src/bin/dhcp4/tests/host_options_unittest.cc | 232 +++++++++++++++++++ src/bin/dhcp4/tests/inform_unittest.cc | 26 +-- src/lib/dhcpsrv/host.cc | 2 +- 6 files changed, 258 insertions(+), 17 deletions(-) create mode 100644 src/bin/dhcp4/tests/host_options_unittest.cc diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 2ea7af9c34..47afca878a 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -916,13 +916,21 @@ void Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) { CfgOptionList& co_list = ex.getCfgOptionList(); - // First subnet configured options + // Retrieve subnet. Subnet4Ptr subnet = ex.getContext()->subnet_; if (!subnet) { // All methods using the CfgOptionList object return soon when // there is no subnet so do the same return; } + + // Firstly, host specific options. + const ConstHostPtr& host = ex.getContext()->host_; + if (host) { + co_list.push_back(host->getCfgOption4()); + } + + // Secondly, subnet configured options. if (!subnet->getCfgOption()->empty()) { co_list.push_back(subnet->getCfgOption()); } diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index a0856b21fd..a11d771b7e 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -89,6 +89,7 @@ dhcp4_unittests_SOURCES += dhcp4_client.cc dhcp4_client.h dhcp4_unittests_SOURCES += hooks_unittest.cc dhcp4_unittests_SOURCES += inform_unittest.cc dhcp4_unittests_SOURCES += dora_unittest.cc +dhcp4_unittests_SOURCES += host_options_unittest.cc dhcp4_unittests_SOURCES += release_unittest.cc dhcp4_unittests_SOURCES += out_of_range_unittest.cc dhcp4_unittests_SOURCES += decline_unittest.cc diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc index f4926de0df..c801b57edf 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.cc +++ b/src/bin/dhcp4/tests/dhcp4_client.cc @@ -163,13 +163,13 @@ Dhcp4Client::applyConfiguration() { Option4AddrLstPtr opt_log_servers = boost::dynamic_pointer_cast< Option4AddrLst>(resp->getOption(DHO_LOG_SERVERS)); if (opt_log_servers) { - config_.log_servers_ = opt_routers->getAddresses(); + config_.log_servers_ = opt_log_servers->getAddresses(); } // Quotes Servers Option4AddrLstPtr opt_quotes_servers = boost::dynamic_pointer_cast< Option4AddrLst>(resp->getOption(DHO_COOKIE_SERVERS)); if (opt_quotes_servers) { - config_.quotes_servers_ = opt_dns_servers->getAddresses(); + config_.quotes_servers_ = opt_quotes_servers->getAddresses(); } // Server Identifier OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast< diff --git a/src/bin/dhcp4/tests/host_options_unittest.cc b/src/bin/dhcp4/tests/host_options_unittest.cc new file mode 100644 index 0000000000..090219db95 --- /dev/null +++ b/src/bin/dhcp4/tests/host_options_unittest.cc @@ -0,0 +1,232 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include +#include +#include + +using namespace isc; +using namespace isc::asiolink; +using namespace isc::dhcp; +using namespace isc::dhcp::test; + +namespace { + +/// @brief Set of JSON configurations used throughout the tests. +/// +/// - Configuration 0: +/// - Used for testing direct traffic +/// - 1 subnet: 10.0.0.0/24 +/// - 1 pool: 10.0.0.10-10.0.0.100 +/// - Router option present: 10.0.0.200 and 10.0.0.201 +/// - Domain Name Server option present: 10.0.0.202, 10.0.0.203. +/// - Log Servers option present: 192.0.2.200 and 192.0.2.201 +/// - Quotes Servers option present: 192.0.2.202, 192.0.2.203. +/// +const char* HOST_CONFIGS[] = { +// Configuration 0 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"option-data\": [ {" + " \"name\": \"routers\"," + " \"data\": \"10.0.0.200,10.0.0.201\"" + " }," + " {" + " \"name\": \"domain-name-servers\"," + " \"data\": \"10.0.0.202,10.0.0.203\"" + " }," + " {" + " \"name\": \"log-servers\"," + " \"data\": \"10.0.0.200,10.0.0.201\"" + " }," + " {" + " \"name\": \"cookie-servers\"," + " \"data\": \"10.0.0.202,10.0.0.203\"" + " } ]," + " \"reservations\": [ " + " {" + " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," + " \"ip-address\": \"10.0.0.7\"," + " \"option-data\": [ {" + " \"name\": \"cookie-servers\"," + " \"data\": \"10.1.1.202,10.1.1.203\"" + " }," + " {" + " \"name\": \"log-servers\"," + " \"data\": \"10.1.1.200,10.1.1.201\"" + " } ]" + " } ]" + " } ]" + "}", + +// Configuration 1 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"option-data\": [ {" + " \"name\": \"routers\"," + " \"data\": \"10.0.0.200,10.0.0.201\"" + " }," + " {" + " \"name\": \"domain-name-servers\"," + " \"data\": \"10.0.0.202,10.0.0.203\"" + " }," + " {" + " \"name\": \"log-servers\"," + " \"data\": \"10.0.0.200,10.0.0.201\"" + " }," + " {" + " \"name\": \"cookie-servers\"," + " \"data\": \"10.0.0.202,10.0.0.203\"" + " } ]," + " \"reservations\": [ " + " {" + " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," + " \"ip-address\": \"10.0.0.7\"," + " \"option-data\": [ {" + " \"name\": \"routers\"," + " \"data\": \"10.1.1.200,10.1.1.201\"" + " }," + " {" + " \"name\": \"domain-name-servers\"," + " \"data\": \"10.1.1.202,10.1.1.203\"" + " } ]" + " } ]" + " } ]" + "}" +}; + +/// @brief Test fixture class for testing static reservations of options. +class HostOptionsTest : public Dhcpv4SrvTest { +public: + + /// @brief Constructor. + /// + /// Sets up fake interfaces. + HostOptionsTest() + : Dhcpv4SrvTest(), + iface_mgr_test_config_(true) { + IfaceMgr::instance().openSockets4(); + + // Let's wipe all existing statistics. + isc::stats::StatsMgr::instance().removeAll(); + } + + /// @brief Destructor. + /// + /// Cleans up statistics after the test. + virtual ~HostOptionsTest() { + // Let's wipe all existing statistics. + isc::stats::StatsMgr::instance().removeAll(); + } + + /// @brief Interface Manager's fake configuration control. + IfaceMgrTestConfig iface_mgr_test_config_; + +}; + +TEST_F(HostOptionsTest, overrideRequestedOptions) { + Dhcp4Client client(Dhcp4Client::SELECTING); + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + client.requestOptions(DHO_DOMAIN_NAME_SERVERS, DHO_LOG_SERVERS, + DHO_COOKIE_SERVERS); + + // Configure DHCP server. + configure(HOST_CONFIGS[0], *client.getServer()); + + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + + ASSERT_EQ(2, client.config_.routers_.size()); + EXPECT_EQ("10.0.0.200", client.config_.routers_[0].toText()); + EXPECT_EQ("10.0.0.201", client.config_.routers_[1].toText()); + // Make sure that the DNS Servers option has been received. + ASSERT_EQ(2, client.config_.dns_servers_.size()); + EXPECT_EQ("10.0.0.202", client.config_.dns_servers_[0].toText()); + EXPECT_EQ("10.0.0.203", client.config_.dns_servers_[1].toText()); + // Make sure that the Quotes Servers option has been received. + ASSERT_EQ(2, client.config_.quotes_servers_.size()); + EXPECT_EQ("10.1.1.202", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.1.1.203", client.config_.quotes_servers_[1].toText()); + // Make sure that the Log Servers option has been received. + ASSERT_EQ(2, client.config_.log_servers_.size()); + EXPECT_EQ("10.1.1.200", client.config_.log_servers_[0].toText()); + EXPECT_EQ("10.1.1.201", client.config_.log_servers_[1].toText()); +} + +TEST_F(HostOptionsTest, overrideDefaultOptions) { + Dhcp4Client client(Dhcp4Client::SELECTING); + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + + client.requestOptions(DHO_LOG_SERVERS, DHO_COOKIE_SERVERS); + + // Configure DHCP server. + configure(HOST_CONFIGS[1], *client.getServer()); + + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + + ASSERT_EQ(2, client.config_.routers_.size()); + EXPECT_EQ("10.1.1.200", client.config_.routers_[0].toText()); + EXPECT_EQ("10.1.1.201", client.config_.routers_[1].toText()); + // Make sure that the DNS Servers option has been received. + ASSERT_EQ(2, client.config_.dns_servers_.size()); + EXPECT_EQ("10.1.1.202", client.config_.dns_servers_[0].toText()); + EXPECT_EQ("10.1.1.203", client.config_.dns_servers_[1].toText()); + // Make sure that the Quotes Servers option has been received. + ASSERT_EQ(2, client.config_.quotes_servers_.size()); + EXPECT_EQ("10.0.0.202", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.0.0.203", client.config_.quotes_servers_[1].toText()); + // Make sure that the Log Servers option has been received. + ASSERT_EQ(2, client.config_.log_servers_.size()); + EXPECT_EQ("10.0.0.200", client.config_.log_servers_[0].toText()); + EXPECT_EQ("10.0.0.201", client.config_.log_servers_[1].toText()); +} + + +} // end of anonymous namespace diff --git a/src/bin/dhcp4/tests/inform_unittest.cc b/src/bin/dhcp4/tests/inform_unittest.cc index f168f38d9e..dd75f4f3a3 100644 --- a/src/bin/dhcp4/tests/inform_unittest.cc +++ b/src/bin/dhcp4/tests/inform_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -58,11 +58,11 @@ const char* INFORM_CONFIGS[] = { " }," " {" " \"name\": \"log-servers\"," - " \"data\": \"10.0.0.200,10.0.0.201\"" + " \"data\": \"10.0.0.202,10.0.0.203\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"data\": \"10.0.0.202,10.0.0.203\"" + " \"data\": \"10.0.0.200,10.0.0.201\"" " } ]" " } ]" "}", @@ -159,12 +159,12 @@ TEST_F(InformTest, directClientBroadcast) { EXPECT_EQ("10.0.0.203", client.config_.dns_servers_[1].toText()); // Make sure that the Log Servers option has been received. ASSERT_EQ(2, client.config_.quotes_servers_.size()); - EXPECT_EQ("10.0.0.200", client.config_.routers_[0].toText()); - EXPECT_EQ("10.0.0.201", client.config_.routers_[1].toText()); + EXPECT_EQ("10.0.0.200", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.0.0.201", client.config_.quotes_servers_[1].toText()); // Make sure that the Quotes Servers option has been received. ASSERT_EQ(2, client.config_.log_servers_.size()); - EXPECT_EQ("10.0.0.202", client.config_.dns_servers_[0].toText()); - EXPECT_EQ("10.0.0.203", client.config_.dns_servers_[1].toText()); + EXPECT_EQ("10.0.0.202", client.config_.log_servers_[0].toText()); + EXPECT_EQ("10.0.0.203", client.config_.log_servers_[1].toText()); // Check that we can send another DHCPINFORM message using // different ciaddr and we will get the configuration. @@ -313,14 +313,14 @@ TEST_F(InformTest, relayedClient) { ASSERT_EQ(2, client.config_.dns_servers_.size()); EXPECT_EQ("192.0.2.202", client.config_.dns_servers_[0].toText()); EXPECT_EQ("192.0.2.203", client.config_.dns_servers_[1].toText()); - // Make sure that the Log Servers option has been received. - ASSERT_EQ(2, client.config_.quotes_servers_.size()); - EXPECT_EQ("192.0.2.200", client.config_.routers_[0].toText()); - EXPECT_EQ("192.0.2.201", client.config_.routers_[1].toText()); // Make sure that the Quotes Servers option has been received. + ASSERT_EQ(2, client.config_.quotes_servers_.size()); + EXPECT_EQ("10.0.0.202", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.0.0.203", client.config_.quotes_servers_[1].toText()); + // Make sure that the Log Servers option has been received. ASSERT_EQ(2, client.config_.log_servers_.size()); - EXPECT_EQ("192.0.2.202", client.config_.dns_servers_[0].toText()); - EXPECT_EQ("192.0.2.203", client.config_.dns_servers_[1].toText()); + EXPECT_EQ("10.0.0.200", client.config_.log_servers_[0].toText()); + EXPECT_EQ("10.0.0.201", client.config_.log_servers_[1].toText()); } // This test checks that the server can respond to the DHCPINFORM message diff --git a/src/lib/dhcpsrv/host.cc b/src/lib/dhcpsrv/host.cc index c387d057ff..ac5b9f7660 100644 --- a/src/lib/dhcpsrv/host.cc +++ b/src/lib/dhcpsrv/host.cc @@ -81,7 +81,7 @@ Host::Host(const uint8_t* identifier, const size_t identifier_len, ipv4_reservation_(asiolink::IOAddress::IPV4_ZERO_ADDRESS()), hostname_(hostname), dhcp4_client_classes_(dhcp4_client_classes), dhcp6_client_classes_(dhcp6_client_classes), host_id_(0), - cfg_option4_(), cfg_option6_() { + cfg_option4_(new CfgOption()), cfg_option6_(new CfgOption()) { // Initialize host identifier. setIdentifier(identifier, identifier_len, identifier_type); From 8f75806506d6879c93f5fbc2e74884cb3c6a9df5 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 23 May 2016 14:52:14 +0200 Subject: [PATCH 2/5] [3572] Test that vendor specific options are overriden. In the new test the vendor specific options are defined on the host and global levels. Host specific options should override globally defined options. --- src/bin/dhcp4/tests/dhcp4_client.cc | 11 ++ src/bin/dhcp4/tests/dhcp4_client.h | 3 + src/bin/dhcp4/tests/host_options_unittest.cc | 154 +++++++++++++++++-- src/lib/dhcp/std_option_defs.h | 2 +- 4 files changed, 154 insertions(+), 16 deletions(-) diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc index c801b57edf..82c78e7a60 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.cc +++ b/src/bin/dhcp4/tests/dhcp4_client.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -171,6 +172,12 @@ Dhcp4Client::applyConfiguration() { if (opt_quotes_servers) { config_.quotes_servers_ = opt_quotes_servers->getAddresses(); } + // Vendor Specific options + OptionVendorPtr opt_vendor = boost::dynamic_pointer_cast< + OptionVendor>(resp->getOption(DHO_VIVSO_SUBOPTIONS)); + if (opt_vendor) { + config_.vendor_suboptions_ = opt_vendor->getOptions(); + } // Server Identifier OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast< OptionCustom>(resp->getOption(DHO_DHCP_SERVER_IDENTIFIER)); @@ -254,6 +261,8 @@ Dhcp4Client::doInform(const bool set_ciaddr) { context_.query_ = createMsg(DHCPINFORM); // Request options if any. appendPRL(); + // Any other options to be sent by a client. + appendExtraOptions(); // The client sending a DHCPINFORM message has an IP address obtained // by some other means, e.g. static configuration. The lease which we // are using here is most likely set by the createLease method. @@ -368,6 +377,8 @@ Dhcp4Client::doRequest() { appendName(); // Include Client Identifier appendClientId(); + // Any other options to be sent by a client. + appendExtraOptions(); // Send the message to the server. sendMsg(context_.query_); // Expect response. diff --git a/src/bin/dhcp4/tests/dhcp4_client.h b/src/bin/dhcp4/tests/dhcp4_client.h index 71dc9544ee..480a8a426d 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.h +++ b/src/bin/dhcp4/tests/dhcp4_client.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -73,6 +74,8 @@ public: Option4AddrLst::AddressContainer log_servers_; /// @brief Holds IP addresses received in the Quotes Servers option. Option4AddrLst::AddressContainer quotes_servers_; + /// @brief Vendor Specific options + OptionCollection vendor_suboptions_; /// @brief Holds a lease obtained by the client. Lease4 lease_; /// @brief Holds server id of the server which responded to the client's diff --git a/src/bin/dhcp4/tests/host_options_unittest.cc b/src/bin/dhcp4/tests/host_options_unittest.cc index 090219db95..901ec6dcc4 100644 --- a/src/bin/dhcp4/tests/host_options_unittest.cc +++ b/src/bin/dhcp4/tests/host_options_unittest.cc @@ -6,6 +6,9 @@ #include #include +#include +#include +#include #include #include #include @@ -21,13 +24,50 @@ namespace { /// @brief Set of JSON configurations used throughout the tests. /// /// - Configuration 0: -/// - Used for testing direct traffic -/// - 1 subnet: 10.0.0.0/24 -/// - 1 pool: 10.0.0.10-10.0.0.100 -/// - Router option present: 10.0.0.200 and 10.0.0.201 -/// - Domain Name Server option present: 10.0.0.202, 10.0.0.203. -/// - Log Servers option present: 192.0.2.200 and 192.0.2.201 -/// - Quotes Servers option present: 192.0.2.202, 192.0.2.203. +/// - Used to test that host specific options override subnet specific +/// options when these options are requested with PRL option. +/// - Single subnet 10.0.0.0/24 with a pool of 10.0.0.10-10.0.0.100 +/// - 4 options configured within subnet scope +/// - routers: 10.0.0.200,10.0.0.201, +/// - domain-name-servers: 10.0.0.202,10.0.0.203, +/// - log-servers: 10.0.0.200,10.0.0.201, +/// - cookie-servers: 10.0.0.202,10.0.0.203 +/// - Single reservation within the subnet: +/// - HW address: aa:bb:cc:dd:ee:ff +/// - ip-address: 10.0.0.7 +/// - Two options overriding subnet specific options: +/// - cookie-servers: 10.1.1.202, 10.1.1.203 +/// - log-servers: 10.1.1.200, 10.1.1.201 +/// +/// - Configuration 1: +/// - Used to test that host specific options override subnet specific +/// default options. Default options are those that are sent even when +/// not requested by a client. +/// - Single subnet 10.0.0.0/24 with a pool of 10.0.0.10-10.0.0.100 +/// - 4 options configured within subnet scope +/// - routers: 10.0.0.200,10.0.0.201, +/// - domain-name-servers: 10.0.0.202,10.0.0.203, +/// - log-servers: 10.0.0.200,10.0.0.201, +/// - cookie-servers: 10.0.0.202,10.0.0.203 +/// - Single reservation within the subnet: +/// - HW address: aa:bb:cc:dd:ee:ff +/// - ip-address: 10.0.0.7 +/// - Two options overriding subnet specific default options: +/// - routers: 10.1.1.200, 10.1.1.201 +/// - domain-name-servers: 10.1.1.202, 10.1.1.203 +/// +/// - Configuration 2: +/// - Used to test that host specific vendor options override globally +/// specified vendor options. +/// - Globally specified option 125 with Cable Labs vendor id. +/// - TFTP servers sub option: 10.0.0.202,10.0.0.203 +/// - Single subnet 10.0.0.0/24 with a pool of 10.0.0.10-10.0.0.100 +/// - Single reservation within the subnet: +/// - HW address: aa:bb:cc:dd:ee:ff +/// - ip-adress 10.0.0.7 +/// - Vendor option for Cable Labs vendor id specified for the reservation: +/// - TFTP servers suboption overriding globally spececified suboption: +/// 10.1.1.202,10.1.1.203 /// const char* HOST_CONFIGS[] = { // Configuration 0 @@ -110,6 +150,41 @@ const char* HOST_CONFIGS[] = { " } ]" " } ]" " } ]" + "}", + +// Configuration 2 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"option-data\": [ {" + " \"name\": \"vivso-suboptions\"," + " \"data\": 4491" + "}," + "{" + " \"name\": \"tftp-servers\"," + " \"space\": \"vendor-4491\"," + " \"data\": \"10.0.0.202,10.0.0.203\"" + "} ]," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"reservations\": [ " + " {" + " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," + " \"ip-address\": \"10.0.0.7\"," + " \"option-data\": [ {" + " \"name\": \"vivso-suboptions\"," + " \"data\": 4491" + " }," + " {" + " \"name\": \"tftp-servers\"," + " \"space\": \"vendor-4491\"," + " \"data\": \"10.1.1.202,10.1.1.203\"" + " } ]" + " } ]" + " } ]" "}" }; @@ -142,6 +217,9 @@ public: }; +// This test checks that host specific options override subnet specific +// options. Overridden options are requested with Parameter Request List +// option. TEST_F(HostOptionsTest, overrideRequestedOptions) { Dhcp4Client client(Dhcp4Client::SELECTING); client.setHWAddress("aa:bb:cc:dd:ee:ff"); @@ -160,10 +238,7 @@ TEST_F(HostOptionsTest, overrideRequestedOptions) { Pkt4Ptr resp = client.getContext().response_; // Make sure that the server has responded with DHCPACK. ASSERT_EQ(DHCPACK, static_cast(resp->getType())); - // Response must not be relayed. - EXPECT_FALSE(resp->isRelayed()); - // Make sure that the server id is present. - EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease for the reserved // address. ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); @@ -185,6 +260,10 @@ TEST_F(HostOptionsTest, overrideRequestedOptions) { EXPECT_EQ("10.1.1.201", client.config_.log_servers_[1].toText()); } +// This test checks that host specific options override subnet specific +// options. Overridden options are the options which server sends +// regardless if they are requested with Parameter Request List option +// or not. TEST_F(HostOptionsTest, overrideDefaultOptions) { Dhcp4Client client(Dhcp4Client::SELECTING); client.setHWAddress("aa:bb:cc:dd:ee:ff"); @@ -203,10 +282,7 @@ TEST_F(HostOptionsTest, overrideDefaultOptions) { Pkt4Ptr resp = client.getContext().response_; // Make sure that the server has responded with DHCPACK. ASSERT_EQ(DHCPACK, static_cast(resp->getType())); - // Response must not be relayed. - EXPECT_FALSE(resp->isRelayed()); - // Make sure that the server id is present. - EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease for the reserved // address. ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); @@ -228,5 +304,53 @@ TEST_F(HostOptionsTest, overrideDefaultOptions) { EXPECT_EQ("10.0.0.201", client.config_.log_servers_[1].toText()); } +// This test checks that host specific vendor options override vendor +// options defined in the global scope. +TEST_F(HostOptionsTest, overrideVendorOptions) { + Dhcp4Client client(Dhcp4Client::SELECTING); + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + + // Client needs to include V-I Vendor Specific Information option + // to include ORO suboption, which the server will use to determine + // which suboptions should be returned to the client. + OptionVendorPtr opt_vendor(new OptionVendor(Option::V4, + VENDOR_ID_CABLE_LABS)); + // Include ORO with TFTP servers suboption code being requested. + opt_vendor->addOption(OptionPtr(new OptionUint8(Option::V4, DOCSIS3_V4_ORO, + DOCSIS3_V4_TFTP_SERVERS))); + client.addExtraOption(opt_vendor); + + // Configure DHCP server. + configure(HOST_CONFIGS[2], *client.getServer()); + + // Perform 4-way exchange with the server. + ASSERT_NO_THROW(client.doDORA()); + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + + // Make sure the server has responded with a V-I Vendor Specific + // Information option with exactly one suboption. + ASSERT_EQ(1, client.config_.vendor_suboptions_.size()); + // Assume this suboption is a TFTP servers suboption. + std::multimap::const_iterator opt = + client.config_.vendor_suboptions_.find(DOCSIS3_V4_TFTP_SERVERS); + Option4AddrLstPtr opt_tftp = boost::dynamic_pointer_cast< + Option4AddrLst>(opt->second); + ASSERT_TRUE(opt_tftp); + // TFTP servers suboption should contain addresses specified on host level. + const Option4AddrLst::AddressContainer& tftps = opt_tftp->getAddresses(); + ASSERT_EQ(2, tftps.size()); + EXPECT_EQ("10.1.1.202", tftps[0].toText()); + EXPECT_EQ("10.1.1.203", tftps[1].toText()); +} + } // end of anonymous namespace diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h index 9bbc29f88c..1a62fd8fa7 100644 --- a/src/lib/dhcp/std_option_defs.h +++ b/src/lib/dhcp/std_option_defs.h @@ -211,7 +211,7 @@ const OptionDefParams OPTION_DEF_PARAMS4[] = { { "domain-search", DHO_DOMAIN_SEARCH, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "vivco-suboptions", DHO_VIVCO_SUBOPTIONS, OPT_RECORD_TYPE, false, RECORD_DEF(VIVCO_RECORDS), "" }, - { "vivso-suboptions", DHO_VIVSO_SUBOPTIONS, OPT_BINARY_TYPE, + { "vivso-suboptions", DHO_VIVSO_SUBOPTIONS, OPT_UINT32_TYPE, false, NO_RECORD_DEF, "" } // @todo add definitions for all remaning options. From 68202299ffd7f7b1a01d93f9c7af0cb66c352a22 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 23 May 2016 16:54:37 +0200 Subject: [PATCH 3/5] [3572] Test assignment of host specific options with DHCPINFORM. --- src/bin/dhcp4/tests/host_options_unittest.cc | 263 +++++++++++++++++-- 1 file changed, 236 insertions(+), 27 deletions(-) diff --git a/src/bin/dhcp4/tests/host_options_unittest.cc b/src/bin/dhcp4/tests/host_options_unittest.cc index 901ec6dcc4..9598c4a436 100644 --- a/src/bin/dhcp4/tests/host_options_unittest.cc +++ b/src/bin/dhcp4/tests/host_options_unittest.cc @@ -5,6 +5,7 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include #include #include @@ -21,6 +22,12 @@ using namespace isc::dhcp::test; namespace { +/// @brief Boolean value used to signal stateless configuration test. +const bool STATELESS = true; + +/// @brief Boolean value used to signal stateful configuration test. +const bool STATEFUL = false; + /// @brief Set of JSON configurations used throughout the tests. /// /// - Configuration 0: @@ -57,6 +64,16 @@ namespace { /// - domain-name-servers: 10.1.1.202, 10.1.1.203 /// /// - Configuration 2: +/// - Used to test that client receives options solely specified in a +/// host scope. +/// - Single reservation within the subnet: +/// - HW address: aa:bb:cc:dd:ee:ff +/// - ip-address: 10.0.0.7 +/// - Two options: +/// - routers: 10.1.1.200, 10.1.1.201 +/// - cookie-servers: 10.1.1.202, 10.1.1.203 +/// +/// - Configuration 3: /// - Used to test that host specific vendor options override globally /// specified vendor options. /// - Globally specified option 125 with Cable Labs vendor id. @@ -78,6 +95,7 @@ const char* HOST_CONFIGS[] = { "\"subnet4\": [ { " " \"subnet\": \"10.0.0.0/24\", " " \"id\": 1," + " \"relay\": { \"ip-address\": \"10.0.0.233\" }," " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," @@ -119,6 +137,7 @@ const char* HOST_CONFIGS[] = { "\"subnet4\": [ { " " \"subnet\": \"10.0.0.0/24\", " " \"id\": 1," + " \"relay\": { \"ip-address\": \"10.0.0.233\" }," " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"option-data\": [ {" " \"name\": \"routers\"," @@ -153,6 +172,31 @@ const char* HOST_CONFIGS[] = { "}", // Configuration 2 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"relay\": { \"ip-address\": \"10.0.0.233\" }," + " \"reservations\": [ " + " {" + " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," + " \"ip-address\": \"10.0.0.7\"," + " \"option-data\": [ {" + " \"name\": \"routers\"," + " \"data\": \"10.1.1.200,10.1.1.201\"" + " }," + " {" + " \"name\": \"cookie-servers\"," + " \"data\": \"10.1.1.202,10.1.1.203\"" + " } ]" + " } ]" + " } ]" + "}", + +// Configuration 3 "{ \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" "}," @@ -169,6 +213,7 @@ const char* HOST_CONFIGS[] = { "\"subnet4\": [ { " " \"subnet\": \"10.0.0.0/24\", " " \"id\": 1," + " \"relay\": { \"ip-address\": \"10.0.0.233\" }," " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," " \"reservations\": [ " " {" @@ -212,15 +257,47 @@ public: isc::stats::StatsMgr::instance().removeAll(); } + /// @brief Verifies that host specific options override subnet specific + /// options. + /// + /// Overridden options are requested with Parameter Request List + /// option. + /// + /// @param stateless Boolean value indicating if statless or stateful + /// configuration should be performed. + void testOverrideRequestedOptions(const bool stateless); + + /// @brief Verifies that host specific options override subnet specific + /// options. + /// + /// Overridden options are the options which server sends regardless + /// if they are requested with Parameter Request List option or not. + /// + /// @param stateless Boolean value indicating if statless or stateful + /// configuration should be performed. + void testOverrideDefaultOptions(const bool stateless); + + /// @brief Verifies that client receives options when they are solely + /// defined in the host scope (and not in the global or subnet scope). + /// + /// @param stateless Boolean value indicating if statless or stateful + /// configuration should be performed. + void testHostOnlyOptions(const bool stateless); + + /// @brief Verifies that host specific vendor options override vendor + /// options defined in the global scope. + /// + /// @param stateless Boolean value indicating if statless or stateful + /// configuration should be performed. + void testOverrideVendorOptions(const bool stateless); + /// @brief Interface Manager's fake configuration control. IfaceMgrTestConfig iface_mgr_test_config_; }; -// This test checks that host specific options override subnet specific -// options. Overridden options are requested with Parameter Request List -// option. -TEST_F(HostOptionsTest, overrideRequestedOptions) { +void +HostOptionsTest::testOverrideRequestedOptions(const bool stateless) { Dhcp4Client client(Dhcp4Client::SELECTING); client.setHWAddress("aa:bb:cc:dd:ee:ff"); client.requestOptions(DHO_DOMAIN_NAME_SERVERS, DHO_LOG_SERVERS, @@ -229,9 +306,17 @@ TEST_F(HostOptionsTest, overrideRequestedOptions) { // Configure DHCP server. configure(HOST_CONFIGS[0], *client.getServer()); - // Perform 4-way exchange with the server but to not request any - // specific address in the DHCPDISCOVER message. - ASSERT_NO_THROW(client.doDORA()); + if (stateless) { + // Need to relay the message from a specific address which can + // be matched with a configured subnet. + client.useRelay(true, IOAddress("10.0.0.233")); + ASSERT_NO_THROW(client.doInform()); + + } else { + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + } // Make sure that the server responded. ASSERT_TRUE(client.getContext().response_); @@ -239,9 +324,11 @@ TEST_F(HostOptionsTest, overrideRequestedOptions) { // Make sure that the server has responded with DHCPACK. ASSERT_EQ(DHCPACK, static_cast(resp->getType())); - // Make sure that the client has got the lease for the reserved - // address. - ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + if (!stateless) { + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + } ASSERT_EQ(2, client.config_.routers_.size()); EXPECT_EQ("10.0.0.200", client.config_.routers_[0].toText()); @@ -260,11 +347,8 @@ TEST_F(HostOptionsTest, overrideRequestedOptions) { EXPECT_EQ("10.1.1.201", client.config_.log_servers_[1].toText()); } -// This test checks that host specific options override subnet specific -// options. Overridden options are the options which server sends -// regardless if they are requested with Parameter Request List option -// or not. -TEST_F(HostOptionsTest, overrideDefaultOptions) { +void +HostOptionsTest::testOverrideDefaultOptions(const bool stateless) { Dhcp4Client client(Dhcp4Client::SELECTING); client.setHWAddress("aa:bb:cc:dd:ee:ff"); @@ -273,6 +357,18 @@ TEST_F(HostOptionsTest, overrideDefaultOptions) { // Configure DHCP server. configure(HOST_CONFIGS[1], *client.getServer()); + if (stateless) { + // Need to relay the message from a specific address which can + // be matched with a configured subnet. + client.useRelay(true, IOAddress("10.0.0.233")); + ASSERT_NO_THROW(client.doInform()); + + } else { + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + } + // Perform 4-way exchange with the server but to not request any // specific address in the DHCPDISCOVER message. ASSERT_NO_THROW(client.doDORA()); @@ -283,9 +379,11 @@ TEST_F(HostOptionsTest, overrideDefaultOptions) { // Make sure that the server has responded with DHCPACK. ASSERT_EQ(DHCPACK, static_cast(resp->getType())); - // Make sure that the client has got the lease for the reserved - // address. - ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + if (!stateless) { + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + } ASSERT_EQ(2, client.config_.routers_.size()); EXPECT_EQ("10.1.1.200", client.config_.routers_[0].toText()); @@ -304,9 +402,55 @@ TEST_F(HostOptionsTest, overrideDefaultOptions) { EXPECT_EQ("10.0.0.201", client.config_.log_servers_[1].toText()); } -// This test checks that host specific vendor options override vendor -// options defined in the global scope. -TEST_F(HostOptionsTest, overrideVendorOptions) { +void +HostOptionsTest::testHostOnlyOptions(const bool stateless) { + Dhcp4Client client(Dhcp4Client::SELECTING); + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + client.requestOptions(DHO_COOKIE_SERVERS); + + // Configure DHCP server. + configure(HOST_CONFIGS[2], *client.getServer()); + + if (stateless) { + // Need to relay the message from a specific address which can + // be matched with a configured subnet. + client.useRelay(true, IOAddress("10.0.0.233")); + ASSERT_NO_THROW(client.doInform()); + + } else { + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + ASSERT_NO_THROW(client.doDORA()); + } + + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + + if (!stateless) { + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + } + + // Make sure that the Routers options has been received. + ASSERT_EQ(2, client.config_.routers_.size()); + EXPECT_EQ("10.1.1.200", client.config_.routers_[0].toText()); + EXPECT_EQ("10.1.1.201", client.config_.routers_[1].toText()); + // Make sure that the Quotes Servers option has been received. + ASSERT_EQ(2, client.config_.quotes_servers_.size()); + EXPECT_EQ("10.1.1.202", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.1.1.203", client.config_.quotes_servers_[1].toText()); + + // Other options are not configured and should not be delivered. + EXPECT_EQ(0, client.config_.dns_servers_.size()); + EXPECT_EQ(0, client.config_.log_servers_.size()); +} + +void +HostOptionsTest::testOverrideVendorOptions(const bool stateless) { Dhcp4Client client(Dhcp4Client::SELECTING); client.setHWAddress("aa:bb:cc:dd:ee:ff"); @@ -321,10 +465,18 @@ TEST_F(HostOptionsTest, overrideVendorOptions) { client.addExtraOption(opt_vendor); // Configure DHCP server. - configure(HOST_CONFIGS[2], *client.getServer()); + configure(HOST_CONFIGS[3], *client.getServer()); - // Perform 4-way exchange with the server. - ASSERT_NO_THROW(client.doDORA()); + if (stateless) { + // Need to relay the message from a specific address which can + // be matched with a configured subnet. + client.useRelay(true, IOAddress("10.0.0.233")); + ASSERT_NO_THROW(client.doInform()); + + } else { + // Perform 4-way exchange with the server. + ASSERT_NO_THROW(client.doDORA()); + } // Make sure that the server responded. ASSERT_TRUE(client.getContext().response_); @@ -332,9 +484,11 @@ TEST_F(HostOptionsTest, overrideVendorOptions) { // Make sure that the server has responded with DHCPACK. ASSERT_EQ(DHCPACK, static_cast(resp->getType())); - // Make sure that the client has got the lease for the reserved - // address. - ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + if (!stateless) { + // Make sure that the client has got the lease for the reserved + // address. + ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText()); + } // Make sure the server has responded with a V-I Vendor Specific // Information option with exactly one suboption. @@ -352,5 +506,60 @@ TEST_F(HostOptionsTest, overrideVendorOptions) { EXPECT_EQ("10.1.1.203", tftps[1].toText()); } +// This test checks that host specific options override subnet specific +// options. Overridden options are requested with Parameter Request List +// option (stateless case). +TEST_F(HostOptionsTest, overrideRequestedOptionsStateless) { + testOverrideRequestedOptions(STATELESS); +} + +// This test checks that host specific options override subnet specific +// options. Overridden options are requested with Parameter Request List +// option (stateful case). +TEST_F(HostOptionsTest, overrideRequestedOptionsStateful) { + testOverrideRequestedOptions(STATEFUL); +} + +// This test checks that host specific options override subnet specific +// options. Overridden options are the options which server sends +// regardless if they are requested with Parameter Request List option +// or not (stateless case). +TEST_F(HostOptionsTest, overrideDefaultOptionsStateless) { + testOverrideDefaultOptions(STATELESS); +} + +// This test checks that host specific options override subnet specific +// options. Overridden options are the options which server sends +// regardless if they are requested with Parameter Request List option +// or not (stateful case). +TEST_F(HostOptionsTest, overrideDefaultOptionsStateful) { + testOverrideDefaultOptions(STATEFUL); +} + +// This test checks that client receives options when they are +// solely defined in the host scope and not in the global or subnet +// scope (stateless case). +TEST_F(HostOptionsTest, hostOnlyOptionsStateless) { + testHostOnlyOptions(STATELESS); +} + +// This test checks that client receives options when they are +// solely defined in the host scope and not in the global or subnet +// scope (stateful case). +TEST_F(HostOptionsTest, hostOnlyOptionsStateful) { + testHostOnlyOptions(STATEFUL); +} + +// This test checks that host specific vendor options override vendor +// options defined in the global scope (stateless case). +TEST_F(HostOptionsTest, overrideVendorOptionsStateless) { + testOverrideVendorOptions(STATELESS); +} + +// This test checks that host specific vendor options override vendor +// options defined in the global scope (stateful case). +TEST_F(HostOptionsTest, overrideVendorOptionsStateful) { + testOverrideVendorOptions(STATEFUL); +} } // end of anonymous namespace From 97113367f5c999d9c2030a5e9ec464b63879b7da Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 31 May 2016 11:37:28 +0200 Subject: [PATCH 4/5] [3572] Discard DHCPv4 host options container if it is empty. This is a small performance improvement. --- src/bin/dhcp4/dhcp4_srv.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 47afca878a..05333e0cb7 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -926,7 +926,7 @@ Dhcpv4Srv::buildCfgOptionList(Dhcpv4Exchange& ex) { // Firstly, host specific options. const ConstHostPtr& host = ex.getContext()->host_; - if (host) { + if (host && !host->getCfgOption4()->empty()) { co_list.push_back(host->getCfgOption4()); } From 2487e1e2239904a661b9684ea0efc19e2c11b3cd Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 8 Jun 2016 18:27:40 +0200 Subject: [PATCH 5/5] [3572] Addressed review comments. --- src/bin/dhcp4/tests/host_options_unittest.cc | 34 +++++++------------- src/lib/dhcp/std_option_defs.h | 12 +++++++ 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/bin/dhcp4/tests/host_options_unittest.cc b/src/bin/dhcp4/tests/host_options_unittest.cc index 9598c4a436..b2c86e0660 100644 --- a/src/bin/dhcp4/tests/host_options_unittest.cc +++ b/src/bin/dhcp4/tests/host_options_unittest.cc @@ -107,11 +107,11 @@ const char* HOST_CONFIGS[] = { " }," " {" " \"name\": \"log-servers\"," - " \"data\": \"10.0.0.200,10.0.0.201\"" + " \"data\": \"10.0.0.204,10.0.0.205\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"data\": \"10.0.0.202,10.0.0.203\"" + " \"data\": \"10.0.0.206,10.0.0.207\"" " } ]," " \"reservations\": [ " " {" @@ -149,11 +149,11 @@ const char* HOST_CONFIGS[] = { " }," " {" " \"name\": \"log-servers\"," - " \"data\": \"10.0.0.200,10.0.0.201\"" + " \"data\": \"10.0.0.204,10.0.0.205\"" " }," " {" " \"name\": \"cookie-servers\"," - " \"data\": \"10.0.0.202,10.0.0.203\"" + " \"data\": \"10.0.0.206,10.0.0.207\"" " } ]," " \"reservations\": [ " " {" @@ -190,7 +190,7 @@ const char* HOST_CONFIGS[] = { " }," " {" " \"name\": \"cookie-servers\"," - " \"data\": \"10.1.1.202,10.1.1.203\"" + " \"data\": \"10.1.1.206,10.1.1.207\"" " } ]" " } ]" " } ]" @@ -244,17 +244,6 @@ public: : Dhcpv4SrvTest(), iface_mgr_test_config_(true) { IfaceMgr::instance().openSockets4(); - - // Let's wipe all existing statistics. - isc::stats::StatsMgr::instance().removeAll(); - } - - /// @brief Destructor. - /// - /// Cleans up statistics after the test. - virtual ~HostOptionsTest() { - // Let's wipe all existing statistics. - isc::stats::StatsMgr::instance().removeAll(); } /// @brief Verifies that host specific options override subnet specific @@ -394,12 +383,12 @@ HostOptionsTest::testOverrideDefaultOptions(const bool stateless) { EXPECT_EQ("10.1.1.203", client.config_.dns_servers_[1].toText()); // Make sure that the Quotes Servers option has been received. ASSERT_EQ(2, client.config_.quotes_servers_.size()); - EXPECT_EQ("10.0.0.202", client.config_.quotes_servers_[0].toText()); - EXPECT_EQ("10.0.0.203", client.config_.quotes_servers_[1].toText()); + EXPECT_EQ("10.0.0.206", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.0.0.207", client.config_.quotes_servers_[1].toText()); // Make sure that the Log Servers option has been received. ASSERT_EQ(2, client.config_.log_servers_.size()); - EXPECT_EQ("10.0.0.200", client.config_.log_servers_[0].toText()); - EXPECT_EQ("10.0.0.201", client.config_.log_servers_[1].toText()); + EXPECT_EQ("10.0.0.204", client.config_.log_servers_[0].toText()); + EXPECT_EQ("10.0.0.205", client.config_.log_servers_[1].toText()); } void @@ -441,8 +430,8 @@ HostOptionsTest::testHostOnlyOptions(const bool stateless) { EXPECT_EQ("10.1.1.201", client.config_.routers_[1].toText()); // Make sure that the Quotes Servers option has been received. ASSERT_EQ(2, client.config_.quotes_servers_.size()); - EXPECT_EQ("10.1.1.202", client.config_.quotes_servers_[0].toText()); - EXPECT_EQ("10.1.1.203", client.config_.quotes_servers_[1].toText()); + EXPECT_EQ("10.1.1.206", client.config_.quotes_servers_[0].toText()); + EXPECT_EQ("10.1.1.207", client.config_.quotes_servers_[1].toText()); // Other options are not configured and should not be delivered. EXPECT_EQ(0, client.config_.dns_servers_.size()); @@ -496,6 +485,7 @@ HostOptionsTest::testOverrideVendorOptions(const bool stateless) { // Assume this suboption is a TFTP servers suboption. std::multimap::const_iterator opt = client.config_.vendor_suboptions_.find(DOCSIS3_V4_TFTP_SERVERS); + ASSERT_TRUE(opt->second); Option4AddrLstPtr opt_tftp = boost::dynamic_pointer_cast< Option4AddrLst>(opt->second); ASSERT_TRUE(opt_tftp); diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h index 1a62fd8fa7..10935453ba 100644 --- a/src/lib/dhcp/std_option_defs.h +++ b/src/lib/dhcp/std_option_defs.h @@ -211,6 +211,18 @@ const OptionDefParams OPTION_DEF_PARAMS4[] = { { "domain-search", DHO_DOMAIN_SEARCH, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" }, { "vivco-suboptions", DHO_VIVCO_SUBOPTIONS, OPT_RECORD_TYPE, false, RECORD_DEF(VIVCO_RECORDS), "" }, + // Vendor-Identifying Vendor Specific Information option payload begins with a + // 32-bit log enterprise number, followed by a tuple of data-len/option-data. + // The format defined here includes 32-bit field holding enterprise number. + // This allows for specifying option-data information where the enterprise-id + // is represented by a uint32_t value. Previously we represented this option + // as a binary, but that would imply that enterprise number would have to be + // represented in binary format in the server configuration. That would be + // inconvenient and non-intuitive. + /// @todo We need to extend support for vendor options with ability to specify + /// multiple enterprise numbers for a single option. Perhaps it would be + /// ok to specify multiple instances of the "vivso-suboptions" which will be + /// combined in a single option by the server before responding to a client. { "vivso-suboptions", DHO_VIVSO_SUBOPTIONS, OPT_UINT32_TYPE, false, NO_RECORD_DEF, "" }