From: Marcin Siodelski Date: Wed, 8 Jun 2016 16:27:40 +0000 (+0200) Subject: [3572] Addressed review comments. X-Git-Tag: trac4272a_base~3^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2487e1e2239904a661b9684ea0efc19e2c11b3cd;p=thirdparty%2Fkea.git [3572] Addressed review comments. --- 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, "" }