Browse Source

[1555] Implemented configuration parameter to select interfaces for DHCPv4

Marcin Siodelski 11 years ago
parent
commit
ffdc326d41

+ 11 - 1
src/bin/dhcp4/config_parser.cc

@@ -347,7 +347,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) {
         (config_id.compare("rebind-timer") == 0))  {
         (config_id.compare("rebind-timer") == 0))  {
         parser = new Uint32Parser(config_id, 
         parser = new Uint32Parser(config_id, 
                                  globalContext()->uint32_values_);
                                  globalContext()->uint32_values_);
-    } else if (config_id.compare("interface") == 0) {
+    } else if (config_id.compare("interfaces") == 0) {
         parser = new InterfaceListConfigParser(config_id);
         parser = new InterfaceListConfigParser(config_id);
     } else if (config_id.compare("subnet4") == 0) {
     } else if (config_id.compare("subnet4") == 0) {
         parser = new Subnets4ListConfigParser(config_id);
         parser = new Subnets4ListConfigParser(config_id);
@@ -397,6 +397,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     ParserCollection independent_parsers;
     ParserCollection independent_parsers;
     ParserPtr subnet_parser;
     ParserPtr subnet_parser;
     ParserPtr option_parser;
     ParserPtr option_parser;
+    ParserPtr iface_parser;
 
 
     // The subnet parsers implement data inheritance by directly
     // The subnet parsers implement data inheritance by directly
     // accessing global storage. For this reason the global data
     // accessing global storage. For this reason the global data
@@ -428,6 +429,11 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
                 subnet_parser = parser;
                 subnet_parser = parser;
             } else if (config_pair.first == "option-data") {
             } else if (config_pair.first == "option-data") {
                 option_parser = parser;
                 option_parser = parser;
+            } else if (config_pair.first == "interfaces") {
+                // The interface parser is independent from any other
+                // parser and can be run here before any other parsers.
+                iface_parser = parser;
+                parser->build(config_pair.second);
             } else {
             } else {
                 // Those parsers should be started before other
                 // Those parsers should be started before other
                 // parsers so we can call build straight away.
                 // parsers so we can call build straight away.
@@ -483,6 +489,10 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
             if (subnet_parser) {
             if (subnet_parser) {
                 subnet_parser->commit();
                 subnet_parser->commit();
             }
             }
+
+            if (iface_parser) {
+                iface_parser->commit();
+            }
         }
         }
         catch (const isc::Exception& ex) {
         catch (const isc::Exception& ex) {
             LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what());
             LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what());

+ 1 - 1
src/bin/dhcp4/dhcp4.spec

@@ -3,7 +3,7 @@
     "module_name": "Dhcp4",
     "module_name": "Dhcp4",
     "module_description": "DHCPv4 server daemon",
     "module_description": "DHCPv4 server daemon",
     "config_data": [
     "config_data": [
-      { "item_name": "interface",
+      { "item_name": "interfaces",
         "item_type": "list",
         "item_type": "list",
         "item_optional": false,
         "item_optional": false,
         "item_default": [ "all" ],
         "item_default": [ "all" ],

+ 81 - 16
src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -51,6 +51,7 @@ public:
         // deal with sockets here, just check if configuration handling
         // deal with sockets here, just check if configuration handling
         // is sane.
         // is sane.
         srv_.reset(new Dhcpv4Srv(0));
         srv_.reset(new Dhcpv4Srv(0));
+        CfgMgr::instance().deleteActiveIfaces();
     }
     }
 
 
     // Checks if global parameter of name have expected_value
     // Checks if global parameter of name have expected_value
@@ -138,7 +139,7 @@ public:
     /// describing an option.
     /// describing an option.
     std::string createConfigWithOption(const std::map<std::string, std::string>& params) {
     std::string createConfigWithOption(const std::map<std::string, std::string>& params) {
         std::ostringstream stream;
         std::ostringstream stream;
-        stream << "{ \"interface\": [ \"all\" ],"
+        stream << "{ \"interfaces\": [ \"all\" ],"
             "\"rebind-timer\": 2000, "
             "\"rebind-timer\": 2000, "
             "\"renew-timer\": 1000, "
             "\"renew-timer\": 1000, "
             "\"subnet4\": [ { "
             "\"subnet4\": [ { "
@@ -245,7 +246,7 @@ public:
     void resetConfiguration() {
     void resetConfiguration() {
         ConstElementPtr status;
         ConstElementPtr status;
 
 
-        string config = "{ \"interface\": [ \"all\" ],"
+        string config = "{ \"interfaces\": [ \"all\" ],"
             "\"rebind-timer\": 2000, "
             "\"rebind-timer\": 2000, "
             "\"renew-timer\": 1000, "
             "\"renew-timer\": 1000, "
             "\"valid-lifetime\": 4000, "
             "\"valid-lifetime\": 4000, "
@@ -322,7 +323,7 @@ TEST_F(Dhcp4ParserTest, emptySubnet) {
     ConstElementPtr status;
     ConstElementPtr status;
 
 
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_,
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_,
-                    Element::fromJSON("{ \"interface\": [ \"all\" ],"
+                    Element::fromJSON("{ \"interfaces\": [ \"all\" ],"
                                       "\"rebind-timer\": 2000, "
                                       "\"rebind-timer\": 2000, "
                                       "\"renew-timer\": 1000, "
                                       "\"renew-timer\": 1000, "
                                       "\"subnet4\": [  ], "
                                       "\"subnet4\": [  ], "
@@ -342,7 +343,7 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
 
 
     ConstElementPtr status;
     ConstElementPtr status;
 
 
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000, "
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "\"subnet4\": [ { "
@@ -372,7 +373,7 @@ TEST_F(Dhcp4ParserTest, subnetLocal) {
 
 
     ConstElementPtr status;
     ConstElementPtr status;
 
 
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000, "
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "\"subnet4\": [ { "
@@ -403,7 +404,7 @@ TEST_F(Dhcp4ParserTest, poolOutOfSubnet) {
 
 
     ConstElementPtr status;
     ConstElementPtr status;
 
 
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000, "
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "\"subnet4\": [ { "
@@ -427,7 +428,7 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) {
 
 
     ConstElementPtr status;
     ConstElementPtr status;
 
 
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000, "
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "\"subnet4\": [ { "
@@ -949,7 +950,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) {
 // configuration does not include options configuration.
 // configuration does not include options configuration.
 TEST_F(Dhcp4ParserTest, optionDataDefaults) {
 TEST_F(Dhcp4ParserTest, optionDataDefaults) {
     ConstElementPtr x;
     ConstElementPtr x;
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1022,7 +1023,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) {
     // The definition is not required for the option that
     // The definition is not required for the option that
     // belongs to the 'dhcp4' option space as it is the
     // belongs to the 'dhcp4' option space as it is the
     // standard option.
     // standard option.
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1100,7 +1101,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
     // at the very end (when all other parameters are configured).
     // at the very end (when all other parameters are configured).
 
 
     // Starting stage 1. Configure sub-options and their definitions.
     // Starting stage 1. Configure sub-options and their definitions.
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1149,7 +1150,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
     // the configuration from the stage 2 is repeated because BIND
     // the configuration from the stage 2 is repeated because BIND
     // configuration manager sends whole configuration for the lists
     // configuration manager sends whole configuration for the lists
     // where at least one element is being modified or added.
     // where at least one element is being modified or added.
-    config = "{ \"interface\": [ \"all\" ],"
+    config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1245,7 +1246,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
 // option setting.
 // option setting.
 TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
 TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
     ConstElementPtr x;
     ConstElementPtr x;
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000, "
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"renew-timer\": 1000, "
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1317,7 +1318,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
 // for multiple subnets.
 // for multiple subnets.
 TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
 TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
     ConstElementPtr x;
     ConstElementPtr x;
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000, "
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ { "
         "\"subnet4\": [ { "
@@ -1597,7 +1598,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
     // In the first stahe we create definitions of suboptions
     // In the first stahe we create definitions of suboptions
     // that we will add to the base option.
     // that we will add to the base option.
     // Let's create some dummy options: foo and foo2.
     // Let's create some dummy options: foo and foo2.
-    string config = "{ \"interface\": [ \"all\" ],"
+    string config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1650,7 +1651,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
     // We add our dummy options to this option space and thus
     // We add our dummy options to this option space and thus
     // they should be included as sub-options in the 'vendor-opts'
     // they should be included as sub-options in the 'vendor-opts'
     // option.
     // option.
-    config = "{ \"interface\": [ \"all\" ],"
+    config = "{ \"interfaces\": [ \"all\" ],"
         "\"rebind-timer\": 2000,"
         "\"rebind-timer\": 2000,"
         "\"renew-timer\": 1000,"
         "\"renew-timer\": 1000,"
         "\"option-data\": [ {"
         "\"option-data\": [ {"
@@ -1749,5 +1750,69 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
     EXPECT_FALSE(desc.option->getOption(3));
     EXPECT_FALSE(desc.option->getOption(3));
 }
 }
 
 
+// This test verifies that it is possible to select subset of interfaces
+// on which server should listen.
+TEST_F(Dhcp4ParserTest, selectedInterfaces) {
+    ConstElementPtr x;
+    string config = "{ \"interfaces\": [ \"eth0\", \"eth1\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"valid-lifetime\": 4000 }";
 
 
-};
+    ElementPtr json = Element::fromJSON(config);
+
+    ConstElementPtr status;
+
+    // Make sure the config manager is clean and there is no hanging
+    // interface configuration.
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth0"));
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth1"));
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth2"));
+
+    // Apply configuration.
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // eth0 and eth1 were explicitly selected. eth2 was not.
+    EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth0"));
+    EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth1"));
+    EXPECT_FALSE(CfgMgr::instance().isActiveIface("eth2"));
+}
+
+// This test verifies that it is possible to configure the server in such a way
+// that it listens on all interfaces.
+TEST_F(Dhcp4ParserTest, allInterfaces) {
+    ConstElementPtr x;
+    // This configuration specifies two interfaces on which server should listen
+    // but it also includes keyword 'all'. This keyword switches server into the
+    // mode when it listens on all interfaces regardless of what interface names
+    // were specified in the "interfaces" parameter.
+    string config = "{ \"interfaces\": [ \"eth0\",\"all\",\"eth1\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"valid-lifetime\": 4000 }";
+
+    ElementPtr json = Element::fromJSON(config);
+
+    ConstElementPtr status;
+
+    // Make sure there is no old configuration.
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth0"));
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth1"));
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth2"));
+
+    // Apply configuration.
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // All interfaces should be now active.
+    EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth0"));
+    EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth1"));
+    EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth2"));
+}
+
+
+
+}

+ 1 - 1
src/bin/dhcp6/dhcp6.spec

@@ -3,7 +3,7 @@
     "module_name": "Dhcp6",
     "module_name": "Dhcp6",
     "module_description": "DHCPv6 server daemon",
     "module_description": "DHCPv6 server daemon",
     "config_data": [
     "config_data": [
-      { "item_name": "interface",
+      { "item_name": "interfaces",
         "item_type": "list",
         "item_type": "list",
         "item_optional": false,
         "item_optional": false,
         "item_default": [ "all" ],
         "item_default": [ "all" ],

+ 53 - 1
src/lib/dhcpsrv/cfgmgr.cc

@@ -266,9 +266,61 @@ std::string CfgMgr::getDataDir() {
     return (datadir_);
     return (datadir_);
 }
 }
 
 
+void
+CfgMgr::addActiveIface(const std::string& iface) {
+    if (isIfaceListedActive(iface)) {
+        isc_throw(DuplicateListeningIface,
+                  "attempt to add duplicate interface '" << iface << "'"
+                  " to the set of interfaces on which server listens");
+    }
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_ADD_IFACE)
+        .arg(iface);
+    active_ifaces_.push_back(iface);
+}
+
+void
+CfgMgr::activateAllIfaces() {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
+              DHCPSRV_CFGMGR_ALL_IFACES_ACTIVE);
+    all_ifaces_active_ = true;
+}
+
+void
+CfgMgr::deleteActiveIfaces() {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
+              DHCPSRV_CFGMGR_CLEAR_ACTIVE_IFACES);
+    active_ifaces_.clear();
+    all_ifaces_active_ = false;
+}
+
+bool
+CfgMgr::isActiveIface(const std::string& iface) const {
+
+    // @todo Verify that the interface with the specified name is
+    // present in the system.
+
+    // If all interfaces are marked active, there is no need to check that
+    // the name of this interface has been explicitly listed.
+    if (all_ifaces_active_) {
+        return (true);
+    }
+    return (isIfaceListedActive(iface));
+}
+
+bool
+CfgMgr::isIfaceListedActive(const std::string& iface) const {
+    for (ActiveIfacesCollection::const_iterator it = active_ifaces_.begin();
+         it != active_ifaces_.end(); ++it) {
+        if (iface == *it) {
+            return (true);
+        }
+    }
+    return (false);
+}
 
 
 CfgMgr::CfgMgr()
 CfgMgr::CfgMgr()
-    :datadir_(DHCP_DATA_DIR) {
+    : datadir_(DHCP_DATA_DIR),
+      all_ifaces_active_(false) {
     // DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
     // DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
     // Note: the definition of DHCP_DATA_DIR needs to include quotation marks
     // Note: the definition of DHCP_DATA_DIR needs to include quotation marks
     // See AM_CPPFLAGS definition in Makefile.am
     // See AM_CPPFLAGS definition in Makefile.am

+ 65 - 0
src/lib/dhcpsrv/cfgmgr.h

@@ -30,10 +30,21 @@
 #include <map>
 #include <map>
 #include <string>
 #include <string>
 #include <vector>
 #include <vector>
+#include <list>
 
 
 namespace isc {
 namespace isc {
 namespace dhcp {
 namespace dhcp {
 
 
+/// @brief Exception thrown when the same interface has been specified twice.
+///
+/// In particular, this exception is thrown when adding interface to the set
+/// of interfaces on which server is supposed to listen.
+class DuplicateListeningIface : public Exception {
+public:
+    DuplicateListeningIface(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 
 
 /// @brief Configuration Manager
 /// @brief Configuration Manager
 ///
 ///
@@ -237,6 +248,36 @@ public:
     /// @return data directory
     /// @return data directory
     std::string getDataDir();
     std::string getDataDir();
 
 
+    /// @brief Adds the name of the interface to the set of interfaces on which
+    /// server should listen.
+    ///
+    /// @param iface A name of the interface being added to the listening set.
+    void addActiveIface(const std::string& iface);
+
+    /// @brief Configures the server to listen on all interfaces.
+    ///
+    /// This function configrues the server to listen on all available
+    /// interfaces regardless of the interfaces specified with
+    /// @c CfgMgr::addListeningInterface.
+    void activateAllIfaces();
+
+    /// @brief Clear the collection of the interfaces that server is configured
+    /// to use to listen for incoming requests.
+    ///
+    /// Apart from clearing the list of interfaces specified with
+    /// @c CfgMgr::addListeningInterface, it also disables listening on all
+    /// interfaces if it has been enabled using @c CfgMgr::listenAllInterfaces.
+    void deleteActiveIfaces();
+
+    /// @brief Check if server is configured to listen on the interface which
+    /// name is specified as an argument to this function.
+    ///
+    /// @param iface A name of the interface to be checked.
+    ///
+    /// @return true if the specified interface belongs to the set of the
+    /// interfaces on which server is configured to listen.
+    bool isActiveIface(const std::string& iface) const;
+
 protected:
 protected:
 
 
     /// @brief Protected constructor.
     /// @brief Protected constructor.
@@ -268,6 +309,20 @@ protected:
 
 
 private:
 private:
 
 
+    /// @brief Checks if the specified interface is listed as active.
+    ///
+    /// This function searches for the specified interface name on the list of
+    /// active interfaces: @c CfgMgr::active_ifaces_. It does not take into
+    /// account @c CfgMgr::all_ifaces_active_ flag. If this flag is set to true
+    /// but the specified interface does not belong to
+    /// @c CfgMgr::active_ifaces_, it will return false.
+    ///
+    /// @param iface interface name.
+    ///
+    /// @return true if specified interface belongs to
+    /// @c CfgMgr::active_ifaces_.
+    bool isIfaceListedActive(const std::string& iface) const;
+
     /// @brief A collection of option definitions.
     /// @brief A collection of option definitions.
     ///
     ///
     /// A collection of option definitions that can be accessed
     /// A collection of option definitions that can be accessed
@@ -283,6 +338,16 @@ private:
 
 
     /// @brief directory where data files (e.g. server-id) are stored
     /// @brief directory where data files (e.g. server-id) are stored
     std::string datadir_;
     std::string datadir_;
+
+    /// @name A collection of interface names on which server listens.
+    //@{
+    typedef std::list<std::string> ActiveIfacesCollection;
+    std::list<std::string> active_ifaces_;
+    //@}
+
+    /// A flag which indicates that server should listen on all available
+    /// interfaces.
+    bool all_ifaces_active_;
 };
 };
 
 
 } // namespace isc::dhcp
 } // namespace isc::dhcp

+ 65 - 11
src/lib/dhcpsrv/dhcp_parsers.cc

@@ -33,6 +33,10 @@ using namespace isc::data;
 namespace isc {
 namespace isc {
 namespace dhcp {
 namespace dhcp {
 
 
+namespace {
+const char* ALL_IFACES_KEYWORD = "all";
+}
+
 // *********************** ParserContext  *************************
 // *********************** ParserContext  *************************
 
 
 ParserContext::ParserContext(Option::Universe universe):
 ParserContext::ParserContext(Option::Universe universe):
@@ -140,33 +144,83 @@ template <> void ValueParser<std::string>::build(ConstElementPtr value) {
 
 
 // ******************** InterfaceListConfigParser *************************
 // ******************** InterfaceListConfigParser *************************
 
 
-InterfaceListConfigParser::InterfaceListConfigParser(const std::string& 
+InterfaceListConfigParser::
-                                                     param_name) {
+InterfaceListConfigParser(const std::string& param_name)
-    if (param_name != "interface") {
+    : activate_all_(false),
+      param_name_(param_name) {
+    if (param_name_ != "interfaces") {
         isc_throw(BadValue, "Internal error. Interface configuration "
         isc_throw(BadValue, "Internal error. Interface configuration "
             "parser called for the wrong parameter: " << param_name);
             "parser called for the wrong parameter: " << param_name);
     }
     }
 }
 }
 
 
-void 
+void
 InterfaceListConfigParser::build(ConstElementPtr value) {
 InterfaceListConfigParser::build(ConstElementPtr value) {
+    // First, we iterate over all specified entries and add it to the
+    // local container so as we can do some basic validation, e.g. eliminate
+    // duplicates.
     BOOST_FOREACH(ConstElementPtr iface, value->listValue()) {
     BOOST_FOREACH(ConstElementPtr iface, value->listValue()) {
-        interfaces_.push_back(iface->str());
+        std::string iface_name = iface->stringValue();
+        if (iface_name != ALL_IFACES_KEYWORD) {
+            // Let's eliminate duplicates. We could possibly allow duplicates,
+            // but if someone specified duplicated interface name it is likely
+            // that he mistyped the configuration. Failing here should draw his
+            // attention.
+            if (isIfaceAdded(iface_name)) {
+                isc_throw(isc::dhcp::DhcpConfigError, "duplicate interface"
+                          << " name '" << iface_name << "' specified in '"
+                          << param_name_ << "' configuration parameter");
+            }
+            // @todo check that this interface exists in the system!
+            // The IfaceMgr exposes mechanisms to check this.
+
+            // Add the interface name if ok.
+            interfaces_.push_back(iface_name);
+
+        } else {
+            activate_all_ = true;
+
+        }
     }
     }
 }
 }
 
 
-void 
+void
 InterfaceListConfigParser::commit() {
 InterfaceListConfigParser::commit() {
-    /// @todo: Implement per interface listening. Currently always listening
+    CfgMgr& cfg_mgr = CfgMgr::instance();
-    /// on all interfaces.
+    // Remove active interfaces and clear a flag which marks all interfaces
+    // active
+    cfg_mgr.deleteActiveIfaces();
+
+    if (activate_all_) {
+        // Activate all interfaces. There is not need to add their names
+        // explicitly.
+        cfg_mgr.activateAllIfaces();
+
+    } else {
+        // Explicitly add names of the interfaces which server should listen on.
+        BOOST_FOREACH(std::string iface, interfaces_) {
+            cfg_mgr.addActiveIface(iface);
+        }
+    }
+}
+
+bool
+InterfaceListConfigParser::isIfaceAdded(const std::string& iface) const {
+    for (IfaceListStorage::const_iterator it = interfaces_.begin();
+         it != interfaces_.end(); ++it) {
+        if (iface == *it) {
+            return (true);
+        }
+    }
+    return (false);
 }
 }
 
 
 // **************************** OptionDataParser *************************
 // **************************** OptionDataParser *************************
 OptionDataParser::OptionDataParser(const std::string&, OptionStoragePtr options,
 OptionDataParser::OptionDataParser(const std::string&, OptionStoragePtr options,
                                   ParserContextPtr global_context)
                                   ParserContextPtr global_context)
-    : boolean_values_(new BooleanStorage()), 
+    : boolean_values_(new BooleanStorage()),
-    string_values_(new StringStorage()), uint32_values_(new Uint32Storage()), 
+    string_values_(new StringStorage()), uint32_values_(new Uint32Storage()),
-    options_(options), option_descriptor_(false), 
+    options_(options), option_descriptor_(false),
     global_context_(global_context) {
     global_context_(global_context) {
     if (!options_) {
     if (!options_) {
         isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:"
         isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:"

+ 16 - 1
src/lib/dhcpsrv/dhcp_parsers.h

@@ -302,8 +302,23 @@ public:
     virtual void commit();
     virtual void commit();
 
 
 private:
 private:
+    /// @brief Check that specified interface exists in
+    /// @c InterfaceListConfigParser::interfaces_.
+    ///
+    /// @param iface A name of the interface.
+    ///
+    /// @return true if specified interface name was found.
+    bool isIfaceAdded(const std::string& iface) const;
+
     /// contains list of network interfaces
     /// contains list of network interfaces
-    std::vector<std::string> interfaces_;
+    typedef std::list<std::string> IfaceListStorage;
+    IfaceListStorage interfaces_;
+
+    // Should server listen on all interfaces.
+    bool activate_all_;
+
+    // Parsed parameter name
+    std::string param_name_;
 };
 };
 
 
 
 

+ 14 - 0
src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -54,6 +54,10 @@ consider reducing the lease lifetime.  In this way, addresses allocated
 to clients that are no longer active on the network will become available
 to clients that are no longer active on the network will become available
 available sooner.
 available sooner.
 
 
+% DHCPSRV_CFGMGR_ADD_IFACE adding listening interface %1
+A debug message issued when new interface is being added to the collection of
+interfaces on which server listens to DHCP messages.
+
 % DHCPSRV_CFGMGR_ADD_SUBNET4 adding subnet %1
 % DHCPSRV_CFGMGR_ADD_SUBNET4 adding subnet %1
 A debug message reported when the DHCP configuration manager is adding the
 A debug message reported when the DHCP configuration manager is adding the
 specified IPv4 subnet to its database.
 specified IPv4 subnet to its database.
@@ -62,6 +66,16 @@ specified IPv4 subnet to its database.
 A debug message reported when the DHCP configuration manager is adding the
 A debug message reported when the DHCP configuration manager is adding the
 specified IPv6 subnet to its database.
 specified IPv6 subnet to its database.
 
 
+% DHCPSRV_CFGMGR_ALL_IFACES_ACTIVE enabling listening on all interfaces
+A debug message issued when server is being configured to listen on all
+interfaces.
+
+% DHCPSRV_CFGMGR_CLEAR_ACTIVE_IFACES stop listening on all interfaces
+A debug message issued when configuration manager clears the internal list
+of active interfaces. This doesn't prevent the server from listening to
+the DHCP traffic through open sockets, but will rather be used by Interface
+Manager to select active interfaces when sockets are re-opened.
+
 % DHCPSRV_CFGMGR_DELETE_SUBNET4 deleting all IPv4 subnets
 % DHCPSRV_CFGMGR_DELETE_SUBNET4 deleting all IPv4 subnets
 A debug message noting that the DHCP configuration manager has deleted all IPv4
 A debug message noting that the DHCP configuration manager has deleted all IPv4
 subnets in its database.
 subnets in its database.

+ 45 - 0
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -165,6 +165,7 @@ public:
         CfgMgr::instance().deleteSubnets4();
         CfgMgr::instance().deleteSubnets4();
         CfgMgr::instance().deleteSubnets6();
         CfgMgr::instance().deleteSubnets6();
         CfgMgr::instance().deleteOptionDefs();
         CfgMgr::instance().deleteOptionDefs();
+        CfgMgr::instance().deleteActiveIfaces();
     }
     }
 
 
     /// @brief generates interface-id option based on provided text
     /// @brief generates interface-id option based on provided text
@@ -573,6 +574,50 @@ TEST_F(CfgMgrTest, optionSpace6) {
     // @todo decide if a duplicate vendor space is allowed.
     // @todo decide if a duplicate vendor space is allowed.
 }
 }
 
 
+// This test verifies that it is possible to specify interfaces that server
+// should listen on.
+TEST_F(CfgMgrTest, addActiveIface) {
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+
+    cfg_mgr.addActiveIface("eth0");
+    cfg_mgr.addActiveIface("eth1");
+
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth0"));
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth1"));
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth2"));
+
+    cfg_mgr.deleteActiveIfaces();
+
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth0"));
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth1"));
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth2"));
+}
+
+// This test verifies that it is possible to set the flag which configures the
+// server to listen on all interfaces.
+TEST_F(CfgMgrTest, activateAllIfaces) {
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+
+    cfg_mgr.addActiveIface("eth0");
+    cfg_mgr.addActiveIface("eth1");
+
+    ASSERT_TRUE(cfg_mgr.isActiveIface("eth0"));
+    ASSERT_TRUE(cfg_mgr.isActiveIface("eth1"));
+    ASSERT_FALSE(cfg_mgr.isActiveIface("eth2"));
+
+    cfg_mgr.activateAllIfaces();
+
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth0"));
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth1"));
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth2"));
+
+    cfg_mgr.deleteActiveIfaces();
+
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth0"));
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth1"));
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth2"));
+}
+
 // No specific tests for getSubnet6. That method (2 overloaded versions) is tested
 // No specific tests for getSubnet6. That method (2 overloaded versions) is tested
 // in Dhcpv6SrvTest.selectSubnetAddr and Dhcpv6SrvTest.selectSubnetIface
 // in Dhcpv6SrvTest.selectSubnetAddr and Dhcpv6SrvTest.selectSubnetIface
 // (see src/bin/dhcp6/tests/dhcp6_srv_unittest.cc)
 // (see src/bin/dhcp6/tests/dhcp6_srv_unittest.cc)

+ 39 - 7
src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -42,6 +42,7 @@ public:
     /// @brief Constructor
     /// @brief Constructor
     ///
     ///
     DhcpParserTest() {
     DhcpParserTest() {
+        CfgMgr::instance().deleteActiveIfaces();
     }
     }
 };
 };
 
 
@@ -197,25 +198,56 @@ TEST_F(DhcpParserTest, uint32ParserTest) {
 ///
 ///
 /// Verifies that the parser:
 /// Verifies that the parser:
 /// 1. Does not allow empty for storage.
 /// 1. Does not allow empty for storage.
-/// 2. Does not allow name other than "interface"
+/// 2. Does not allow name other than "interfaces"
-///
+/// 3. Parses list of interfaces and adds them to CfgMgr
-/// InterfaceListParser doesn't do very much, this test will need to 
+/// 4. Parses 'all' keyword and sets a CfgMgr flag which indicates that
-/// expand once it does.
+///    server will listen on all interfaces.
 TEST_F(DhcpParserTest, interfaceListParserTest) {
 TEST_F(DhcpParserTest, interfaceListParserTest) {
 
 
-    const std::string name = "interface";
+    const std::string name = "interfaces";
 
 
     // Verify that parser constructor fails if parameter name isn't "interface"
     // Verify that parser constructor fails if parameter name isn't "interface"
     EXPECT_THROW(InterfaceListConfigParser("bogus_name"), isc::BadValue);
     EXPECT_THROW(InterfaceListConfigParser("bogus_name"), isc::BadValue);
 
 
-    InterfaceListConfigParser parser(name);
+    boost::scoped_ptr<InterfaceListConfigParser>
+        parser(new InterfaceListConfigParser(name));
     ElementPtr list_element = Element::createList();
     ElementPtr list_element = Element::createList();
     list_element->add(Element::create("eth0"));
     list_element->add(Element::create("eth0"));
     list_element->add(Element::create("eth1"));
     list_element->add(Element::create("eth1"));
+
+    // Make sure there are no interfaces added yet.
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth0"));
+    ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth1"));
+
+    // This should parse the configuration and add eth0 and eth1 to the list
+    // of interfaces that server should listen on.
+    parser->build(list_element);
+    parser->commit();
+
+    // Use CfgMgr instance to check if eth0 and eth1 was added, and that
+    // eth2 was not added.
+    CfgMgr& cfg_mgr = CfgMgr::instance();
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth0"));
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth1"));
+    EXPECT_FALSE(cfg_mgr.isActiveIface("eth2"));
+
+    // Add keyword all to the configuration. This should activate all
+    // interfaces, including eth2, even though it has not been explicitly
+    // added.
+    list_element->add(Element::create("all"));
+
+    // Reset parser's state.
+    parser.reset(new InterfaceListConfigParser(name));
+    parser->build(list_element);
+    parser->commit();
+
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth0"));
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth1"));
+    EXPECT_TRUE(cfg_mgr.isActiveIface("eth2"));
 }
 }
 
 
 /// @brief Test Implementation of abstract OptionDataParser class. Allows 
 /// @brief Test Implementation of abstract OptionDataParser class. Allows 
-/// testing basic option parsing.   
+/// testing basic option parsing.
 class UtestOptionDataParser : public OptionDataParser {
 class UtestOptionDataParser : public OptionDataParser {
 public:
 public: