[1651] Changes after second review (DHCPv4 msgq integration)

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -22,14 +22,10 @@
 #include <cc/session.h>
 #include <config/ccsession.h>
 #include <util/buffer.h>
-#include <log/dummylog.h>
 #include <dhcp4/spec_config.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp/iface_mgr.h>
 #include <asiolink/asiolink.h>
-#include <log/logger_support.h>
-
-const char* const DHCP4_NAME = "b10-dhcp4";
 
 using namespace std;
 using namespace isc::util;
@@ -60,6 +56,11 @@ ControlledDhcpv4Srv::dhcp4CommandHandler(const string& command, ConstElementPtr
     if (command == "shutdown") {
         if (ControlledDhcpv4Srv::server_) {
             ControlledDhcpv4Srv::server_->shutdown();
+        } else {
+            cout << "Server not initialized yet or already shut down." << endl;
+            ConstElementPtr answer = isc::config::createAnswer(1,
+                                     "Shutdown failure.");
+            return (answer);
         }
         ConstElementPtr answer = isc::config::createAnswer(0,
                                  "Shutting down.");
@@ -118,16 +119,13 @@ void ControlledDhcpv4Srv::disconnectSession() {
         delete cc_session_;
         cc_session_ = NULL;
     }
+
+    // deregister session socket
+    IfaceMgr::instance().set_session_socket(IfaceMgr::INVALID_SOCKET, NULL);
 }
 
-ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/,
-                                             bool verbose /* false */)
+ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
     :Dhcpv4Srv(port), cc_session_(NULL), config_session_(NULL) {
-
-    // Initialize logging.  If verbose, we'll use maximum verbosity.
-    isc::log::initLogger(DHCP4_NAME,
-                         (verbose ? isc::log::DEBUG : isc::log::INFO),
-                         isc::log::MAX_DEBUG_LEVEL, NULL);
     server_ = this; // remember this instance for use in callback
 }
 
@@ -147,7 +145,7 @@ isc::data::ConstElementPtr
 ControlledDhcpv4Srv::execDhcpv4ServerCommand(const std::string& command_id,
                                              isc::data::ConstElementPtr args) {
     try {
-        return dhcp4CommandHandler(command_id, args);
+        return (dhcp4CommandHandler(command_id, args));
     } catch (const Exception& ex) {
         ConstElementPtr answer = isc::config::createAnswer(1, ex.what());
         return (answer);

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -41,9 +41,7 @@ public:
     /// @brief Constructor
     ///
     /// @param port UDP port to be opened for DHCP traffic
-    /// @param verbose should server print out additional commands?
-    ControlledDhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT,
-                        bool verbose = false);
+    ControlledDhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT);
 
     /// @brief Destructor.
     ~ControlledDhcpv4Srv();
@@ -52,12 +50,16 @@ public:
     ///
     /// Creates session that will be used to receive commands and updated
     /// configuration from boss (or indirectly from user via bindctl).
+    ///
+    /// Integrate the asynchronous I/O model of BIND 10 configuration
+    /// control with the "select" model of the DHCP server.  This is
+    /// fully explained in \ref dhcpv4Session.
     void establishSession();
 
     /// @brief Terminates existing msgq session.
     ///
     /// This method terminates existing session with msgq. After calling
-    /// it, not further messages over msgq (commands or configuration updates)
+    /// it, no further messages over msgq (commands or configuration updates)
     /// may be received.
     ///
     /// It is ok to call this method when session is disconnected already.
@@ -76,12 +78,12 @@ public:
     execDhcpv4ServerCommand(const std::string& command,
                             isc::data::ConstElementPtr args);
 
+protected:
     /// @brief Static pointer to the sole instance of the DHCP server.
     ///
     /// This is required for config and command handlers to gain access to
     /// the server
     static ControlledDhcpv4Srv* server_;
-protected:
 
     /// @brief A callback for handling incoming configuration updates.
     ///

src/bin/dhcp4/main.cc

@@ -16,12 +16,15 @@
 #include <iostream>
 #include <exceptions/exceptions.h>
 #include <log/dummylog.h>
+#include <log/logger_support.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp/iface_mgr.h>
 
 using namespace std;
 using namespace isc::dhcp;
 
+
+
 /// This file contains entry point (main() function) for standard DHCPv4 server
 /// component for BIND10 framework. It parses command-line arguments and
 /// instantiates ControlledDhcpv4Srv class that is responsible for establishing
@@ -33,6 +36,8 @@ using namespace isc::dhcp;
 
 namespace {
 
+const char* const DHCP4_NAME = "b10-dhcp4";
+
 void
 usage() {
     cerr << "Usage:  b10-dhcp4 [-v]"
@@ -47,18 +52,22 @@ main(int argc, char* argv[]) {
     int ch;
     bool verbose_mode = false; // should server be verbose?
 
-    while ((ch = getopt(argc, argv, ":v")) != -1) {
+    while ((ch = getopt(argc, argv, "v")) != -1) {
         switch (ch) {
         case 'v':
             verbose_mode = true;
             isc::log::denabled = true;
             break;
-        case ':':
         default:
             usage();
         }
     }
 
+    // Initialize logging.  If verbose, we'll use maximum verbosity.
+    isc::log::initLogger(DHCP4_NAME,
+                         (verbose_mode ? isc::log::DEBUG : isc::log::INFO),
+                         isc::log::MAX_DEBUG_LEVEL, NULL);
+
     cout << "b10-dhcp4: My pid is " << getpid() << endl;
 
     if (argc - optind > 0) {
@@ -66,13 +75,12 @@ main(int argc, char* argv[]) {
     }
 
     int ret = 0;
-    ControlledDhcpv4Srv* server = NULL;
 
     try {
 
         cout << "[b10-dhcp4] Initiating DHCPv4 server operation." << endl;
 
-        server = new ControlledDhcpv4Srv(DHCP4_SERVER_PORT, verbose_mode);
+        ControlledDhcpv4Srv* server = new ControlledDhcpv4Srv(DHCP4_SERVER_PORT);
         server->run();
         delete server;
 

src/lib/dhcp/iface_mgr.cc

@@ -123,7 +123,7 @@ bool IfaceMgr::Iface::delSocket(uint16_t sockfd) {
 IfaceMgr::IfaceMgr()
     :control_buf_len_(CMSG_SPACE(sizeof(struct in6_pktinfo))),
      control_buf_(new char[control_buf_len_]),
-     session_socket_(InvalidSocket), session_callback_(NULL)
+     session_socket_(INVALID_SOCKET), session_callback_(NULL)
 {
 
     cout << "IfaceMgr initialization." << endl;
@@ -718,7 +718,7 @@ IfaceMgr::receive4(uint32_t timeout) {
     }
 
     // if there is session socket registered...
-    if (session_socket_ != InvalidSocket) {
+    if (session_socket_ != INVALID_SOCKET) {
         // at it to the set as well
         FD_SET(session_socket_, &sockets);
         if (maxfd < session_socket_)
@@ -747,8 +747,7 @@ IfaceMgr::receive4(uint32_t timeout) {
     }
 
     // Let's find out which socket has the data
-
-    if ((session_socket_ != InvalidSocket) && (FD_ISSET(session_socket_, &sockets))) {
+    if ((session_socket_ != INVALID_SOCKET) && (FD_ISSET(session_socket_, &sockets))) {
         // something received over session socket
         cout << "BIND10 command or config available over session socket." << endl;
 

src/lib/dhcp/iface_mgr.h

@@ -415,7 +415,7 @@ public:
     }
 
     /// A value of socket descriptor representing "not specified" state.
-    static const int InvalidSocket = -1;
+    static const int INVALID_SOCKET = -1;
 
     // don't use private, we need derived classes in tests
 protected: