From: Marcin Siodelski Date: Wed, 21 Jun 2017 17:05:45 +0000 (+0200) Subject: [5317] Cleanup new code added to facilitate unix domain sockets. X-Git-Tag: trac5227_base~26^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0ea522a2916bfbffa776aa238ed7b0ebe72b9570;p=thirdparty%2Fkea.git [5317] Cleanup new code added to facilitate unix domain sockets. --- diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 59f22c6fca..afb0e2dbdb 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -593,7 +593,10 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/) } server_ = this; // remember this instance for later use in handlers + // TimerMgr uses IO service to run asynchronous timers. TimerMgr::instance()->setIOService(getIOService()); + + // CommandMgr uses IO service to run asynchronous socket operations. CommandMgr::instance().setIOService(getIOService()); // These are the commands always supported by the DHCPv4 server. diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 2c11cb1f75..dfc4dc2c92 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -21,8 +21,6 @@ #include #include -#include - #include "marker_file.h" #include "test_libraries.h" @@ -87,15 +85,14 @@ public: StatsMgr::instance().removeAll(); CommandMgr::instance().closeCommandSocket(); - if (getIOService()) { - getIOService()->stopWork(); - getIOService()->poll(); - } server_.reset(); }; /// @brief Returns pointer to the server's IO service. + /// + /// @return Pointer to the server's IO service or null pointer if the server + /// hasn't been created. IOServicePtr getIOService() { return (server_ ? server_->getIOService() : IOServicePtr()); } @@ -134,7 +131,6 @@ public: std::string config_txt = header + socket_path_ + footer; server_.reset(new NakedControlledDhcpv4Srv()); - CommandMgr::instance().setIOService(getIOService()); ConstElementPtr config; ASSERT_NO_THROW(config = parseDHCP4(config_txt)); @@ -309,7 +305,6 @@ TEST_F(CtrlChannelDhcpv4SrvTest, commands) { ASSERT_NO_THROW( server_.reset(new NakedControlledDhcpv4Srv()); - CommandMgr::instance().setIOService(getIOService()); ); // Use empty parameters list @@ -394,7 +389,6 @@ TEST_F(CtrlChannelDhcpv4SrvTest, commandsRegistration) { // Created server should register several additional commands. ASSERT_NO_THROW( server_.reset(new NakedControlledDhcpv4Srv()); - CommandMgr::instance().setIOService(getIOService()); ); EXPECT_NO_THROW(answer = CommandMgr::instance().processCommand(list_cmds)); @@ -696,7 +690,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configSet) { response); /// Check that the config was indeed applied. - /* const Subnet4Collection* subnets = + const Subnet4Collection* subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); EXPECT_EQ(1, subnets->size()); @@ -757,7 +751,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configSet) { EXPECT_EQ(2, subnets->size()); // Clean up after the test. - CfgMgr::instance().clear(); */ + CfgMgr::instance().clear(); } // Tests that the server properly responds to shtudown command sent diff --git a/src/lib/asiolink/io_acceptor.h b/src/lib/asiolink/io_acceptor.h index c014c041f4..4597f61d41 100644 --- a/src/lib/asiolink/io_acceptor.h +++ b/src/lib/asiolink/io_acceptor.h @@ -17,13 +17,21 @@ namespace isc { namespace asiolink { +/// @brief Base class for acceptor services in Kea. +/// +/// This is a wrapper class for ASIO acceptor service. Classes implementing +/// services for specific protocol types should derive from this class. +/// +/// @tparam ProtocolType ASIO protocol type, e.g. stream_protocol +/// @tparam CallbackType Callback function type which should have the following +/// signature: @c void(const boost::system::error_code&). template class IOAcceptor : public IOSocket { public: /// @brief Constructor. /// - /// @param io_service IO service. + /// @param io_service Reference to the IO service. explicit IOAcceptor(IOService& io_service) : IOSocket(), acceptor_(new typename ProtocolType::acceptor(io_service.get_io_service())) { @@ -39,8 +47,10 @@ public: /// @brief Opens acceptor socket given the endpoint. /// - /// @param endpoint Reference to the endpoint object which specifies the - /// address and port on which the acceptor service will run. + /// @param endpoint Reference to the endpoint object defining local + /// acceptor endpoint. + /// + /// @tparam EndpointType Endpoint type. template void open(const EndpointType& endpoint) { acceptor_->open(endpoint.getASIOEndpoint().protocol()); @@ -48,8 +58,10 @@ public: /// @brief Binds socket to an endpoint. /// - /// @param endpoint Reference to an endpoint to which the socket is to - /// be bound. + /// @param endpoint Reference to the endpoint object defining local + /// acceptor endpoint. + /// + /// @tparam EndpointType Endpoint type. template void bind(const EndpointType& endpoint) { acceptor_->bind(endpoint.getASIOEndpoint()); @@ -66,17 +78,11 @@ public: acceptor_->set_option(socket_option); } - /// @brief Starts listening for the new connections. + /// @brief Starts listening new connections. void listen() { acceptor_->listen(); } - template class SocketType, typename SocketCallback> - void asyncAccept(const SocketType& socket, - const CallbackType& callback) { - acceptor_->async_accept(socket.getASIOSocket(), callback); - } - /// @brief Checks if the acceptor is open. /// /// @return true if acceptor is open. @@ -94,15 +100,17 @@ protected: /// @brief Asynchronously accept new connection. /// /// This method accepts new connection into the specified socket. When the - /// new connection arrives or an error occurs the specified callback function - /// is invoked. + /// new connection arrives or an error occurs the specified callback + /// function is invoked. /// /// @param socket Socket into which connection should be accepted. /// @param callback Callback function to be invoked when the new connection /// arrives. - /// @tparam SocketType + /// @tparam SocketType Socket type, e.g. @ref UnixDomainSocket. It must + /// implement @c getASIOSocket method. template - void asyncAcceptInternal(const SocketType& socket, const CallbackType& callback) { + void asyncAcceptInternal(const SocketType& socket, + const CallbackType& callback) { acceptor_->async_accept(socket.getASIOSocket(), callback); } diff --git a/src/lib/asiolink/io_socket.h b/src/lib/asiolink/io_socket.h index 3597bb870a..9c9cee16fd 100644 --- a/src/lib/asiolink/io_socket.h +++ b/src/lib/asiolink/io_socket.h @@ -2,7 +2,7 @@ // // 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 -// file obtain one at http://mozilla.org/MPL/2.0/. +// file, You can obtain one at http://mozilla.org/MPL/2.0/. #ifndef IO_SOCKET_H #define IO_SOCKET_H 1 diff --git a/src/lib/asiolink/tcp_acceptor.h b/src/lib/asiolink/tcp_acceptor.h index 67759863f7..4fab1ddbbe 100644 --- a/src/lib/asiolink/tcp_acceptor.h +++ b/src/lib/asiolink/tcp_acceptor.h @@ -11,6 +11,7 @@ #error "asio.hpp must be included before including this, see asiolink.h as to why" #endif +#include #include #include #include @@ -28,23 +29,14 @@ namespace asiolink { /// /// @tparam C Acceptor callback type. template -class TCPAcceptor : public IOSocket { +class TCPAcceptor : public IOAcceptor { public: /// @brief Constructor. /// /// @param io_service IO service. explicit TCPAcceptor(IOService& io_service) - : IOSocket(), - acceptor_(new boost::asio::ip::tcp::acceptor(io_service.get_io_service())) { - } - - /// @brief Destructor. - virtual ~TCPAcceptor() { } - - /// @brief Returns file descriptor of the underlying socket. - virtual int getNative() const final { - return (acceptor_->native()); + : IOAcceptor(io_service) { } /// @brief Returns protocol of the socket. @@ -54,45 +46,6 @@ public: return (IPPROTO_TCP); } - /// @brief Opens acceptor socket given the endpoint. - /// - /// @param endpoint Reference to the endpoint object which specifies the - /// address and port on which the acceptor service will run. - void open(const TCPEndpoint& endpoint) { - acceptor_->open(endpoint.getASIOEndpoint().protocol()); - } - - /// @brief Sets socket option. - /// - /// Typically, this method is used to set SO_REUSEADDR option on the socket: - /// @code - /// IOService io_service; - /// TCPAcceptor acceptor(io_service); - /// acceptor.setOption(TCPAcceptor::ReuseAddress(true)) - /// @endcode - /// - /// @param socket_option Reference to the object encapsulating an option to - /// be set for the socket. - /// @tparam SettableSocketOption Type of the object encapsulating socket option - /// being set. - template - void setOption(const SettableSocketOption& socket_option) { - acceptor_->set_option(socket_option); - } - - /// @brief Binds socket to an endpoint. - /// - /// @param endpoint Reference to an endpoint to which the socket is to - /// be bound. - void bind(const TCPEndpoint& endpoint) { - acceptor_->bind(endpoint.getASIOEndpoint()); - } - - /// @brief Starts listening for the new connections. - void listen() { - acceptor_->listen(); - } - /// @brief Asynchronously accept new connection. /// /// This method accepts new connection into the specified socket. When the @@ -105,26 +58,8 @@ public: /// @tparam SocketCallback Type of the callback for the @ref TCPSocket. template void asyncAccept(const TCPSocket& socket, C& callback) { - acceptor_->async_accept(socket.getASIOSocket(), callback); - } - - /// @brief Checks if the acceptor is open. - /// - /// @return true if acceptor is open. - bool isOpen() const { - return (acceptor_->is_open()); + IOAcceptor::asyncAcceptInternal(socket, callback); } - - /// @brief Closes the acceptor. - void close() const { - acceptor_->close(); - } - -private: - - /// @brief Underlying ASIO acceptor implementation. - boost::shared_ptr acceptor_; - }; diff --git a/src/lib/asiolink/unix_domain_socket_acceptor.h b/src/lib/asiolink/unix_domain_socket_acceptor.h index d0fc7d16a3..1cec340c67 100644 --- a/src/lib/asiolink/unix_domain_socket_acceptor.h +++ b/src/lib/asiolink/unix_domain_socket_acceptor.h @@ -18,19 +18,25 @@ namespace isc { namespace asiolink { -class UnixDomainSocketAcceptor - : public IOAcceptor > { +/// @brief Implements acceptor service for @ref UnixDomainSocket. +class UnixDomainSocketAcceptor : public IOAcceptor > { public: /// @brief Callback type used in call to @ref UnixDomainSocketAcceptor::asyncAccept. typedef std::function AcceptHandler; + /// @brief Constructor. + /// + /// @param io_service Reference to the IO service. explicit UnixDomainSocketAcceptor(IOService& io_service) : IOAcceptor >(io_service) { } + /// @brief Returns the transport protocol of the socket. + /// + /// @return AF_LOCAL. virtual int getProtocol() const final { return (AF_LOCAL); } diff --git a/src/lib/asiolink/unix_domain_socket_endpoint.h b/src/lib/asiolink/unix_domain_socket_endpoint.h index 1b3b76217e..85f019cf5f 100644 --- a/src/lib/asiolink/unix_domain_socket_endpoint.h +++ b/src/lib/asiolink/unix_domain_socket_endpoint.h @@ -16,13 +16,20 @@ namespace isc { namespace asiolink { +/// @brief Endpoint for @ref UnixDomainSocket. +/// +/// This is a simple class encapsulating ASIO unix domain socket. class UnixDomainSocketEndpoint { public: + /// @brief Constructor. + /// + /// @param endpoint_path Path to the socket descriptor. explicit UnixDomainSocketEndpoint(const std::string& endpoint_path) : endpoint_(endpoint_path) { } + /// @brief Returns underlying ASIO endpoint. const boost::asio::local::stream_protocol::endpoint& getASIOEndpoint() const { return (endpoint_); @@ -30,11 +37,12 @@ public: private: + /// @brief Underlying ASIO endpoint. boost::asio::local::stream_protocol::endpoint endpoint_; }; -} -} +} // end of namespace isc::asiolink +} // end of namespace isc #endif // UNIX_DOMAIN_SOCKET_ENDPOINT_H diff --git a/src/lib/config/Makefile.am b/src/lib/config/Makefile.am index bbeb9f12aa..c1487d59f7 100644 --- a/src/lib/config/Makefile.am +++ b/src/lib/config/Makefile.am @@ -18,8 +18,6 @@ libkea_cfgclient_la_SOURCES += module_spec.h module_spec.cc libkea_cfgclient_la_SOURCES += base_command_mgr.cc base_command_mgr.h libkea_cfgclient_la_SOURCES += client_connection.cc client_connection.h libkea_cfgclient_la_SOURCES += command_mgr.cc command_mgr.h -libkea_cfgclient_la_SOURCES += command_socket.cc command_socket.h -libkea_cfgclient_la_SOURCES += command_socket_factory.cc command_socket_factory.h libkea_cfgclient_la_SOURCES += config_log.h config_log.cc libkea_cfgclient_la_SOURCES += hooked_command_mgr.cc hooked_command_mgr.h diff --git a/src/lib/config/command_mgr.cc b/src/lib/config/command_mgr.cc index 871c075b8c..17b4553ff9 100644 --- a/src/lib/config/command_mgr.cc +++ b/src/lib/config/command_mgr.cc @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -51,6 +50,10 @@ public: void stop() { if (!response_in_progress_) { + LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_CLOSED) + .arg(socket_->getNative()); + + isc::dhcp::IfaceMgr::instance().deleteExternalSocket(socket_->getNative()); socket_->close(); } } @@ -105,11 +108,27 @@ void Connection::receiveHandler(const boost::system::error_code& ec, size_t bytes_transferred) { if (ec) { + + if (ec.value() != boost::asio::error::operation_aborted) { + LOG_ERROR(command_logger, COMMAND_SOCKET_READ_FAIL) + .arg(ec.value()).arg(socket_->getNative()); + } + + /// @todo: Should we close the connection, similar to what is already + /// being done for bytes_transferred == 0. + return; + + } else if (bytes_transferred == 0) { + connection_pool_.stop(shared_from_this()); return; } ConstElementPtr cmd, rsp; + LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ) + .arg(bytes_transferred).arg(socket_->getNative()); + + try { response_in_progress_ = true; @@ -120,6 +139,7 @@ Connection::receiveHandler(const boost::system::error_code& ec, // If successful, then process it as a command. rsp = CommandMgr::instance().processCommand(cmd); + } catch (const Exception& ex) { LOG_WARN(command_logger, COMMAND_PROCESS_ERROR1).arg(ex.what()); rsp = createAnswer(CONTROL_RESULT_ERROR, std::string(ex.what())); @@ -143,15 +163,22 @@ Connection::receiveHandler(const boost::system::error_code& ec, len = 65535; } - // Send the data back over socket. - socket_->write(txt.c_str(), len); - - response_in_progress_ = false; + try { + // Send the data back over socket. + socket_->write(txt.c_str(), len); + } catch (const std::exception& ex) { + // Response transmission failed. Since the response failed, it doesn't + // make sense to send any status codes. Let's log it and be done with + // it. + LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL) + .arg(len).arg(socket_->getNative()).arg(ex.what()); + } - isc::dhcp::IfaceMgr::instance().deleteExternalSocket(socket_->getNative()); + response_in_progress_ = false; connection_pool_.stop(shared_from_this()); + } } @@ -185,7 +212,11 @@ public: void CommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr& socket_info) { socket_name_.clear(); - + + if(!socket_info) { + isc_throw(BadSocketInfo, "Missing socket_info parameters, can't create socket."); + } + ConstElementPtr type = socket_info->get("socket-type"); if (!type) { isc_throw(BadSocketInfo, "Mandatory 'socket-type' parameter missing"); @@ -209,27 +240,37 @@ CommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr& socket_info) socket_name_ = name->stringValue(); - acceptor_.reset(new UnixDomainSocketAcceptor(*io_service_)); - UnixDomainSocketEndpoint endpoint(socket_name_); - acceptor_->open(endpoint); - acceptor_->bind(endpoint); - acceptor_->listen(); + try { + // Start asynchronous acceptor service. + acceptor_.reset(new UnixDomainSocketAcceptor(*io_service_)); + UnixDomainSocketEndpoint endpoint(socket_name_); + acceptor_->open(endpoint); + acceptor_->bind(endpoint); + acceptor_->listen(); - doAccept(); + // Install this socket in Interface Manager. + isc::dhcp::IfaceMgr::instance().addExternalSocket(acceptor_->getNative(), 0); - // Install this socket in Interface Manager. - isc::dhcp::IfaceMgr::instance().addExternalSocket(acceptor_->getNative(), 0); + doAccept(); + } catch (const std::exception& ex) { + isc_throw(SocketError, ex.what()); + } } void CommandMgrImpl::doAccept() { + // Create a socket into which the acceptor will accept new connection. socket_.reset(new UnixDomainSocket(*io_service_)); - acceptor_->asyncAccept(*socket_, - [this](const boost::system::error_code& ec) { + acceptor_->asyncAccept(*socket_, [this](const boost::system::error_code& ec) { if (!ec) { + // New connection is arriving. Start asynchronous transmission. ConnectionPtr connection(new Connection(socket_, connection_pool_)); connection_pool_.start(connection); + } + + // Unless we're stopping the service, start accepting connections again. + if (ec.value() != boost::asio::error::operation_aborted) { doAccept(); } }); @@ -245,42 +286,18 @@ CommandMgr::openCommandSocket(const isc::data::ConstElementPtr& socket_info) { } void CommandMgr::closeCommandSocket() { - impl_->connection_pool_.stopAll(); - - if (impl_->acceptor_) { + // Close acceptor if the acceptor is open. + if (impl_->acceptor_ && impl_->acceptor_->isOpen()) { isc::dhcp::IfaceMgr::instance().deleteExternalSocket(impl_->acceptor_->getNative()); impl_->acceptor_->close(); static_cast(::remove(impl_->socket_name_.c_str())); } -/* // Now let's close all existing connections that we may have. - for (std::list::iterator conn = connections_.begin(); - conn != connections_.end(); ++conn) { - (*conn)->close(); - } - connections_.clear(); */ -} - - -void CommandMgr::addConnection(const CommandSocketPtr& conn) { - connections_.push_back(conn); -} - -bool CommandMgr::closeConnection(int fd) { - - // Let's iterate over all currently registered connections. - for (std::list::iterator conn = connections_.begin(); - conn != connections_.end(); ++conn) { - - // If found, close it. - if ((*conn)->getFD() == fd) { - (*conn)->close(); - connections_.erase(conn); - return (true); - } - } - - return (false); + // Stop all connections which can be closed. The only connection that won't + // be closed is the one over which we have received a request to reconfigure + // the server. This connection will be held until the CommandMgr responds to + // such request. + impl_->connection_pool_.stopAll(); } int @@ -297,107 +314,8 @@ CommandMgr::instance() { void CommandMgr::setIOService(const IOServicePtr& io_service) { - closeCommandSocket(); impl_->io_service_ = io_service; } -void -CommandMgr::commandReader(int sockfd) { - - /// @todo: We do not handle commands that are larger than 64K. - - // We should not expect commands bigger than 64K. - char buf[65536]; - memset(buf, 0, sizeof(buf)); - ConstElementPtr cmd, rsp; - - // Read incoming data. - int rval = read(sockfd, buf, sizeof(buf)); - if (rval < 0) { - // Read failed - LOG_ERROR(command_logger, COMMAND_SOCKET_READ_FAIL).arg(rval).arg(sockfd); - - /// @todo: Should we close the connection, similar to what is already - /// being done for rval == 0? - return; - } else if (rval == 0) { - - // Remove it from the active connections list. - instance().closeConnection(sockfd); - - return; - } - - // Duplicate the connection's socket in the event, the command causes the - // channel to close (like a reconfig). This permits us to always have - // a socket on which to respond. If for some reason we can't fall back - // to the connection socket. - int rsp_fd = dup(sockfd); - if (rsp_fd < 0 ) { - // Highly unlikely - const char* errmsg = strerror(errno); - LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_DUP_WARN) - .arg(errmsg); - rsp_fd = sockfd; - } - - LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ).arg(rval).arg(sockfd); - - // Ok, we received something. Let's see if we can make any sense of it. - try { - - // Try to interpret it as JSON. - std::string sbuf(buf, static_cast(rval)); - cmd = Element::fromJSON(sbuf, true); - - // If successful, then process it as a command. - rsp = CommandMgr::instance().processCommand(cmd); - } catch (const Exception& ex) { - LOG_WARN(command_logger, COMMAND_PROCESS_ERROR1).arg(ex.what()); - rsp = createAnswer(CONTROL_RESULT_ERROR, std::string(ex.what())); - } - - if (!rsp) { - LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR); - // Only close the duped socket if it's different (should be) - if (rsp_fd != sockfd) { - close(rsp_fd); - } - - return; - } - - // Let's convert JSON response to text. Note that at this stage - // the rsp pointer is always set. - std::string txt = rsp->str(); - size_t len = txt.length(); - if (len > 65535) { - // Hmm, our response is too large. Let's send the first - // 64KB and hope for the best. - LOG_ERROR(command_logger, COMMAND_SOCKET_RESPONSE_TOOLARGE).arg(len); - - len = 65535; - } - - // Send the data back over socket. - rval = write(rsp_fd, txt.c_str(), len); - int saverr = errno; - - LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_WRITE).arg(len).arg(sockfd); - - if (rval < 0) { - // Response transmission failed. Since the response failed, it doesn't - // make sense to send any status codes. Let's log it and be done with - // it. - LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL) - .arg(len).arg(sockfd).arg(strerror(saverr)); - } - - // Only close the duped socket if it's different (should be) - if (rsp_fd != sockfd) { - close(rsp_fd); - } -} - }; // end of isc::config }; // end of isc diff --git a/src/lib/config/command_mgr.h b/src/lib/config/command_mgr.h index a17a399a24..75e2b548b6 100644 --- a/src/lib/config/command_mgr.h +++ b/src/lib/config/command_mgr.h @@ -10,14 +10,28 @@ #include #include #include -#include +#include #include #include -#include namespace isc { namespace config { +/// @brief An exception indicating that specified socket parameters are invalid +class BadSocketInfo : public Exception { +public: + BadSocketInfo(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; + +/// @brief An exception indicating a problem with socket operation +class SocketError : public Exception { +public: + SocketError(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; + + class CommandMgrImpl; /// @brief Commands Manager implementation for the Kea servers. @@ -56,29 +70,6 @@ public: /// @brief Shuts down any open control sockets void closeCommandSocket(); - /// @brief Reads data from a socket, parses as JSON command and processes it - /// - /// This method is used to handle traffic on connected socket. This callback - /// is installed by the @c isc::config::UnixCommandSocket::receiveHandler - /// (located in the src/lib/config/command_socket_factory.cc) - /// once the incoming connection is accepted. If end-of-file is detected, this - /// method will close the socket and will uninstall itself from - /// @ref isc::dhcp::IfaceMgr. - /// - /// @param sockfd socket descriptor of a connected socket - static void commandReader(int sockfd); - - /// @brief Adds an information about opened connection socket - /// - /// @param conn Connection socket to be stored - void addConnection(const CommandSocketPtr& conn); - - /// @brief Closes connection with a specific socket descriptor - /// - /// @param fd socket descriptor - /// @return true if closed successfully, false if not found - bool closeConnection(int fd); - /// @brief Returns control socket descriptor /// /// This method should be used only in tests. @@ -87,23 +78,10 @@ public: private: /// @brief Private constructor - /// - /// Registers internal 'list-commands' command. CommandMgr(); + /// @brief Pointer to the implementation of the @ref CommandMgr. boost::shared_ptr impl_; - - /// @brief Control socket structure - /// - /// This is the socket that accepts incoming connections. There can be at - /// most one (if command channel is configured). - CommandSocketPtr socket_; - - /// @brief Sockets for open connections - /// - /// These are the sockets that are dedicated to handle a specific connection. - /// Their number is equal to number of current control connections. - std::list connections_; }; }; // end of isc::config namespace diff --git a/src/lib/config/command_socket.cc b/src/lib/config/command_socket.cc deleted file mode 100644 index 0f4cf2c9d2..0000000000 --- a/src/lib/config/command_socket.cc +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright (C) 2015 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#include -#include -#include -#include -#include -#include - -namespace isc { -namespace config { - -ConnectionSocket::ConnectionSocket(int sockfd) { - sockfd_ = sockfd; - - // Install commandReader callback. When there's any data incoming on this - // socket, commandReader will be called and process it. It may also - // eventually close this socket. - isc::dhcp::IfaceMgr::instance().addExternalSocket(sockfd, - boost::bind(&ConnectionSocket::receiveHandler, this)); - } - -void ConnectionSocket::close() { - LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_CLOSED).arg(sockfd_); - - // Unregister this callback - isc::dhcp::IfaceMgr::instance().deleteExternalSocket(sockfd_); - - // We're closing a connection, not the whole socket. It's ok to just - // close the connection and don't delete anything. - ::close(sockfd_); -} - -void ConnectionSocket::receiveHandler() { - CommandMgr::instance().commandReader(sockfd_); -} - -}; -}; diff --git a/src/lib/config/command_socket.h b/src/lib/config/command_socket.h deleted file mode 100644 index 134c223309..0000000000 --- a/src/lib/config/command_socket.h +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright (C) 2015 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#ifndef COMMAND_SOCKET_H -#define COMMAND_SOCKET_H - -#include -#include - -namespace isc { -namespace config { - -/// @brief An exception indicating that specified socket parameters are invalid -class BadSocketInfo : public Exception { -public: - BadSocketInfo(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) { }; -}; - -/// @brief An exception indicating a problem with socket operation -class SocketError : public Exception { -public: - SocketError(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) { }; -}; - -/// @brief Abstract base class that represents an open command socket -/// -/// Derived classes are expected to handle specific socket types (e.g. UNIX -/// or https). -/// -/// For derived classes, see @ref UnixCommandSocket for a socket that -/// accepts connections over UNIX socket and @ref ConnectionSocket that -/// handles established connections (currently over UNIX sockets, but -/// should be generic). -class CommandSocket { -public: - /// @brief Method used to handle incoming data - /// - /// This may be registered in @ref isc::dhcp::IfaceMgr - virtual void receiveHandler() = 0; - - /// @brief General method for closing socket. - /// - /// This is the default implementation that simply closes - /// the socket. Derived classes may do additional steps - /// to terminate the connection. - virtual void close() { - ::close(sockfd_); - } - - /// @brief Virtual destructor. - virtual ~CommandSocket() { - close(); - } - - /// @brief Returns socket descriptor. - int getFD() const { - return (sockfd_); - } - -protected: - /// Stores socket descriptor. - int sockfd_; -}; - -/// Pointer to a command socket object -typedef boost::shared_ptr CommandSocketPtr; - -/// @brief This class represents a streaming socket for handling connections -/// -/// Initially a socket (e.g. UNIX) is opened (represented by other classes, e.g. -/// @ref UnixCommandSocket). Once incoming connection is detected, that class -/// calls accept(), which returns a new socket dedicated to handling that -/// specific connection. That socket is represented by this class. -class ConnectionSocket : public CommandSocket { -public: - /// @brief Default constructor - /// - /// This constructor is used in methods that call accept on existing - /// sockets. accept() returns a socket descriptor. Hence only one - /// parameter here. - /// - /// @param sockfd socket descriptor - ConnectionSocket(int sockfd); - - /// @brief Method used to handle incoming data - /// - /// This method calls isc::config::CommandMgr::commandReader method. - virtual void receiveHandler(); - - /// @brief Closes socket. - /// - /// This method closes the socket, prints appropriate log message and - /// unregisters callback from @ref isc::dhcp::IfaceMgr. - virtual void close(); -}; - -}; -}; - -#endif diff --git a/src/lib/config/command_socket_factory.cc b/src/lib/config/command_socket_factory.cc deleted file mode 100644 index 83534e3e9c..0000000000 --- a/src/lib/config/command_socket_factory.cc +++ /dev/null @@ -1,226 +0,0 @@ -// Copyright (C) 2015 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -using namespace isc::data; - -namespace isc { -namespace config { - -/// @brief Wrapper for UNIX stream sockets -/// -/// There are two UNIX socket types: datagram-based (equivalent of UDP) and -/// stream-based (equivalent of TCP). This class represents stream-based -/// sockets. It opens up a unix-socket and waits for incoming connections. -/// Once incoming connection is detected, accept() system call is called -/// and a new socket for that particular connection is returned. A new -/// object of @ref ConnectionSocket is created. -class UnixCommandSocket : public CommandSocket { -public: - /// @brief Default constructor - /// - /// Opens specified UNIX socket. - /// - /// @param filename socket filename - UnixCommandSocket(const std::string& filename) - : filename_(filename) { - - // Create the socket and set it up. - sockfd_ = createUnixSocket(filename_); - - // Install this socket in Interface Manager. - isc::dhcp::IfaceMgr::instance().addExternalSocket(sockfd_, - boost::bind(&UnixCommandSocket::receiveHandler, this)); - } - -private: - - /// @brief Auxiliary method for creating a UNIX socket - /// - /// @param file_name specifies socket file path - /// @return socket file descriptor - int createUnixSocket(const std::string& file_name) { - - struct sockaddr_un addr; - - // string.size() returns number of bytes (without trailing zero) - // we need 1 extra byte for terminating 0. - if (file_name.size() > sizeof(addr.sun_path) - 1) { - isc_throw(SocketError, "Failed to open socket: path specified (" - << file_name << ") is longer (" << file_name.size() - << " bytes) than allowed " - << (sizeof(addr.sun_path) - 1) << " bytes."); - } - - int fd = socket(AF_UNIX, SOCK_STREAM, 0); - if (fd == -1) { - isc_throw(isc::config::SocketError, "Failed to create AF_UNIX socket:" - << strerror(errno)); - } - - // Let's remove the old file. We don't care about any possible - // errors here. The file should not be there if the file was - // shut down properly. - static_cast(remove(file_name.c_str())); - - // Set this socket to be closed-on-exec. - if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) { - const char* errmsg = strerror(errno); - ::close(fd); - isc_throw(SocketError, "Failed to set close-on-exec on unix socket\ - " - << fd << ": " << errmsg); - } - - // Set this socket to be non-blocking one. - if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) { - const char* errmsg = strerror(errno); - ::close(fd); - isc_throw(SocketError, "Failed to set non-block mode on unix socket " - << fd << ": " << errmsg); - } - - // Now bind the socket to the specified path. - memset(&addr, 0, sizeof(addr)); - addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, file_name.c_str(), sizeof(addr.sun_path) - 1); - if (bind(fd, (struct sockaddr*)&addr, sizeof(addr))) { - const char* errmsg = strerror(errno); - ::close(fd); - static_cast(remove(file_name.c_str())); - isc_throw(isc::config::SocketError, "Failed to bind socket " << fd - << " to " << file_name << ": " << errmsg); - } - - // One means that we allow at most 1 awaiting connections. - // Any additional attempts will get ECONNREFUSED error. - // That means that at any given time, there may be at most one controlling - // connection. - /// @todo: Make the number of parallel connections configurable. - int status = listen(fd, 1); - if (status < 0) { - const char* errmsg = strerror(errno); - ::close(fd); - static_cast(remove(file_name.c_str())); - isc_throw(isc::config::SocketError, "Failed to listen on socket fd=" - << fd << ", filename=" << file_name << ": " << errmsg); - } - - // Woohoo! Socket opened, let's log it! - LOG_INFO(command_logger, COMMAND_SOCKET_UNIX_OPEN).arg(fd).arg(file_name); - - return (fd); - } - - /// @public - - /// @brief Connection acceptor, a callback used to accept incoming connections. - /// - /// This callback is used on a control socket. Once called, it will accept - /// incoming connection, create a new socket for it and create an instance - /// of ConnectionSocket, which will take care of the rest (i.e. install - /// appropriate callback for that new socket in @ref isc::dhcp::IfaceMgr). - void receiveHandler() { - - // This method is specific to receiving data over UNIX socket, so using - // sockaddr_un instead of sockaddr_storage here is ok. - struct sockaddr_un client_addr; - socklen_t client_addr_len; - client_addr_len = sizeof(client_addr); - - // Accept incoming connection. This will create a separate socket for - // handling this specific connection. - int fd2 = accept(sockfd_, reinterpret_cast(&client_addr), - &client_addr_len); - if (fd2 == -1) { - LOG_ERROR(command_logger, COMMAND_SOCKET_ACCEPT_FAIL) - .arg(sockfd_).arg(strerror(errno)); - return; - } - - // And now create an object that represents that new connection. - CommandSocketPtr conn(new ConnectionSocket(fd2)); - - // Not sure if this is really needed, but let's set it to non-blocking - // mode. - if (fcntl(fd2, F_SETFL, O_NONBLOCK) != 0) { - // Failed to set socket to non-blocking mode. - LOG_ERROR(command_logger, COMMAND_SOCKET_FAIL_NONBLOCK) - .arg(fd2).arg(sockfd_).arg(strerror(errno)); - - conn.reset(); - return; - } - - // Remember this socket descriptor. It will be needed when we shut down - // the server. - CommandMgr::instance().addConnection(conn); - - LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_OPENED).arg(fd2) - .arg(sockfd_); - } - - /// @private - - // This method is called when we shutdown the connection. - void close() { - LOG_INFO(command_logger, COMMAND_SOCKET_UNIX_CLOSE).arg(sockfd_) - .arg(filename_); - - isc::dhcp::IfaceMgr::instance().deleteExternalSocket(sockfd_); - - // Close should always succeed. We don't care if we're able to delete - // the socket or not. - ::close(sockfd_); - static_cast(remove(filename_.c_str())); - } - - /// @brief UNIX filename representing this socket - std::string filename_; -}; - -CommandSocketPtr -CommandSocketFactory::create(const isc::data::ConstElementPtr& socket_info) { - if(!socket_info) { - isc_throw(BadSocketInfo, "Missing socket_info parameters, can't create socket."); - } - - ConstElementPtr type = socket_info->get("socket-type"); - if (!type) { - isc_throw(BadSocketInfo, "Mandatory 'socket-type' parameter missing"); - } - - if (type->stringValue() == "unix") { - // UNIX socket is requested. It takes one parameter: socket-name that - // specifies UNIX path of the socket. - ConstElementPtr name = socket_info->get("socket-name"); - if (!name) { - isc_throw(BadSocketInfo, "Mandatory 'socket-name' parameter missing"); - } - if (name->getType() != Element::string) { - isc_throw(BadSocketInfo, "'socket-name' parameter expected to be a string"); - } - - return (CommandSocketPtr(new UnixCommandSocket(name->stringValue()))); - } else { - isc_throw(BadSocketInfo, "Specified socket type ('" + type->stringValue() - + "') is not supported."); - } -} - -}; -}; diff --git a/src/lib/config/command_socket_factory.h b/src/lib/config/command_socket_factory.h deleted file mode 100644 index 7b9fa9f7a1..0000000000 --- a/src/lib/config/command_socket_factory.h +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (C) 2015 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#ifndef COMMAND_SOCKET_FACTORY_H -#define COMMAND_SOCKET_FACTORY_H - -#include -#include - -namespace isc { -namespace config { - -/// A factory class for opening command socket -/// -/// This class provides an interface for opening command socket. -class CommandSocketFactory { -public: - - /// @brief Creates a socket specified by socket_info structure - /// - /// - /// Currently supported types are: - /// - unix - /// - /// See @ref CommandMgr::openCommandSocket for detailed description. - /// @throw CommandSocketError - /// - /// @param socket_info structure that describes the socket - /// @return socket descriptor - static CommandSocketPtr create(const isc::data::ConstElementPtr& socket_info); -}; - -}; -}; - -#endif diff --git a/src/lib/config/tests/Makefile.am b/src/lib/config/tests/Makefile.am index e3a6b67cc8..d2b6bbe160 100644 --- a/src/lib/config/tests/Makefile.am +++ b/src/lib/config/tests/Makefile.am @@ -20,7 +20,6 @@ if HAVE_GTEST TESTS += run_unittests run_unittests_SOURCES = module_spec_unittests.cc run_unittests_SOURCES += client_connection_unittests.cc -run_unittests_SOURCES += command_socket_factory_unittests.cc run_unittests_SOURCES += config_data_unittests.cc run_unittests.cc run_unittests_SOURCES += command_mgr_unittests.cc diff --git a/src/lib/config/tests/command_mgr_unittests.cc b/src/lib/config/tests/command_mgr_unittests.cc index 95e736379e..9bff3f9839 100644 --- a/src/lib/config/tests/command_mgr_unittests.cc +++ b/src/lib/config/tests/command_mgr_unittests.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -16,6 +17,7 @@ #include #include +using namespace isc::asiolink; using namespace isc::config; using namespace isc::data; using namespace isc::hooks; @@ -26,7 +28,11 @@ class CommandMgrTest : public ::testing::Test { public: /// Default constructor - CommandMgrTest() { + CommandMgrTest() + : io_service_(new IOService()) { + + CommandMgr::instance().setIOService(io_service_); + handler_name = ""; handler_params = ElementPtr(); handler_called = false; @@ -46,6 +52,20 @@ public: "control_command_receive"); } + /// @brief Returns socket path (using either hardcoded path or env variable) + /// @return path to the unix socket + std::string getSocketPath() { + + std::string socket_path; + const char* env = getenv("KEA_SOCKET_TEST_DIR"); + if (env) { + socket_path = std::string(env) + "/test-socket"; + } else { + socket_path = std::string(TEST_DATA_BUILDDIR) + "/test-socket"; + } + return (socket_path); + } + /// @brief Resets indicators related to callout invocation. static void resetCalloutIndicators() { callout_name = ""; @@ -162,6 +182,9 @@ public: return (0); } + /// @brief IO service used by these tests. + IOServicePtr io_service_; + /// @brief Name of the command (used in my_handler) static std::string handler_name; @@ -490,3 +513,43 @@ TEST_F(CommandMgrTest, modifyCommandArgsInHook) { EXPECT_EQ("response", callout_argument_names[1]); } + +// This test verifies that a Unix socket can be opened properly and that input +// parameters (socket-type and socket-name) are verified. +TEST_F(CommandMgrTest, unixCreate) { + // Null pointer is obviously a bad idea. + EXPECT_THROW(CommandMgr::instance().openCommandSocket(ConstElementPtr()), + isc::config::BadSocketInfo); + + // So is passing no parameters. + ElementPtr socket_info = Element::createMap(); + EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info), + isc::config::BadSocketInfo); + + // We don't support ipx sockets + socket_info->set("socket-type", Element::create("ipx")); + EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info), + isc::config::BadSocketInfo); + + socket_info->set("socket-type", Element::create("unix")); + EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info), + isc::config::BadSocketInfo); + + socket_info->set("socket-name", Element::create(getSocketPath())); + EXPECT_NO_THROW(CommandMgr::instance().openCommandSocket(socket_info)); + EXPECT_GE(CommandMgr::instance().getControlSocketFD(), 0); + + // It should be possible to close the socket. + EXPECT_NO_THROW(CommandMgr::instance().closeCommandSocket()); +} + +// This test checks that when unix path is too long, the socket cannot be opened. +TEST_F(CommandMgrTest, unixCreateTooLong) { + ElementPtr socket_info = Element::fromJSON("{ \"socket-type\": \"unix\"," + "\"socket-name\": \"/tmp/toolongtoolongtoolongtoolongtoolongtoolong" + "toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong" + "\" }"); + + EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info), + SocketError); +} diff --git a/src/lib/config/tests/command_socket_factory_unittests.cc b/src/lib/config/tests/command_socket_factory_unittests.cc deleted file mode 100644 index 567d169364..0000000000 --- a/src/lib/config/tests/command_socket_factory_unittests.cc +++ /dev/null @@ -1,94 +0,0 @@ -// Copyright (C) 2015 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#include - -#include -#include -#include -#include -#include -#include - -using namespace isc::config; -using namespace isc::data; - -// Test class for Command Manager -class CommandSocketFactoryTest : public ::testing::Test { -public: - - /// Default constructor - CommandSocketFactoryTest() - :SOCKET_NAME(getSocketPath()) { - - // Remove any stale socket files - static_cast(remove(SOCKET_NAME.c_str())); - } - - /// Default destructor - ~CommandSocketFactoryTest() { - - // Remove any stale socket files - static_cast(remove(SOCKET_NAME.c_str())); - } - - /// @brief Returns socket path (using either hardcoded path or env variable) - /// @return path to the unix socket - std::string getSocketPath() { - - std::string socket_path; - const char* env = getenv("KEA_SOCKET_TEST_DIR"); - if (env) { - socket_path = std::string(env) + "/test-socket"; - } else { - socket_path = std::string(TEST_DATA_BUILDDIR) + "/test-socket"; - } - return (socket_path); - } - - std::string SOCKET_NAME; -}; - -// This test verifies that a Unix socket can be opened properly and that input -// parameters (socket-type and socket-name) are verified. -TEST_F(CommandSocketFactoryTest, unixCreate) { - // Null pointer is obviously a bad idea. - EXPECT_THROW(CommandSocketFactory::create(ConstElementPtr()), - isc::config::BadSocketInfo); - - // So is passing no parameters. - ElementPtr socket_info = Element::createMap(); - EXPECT_THROW(CommandSocketFactory::create(socket_info), - isc::config::BadSocketInfo); - - // We don't support ipx sockets - socket_info->set("socket-type", Element::create("ipx")); - EXPECT_THROW(CommandSocketFactory::create(socket_info), - isc::config::BadSocketInfo); - - socket_info->set("socket-type", Element::create("unix")); - EXPECT_THROW(CommandSocketFactory::create(socket_info), - isc::config::BadSocketInfo); - - socket_info->set("socket-name", Element::create(SOCKET_NAME)); - CommandSocketPtr sock; - EXPECT_NO_THROW(sock = CommandSocketFactory::create(socket_info)); - ASSERT_TRUE(sock); - EXPECT_NE(-1, sock->getFD()); - - // It should be possible to close the socket. - EXPECT_NO_THROW(sock->close()); -} - -// This test checks that when unix path is too long, the socket cannot be opened. -TEST_F(CommandSocketFactoryTest, unixCreateTooLong) { - ElementPtr socket_info = Element::fromJSON("{ \"socket-type\": \"unix\"," - "\"socket-name\": \"/tmp/toolongtoolongtoolongtoolongtoolongtoolong" - "toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong" - "\" }"); - - EXPECT_THROW(CommandSocketFactory::create(socket_info), SocketError); -}