From ffdc326d413b2eb9d95dc73f3948788ca7ece728 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 10 Jul 2013 21:14:03 +0200 Subject: [PATCH 01/17] [1555] Implemented configuration parameter to select interfaces for DHCPv4 --- src/bin/dhcp4/config_parser.cc | 12 ++- src/bin/dhcp4/dhcp4.spec | 2 +- src/bin/dhcp4/tests/config_parser_unittest.cc | 97 ++++++++++++++++--- src/bin/dhcp6/dhcp6.spec | 2 +- src/lib/dhcpsrv/cfgmgr.cc | 54 ++++++++++- src/lib/dhcpsrv/cfgmgr.h | 65 +++++++++++++ src/lib/dhcpsrv/dhcp_parsers.cc | 76 ++++++++++++--- src/lib/dhcpsrv/dhcp_parsers.h | 17 +++- src/lib/dhcpsrv/dhcpsrv_messages.mes | 14 +++ src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 45 +++++++++ .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 46 +++++++-- 11 files changed, 391 insertions(+), 39 deletions(-) diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index f30ff21e76..57ebf58cb8 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -347,7 +347,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) { (config_id.compare("rebind-timer") == 0)) { parser = new Uint32Parser(config_id, globalContext()->uint32_values_); - } else if (config_id.compare("interface") == 0) { + } else if (config_id.compare("interfaces") == 0) { parser = new InterfaceListConfigParser(config_id); } else if (config_id.compare("subnet4") == 0) { parser = new Subnets4ListConfigParser(config_id); @@ -397,6 +397,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { ParserCollection independent_parsers; ParserPtr subnet_parser; ParserPtr option_parser; + ParserPtr iface_parser; // The subnet parsers implement data inheritance by directly // accessing global storage. For this reason the global data @@ -428,6 +429,11 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { subnet_parser = parser; } else if (config_pair.first == "option-data") { 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 { // Those parsers should be started before other // parsers so we can call build straight away. @@ -483,6 +489,10 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { if (subnet_parser) { subnet_parser->commit(); } + + if (iface_parser) { + iface_parser->commit(); + } } catch (const isc::Exception& ex) { LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what()); diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 59d727ee64..063910c2e8 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/src/bin/dhcp4/dhcp4.spec @@ -3,7 +3,7 @@ "module_name": "Dhcp4", "module_description": "DHCPv4 server daemon", "config_data": [ - { "item_name": "interface", + { "item_name": "interfaces", "item_type": "list", "item_optional": false, "item_default": [ "all" ], diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 32770b3690..6aae9f2ad1 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -51,6 +51,7 @@ public: // deal with sockets here, just check if configuration handling // is sane. srv_.reset(new Dhcpv4Srv(0)); + CfgMgr::instance().deleteActiveIfaces(); } // Checks if global parameter of name have expected_value @@ -138,7 +139,7 @@ public: /// describing an option. std::string createConfigWithOption(const std::map& params) { std::ostringstream stream; - stream << "{ \"interface\": [ \"all\" ]," + stream << "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -245,7 +246,7 @@ public: void resetConfiguration() { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"valid-lifetime\": 4000, " @@ -322,7 +323,7 @@ TEST_F(Dhcp4ParserTest, emptySubnet) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, - Element::fromJSON("{ \"interface\": [ \"all\" ]," + Element::fromJSON("{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ ], " @@ -342,7 +343,7 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -372,7 +373,7 @@ TEST_F(Dhcp4ParserTest, subnetLocal) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -403,7 +404,7 @@ TEST_F(Dhcp4ParserTest, poolOutOfSubnet) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -427,7 +428,7 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -949,7 +950,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { // configuration does not include options configuration. TEST_F(Dhcp4ParserTest, optionDataDefaults) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1022,7 +1023,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { // The definition is not required for the option that // belongs to the 'dhcp4' option space as it is the // standard option. - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1100,7 +1101,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // at the very end (when all other parameters are configured). // Starting stage 1. Configure sub-options and their definitions. - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1149,7 +1150,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // the configuration from the stage 2 is repeated because BIND // configuration manager sends whole configuration for the lists // where at least one element is being modified or added. - config = "{ \"interface\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1245,7 +1246,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // option setting. TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"option-data\": [ {" @@ -1317,7 +1318,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { // for multiple subnets. TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -1597,7 +1598,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { // In the first stahe we create definitions of suboptions // that we will add to the base option. // Let's create some dummy options: foo and foo2. - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1650,7 +1651,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { // We add our dummy options to this option space and thus // they should be included as sub-options in the 'vendor-opts' // option. - config = "{ \"interface\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1749,5 +1750,69 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { 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")); +} + + + +} diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index bb5de0a086..39e054996d 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -3,7 +3,7 @@ "module_name": "Dhcp6", "module_description": "DHCPv6 server daemon", "config_data": [ - { "item_name": "interface", + { "item_name": "interfaces", "item_type": "list", "item_optional": false, "item_default": [ "all" ], diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 592efb7942..bb434a67e7 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -266,9 +266,61 @@ std::string CfgMgr::getDataDir() { 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() - :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 // Note: the definition of DHCP_DATA_DIR needs to include quotation marks // See AM_CPPFLAGS definition in Makefile.am diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 05c1752944..5f82aaefb7 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -30,10 +30,21 @@ #include #include #include +#include namespace isc { 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 /// @@ -237,6 +248,36 @@ public: /// @return data directory 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: /// @brief Protected constructor. @@ -268,6 +309,20 @@ protected: 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. /// /// 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 std::string datadir_; + + /// @name A collection of interface names on which server listens. + //@{ + typedef std::list ActiveIfacesCollection; + std::list active_ifaces_; + //@} + + /// A flag which indicates that server should listen on all available + /// interfaces. + bool all_ifaces_active_; }; } // namespace isc::dhcp diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index 4d1dc73086..e94b49caad 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -33,6 +33,10 @@ using namespace isc::data; namespace isc { namespace dhcp { +namespace { +const char* ALL_IFACES_KEYWORD = "all"; +} + // *********************** ParserContext ************************* ParserContext::ParserContext(Option::Universe universe): @@ -140,33 +144,83 @@ template <> void ValueParser::build(ConstElementPtr value) { // ******************** InterfaceListConfigParser ************************* -InterfaceListConfigParser::InterfaceListConfigParser(const std::string& - param_name) { - if (param_name != "interface") { +InterfaceListConfigParser:: +InterfaceListConfigParser(const std::string& param_name) + : activate_all_(false), + param_name_(param_name) { + if (param_name_ != "interfaces") { isc_throw(BadValue, "Internal error. Interface configuration " "parser called for the wrong parameter: " << param_name); } } -void +void 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()) { - 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() { - /// @todo: Implement per interface listening. Currently always listening - /// on all interfaces. + CfgMgr& cfg_mgr = CfgMgr::instance(); + // 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(const std::string&, OptionStoragePtr options, ParserContextPtr global_context) - : boolean_values_(new BooleanStorage()), - string_values_(new StringStorage()), uint32_values_(new Uint32Storage()), - options_(options), option_descriptor_(false), + : boolean_values_(new BooleanStorage()), + string_values_(new StringStorage()), uint32_values_(new Uint32Storage()), + options_(options), option_descriptor_(false), global_context_(global_context) { if (!options_) { isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:" diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h index e453204bd9..5551133c07 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.h +++ b/src/lib/dhcpsrv/dhcp_parsers.h @@ -302,8 +302,23 @@ public: virtual void commit(); 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 - std::vector interfaces_; + typedef std::list IfaceListStorage; + IfaceListStorage interfaces_; + + // Should server listen on all interfaces. + bool activate_all_; + + // Parsed parameter name + std::string param_name_; }; diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index b2a780706a..af8d78cc0d 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/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 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 A debug message reported when the DHCP configuration manager is adding the 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 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 A debug message noting that the DHCP configuration manager has deleted all IPv4 subnets in its database. diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 77c3e36775..38d2f0a525 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@ -165,6 +165,7 @@ public: CfgMgr::instance().deleteSubnets4(); CfgMgr::instance().deleteSubnets6(); CfgMgr::instance().deleteOptionDefs(); + CfgMgr::instance().deleteActiveIfaces(); } /// @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. } +// 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 // in Dhcpv6SrvTest.selectSubnetAddr and Dhcpv6SrvTest.selectSubnetIface // (see src/bin/dhcp6/tests/dhcp6_srv_unittest.cc) diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 687ef92b83..b4b9451856 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -42,6 +42,7 @@ public: /// @brief Constructor /// DhcpParserTest() { + CfgMgr::instance().deleteActiveIfaces(); } }; @@ -197,25 +198,56 @@ TEST_F(DhcpParserTest, uint32ParserTest) { /// /// Verifies that the parser: /// 1. Does not allow empty for storage. -/// 2. Does not allow name other than "interface" -/// -/// InterfaceListParser doesn't do very much, this test will need to -/// expand once it does. +/// 2. Does not allow name other than "interfaces" +/// 3. Parses list of interfaces and adds them to CfgMgr +/// 4. Parses 'all' keyword and sets a CfgMgr flag which indicates that +/// server will listen on all interfaces. 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" EXPECT_THROW(InterfaceListConfigParser("bogus_name"), isc::BadValue); - InterfaceListConfigParser parser(name); + boost::scoped_ptr + parser(new InterfaceListConfigParser(name)); ElementPtr list_element = Element::createList(); list_element->add(Element::create("eth0")); 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 -/// testing basic option parsing. +/// testing basic option parsing. class UtestOptionDataParser : public OptionDataParser { public: From 339d60a7b7a5b83a2b9c5064af5d621e3047582f Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 10:25:46 +0200 Subject: [PATCH 02/17] [1555] Implemented configuration parameter to select interfaces for DHCPv6. --- src/bin/dhcp6/config_parser.cc | 12 +- src/bin/dhcp6/tests/config_parser_unittest.cc | 124 ++++++++++++++---- src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 2 +- 3 files changed, 114 insertions(+), 24 deletions(-) diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index 74012accaa..a5ef1f8686 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -413,7 +413,7 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id) { (config_id.compare("rebind-timer") == 0)) { parser = new Uint32Parser(config_id, globalContext()->uint32_values_); - } else if (config_id.compare("interface") == 0) { + } else if (config_id.compare("interfaces") == 0) { parser = new InterfaceListConfigParser(config_id); } else if (config_id.compare("subnet6") == 0) { parser = new Subnets6ListConfigParser(config_id); @@ -463,6 +463,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { ParserCollection independent_parsers; ParserPtr subnet_parser; ParserPtr option_parser; + ParserPtr iface_parser; // The subnet parsers implement data inheritance by directly // accessing global storage. For this reason the global data @@ -495,6 +496,11 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { subnet_parser = parser; } else if (config_pair.first == "option-data") { option_parser = parser; + } else if (config_pair.first == "interfaces") { + // The interface parser is independent from any other parser and + // can be run here before other parsers. + parser->build(config_pair.second); + iface_parser = parser; } else { // Those parsers should be started before other // parsers so we can call build straight away. @@ -548,6 +554,10 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { if (subnet_parser) { subnet_parser->commit(); } + + if (iface_parser) { + iface_parser->commit(); + } } catch (const isc::Exception& ex) { LOG_ERROR(dhcp6_logger, DHCP6_PARSER_COMMIT_FAIL).arg(ex.what()); diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index d9c5859a64..2f96462460 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -66,11 +66,12 @@ public: << " while the test assumes that it doesn't, to execute" << " some negative scenarios. Can't continue this test."; } + + // Reset configuration for each test. + resetConfiguration(); } ~Dhcp6ParserTest() { - // Reset configuration database after each test. - resetConfiguration(); }; // Checks if config_result (result of DHCP server configuration) has @@ -133,7 +134,7 @@ public: std::string>& params) { std::ostringstream stream; - stream << "{ \"interface\": [ \"all\" ]," + stream << "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -173,13 +174,13 @@ public: /// /// This function resets configuration data base by /// removing all subnets and option-data. Reset must - /// be performed after each test to make sure that + /// be performed before each test to make sure that /// contents of the database do not affect result of - /// subsequent tests. + /// the test being executed. void resetConfiguration() { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -213,6 +214,12 @@ public: << " after the test. Configuration function returned" << " error code " << rcode_ << std::endl; } + + // The default setting is to listen on all interfaces. In order to + // properly test interface configuration we disable listening on + // all interfaces before each test and later check that this setting + // has been overriden by the configuration used in the test. + CfgMgr::instance().deleteActiveIfaces(); } /// @brief Test invalid option parameter value. @@ -324,7 +331,7 @@ TEST_F(Dhcp6ParserTest, emptySubnet) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, - Element::fromJSON("{ \"interface\": [ \"all\" ]," + Element::fromJSON("{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -343,7 +350,7 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -377,7 +384,7 @@ TEST_F(Dhcp6ParserTest, subnetLocal) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -415,7 +422,7 @@ TEST_F(Dhcp6ParserTest, subnetInterface) { // There should be at least one interface - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -448,7 +455,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceBogus) { // There should be at least one interface - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -479,7 +486,7 @@ TEST_F(Dhcp6ParserTest, interfaceGlobal) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -549,7 +556,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) { // parameter. TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { - const string config = "{ \"interface\": [ \"all\" ]," + const string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -604,7 +611,7 @@ TEST_F(Dhcp6ParserTest, poolOutOfSubnet) { ConstElementPtr status; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -632,7 +639,7 @@ TEST_F(Dhcp6ParserTest, poolPrefixLen) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1152,7 +1159,7 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { // configuration does not include options configuration. TEST_F(Dhcp6ParserTest, optionDataDefaults) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," @@ -1234,7 +1241,7 @@ TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) { // The definition is not required for the option that // belongs to the 'dhcp6' option space as it is the // standard option. - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1312,7 +1319,7 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // at the very end (when all other parameters are configured). // Starting stage 1. Configure sub-options and their definitions. - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1361,7 +1368,7 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // the configuration from the stage 2 is repeated because BIND // configuration manager sends whole configuration for the lists // where at least one element is being modified or added. - config = "{ \"interface\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1455,7 +1462,7 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // for multiple subnets. TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1698,7 +1705,7 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) { // In the first stahe we create definitions of suboptions // that we will add to the base option. // Let's create some dummy options: foo and foo2. - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1751,7 +1758,7 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) { // We add our dummy options to this option space and thus // they should be included as sub-options in the 'vendor-opts' // option. - config = "{ \"interface\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"all\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1850,4 +1857,77 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) { EXPECT_FALSE(desc.option->getOption(112)); } +// This test verifies that it is possible to select subset of interfaces on +// which server should listen. +TEST_F(Dhcp6ParserTest, selectedInterfaces) { + + // Make sure there is no garbage interface configuration in the CfgMgr. + ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth0")); + ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth1")); + ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth2")); + + ConstElementPtr status; + + string config = "{ \"interfaces\": [ \"eth0\", \"eth1\" ]," + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"valid-lifetime\": 4000 }"; + + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + + // returned value must be 1 (values error) + // as the pool does not belong to that subnet + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + EXPECT_EQ(0, rcode_); + + // 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 to listen on +// all interfaces. +TEST_F(Dhcp6ParserTest, allInterfaces) { + + // Make sure there is no garbage interface configuration in the CfgMgr. + ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth0")); + ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth1")); + ASSERT_FALSE(CfgMgr::instance().isActiveIface("eth2")); + + ConstElementPtr status; + + // This configuration specifies two interfaces on which server should listen + // bu 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\", \"eth1\", \"all\" ]," + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"valid-lifetime\": 4000 }"; + + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + + // returned value must be 1 (values error) + // as the pool does not belong to that subnet + ASSERT_TRUE(status); + comment_ = parseAnswer(rcode_, status); + EXPECT_EQ(0, rcode_); + + // All interfaces should be now active. + EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth0")); + EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth1")); + EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth2")); +} + + }; diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index 328a3c587b..af58f35c50 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -555,7 +555,7 @@ TEST_F(Dhcpv6SrvTest, DUID) { // and the requested options are actually assigned. TEST_F(Dhcpv6SrvTest, advertiseOptions) { ConstElementPtr x; - string config = "{ \"interface\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"all\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " From 42de7b42fd0fd6653489efe5412b34365d3a4a77 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 10:34:43 +0200 Subject: [PATCH 03/17] [1555] Trivial: whitespace cleanup. --- src/bin/dhcp4/config_parser.cc | 82 +++++------ src/bin/dhcp4/config_parser.h | 4 +- src/bin/dhcp4/tests/config_parser_unittest.cc | 2 +- src/bin/dhcp6/config_parser.cc | 80 +++++------ src/bin/dhcp6/config_parser.h | 6 +- src/lib/dhcpsrv/dhcp_parsers.cc | 130 +++++++++--------- src/lib/dhcpsrv/dhcp_parsers.h | 125 +++++++++-------- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 82 +++++------ 8 files changed, 255 insertions(+), 256 deletions(-) diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index 57ebf58cb8..726a4a69b3 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -53,10 +53,10 @@ public: /// @param dummy first param, option names are always "Dhcp4/option-data[n]" /// @param options is the option storage in which to store the parsed option /// upon "commit". - /// @param global_context is a pointer to the global context which + /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. - Dhcp4OptionDataParser(const std::string&, - OptionStoragePtr options, ParserContextPtr global_context) + Dhcp4OptionDataParser(const std::string&, + OptionStoragePtr options, ParserContextPtr global_context) :OptionDataParser("", options, global_context) { } @@ -64,7 +64,7 @@ public: /// /// @param param_name name of the parameter to be parsed. /// @param options storage where the parameter value is to be stored. - /// @param global_context is a pointer to the global context which + /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. /// @return returns a pointer to a new OptionDataParser. Caller is /// is responsible for deleting it when it is no longer needed. @@ -75,16 +75,16 @@ public: protected: /// @brief Finds an option definition within the server's option space - /// - /// Given an option space and an option code, find the correpsonding + /// + /// Given an option space and an option code, find the correpsonding /// option defintion within the server's option defintion storage. /// - /// @param option_space name of the parameter option space - /// @param option_code numeric value of the parameter to find - /// @return OptionDefintionPtr of the option defintion or an + /// @param option_space name of the parameter option space + /// @param option_code numeric value of the parameter to find + /// @return OptionDefintionPtr of the option defintion or an /// empty OptionDefinitionPtr if not found. - /// @throw DhcpConfigError if the option space requested is not valid - /// for this server. + /// @throw DhcpConfigError if the option space requested is not valid + /// for this server. virtual OptionDefinitionPtr findServerSpaceOptionDefinition ( std::string& option_space, uint32_t option_code) { OptionDefinitionPtr def; @@ -100,11 +100,11 @@ protected: } }; -/// @brief Parser for IPv4 pool definitions. +/// @brief Parser for IPv4 pool definitions. /// -/// This is the IPv4 derivation of the PoolParser class and handles pool -/// definitions, i.e. a list of entries of one of two syntaxes: min-max and -/// prefix/len for IPv4 pools. Pool4 objects are created and stored in chosen +/// This is the IPv4 derivation of the PoolParser class and handles pool +/// definitions, i.e. a list of entries of one of two syntaxes: min-max and +/// prefix/len for IPv4 pools. Pool4 objects are created and stored in chosen /// PoolStorage container. /// /// It is useful for parsing Dhcp4/subnet4[X]/pool parameters. @@ -126,9 +126,9 @@ protected: /// /// @param addr is the IPv4 prefix of the pool. /// @param len is the prefix length. - /// @param ignored dummy parameter to provide symmetry between the + /// @param ignored dummy parameter to provide symmetry between the /// PoolParser derivations. The V6 derivation requires a third value. - /// @return returns a PoolPtr to the new Pool4 object. + /// @return returns a PoolPtr to the new Pool4 object. PoolPtr poolMaker (IOAddress &addr, uint32_t len, int32_t) { return (PoolPtr(new Pool4(addr, len))); } @@ -137,9 +137,9 @@ protected: /// /// @param min is the first IPv4 address in the pool. /// @param max is the last IPv4 address in the pool. - /// @param ignored dummy parameter to provide symmetry between the + /// @param ignored dummy parameter to provide symmetry between the /// PoolParser derivations. The V6 derivation requires a third value. - /// @return returns a PoolPtr to the new Pool4 object. + /// @return returns a PoolPtr to the new Pool4 object. PoolPtr poolMaker (IOAddress &min, IOAddress &max, int32_t) { return (PoolPtr(new Pool4(min, max))); } @@ -147,8 +147,8 @@ protected: /// @brief This class parses a single IPv4 subnet. /// -/// This is the IPv4 derivation of the SubnetConfigParser class and it parses -/// the whole subnet definition. It creates parsersfor received configuration +/// This is the IPv4 derivation of the SubnetConfigParser class and it parses +/// the whole subnet definition. It creates parsersfor received configuration /// parameters as needed. class Subnet4ConfigParser : public SubnetConfigParser { public: @@ -158,7 +158,7 @@ public: /// stores global scope parameters, options, option defintions. Subnet4ConfigParser(const std::string&) :SubnetConfigParser("", globalContext()) { - } + } /// @brief Adds the created subnet to a server's configuration. /// @throw throws Unexpected if dynamic cast fails. @@ -167,7 +167,7 @@ public: Subnet4Ptr sub4ptr = boost::dynamic_pointer_cast(subnet_); if (!sub4ptr) { // If we hit this, it is a programming error. - isc_throw(Unexpected, + isc_throw(Unexpected, "Invalid cast in Subnet4ConfigParser::commit"); } @@ -191,13 +191,13 @@ protected: (config_id.compare("renew-timer") == 0) || (config_id.compare("rebind-timer") == 0)) { parser = new Uint32Parser(config_id, uint32_values_); - } else if ((config_id.compare("subnet") == 0) || + } else if ((config_id.compare("subnet") == 0) || (config_id.compare("interface") == 0)) { parser = new StringParser(config_id, string_values_); } else if (config_id.compare("pool") == 0) { parser = new Pool4Parser(config_id, pools_); } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, options_, + parser = new OptionDataListParser(config_id, options_, global_context_, Dhcp4OptionDataParser::factory); } else { @@ -210,7 +210,7 @@ protected: /// @brief Determines if the given option space name and code describe - /// a standard option for the DCHP4 server. + /// a standard option for the DCHP4 server. /// /// @param option_space is the name of the option space to consider /// @param code is the numeric option code to consider @@ -230,12 +230,12 @@ protected: } /// @brief Issues a DHCP4 server specific warning regarding duplicate subnet - /// options. - /// + /// options. + /// /// @param code is the numeric option code of the duplicate option - /// @param addr is the subnet address + /// @param addr is the subnet address /// @todo a means to know the correct logger and perhaps a common - /// message would allow this method to be emitted by the base class. + /// message would allow this method to be emitted by the base class. virtual void duplicate_option_warning(uint32_t code, isc::asiolink::IOAddress& addr) { LOG_WARN(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE) @@ -243,10 +243,10 @@ protected: } /// @brief Instantiates the IPv4 Subnet based on a given IPv4 address - /// and prefix length. - /// + /// and prefix length. + /// /// @param addr is IPv4 address of the subnet. - /// @param len is the prefix length + /// @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 @@ -338,32 +338,32 @@ namespace dhcp { /// /// @param config_id pointer to received global configuration entry /// @return parser for specified global DHCPv4 parameter -/// @throw NotImplemented if trying to create a parser for unknown +/// @throw NotImplemented if trying to create a parser for unknown /// config element DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id) { DhcpConfigParser* parser = NULL; if ((config_id.compare("valid-lifetime") == 0) || (config_id.compare("renew-timer") == 0) || (config_id.compare("rebind-timer") == 0)) { - parser = new Uint32Parser(config_id, + parser = new Uint32Parser(config_id, globalContext()->uint32_values_); } else if (config_id.compare("interfaces") == 0) { parser = new InterfaceListConfigParser(config_id); } else if (config_id.compare("subnet4") == 0) { parser = new Subnets4ListConfigParser(config_id); } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, - globalContext()->options_, + parser = new OptionDataListParser(config_id, + globalContext()->options_, globalContext(), Dhcp4OptionDataParser::factory); } else if (config_id.compare("option-def") == 0) { - parser = new OptionDefListParser(config_id, + parser = new OptionDefListParser(config_id, globalContext()->option_defs_); } else if (config_id.compare("version") == 0) { - parser = new StringParser(config_id, + parser = new StringParser(config_id, globalContext()->string_values_); } else if (config_id.compare("lease-database") == 0) { - parser = new DbAccessParser(config_id); + parser = new DbAccessParser(config_id); } else { isc_throw(NotImplemented, "Parser error: Global configuration parameter not supported: " @@ -384,7 +384,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { /// @todo: Append most essential info here (like "2 new subnets configured") string config_details; - LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_START).arg(config_set->str()); // Some of the values specified in the configuration depend on diff --git a/src/bin/dhcp4/config_parser.h b/src/bin/dhcp4/config_parser.h index ea84cc63d8..3af9911f5d 100644 --- a/src/bin/dhcp4/config_parser.h +++ b/src/bin/dhcp4/config_parser.h @@ -30,7 +30,7 @@ namespace dhcp { class Dhcpv4Srv; -/// @brief Configure DHCPv4 server (@c Dhcpv4Srv) with a set of configuration +/// @brief Configure DHCPv4 server (@c Dhcpv4Srv) with a set of configuration /// values. /// /// This function parses configuration information stored in @c config_set @@ -44,7 +44,7 @@ class Dhcpv4Srv; /// (such as malformed configuration or invalid configuration parameter), /// this function returns appropriate error code. /// -/// This function is called every time a new configuration is received. The +/// This function is called every time a new configuration is received. The /// extra parameter is a reference to DHCPv4 server component. It is currently /// not used and CfgMgr::instance() is accessed instead. /// diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 6aae9f2ad1..7c7caa278e 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -56,7 +56,7 @@ public: // Checks if global parameter of name have expected_value void checkGlobalUint32(string name, uint32_t expected_value) { - const Uint32StoragePtr uint32_defaults = + const Uint32StoragePtr uint32_defaults = globalContext()->uint32_values_; try { uint32_t actual_value = uint32_defaults->getParam(name); diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index a5ef1f8686..cc6bd028c9 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -67,10 +67,10 @@ public: /// @param dummy first param, option names are always "Dhcp6/option-data[n]" /// @param options is the option storage in which to store the parsed option /// upon "commit". - /// @param global_context is a pointer to the global context which + /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. - Dhcp6OptionDataParser(const std::string&, OptionStoragePtr options, - ParserContextPtr global_context) + Dhcp6OptionDataParser(const std::string&, OptionStoragePtr options, + ParserContextPtr global_context) :OptionDataParser("", options, global_context) { } @@ -78,7 +78,7 @@ public: /// /// @param param_name name of the parameter to be parsed. /// @param options storage where the parameter value is to be stored. - /// @param global_context is a pointer to the global context which + /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. /// @return returns a pointer to a new OptionDataParser. Caller is /// is responsible for deleting it when it is no longer needed. @@ -90,16 +90,16 @@ public: protected: /// @brief Finds an option definition within the server's option space - /// - /// Given an option space and an option code, find the correpsonding + /// + /// Given an option space and an option code, find the correpsonding /// option defintion within the server's option defintion storage. /// - /// @param option_space name of the parameter option space - /// @param option_code numeric value of the parameter to find - /// @return OptionDefintionPtr of the option defintion or an + /// @param option_space name of the parameter option space + /// @param option_code numeric value of the parameter to find + /// @return OptionDefintionPtr of the option defintion or an /// empty OptionDefinitionPtr if not found. - /// @throw DhcpConfigError if the option space requested is not valid - /// for this server. + /// @throw DhcpConfigError if the option space requested is not valid + /// for this server. virtual OptionDefinitionPtr findServerSpaceOptionDefinition ( std::string& option_space, uint32_t option_code) { OptionDefinitionPtr def; @@ -115,11 +115,11 @@ protected: } }; -/// @brief Parser for IPv4 pool definitions. +/// @brief Parser for IPv4 pool definitions. /// -/// This is the IPv6 derivation of the PoolParser class and handles pool -/// definitions, i.e. a list of entries of one of two syntaxes: min-max and -/// prefix/len for IPv6 pools. Pool6 objects are created and stored in chosen +/// This is the IPv6 derivation of the PoolParser class and handles pool +/// definitions, i.e. a list of entries of one of two syntaxes: min-max and +/// prefix/len for IPv6 pools. Pool6 objects are created and stored in chosen /// PoolStorage container. /// /// It is useful for parsing Dhcp6/subnet6[X]/pool parameters. @@ -142,9 +142,9 @@ protected: /// @param addr is the IPv6 prefix of the pool. /// @param len is the prefix length. /// @param ptype is the type of IPv6 pool (Pool6::Pool6Type). Note this is - /// passed in as an int32_t and cast to Pool6Type to accommodate a + /// passed in as an int32_t and cast to Pool6Type to accommodate a /// polymorphic interface. - /// @return returns a PoolPtr to the new Pool4 object. + /// @return returns a PoolPtr to the new Pool4 object. PoolPtr poolMaker (IOAddress &addr, uint32_t len, int32_t ptype) { return (PoolPtr(new Pool6(static_cast @@ -156,9 +156,9 @@ protected: /// @param min is the first IPv6 address in the pool. /// @param max is the last IPv6 address in the pool. /// @param ptype is the type of IPv6 pool (Pool6::Pool6Type). Note this is - /// passed in as an int32_t and cast to Pool6Type to accommodate a + /// passed in as an int32_t and cast to Pool6Type to accommodate a /// polymorphic interface. - /// @return returns a PoolPtr to the new Pool4 object. + /// @return returns a PoolPtr to the new Pool4 object. PoolPtr poolMaker (IOAddress &min, IOAddress &max, int32_t ptype) { return (PoolPtr(new Pool6(static_cast @@ -168,8 +168,8 @@ protected: /// @brief This class parses a single IPv6 subnet. /// -/// This is the IPv6 derivation of the SubnetConfigParser class and it parses -/// the whole subnet definition. It creates parsersfor received configuration +/// This is the IPv6 derivation of the SubnetConfigParser class and it parses +/// the whole subnet definition. It creates parsersfor received configuration /// parameters as needed. class Subnet6ConfigParser : public SubnetConfigParser { public: @@ -178,7 +178,7 @@ public: /// /// @param ignored first parameter /// stores global scope parameters, options, option defintions. - Subnet6ConfigParser(const std::string&) + Subnet6ConfigParser(const std::string&) :SubnetConfigParser("", globalContext()) { } @@ -220,7 +220,7 @@ protected: } else if (config_id.compare("pool") == 0) { parser = new Pool6Parser(config_id, pools_); } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, options_, + parser = new OptionDataListParser(config_id, options_, global_context_, Dhcp6OptionDataParser::factory); } else { @@ -233,14 +233,14 @@ protected: /// @brief Determines if the given option space name and code describe - /// a standard option for the DHCP6 server. + /// a standard option for the DHCP6 server. /// /// @param option_space is the name of the option space to consider /// @param code is the numeric option code to consider /// @return returns true if the space and code are part of the server's /// standard options. bool isServerStdOption(std::string option_space, uint32_t code) { - return ((option_space.compare("dhcp6") == 0) + return ((option_space.compare("dhcp6") == 0) && LibDHCP::isStandardOption(Option::V6, code)); } @@ -253,23 +253,23 @@ protected: } /// @brief Issues a DHCP6 server specific warning regarding duplicate subnet - /// options. - /// + /// options. + /// /// @param code is the numeric option code of the duplicate option /// @param addr is the subnet address /// @todo A means to know the correct logger and perhaps a common /// message would allow this message to be emitted by the base class. - virtual void duplicate_option_warning(uint32_t code, + virtual void duplicate_option_warning(uint32_t code, isc::asiolink::IOAddress& addr) { LOG_WARN(dhcp6_logger, DHCP6_CONFIG_OPTION_DUPLICATE) .arg(code).arg(addr.toText()); } /// @brief Instantiates the IPv6 Subnet based on a given IPv6 address - /// and prefix length. - /// + /// and prefix length. + /// /// @param addr is IPv6 prefix of the subnet. - /// @param len is the prefix length + /// @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 @@ -292,13 +292,13 @@ protected: // Specifying both interface for locally reachable subnets and // interface id for relays is mutually exclusive. Need to test for - // this condition. + // this condition. if (!ifaceid.empty()) { std::string iface; try { iface = string_values_->getParam("interface"); } catch (const DhcpConfigError &) { - // iface not mandatory + // iface not mandatory } if (!iface.empty()) { @@ -403,7 +403,7 @@ namespace dhcp { /// /// @param config_id pointer to received global configuration entry /// @return parser for specified global DHCPv6 parameter -/// @throw NotImplemented if trying to create a parser for unknown config +/// @throw NotImplemented if trying to create a parser for unknown config /// element DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id) { DhcpConfigParser* parser = NULL; @@ -411,22 +411,22 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id) { (config_id.compare("valid-lifetime") == 0) || (config_id.compare("renew-timer") == 0) || (config_id.compare("rebind-timer") == 0)) { - parser = new Uint32Parser(config_id, + parser = new Uint32Parser(config_id, globalContext()->uint32_values_); } else if (config_id.compare("interfaces") == 0) { parser = new InterfaceListConfigParser(config_id); } else if (config_id.compare("subnet6") == 0) { parser = new Subnets6ListConfigParser(config_id); } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, - globalContext()->options_, + parser = new OptionDataListParser(config_id, + globalContext()->options_, globalContext(), Dhcp6OptionDataParser::factory); } else if (config_id.compare("option-def") == 0) { - parser = new OptionDefListParser(config_id, + parser = new OptionDefListParser(config_id, globalContext()->option_defs_); } else if (config_id.compare("version") == 0) { - parser = new StringParser(config_id, + parser = new StringParser(config_id, globalContext()->string_values_); } else if (config_id.compare("lease-database") == 0) { parser = new DbAccessParser(config_id); @@ -450,7 +450,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { /// @todo: Append most essential info here (like "2 new subnets configured") string config_details; - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_START).arg(config_set->str()); // Some of the values specified in the configuration depend on diff --git a/src/bin/dhcp6/config_parser.h b/src/bin/dhcp6/config_parser.h index 2dce79e238..bd571af0bd 100644 --- a/src/bin/dhcp6/config_parser.h +++ b/src/bin/dhcp6/config_parser.h @@ -31,8 +31,8 @@ class Dhcpv6Srv; /// @brief Configures DHCPv6 server /// -/// This function is called every time a new configuration is received. The -/// extra parameter is a reference to DHCPv6 server component. It is currently +/// This function is called every time a new configuration is received. The +/// extra parameter is a reference to DHCPv6 server component. It is currently /// not used and CfgMgr::instance() is accessed instead. /// /// This method does not throw. It catches all exceptions and returns them as @@ -53,7 +53,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set); /// /// @returns a reference to the global context ParserContextPtr& globalContext(); - + }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index e94b49caad..04cf508ebc 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -57,17 +57,17 @@ ParserContext::ParserContext(const ParserContext& rhs): universe_(rhs.universe_) { } -ParserContext& +ParserContext& ParserContext::operator=(const ParserContext& rhs) { if (this != &rhs) { - boolean_values_ = + boolean_values_ = BooleanStoragePtr(new BooleanStorage(*(rhs.boolean_values_))); - uint32_values_ = + uint32_values_ = Uint32StoragePtr(new Uint32Storage(*(rhs.uint32_values_))); - string_values_ = + string_values_ = StringStoragePtr(new StringStorage(*(rhs.string_values_))); options_ = OptionStoragePtr(new OptionStorage(*(rhs.options_))); - option_defs_ = + option_defs_ = OptionDefStoragePtr(new OptionDefStorage(*(rhs.option_defs_))); universe_ = rhs.universe_; } @@ -81,14 +81,14 @@ DebugParser::DebugParser(const std::string& param_name) :param_name_(param_name) { } -void +void DebugParser::build(ConstElementPtr new_config) { value_ = new_config; std::cout << "Build for token: [" << param_name_ << "] = [" - << value_->str() << "]" << std::endl; + << value_->str() << "]" << std::endl; } -void +void DebugParser::commit() { // Debug message. The whole DebugParser class is used only for parser // debugging, and is not used in production code. It is very convenient @@ -106,7 +106,7 @@ template<> void ValueParser::build(isc::data::ConstElementPtr value) { try { value_ = value->boolValue(); } catch (const isc::data::TypeError &) { - isc_throw(BadValue, " Wrong value type for " << param_name_ + isc_throw(BadValue, " Wrong value type for " << param_name_ << " : build called with a non-boolean element."); } } @@ -233,22 +233,22 @@ OptionDataParser::OptionDataParser(const std::string&, OptionStoragePtr options, } } -void +void OptionDataParser::build(ConstElementPtr option_data_entries) { BOOST_FOREACH(ConfigPair param, option_data_entries->mapValue()) { ParserPtr parser; if (param.first == "name" || param.first == "data" || param.first == "space") { - StringParserPtr name_parser(new StringParser(param.first, - string_values_)); + StringParserPtr name_parser(new StringParser(param.first, + string_values_)); parser = name_parser; } else if (param.first == "code") { - Uint32ParserPtr code_parser(new Uint32Parser(param.first, - uint32_values_)); + Uint32ParserPtr code_parser(new Uint32Parser(param.first, + uint32_values_)); parser = code_parser; } else if (param.first == "csv-format") { - BooleanParserPtr value_parser(new BooleanParser(param.first, - boolean_values_)); + BooleanParserPtr value_parser(new BooleanParser(param.first, + boolean_values_)); parser = value_parser; } else { isc_throw(DhcpConfigError, @@ -270,12 +270,12 @@ OptionDataParser::build(ConstElementPtr option_data_entries) { createOption(); } -void +void OptionDataParser::commit() { if (!option_descriptor_.option) { - // Before we can commit the new option should be configured. If it is + // Before we can commit the new option should be configured. If it is // not than somebody must have called commit() before build(). - isc_throw(isc::InvalidOperation, + isc_throw(isc::InvalidOperation, "parser logic error: no option has been configured and" " thus there is nothing to commit. Has build() been called?"); } @@ -299,7 +299,7 @@ OptionDataParser::commit() { options_->addItem(option_descriptor_, option_space_); } -void +void OptionDataParser::createOption() { // Option code is held in the uint32_t storage but is supposed to // be uint16_t value. We need to check that value in the configuration @@ -316,7 +316,7 @@ OptionDataParser::createOption() { // Check that the option name has been specified, is non-empty and does not // contain spaces - std::string option_name = string_values_->getParam("name"); + std::string option_name = string_values_->getParam("name"); if (option_name.empty()) { isc_throw(DhcpConfigError, "name of the option with code '" << option_code << "' is empty"); @@ -325,7 +325,7 @@ OptionDataParser::createOption() { << "', space character is not allowed"); } - std::string option_space = string_values_->getParam("space"); + std::string option_space = string_values_->getParam("space"); if (!OptionSpace::validateName(option_space)) { isc_throw(DhcpConfigError, "invalid option space name '" << option_space << "' specified for option '" @@ -341,7 +341,7 @@ OptionDataParser::createOption() { // need to search for its definition among user-configured // options. They are expected to be in the global storage // already. - OptionDefContainerPtr defs = + OptionDefContainerPtr defs = global_context_->option_defs_->getItems(option_space); // The getItems() should never return the NULL pointer. If there are @@ -395,16 +395,16 @@ OptionDataParser::createOption() { << " does not have a definition."); } - // @todo We have a limited set of option definitions intiialized at - // the moment. In the future we want to initialize option definitions - // for all options. Consequently an error will be issued if an option + // @todo We have a limited set of option definitions intiialized at + // the moment. In the future we want to initialize option definitions + // for all options. Consequently an error will be issued if an option // definition does not exist for a particular option code. For now it is // ok to create generic option if definition does not exist. - OptionPtr option(new Option(global_context_->universe_, + OptionPtr option(new Option(global_context_->universe_, static_cast(option_code), binary)); - // The created option is stored in option_descriptor_ class member - // until the commit stage when it is inserted into the main storage. - // If an option with the same code exists in main storage already the + // The created option is stored in option_descriptor_ class member + // until the commit stage when it is inserted into the main storage. + // If an option with the same code exists in main storage already the // old option is replaced. option_descriptor_.option = option; option_descriptor_.persistent = false; @@ -426,9 +426,9 @@ OptionDataParser::createOption() { // an instance of our option. try { OptionPtr option = csv_format ? - def->optionFactory(global_context_->universe_, + def->optionFactory(global_context_->universe_, option_code, data_tokens) : - def->optionFactory(global_context_->universe_, + def->optionFactory(global_context_->universe_, option_code, binary); Subnet::OptionDescriptor desc(option, false); option_descriptor_.option = option; @@ -446,10 +446,10 @@ OptionDataParser::createOption() { } // **************************** OptionDataListParser ************************* -OptionDataListParser::OptionDataListParser(const std::string&, +OptionDataListParser::OptionDataListParser(const std::string&, OptionStoragePtr options, ParserContextPtr global_context, OptionDataParserFactory* optionDataParserFactory) - : options_(options), local_options_(new OptionStorage()), + : options_(options), local_options_(new OptionStorage()), global_context_(global_context), optionDataParserFactory_(optionDataParserFactory) { if (!options_) { @@ -468,11 +468,11 @@ OptionDataListParser::OptionDataListParser(const std::string&, } } -void +void OptionDataListParser::build(ConstElementPtr option_data_list) { BOOST_FOREACH(ConstElementPtr option_value, option_data_list->listValue()) { - boost::shared_ptr - parser((*optionDataParserFactory_)("option-data", + boost::shared_ptr + parser((*optionDataParserFactory_)("option-data", local_options_, global_context_)); // options_ member will hold instances of all options thus @@ -484,7 +484,7 @@ OptionDataListParser::build(ConstElementPtr option_data_list) { } } -void +void OptionDataListParser::commit() { BOOST_FOREACH(ParserPtr parser, parsers_) { parser->commit(); @@ -497,7 +497,7 @@ OptionDataListParser::commit() { } // ******************************** OptionDefParser **************************** -OptionDefParser::OptionDefParser(const std::string&, +OptionDefParser::OptionDefParser(const std::string&, OptionDefStoragePtr storage) : storage_(storage), boolean_values_(new BooleanStorage()), string_values_(new StringStorage()), uint32_values_(new Uint32Storage()) { @@ -507,23 +507,23 @@ OptionDefParser::OptionDefParser(const std::string&, } } -void +void OptionDefParser::build(ConstElementPtr option_def) { // Parse the elements that make up the option definition. BOOST_FOREACH(ConfigPair param, option_def->mapValue()) { std::string entry(param.first); ParserPtr parser; - if (entry == "name" || entry == "type" || entry == "record-types" + if (entry == "name" || entry == "type" || entry == "record-types" || entry == "space" || entry == "encapsulate") { - StringParserPtr str_parser(new StringParser(entry, + StringParserPtr str_parser(new StringParser(entry, string_values_)); parser = str_parser; } else if (entry == "code") { - Uint32ParserPtr code_parser(new Uint32Parser(entry, + Uint32ParserPtr code_parser(new Uint32Parser(entry, uint32_values_)); parser = code_parser; } else if (entry == "array") { - BooleanParserPtr array_parser(new BooleanParser(entry, + BooleanParserPtr array_parser(new BooleanParser(entry, boolean_values_)); parser = array_parser; } else { @@ -555,7 +555,7 @@ OptionDefParser::build(ConstElementPtr option_def) { } } -void +void OptionDefParser::commit() { if (storage_ && option_definition_ && OptionSpace::validateName(option_space_name_)) { @@ -563,7 +563,7 @@ OptionDefParser::commit() { } } -void +void OptionDefParser::createOptionDef() { // Get the option space name and validate it. std::string space = string_values_->getParam("space"); @@ -643,7 +643,7 @@ OptionDefParser::createOptionDef() { } // ******************************** OptionDefListParser ************************ -OptionDefListParser::OptionDefListParser(const std::string&, +OptionDefListParser::OptionDefListParser(const std::string&, OptionDefStoragePtr storage) :storage_(storage) { if (!storage_) { isc_throw(isc::dhcp::DhcpConfigError, "parser logic error:" @@ -651,7 +651,7 @@ OptionDefListParser::OptionDefListParser(const std::string&, } } -void +void OptionDefListParser::build(ConstElementPtr option_def_list) { // Clear existing items in the storage. // We are going to replace all of them. @@ -670,7 +670,7 @@ OptionDefListParser::build(ConstElementPtr option_def_list) { } } -void +void OptionDefListParser::commit() { CfgMgr& cfg_mgr = CfgMgr::instance(); cfg_mgr.deleteOptionDefs(); @@ -702,7 +702,7 @@ PoolParser::PoolParser(const std::string&, PoolStoragePtr pools) } } -void +void PoolParser::build(ConstElementPtr pools_list) { BOOST_FOREACH(ConstElementPtr text_pool, pools_list->listValue()) { // That should be a single pool representation. It should contain @@ -730,7 +730,7 @@ PoolParser::build(ConstElementPtr pools_list) { // will result in interpreting the first digit as output // value and throwing exception if length is written on two // digits (because there are extra characters left over). - + // No checks for values over 128. Range correctness will // be checked in Pool4 constructor. len = boost::lexical_cast(prefix_len); @@ -762,7 +762,7 @@ PoolParser::build(ConstElementPtr pools_list) { } } -void +void PoolParser::commit() { if (pools_) { // local_pools_ holds the values produced by the build function. @@ -774,9 +774,9 @@ PoolParser::commit() { //****************************** SubnetConfigParser ************************* -SubnetConfigParser::SubnetConfigParser(const std::string&, - ParserContextPtr global_context) - : uint32_values_(new Uint32Storage()), string_values_(new StringStorage()), +SubnetConfigParser::SubnetConfigParser(const std::string&, + ParserContextPtr global_context) + : uint32_values_(new Uint32Storage()), string_values_(new StringStorage()), pools_(new PoolStorage()), options_(new OptionStorage()), global_context_(global_context) { // The first parameter should always be "subnet", but we don't check @@ -787,7 +787,7 @@ SubnetConfigParser::SubnetConfigParser(const std::string&, } } -void +void SubnetConfigParser::build(ConstElementPtr subnet) { BOOST_FOREACH(ConfigPair param, subnet->mapValue()) { ParserPtr parser(createSubnetConfigParser(param.first)); @@ -811,8 +811,8 @@ SubnetConfigParser::build(ConstElementPtr subnet) { createSubnet(); } -void -SubnetConfigParser::appendSubOptions(const std::string& option_space, +void +SubnetConfigParser::appendSubOptions(const std::string& option_space, OptionPtr& option) { // Only non-NULL options are stored in option container. // If this option pointer is NULL this is a serious error. @@ -866,7 +866,7 @@ SubnetConfigParser::appendSubOptions(const std::string& option_space, } } -void +void SubnetConfigParser::createSubnet() { std::string subnet_txt; try { @@ -897,11 +897,11 @@ SubnetConfigParser::createSubnet() { isc::asiolink::IOAddress addr(subnet_txt.substr(0, pos)); uint8_t len = boost::lexical_cast(subnet_txt.substr(pos + 1)); - // Call the subclass's method to instantiate the subnet + // Call the subclass's method to instantiate the subnet initSubnet(addr, len); // Add pools to it. - for (PoolStorage::iterator it = pools_->begin(); it != pools_->end(); + for (PoolStorage::iterator it = pools_->begin(); it != pools_->end(); ++it) { subnet_->addPool(*it); } @@ -976,7 +976,7 @@ SubnetConfigParser::createSubnet() { // values we don't add option from the global storage // if there is one already. Subnet::OptionDescriptor existing_desc = - subnet_->getOptionDescriptor(option_space, + subnet_->getOptionDescriptor(option_space, desc.option->getType()); if (!existing_desc.option) { // Add sub-options (if any). @@ -987,15 +987,15 @@ SubnetConfigParser::createSubnet() { } } -isc::dhcp::Triplet +isc::dhcp::Triplet SubnetConfigParser::getParam(const std::string& name) { uint32_t value = 0; try { - // look for local value + // look for local value value = uint32_values_->getParam(name); } catch (const DhcpConfigError &) { try { - // no local, use global value + // no local, use global value value = global_context_->uint32_values_->getParam(name); } catch (const DhcpConfigError &) { isc_throw(DhcpConfigError, "Mandatory parameter " << name diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h index 5551133c07..5b3f09a6dd 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.h +++ b/src/lib/dhcpsrv/dhcp_parsers.h @@ -47,8 +47,8 @@ typedef boost::shared_ptr OptionStoragePtr; /// @brief A template class that stores named elements of a given data type. /// /// This template class is provides data value storage for configuration parameters -/// of a given data type. The values are stored by parameter name and as instances -/// of type "ValueType". +/// of a given data type. The values are stored by parameter name and as instances +/// of type "ValueType". /// /// @param ValueType is the data type of the elements to store. template @@ -57,7 +57,7 @@ class ValueStorage { /// @brief Stores the the parameter and its value in the store. /// /// If the parameter does not exist in the store, then it will be added, - /// otherwise its data value will be updated with the given value. + /// otherwise its data value will be updated with the given value. /// /// @param name is the name of the paramater to store. /// @param value is the data value to store. @@ -74,7 +74,7 @@ class ValueStorage { /// @return The paramater's data value of type . /// @throw DhcpConfigError if the parameter is not found. ValueType getParam(const std::string& name) const { - typename std::map::const_iterator param + typename std::map::const_iterator param = values_.find(name); if (param == values_.end()) { @@ -87,8 +87,8 @@ class ValueStorage { /// @brief Remove the parameter from the store. /// - /// Deletes the entry for the given parameter from the store if it - /// exists. + /// Deletes the entry for the given parameter from the store if it + /// exists. /// /// @param name is the name of the paramater to delete. void delParam(const std::string& name) { @@ -108,7 +108,7 @@ class ValueStorage { }; -/// @brief a collection of elements that store uint32 values +/// @brief a collection of elements that store uint32 values typedef ValueStorage Uint32Storage; typedef boost::shared_ptr Uint32StoragePtr; @@ -128,9 +128,9 @@ typedef boost::shared_ptr BooleanStoragePtr; class ParserContext { public: /// @brief Constructor - /// + /// /// @param universe is the Option::Universe value of this - /// context. + /// context. ParserContext(Option::Universe universe); /// @brief Copy constructor @@ -161,12 +161,12 @@ public: /// @brief Pointer to various parser context. typedef boost::shared_ptr ParserContextPtr; -/// @brief Simple data-type parser template class +/// @brief Simple data-type parser template class /// /// This is the template class for simple data-type parsers. It supports -/// parsing a configuration parameter with specific data-type for its -/// possible values. It provides a common constructor, commit, and templated -/// data storage. The "build" method implementation must be provided by a +/// parsing a configuration parameter with specific data-type for its +/// possible values. It provides a common constructor, commit, and templated +/// data storage. The "build" method implementation must be provided by a /// declaring type. /// @param ValueType is the data type of the configuration paramater value /// the parser should handle. @@ -182,7 +182,7 @@ public: /// @throw isc::dhcp::DhcpConfigError if a provided parameter's /// name is empty. /// @throw isc::dhcp::DhcpConfigError if storage is null. - ValueParser(const std::string& param_name, + ValueParser(const std::string& param_name, boost::shared_ptr > storage) : storage_(storage), param_name_(param_name), value_() { // Empty parameter name is invalid. @@ -204,7 +204,7 @@ public: /// @param value a value to be parsed. /// /// @throw isc::BadValue Typically the implementing type will throw - /// a BadValue exception when given an invalid Element to parse. + /// a BadValue exception when given an invalid Element to parse. void build(isc::data::ConstElementPtr value); /// @brief Put a parsed value to the storage. @@ -213,7 +213,7 @@ public: // its value. If it doesn't we insert a new element. storage_->setParam(param_name_, value_); } - + private: /// Pointer to the storage where committed value is stored. boost::shared_ptr > storage_; @@ -347,11 +347,11 @@ public: /// @param dummy first argument is ignored, all Parser constructors /// accept string as first argument. /// @param options is the option storage in which to store the parsed option - /// upon "commit". - /// @param global_context is a pointer to the global context which - /// stores global scope parameters, options, option defintions. + /// upon "commit". + /// @param global_context is a pointer to the global context which + /// stores global scope parameters, options, option defintions. /// @throw isc::dhcp::DhcpConfigError if options or global_context are null. - OptionDataParser(const std::string&, OptionStoragePtr options, + OptionDataParser(const std::string&, OptionStoragePtr options, ParserContextPtr global_context); /// @brief Parses the single option data. @@ -371,31 +371,31 @@ public: /// @brief Commits option value. /// - /// This function adds a new option to the storage or replaces an existing + /// This function adds a new option to the storage or replaces an existing /// option with the same code. /// - /// @throw isc::InvalidOperation if failed to set pointer to storage or + /// @throw isc::InvalidOperation if failed to set pointer to storage or /// failed /// to call build() prior to commit. If that happens data in the storage /// remain un-modified. virtual void commit(); - /// @brief virtual destructor to ensure orderly destruction of derivations. + /// @brief virtual destructor to ensure orderly destruction of derivations. virtual ~OptionDataParser(){}; protected: /// @brief Finds an option definition within the server's option space - /// - /// Given an option space and an option code, find the correpsonding + /// + /// Given an option space and an option code, find the correpsonding /// option defintion within the server's option defintion storage. This /// method is pure virtual requiring derivations to manage which option /// space(s) is valid for search. /// - /// @param option_space name of the parameter option space - /// @param option_code numeric value of the parameter to find - /// @return OptionDefintionPtr of the option defintion or an + /// @param option_space name of the parameter option space + /// @param option_code numeric value of the parameter to find + /// @return OptionDefintionPtr of the option defintion or an /// empty OptionDefinitionPtr if not found. - /// @throw DhcpConfigError if the option space requested is not valid + /// @throw DhcpConfigError if the option space requested is not valid /// for this server. virtual OptionDefinitionPtr findServerSpaceOptionDefinition ( std::string& option_space, uint32_t option_code) = 0; @@ -435,13 +435,13 @@ private: /// Option space name where the option belongs to. std::string option_space_; - /// Parsing context which contains global values, options and option + /// Parsing context which contains global values, options and option /// definitions. ParserContextPtr global_context_; }; ///@brief Function pointer for OptionDataParser factory methods -typedef OptionDataParser *OptionDataParserFactory(const std::string&, +typedef OptionDataParser *OptionDataParserFactory(const std::string&, OptionStoragePtr options, ParserContextPtr global_context); /// @brief Parser for option data values within a subnet. @@ -456,13 +456,13 @@ public: /// /// @param string& nominally would be param name, this is always ignored. /// @param options parsed option storage for options in this list - /// @param global_context is a pointer to the global context which - /// stores global scope parameters, options, option defintions. - /// @param optionDataParserFactory factory method for creating individual - /// option parsers + /// @param global_context is a pointer to the global context which + /// stores global scope parameters, options, option defintions. + /// @param optionDataParserFactory factory method for creating individual + /// option parsers /// @throw isc::dhcp::DhcpConfigError if options or global_context are null. - OptionDataListParser(const std::string&, OptionStoragePtr options, - ParserContextPtr global_context, + OptionDataListParser(const std::string&, OptionStoragePtr options, + ParserContextPtr global_context, OptionDataParserFactory *optionDataParserFactory); /// @brief Parses entries that define options' data for a subnet. @@ -492,7 +492,7 @@ private: /// Collection of parsers; ParserCollection parsers_; - /// Parsing context which contains global values, options and option + /// Parsing context which contains global values, options and option /// definitions. ParserContextPtr global_context_; @@ -510,8 +510,8 @@ public: /// /// @param dummy first argument is ignored, all Parser constructors /// accept string as first argument. - /// @param storage is the definition storage in which to store the parsed - /// definition upon "commit". + /// @param storage is the definition storage in which to store the parsed + /// definition upon "commit". /// @throw isc::dhcp::DhcpConfigError if storage is null. OptionDefParser(const std::string&, OptionDefStoragePtr storage); @@ -561,8 +561,8 @@ public: /// /// @param dummy first argument is ignored, all Parser constructors /// accept string as first argument. - /// @param storage is the definition storage in which to store the parsed - /// definitions in this list + /// @param storage is the definition storage in which to store the parsed + /// definitions in this list /// @throw isc::dhcp::DhcpConfigError if storage is null. OptionDefListParser(const std::string&, OptionDefStoragePtr storage); @@ -581,7 +581,7 @@ public: private: /// @brief storage for option definitions. - OptionDefStoragePtr storage_; + OptionDefStoragePtr storage_; }; /// @brief a collection of pools @@ -602,12 +602,11 @@ class PoolParser : public DhcpConfigParser { public: /// @brief constructor. - - + /// /// @param dummy first argument is ignored, all Parser constructors /// accept string as first argument. - /// @param pools is the storage in which to store the parsed pool - /// upon "commit". + /// @param pools is the storage in which to store the parsed pool + /// upon "commit". /// @throw isc::dhcp::DhcpConfigError if storage is null. PoolParser(const std::string&, PoolStoragePtr pools); @@ -629,9 +628,9 @@ protected: /// /// @param addr is the IP prefix of the pool. /// @param len is the prefix length. - /// @param ignored dummy parameter to provide symmetry between - /// @return returns a PoolPtr to the new Pool object. - virtual PoolPtr poolMaker(isc::asiolink::IOAddress &addr, uint32_t len, + /// @param ignored dummy parameter to provide symmetry between + /// @return returns a PoolPtr to the new Pool object. + virtual PoolPtr poolMaker(isc::asiolink::IOAddress &addr, uint32_t len, int32_t ptype=0) = 0; /// @brief Creates a Pool object given starting and ending IP addresses. @@ -640,7 +639,7 @@ protected: /// @param max is the last IP address in the pool. /// @param ptype is the type of pool to create (not used by all derivations) /// @return returns a PoolPtr to the new Pool object. - virtual PoolPtr poolMaker(isc::asiolink::IOAddress &min, + virtual PoolPtr poolMaker(isc::asiolink::IOAddress &min, isc::asiolink::IOAddress &max, int32_t ptype=0) = 0; /// @brief pointer to the actual Pools storage @@ -669,7 +668,7 @@ public: /// @param subnet pointer to the content of subnet definition /// /// @throw isc::DhcpConfigError if subnet configuration parsing failed. - virtual void build(isc::data::ConstElementPtr subnet); + virtual void build(isc::data::ConstElementPtr subnet); /// @brief Adds the created subnet to a server's configuration. virtual void commit() = 0; @@ -686,7 +685,7 @@ protected: const std::string& config_id) = 0; /// @brief Determines if the given option space name and code describe - /// a standard option for the server. + /// a standard option for the server. /// /// @param option_space is the name of the option space to consider /// @param code is the numeric option code to consider @@ -702,20 +701,20 @@ protected: uint32_t code) = 0; /// @brief Issues a server specific warning regarding duplicate subnet - /// options. - /// + /// options. + /// /// @param code is the numeric option code of the duplicate option - /// @param addr is the subnet address + /// @param addr is the subnet address /// @todo a means to know the correct logger and perhaps a common /// message would allow this method to be emitted by the base class. - virtual void duplicate_option_warning(uint32_t code, + virtual void duplicate_option_warning(uint32_t code, isc::asiolink::IOAddress& addr) = 0; - /// @brief Instantiates the subnet based on a given IP prefix and prefix - /// length. - /// + /// @brief Instantiates the subnet based on a given IP prefix and prefix + /// length. + /// /// @param addr is the IP prefix of the subnet. - /// @param len is the prefix length + /// @param len is the prefix length virtual void initSubnet(isc::asiolink::IOAddress addr, uint8_t len) = 0; /// @brief Returns value for a given parameter (after using inheritance) @@ -739,7 +738,7 @@ private: /// @brief Create a new subnet using a data from child parsers. /// - /// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing + /// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing /// failed. void createSubnet(); @@ -763,7 +762,7 @@ protected: /// Pointer to the created subnet object. isc::dhcp::SubnetPtr subnet_; - /// Parsing context which contains global values, options and option + /// Parsing context which contains global values, options and option /// definitions. ParserContextPtr global_context_; }; diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index b4b9451856..cde227acee 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -48,7 +48,7 @@ public: /// @brief Check BooleanParser basic functionality. -/// +/// /// Verifies that the parser: /// 1. Does not allow empty for storage. /// 2. Rejects a non-boolean element. @@ -95,7 +95,7 @@ TEST_F(DhcpParserTest, booleanParserTest) { } /// @brief Check StringParser basic functionality -/// +/// /// Verifies that the parser: /// 1. Does not allow empty for storage. /// 2. Builds with a nont string value. @@ -135,7 +135,7 @@ TEST_F(DhcpParserTest, stringParserTest) { } /// @brief Check Uint32Parser basic functionality -/// +/// /// Verifies that the parser: /// 1. Does not allow empty for storage. /// 2. Rejects a non-integer element. @@ -164,8 +164,8 @@ TEST_F(DhcpParserTest, uint32ParserTest) { ElementPtr int_element = Element::create(-1); EXPECT_THROW(parser.build(int_element), isc::BadValue); - // Verify that parser with rejects too large a value provided we are on - // 64-bit platform. + // Verify that parser with rejects too large a value provided we are on + // 64-bit platform. if (sizeof(long) > sizeof(uint32_t)) { long max = (long)(std::numeric_limits::max()) + 1; int_element->setValue(max); @@ -246,13 +246,13 @@ TEST_F(DhcpParserTest, interfaceListParserTest) { 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. class UtestOptionDataParser : public OptionDataParser { public: - UtestOptionDataParser(const std::string&, - OptionStoragePtr options, ParserContextPtr global_context) + UtestOptionDataParser(const std::string&, + OptionStoragePtr options, ParserContextPtr global_context) :OptionDataParser("", options, global_context) { } @@ -266,12 +266,12 @@ protected: virtual OptionDefinitionPtr findServerSpaceOptionDefinition ( std::string&, uint32_t) { OptionDefinitionPtr def; - // always return empty + // always return empty return (def); } }; -/// @brief Test Fixture class which provides basic structure for testing +/// @brief Test Fixture class which provides basic structure for testing /// configuration parsing. This is essentially the same structure provided /// by dhcp servers. class ParseConfigTest : public ::testing::Test { @@ -285,15 +285,15 @@ public: reset_context(); } - /// @brief Parses a configuration. + /// @brief Parses a configuration. /// /// Parse the given configuration, populating the context storage with - /// the parsed elements. - /// + /// the parsed elements. + /// /// @param config_set is the set of elements to parse. /// @return returns an ConstElementPtr containing the numeric result /// code and outcome comment. - isc::data::ConstElementPtr parseElementSet(isc::data::ConstElementPtr + isc::data::ConstElementPtr parseElementSet(isc::data::ConstElementPtr config_set) { // Answer will hold the result. ConstElementPtr answer; @@ -325,7 +325,7 @@ public: } // The option values parser is the next one to be run. - std::map::const_iterator + std::map::const_iterator option_config = values_map.find("option-data"); if (option_config != values_map.end()) { option_parser->build(option_config->second); @@ -348,21 +348,21 @@ public: /// @brief Create an element parser based on the element name. /// - /// Note that currently it only supports option-defs and option-data, - /// - /// @param config_id is the name of the configuration element. + /// Note that currently it only supports option-defs and option-data, + /// + /// @param config_id is the name of the configuration element. /// @return returns a raw pointer to DhcpConfigParser. Note caller is /// responsible for deleting it once no longer needed. /// @throw throws NotImplemented if element name isn't supported. DhcpConfigParser* createConfigParser(const std::string& config_id) { DhcpConfigParser* parser = NULL; if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, - parser_context_->options_, + parser = new OptionDataListParser(config_id, + parser_context_->options_, parser_context_, UtestOptionDataParser::factory); } else if (config_id.compare("option-def") == 0) { - parser = new OptionDefListParser(config_id, + parser = new OptionDefListParser(config_id, parser_context_->option_defs_); } else { isc_throw(NotImplemented, @@ -373,15 +373,15 @@ public: return (parser); } - /// @brief Convenicee method for parsing a configuration - /// + /// @brief Convenicee method for parsing a configuration + /// /// Given a configuration string, convert it into Elements - /// and parse them. + /// and parse them. /// @param config is the configuration string to parse /// - /// @return retuns 0 if the configuration parsed successfully, + /// @return retuns 0 if the configuration parsed successfully, /// non-zero otherwise failure. - int parseConfiguration (std::string &config) { + int parseConfiguration (std::string &config) { int rcode_ = 1; // Turn config into elements. // Test json just to make sure its valid. @@ -395,17 +395,17 @@ public: return (rcode_); } - /// @brief Find an option definition for a given space and code within + /// @brief Find an option definition for a given space and code within /// the parser context. /// @param space is the space name of the desired option. /// @param code is the numeric "type" of the desired option. /// @return returns an OptionDefinitionPtr which points to the found /// definition or is empty. - /// ASSERT_ tests don't work inside functions that return values + /// ASSERT_ tests don't work inside functions that return values OptionDefinitionPtr getOptionDef(std::string space, uint32_t code) { OptionDefinitionPtr def; - OptionDefContainerPtr defs = + OptionDefContainerPtr defs = parser_context_->option_defs_->getItems(space); // Should always be able to get definitions list even if it is empty. EXPECT_TRUE(defs); @@ -419,41 +419,41 @@ public: def = *(idx.begin()); } } - return (def); + return (def); } - /// @brief Find an option for a given space and code within the parser + /// @brief Find an option for a given space and code within the parser /// context. /// @param space is the space name of the desired option. /// @param code is the numeric "type" of the desired option. /// @return returns an OptionPtr which points to the found /// option or is empty. - /// ASSERT_ tests don't work inside functions that return values + /// ASSERT_ tests don't work inside functions that return values OptionPtr getOptionPtr(std::string space, uint32_t code) { OptionPtr option_ptr; - Subnet::OptionContainerPtr options = + Subnet::OptionContainerPtr options = parser_context_->options_->getItems(space); // Should always be able to get options list even if it is empty. EXPECT_TRUE(options); if (options) { // Attempt to find desired option. const Subnet::OptionContainerTypeIndex& idx = options->get<1>(); - const Subnet::OptionContainerTypeRange& range = + const Subnet::OptionContainerTypeRange& range = idx.equal_range(code); int cnt = std::distance(range.first, range.second); EXPECT_EQ(1, cnt); if (cnt == 1) { - Subnet::OptionDescriptor desc = *(idx.begin()); - option_ptr = desc.option; + Subnet::OptionDescriptor desc = *(idx.begin()); + option_ptr = desc.option; EXPECT_TRUE(option_ptr); } } - return (option_ptr); + return (option_ptr); } - /// @brief Wipes the contents of the context to allowing another parsing + /// @brief Wipes the contents of the context to allowing another parsing /// during a given test if needed. void reset_context(){ // Note set context universe to V6 as it has to be something. @@ -468,7 +468,7 @@ public: }; /// @brief Check Basic parsing of option definitions. -/// +/// /// Note that this tests basic operation of the OptionDefinitionListParser and /// OptionDefinitionParser. It uses a simple configuration consisting of one /// one definition and verifies that it is parsed and committed to storage @@ -493,7 +493,7 @@ TEST_F(ParseConfigTest, basicOptionDefTest) { ASSERT_TRUE(rcode == 0); // Verify that the option definition can be retrieved. - OptionDefinitionPtr def = getOptionDef("isc", 100); + OptionDefinitionPtr def = getOptionDef("isc", 100); ASSERT_TRUE(def); // Verify that the option definition is correct. @@ -505,7 +505,7 @@ TEST_F(ParseConfigTest, basicOptionDefTest) { } /// @brief Check Basic parsing of options. -/// +/// /// Note that this tests basic operation of the OptionDataListParser and /// OptionDataParser. It uses a simple configuration consisting of one /// one definition and matching option data. It verifies that the option From 9517e24b3bc274e4a8f080cb15358db8017c3f44 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 10:54:55 +0200 Subject: [PATCH 04/17] [1555] Trivial: fixed doxygen issues in dhcp_parsers.h. --- src/lib/dhcpsrv/dhcp_parsers.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h index 5b3f09a6dd..37dc95073e 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.h +++ b/src/lib/dhcpsrv/dhcp_parsers.h @@ -71,7 +71,7 @@ class ValueStorage { /// @param name is the name of the parameter for which the data /// value is desired. /// - /// @return The paramater's data value of type . + /// @return The paramater's data value of type @c ValueType. /// @throw DhcpConfigError if the parameter is not found. ValueType getParam(const std::string& name) const { typename std::map::const_iterator param @@ -199,7 +199,7 @@ public: } - /// @brief Parse a given element into a value of type + /// @brief Parse a given element into a value of type @c ValueType /// /// @param value a value to be parsed. /// @@ -351,7 +351,7 @@ public: /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. /// @throw isc::dhcp::DhcpConfigError if options or global_context are null. - OptionDataParser(const std::string&, OptionStoragePtr options, + OptionDataParser(const std::string& dummy, OptionStoragePtr options, ParserContextPtr global_context); /// @brief Parses the single option data. @@ -454,14 +454,14 @@ class OptionDataListParser : public DhcpConfigParser { public: /// @brief Constructor. /// - /// @param string& nominally would be param name, this is always ignored. + /// @param dummy nominally would be param name, this is always ignored. /// @param options parsed option storage for options in this list /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. /// @param optionDataParserFactory factory method for creating individual /// option parsers /// @throw isc::dhcp::DhcpConfigError if options or global_context are null. - OptionDataListParser(const std::string&, OptionStoragePtr options, + OptionDataListParser(const std::string& dummy, OptionStoragePtr options, ParserContextPtr global_context, OptionDataParserFactory *optionDataParserFactory); @@ -513,7 +513,7 @@ public: /// @param storage is the definition storage in which to store the parsed /// definition upon "commit". /// @throw isc::dhcp::DhcpConfigError if storage is null. - OptionDefParser(const std::string&, OptionDefStoragePtr storage); + OptionDefParser(const std::string& dummy, OptionDefStoragePtr storage); /// @brief Parses an entry that describes single option definition. /// @@ -564,7 +564,7 @@ public: /// @param storage is the definition storage in which to store the parsed /// definitions in this list /// @throw isc::dhcp::DhcpConfigError if storage is null. - OptionDefListParser(const std::string&, OptionDefStoragePtr storage); + OptionDefListParser(const std::string& dummy, OptionDefStoragePtr storage); /// @brief Parse configuration entries. /// @@ -608,7 +608,7 @@ public: /// @param pools is the storage in which to store the parsed pool /// upon "commit". /// @throw isc::dhcp::DhcpConfigError if storage is null. - PoolParser(const std::string&, PoolStoragePtr pools); + PoolParser(const std::string& dummy, PoolStoragePtr pools); /// @brief parses the actual list /// @@ -628,7 +628,7 @@ protected: /// /// @param addr is the IP prefix of the pool. /// @param len is the prefix length. - /// @param ignored dummy parameter to provide symmetry between + /// @param ptype is the type of pool to create. /// @return returns a PoolPtr to the new Pool object. virtual PoolPtr poolMaker(isc::asiolink::IOAddress &addr, uint32_t len, int32_t ptype=0) = 0; From 0a5d26dfc2ee6a4a84d596cbcb39e894e7432f3c Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 13:31:01 +0200 Subject: [PATCH 05/17] [1555] Reopen active sockets when configuration changes. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 46 ++++++++++++++++++++++++++++++--- src/bin/dhcp4/ctrl_dhcp4_srv.h | 11 ++++++++ src/bin/dhcp4/dhcp4_srv.cc | 5 ++-- src/bin/dhcp4/dhcp4_srv.h | 29 +++++++++++++++++++++ src/lib/dhcp/iface_mgr.cc | 9 ++++--- src/lib/dhcp/iface_mgr.h | 4 +++ 6 files changed, 96 insertions(+), 8 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index eb7d5599a0..9050a1651c 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -20,17 +20,18 @@ #include #include #include +#include #include #include #include #include #include +#include #include -#include -#include #include #include +#include using namespace isc::asiolink; using namespace isc::cc; @@ -101,7 +102,27 @@ ControlledDhcpv4Srv::dhcp4ConfigHandler(ConstElementPtr new_config) { } // Configure the server. - return (configureDhcp4Server(*server_, merged_config)); + ConstElementPtr answer = configureDhcp4Server(*server_, merged_config); + + // Check that configuration was successful. If not, do not reopen sockets. + int rcode = 0; + parseAnswer(rcode, answer); + if (rcode != 0) { + return (answer); + } + + // Configuration may change active interfaces. Therefore, we have to reopen + // sockets according to new configuration. This operation is not exception + // safe and we really don't want to emit exceptions to the callback caller. + // Instead, catch an exception and create appropriate answer. + try { + server_->openActiveSockets(server_->getPort(), server_->useBroadcast()); + } catch (std::exception& ex) { + std::ostringstream err; + err << "failed to open sockets after server reconfiguration: " << ex.what(); + answer = isc::config::createAnswer(1, err.str()); + } + return (answer); } ConstElementPtr @@ -228,6 +249,25 @@ ControlledDhcpv4Srv::execDhcpv4ServerCommand(const std::string& command_id, } } +void +ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, const bool use_bcast) { + IfaceMgr::instance().closeSockets(); + + // Get the reference to the collection of interfaces. This reference should be + // valid as long as the program is run because IfaceMgr is a singleton. + // Therefore we can safely iterate over instances of all interfaces and modify + // their flags. Here we modify flags which indicate wheter socket should be + // open for a particular interface or not. + IfaceMgr::IfaceCollection ifaces = IfaceMgr::instance().getIfaces(); + for (IfaceMgr::IfaceCollection::iterator iface = ifaces.begin(); + iface != ifaces.end(); ++iface) { + iface->inactive_ = !CfgMgr::instance().isActiveIface(iface->getName()); + } + // Let's reopen active sockets. openSockets4 will check internally whether + // sockets are marked active or inactive. + IfaceMgr::instance().openSockets4(port, use_bcast); +} + }; }; diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 8f14f4b909..e5104b5d41 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -130,6 +130,17 @@ protected: /// when there is a new command or configuration sent over msgq. static void sessionReader(void); + /// @brief Open sockets which are marked as active in @c CfgMgr. + /// + /// This function reopens sockets according to the current settings in the + /// Configuration Manager. It holds the list of the interfaces which server + /// should listen on. This function will open sockets on these interfaces + /// only. This function is not exception safe. + /// + /// @param port UDP port on which server should listen. + /// @param use_bcast should broadcast flags be set on the sockets. + static void openActiveSockets(const uint16_t port, const bool use_bcast); + /// @brief IOService object, used for all ASIO operations. isc::asiolink::IOService io_service_; diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 8aa913cffc..f414e5ae93 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -58,7 +58,8 @@ static const char* SERVER_ID_FILE = "b10-dhcp4-serverid"; // grants those options and a single, fixed, hardcoded lease. Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast, - const bool direct_response_desired) { + const bool direct_response_desired) + : port_(port), use_bcast_(use_bcast) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port); try { // First call to instance() will create IfaceMgr (it's a singleton) @@ -73,7 +74,7 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast, if (port) { // open sockets only if port is non-zero. Port 0 is used // for non-socket related testing. - IfaceMgr::instance().openSockets4(port, use_bcast); + IfaceMgr::instance().openSockets4(port_, use_bcast_); } string srvid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_ID_FILE); diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index f98233c4eb..59bb95c215 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -113,6 +113,32 @@ public: /// be freed by the caller. static const char* serverReceivedPacketName(uint8_t type); + /// + /// @name Public accessors returning values required to (re)open sockets. + /// + /// These accessors must be public because sockets are reopened from the + /// static configuration callback handler. This callback handler invokes + /// @c ControlledDhcpv4Srv::openActiveSockets which requires parameters + /// which has to be retrieved from the @c ControlledDhcpv4Srv object. + /// They are retrieved using these public functions + //@{ + /// + /// @brief Get UDP port on which server should listen. + /// + /// @return UDP port on which server should listen. + uint16_t getPort() const { + return (port_); + } + + /// @brief Return bool value indicating that broadcast flags should be set + /// on sockets. + /// + /// @return A bool value indicating that broadcast should be used (if true). + bool useBroadcast() const { + return (use_bcast_); + } + //@} + protected: /// @brief verifies if specified packet meets RFC requirements @@ -310,6 +336,9 @@ private: /// during normal operation (e.g. to use different allocators) boost::shared_ptr alloc_engine_; + uint16_t port_; ///< UDP port number on which server listens. + bool use_bcast_; ///< Should broadcast be enabled on sockets (if true). + }; }; // namespace isc::dhcp diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 1e97205287..dfe0f36604 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -51,7 +51,8 @@ IfaceMgr::instance() { Iface::Iface(const std::string& name, int ifindex) :name_(name), ifindex_(ifindex), mac_len_(0), hardware_type_(0), flag_loopback_(false), flag_up_(false), flag_running_(false), - flag_multicast_(false), flag_broadcast_(false), flags_(0) + flag_multicast_(false), flag_broadcast_(false), flags_(0), + inactive_(false) { memset(mac_, 0, sizeof(mac_)); } @@ -295,7 +296,8 @@ bool IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast) { if (iface->flag_loopback_ || !iface->flag_up_ || - !iface->flag_running_) { + !iface->flag_running_, + iface->inactive_) { continue; } @@ -361,7 +363,8 @@ bool IfaceMgr::openSockets6(const uint16_t port) { if (iface->flag_loopback_ || !iface->flag_up_ || - !iface->flag_running_) { + !iface->flag_running_, + !iface->inactive_) { continue; } diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 35b4dc0248..442107db65 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -309,6 +309,10 @@ public: /// Interface flags (this value is as is returned by OS, /// it may mean different things on different OSes). uint32_t flags_; + + /// Interface is inactive. This can be explicitly set to prevent Interface + /// Manager from opening the socket on this interface. + bool inactive_; }; /// @brief Handles network interfaces, transmission and reception. From 9f8d746079cad652e014d482327b6a73d5ee2f12 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 14:03:21 +0200 Subject: [PATCH 06/17] [1555] Open active V6 sockets when configuration is changed. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 1 - src/bin/dhcp6/ctrl_dhcp6_srv.cc | 41 ++++++++++++++++++++++++++++++++- src/bin/dhcp6/ctrl_dhcp6_srv.h | 11 +++++++++ src/bin/dhcp6/dhcp6_srv.cc | 4 ++-- src/bin/dhcp6/dhcp6_srv.h | 15 ++++++++++++ src/lib/dhcp/iface_mgr.cc | 2 +- 6 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 9050a1651c..78cfb9b8a8 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -26,7 +26,6 @@ #include #include #include -#include #include #include diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index c3488e5af0..cbd29b90fe 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -100,7 +101,27 @@ ControlledDhcpv6Srv::dhcp6ConfigHandler(ConstElementPtr new_config) { } // Configure the server. - return (configureDhcp6Server(*server_, merged_config)); + ConstElementPtr answer = configureDhcp6Server(*server_, merged_config); + + // Check that configuration was successful. If not, do not reopen sockets. + int rcode = 0; + parseAnswer(rcode, answer); + if (rcode != 0) { + return (answer); + } + + // Configuration may change active interfaces. Therefore, we have to reopen + // sockets according to new configuration. This operation is not exception + // safe and we really don't want to emit exceptions to the callback caller. + // Instead, catch an exception and create appropriate answer. + try { + server_->openActiveSockets(server_->getPort()); + } catch (const std::exception& ex) { + std::ostringstream err; + err << "failed to open sockets after server reconfiguration: " << ex.what(); + answer = isc::config::createAnswer(1, err.str()); + } + return (answer); } ConstElementPtr @@ -228,6 +249,24 @@ ControlledDhcpv6Srv::execDhcpv6ServerCommand(const std::string& command_id, } } +void +ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { + IfaceMgr::instance().closeSockets(); + + // Get the reference to the collection of interfaces. This reference should be + // valid as long as the program is run because IfaceMgr is a singleton. + // Therefore we can safely iterate over instances of all interfaces and modify + // their flags. Here we modify flags which indicate wheter socket should be + // open for a particular interface or not. + IfaceMgr::IfaceCollection ifaces = IfaceMgr::instance().getIfaces(); + for (IfaceMgr::IfaceCollection::iterator iface = ifaces.begin(); + iface != ifaces.end(); ++iface) { + iface->inactive_ = !CfgMgr::instance().isActiveIface(iface->getName()); + } + // Let's reopen active sockets. openSockets6 will check internally whether + // sockets are marked active or inactive. + IfaceMgr::instance().openSockets6(port); +} }; }; diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index ffd43c3a2e..d1599f807c 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -128,6 +128,17 @@ protected: /// when there is a new command or configuration sent over msgq. static void sessionReader(void); + /// @brief Open sockets which are marked as active in @c CfgMgr. + /// + /// This function reopens sockets according to the current settings in the + /// Configuration Manager. It holds the list of the interfaces which server + /// should listen on. This function will open sockets on these interfaces + /// only. This function is not exception safe. + /// + /// @param port UDP port on which server should listen. + /// @param use_bcast should broadcast flags be set on the sockets. + static void openActiveSockets(const uint16_t port); + /// @brief IOService object, used for all ASIO operations. isc::asiolink::IOService io_service_; diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index ef69a9405c..f0e4a21bc8 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -67,7 +67,7 @@ namespace dhcp { static const char* SERVER_DUID_FILE = "b10-dhcp6-serverid"; Dhcpv6Srv::Dhcpv6Srv(uint16_t port) - : alloc_engine_(), serverid_(), shutdown_(true) { + : alloc_engine_(), serverid_(), shutdown_(true), port_(port) { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_OPEN_SOCKET).arg(port); @@ -82,7 +82,7 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) LOG_ERROR(dhcp6_logger, DHCP6_NO_INTERFACES); return; } - IfaceMgr::instance().openSockets6(port); + IfaceMgr::instance().openSockets6(port_); } string duid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_DUID_FILE); diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index c7b1f0f8e9..4c45b01951 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -87,6 +87,19 @@ public: /// @brief Instructs the server to shut down. void shutdown(); + /// @brief Get UDP port on which server should listen. + /// + /// This accessor must be public because sockets are reopened from the + /// static configuration callback handler. This callback handler invokes + /// @c ControlledDhcpv4Srv::openActiveSockets which requires port parameter + /// which has to be retrieved from the @c ControlledDhcpv4Srv object. + /// They are retrieved using this public function. + /// + /// @return UDP port on which server should listen. + uint16_t getPort() const { + return (port_); + } + protected: /// @brief verifies if specified packet meets RFC requirements @@ -334,6 +347,8 @@ private: /// Indicates if shutdown is in progress. Setting it to true will /// initiate server shutdown procedure. volatile bool shutdown_; + + uint16_t port_; ///< UDP port number on which server listens. }; }; // namespace isc::dhcp diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index dfe0f36604..02093267d4 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -364,7 +364,7 @@ bool IfaceMgr::openSockets6(const uint16_t port) { if (iface->flag_loopback_ || !iface->flag_up_ || !iface->flag_running_, - !iface->inactive_) { + iface->inactive_) { continue; } From 4337b26b8553e2815c61e979a3c6442b38687f8c Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 14:05:29 +0200 Subject: [PATCH 07/17] [1555] Trivial: doxygen fix. --- src/bin/dhcp6/ctrl_dhcp6_srv.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index d1599f807c..e38219bb7b 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -136,7 +136,6 @@ protected: /// only. This function is not exception safe. /// /// @param port UDP port on which server should listen. - /// @param use_bcast should broadcast flags be set on the sockets. static void openActiveSockets(const uint16_t port); /// @brief IOService object, used for all ASIO operations. From abad8addef410474f0bb6bf6e15831902017e579 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 16:56:16 +0200 Subject: [PATCH 08/17] [1555] Set inactivity flag using pointer to an interface. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 9 +++++---- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 78cfb9b8a8..4ff7f37076 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -255,12 +255,13 @@ ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, const bool use_bcast // Get the reference to the collection of interfaces. This reference should be // valid as long as the program is run because IfaceMgr is a singleton. // Therefore we can safely iterate over instances of all interfaces and modify - // their flags. Here we modify flags which indicate wheter socket should be + // their flags. Here we modify flags which indicate whether socket should be // open for a particular interface or not. - IfaceMgr::IfaceCollection ifaces = IfaceMgr::instance().getIfaces(); - for (IfaceMgr::IfaceCollection::iterator iface = ifaces.begin(); + const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); + for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); iface != ifaces.end(); ++iface) { - iface->inactive_ = !CfgMgr::instance().isActiveIface(iface->getName()); + IfaceMgr::instance().getIface(iface->getName())->inactive_ = + !CfgMgr::instance().isActiveIface(iface->getName()); } // Let's reopen active sockets. openSockets4 will check internally whether // sockets are marked active or inactive. diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index cbd29b90fe..af473d1dbf 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -261,7 +261,8 @@ ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { IfaceMgr::IfaceCollection ifaces = IfaceMgr::instance().getIfaces(); for (IfaceMgr::IfaceCollection::iterator iface = ifaces.begin(); iface != ifaces.end(); ++iface) { - iface->inactive_ = !CfgMgr::instance().isActiveIface(iface->getName()); + IfaceMgr::instance().getIface(iface->getName())->inactive_ = + !CfgMgr::instance().isActiveIface(iface->getName()); } // Let's reopen active sockets. openSockets6 will check internally whether // sockets are marked active or inactive. From 98c85cb61b5bcbf948e1ea0923adcfcdca753cbb Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 17:02:01 +0200 Subject: [PATCH 09/17] [1555] Open only selected sockets when Kea starts up. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 5 +++++ src/bin/dhcp6/ctrl_dhcp6_srv.cc | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 4ff7f37076..ebe2455cb2 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -192,8 +192,13 @@ void ControlledDhcpv4Srv::establishSession() { try { configureDhcp4Server(*this, config_session_->getFullConfig()); + // Configuration may disable or enable interfaces so we have to + // reopen sockets according to new configuration. + openActiveSockets(getPort(), useBroadcast()); + } catch (const DhcpConfigError& ex) { LOG_ERROR(dhcp4_logger, DHCP4_CONFIG_LOAD_FAIL).arg(ex.what()); + } /// Integrate the asynchronous I/O model of BIND 10 configuration diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index af473d1dbf..0bd9c51320 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -193,8 +193,13 @@ void ControlledDhcpv6Srv::establishSession() { try { // Pull the full configuration out from the session. configureDhcp6Server(*this, config_session_->getFullConfig()); + // Configuration may disable or enable interfaces so we have to + // reopen sockets according to new configuration. + openActiveSockets(getPort()); + } catch (const DhcpConfigError& ex) { LOG_ERROR(dhcp6_logger, DHCP6_CONFIG_LOAD_FAIL).arg(ex.what()); + } /// Integrate the asynchronous I/O model of BIND 10 configuration From 39fa196b6b7eda249caa0501feb6d049cdfd9123 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 18:35:28 +0200 Subject: [PATCH 10/17] [1555] Log activation and deactivation of the interface. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 30 ++++++++++++++++++++++-------- src/bin/dhcp4/dhcp4_messages.mes | 10 ++++++++++ src/bin/dhcp6/ctrl_dhcp6_srv.cc | 21 +++++++++++++++++---- src/bin/dhcp6/dhcp6_messages.mes | 10 ++++++++++ 4 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index ebe2455cb2..7d7354eefe 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -254,19 +254,33 @@ ControlledDhcpv4Srv::execDhcpv4ServerCommand(const std::string& command_id, } void -ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, const bool use_bcast) { +ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, + const bool use_bcast) { IfaceMgr::instance().closeSockets(); - // Get the reference to the collection of interfaces. This reference should be - // valid as long as the program is run because IfaceMgr is a singleton. - // Therefore we can safely iterate over instances of all interfaces and modify - // their flags. Here we modify flags which indicate whether socket should be - // open for a particular interface or not. + // Get the reference to the collection of interfaces. This reference should + // be valid as long as the program is run because IfaceMgr is a singleton. + // Therefore we can safely iterate over instances of all interfaces and + // modify their flags. Here we modify flags which indicate whether socket + // should be open for a particular interface or not. const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); iface != ifaces.end(); ++iface) { - IfaceMgr::instance().getIface(iface->getName())->inactive_ = - !CfgMgr::instance().isActiveIface(iface->getName()); + Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); + if (CfgMgr::instance().isActiveIface(iface->getName())) { + iface_ptr->inactive_ = false; + LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE) + .arg(iface->getFullName()); + + } else { + // For deactivating interface, it should be sufficient to log it + // on the debug level because it is more useful to know what + // interface is activated which is logged on the info level. + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, + DHCP4_DEACTIVATE_INTERFACE).arg(iface->getName()); + iface_ptr->inactive_ = true; + + } } // Let's reopen active sockets. openSockets4 will check internally whether // sockets are marked active or inactive. diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 8b3e2552dd..0f0fd26369 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -14,6 +14,11 @@ $NAMESPACE isc::dhcp +% DHCP4_ACTIVATE_INTERFACE activating interface %1 +This message is printed when DHCPv4 server enabled an interface to be used +to receive DHCPv4 traffic. IPv4 socket on this interface will be opened once +Interface Manager starts up procedure of opening sockets. + % DHCP4_CCSESSION_STARTED control channel session started on socket %1 A debug message issued during startup after the IPv4 DHCP server has successfully established a session with the BIND 10 control channel. @@ -60,6 +65,11 @@ This informational message is printed every time DHCPv4 server is started and gives both the type and name of the database being used to store lease and other information. +% DHCP4_DEACTIVATE_INTERFACE deactivate interface %1 +This message is printed when DHCPv4 server disables an interface from being +used to receive DHCPv4 traffic. Sockets on this interface will not be opened +by the Interface Manager until interface is enabled. + % DHCP4_LEASE_ADVERT lease %1 advertised (client client-id %2, hwaddr %3) This debug message indicates that the server successfully advertised a lease. It is up to the client to choose one server out of othe advertised diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 0bd9c51320..60b873563d 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -263,11 +263,24 @@ ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { // Therefore we can safely iterate over instances of all interfaces and modify // their flags. Here we modify flags which indicate wheter socket should be // open for a particular interface or not. - IfaceMgr::IfaceCollection ifaces = IfaceMgr::instance().getIfaces(); - for (IfaceMgr::IfaceCollection::iterator iface = ifaces.begin(); + const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); + for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); iface != ifaces.end(); ++iface) { - IfaceMgr::instance().getIface(iface->getName())->inactive_ = - !CfgMgr::instance().isActiveIface(iface->getName()); + Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); + if (CfgMgr::instance().isActiveIface(iface->getName())) { + iface_ptr->inactive_ = false; + LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE) + .arg(iface->getFullName()); + + } else { + // For deactivating interface, it should be sufficient to log it + // on the debug level because it is more useful to know what + // interface is activated which is logged on the info level. + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, + DHCP6_DEACTIVATE_INTERFACE).arg(iface->getName()); + iface_ptr->inactive_ = true; + + } } // Let's reopen active sockets. openSockets6 will check internally whether // sockets are marked active or inactive. diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 6b61473dc7..5e0eff7f5c 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -14,6 +14,11 @@ $NAMESPACE isc::dhcp +% DHCP6_ACTIVATE_INTERFACE activating interface %1 +This message is printed when DHCPv6 server enabled an interface to be used +to receive DHCPv6 traffic. IPv6 socket on this interface will be opened once +Interface Manager starts up procedure of opening sockets. + % DHCP6_CCSESSION_STARTED control channel session started on socket %1 A debug message issued during startup after the IPv6 DHCP server has successfully established a session with the BIND 10 control channel. @@ -65,6 +70,11 @@ This informational message is printed every time the IPv6 DHCP server is started. It indicates what database backend type is being to store lease and other information. +% DHCP6_DEACTIVATE_INTERFACE deactivate interface %1 +This message is printed when DHCPv6 server disables an interface from being +used to receive DHCPv6 traffic. Sockets on this interface will not be opened +by the Interface Manager until interface is enabled. + % DHCP6_LEASE_ADVERT lease %1 advertised (client duid=%2, iaid=%3) This debug message indicates that the server successfully advertised a lease. It is up to the client to choose one server out of the From 75df517663d7743adfa1295e87f8a77ca97a059d Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 11 Jul 2013 19:15:42 +0200 Subject: [PATCH 11/17] [1555] Differentiate between activation of IPv4 and IPv6 sockets. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 4 ++-- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 4 ++-- src/lib/dhcp/iface_mgr.cc | 6 +++--- src/lib/dhcp/iface_mgr.h | 10 +++++++--- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 7d7354eefe..f0b45478a2 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -268,7 +268,7 @@ ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, iface != ifaces.end(); ++iface) { Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); if (CfgMgr::instance().isActiveIface(iface->getName())) { - iface_ptr->inactive_ = false; + iface_ptr->inactive4_ = false; LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE) .arg(iface->getFullName()); @@ -278,7 +278,7 @@ ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, // interface is activated which is logged on the info level. LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_DEACTIVATE_INTERFACE).arg(iface->getName()); - iface_ptr->inactive_ = true; + iface_ptr->inactive4_ = true; } } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 60b873563d..6d35c46253 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -268,7 +268,7 @@ ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { iface != ifaces.end(); ++iface) { Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); if (CfgMgr::instance().isActiveIface(iface->getName())) { - iface_ptr->inactive_ = false; + iface_ptr->inactive4_ = false; LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE) .arg(iface->getFullName()); @@ -278,7 +278,7 @@ ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { // interface is activated which is logged on the info level. LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_DEACTIVATE_INTERFACE).arg(iface->getName()); - iface_ptr->inactive_ = true; + iface_ptr->inactive6_ = true; } } diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 02093267d4..e47e8f06dd 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -52,7 +52,7 @@ Iface::Iface(const std::string& name, int ifindex) :name_(name), ifindex_(ifindex), mac_len_(0), hardware_type_(0), flag_loopback_(false), flag_up_(false), flag_running_(false), flag_multicast_(false), flag_broadcast_(false), flags_(0), - inactive_(false) + inactive4_(false), inactive6_(false) { memset(mac_, 0, sizeof(mac_)); } @@ -297,7 +297,7 @@ bool IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast) { if (iface->flag_loopback_ || !iface->flag_up_ || !iface->flag_running_, - iface->inactive_) { + iface->inactive4_) { continue; } @@ -364,7 +364,7 @@ bool IfaceMgr::openSockets6(const uint16_t port) { if (iface->flag_loopback_ || !iface->flag_up_ || !iface->flag_running_, - iface->inactive_) { + iface->inactive6_) { continue; } diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 442107db65..217524da0d 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -310,9 +310,13 @@ public: /// it may mean different things on different OSes). uint32_t flags_; - /// Interface is inactive. This can be explicitly set to prevent Interface - /// Manager from opening the socket on this interface. - bool inactive_; + /// Indicates that IPv4 sockets should (true) or should not (false) + /// be opened on this interface. + bool inactive4_; + + /// Indicates that IPv6 sockets should (true) or should not (false) + /// be opened on this interface. + bool inactive6_; }; /// @brief Handles network interfaces, transmission and reception. From 36815d6a2501933007ae6e8f823be08a64064058 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 12 Jul 2013 10:21:38 +0200 Subject: [PATCH 12/17] [1555] Updated bind10-guide with instructions to select active interfaces. --- doc/guide/bind10-guide.xml | 107 +++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index 88f2536a04..2f2a5255dc 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -3686,6 +3686,60 @@ Dhcp4/subnet4 [] list (default) +
+ Interface selection + + When DHCPv4 server starts up, by default it will listen to the DHCP + traffic and respond to it on all interfaces detected during startup. + However, in many cases it is desired to configure the server to listen and + respond on selected interfaces only. The sample commands in this section + show how to make interface selection using bindctl. + + + The default configuration can be presented with the following command: + +> config show Dhcp4/interfaces +Dhcp4/interfaces[0] "*" string + An asterisk sign plays a role of the wildcard and means "listen on all interfaces". + + + In order to override the default configuration, the existing entry can be replaced + with the actual interface name: + +> config set Dhcp4/interfaces[0] eth1 +> config commit + Other interface names can be added on one-by-one basis: + +> config add Dhcp4/interfaces eth2 +> config commit + Configuration will now contain two interfaces which can be presented as follows: + +> config show Dhcp4/interfaces +Dhcp4/interfaces[0] "eth1" string +Dhcp4/interfaces[1] "eth2" string + When configuration gets committed, the server will start to listen on + eth1 and eth2 interfaces only. + + + It is possible to use wildcard interface name (asterisk) concurrently with explicit + interface names: + +> config add Dhcp4/interfaces * +> config commit + This will result in the following configuration: + +> config show Dhcp4/interfaces +Dhcp4/interfaces[0] "eth1" string +Dhcp4/interfaces[1] "eth2" string +Dhcp4/interfaces[2] "*" string + The presence of the wildcard name implies that server will listen on all interfaces. + In order to fall back to the previous configuration when server listens on eth1 and eth2: + +> config remove Dhcp4/interfaces[2] +> config commit + +
+
Configuration of Address Pools @@ -4459,6 +4513,59 @@ Dhcp6/subnet6/ list
+
+ Interface selection + + When DHCPv6 server starts up, by default it will listen to the DHCP + traffic and respond to it on all interfaces detected during startup. + However, in many cases it is desired to configure the server to listen and + respond on selected interfaces only. The sample commands in this section + show how to make interface selection using bindctl. + + + The default configuration can be presented with the following command: + +> config show Dhcp6/interfaces +Dhcp6/interfaces[0] "*" string + An asterisk sign plays a role of the wildcard and means "listen on all interfaces". + + + In order to override the default configuration, the existing entry can be replaced + with the actual interface name: + +> config set Dhcp6/interfaces[0] eth1 +> config commit + Other interface names can be added on one-by-one basis: + +> config add Dhcp6/interfaces eth2 +> config commit + Configuration will now contain two interfaces which can be presented as follows: + +> config show Dhcp6/interfaces +Dhcp6/interfaces[0] "eth1" string +Dhcp6/interfaces[1] "eth2" string + When configuration gets committed, the server will start to listen on + eth1 and eth2 interfaces only. + + + It is possible to use wildcard interface name (asterisk) concurrently with explicit + interface names: + +> config add Dhcp6/interfaces * +> config commit + This will result in the following configuration: + +> config show Dhcp6/interfaces +Dhcp6/interfaces[0] "eth1" string +Dhcp6/interfaces[1] "eth2" string +Dhcp6/interfaces[2] "*" string + The presence of the wildcard name implies that server will listen on all interfaces. + In order to fall back to the previous configuration when server listens on eth1 and eth2: + +> config remove Dhcp6/interfaces[2] +> config commit + +
Subnet and Address Pool From 2221a1de37ddc5c35e52ad0b8b99d7dec430b00c Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 12 Jul 2013 13:29:52 +0200 Subject: [PATCH 13/17] [1555] Log warning if no interfaces are configured to listen DHCP traffic. --- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 6 ++++-- src/bin/dhcp4/dhcp4_messages.mes | 5 +++++ src/bin/dhcp6/ctrl_dhcp6_srv.cc | 4 +++- src/bin/dhcp6/dhcp6_messages.mes | 5 +++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index f0b45478a2..bf22a47ced 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -284,7 +284,9 @@ ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, } // Let's reopen active sockets. openSockets4 will check internally whether // sockets are marked active or inactive. - IfaceMgr::instance().openSockets4(port, use_bcast); + if (!IfaceMgr::instance().openSockets4(port, use_bcast)) { + LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN); + } } diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 0f0fd26369..bcf148b2d9 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -92,6 +92,11 @@ specified client after receiving a REQUEST message from it. There are many possible reasons for such a failure. Additional messages will indicate the reason. +% DHCP4_NO_SOCKETS_OPEN no interface configured to listen to DHCP traffic +This warning message is issued when current server configuration specifies +no interfaces that server should listen on, or specified interfaces are not +configured to receive the traffic. + % DHCP4_NOT_RUNNING IPv4 DHCP server is not running A warning message is issued when an attempt is made to shut down the IPv4 DHCP server but it is not running. diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 6d35c46253..81ffb0f136 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -284,7 +284,9 @@ ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { } // Let's reopen active sockets. openSockets6 will check internally whether // sockets are marked active or inactive. - IfaceMgr::instance().openSockets6(port); + if (!IfaceMgr::instance().openSockets6(port)) { + LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN); + } } }; diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 5e0eff7f5c..8630761994 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -111,6 +111,11 @@ IPv6 DHCP server but it is not running. During startup the IPv6 DHCP server failed to detect any network interfaces and is therefore shutting down. +% DHCP6_NO_SOCKETS_OPEN no interface configured to listen to DHCP traffic +This warning message is issued when current server configuration specifies +no interfaces that server should listen on, or specified interfaces are not +configured to receive the traffic. + % DHCP6_OPEN_SOCKET opening sockets on port %1 A debug message issued during startup, this indicates that the IPv6 DHCP server is about to open sockets on the specified port. From 87ba16e0b1db1a831e394650c7713b028a7d42c8 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 12 Jul 2013 15:30:46 +0200 Subject: [PATCH 14/17] [1555] Use asterisk as wildcard for all interfaces that server listens on. --- src/bin/dhcp4/ctrl_dhcp4_srv.h | 2 +- src/bin/dhcp4/dhcp4.spec | 4 +- src/bin/dhcp4/tests/config_parser_unittest.cc | 32 ++++++++-------- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 2 +- src/bin/dhcp6/dhcp6.spec | 4 +- src/bin/dhcp6/tests/config_parser_unittest.cc | 38 +++++++++---------- src/lib/dhcpsrv/dhcp_parsers.cc | 2 +- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 6 +-- 8 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index e5104b5d41..592b227dff 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -1,4 +1,4 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 063910c2e8..e940cb7cbf 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/src/bin/dhcp4/dhcp4.spec @@ -6,13 +6,13 @@ { "item_name": "interfaces", "item_type": "list", "item_optional": false, - "item_default": [ "all" ], + "item_default": [ "*" ], "list_item_spec": { "item_name": "interface_name", "item_type": "string", "item_optional": false, - "item_default": "all" + "item_default": "*" } } , diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 7c7caa278e..8a72589f3b 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -139,7 +139,7 @@ public: /// describing an option. std::string createConfigWithOption(const std::map& params) { std::ostringstream stream; - stream << "{ \"interfaces\": [ \"all\" ]," + stream << "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -246,7 +246,7 @@ public: void resetConfiguration() { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"valid-lifetime\": 4000, " @@ -323,7 +323,7 @@ TEST_F(Dhcp4ParserTest, emptySubnet) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, - Element::fromJSON("{ \"interfaces\": [ \"all\" ]," + Element::fromJSON("{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ ], " @@ -343,7 +343,7 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -373,7 +373,7 @@ TEST_F(Dhcp4ParserTest, subnetLocal) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -404,7 +404,7 @@ TEST_F(Dhcp4ParserTest, poolOutOfSubnet) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -428,7 +428,7 @@ TEST_F(Dhcp4ParserTest, poolPrefixLen) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -950,7 +950,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { // configuration does not include options configuration. TEST_F(Dhcp4ParserTest, optionDataDefaults) { ConstElementPtr x; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1023,7 +1023,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { // The definition is not required for the option that // belongs to the 'dhcp4' option space as it is the // standard option. - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1101,7 +1101,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // at the very end (when all other parameters are configured). // Starting stage 1. Configure sub-options and their definitions. - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1150,7 +1150,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // the configuration from the stage 2 is repeated because BIND // configuration manager sends whole configuration for the lists // where at least one element is being modified or added. - config = "{ \"interfaces\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1246,7 +1246,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // option setting. TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { ConstElementPtr x; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"option-data\": [ {" @@ -1318,7 +1318,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { // for multiple subnets. TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { ConstElementPtr x; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"subnet4\": [ { " @@ -1598,7 +1598,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { // In the first stahe we create definitions of suboptions // that we will add to the base option. // Let's create some dummy options: foo and foo2. - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1651,7 +1651,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { // We add our dummy options to this option space and thus // they should be included as sub-options in the 'vendor-opts' // option. - config = "{ \"interfaces\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1788,7 +1788,7 @@ TEST_F(Dhcp4ParserTest, allInterfaces) { // 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\" ]," + string config = "{ \"interfaces\": [ \"eth0\",\"*\",\"eth1\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"valid-lifetime\": 4000 }"; diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 81ffb0f136..4f6ed91a0e 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index 39e054996d..b61e811ddf 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -6,13 +6,13 @@ { "item_name": "interfaces", "item_type": "list", "item_optional": false, - "item_default": [ "all" ], + "item_default": [ "*" ], "list_item_spec": { "item_name": "interface_name", "item_type": "string", "item_optional": false, - "item_default": "all" + "item_default": "*" } } , diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 2f96462460..cda9066d1a 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -134,7 +134,7 @@ public: std::string>& params) { std::ostringstream stream; - stream << "{ \"interfaces\": [ \"all\" ]," + stream << "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -180,7 +180,7 @@ public: void resetConfiguration() { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -331,7 +331,7 @@ TEST_F(Dhcp6ParserTest, emptySubnet) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, - Element::fromJSON("{ \"interfaces\": [ \"all\" ]," + Element::fromJSON("{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -350,7 +350,7 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -384,7 +384,7 @@ TEST_F(Dhcp6ParserTest, subnetLocal) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -422,7 +422,7 @@ TEST_F(Dhcp6ParserTest, subnetInterface) { // There should be at least one interface - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -455,7 +455,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceBogus) { // There should be at least one interface - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -486,7 +486,7 @@ TEST_F(Dhcp6ParserTest, interfaceGlobal) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -556,7 +556,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) { // parameter. TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { - const string config = "{ \"interfaces\": [ \"all\" ]," + const string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -611,7 +611,7 @@ TEST_F(Dhcp6ParserTest, poolOutOfSubnet) { ConstElementPtr status; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -639,7 +639,7 @@ TEST_F(Dhcp6ParserTest, poolPrefixLen) { ConstElementPtr x; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1159,7 +1159,7 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { // configuration does not include options configuration. TEST_F(Dhcp6ParserTest, optionDataDefaults) { ConstElementPtr x; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," @@ -1241,7 +1241,7 @@ TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) { // The definition is not required for the option that // belongs to the 'dhcp6' option space as it is the // standard option. - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1319,7 +1319,7 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // at the very end (when all other parameters are configured). // Starting stage 1. Configure sub-options and their definitions. - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1368,7 +1368,7 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // the configuration from the stage 2 is repeated because BIND // configuration manager sends whole configuration for the lists // where at least one element is being modified or added. - config = "{ \"interfaces\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1462,7 +1462,7 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // for multiple subnets. TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { ConstElementPtr x; - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -1705,7 +1705,7 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) { // In the first stahe we create definitions of suboptions // that we will add to the base option. // Let's create some dummy options: foo and foo2. - string config = "{ \"interfaces\": [ \"all\" ]," + string config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1758,7 +1758,7 @@ TEST_F(Dhcp6ParserTest, stdOptionDataEncapsulate) { // We add our dummy options to this option space and thus // they should be included as sub-options in the 'vendor-opts' // option. - config = "{ \"interfaces\": [ \"all\" ]," + config = "{ \"interfaces\": [ \"*\" ]," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -1906,7 +1906,7 @@ TEST_F(Dhcp6ParserTest, allInterfaces) { // bu 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\", \"eth1\", \"all\" ]," + string config = "{ \"interfaces\": [ \"eth0\", \"eth1\", \"*\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index 04cf508ebc..52c226b6d2 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -34,7 +34,7 @@ namespace isc { namespace dhcp { namespace { -const char* ALL_IFACES_KEYWORD = "all"; +const char* ALL_IFACES_KEYWORD = "*"; } // *********************** ParserContext ************************* diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index cde227acee..6704c57f91 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -200,8 +200,8 @@ TEST_F(DhcpParserTest, uint32ParserTest) { /// 1. Does not allow empty for storage. /// 2. Does not allow name other than "interfaces" /// 3. Parses list of interfaces and adds them to CfgMgr -/// 4. Parses 'all' keyword and sets a CfgMgr flag which indicates that -/// server will listen on all interfaces. +/// 4. Parses wildcard interface name and sets a CfgMgr flag which indicates +/// that server will listen on all interfaces. TEST_F(DhcpParserTest, interfaceListParserTest) { const std::string name = "interfaces"; @@ -234,7 +234,7 @@ TEST_F(DhcpParserTest, interfaceListParserTest) { // 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")); + list_element->add(Element::create("*")); // Reset parser's state. parser.reset(new InterfaceListConfigParser(name)); From 0d4abba272e2e77132b374a657aa96f65505f26c Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 19 Jul 2013 12:45:00 +0200 Subject: [PATCH 15/17] [1555] Address review comments. --- doc/guide/bind10-guide.xml | 4 +- src/bin/dhcp4/ctrl_dhcp4_srv.cc | 37 ------------------- src/bin/dhcp4/ctrl_dhcp4_srv.h | 10 ----- src/bin/dhcp4/dhcp4_srv.cc | 37 +++++++++++++++++++ src/bin/dhcp4/dhcp4_srv.h | 14 +++++++ src/bin/dhcp4/tests/config_parser_unittest.cc | 4 +- src/bin/dhcp6/ctrl_dhcp6_srv.cc | 35 ------------------ src/bin/dhcp6/ctrl_dhcp6_srv.h | 10 ----- src/bin/dhcp6/dhcp6_srv.cc | 35 ++++++++++++++++++ src/bin/dhcp6/dhcp6_srv.h | 13 +++++++ src/lib/dhcp/iface_mgr.cc | 4 +- src/lib/dhcpsrv/cfgmgr.h | 25 ++++++++----- 12 files changed, 121 insertions(+), 107 deletions(-) diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index 2f2a5255dc..432fb8d2d7 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -3599,7 +3599,7 @@ $ will be available. It will look similar to this: > config show Dhcp4 -Dhcp4/interface/ list (default) +Dhcp4/interfaces/ list (default) Dhcp4/renew-timer 1000 integer (default) Dhcp4/rebind-timer 2000 integer (default) Dhcp4/valid-lifetime 4000 integer (default) @@ -4420,7 +4420,7 @@ Dhcp4/renew-timer 1000 integer (default) will be available. It will look similar to this: > config show Dhcp6 -Dhcp6/interface/ list (default) +Dhcp6/interfaces/ list (default) Dhcp6/renew-timer 1000 integer (default) Dhcp6/rebind-timer 2000 integer (default) Dhcp6/preferred-lifetime 3000 integer (default) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index bf22a47ced..46a7ca0c2b 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -253,42 +253,5 @@ ControlledDhcpv4Srv::execDhcpv4ServerCommand(const std::string& command_id, } } -void -ControlledDhcpv4Srv::openActiveSockets(const uint16_t port, - const bool use_bcast) { - IfaceMgr::instance().closeSockets(); - - // Get the reference to the collection of interfaces. This reference should - // be valid as long as the program is run because IfaceMgr is a singleton. - // Therefore we can safely iterate over instances of all interfaces and - // modify their flags. Here we modify flags which indicate whether socket - // should be open for a particular interface or not. - const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); - for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); - iface != ifaces.end(); ++iface) { - Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); - if (CfgMgr::instance().isActiveIface(iface->getName())) { - iface_ptr->inactive4_ = false; - LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE) - .arg(iface->getFullName()); - - } else { - // For deactivating interface, it should be sufficient to log it - // on the debug level because it is more useful to know what - // interface is activated which is logged on the info level. - LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, - DHCP4_DEACTIVATE_INTERFACE).arg(iface->getName()); - iface_ptr->inactive4_ = true; - - } - } - // Let's reopen active sockets. openSockets4 will check internally whether - // sockets are marked active or inactive. - if (!IfaceMgr::instance().openSockets4(port, use_bcast)) { - LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN); - } -} - - }; }; diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 592b227dff..526d98753c 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -130,16 +130,6 @@ protected: /// when there is a new command or configuration sent over msgq. static void sessionReader(void); - /// @brief Open sockets which are marked as active in @c CfgMgr. - /// - /// This function reopens sockets according to the current settings in the - /// Configuration Manager. It holds the list of the interfaces which server - /// should listen on. This function will open sockets on these interfaces - /// only. This function is not exception safe. - /// - /// @param port UDP port on which server should listen. - /// @param use_bcast should broadcast flags be set on the sockets. - static void openActiveSockets(const uint16_t port, const bool use_bcast); /// @brief IOService object, used for all ASIO operations. isc::asiolink::IOService io_service_; diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index f414e5ae93..bc5a4d6371 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -821,5 +821,42 @@ Dhcpv4Srv::sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid) { } } +void +Dhcpv4Srv::openActiveSockets(const uint16_t port, + const bool use_bcast) { + IfaceMgr::instance().closeSockets(); + + // Get the reference to the collection of interfaces. This reference should + // be valid as long as the program is run because IfaceMgr is a singleton. + // Therefore we can safely iterate over instances of all interfaces and + // modify their flags. Here we modify flags which indicate whether socket + // should be open for a particular interface or not. + const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); + for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); + iface != ifaces.end(); ++iface) { + Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); + if (CfgMgr::instance().isActiveIface(iface->getName())) { + iface_ptr->inactive4_ = false; + LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE) + .arg(iface->getFullName()); + + } else { + // For deactivating interface, it should be sufficient to log it + // on the debug level because it is more useful to know what + // interface is activated which is logged on the info level. + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, + DHCP4_DEACTIVATE_INTERFACE).arg(iface->getName()); + iface_ptr->inactive4_ = true; + + } + } + // Let's reopen active sockets. openSockets4 will check internally whether + // sockets are marked active or inactive. + if (!IfaceMgr::instance().openSockets4(port, use_bcast)) { + LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN); + } +} + + } // namespace dhcp } // namespace isc diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index 59bb95c215..4160486377 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -125,6 +125,9 @@ public: /// /// @brief Get UDP port on which server should listen. /// + /// Typically, server listens on UDP port number 67. Other ports are used + /// for testing purposes only. + /// /// @return UDP port on which server should listen. uint16_t getPort() const { return (port_); @@ -139,6 +142,17 @@ public: } //@} + /// @brief Open sockets which are marked as active in @c CfgMgr. + /// + /// This function reopens sockets according to the current settings in the + /// Configuration Manager. It holds the list of the interfaces which server + /// should listen on. This function will open sockets on these interfaces + /// only. This function is not exception safe. + /// + /// @param port UDP port on which server should listen. + /// @param use_bcast should broadcast flags be set on the sockets. + static void openActiveSockets(const uint16_t port, const bool use_bcast); + protected: /// @brief verifies if specified packet meets RFC requirements diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 8a72589f3b..05c951d9f9 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1785,10 +1785,10 @@ TEST_F(Dhcp4ParserTest, selectedInterfaces) { 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 + // but it also includes asterisk. The asterisk 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\",\"*\",\"eth1\" ]," + string config = "{ \"interfaces\": [ \"eth0\", \"*\", \"eth1\" ]," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"valid-lifetime\": 4000 }"; diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 4f6ed91a0e..dffac6156f 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -254,40 +254,5 @@ ControlledDhcpv6Srv::execDhcpv6ServerCommand(const std::string& command_id, } } -void -ControlledDhcpv6Srv::openActiveSockets(const uint16_t port) { - IfaceMgr::instance().closeSockets(); - - // Get the reference to the collection of interfaces. This reference should be - // valid as long as the program is run because IfaceMgr is a singleton. - // Therefore we can safely iterate over instances of all interfaces and modify - // their flags. Here we modify flags which indicate wheter socket should be - // open for a particular interface or not. - const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); - for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); - iface != ifaces.end(); ++iface) { - Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); - if (CfgMgr::instance().isActiveIface(iface->getName())) { - iface_ptr->inactive4_ = false; - LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE) - .arg(iface->getFullName()); - - } else { - // For deactivating interface, it should be sufficient to log it - // on the debug level because it is more useful to know what - // interface is activated which is logged on the info level. - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, - DHCP6_DEACTIVATE_INTERFACE).arg(iface->getName()); - iface_ptr->inactive6_ = true; - - } - } - // Let's reopen active sockets. openSockets6 will check internally whether - // sockets are marked active or inactive. - if (!IfaceMgr::instance().openSockets6(port)) { - LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN); - } -} - }; }; diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index e38219bb7b..ffd43c3a2e 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -128,16 +128,6 @@ protected: /// when there is a new command or configuration sent over msgq. static void sessionReader(void); - /// @brief Open sockets which are marked as active in @c CfgMgr. - /// - /// This function reopens sockets according to the current settings in the - /// Configuration Manager. It holds the list of the interfaces which server - /// should listen on. This function will open sockets on these interfaces - /// only. This function is not exception safe. - /// - /// @param port UDP port on which server should listen. - static void openActiveSockets(const uint16_t port); - /// @brief IOService object, used for all ASIO operations. isc::asiolink::IOService io_service_; diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index f0e4a21bc8..30eff1e2fb 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1103,5 +1103,40 @@ Dhcpv6Srv::processInfRequest(const Pkt6Ptr& infRequest) { return reply; } +void +Dhcpv6Srv::openActiveSockets(const uint16_t port) { + IfaceMgr::instance().closeSockets(); + + // Get the reference to the collection of interfaces. This reference should be + // valid as long as the program is run because IfaceMgr is a singleton. + // Therefore we can safely iterate over instances of all interfaces and modify + // their flags. Here we modify flags which indicate wheter socket should be + // open for a particular interface or not. + const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); + for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); + iface != ifaces.end(); ++iface) { + Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); + if (CfgMgr::instance().isActiveIface(iface->getName())) { + iface_ptr->inactive4_ = false; + LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE) + .arg(iface->getFullName()); + + } else { + // For deactivating interface, it should be sufficient to log it + // on the debug level because it is more useful to know what + // interface is activated which is logged on the info level. + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, + DHCP6_DEACTIVATE_INTERFACE).arg(iface->getName()); + iface_ptr->inactive6_ = true; + + } + } + // Let's reopen active sockets. openSockets6 will check internally whether + // sockets are marked active or inactive. + if (!IfaceMgr::instance().openSockets6(port)) { + LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN); + } +} + }; }; diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index 4c45b01951..9674234008 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -89,6 +89,9 @@ public: /// @brief Get UDP port on which server should listen. /// + /// Typically, server listens on UDP port 547. Other ports are only + /// used for testing purposes. + /// /// This accessor must be public because sockets are reopened from the /// static configuration callback handler. This callback handler invokes /// @c ControlledDhcpv4Srv::openActiveSockets which requires port parameter @@ -100,6 +103,16 @@ public: return (port_); } + /// @brief Open sockets which are marked as active in @c CfgMgr. + /// + /// This function reopens sockets according to the current settings in the + /// Configuration Manager. It holds the list of the interfaces which server + /// should listen on. This function will open sockets on these interfaces + /// only. This function is not exception safe. + /// + /// @param port UDP port on which server should listen. + static void openActiveSockets(const uint16_t port); + protected: /// @brief verifies if specified packet meets RFC requirements diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index e47e8f06dd..128aafeb01 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -296,7 +296,7 @@ bool IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast) { if (iface->flag_loopback_ || !iface->flag_up_ || - !iface->flag_running_, + !iface->flag_running_ || iface->inactive4_) { continue; } @@ -363,7 +363,7 @@ bool IfaceMgr::openSockets6(const uint16_t port) { if (iface->flag_loopback_ || !iface->flag_up_ || - !iface->flag_running_, + !iface->flag_running_ || iface->inactive6_) { continue; } diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 5f82aaefb7..d7d4c676e4 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -254,23 +254,30 @@ public: /// @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. + /// @brief Sets the flag which indicates that server is supposed to listen + /// on all available interfaces. /// - /// This function configrues the server to listen on all available - /// interfaces regardless of the interfaces specified with - /// @c CfgMgr::addListeningInterface. + /// This function does not close or open sockets. It simply marks that + /// server should start to listen on all interfaces the next time sockets + /// are reopened. Server should examine this flag when it gets reconfigured + /// and configuration changes the interfaces that server should listen on. void activateAllIfaces(); - /// @brief Clear the collection of the interfaces that server is configured - /// to use to listen for incoming requests. + /// @brief Clear the collection of the interfaces that server should listen + /// on. /// /// 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. + /// interfaces if it has been enabled using + /// @c CfgMgr::activateAllInterfaces. + /// Likewise @c CfgMgr::activateAllIfaces, this function does not close or + /// open sockets. It marks all interfaces inactive for DHCP traffic. + /// Server should examine this new setting when it attempts to + /// reopen sockets (as a result of reconfiguration). void deleteActiveIfaces(); - /// @brief Check if server is configured to listen on the interface which - /// name is specified as an argument to this function. + /// @brief Check if specified interface should be used to listen to DHCP + /// traffic. /// /// @param iface A name of the interface to be checked. /// From 6b65ea8a618d32efcdc0697cf0acc534b6f13404 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 19 Jul 2013 13:14:02 +0200 Subject: [PATCH 16/17] [1555] Minor: added a todo comment. --- src/bin/dhcp4/dhcp4_srv.cc | 2 ++ src/bin/dhcp6/dhcp6_srv.cc | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index bc5a4d6371..a9f0a5099e 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -852,6 +852,8 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port, } // Let's reopen active sockets. openSockets4 will check internally whether // sockets are marked active or inactive. + // @todo Optimization: we should not reopen all sockets but rather select + // those that have been affected by the new configuration. if (!IfaceMgr::instance().openSockets4(port, use_bcast)) { LOG_WARN(dhcp4_logger, DHCP4_NO_SOCKETS_OPEN); } diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 30eff1e2fb..f6991ec34f 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1133,6 +1133,8 @@ Dhcpv6Srv::openActiveSockets(const uint16_t port) { } // Let's reopen active sockets. openSockets6 will check internally whether // sockets are marked active or inactive. + // @todo Optimization: we should not reopen all sockets but rather select + // those that have been affected by the new configuration. if (!IfaceMgr::instance().openSockets6(port)) { LOG_WARN(dhcp6_logger, DHCP6_NO_SOCKETS_OPEN); } From 4084c5f2a9a24009d645def53697b139bf989124 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 19 Jul 2013 16:35:36 +0200 Subject: [PATCH 17/17] [1555] Throw exception if the interface instance is NULL. --- src/bin/dhcp4/dhcp4_srv.cc | 5 +++++ src/bin/dhcp6/dhcp6_srv.cc | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index a9f0a5099e..bf877b98cd 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -835,6 +835,11 @@ Dhcpv4Srv::openActiveSockets(const uint16_t port, for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); iface != ifaces.end(); ++iface) { Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); + if (iface_ptr == NULL) { + isc_throw(isc::Unexpected, "Interface Manager returned NULL" + << " instance of the interface when DHCPv4 server was" + << " trying to reopen sockets after reconfiguration"); + } if (CfgMgr::instance().isActiveIface(iface->getName())) { iface_ptr->inactive4_ = false; LOG_INFO(dhcp4_logger, DHCP4_ACTIVATE_INTERFACE) diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index f6991ec34f..70154458af 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1116,6 +1116,11 @@ Dhcpv6Srv::openActiveSockets(const uint16_t port) { for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin(); iface != ifaces.end(); ++iface) { Iface* iface_ptr = IfaceMgr::instance().getIface(iface->getName()); + if (iface_ptr == NULL) { + isc_throw(isc::Unexpected, "Interface Manager returned NULL" + << " instance of the interface when DHCPv6 server was" + << " trying to reopen sockets after reconfiguration"); + } if (CfgMgr::instance().isActiveIface(iface->getName())) { iface_ptr->inactive4_ = false; LOG_INFO(dhcp6_logger, DHCP6_ACTIVATE_INTERFACE)