[3336] renew-timer and rebind-timer are optional parameters in Kea4.

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<uint32_t> t1 = getParam("renew-timer");
-        Triplet<uint32_t> 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<uint32_t> t1 = getOptionalParam("renew-timer");
+        Triplet<uint32_t> 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<uint32_t> 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<SubnetID>(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<int>(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;

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<std::string>& libraries) {
     // Append the remainder of the configuration.
     config += string(
         "],"
+        "\"valid-lifetime\": 4000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"

src/lib/dhcpsrv/dhcp_parsers.cc

@@ -1336,6 +1336,16 @@ SubnetConfigParser::getParam(const std::string& name) {
     return (Triplet<uint32_t>(value));
 }
 
+isc::dhcp::Triplet<uint32_t>
+SubnetConfigParser::getOptionalParam(const std::string& name) {
+    try {
+        return (getParam(name));
+    } catch (const DhcpConfigError &) {
+        // No error. We will return an unspecified value.
+    }
+    return (Triplet<uint32_t>());
+}
+
 //**************************** D2ClientConfigParser **********************
 D2ClientConfigParser::D2ClientConfigParser(const std::string& entry_name)
     : entry_name_(entry_name), boolean_values_(new BooleanStorage()),

src/lib/dhcpsrv/dhcp_parsers.h

@@ -993,6 +993,18 @@ protected:
     /// @throw DhcpConfigError when requested parameter is not present
     isc::dhcp::Triplet<uint32_t> 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<uint32_t> getOptionalParam(const std::string& name);
+
 private:
 
     /// @brief Append sub-options to an option.