diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index 0e86217309..3e3cebc8ea 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -281,24 +281,32 @@ protected: /// @param addr is IPv4 address of the subnet. /// @param len is the prefix length void initSubnet(isc::asiolink::IOAddress addr, uint8_t len) { - // Get all 'time' parameters using inheritance. - // If the subnet-specific value is defined then use it, else - // use the global value. The global value must always be - // present. If it is not, it is an internal error and exception - // is thrown. - Triplet t1 = getParam("renew-timer"); - Triplet t2 = getParam("rebind-timer"); + // The renew-timer and rebind-timer are optional. If not set, the + // option 58 and 59 will not be sent to a client. In this case the + // client will use default values based on the valid-lifetime. + Triplet t1 = getOptionalParam("renew-timer"); + Triplet t2 = getOptionalParam("rebind-timer"); + // The valid-lifetime is mandatory. It may be specified for a + // particular subnet. If not, the global value should be present. + // If there is no global value, exception is thrown. Triplet valid = getParam("valid-lifetime"); // Subnet ID is optional. If it is not supplied the value of 0 is used, // which means autogenerate. SubnetID subnet_id = static_cast(uint32_values_->getOptionalParam("id", 0)); - stringstream tmp; - tmp << addr << "/" << (int)len - << " with params t1=" << t1 << ", t2=" << t2 << ", valid=" << valid; + stringstream s; + s << addr << "/" << static_cast(len) << " with params: "; + // t1 and t2 are optional may be not specified. + if (!t1.unspecified()) { + s << "t1=" << t1 << ", "; + } + if (!t2.unspecified()) { + s << "t2=" << t2 << ", "; + } + s <<"valid-lifetime=" << valid; - LOG_INFO(dhcp4_logger, DHCP4_CONFIG_NEW_SUBNET).arg(tmp.str()); + LOG_INFO(dhcp4_logger, DHCP4_CONFIG_NEW_SUBNET).arg(s.str()); Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid, subnet_id)); subnet_ = subnet4; diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 2afee03f59..c0135b4b97 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -79,6 +79,8 @@ public: // is sane. srv_.reset(new Dhcpv4Srv(0)); CfgMgr::instance().deleteActiveIfaces(); + // Create fresh context. + globalContext()->copyContext(ParserContext(Option::V4)); } // Check that no hooks libraries are loaded. This is a pre-condition for @@ -510,6 +512,73 @@ TEST_F(Dhcp4ParserTest, emptySubnet) { checkGlobalUint32("valid-lifetime", 4000); } +/// Check that the renew-timer doesn't have to be specified, in which case +/// it is marked unspecified in the Subnet. +TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) { + ConstElementPtr status; + + string config = "{ \"interfaces\": [ \"*\" ]," + "\"rebind-timer\": 2000, " + "\"subnet4\": [ { " + " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // returned value should be 0 (success) + checkResult(status, 0); + checkGlobalUint32("rebind-timer", 2000); + checkGlobalUint32("valid-lifetime", 4000); + + Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"), + classify_); + ASSERT_TRUE(subnet); + EXPECT_TRUE(subnet->getT1().unspecified()); + EXPECT_FALSE(subnet->getT2().unspecified()); + EXPECT_EQ(2000, subnet->getT2()); + EXPECT_EQ(4000, subnet->getValid()); + + // Check that subnet-id is 1 + EXPECT_EQ(1, subnet->getID()); + +} + +/// Check that the rebind-timer doesn't have to be specified, in which case +/// it is marked unspecified in the Subnet. +TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) { + ConstElementPtr status; + + string config = "{ \"interfaces\": [ \"*\" ]," + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + + // returned value should be 0 (success) + checkResult(status, 0); + checkGlobalUint32("renew-timer", 1000); + checkGlobalUint32("valid-lifetime", 4000); + + Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"), + classify_); + ASSERT_TRUE(subnet); + EXPECT_FALSE(subnet->getT1().unspecified()); + EXPECT_EQ(1000, subnet->getT1()); + EXPECT_TRUE(subnet->getT2().unspecified()); + EXPECT_EQ(4000, subnet->getValid()); + + // Check that subnet-id is 1 + EXPECT_EQ(1, subnet->getID()); +} + /// The goal of this test is to verify if defined subnet uses global /// parameter timer definitions. TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) { @@ -1700,6 +1769,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { // belongs to the 'dhcp4' option space as it is the // standard option. string config = "{ \"interfaces\": [ \"*\" ]," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1779,6 +1849,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // Starting stage 1. Configure sub-options and their definitions. string config = "{ \"interfaces\": [ \"*\" ]," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -2352,6 +2423,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { // that we will add to the base option. // Let's create some dummy options: foo and foo2. string config = "{ \"interfaces\": [ \"*\" ]," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -2512,6 +2584,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsHex) { // sharing the code 1 and belonging to the different vendor spaces. // (different vendor-id values). string config = "{ \"interfaces\": [ \"*\" ]," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -2570,6 +2643,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsCsv) { // sharing the code 1 and belonging to the different vendor spaces. // (different vendor-id values). string config = "{ \"interfaces\": [ \"*\" ]," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -2646,6 +2720,7 @@ buildHooksLibrariesConfig(const std::vector& libraries) { // Append the remainder of the configuration. config += string( "]," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index c3f809812a..37a567841f 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -1336,6 +1336,16 @@ SubnetConfigParser::getParam(const std::string& name) { return (Triplet(value)); } +isc::dhcp::Triplet +SubnetConfigParser::getOptionalParam(const std::string& name) { + try { + return (getParam(name)); + } catch (const DhcpConfigError &) { + // No error. We will return an unspecified value. + } + return (Triplet()); +} + //**************************** D2ClientConfigParser ********************** D2ClientConfigParser::D2ClientConfigParser(const std::string& entry_name) : entry_name_(entry_name), boolean_values_(new BooleanStorage()), diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h index f6da862cd4..3ca074e76e 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.h +++ b/src/lib/dhcpsrv/dhcp_parsers.h @@ -993,6 +993,18 @@ protected: /// @throw DhcpConfigError when requested parameter is not present isc::dhcp::Triplet getParam(const std::string& name); + /// @brief Returns optional value for a given parameter. + /// + /// This method checks if an optional parameter has been specified for + /// a subnet. If not, it will try to use a global value. If the global + /// value is not specified it will return an object representing an + /// unspecified value. + /// + /// @param name name of the configuration parameter. + /// @return An optional value or a @c Triplet object representing + /// unspecified value. + isc::dhcp::Triplet getOptionalParam(const std::string& name); + private: /// @brief Append sub-options to an option.