From 31f30b1e995c47d23a7ad1ec28ea665da775aa1c Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Fri, 1 Jun 2012 14:46:05 +0200 Subject: [PATCH] [1651] Tests for IfaceMgr callback implemented, devel doc updated --- doc/devel/02-dhcp.dox | 36 ++++++++++-- doc/devel/mainpage.dox | 3 + src/bin/dhcp4/main.cc | 72 +++++++++++++++--------- src/lib/dhcp/tests/iface_mgr_unittest.cc | 51 ++++++++++++++++- 4 files changed, 128 insertions(+), 34 deletions(-) diff --git a/doc/devel/02-dhcp.dox b/doc/devel/02-dhcp.dox index 7ebd9b203a..782301d5a6 100644 --- a/doc/devel/02-dhcp.dox +++ b/doc/devel/02-dhcp.dox @@ -15,11 +15,35 @@ * only), as support for transmission to hosts without IPv4 address * assigned is not implemented in IfaceMgr yet. * - * DHCPv4 server component does not listen to BIND10 message queue. - * * DHCPv4 server component does not use BIND10 logging yet. * - * DHCPv4 server component is not integrated with boss yet. + * @section dhcpv4Session BIND10 message queue integration + * + * DHCPv4 server component is now integrated with BIND10 message queue. + * The integration is performed by establish_session() and disconnect_session() + * functions in src/bin/dhcp4/main.cc file. isc::cc::Session cc_session + * object uses ASIO for establishing connection. It registers its socket + * in isc::asiolink::IOService io_service object. Typically, other components that + * use ASIO for their communication, register their other sockets in the + * same io_service and then just call io_service.run() method that does + * not return, until one of the callback decides that it is time to shut down + * the whole component cal calls io_service.stop(). DHCPv4 works in a + * different way. It does receive messages using select() + * (see isc::dhcp::IfaceMgr::receive4()), which is incompatible with ASIO. + * To solve this problem, socket descriptor is extracted from cc_session + * object and is passed to IfaceMgr by using isc::dhcp::IfaceMgr::set_session_socket(). + * IfaceMgr then uses this socket in its select() call. If there is some + * data to be read, it calls registered callback that is supposed to + * read and process incoming data. + * + * This somewhat complicated approach is needed for a simple reason. In + * embedded deployments there will be no message queue. Not referring directly + * to anything related to message queue in isc::dhcp::Dhcpv4Srv and + * isc::dhcp::IfaceMgr classes brings in two benefits. First, the can + * be used with and without message queue. Second benefit is related to the + * first one: \ref libdhcp is supposed to be simple and robust and not require + * many dependencies. One notable example of a use case that benefits from + * this approach is a perfdhcp tool. * * @page dhcpv6 DHCPv6 Server Component * @@ -42,9 +66,9 @@ * * DHCPv6 server component is not integrated with boss yet. * - * @page libdhcp libdhcp++ library + * @page libdhcp libdhcp++ * - * @section libdhcpIntro Libdhcp++ Introduction + * @section libdhcpIntro Libdhcp++ Library Introduction * * libdhcp++ is an all-purpose DHCP-manipulation library, written in * C++. It offers packet parsing and assembly, DHCPv4 and DHCPv6 @@ -82,7 +106,7 @@ * isc::dhcp::Option::delOption(), isc::dhcp::Option::getOption() can be used * for that purpose. * - * @section lidhcpIfaceMgr Interface Manager + * @section libdhcpIfaceMgr Interface Manager * * Interface Manager (or IfaceMgr) is an abstraction layer about low-level * network operations. In particlar, it provides information about existing diff --git a/doc/devel/mainpage.dox b/doc/devel/mainpage.dox index a472add985..a7fd094152 100644 --- a/doc/devel/mainpage.dox +++ b/doc/devel/mainpage.dox @@ -19,8 +19,11 @@ * * @section DHCP * - @subpage dhcpv4 + * - @subpage dhcpv4Session * - @subpage dhcpv6 * - @subpage libdhcp + * - @subpage libdhcpIntro + * - @subpage libdhcpIfaceMgr * * @section misc Miscellaneous topics * - @subpage LoggingApi diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index c7a60115d8..8d1944765d 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -66,18 +66,31 @@ usage() { } } // end of anonymous namespace +/// @brief DHCPv4 context (provides access to essential objects) +/// +/// This is a structure that provides access to essential objects +/// used during DHCPv4 operation: Dhcpv4Srv object itself and +/// also objects required for msgq session management. +struct DHCPv4Context { + IOService io_service; + Session* cc_session; + ModuleCCSession* config_session; + Dhcpv4Srv* server; + DHCPv4Context(): cc_session(NULL), config_session(NULL), server(NULL) { }; +}; + +DHCPv4Context dhcp4; + // Global objects are ugly, but that is the most convenient way of // having it accessible from handlers. -IOService io_service; // The same applies to global pointers. Ugly, but useful. -Dhcpv4Srv* server = NULL; ConstElementPtr dhcp4_config_handler(ConstElementPtr new_config) { cout << "b10-dhcp4: Received new config:" << new_config->str() << endl; ConstElementPtr answer = isc::config::createAnswer(0, - "Thanks for sending config."); + "Thank you for sending config."); return (answer); } @@ -86,10 +99,10 @@ dhcp4_command_handler(const string& command, ConstElementPtr args) { cout << "b10-dhcp4: Received new command: [" << command << "], args=" << args->str() << endl; if (command == "shutdown") { - if (server) { - server->shutdown(); + if (dhcp4.server) { + dhcp4.server->shutdown(); } - io_service.stop(); + dhcp4.io_service.stop(); ConstElementPtr answer = isc::config::createAnswer(0, "Shutting down."); return (answer); @@ -101,15 +114,19 @@ dhcp4_command_handler(const string& command, ConstElementPtr args) { return (answer); } +/// @brief callback that will be called from iface_mgr when command/config arrives void session_reader(void) { - io_service.run_one(); + // Process one asio event. If there are more events, iface_mgr will call + // this callback more than once. + dhcp4.io_service.run_one(); } +/// @brief Establishes msgq session. +/// +/// Creates session that will be used to receive commands and updated +/// configuration from boss (or indirectly from user via bindctl). void establish_session() { - Session* cc_session = NULL; - ModuleCCSession* config_session = NULL; - string specfile; if (getenv("B10_FROM_BUILD")) { specfile = string(getenv("B10_FROM_BUILD")) + @@ -120,23 +137,26 @@ void establish_session() { cout << "b10-dhcp4: my specfile is " << specfile << endl; - cc_session = new Session(io_service.get_io_service()); + dhcp4.cc_session = new Session(dhcp4.io_service.get_io_service()); - config_session = new ModuleCCSession(specfile, *cc_session, - dhcp4_config_handler, - dhcp4_command_handler, false); + dhcp4.config_session = new ModuleCCSession(specfile, *dhcp4.cc_session, + dhcp4_config_handler, + dhcp4_command_handler, false); + dhcp4.config_session->start(); - config_session->start(); - - int ctrl_socket = cc_session->getSocketDesc(); + int ctrl_socket = dhcp4.cc_session->getSocketDesc(); cout << "b10-dhcp4: Control session started, socket=" << ctrl_socket << endl; IfaceMgr::instance().set_session_socket(ctrl_socket, session_reader); +} - // cout << "b10-dhcp4: About to call io_service.run()" << endl; - // io_service.run(); - // cout << "b10-dhcp4: Returned from io_service.run()" << endl; +void disconnect_session() { + dhcp4.cc_session->disconnect(); + delete dhcp4.config_session; + dhcp4.config_session = NULL; + delete dhcp4.cc_session; + dhcp4.cc_session = NULL; } int @@ -160,8 +180,7 @@ main(int argc, char* argv[]) { (verbose_mode ? isc::log::DEBUG : isc::log::INFO), isc::log::MAX_DEBUG_LEVEL, NULL); - - cout << "My pid=" << getpid() << endl; + cout << "b10-dhcp4: My pid is " << getpid() << endl; if (argc - optind > 0) { usage(); @@ -174,12 +193,13 @@ main(int argc, char* argv[]) { establish_session(); cout << "[b10-dhcp4] Initiating DHCPv4 server operation." << endl; + dhcp4.server = new Dhcpv4Srv(); + dhcp4.server->run(); - server = new Dhcpv4Srv(); + disconnect_session(); - server->run(); - - delete server; + delete dhcp4.server; + dhcp4.server = NULL; } catch (const std::exception& ex) { cerr << "[b10-dhcp4] Server failed: " << ex.what() << endl; diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc index 16ac706530..c065438214 100644 --- a/src/lib/dhcp/tests/iface_mgr_unittest.cc +++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -153,8 +154,6 @@ TEST_F(IfaceMgrTest, basic) { // checks that IfaceManager can be instantiated createLoInterfacesTxt(); - createLoInterfacesTxt(); - IfaceMgr & ifacemgr = IfaceMgr::instance(); ASSERT_TRUE(&ifacemgr != 0); } @@ -948,4 +947,52 @@ TEST_F(IfaceMgrTest, DISABLED_detectIfaces_linux) { } #endif +volatile bool callback_ok; + +void my_callback(void) { + cout << "Callback triggered." << endl; + callback_ok = true; +} + +TEST_F(IfaceMgrTest, controlSession) { + // tests if extra control socket and its callback can be passed and + // it is supported properly by receive4() method. + + callback_ok = false; + + NakedIfaceMgr* ifacemgr = new NakedIfaceMgr(); + + // create pipe and register it as extra socket + int pipefd[2]; + EXPECT_TRUE(pipe(pipefd) == 0); + EXPECT_NO_THROW(ifacemgr->set_session_socket(pipefd[0], my_callback)); + + Pkt4Ptr pkt4; + pkt4 = ifacemgr->receive4(1); + + // Our callback should not be called this time (there was no data) + EXPECT_FALSE(callback_ok); + + // IfaceMgr should not process control socket data as incoming packets + EXPECT_FALSE(pkt4); + + // Now, send some data over pipe (38 bytes) + EXPECT_EQ(38, write(pipefd[1], "Hi, this is a message sent over a pipe", 38)); + + // ... and repeat + pkt4 = ifacemgr->receive4(1); + + // IfaceMgr should not process control socket data as incoming packets + EXPECT_FALSE(pkt4); + + // There was some data, so this time callback should be called + EXPECT_TRUE(callback_ok); + + delete ifacemgr; + + // close both pipe ends + close(pipefd[1]); + close(pipefd[0]); +} + }