[1651] Tests for IfaceMgr callback implemented, devel doc updated

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

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

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;

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -17,6 +17,7 @@
 #include <fstream>
 #include <sstream>
 
+#include <unistd.h>
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
@@ -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]);
+}
+
 }