From: Tomek Mrugalski Date: Tue, 27 Dec 2016 17:58:22 +0000 (+0100) Subject: [5020] Interface parser migrated to SimpleParser X-Git-Tag: trac5020fd_base~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=600236eea38eb7569a5719ec98b085ee7acccf9e;p=thirdparty%2Fkea.git [5020] Interface parser migrated to SimpleParser --- diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index bcfad010e8..36c4049275 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -420,10 +420,9 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, (config_id.compare("dhcp4o6-port") == 0) ) { parser = new Uint32Parser(config_id, globalContext()->uint32_values_); - } else if (config_id.compare("interfaces-config") == 0) { - parser = new IfacesConfigParser4(); } else if (config_id.compare("subnet4") == 0) { parser = new Subnets4ListConfigParser(config_id); + // interface-config has been migrated to SimpleParser already. // option-data and option-def have been converted to SimpleParser already. } else if ((config_id.compare("next-server") == 0)) { parser = new StringParser(config_id, @@ -583,6 +582,11 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { const std::map& values_map = mutable_cfg->mapValue(); BOOST_FOREACH(config_pair, values_map) { + // In principle we could have the following code structured as a series + // of long if else if clauses. That would give a marginal performance + // boost, but would make the code less readable. We had serious issues + // with the parser code debuggability, so I decided to keep it as a + // series of independent ifs. if (config_pair.first == "option-def") { // This is converted to SimpleParser and is handled already above. @@ -596,6 +600,14 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { continue; } + if (config_pair.first == "interfaces-config") { + IfacesConfigParser parser(AF_INET); + CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); + parser.parse(cfg_iface, config_pair.second); + continue; + } + + // Legacy DhcpConfigParser stuff below ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED) @@ -604,11 +616,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { subnet_parser = parser; } else if (config_pair.first == "lease-database") { leases_parser = parser; - } else if (config_pair.first == "interfaces-config") { - // 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 if (config_pair.first == "hooks-libraries") { // Executing commit will alter currently-loaded hooks // libraries. Check if the supplied libraries are valid, diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 98e1c081b0..5d55658299 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -701,12 +701,11 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, (config_id.compare("dhcp4o6-port") == 0) ) { parser = new Uint32Parser(config_id, globalContext()->uint32_values_); - } else if (config_id.compare("interfaces-config") == 0) { - parser = new IfacesConfigParser6(); } else if (config_id.compare("subnet6") == 0) { parser = new Subnets6ListConfigParser(config_id); // option-data and option-def are no longer needed here. They're now - // converted to SimpleParser and are handled in configureDhcp6Server + // converted to SimpleParser and are handled in configureDhcp6Server. + // interfaces-config has been converted to SimpleParser. } else if (config_id.compare("version") == 0) { parser = new StringParser(config_id, globalContext()->string_values_); @@ -857,6 +856,11 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { } BOOST_FOREACH(config_pair, values_map) { + // In principle we could have the following code structured as a series + // of long if else if clauses. That would give a marginal performance + // boost, but would make the code less readable. We had serious issues + // with the parser code debuggability, so I decided to keep it as a + // series of independent ifs. if (config_pair.first == "option-def") { // This is converted to SimpleParser and is handled already above. @@ -870,6 +874,13 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { continue; } + if (config_pair.first == "interfaces-config") { + IfacesConfigParser parser(AF_INET6); + CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); + parser.parse(cfg_iface, config_pair.second); + continue; + } + ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED) diff --git a/src/lib/dhcpsrv/parsers/ifaces_config_parser.cc b/src/lib/dhcpsrv/parsers/ifaces_config_parser.cc index b1c9d32f44..212f4a837c 100644 --- a/src/lib/dhcpsrv/parsers/ifaces_config_parser.cc +++ b/src/lib/dhcpsrv/parsers/ifaces_config_parser.cc @@ -18,82 +18,52 @@ using namespace isc::data; namespace isc { namespace dhcp { -InterfaceListConfigParser::InterfaceListConfigParser(const uint16_t protocol) - : protocol_(protocol) { -} - void -InterfaceListConfigParser::build(ConstElementPtr value) { - CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); - - BOOST_FOREACH(ConstElementPtr iface, value->listValue()) { +IfacesConfigParser::parseInterfacesList(const CfgIfacePtr& cfg_iface, + ConstElementPtr ifaces_list) { + BOOST_FOREACH(ConstElementPtr iface, ifaces_list->listValue()) { std::string iface_name = iface->stringValue(); try { cfg_iface->use(protocol_, iface_name); } catch (const std::exception& ex) { isc_throw(DhcpConfigError, "Failed to select interface: " - << ex.what() << " (" << value->getPosition() << ")"); + << ex.what() << " (" << iface->getPosition() << ")"); } } } -void -InterfaceListConfigParser::commit() { - // Nothing to do. -} - IfacesConfigParser::IfacesConfigParser(const uint16_t protocol) : protocol_(protocol) { } void -IfacesConfigParser::build(isc::data::ConstElementPtr ifaces_config) { - BOOST_FOREACH(ConfigPair element, ifaces_config->mapValue()) { - try { - if (element.first == "interfaces") { - InterfaceListConfigParser parser(protocol_); - parser.build(element.second); - - } - - } catch (const std::exception& ex) { - // Append line number where the error occurred. - isc_throw(DhcpConfigError, ex.what() << " (" - << element.second->getPosition() << ")"); - } - } -} - -bool -IfacesConfigParser::isGenericParameter(const std::string& parameter) const { - // Currently, the "interfaces" is the only common parameter for - // DHCPv4 and DHCPv6. - return (parameter == "interfaces"); -} - -IfacesConfigParser4::IfacesConfigParser4() - : IfacesConfigParser(AF_INET) { -} - -void -IfacesConfigParser4::build(isc::data::ConstElementPtr ifaces_config) { - IfacesConfigParser::build(ifaces_config); +IfacesConfigParser::parse(const CfgIfacePtr& cfg, + const isc::data::ConstElementPtr& ifaces_config) { // Get the pointer to the interface configuration. - CfgIfacePtr cfg = CfgMgr::instance().getStagingCfg()->getCfgIface(); bool socket_type_specified = false; BOOST_FOREACH(ConfigPair element, ifaces_config->mapValue()) { try { - if (element.first == "dhcp-socket-type") { - cfg->useSocketType(AF_INET, element.second->stringValue()); - socket_type_specified = true; + if (element.first == "interfaces") { + parseInterfacesList(cfg, element.second); + continue; + + } - } else if (!isGenericParameter(element.first)) { - isc_throw(DhcpConfigError, "usupported parameter '" - << element.first << "'"); + if (element.first == "dhcp-socket-type") { + if (protocol_ == AF_INET) { + cfg->useSocketType(AF_INET, element.second->stringValue()); + socket_type_specified = true; + continue; + } else { + isc_throw(DhcpConfigError, + "dhcp-socket-type is not supported in DHCPv6"); + } } + isc_throw(DhcpConfigError, "usupported parameter '" + << element.first << "'"); } catch (const std::exception& ex) { // Append line number where the error occurred. isc_throw(DhcpConfigError, ex.what() << " (" @@ -109,28 +79,5 @@ IfacesConfigParser4::build(isc::data::ConstElementPtr ifaces_config) { } } -IfacesConfigParser6::IfacesConfigParser6() - : IfacesConfigParser(AF_INET6) { -} - -void -IfacesConfigParser6::build(isc::data::ConstElementPtr ifaces_config) { - IfacesConfigParser::build(ifaces_config); - - BOOST_FOREACH(ConfigPair element, ifaces_config->mapValue()) { - try { - if (!isGenericParameter(element.first)) { - isc_throw(DhcpConfigError, "usupported parameter '" - << element.first << "'"); - } - - } catch (const std::exception& ex) { - // Append line number where the error occurred. - isc_throw(DhcpConfigError, ex.what() << " (" - << element.second->getPosition() << ")"); - } - } -} - } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/parsers/ifaces_config_parser.h b/src/lib/dhcpsrv/parsers/ifaces_config_parser.h index e4e26430e6..9748b9c9d6 100644 --- a/src/lib/dhcpsrv/parsers/ifaces_config_parser.h +++ b/src/lib/dhcpsrv/parsers/ifaces_config_parser.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -8,65 +8,22 @@ #define IFACES_CONFIG_PARSER_H #include -#include +#include +#include #include namespace isc { namespace dhcp { -/// @brief Parser for interface list definition. -/// -/// This parser handles Dhcp4/interfaces-config/interfaces and -/// Dhcp6/interfaces-config/interfaces entries. -/// It contains a list of network interfaces that the server listens on. -/// In particular, it can contain an "*" that designates all interfaces. -class InterfaceListConfigParser : public DhcpConfigParser { -public: - - /// @brief Constructor - /// - /// @param protocol AF_INET for DHCPv4 and AF_INET6 for DHCPv6. - InterfaceListConfigParser(const uint16_t protocol); - - /// @brief Parses a list of interface names. - /// - /// This method parses a list of interface/address tuples in a text - /// format. The tuples specify the IP addresses and corresponding - /// interface names on which the server should listen to the DHCP - /// messages. The address is optional in each tuple and, if not - /// specified, the interface name (without slash character) should - /// be present. - /// - /// @param value pointer to the content of parsed values - /// - /// @throw DhcpConfigError if the interface names and/or addresses - /// are invalid. - virtual void build(isc::data::ConstElementPtr value); - - /// @brief Does nothing. - virtual void commit(); - -private: - - /// @brief AF_INET for DHCPv4 and AF_INET6 for DHCPv6. - uint16_t protocol_; - -}; - - /// @brief Parser for the configuration of interfaces. /// /// This parser parses the "interfaces-config" parameter which holds the /// full configuration of the DHCP server with respect to the use of /// interfaces, sockets and alike. /// -/// This parser uses the @c InterfaceListConfigParser to parse the -/// list of interfaces on which the server should listen. It handles -/// remaining parameters internally. -/// -/// This parser is used as a base for the DHCPv4 and DHCPv6 specific -/// parsers and should not be used directly. -class IfacesConfigParser : public DhcpConfigParser { +/// This parser is used in both DHCPv4 and DHCPv6. Derived parsers +/// are not needed. +class IfacesConfigParser : public isc::data::SimpleParser { public: /// @brief Constructor @@ -74,81 +31,30 @@ public: /// @param protocol AF_INET for DHCPv4 and AF_INET6 for DHCPv6. IfacesConfigParser(const uint16_t protocol); - /// @brief Parses generic parameters in "interfaces-config". + /// @brief Parses content of the "interfaces-config". /// - /// The generic parameters in the "interfaces-config" map are - /// the ones that are common for DHCPv4 and DHCPv6. + /// @param config parsed structures will be stored here + /// @param values pointer to the content of parsed values /// - /// @param ifaces_config A data element holding configuration of - /// interfaces. - virtual void build(isc::data::ConstElementPtr ifaces_config); - - /// @brief Commit, unused. - virtual void commit() { } + /// @throw DhcpConfigError if the interface names and/or addresses + /// are invalid. + void parse(const CfgIfacePtr& config, const isc::data::ConstElementPtr& values); - /// @brief Checks if the specified parameter is a common parameter - /// for DHCPv4 and DHCPv6 interface configuration. - /// - /// This method is invoked by the derived classes to check if the - /// particular parameter is supported. +private: + /// @brief parses interfaces-list structure /// - /// @param parameter A name of the parameter. + /// This method goes through all the interfaces-specified in + /// 'interfaces-list' and enabled them in the specified configuration + /// structure /// - /// @return true if the specified parameter is a common parameter - /// for DHCPv4 and DHCPv6 server. - bool isGenericParameter(const std::string& parameter) const; - -private: + /// @param cfg_iface parsed interfaces will be specified here + /// @param ifaces_list interfaces-list to be parsed + /// @throw DhcpConfigError if the interface names are invalid. + void parseInterfacesList(const CfgIfacePtr& cfg_iface, + isc::data::ConstElementPtr ifaces_list); /// @brief AF_INET for DHCPv4 and AF_INET6 for DHCPv6. int protocol_; - -}; - - -/// @brief Parser for the "interfaces-config" parameter of the DHCPv4 server. -class IfacesConfigParser4 : public IfacesConfigParser { -public: - - /// @brief Constructor. - /// - /// Sets the protocol to AF_INET. - IfacesConfigParser4(); - - /// @brief Parses DHCPv4 specific parameters. - /// - /// Internally it invokes the @c InterfaceConfigParser::build to parse - /// generic parameters. In addition, it parses the following parameters: - /// - dhcp-socket-type - /// - /// @param ifaces_config A data element holding configuration of - /// interfaces. - /// - /// @throw DhcpConfigError if unsupported parameters is specified. - virtual void build(isc::data::ConstElementPtr ifaces_config); - -}; - -/// @brief Parser for the "interfaces-config" parameter of the DHCPv4 server. -class IfacesConfigParser6 : public IfacesConfigParser { -public: - - /// @brief Constructor. - /// - /// Sets the protocol to AF_INET6. - IfacesConfigParser6(); - - /// @brief Parses DHCPv6 specific parameters. - /// - /// Internally it invokes the @c InterfaceConfigParser::build to parse - /// generic parameters. Currently it doesn't parse any other parameters. - /// - /// @param ifaces_config A data element holding configuration of - /// interfaces. - /// - /// @throw DhcpConfigError if unsupported parameters is specified. - virtual void build(isc::data::ConstElementPtr ifaces_config); - }; } diff --git a/src/lib/dhcpsrv/tests/ifaces_config_parser_unittest.cc b/src/lib/dhcpsrv/tests/ifaces_config_parser_unittest.cc index 67eed945ca..9c88b27f31 100644 --- a/src/lib/dhcpsrv/tests/ifaces_config_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/ifaces_config_parser_unittest.cc @@ -56,8 +56,9 @@ TEST_F(IfacesConfigParserTest, interfaces) { ElementPtr config_element = Element::fromJSON(config); // Parse the configuration. - IfacesConfigParser4 parser; - ASSERT_NO_THROW(parser.build(config_element)); + IfacesConfigParser parser(AF_INET); + CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); + ASSERT_NO_THROW(parser.parse(cfg_iface, config_element)); // Open sockets according to the parsed configuration. SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg(); @@ -77,7 +78,8 @@ TEST_F(IfacesConfigParserTest, interfaces) { config = "{ \"interfaces\": [ \"eth0\", \"*\" ] }"; config_element = Element::fromJSON(config); - ASSERT_NO_THROW(parser.build(config_element)); + cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); + ASSERT_NO_THROW(parser.parse(cfg_iface, config_element)); cfg = CfgMgr::instance().getStagingCfg(); ASSERT_NO_THROW(cfg->getCfgIface()->openSockets(AF_INET, 10000)); @@ -100,8 +102,9 @@ TEST_F(IfacesConfigParserTest, socketTypeRaw) { ElementPtr config_element = Element::fromJSON(config); // Parse the configuration. - IfacesConfigParser4 parser; - ASSERT_NO_THROW(parser.build(config_element)); + IfacesConfigParser parser(AF_INET); + CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); + ASSERT_NO_THROW(parser.parse(cfg_iface, config_element)); // Compare the resulting configuration with a reference // configuration using the raw socket. @@ -125,8 +128,9 @@ TEST_F(IfacesConfigParserTest, socketTypeDatagram) { ElementPtr config_element = Element::fromJSON(config); // Parse the configuration. - IfacesConfigParser4 parser; - ASSERT_NO_THROW(parser.build(config_element)); + IfacesConfigParser parser(AF_INET); + CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); + ASSERT_NO_THROW(parser.parse(cfg_iface, config_element)); // Compare the resulting configuration with a reference // configuration using the raw socket. @@ -139,18 +143,19 @@ TEST_F(IfacesConfigParserTest, socketTypeDatagram) { // Test that the configuration rejects the invalid socket type. TEST_F(IfacesConfigParserTest, socketTypeInvalid) { // For DHCPv4 we only accept the raw socket or datagram socket. - IfacesConfigParser4 parser4; + IfacesConfigParser parser4(AF_INET); + CfgIfacePtr cfg_iface = CfgMgr::instance().getStagingCfg()->getCfgIface(); std::string config = "{ \"interfaces\": [ ]," "\"dhcp-socket-type\": \"default\" }"; ElementPtr config_element = Element::fromJSON(config); - ASSERT_THROW(parser4.build(config_element), DhcpConfigError); + ASSERT_THROW(parser4.parse(cfg_iface, config_element), DhcpConfigError); // For DHCPv6 we don't accept any socket type. - IfacesConfigParser6 parser6; + IfacesConfigParser parser6(AF_INET6); config = "{ \"interfaces\": [ ]," " \"dhcp-socket-type\": \"udp\" }"; config_element = Element::fromJSON(config); - ASSERT_THROW(parser6.build(config_element), DhcpConfigError); + ASSERT_THROW(parser6.parse(cfg_iface, config_element), DhcpConfigError); } } // end of anonymous namespace