[2596] Changes after review.

src/bin/dhcp6/config_parser.cc

@@ -18,6 +18,7 @@
 #include <dhcp/libdhcp++.h>
 #include <dhcp6/config_parser.h>
 #include <dhcp6/dhcp6_log.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/dhcp_config_parser.h>
 #include <dhcpsrv/pool.h>
@@ -1139,7 +1140,7 @@ private:
         // directly over specified network interface.
 
         string iface;
-        StringStorage::iterator iface_iter = string_values_.find("interface");
+        StringStorage::const_iterator iface_iter = string_values_.find("interface");
         if (iface_iter != string_values_.end()) {
             iface = iface_iter->second;
         }
@@ -1161,7 +1162,13 @@ private:
         }
 
         // Configure interface, if defined
-        if (iface.length()) {
+        if (!iface.empty()) {
+            if (!IfaceMgr::instance().getIface(iface)) {
+                isc_throw(Dhcp6ConfigError, "Specified interface name " << iface
+                          << " for subnet " << subnet_->toText() << " is not present"
+                          << " in the system.");
+            }
+
             subnet_->setIface(iface);
         }
 

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -17,6 +17,7 @@
 #include <config/ccsession.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option6_ia.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcp6/config_parser.h>
 #include <dhcp6/dhcp6_srv.h>
 #include <dhcpsrv/cfgmgr.h>
@@ -46,6 +47,16 @@ public:
         // srv_(0) means to not open any sockets. We don't want to
         // deal with sockets here, just check if configuration handling
         // is sane.
+
+        const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
+
+        // There must be some interface detected
+        if (ifaces.empty()) {
+            ADD_FAILURE() << "No interfaces detected.";
+        }
+
+        valid_iface_ = ifaces.begin()->getName();
+        bogus_iface_ = "nonexisting0";
     }
 
     ~Dhcp6ParserTest() {
@@ -239,6 +250,9 @@ public:
 
     int rcode_;
     ConstElementPtr comment_;
+
+    string valid_iface_;
+    string bogus_iface_;
 };
 
 // Goal of this test is a verification if a very simple config update
@@ -370,13 +384,15 @@ TEST_F(Dhcp6ParserTest, subnetInterface) {
 
     ConstElementPtr status;
 
+    // There should be at least one interface
+
     string config = "{ \"interface\": [ \"all\" ],"
         "\"preferred-lifetime\": 3000,"
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"subnet6\": [ { "
         "    \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ],"
-        "    \"interface\": \"eth0\","
+        "    \"interface\": \"" + valid_iface_ + "\","
         "    \"subnet\": \"2001:db8:1::/64\" } ],"
         "\"valid-lifetime\": 4000 }";
     cout << config << endl;
@@ -392,9 +408,42 @@ TEST_F(Dhcp6ParserTest, subnetInterface) {
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
     ASSERT_TRUE(subnet);
-    EXPECT_EQ("eth0", subnet->getIface());
+    EXPECT_EQ(valid_iface_, subnet->getIface());
+}
+
+// This test checks if invalid interface name will be rejected in
+// Subnet6 definition.
+TEST_F(Dhcp6ParserTest, subnetInterfaceBogus) {
+
+    ConstElementPtr status;
+
+    // There should be at least one interface
+
+    string config = "{ \"interface\": [ \"all\" ],"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ],"
+        "    \"interface\": \"" + bogus_iface_ + "\","
+        "    \"subnet\": \"2001:db8:1::/64\" } ],"
+        "\"valid-lifetime\": 4000 }";
+    cout << config << endl;
+
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+
+    // returned value should be 1 (configuration error)
+    ASSERT_TRUE(status);
+    comment_ = parseAnswer(rcode_, status);
+    EXPECT_EQ(1, rcode_);
+
+    Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
+    EXPECT_FALSE(subnet);
 }
 
+
 // This test checks if it is not allowed to define global interface
 // parameter.
 TEST_F(Dhcp6ParserTest, interfaceGlobal) {
@@ -405,7 +454,7 @@ TEST_F(Dhcp6ParserTest, interfaceGlobal) {
         "\"preferred-lifetime\": 3000,"
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
-        "\"interface\": \"eth0\"," // Not valid. Can be defined in subnet only
+        "\"interface\": \"" + valid_iface_ + "\"," // Not valid. Can be defined in subnet only
         "\"subnet6\": [ { "
         "    \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ],"
         "    \"subnet\": \"2001:db8:1::/64\" } ],"

src/lib/dhcpsrv/subnet.cc

@@ -207,7 +207,7 @@ void Subnet6::setIface(const std::string& iface_name) {
     iface_ = iface_name;
 }
 
-std::string Subnet6::getIface() {
+std::string Subnet6::getIface() const {
     return (iface_);
 }
 

src/lib/dhcpsrv/subnet.h

@@ -504,11 +504,12 @@ public:
     /// A subnet may be reachable directly (not via relays). In DHCPv6 it is not
     /// possible to decide that based on addresses assigned to network interfaces,
     /// as DHCPv6 operates on link-local (and site local) addresses.
+    /// @param iface_name name of the interface
     void setIface(const std::string& iface_name);
 
     /// @brief network interface name used to reach subnet (or "" for remote subnets)
     /// @return network interface name for directly attached subnets or ""
-    std::string getIface();
+    std::string getIface() const;
 
 protected:
 

src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -510,7 +510,7 @@ TEST(Subnet6Test, get) {
 TEST(Subnet6Test, iface) {
     Subnet6 subnet(IOAddress("2001:db8::"), 32, 1, 2, 3, 4);
 
-    EXPECT_EQ("", subnet.getIface());
+    EXPECT_TRUE(subnet.getIface().empty());
 
     subnet.setIface("en1");
     EXPECT_EQ("en1", subnet.getIface());