[3191] Parser is now able to parse next-server parameter

src/bin/dhcp4/config_parser.cc

@@ -192,7 +192,8 @@ protected:
             (config_id.compare("rebind-timer") == 0))  {
             parser = new Uint32Parser(config_id, uint32_values_);
         } else if ((config_id.compare("subnet") == 0) ||
-                   (config_id.compare("interface") == 0)) {
+                   (config_id.compare("interface") == 0) ||
+                   (config_id.compare("next-server") == 0)) {
             parser = new StringParser(config_id, string_values_);
         } else if (config_id.compare("pool") == 0) {
             parser = new Pool4Parser(config_id, pools_);
@@ -257,14 +258,30 @@ protected:
         Triplet<uint32_t> t2 = getParam("rebind-timer");
         Triplet<uint32_t> valid = getParam("valid-lifetime");
 
-        /// @todo: Convert this to logger once the parser is working reliably
         stringstream tmp;
         tmp << addr.toText() << "/" << (int)len
             << " with params t1=" << t1 << ", t2=" << t2 << ", valid=" << valid;
 
         LOG_INFO(dhcp4_logger, DHCP4_CONFIG_NEW_SUBNET).arg(tmp.str());
 
-        subnet_.reset(new Subnet4(addr, len, t1, t2, valid));
+        Subnet4* subnet4 = new Subnet4(addr, len, t1, t2, valid);
+        subnet_.reset(subnet4);
+
+        // Try global value first
+        try {
+            string next_server = globalContext()->string_values_->getParam("next-server");
+            subnet4->setSiaddr(IOAddress(next_server));
+        } catch (const DhcpConfigError&) {
+            // don't care. next_server is optional. We can live without it
+        }
+
+        // Try subnet specific value if it's available
+        try {
+            string next_server = string_values_->getParam("next-server");
+            subnet4->setSiaddr(IOAddress(next_server));
+        } catch (const DhcpConfigError&) {
+            // don't care. next_server is optional. We can live without it
+        }
     }
 };
 
@@ -359,7 +376,8 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) {
     } else if (config_id.compare("option-def") == 0) {
         parser  = new OptionDefListParser(config_id,
                                           globalContext()->option_defs_);
-    } else if (config_id.compare("version") == 0) {
+    } else if ((config_id.compare("version") == 0) ||
+               (config_id.compare("next-server") == 0)) {
         parser  = new StringParser(config_id,
                                     globalContext()->string_values_);
     } else if (config_id.compare("lease-database") == 0) {

src/bin/dhcp4/dhcp4.spec

@@ -16,7 +16,7 @@
           "item_default": ""
         }
       },
- 
+
       { "item_name": "interfaces",
         "item_type": "list",
         "item_optional": false,
@@ -48,6 +48,12 @@
         "item_default": 4000
       },
 
+      { "item_name": "next-server",
+        "item_type": "string",
+        "item_optional": true,
+        "item_default": "0.0.0.0"
+      },
+
       { "item_name": "option-def",
         "item_type": "list",
         "item_optional": false,
@@ -218,6 +224,13 @@
                   "item_optional": false,
                   "item_default": 7200
                 },
+
+                { "item_name": "next-server",
+                    "item_type": "string",
+                    "item_optional": true,
+                    "item_default": "0.0.0.0"
+                },
+
                 { "item_name": "pool",
                   "item_type": "list",
                   "item_optional": false,
@@ -290,7 +303,7 @@
 
         {
             "command_name": "libreload",
-            "command_description": "Reloads the current hooks libraries.", 
+            "command_description": "Reloads the current hooks libraries.",
             "command_args": []
         }
 

src/bin/dhcp4/dhcp4_srv.cc

@@ -701,6 +701,13 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         return;
     }
 
+    // Set up siaddr. Perhaps assignLease is not the best place to call this
+    // as siaddr has nothing to do with a lease, but otherwise we would have
+    // to select subnet twice (performance hit) or update too many functions
+    // at once.
+    // @todo: move subnet selection to a common code
+    answer->setSiaddr(subnet->getSiaddr());
+
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED)
         .arg(subnet->toText());
 

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -411,6 +411,94 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
     EXPECT_EQ(4000, subnet->getValid());
 }
 
+// Checks if the next-server defined as global parameter is taken into
+// consideration.
+TEST_F(Dhcp4ParserTest, nextServerGlobal) {
+
+    ConstElementPtr status;
+
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"next-server\": \"1.2.3.4\", "
+        "\"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));
+
+    // check if returned status is OK
+    checkResult(status, 0);
+
+    // Now check if the configuration was indeed handled and we have
+    // expected pool configured.
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
+}
+
+// Checks if the next-server defined as subnet parameter is taken into
+// consideration.
+TEST_F(Dhcp4ParserTest, nextServerSubnet) {
+
+    ConstElementPtr status;
+
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"next-server\": \"1.2.3.4\", "
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+
+    // check if returned status is OK
+    checkResult(status, 0);
+
+    // Now check if the configuration was indeed handled and we have
+    // expected pool configured.
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
+}
+
+// Checks if the next-server defined as global value is overridden by subnet
+// specific value.
+TEST_F(Dhcp4ParserTest, nextServerOverride) {
+
+    ConstElementPtr status;
+
+    string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"next-server\": \"192.0.0.1\", "
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"next-server\": \"1.2.3.4\", "
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+
+    // check if returned status is OK
+    checkResult(status, 0);
+
+    // Now check if the configuration was indeed handled and we have
+    // expected pool configured.
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ("1.2.3.4", subnet->getSiaddr().toText());
+}
+
 // This test checks if it is possible to override global values
 // on a per subnet basis.
 TEST_F(Dhcp4ParserTest, subnetLocal) {