Browse Source

Cleanup: Added lots of documentation to libasiolink; improved some
variable names and class names.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac327@3115 e5f2f494-b856-4b98-b285-d166d9295462

Evan Hunt 14 years ago
parent
commit
9bc9618f8d

+ 6 - 6
src/bin/auth/auth_srv.cc

@@ -132,7 +132,7 @@ class MessageLookup : public DNSLookup {
 public:
     MessageLookup(AuthSrv* srv) : server_(srv) {}
     virtual void operator()(const IOMessage& io_message, MessagePtr message,
-                            OutputBufferPtr buffer, IOServer* server) const
+                            OutputBufferPtr buffer, DNSServer* server) const
     {
         server_->processMessage(io_message, message, buffer, server);
     }
@@ -167,10 +167,10 @@ private:
     AuthSrv* server_;
 };
 
-// This is a derived class of \c IOCallback, to serve
+// This is a derived class of \c SimpleCallback, to serve
 // as a callback in the asiolink module.  It checks for queued
 // configuration messages, and executes them if found.
-class ConfigChecker : public IOCallback {
+class ConfigChecker : public SimpleCallback {
 public:
     ConfigChecker(AuthSrv* srv) : server_(srv) {}
     virtual void operator()(const IOMessage& io_message UNUSED_PARAM) const {
@@ -184,14 +184,14 @@ private:
 
 AuthSrv::AuthSrv(const bool use_cache, AbstractXfroutClient& xfrout_client) :
     impl_(new AuthSrvImpl(use_cache, xfrout_client)),
-    checkin_provider_(new ConfigChecker(this)),
+    checkin_(new ConfigChecker(this)),
     dns_lookup_(new MessageLookup(this)),
     dns_answer_(new MessageAnswer(this))
 {}
 
 AuthSrv::~AuthSrv() {
     delete impl_;
-    delete checkin_provider_;
+    delete checkin_;
     delete dns_lookup_;
     delete dns_answer_;
 }
@@ -276,7 +276,7 @@ AuthSrv::configSession() const {
 
 void
 AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
-                        OutputBufferPtr buffer, IOServer* server)
+                        OutputBufferPtr buffer, DNSServer* server)
 {
     InputBuffer request_buffer(io_message.getData(), io_message.getDataSize());
 

+ 4 - 4
src/bin/auth/auth_srv.h

@@ -63,7 +63,7 @@ public:
     void processMessage(const asiolink::IOMessage& io_message,
                         isc::dns::MessagePtr message,
                         isc::dns::OutputBufferPtr buffer,
-                        asiolink::IOServer* server);
+                        asiolink::DNSServer* server);
     void setVerbose(bool on);
     bool getVerbose() const;
     isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr config);
@@ -79,8 +79,8 @@ public:
     asiolink::DNSAnswer* getDNSAnswerProvider() const {
         return (dns_answer_);
     }
-    asiolink::IOCallback* getCheckinProvider() const {
-        return (checkin_provider_);
+    asiolink::SimpleCallback* getCheckinProvider() const {
+        return (checkin_);
     }
 
     ///
@@ -100,7 +100,7 @@ public:
 private:
     AuthSrvImpl* impl_;
     asiolink::IOService* io_service_;
-    asiolink::IOCallback* checkin_provider_;
+    asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;
 };

+ 1 - 1
src/bin/auth/main.cc

@@ -177,7 +177,7 @@ main(int argc, char* argv[]) {
         auth_server->setVerbose(verbose_mode);
         cout << "[b10-auth] Server created." << endl;
 
-        IOCallback* checkin = auth_server->getCheckinProvider();
+        SimpleCallback* checkin = auth_server->getCheckinProvider();
         DNSLookup* lookup = auth_server->getDNSLookupProvider();
         DNSAnswer* answer = auth_server->getDNSAnswerProvider();
 

+ 1 - 1
src/bin/auth/tests/auth_srv_unittest.cc

@@ -123,7 +123,7 @@ private:
     };
 
     // A nonoperative task object to be used in calls to processMessage()
-    class MockTask : public IOServer {
+    class MockTask : public DNSServer {
     public:
         MockTask() : done_(false) {}
         void operator()(asio::error_code ec UNUSED_PARAM,

+ 1 - 1
src/bin/recurse/main.cc

@@ -175,7 +175,7 @@ main(int argc, char* argv[]) {
         recursor ->setVerbose(verbose_mode);
         cout << "[b10-recurse] Server created." << endl;
 
-        IOCallback* checkin = recursor->getCheckinProvider();
+        SimpleCallback* checkin = recursor->getCheckinProvider();
         DNSLookup* lookup = recursor->getDNSLookupProvider();
         DNSAnswer* answer = recursor->getDNSAnswerProvider();
 

+ 14 - 14
src/bin/recurse/recursor.cc

@@ -64,7 +64,7 @@ private:
 public:
     RecursorImpl(const char& forward) :
         config_session_(NULL), verbose_mode_(false),
-        forward_(forward), ioquery_()
+        forward_(forward), rec_query_()
     {}
 
     ~RecursorImpl() {
@@ -72,19 +72,19 @@ public:
     }
 
     void querySetup(IOService& ios) {
-        ioquery_ = new IOQuery(ios, forward_);
+        rec_query_ = new RecursiveQuery(ios, forward_);
     }
 
     void queryShutdown() {
-        if (ioquery_) {
-            delete ioquery_;
+        if (rec_query_) {
+            delete rec_query_;
         }
     }
 
     void processNormalQuery(const IOMessage& io_message,
                             const Question& question, MessagePtr message,
                             OutputBufferPtr buffer,
-                            IOServer* server);
+                            DNSServer* server);
     ModuleCCSession* config_session_;
 
     bool verbose_mode_;
@@ -93,7 +93,7 @@ public:
     const char& forward_;
 
     /// Object to handle upstream queries
-    IOQuery* ioquery_;
+    RecursiveQuery* rec_query_;
 
     /// Currently non-configurable, but will be.
     static const uint16_t DEFAULT_LOCAL_UDPSIZE = 4096;
@@ -171,7 +171,7 @@ public:
 
     // \brief Handle the DNS Lookup
     virtual void operator()(const IOMessage& io_message, MessagePtr message,
-                            OutputBufferPtr buffer, IOServer* server) const
+                            OutputBufferPtr buffer, DNSServer* server) const
     {
         server_->processMessage(io_message, message, buffer, server);
     }
@@ -264,10 +264,10 @@ private:
     Recursor* server_;
 };
 
-// This is a derived class of \c IOCallback, to serve
+// This is a derived class of \c SimpleCallback, to serve
 // as a callback in the asiolink module.  It checks for queued
 // configuration messages, and executes them if found.
-class ConfigCheck : public IOCallback {
+class ConfigCheck : public SimpleCallback {
 public:
     ConfigCheck(Recursor* srv) : server_(srv) {}
     virtual void operator()(const IOMessage& io_message UNUSED_PARAM) const {
@@ -322,7 +322,7 @@ Recursor::configSession() const {
 
 void
 Recursor::processMessage(const IOMessage& io_message, MessagePtr message,
-                        OutputBufferPtr buffer, IOServer* server)
+                        OutputBufferPtr buffer, DNSServer* server)
 {
     InputBuffer request_buffer(io_message.getData(), io_message.getDataSize());
     // First, check the header part.  If we fail even for the base header,
@@ -403,8 +403,8 @@ Recursor::processMessage(const IOMessage& io_message, MessagePtr message,
             makeErrorMessage(message, buffer, Rcode::NOTIMP(),
                          impl_->verbose_mode_);
         } else {
-            // The IOQuery object will post the "resume" event to the
-            // IOServer when an answer arrives, so we don't have to do it now.
+            // The RecursiveQuery object will post the "resume" event to the
+            // DNSServer when an answer arrives, so we don't have to do it now.
             sendAnswer = false;
             impl_->processNormalQuery(io_message, *question, message,
                                       buffer, server);
@@ -419,7 +419,7 @@ Recursor::processMessage(const IOMessage& io_message, MessagePtr message,
 void
 RecursorImpl::processNormalQuery(const IOMessage& io_message,
                                  const Question& question, MessagePtr message,
-                                 OutputBufferPtr buffer, IOServer* server)
+                                 OutputBufferPtr buffer, DNSServer* server)
 {
     const bool dnssec_ok = message->isDNSSECSupported();
 
@@ -428,7 +428,7 @@ RecursorImpl::processNormalQuery(const IOMessage& io_message,
     message->setRcode(Rcode::NOERROR());
     message->setDNSSECSupported(dnssec_ok);
     message->setUDPSize(RecursorImpl::DEFAULT_LOCAL_UDPSIZE);
-    ioquery_->sendQuery(io_message, question, buffer, server);
+    rec_query_->sendQuery(io_message, question, buffer, server);
 }
 
 ConstElementPtr

+ 3 - 3
src/bin/recurse/recursor.h

@@ -61,7 +61,7 @@ public:
     void processMessage(const asiolink::IOMessage& io_message,
                         isc::dns::MessagePtr message,
                         isc::dns::OutputBufferPtr buffer,
-                        asiolink::IOServer* server);
+                        asiolink::DNSServer* server);
     void setVerbose(bool on);
     bool getVerbose() const;
     isc::data::ConstElementPtr updateConfig(isc::data::ConstElementPtr config);
@@ -77,14 +77,14 @@ public:
     asiolink::DNSAnswer* getDNSAnswerProvider() {
         return (dns_answer_);
     }
-    asiolink::IOCallback* getCheckinProvider() {
+    asiolink::SimpleCallback* getCheckinProvider() {
         return (checkin_);
     }
 
 private:
     RecursorImpl* impl_;
     asiolink::IOService* io_;
-    asiolink::IOCallback* checkin_;
+    asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;
 };

+ 1 - 1
src/bin/recurse/tests/recursor_unittest.cc

@@ -95,7 +95,7 @@ private:
     };
 
     // A nonoperative task object to be used in calls to processMessage()
-    class MockTask : public IOServer {
+    class MockTask : public DNSServer {
     public:
         MockTask() : done_(false) {}
         void operator()(asio::error_code ec UNUSED_PARAM,

+ 8 - 8
src/lib/asiolink/asiolink.cc

@@ -93,7 +93,7 @@ IOMessage::IOMessage(const void* data, const size_t data_size,
     remote_endpoint_(remote_endpoint)
 {}
 
-IOQuery::IOQuery(IOService& io_service, const char& forward) :
+RecursiveQuery::RecursiveQuery(IOService& io_service, const char& forward) :
     io_service_(io_service)
 {
     error_code err;
@@ -105,9 +105,9 @@ IOQuery::IOQuery(IOService& io_service, const char& forward) :
 }
 
 void
-IOQuery::sendQuery(const IOMessage& io_message,
-                   const Question& question, OutputBufferPtr buffer,
-                   IOServer* server)
+RecursiveQuery::sendQuery(const IOMessage& io_message,
+                          const Question& question, OutputBufferPtr buffer,
+                          DNSServer* server)
 {
 
     // XXX: eventually we will need to be able to determine whether
@@ -123,7 +123,7 @@ class IOServiceImpl {
 public:
     IOServiceImpl(const char& port,
                   const ip::address* v4addr, const ip::address* v6addr,
-                  IOCallback* checkin, DNSLookup* lookup,
+                  SimpleCallback* checkin, DNSLookup* lookup,
                   DNSAnswer* answer);
     asio::io_service io_service_;
 
@@ -138,7 +138,7 @@ public:
 IOServiceImpl::IOServiceImpl(const char& port,
                              const ip::address* const v4addr,
                              const ip::address* const v6addr,
-                             IOCallback* checkin,
+                             SimpleCallback* checkin,
                              DNSLookup* lookup,
                              DNSAnswer* answer) :
     udp4_server_(UDPServerPtr()), udp6_server_(UDPServerPtr()),
@@ -191,7 +191,7 @@ IOServiceImpl::IOServiceImpl(const char& port,
 }
 
 IOService::IOService(const char& port, const char& address,
-                     IOCallback* checkin,
+                     SimpleCallback* checkin,
                      DNSLookup* lookup,
                      DNSAnswer* answer) :
     impl_(NULL)
@@ -211,7 +211,7 @@ IOService::IOService(const char& port, const char& address,
 
 IOService::IOService(const char& port,
                      const bool use_ipv4, const bool use_ipv6,
-                     IOCallback* checkin,
+                     SimpleCallback* checkin,
                      DNSLookup* lookup,
                      DNSAnswer* answer) :
     impl_(NULL)

+ 227 - 99
src/lib/asiolink/asiolink.h

@@ -70,9 +70,9 @@ class io_service;
 /// to the ASIO counterparts.
 ///
 /// Notes to developers:
-/// Currently the wrapper interface is specific to the authoritative
-/// server implementation.  But the plan is to generalize it and have
-/// other modules use it.
+/// Currently the wrapper interface is fairly specific to use by a
+/// DNS server, i.e., b10-auth or b10-recurse.  But the plan is to
+/// generalize it and have other modules use it as well.
 ///
 /// One obvious drawback of this approach is performance overhead
 /// due to the additional layer.  We should eventually evaluate the cost
@@ -102,6 +102,11 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+/// \brief Forward declarations for classes used below
+class SimpleCallback;
+class DNSLookup;
+class DNSAnswer;
+
 /// \brief The \c IOAddress class represents an IP addresses (version
 /// agnostic)
 ///
@@ -188,11 +193,14 @@ public:
     ///
     /// This method returns an IOAddress object corresponding to \c this
     /// endpoint.
+    ///
     /// Note that the return value is a real object, not a reference or
     /// a pointer.
+    ///
     /// This is aligned with the interface of the ASIO counterpart:
     /// the \c address() method of \c ip::xxx::endpoint classes returns
     /// an \c ip::address object.
+    ///
     /// This also means handling the address of an endpoint using this method
     /// can be expensive.  If the address information is necessary in a
     /// performance sensitive context and there's a more efficient interface
@@ -246,6 +254,7 @@ public:
 /// Derived class implementations are completely hidden within the
 /// implementation.  User applications only get access to concrete
 /// \c IOSocket objects via the abstract interfaces.
+///
 /// We may revisit this decision when we generalize the wrapper and more
 /// modules use it.  Also, at that point we may define a separate (visible)
 /// derived class for testing purposes rather than providing factory methods
@@ -381,6 +390,7 @@ public:
 
     /// \brief Returns the endpoint that sends the message.
     const IOEndpoint& getRemoteEndpoint() const { return (remote_endpoint_); }
+
 private:
     const void* data_;
     const size_t data_size_;
@@ -388,51 +398,200 @@ private:
     const IOEndpoint& remote_endpoint_;
 };
 
-/// XXX: need to add doc
-class IOServer;
-typedef boost::shared_ptr<IOServer> IOServerPtr;
-class IOServer {
+/// \brief The \c IOService class is a wrapper for the ASIO \c io_service
+/// class.
+///
+/// Currently, the interface of this class is very specific to the
+/// authoritative/recursive DNS server implementationss in b10-auth
+/// and b10-recurse; this is reflected in the constructor signatures.
+/// Ultimately the plan is to generalize it so that other BIND 10
+/// modules can use this interface, too.
+class IOService {
+    ///
+    /// \name Constructors and Destructor
+    ///
+    /// These are currently very specific to the authoritative server
+    /// implementation.
+    ///
+    /// Note: The copy constructor and the assignment operator are
+    /// intentionally defined as private, making this class non-copyable.
+    //@{
+private:
+    IOService(const IOService& source);
+    IOService& operator=(const IOService& source);
+public:
+    /// \brief The constructor with a specific IP address and port on which
+    /// the services listen on.
+    IOService(const char& port, const char& address,
+              SimpleCallback* checkin,
+              DNSLookup* lookup,
+              DNSAnswer* answer);
+    /// \brief The constructor with a specific port on which the services
+    /// listen on.
+    ///
+    /// It effectively listens on "any" IPv4 and/or IPv6 addresses.
+    /// IPv4/IPv6 services will be available if and only if \c use_ipv4
+    /// or \c use_ipv6 is \c true, respectively.
+    IOService(const char& port, const bool use_ipv4, const bool use_ipv6,
+              SimpleCallback* checkin,
+              DNSLookup* lookup,
+              DNSAnswer* answer);
+    /// \brief The destructor.
+    ~IOService();
+    //@}
+
+    /// \brief Start the underlying event loop.
+    ///
+    /// This method does not return control to the caller until
+    /// the \c stop() method is called via some handler.
+    void run();
+
+    /// \brief Stop the underlying event loop.
+    ///
+    /// This will return the control to the caller of the \c run() method.
+    void stop();
+
+    /// \brief Return the native \c io_service object used in this wrapper.
+    ///
+    /// This is a short term work around to support other BIND 10 modules
+    /// that share the same \c io_service with the authoritative server.
+    /// It will eventually be removed once the wrapper interface is
+    /// generalized.
+    asio::io_service& get_io_service();
+private:
+    IOServiceImpl* impl_;
+};
+
+/// \brief The \c DNSServer class is a wrapper (and base class) for
+/// classes which provide DNS server functionality.
+/// 
+/// The classes derived from this one, \c TCPServer and \c UDPServer,
+/// act as the interface layer between clients sending queries, and
+/// functions defined elsewhere that provide answers to those queries.
+/// Those functions are described in more detail below under
+/// \c SimpleCallback, \c DNSLookup, and \c DNSAnswer.
+///
+/// Notes to developers:
+/// When constructed, this class (and its derived classes) will have its
+/// "self_" member set to point to "this".  Calls to methods in the base
+/// class are then rerouted via this pointer to methods in the derived
+/// class.  This allows code from outside asiolink, with no specific
+/// knowledge of \c TCPServer or \c UDPServer, to access their methods.
+///
+/// This class is both assignable and copy-constructable.  Its subclasses
+/// use the "stackless coroutine" pattern, meaning that it will copy itself
+/// when "forking", and that instances will be posted as ASIO handler
+/// objects, which are always copied.
+///
+/// Because these objects are frequently copied, it is recommended 
+/// that derived classes be kept small to reduce copy overhead.
+class DNSServer {
+protected: 
+    ///
+    /// \name Constructors and destructors
+    ///
+    /// This is intentionally defined as \c protected, as this base class
+    /// should never be instantiated except as part of a derived class.
+    //@{
+    DNSServer() : self_(this) {}
 public:
-    IOServer() : self_(this), cloned_(false) {}
+    /// \brief The destructor
+    virtual ~DNSServer() {}
+    //@}
 
+    ///
+    /// \name Class methods
+    ///
+    /// These methods all make their calls indirectly via the "self_"
+    /// pointer, ensuring that the functions ultimately invoked will be
+    /// the ones in the derived class.
+    ///
+    //@{
+    /// \brief The funtion operator
     virtual void operator()(asio::error_code ec = asio::error_code(),
                             size_t length = 0)
     {
         (*self_)(ec, length);
     }
 
-    virtual void doLookup() { self_->doLookup(); }
-    virtual void resume(const bool done) { self_->resume(done); }
-    virtual bool hasAnswer() { return (self_->hasAnswer()); }
-    virtual int value() { return (self_->value()); }
-    virtual IOServer* clone() { return (self_->clone()); }
+    /// \brief Resume processing of the server coroutine after an 
+    /// asynchronous call (e.g., to the DNS Lookup provider) has completed.
+    virtual inline void resume(const bool done) { self_->resume(done); }
 
-private:
-    IOServer* self_;
+    /// \brief Indicate whether the server is able to send an answer
+    /// to a query.
+    /// 
+    /// This is presently used only for testing purposes.
+    virtual inline bool hasAnswer() { return (self_->hasAnswer()); }
+
+    /// \brief Returns the current value of the 'coroutine' object
+    ///
+    /// This is a temporary method, intended to be used for debugging
+    /// purposes during development and removed later.  It allows
+    /// callers from outside the coroutine object to retrieve information
+    /// about its current state.
+    virtual inline int value() { return (self_->value()); }
+
+    /// \brief Returns a pointer to a clone of this DNSServer object.
+    ///
+    /// When a \c DNSServer object is copied or assigned, the result will
+    /// normally be another \c DNSServer object containing a copy
+    /// of the original "self_" pointer.  Calling clone() guarantees
+    /// that the underlying object is also correctly copied.
+    virtual inline DNSServer* clone() { return (self_->clone()); }
+    //@}
 
 protected:
-    bool cloned_;
-};
+    /// \brief Lookup handler object.
+    ///
+    /// This is a protected class; it can only be instantiated
+    /// from within a derived class of \c DNSServer.
+    ///
+    /// A server object that has received a query creates an instance
+    /// of this class and scheudles it on the ASIO service queue
+    /// using asio::io_service::post().  When the handler executes, it
+    /// calls the asyncLookup() method in the server object to start a
+    /// DNS lookup.  When the lookup is complete, the server object is
+    /// scheduled to resume, again using io_service::post().
+    ///
+    /// Note that the calling object is copied into the handler object,
+    /// not referenced.  This is because, once the calling object yields
+    /// control to the handler, it falls out of scope and may disappear
+    template <typename T>
+    class AsyncLookup {
+    public:
+        AsyncLookup(T caller) : caller_(caller) {}
+        inline void operator()() { caller_.asyncLookup(); }
+    private:
+        T caller_;
+    };
+
+    /// \brief Carries out a DNS lookup.
+    ///
+    /// This function calls the \c DNSLookup object specified by the
+    /// DNS server when the \c IOService was created, passing along
+    /// the details of the query and a pointer back to the current
+    /// server object.  It is called asynchronously via the AsyncLookup
+    /// handler class.
+    virtual inline void asyncLookup() { self_->asyncLookup(); }
 
-template <typename T>
-class LookupHandler {
-public:
-    LookupHandler(T caller) : caller_(caller) {}
-    void operator()() {
-        caller_.doLookup();
-    }
 private:
-    T caller_;
+    DNSServer* self_;
 };
 
 /// \brief The \c DNSLookup class is an abstract base class for a DNS
-/// provider function.
+/// Lookup provider function.
 ///
 /// Specific derived class implementations are hidden within the
 /// implementation.  Instances of the derived classes can be called
 /// as functions via the operator() interface.  Pointers to these
 /// instances can then be provided to the \c IOService class
 /// via its constructor.
+///
+/// A DNS Lookup provider function obtains the data needed to answer
+/// a DNS query (e.g., from authoritative data source, cache, or upstream
+/// query).  After it has run, the OutputBuffer object passed to it
+/// should contain the answer to the query, in an internal representation.
 class DNSLookup {
     ///
     /// \name Constructors and Destructor
@@ -452,6 +611,7 @@ protected:
 public:
     /// \brief The destructor
     virtual ~DNSLookup() {}
+    //@}
     /// \brief The function operator
     ///
     /// This makes its call indirectly via the "self" pointer, ensuring
@@ -460,23 +620,27 @@ public:
     virtual void operator()(const IOMessage& io_message,
                             isc::dns::MessagePtr message,
                             isc::dns::OutputBufferPtr buffer,
-                            IOServer* server) const
+                            DNSServer* server) const
     {
         (*self_)(io_message, message, buffer, server);
     }
-    //@}
 private:
     DNSLookup* self_;
 };
 
 /// \brief The \c DNSAnswer class is an abstract base class for a DNS
-/// provider function.
+/// Answer provider function.
 ///
 /// Specific derived class implementations are hidden within the
 /// implementation.  Instances of the derived classes can be called
 /// as functions via the operator() interface.  Pointers to these
 /// instances can then be provided to the \c IOService class
 /// via its constructor.
+///
+/// A DNS Answer provider function takes answer data that has been obtained
+/// from a DNS Lookup provider functon and readies it to be sent to the
+/// client.  After it has run, the OutputBuffer object passed to it should
+/// contain the answer to the query rendered into wire format.
 class DNSAnswer {
     ///
     /// \name Constructors and Destructor
@@ -496,14 +660,14 @@ protected:
 public:
     /// \brief The destructor
     virtual ~DNSAnswer() {}
+    //@}
     /// \brief The function operator
     virtual void operator()(const IOMessage& io_message,
                             isc::dns::MessagePtr message,
                             isc::dns::OutputBufferPtr buffer) const = 0;
-    //@}
 };
 
-/// \brief The \c IOCallback class is an abstract base class for a
+/// \brief The \c SimpleCallback class is an abstract base class for a
 /// simple callback function with the signature:
 ///
 /// Specific derived class implementations are hidden within the
@@ -511,7 +675,11 @@ public:
 /// as functions via the operator() interface.  Pointers to these
 /// instances can then be provided to the \c IOService class
 /// via its constructor.
-class IOCallback {
+///
+/// The \c SimpleCallback is expected to be used for basic, generic
+/// tasks such as checking for configuration changes.  It may also be
+/// used for testing purposes.
+class SimpleCallback {
     ///
     /// \name Constructors and Destructor
     ///
@@ -519,18 +687,19 @@ class IOCallback {
     /// intentionally defined as private, making this class non-copyable.
     //@{
 private:
-    IOCallback(const IOCallback& source);
-    IOCallback& operator=(const IOCallback& source);
+    SimpleCallback(const SimpleCallback& source);
+    SimpleCallback& operator=(const SimpleCallback& source);
 protected:
     /// \brief The default constructor.
     ///
     /// This is intentionally defined as \c protected as this base class
     /// should never be instantiated (except as part of a derived class).
-    IOCallback() : self_(this) {}
+    SimpleCallback() : self_(this) {}
 public:
     /// \brief The destructor
-    virtual ~IOCallback() {}
+    virtual ~SimpleCallback() {}
     /// \brief The function operator
+    //@}
     ///
     /// This makes its call indirectly via the "self" pointer, ensuring
     /// that the function ultimately invoked will be the one in the derived
@@ -538,85 +707,44 @@ public:
     virtual void operator()(const IOMessage& io_message) const {
         (*self_)(io_message);
     }
-    //@}
 private:
-    IOCallback* self_;
+    SimpleCallback* self_;
 };
 
-/// \brief The \c IOService class is a wrapper for the ASIO \c io_service
-/// class.
+/// \brief The \c RecursiveQuery class provides a layer of abstraction around
+/// the ASIO code that carries out an upstream query.
 ///
-/// Currently, the interface of this class is very specific to the
-/// authoritative server implementation as indicated in the signature of
-/// the constructor, but the plan is to generalize it so that other BIND 10
-/// modules can use this interface, too.
-class IOService {
-    ///
-    /// \name Constructors and Destructor
+/// This design is very preliminary; currently it is only capable of
+/// handling simple forward requests to a single resolver.
+class RecursiveQuery {
     ///
-    /// These are currently very specific to the authoritative server
-    /// implementation.
+    /// \name Constructors
     ///
-    /// Note: The copy constructor and the assignment operator are
-    /// intentionally defined as private, making this class non-copyable.
     //@{
-private:
-    IOService(const IOService& source);
-    IOService& operator=(const IOService& source);
 public:
-    /// \brief The constructor with a specific IP address and port on which
-    /// the services listen on.
-    IOService(const char& port, const char& address,
-              IOCallback* checkin,
-              DNSLookup* lookup,
-              DNSAnswer* answer);
-    /// \brief The constructor with a specific port on which the services
-    /// listen on.
+    /// \brief Constructor for use when acting as a forwarder
     ///
-    /// It effectively listens on "any" IPv4 and/or IPv6 addresses.
-    /// IPv4/IPv6 services will be available if and only if \c use_ipv4
-    /// or \c use_ipv6 is \c true, respectively.
-    IOService(const char& port, const bool use_ipv4, const bool use_ipv6,
-              IOCallback* checkin,
-              DNSLookup* lookup,
-              DNSAnswer* answer);
-    /// \brief The destructor.
-    ~IOService();
+    /// This is currently the only way to construct \c RecursiveQuery
+    /// object.  The address of the forward nameserver is specified,
+    /// and all upstream queries will be sent to that one address.
+    RecursiveQuery(IOService& io_service, const char& forward);
     //@}
 
-    /// \brief Start the underlying event loop.
-    ///
-    /// This method does not return control to the caller until
-    /// the \c stop() method is called via some handler.
-    void run();
-
-    /// \brief Stop the underlying event loop.
+    /// \brief Initiates an upstream query in the \c RecursiveQuery object.
     ///
-    /// This will return the control to the caller of the \c run() method.
-    void stop();
-
-    /// \brief Return the native \c io_service object used in this wrapper.
+    /// \param io_message The message from the original client
+    /// \param question The question being answered <qname/qclass/qtype>
+    /// \param buffer An output buffer into which the response can be copied
+    /// \param server A pointer to the \c DNSServer object handling the client
     ///
-    /// This is a short term work around to support other BIND 10 modules
-    /// that share the same \c io_service with the authoritative server.
-    /// It will eventually be removed once the wrapper interface is
-    /// generalized.
-    asio::io_service& get_io_service();
-private:
-    IOServiceImpl* impl_;
-};
-
-/// \brief The \c IOQuery class provides a layer of abstraction around
-/// the ASIO code that carries out upstream queries.
-class IOQuery {
-public:
-    IOQuery(IOService& io_service, const char& forward);
-
-    /// \brief Sends a query to the IOQuery object.
+    /// When sendQuery() is called, a message is sent asynchronously to
+    /// the upstream name server.  When a reply arrives, 'server'
+    /// is placed on the ASIO service queue via io_service::post(), so
+    /// that the original \c DNSServer objct can resume processing.
     void sendQuery(const IOMessage& io_message,
                    const isc::dns::Question& question,
                    isc::dns::OutputBufferPtr buffer,
-                   IOServer* server);
+                   DNSServer* server);
 private:
     IOService& io_service_;
     asio::ip::address ns_addr_;

+ 63 - 19
src/lib/asiolink/internal/tcpdns.h

@@ -29,12 +29,18 @@
 #include <asiolink/asiolink.h>
 #include <asiolink/internal/coroutine.h>
 
+// This file contains TCP-specific implementations of generic classes 
+// defined in asiolink.h.  It is *not* intended to be part of the public
+// API.
+
 namespace asiolink {
-// Note: this implementation is optimized for the case where this object
-// is created from an ASIO endpoint object in a receiving code path
-// by avoiding to make a copy of the base endpoint.  For TCP it may not be
-// a big deal, but when we receive UDP packets at a high rate, the copy
-// overhead might be significant.
+/// \brief A TCP-specific \c IOEndpoint object.
+///
+/// Note: this implementation is optimized for the case where this object
+/// is created from an ASIO endpoint object in a receiving code path
+/// by avoiding the need to copy of the base endpoint.  For TCP, it may
+/// not be a big deal, but when we receive UDP packets at a high rate,
+/// the copy overhead could be significant.
 class TCPEndpoint : public IOEndpoint {
 public:
     TCPEndpoint(const IOAddress& address, const unsigned short port) :
@@ -57,6 +63,7 @@ private:
     const asio::ip::tcp::endpoint& asio_endpoint_;
 };
 
+/// \brief A TCP-specific \c IOSocket object.
 class TCPSocket : public IOSocket {
 private:
     TCPSocket(const TCPSocket& source);
@@ -69,28 +76,27 @@ private:
     asio::ip::tcp::socket& socket_;
 };
 
-//
-// Asynchronous TCP server coroutine
-//
-class TCPServer : public virtual IOServer, public virtual coroutine {
+/// \brief A TCP-specific \c DNSServer object.
+///
+/// This class inherits from both \c DNSServer and from \c coroutine,
+/// defined in coroutine.h. 
+class TCPServer : public virtual DNSServer, public virtual coroutine {
 public:
     explicit TCPServer(asio::io_service& io_service,
                        const asio::ip::address& addr, const uint16_t port, 
-                       const IOCallback* checkin = NULL,
+                       const SimpleCallback* checkin = NULL,
                        const DNSLookup* lookup = NULL,
                        const DNSAnswer* answer = NULL);
 
     void operator()(asio::error_code ec = asio::error_code(),
                     size_t length = 0);
-
-    void doLookup();
+    void asyncLookup();
     void resume(const bool done);
     bool hasAnswer() { return (done_); }
     int value() { return (get_value()); }
 
-    IOServer* clone() {
+    DNSServer* clone() {
         TCPServer* s = new TCPServer(*this);
-        s->cloned_ = true;
         return (s);
     }
 
@@ -98,21 +104,59 @@ private:
     enum { MAX_LENGTH = 65535 };
     static const size_t TCP_MESSAGE_LENGTHSIZE = 2;
 
+    // The ASIO service object
     asio::io_service& io_;
 
     // Class member variables which are dynamic, and changes to which
     // need to accessible from both sides of a coroutine fork or from
     // outside of the coroutine (i.e., from an asynchronous I/O call),
     // should be declared here as pointers and allocated in the
-    // constructor or in the coroutine.
+    // constructor or in the coroutine.  This allows state information
+    // to persist when an individual copy of the coroutine falls out
+    // scope while waiting for an event, *so long as* there is another
+    // object that is referencing the same data.  As a side-benefit, using
+    // pointers also reduces copy overhead for coroutine objects.
+    //
+    // Note: Currently these objects are allocated by "new" in the
+    // constructor, or in the function operator while processing a query.
+    // Repeated allocations from the heap for every incoming query is
+    // clearly a performance issue; this must be optimized in the future.
+    // The plan is to have a structure pre-allocate several "server state"
+    // objects which can be pulled off a free list and placed on an in-use
+    // list whenever a query comes in.  This will serve the dual purpose
+    // of improving performance and guaranteeing that state information
+    // will *not* be destroyed when any one instance of the coroutine
+    // falls out of scope while waiting for an event.
+    //
+    // An ASIO acceptor object to handle new connections.  Created in
+    // the constructor.
     boost::shared_ptr<asio::ip::tcp::acceptor> acceptor_;
+
+    // Socket used to for listen for queries.  Created in the
+    // constructor and stored in a shared_ptr because socket objects
+    // are not copyable.
     boost::shared_ptr<asio::ip::tcp::socket> socket_;
+
+    // An \c IOSocket object to wrap socket_
+    boost::shared_ptr<asiolink::IOSocket> iosock_;
+
+    // An \c IOEndpoint object to wrap the remote endpoint of socket_
+    boost::shared_ptr<asiolink::IOEndpoint> peer_;
+
+    // A small buffer for writing the length of a DNS message;
+    // this is prepended to the actual response buffer when sending a reply
+    // to the client.
     boost::shared_ptr<isc::dns::OutputBuffer> lenbuf_;
+
+    // The buffer into which the response is written
     boost::shared_ptr<isc::dns::OutputBuffer> respbuf_;
-    boost::shared_ptr<asiolink::IOEndpoint> peer_;
-    boost::shared_ptr<asiolink::IOSocket> iosock_;
+
+    // \c IOMessage and \c Message objects to be passed to the
+    // DNS lookup and answer providers
     boost::shared_ptr<asiolink::IOMessage> io_message_;
     isc::dns::MessagePtr message_;
+
+    // The buffer into which the query packet is written
     boost::shared_ptr<char> data_;
 
     // State information that is entirely internal to a given instance
@@ -120,8 +164,8 @@ private:
     size_t bytes_;
     bool done_;
 
-    // Callbacks
-    const IOCallback* checkin_callback_;
+    // Callback functions provided by the caller
+    const SimpleCallback* checkin_callback_;
     const DNSLookup* lookup_callback_;
     const DNSAnswer* answer_callback_;
 };

+ 80 - 21
src/lib/asiolink/internal/udpdns.h

@@ -30,12 +30,15 @@
 #include <asiolink/asiolink.h>
 #include <asiolink/internal/coroutine.h>
 
+// This file contains UDP-specific implementations of generic classes 
+// defined in asiolink.h.  It is *not* intended to be part of the public
+// API.
+
 namespace asiolink {
 // Note: this implementation is optimized for the case where this object
 // is created from an ASIO endpoint object in a receiving code path
-// by avoiding to make a copy of the base endpoint.  For TCP it may not be
-// a big deal, but when we receive UDP packets at a high rate, the copy
-// overhead might be significant.
+// by avoiding to make a copy of the base endpoint.  Otherwise, when we
+// receive UDP packets at a high rate, the copy overhead might be significant.
 class UDPEndpoint : public IOEndpoint {
 public:
     UDPEndpoint(const IOAddress& address, const unsigned short port) :
@@ -73,55 +76,86 @@ private:
 //
 // Asynchronous UDP server coroutine
 //
-class UDPServer : public virtual IOServer, public virtual coroutine {
+class UDPServer : public virtual DNSServer, public virtual coroutine {
 public:
     explicit UDPServer(asio::io_service& io_service,
                        const asio::ip::address& addr, const uint16_t port,
-                       IOCallback* checkin = NULL,
+                       SimpleCallback* checkin = NULL,
                        DNSLookup* lookup = NULL,
                        DNSAnswer* answer = NULL);
 
     void operator()(asio::error_code ec = asio::error_code(),
                     size_t length = 0);
 
-    enum { MAX_LENGTH = 4096 };
-    asio::ip::udp::endpoint peer;
-
-    void doLookup();
+    void asyncLookup();
     void resume(const bool done);
     bool hasAnswer() { return (done_); }
     int value() { return (get_value()); }
 
-    IOServer* clone() {
+    DNSServer* clone() {
         UDPServer* s = new UDPServer(*this);
-        s->cloned_ = true;
         return (s);
     }
 
 private:
+    enum { MAX_LENGTH = 4096 };
+
+    // The ASIO service object
     asio::io_service& io_;
 
     // Class member variables which are dynamic, and changes to which
     // need to accessible from both sides of a coroutine fork or from
     // outside of the coroutine (i.e., from an asynchronous I/O call),
     // should be declared here as pointers and allocated in the
-    // constructor or in the coroutine.
+    // constructor or in the coroutine.  This allows state information
+    // to persist when an individual copy of the coroutine falls out
+    // scope while waiting for an event, *so long as* there is another
+    // object that is referencing the same data.  As a side-benefit, using
+    // pointers also reduces copy overhead for coroutine objects.
+    //
+    // Note: Currently these objects are allocated by "new" in the
+    // constructor, or in the function operator while processing a query.
+    // Repeated allocations from the heap for every incoming query is
+    // clearly a performance issue; this must be optimized in the future.
+    // The plan is to have a structure pre-allocate several "server state"
+    // objects which can be pulled off a free list and placed on an in-use
+    // list whenever a query comes in.  This will serve the dual purpose
+    // of improving performance and guaranteeing that state information
+    // will *not* be destroyed when any one instance of the coroutine
+    // falls out of scope while waiting for an event.
+    //
+    // Socket used to for listen for queries.  Created in the
+    // constructor and stored in a shared_ptr because socket objects
+    // are not copyable.
     boost::shared_ptr<asio::ip::udp::socket> socket_;
-    boost::shared_ptr<char> data_;
+
+    // An \c IOSocket object to wrap socket_
+    boost::shared_ptr<asiolink::IOSocket> iosock_;
+
+    // The ASIO-enternal endpoint object representing the client
     boost::shared_ptr<asio::ip::udp::endpoint> sender_;
+
+    // An \c IOEndpoint object to wrap sender_
     boost::shared_ptr<asiolink::IOEndpoint> peer_;
-    boost::shared_ptr<asiolink::IOSocket> iosock_;
+
+    // \c IOMessage and \c Message objects to be passed to the
+    // DNS lookup and answer providers
     boost::shared_ptr<asiolink::IOMessage> io_message_;
     isc::dns::MessagePtr message_;
+
+    // The buffer into which the response is written
     isc::dns::OutputBufferPtr respbuf_;
+    
+    // The buffer into which the query packet is written
+    boost::shared_ptr<char> data_;
 
     // State information that is entirely internal to a given instance
     // of the coroutine can be declared here.
     size_t bytes_;
     bool done_;
 
-    // Callbacks
-    const IOCallback* checkin_callback_;
+    // Callback functions provided by the caller
+    const SimpleCallback* checkin_callback_;
     const DNSLookup* lookup_callback_;
     const DNSAnswer* answer_callback_;
 };
@@ -136,22 +170,47 @@ public:
                       const isc::dns::Question& q,
                       const asio::ip::address& addr,
                       isc::dns::OutputBufferPtr buffer,
-                      IOServer* server);
+                      DNSServer* server);
     void operator()(asio::error_code ec = asio::error_code(),
                     size_t length = 0); 
 private:
     enum { MAX_LENGTH = 4096 };
 
+    // The \c UDPQuery coroutine never forks, but it is copied whenever
+    // it calls an async_*() function, so it's best to keep copy overhead
+    // small by using pointers or references when possible.  However, this
+    // is not always possible.
+    //
+    // Socket used to for upstream queries. Created in the
+    // constructor and stored in a shared_ptr because socket objects
+    // are not copyable.
     boost::shared_ptr<asio::ip::udp::socket> socket_;
+
+    // The remote endpoint.  Instantiated in the constructor.  Not
+    // stored as a shared_ptr because copy overhead of an endpoint
+    // object is no larger than that of a shared_ptr.
     asio::ip::udp::endpoint remote_;
+
+    // The question being answered.  Copied rather than referenced
+    // because the object that created it is not guaranteed to persist.
     isc::dns::Question question_;
-    isc::dns::OutputBuffer msgbuf_;
+
+    // The output buffer supplied by the caller.  The resposne frmo
+    // the upstream server will be copied here.
     isc::dns::OutputBufferPtr buffer_;;
+
+    // These are allocated for each new query and are stored as
+    // shared pointers to minimize copy overhead.
+    isc::dns::OutputBufferPtr msgbuf_;
     boost::shared_array<char> data_;
 
-    /// \brief The UDP or TCP Server object from which the query originated.
-    // IOServerPtr server_;
-    IOServer* server_;
+    // The UDP or TCP Server object from which the query originated.
+    // Note: Using a shared_ptr for this can cause problems when
+    // control is being transferred from this coroutine to the server;
+    // the reference count can drop to zero and cause the server to be
+    // destroyed before it executes.  Consequently in this case it's
+    // safer to use a raw pointer.
+    DNSServer* server_;
 };
 }
 

+ 56 - 16
src/lib/asiolink/tcpdns.cc

@@ -42,40 +42,52 @@ using namespace std;
 using namespace isc::dns;
 
 namespace asiolink {
-
+/// The following functions provide TCP-specific concrete implementations
+/// of the \c IOEndpoint and \c IOSocket classes.
+///
+/// \brief Returns the address of an endpoint
 IOAddress
 TCPEndpoint::getAddress() const {
     return (asio_endpoint_.address());
 }
 
+/// \brief Returns the port of an endpoint
 uint16_t
 TCPEndpoint::getPort() const {
     return (asio_endpoint_.port());
 }
 
+/// \brief Returns the protocol number of an endpoint.
 short
 TCPEndpoint::getProtocol() const {
     return (asio_endpoint_.protocol().protocol());
 }
 
+/// \brief Returns the address family of an endpoint.
 short
 TCPEndpoint::getFamily() const {
     return (asio_endpoint_.protocol().family());
 }
 
+/// \brief Returns the native socket descriptor for a socket.
 int
 TCPSocket::getNative() const {
     return (socket_.native());
 }
 
+/// \brief Returns the protocol number for a socket (since this 
+/// is the TCP-specific implementation, that is always IPPROTO_TCP).
 int
 TCPSocket::getProtocol() const {
     return (IPPROTO_TCP);
 }
 
+/// The following functions implement the \c UDPServer class.
+///
+/// The constructor
 TCPServer::TCPServer(io_service& io_service,
                      const ip::address& addr, const uint16_t port, 
-                     const IOCallback* checkin,
+                     const SimpleCallback* checkin,
                      const DNSLookup* lookup,
                      const DNSAnswer* answer) :
     io_(io_service), done_(false),
@@ -93,6 +105,7 @@ TCPServer::TCPServer(io_service& io_service,
     acceptor_->set_option(tcp::acceptor::reuse_address(true));
     acceptor_->bind(endpoint);
     acceptor_->listen();
+
     lenbuf_.reset(new OutputBuffer(TCP_MESSAGE_LENGTHSIZE));
     respbuf_.reset(new OutputBuffer(0));
 }
@@ -103,36 +116,47 @@ TCPServer::operator()(error_code ec, size_t length) {
         return;
     }
 
+    /// Because the coroutine reeentry block is implemented as
+    /// a switch statement, inline variable declarations are not
+    /// permitted.  Certain variables used below can be declared here.
     boost::array<const_buffer,2> bufs;
+
     CORO_REENTER (this) {
         do {
+            /// Create a socket to listen for connections
             socket_.reset(new tcp::socket(acceptor_->get_io_service()));
+
+            /// Wait for new connections
             CORO_YIELD acceptor_->async_accept(*socket_, *this);
+
+            /// Fork the coroutine by creating a copy of this one and
+            /// scheduling it on the ASIO service queue.  The parent
+            /// will continue listening for DNS connections while the
+            /// handles the one that has just arrived.
             CORO_FORK io_.post(TCPServer(*this));
         } while (is_parent());
 
-        // Instantiate the data buffer that will be used by the
-        // asynchronous read call.
+        /// Instantiate the data buffer that will be used by the
+        /// asynchronous read call.
         data_ = boost::shared_ptr<char>(new char[MAX_LENGTH]);
 
-        // Read the message length.
+        /// First, read the message length.
         CORO_YIELD async_read(*socket_, asio::buffer(data_.get(),
                               TCP_MESSAGE_LENGTHSIZE), *this);
 
-        // Now read the message itself. (This is done in a different scope
-        // because CORO_REENTER is implemented as a switch statement; the
-        // inline variable declaration of "msglen" and "dnsbuffer" are
-        // therefore not permitted in this scope.)
+        /// Now read the message itself. (This is done in a different scope
+        /// to allow inline variable declarations.)
         CORO_YIELD {
             InputBuffer dnsbuffer((const void *) data_.get(), length);
             uint16_t msglen = dnsbuffer.readUint16();
             async_read(*socket_, asio::buffer(data_.get(), msglen), *this);
         }
 
-        // Store the io_message data.
+        // Create an \c IOMessage object to store the query.
         peer_.reset(new TCPEndpoint(socket_->remote_endpoint()));
         iosock_.reset(new TCPSocket(*socket_));
         io_message_.reset(new IOMessage(data_.get(), length, *iosock_, *peer_));
+        bytes_ = length;
 
         // Perform any necessary operations prior to processing the incoming
         // packet (e.g., checking for queued configuration messages).
@@ -144,7 +168,8 @@ TCPServer::operator()(error_code ec, size_t length) {
             (*checkin_callback_)(*io_message_);
         }
 
-        // Just stop here if we don't have a DNS callback function.
+        // If we don't have a DNS Lookup provider, there's no point in
+        // continuing; we exit the coroutine permanently.
         if (lookup_callback_ == NULL) {
             CORO_YIELD return;
         }
@@ -154,30 +179,45 @@ TCPServer::operator()(error_code ec, size_t length) {
         respbuf_->clear();
         message_.reset(new Message(Message::PARSE));
 
-        // Process the DNS message.
-        bytes_ = length;
-        CORO_YIELD io_.post(LookupHandler<TCPServer>(*this));
+        // Schedule a DNS lookup, and yield.  When the lookup is
+        // finished, the coroutine will resume immediately after
+        // this point.
+        CORO_YIELD io_.post(AsyncLookup<TCPServer>(*this));
 
+        // The 'done_' flag indicates whether we have an answer
+        // to send back.  If not, exit the coroutine permanently.
         if (!done_) {
             CORO_YIELD return;
         }
 
+        // Call the DNS answer provider to render the answer into
+        // wire format
         (*answer_callback_)(*io_message_, message_, respbuf_);
 
-        // Send the response.
+        // Set up the response, beginning with two length bytes.
         lenbuf_->clear();
         lenbuf_->writeUint16(respbuf_->getLength());
         bufs[0] = buffer(lenbuf_->getData(), lenbuf_->getLength());
         bufs[1] = buffer(respbuf_->getData(), respbuf_->getLength());
+
+        // Begin an asynchronous send, and then yield.  When the
+        // send completes, we will resume immediately after this point
+        // (though we have nothing further to do, so the coroutine
+        // will simply exit at that time).
         CORO_YIELD async_write(*socket_, bufs, *this);
     }
 }
 
+/// Call the DNS lookup provider.  (Expected to be called by the
+/// AsyncLookup<TCPServer> handler.)
 void
-TCPServer::doLookup() {
+TCPServer::asyncLookup() {
     (*lookup_callback_)(*io_message_, message_, respbuf_, this);
 }
 
+/// Post this coroutine on the ASIO service queue so that it will
+/// resume processing where it left off.  The 'done' parameter indicates
+/// whether there is an answer to return to the client.
 void
 TCPServer::resume(const bool done) {
     done_ = done;

+ 1 - 1
src/lib/asiolink/tests/asio_link_unittest.cc

@@ -286,7 +286,7 @@ protected:
                             expected_data, expected_datasize);
     }
 private:
-    class ASIOCallBack : public IOCallback {
+    class ASIOCallBack : public SimpleCallback {
     public:
         ASIOCallBack(ASIOLinkTest* test_obj) : test_obj_(test_obj) {}
         void operator()(const IOMessage& io_message) const {

+ 86 - 20
src/lib/asiolink/udpdns.cc

@@ -45,47 +45,61 @@ using namespace std;
 using namespace isc::dns;
 
 namespace asiolink {
+/// The following functions provide UDP-specific concrete implementations
+/// of the \c IOEndpoint and \c IOSocket classes.
+///
+/// \brief Returns the address of an endpoint
 IOAddress
 UDPEndpoint::getAddress() const {
     return (asio_endpoint_.address());
 }
 
+/// \brief Returns the port of an endpoint
 uint16_t
 UDPEndpoint::getPort() const {
     return (asio_endpoint_.port());
 }
 
+/// \brief Returns the protocol number of an endpoint.
 short
 UDPEndpoint::getProtocol() const {
     return (asio_endpoint_.protocol().protocol());
 }
 
+/// \brief Returns the address family of an endpoint.
 short
 UDPEndpoint::getFamily() const {
     return (asio_endpoint_.protocol().family());
 }
 
+/// \brief Returns the native socket descriptor for a socket.
 int
 UDPSocket::getNative() const {
     return (socket_.native());
 }
 
+/// \brief Returns the protocol number for a socket (since this 
+/// is the UDP-specific implementation, that is always IPPROTO_UDP).
 int
 UDPSocket::getProtocol() const {
     return (IPPROTO_UDP);
 }
 
+/// The following functions implement the \c UDPServer class.
+///
+/// The constructor
 UDPServer::UDPServer(io_service& io_service,
                      const ip::address& addr, const uint16_t port,
-                     IOCallback* checkin,
+                     SimpleCallback* checkin,
                      DNSLookup* lookup,
                      DNSAnswer* answer) :
     io_(io_service), done_(false),
-    checkin_callback_(checkin), lookup_callback_(lookup),
+    checkin_callback_(checkin),
+    lookup_callback_(lookup),
     answer_callback_(answer)
 {
-    // Wwe use a different instantiation for v4,
-    // otherwise asio will bind to both v4 and v6
+    // We must use different instantiations for v4 and v6;
+    // otherwise ASIO will bind to both
     udp proto = addr.is_v4() ? udp::v4() : udp::v6();
     socket_.reset(new udp::socket(io_service, proto));
     socket_->set_option(socket_base::reuse_address(true));
@@ -95,6 +109,8 @@ UDPServer::UDPServer(io_service& io_service,
     socket_->bind(udp::endpoint(proto, port));
 }
 
+/// The function operator is implemented with the "stackless coroutine"
+/// pattern; see internal/coroutine.h for details.
 void
 UDPServer::operator()(error_code ec, size_t length) {
     CORO_REENTER (this) {
@@ -103,105 +119,155 @@ UDPServer::operator()(error_code ec, size_t length) {
             // be used by the asynchronous receive call.
             data_.reset(new char[MAX_LENGTH]);
             sender_.reset(new udp::endpoint());
+
             do {
+                // Begin an asynchronous receive, then yield.
+                // When the receive event is posted, the coroutine
+                // will resume immediately after this point.
                 CORO_YIELD socket_->async_receive_from(buffer(data_.get(),
                                                               MAX_LENGTH),
                                                   *sender_, *this);
             } while (ec || length == 0);
 
             bytes_ = length;
+
+            /// Fork the coroutine by creating a copy of this one and
+            /// scheduling it on the ASIO service queue.  The parent
+            /// will continue listening for DNS packets while the child
+            /// processes the one that has just arrived.
             CORO_FORK io_.post(UDPServer(*this));
         } while (is_parent());
 
-        // Store the io_message data.
+        // Create an \c IOMessage object to store the query.
         peer_.reset(new UDPEndpoint(*sender_));
         iosock_.reset(new UDPSocket(*socket_));
         io_message_.reset(new IOMessage(data_.get(), bytes_, *iosock_, *peer_));
 
         // Perform any necessary operations prior to processing an incoming
-        // packet (e.g., checking for queued configuration messages).
+        // query (e.g., checking for queued configuration messages).
         //
-        // (XXX: it may be a performance issue to have this called for
-        // every single incoming packet; we may wish to throttle it somehow
-        // in the future.)
+        // (XXX: it may be a performance issue to check in for every single
+        // incoming query; we may wish to throttle this in the future.)
         if (checkin_callback_ != NULL) {
             (*checkin_callback_)(*io_message_);
         }
 
-        // Stop here if we don't have a DNS callback function
+        // If we don't have a DNS Lookup provider, there's no point in
+        // continuing; we exit the coroutine permanently.
         if (lookup_callback_ == NULL) {
             CORO_YIELD return;
         }
 
         // Instantiate objects that will be needed by the
-        // asynchronous send call.
+        // asynchronous DNS lookup and/or by the send call.
         respbuf_.reset(new OutputBuffer(0));
         message_.reset(new Message(Message::PARSE));
 
-        CORO_YIELD io_.post(LookupHandler<UDPServer>(*this));
+        // Schedule a DNS lookup, and yield.  When the lookup is
+        // finished, the coroutine will resume immediately after
+        // this point.
+        CORO_YIELD io_.post(AsyncLookup<UDPServer>(*this));
 
+        // The 'done_' flag indicates whether we have an answer
+        // to send back.  If not, exit the coroutine permanently.
         if (!done_) {
             CORO_YIELD return;
         }
 
+        // Call the DNS answer provider to render the answer into
+        // wire format
         (*answer_callback_)(*io_message_, message_, respbuf_);
+
+        // Begin an asynchronous send, and then yield.  When the
+        // send completes, we will resume immediately after this point
+        // (though we have nothing further to do, so the coroutine
+        // will simply exit at that time).
         CORO_YIELD socket_->async_send_to(buffer(respbuf_->getData(),
                                                  respbuf_->getLength()),
                                      *sender_, *this);
     }
 }
 
+/// Call the DNS lookup provider.  (Expected to be called by the
+/// AsyncLookup<UDPServer> handler.)
 void
-UDPServer::doLookup() {
+UDPServer::asyncLookup() {
     (*lookup_callback_)(*io_message_, message_, respbuf_, this);
 }
 
+/// Post this coroutine on the ASIO service queue so that it will
+/// resume processing where it left off.  The 'done' parameter indicates
+/// whether there is an answer to return to the client.
 void
 UDPServer::resume(const bool done) {
     done_ = done;
     io_.post(*this);
 }
 
+/// The following functions implement the \c UDPQuery class.
+///
+/// The constructor
 UDPQuery::UDPQuery(io_service& io_service, const IOMessage& io_message,
                    const Question& q, const ip::address& addr,
-                   OutputBufferPtr buffer, IOServer* server) :
-    question_(q), msgbuf_(512), buffer_(buffer), server_(server->clone())
+                   OutputBufferPtr buffer, DNSServer* server) :
+    question_(q), buffer_(buffer), server_(server->clone())
 {
     udp proto = addr.is_v4() ? udp::v4() : udp::v6();
     socket_.reset(new udp::socket(io_service, proto));
+    msgbuf_.reset(new OutputBuffer(512));
     remote_ = udp::endpoint(addr, 53);
 }
 
+/// The function operator is implemented with the "stackless coroutine"
+/// pattern; see internal/coroutine.h for details.
 void
 UDPQuery::operator()(error_code ec, size_t length) {
     if (ec) {
         return;
-    }
+    } 
 
     CORO_REENTER (this) {
+        /// Generate the upstream query and render it to wire format
+        /// This is done in a different scope to allow inline variable
+        /// declarations.
         {
             Message msg(Message::RENDER);
+            
+            // XXX: replace with boost::random or some other suitable PRNG
             msg.setQid(0);
             msg.setOpcode(Opcode::QUERY());
             msg.setRcode(Rcode::NOERROR());
             msg.setHeaderFlag(MessageFlag::RD());
             msg.addQuestion(question_);
-            MessageRenderer renderer(msgbuf_);
+            MessageRenderer renderer(*msgbuf_);
             msg.toWire(renderer);
         }
 
-        CORO_YIELD socket_->async_send_to(buffer(msgbuf_.getData(),
-                                                 msgbuf_.getLength()),
+        // Begin an asynchronous send, and then yield.  When the
+        // send completes, we will resume immediately after this point.
+        CORO_YIELD socket_->async_send_to(buffer(msgbuf_->getData(),
+                                                 msgbuf_->getLength()),
                                            remote_, *this);
 
+        /// Allocate space for the response.  (XXX: This should be
+        /// optimized by maintaining a free list of pre-allocated blocks)
         data_.reset(new char[MAX_LENGTH]);
+
+        /// Begin an asynchronous receive, and yield.  When the send
+        /// completes, we will resume immediately after this point.
         CORO_YIELD socket_->async_receive_from(buffer(data_.get(), MAX_LENGTH),
                                                remote_, *this);
 
+        /// Copy the answer into the response buffer.  (XXX: If the
+        /// OutputBuffer object were made to meet the requirements of
+        /// a MutableBufferSequence, then it could be written to directly
+        /// by async_recieve_from() and this additional copy step would
+        /// be unnecessary.)
         buffer_->writeData(data_.get(), length);
+
+        /// Signal the DNSServer object to resume processing.
         server_->resume(true);
     }
-
 }
 
 }