Merge recursor config into trac327

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

doc/Doxyfile

@@ -568,7 +568,7 @@ WARN_LOGFILE           =
 # directories like "/usr/src/myproject". Separate the files or directories
 # with spaces.
 
-INPUT                  = ../src/lib/cc ../src/lib/config ../src/lib/dns ../src/lib/exceptions ../src/lib/datasrc ../src/bin/auth ../src/lib/bench
+INPUT                  = ../src/lib/cc ../src/lib/config ../src/lib/dns ../src/lib/exceptions ../src/lib/datasrc ../src/bin/auth ../src/lib/bench ../src/lib/asiolink/
 
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is

src/bin/bind10/bind10.py.in

@@ -363,13 +363,14 @@ class BoB:
             dns_prog = 'b10-recurse'
         else:
             dns_prog = 'b10-auth'
-        dnsargs = [dns_prog, '-p', str(self.dns_port)]
-        if self.forward:
-            dnsargs += ['-f', str(self.forward)]
-        if self.address:
-            dnsargs += ['-a', str(self.address)]
-        if self.nocache:
-            dnsargs += ['-n']
+        dnsargs = [dns_prog]
+        if not self.recursive:
+            # The recursive uses configuration manager for these
+            dnsargs += ['-p', str(self.dns_port)]
+            if self.address:
+                dnsargs += ['-a', str(self.address)]
+            if self.nocache:
+                dnsargs += ['-n']
         if self.uid:
             dnsargs += ['-u', str(self.uid)]
         if self.verbose:

src/bin/recurse/main.cc

@@ -61,7 +61,6 @@ static bool verbose_mode = false;
 
 // Default port current 5300 for testing purposes
 static const string PROGRAM = "Recurse";
-static const char* DNSPORT = "5300";
 
 IOService io_service;
 static Recursor *recursor;
@@ -82,21 +81,13 @@ my_command_handler(const string& command, ConstElementPtr args) {
     } else if (command == "shutdown") {
         io_service.stop();
     }
-    
+
     return (answer);
 }
 
 void
 usage() {
-    cerr << "Usage:  b10-recurse -f nameserver [-a address] [-p port] [-u user]"
-                     "[-4|-6] [-v]" << endl;
-    cerr << "\t-f: specify the nameserver to which queries should be forwarded"
-         << endl;
-    cerr << "\t-a: specify the address to listen on (default: all)" << endl;
-    cerr << "\t-p: specify the port to listen on (default: " << DNSPORT << ")"
-         << endl;
-    cerr << "\t-4: listen on all IPv4 addresses (incompatible with -a)" << endl;
-    cerr << "\t-6: listen on all IPv6 addresses (incompatible with -a)" << endl;
+    cerr << "Usage:  b10-recurse [-u user] [-v]" << endl;
     cerr << "\t-u: change process UID to the specified user" << endl;
     cerr << "\t-v: verbose output" << endl;
     exit(1);
@@ -106,34 +97,10 @@ usage() {
 int
 main(int argc, char* argv[]) {
     int ch;
-    const char* port = DNSPORT;
-    const char* address = NULL;
-    const char* forward = NULL;
     const char* uid = NULL;
-    bool use_ipv4 = true, use_ipv6 = true;
 
-    while ((ch = getopt(argc, argv, "46a:f:p:u:v")) != -1) {
+    while ((ch = getopt(argc, argv, "u:v")) != -1) {
         switch (ch) {
-        case '4':
-            // Note that -4 means "ipv4 only", we need to set "use_ipv6" here,
-            // not "use_ipv4".  We could use something like "ipv4_only", but
-            // we found the negatively named variable could confuse the code
-            // logic.
-            use_ipv6 = false;
-            break;
-        case '6':
-            // The same note as -4 applies.
-            use_ipv4 = false;
-            break;
-        case 'a':
-            address = optarg;
-            break;
-        case 'f':
-            forward = optarg;
-            break;
-        case 'p':
-            port = optarg;
-            break;
         case 'u':
             uid = optarg;
             break;
@@ -150,23 +117,6 @@ main(int argc, char* argv[]) {
         usage();
     }
 
-    if (!use_ipv4 && !use_ipv6) {
-        cerr << "[b10-auth] Error: Cannot specify both -4 and -6 "
-             << "at the same time" << endl;
-        usage();
-    }
-
-    if ((!use_ipv4 || !use_ipv6) && address != NULL) {
-        cerr << "[b10-auth] Error: Cannot specify -4 or -6 "
-             << "at the same time as -a" << endl;
-        usage();
-    }
-
-    if (forward == NULL) {
-        cerr << "[b10-recurse] No forward name server specified" << endl;
-        usage();
-    }
-
     int ret = 0;
 
     // XXX: we should eventually pass io_service here.
@@ -181,7 +131,7 @@ main(int argc, char* argv[]) {
             specfile = string(RECURSE_SPECFILE_LOCATION);
         }
 
-        recursor = new Recursor(*forward);
+        recursor = new Recursor();
         recursor->setVerbose(verbose_mode);
         cout << "[b10-recurse] Server created." << endl;
 
@@ -189,22 +139,9 @@ main(int argc, char* argv[]) {
         DNSLookup* lookup = recursor->getDNSLookupProvider();
         DNSAnswer* answer = recursor->getDNSAnswerProvider();
 
-        DNSService* dns_service;
-
-        if (address != NULL) {
-            // XXX: we can only specify at most one explicit address.
-            // This also means the server cannot run in the dual address
-            // family mode if explicit addresses need to be specified.
-            // We don't bother to fix this problem, however.  The -a option
-            // is a short term workaround until we support dynamic listening
-            // port allocation.
-            dns_service = new DNSService(io_service, *port, *address,
-                                         checkin, lookup, answer);
-        } else {
-            dns_service = new DNSService(io_service, *port, use_ipv4, use_ipv6,
-                                         checkin, lookup, answer);
-        }
-        recursor->setDNSService(*dns_service);
+        DNSService dns_service(io_service, checkin, lookup, answer);
+
+        recursor->setDNSService(dns_service);
         cout << "[b10-recurse] IOService created." << endl;
 
         cc_session = new Session(io_service.get_io_service());
@@ -215,12 +152,13 @@ main(int argc, char* argv[]) {
                                              my_command_handler);
         cout << "[b10-recurse] Configuration channel established." << endl;
 
+        // FIXME: This does not belong here, but inside Boss
         if (uid != NULL) {
             changeUser(uid);
         }
 
         recursor->setConfigSession(config_session);
-        recursor->updateConfig(ElementPtr());
+        recursor->updateConfig(config_session->getFullConfig());
 
         cout << "[b10-recurse] Server started." << endl;
         io_service.run();

src/bin/recurse/recurse.spec.pre.in

@@ -1,18 +1,74 @@
 {
   "module_spec": {
-    "module_name": "Auth",
-    "module_description": "Authoritative service",
+    "module_name": "Recurse",
+    "module_description": "Recursive service",
     "config_data": [
-      { "item_name": "database_file",
-        "item_type": "string",
-        "item_optional": true,
-        "item_default": "@@LOCALSTATEDIR@@/@PACKAGE@/zone.sqlite3"
+      {
+        "item_name": "forward_addresses",
+        "item_type": "list",
+        "item_optional": True,
+        "item_default": [],
+        "list_item_spec" : {
+          "item_name": "address",
+          "item_type": "map",
+          "item_optional": False,
+          "item_default": {},
+          "map_item_spec": [
+            {
+              "item_name": "address",
+              "item_type": "string",
+              "item_optional": False,
+              "item_default": "::1"
+            },
+            {
+              "item_name": "port",
+              "item_type": "integer",
+              "item_optional": False,
+              "item_default": 53
+            }
+          ]
+        }
+      },
+      {
+        "item_name": "listen_on",
+        "item_type": "list",
+        "item_optional": False,
+        "item_default": [
+          {
+            "address": "::1",
+            "port": 5300
+          },
+          {
+            "address": "127.0.0.1",
+            "port": 5300
+          },
+        ],
+        "list_item_spec": {
+          "item_name": "address",
+          "item_type": "map",
+          "item_optional": False,
+          "item_default": {},
+          "map_item_spec": [
+            {
+              "item_name": "address",
+              "item_type": "string",
+              "item_optional": False,
+              "item_default": "::1"
+            },
+            {
+              "item_name": "port",
+              "item_type": "integer",
+              "item_optional": False,
+              "item_default": 5300
+            }
+          ]
+        }
       }
     ],
     "commands": [
       {
         "command_name": "shutdown",
-        "command_description": "Shut down authoritative DNS server",
+        "command_description": "Shut down recursive DNS server",
         "command_args": []
       }
     ]

src/bin/recurse/recursor.cc

@@ -24,6 +24,7 @@
 #include <vector>
 
 #include <asiolink/asiolink.h>
+#include <asiolink/ioaddress.h>
 
 #include <boost/foreach.hpp>
 
@@ -57,15 +58,18 @@ using namespace isc::config;
 using namespace isc::xfr;
 using namespace asiolink;
 
+typedef pair<string, uint16_t> addr_t;
+
 class RecursorImpl {
 private:
     // prohibit copy
     RecursorImpl(const RecursorImpl& source);
     RecursorImpl& operator=(const RecursorImpl& source);
 public:
-    RecursorImpl(const char& forward) :
-        config_session_(NULL), verbose_mode_(false),
-        forward_(forward), rec_query_()
+    RecursorImpl() :
+        config_session_(NULL),
+        verbose_mode_(false),
+        rec_query_()
     {}
 
     ~RecursorImpl() {
@@ -73,12 +77,26 @@ public:
     }
 
     void querySetup(DNSService& dnss) {
-        rec_query_ = new RecursiveQuery(dnss, forward_);
+        rec_query_ = new RecursiveQuery(dnss, upstream_);
     }
 
     void queryShutdown() {
-        if (rec_query_) {
-            delete rec_query_;
+        delete rec_query_;
+        rec_query_ = NULL;
+    }
+
+    void setForwardAddresses(const vector<addr_t>& upstream,
+        DNSService *dnss)
+    {
+        queryShutdown();
+        upstream_ = upstream;
+        if (dnss) {
+            if (upstream_.empty()) {
+                cerr << "[b10-recurse] Asked to do full recursive," << endl <<
+                    "but not implemented yet. I'll do nothing." << endl;
+            } else {
+                querySetup(*dnss);
+            }
         }
     }
 
@@ -92,10 +110,12 @@ public:
     /// These members are public because Recursor accesses them directly.
     ModuleCCSession* config_session_;
     bool verbose_mode_;
+    /// Addresses of the forward nameserver
+    vector<addr_t> upstream_;
+    /// Addresses we listen on
+    vector<addr_t> listen_;
 
 private:
-    /// Address of the forward nameserver
-    const char& forward_;
 
     /// Object to handle upstream queries
     RecursiveQuery* rec_query_;
@@ -281,8 +301,8 @@ private:
     Recursor* server_;
 };
 
-Recursor::Recursor(const char& forward) :
-    impl_(new RecursorImpl(forward)),
+Recursor::Recursor() :
+    impl_(new RecursorImpl()),
     checkin_(new ConfigCheck(this)),
     dns_lookup_(new MessageLookup(this)),
     dns_answer_(new MessageAnswer(this))
@@ -431,11 +451,68 @@ RecursorImpl::processNormalQuery(const Question& question, MessagePtr message,
     rec_query_->sendQuery(question, buffer, server);
 }
 
+namespace {
+
+vector<addr_t>
+parseAddresses(ConstElementPtr addresses) {
+    vector<addr_t> result;
+    if (addresses) {
+        if (addresses->getType() == Element::list) {
+            for (size_t i(0); i < addresses->size(); ++ i) {
+                ConstElementPtr addrPair(addresses->get(i));
+                ConstElementPtr addr(addrPair->get("address"));
+                ConstElementPtr port(addrPair->get("port"));
+                if (!addr || ! port) {
+                    isc_throw(BadValue, "Address must contain both the IP"
+                        "address and port");
+                }
+                try {
+                    IOAddress(addr->stringValue());
+                    if (port->intValue() < 0 ||
+                        port->intValue() > 0xffff) {
+                        isc_throw(BadValue, "Bad port value (" <<
+                            port->intValue() << ")");
+                    }
+                    result.push_back(addr_t(addr->stringValue(),
+                        port->intValue()));
+                }
+                catch (const TypeError &e) { // Better error message
+                    isc_throw(TypeError,
+                        "Address must be a string and port an integer");
+                }
+            }
+        } else if (addresses->getType() != Element::null) {
+            isc_throw(TypeError,
+                "forward_addresses config element must be a list");
+        }
+    }
+    return (result);
+}
+
+}
+
 ConstElementPtr
-Recursor::updateConfig(ConstElementPtr new_config UNUSED_PARAM) {
+Recursor::updateConfig(ConstElementPtr config) {
+    if (impl_->verbose_mode_) {
+        cout << "[b10-recurse] Update with config: " << config->str() << endl;
+    }
     try {
-        // We will do configuration updates here.  None are presently
-        // defined, so we just return an empty answer.
+        // Parse forward_addresses
+        // FIXME Once the config parser is fixed, remove the slashes. They
+        // appear only on the default/startup value and shouldn't be here.
+        // See ticket #384
+        ConstElementPtr forwardAddressesE(config->get("forward_addresses/"));
+        vector<addr_t> forwardAddresses(parseAddresses(forwardAddressesE));
+        ConstElementPtr listenAddressesE(config->get("listen_on/"));
+        vector<addr_t> listenAddresses(parseAddresses(listenAddressesE));
+        // Everything OK, so commit the changes
+        // listenAddresses can fail to bind, so try them first
+        if (listenAddressesE) {
+            setListenAddresses(listenAddresses);
+        }
+        if (forwardAddressesE) {
+            setForwardAddresses(forwardAddresses);
+        }
         return (isc::config::createAnswer());
     } catch (const isc::Exception& error) {
         if (impl_->verbose_mode_) {
@@ -444,3 +521,62 @@ Recursor::updateConfig(ConstElementPtr new_config UNUSED_PARAM) {
         return (isc::config::createAnswer(1, error.what()));
     }
 }
+
+void
+Recursor::setForwardAddresses(const vector<addr_t>& addresses)
+{
+    impl_->setForwardAddresses(addresses, dnss_);
+}
+
+bool
+Recursor::isForwarding() const {
+    return (!impl_->upstream_.empty());
+}
+
+vector<addr_t>
+Recursor::getForwardAddresses() const {
+    return (impl_->upstream_);
+}
+
+namespace {
+
+void
+setAddresses(DNSService *service, const vector<addr_t>& addresses) {
+    service->clearServers();
+    BOOST_FOREACH(const addr_t &address, addresses) {
+        service->addServer(address.second, address.first);
+    }
+}
+
+}
+
+void
+Recursor::setListenAddresses(const vector<addr_t>& addresses) {
+    try {
+        setAddresses(dnss_, addresses);
+        impl_->listen_ = addresses;
+    }
+    catch (const exception& e) {
+        /*
+         * We couldn't set it. So return it back. If that fails as well,
+         * we have a problem.
+         *
+         * If that fails, bad luck, but we are useless anyway, so just die
+         * and let boss start us again.
+         */
+        try {
+            setAddresses(dnss_, impl_->listen_);
+        }
+        catch (const exception& e2) {
+            cerr << "[b10-recurse] Unable to recover from error: " << e.what()
+                << endl << "Rollback failed with: " << e2.what() << endl;
+            abort();
+        }
+        throw e; // Let it fly a little bit further
+    }
+}
+
+vector<addr_t>
+Recursor::getListenAddresses() const {
+    return (impl_->listen_);
+}

src/bin/recurse/recursor.h

@@ -18,6 +18,8 @@
 #define __RECURSOR_H 1
 
 #include <string>
+#include <vector>
+#include <utility>
 
 #include <cc/data.h>
 #include <config/ccsession.h>
@@ -38,12 +40,7 @@ private:
     Recursor& operator=(const Recursor& source);
 public:
     /// The constructor.
-    ///
-    /// \param forward The address of the name server to which requests
-    /// should be forwarded.  (In the future, when the server is running
-    /// in forwarding mode, the forward nameserver addresses will be set
-    /// via the config channel instaed.)
-    Recursor(const char& forward);
+    Recursor();
     ~Recursor();
     //@}
 
@@ -93,6 +90,34 @@ public:
     /// \brief Return pointer to the Checkin callback function
     asiolink::SimpleCallback* getCheckinProvider() { return (checkin_); }
 
+    /**
+     * \brief Specify the list of upstream servers.
+     *
+     * Specify the list off addresses of upstream servers to forward queries
+     * to. If the list is empty, this server is set to full recursive mode.
+     * If it is non-empty, it switches to forwarder.
+     *
+     * @param addresses The list of addresses to use (each one is the address
+     * and port pair).
+     */
+    void setForwardAddresses(const std::vector<std::pair<std::string,
+        uint16_t> >& addresses);
+    /**
+     * \short Get list of upstream addresses.
+     *
+     * \see setForwardAddresses.
+     */
+    std::vector<std::pair<std::string, uint16_t> > getForwardAddresses() const;
+    /// Return if we are in forwarding mode (if not, we are in fully recursive)
+    bool isForwarding() const;
+
+    /**
+     * Set and get the addresses we listen on.
+     */
+    void setListenAddresses(const std::vector<std::pair<std::string,
+        uint16_t> >& addresses);
+    std::vector<std::pair<std::string, uint16_t> > getListenAddresses() const;
+
 private:
     RecursorImpl* impl_;
     asiolink::DNSService* dnss_;

src/bin/recurse/tests/recursor_unittest.cc

@@ -63,7 +63,7 @@ private:
 
 class RecursorTest : public ::testing::Test {
 protected:
-    RecursorTest() : server(*DEFAULT_REMOTE_ADDRESS),
+    RecursorTest() : server(),
                      request_message(Message::RENDER),
                      parse_message(new Message(Message::PARSE)),
                      default_qid(0x1035), opcode(Opcode(Opcode::QUERY())),
@@ -72,7 +72,11 @@ protected:
                      io_message(NULL), endpoint(NULL), request_obuffer(0),
                      request_renderer(request_obuffer),
                      response_obuffer(new OutputBuffer(0))
-    {}
+    {
+        vector<pair<string, uint16_t> > upstream;
+        upstream.push_back(pair<string, uint16_t>(DEFAULT_REMOTE_ADDRESS, 53));
+        server.setForwardAddresses(upstream);
+    }
     ~RecursorTest() {
         delete io_message;
         delete endpoint;
@@ -325,4 +329,181 @@ TEST_F(RecursorTest, notifyFail) {
                 Opcode::NOTIFY().getCode(), QR_FLAG, 0, 0, 0, 0);
 }
 
+class RecursorConfig : public ::testing::Test {
+    public:
+        IOService ios;
+        DNSService dnss;
+        Recursor server;
+        RecursorConfig() :
+            dnss(ios, NULL, NULL, NULL)
+        {
+            server.setDNSService(dnss);
+        }
+        void invalidTest(const string &JOSN);
+};
+
+TEST_F(RecursorConfig, forwardAddresses) {
+    // Default value should be fully recursive
+    EXPECT_TRUE(server.getForwardAddresses().empty());
+    EXPECT_FALSE(server.isForwarding());
+
+    // Try putting there some addresses
+    vector<pair<string, uint16_t> > addresses;
+    addresses.push_back(pair<string, uint16_t>(DEFAULT_REMOTE_ADDRESS, 53));
+    addresses.push_back(pair<string, uint16_t>("::1", 53));
+    server.setForwardAddresses(addresses);
+    EXPECT_EQ(2, server.getForwardAddresses().size());
+    EXPECT_EQ("::1", server.getForwardAddresses()[1].first);
+    EXPECT_TRUE(server.isForwarding());
+
+    // Is it independent from what we do with the vector later?
+    addresses.clear();
+    EXPECT_EQ(2, server.getForwardAddresses().size());
+
+    // Did it return to fully recursive?
+    server.setForwardAddresses(addresses);
+    EXPECT_TRUE(server.getForwardAddresses().empty());
+    EXPECT_FALSE(server.isForwarding());
+}
+
+TEST_F(RecursorConfig, forwardAddressConfig) {
+    // Try putting there some address
+    ElementPtr config(Element::fromJSON("{"
+        "\"forward_addresses/\": ["
+        "   {"
+        "       \"address\": \"192.0.2.1\","
+        "       \"port\": 53"
+        "   }"
+        "]"
+        "}"));
+    ConstElementPtr result(server.updateConfig(config));
+    EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
+    EXPECT_TRUE(server.isForwarding());
+    ASSERT_EQ(1, server.getForwardAddresses().size());
+    EXPECT_EQ("192.0.2.1", server.getForwardAddresses()[0].first);
+    EXPECT_EQ(53, server.getForwardAddresses()[0].second);
+
+    // And then remove all addresses
+    config = Element::fromJSON("{"
+        "\"forward_addresses/\": null"
+        "}");
+    result = server.updateConfig(config);
+    EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
+    EXPECT_FALSE(server.isForwarding());
+    EXPECT_EQ(0, server.getForwardAddresses().size());
+}
+
+void RecursorConfig::invalidTest(const string &JOSN) {
+    ElementPtr config(Element::fromJSON(JOSN));
+    EXPECT_FALSE(server.updateConfig(config)->equals(
+        *isc::config::createAnswer())) << "Accepted config " << JOSN << endl;
+}
+
+TEST_F(RecursorConfig, invalidForwardAddresses) {
+    // Try torturing it with some invalid inputs
+    invalidTest("{"
+        "\"forward_addresses/\": \"error\""
+        "}");
+    invalidTest("{"
+        "\"forward_addresses/\": [{}]"
+        "}");
+    invalidTest("{"
+        "\"forward_addresses/\": [{"
+        "   \"port\": 1.5,"
+        "   \"address\": \"192.0.2.1\""
+        "}]}");
+    invalidTest("{"
+        "\"forward_addresses/\": [{"
+        "   \"port\": -5,"
+        "   \"address\": \"192.0.2.1\""
+        "}]}");
+    invalidTest("{"
+        "\"forward_addresses/\": [{"
+        "   \"port\": 53,"
+        "   \"address\": \"bad_address\""
+        "}]}");
+}
+
+TEST_F(RecursorConfig, listenAddresses) {
+    // Default value should be fully recursive
+    EXPECT_TRUE(server.getListenAddresses().empty());
+
+    // Try putting there some addresses
+    vector<pair<string, uint16_t> > addresses;
+    addresses.push_back(pair<string, uint16_t>("127.0.0.1", 5300));
+    addresses.push_back(pair<string, uint16_t>("::1", 5300));
+    server.setListenAddresses(addresses);
+    EXPECT_EQ(2, server.getListenAddresses().size());
+    EXPECT_EQ("::1", server.getListenAddresses()[1].first);
+
+    // Is it independent from what we do with the vector later?
+    addresses.clear();
+    EXPECT_EQ(2, server.getListenAddresses().size());
+
+    // Did it return to fully recursive?
+    server.setListenAddresses(addresses);
+    EXPECT_TRUE(server.getListenAddresses().empty());
+}
+
+TEST_F(RecursorConfig, DISABLED_listenAddressConfig) {
+    // Try putting there some address
+    ElementPtr config(Element::fromJSON("{"
+        "\"listen_on/\": ["
+        "   {"
+        "       \"address\": \"127.0.0.1\","
+        "       \"port\": 5300"
+        "   }"
+        "]"
+        "}"));
+    ConstElementPtr result(server.updateConfig(config));
+    EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
+    ASSERT_EQ(1, server.getListenAddresses().size());
+    EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
+    EXPECT_EQ(5300, server.getListenAddresses()[0].second);
+
+    // As this is example address, the machine should not have it on
+    // any interface
+    // FIXME: This test aborts, because it tries to rollback and
+    //     it is impossible, since the sockets are not closed.
+    //     Once #388 is solved, enable this test.
+    config = Element::fromJSON("{"
+        "\"listen_on/\": ["
+        "   {"
+        "       \"address\": \"192.0.2.0\","
+        "       \"port\": 5300"
+        "   }"
+        "]"
+        "}");
+    result = server.updateConfig(config);
+    EXPECT_FALSE(result->equals(*isc::config::createAnswer()));
+    ASSERT_EQ(1, server.getListenAddresses().size());
+    EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
+    EXPECT_EQ(5300, server.getListenAddresses()[0].second);
+}
+
+TEST_F(RecursorConfig, invalidListenAddresses) {
+    // Try torturing it with some invalid inputs
+    invalidTest("{"
+        "\"listen_on/\": \"error\""
+        "}");
+    invalidTest("{"
+        "\"listen_on/\": [{}]"
+        "}");
+    invalidTest("{"
+        "\"listen_on/\": [{"
+        "   \"port\": 1.5,"
+        "   \"address\": \"192.0.2.1\""
+        "}]}");
+    invalidTest("{"
+        "\"listen_on/\": [{"
+        "   \"port\": -5,"
+        "   \"address\": \"192.0.2.1\""
+        "}]}");
+    invalidTest("{"
+        "\"listen_on/\": [{"
+        "   \"port\": 53,"
+        "   \"address\": \"bad_address\""
+        "}]}");
+}
+
 }

src/lib/asiolink/asiolink.cc

@@ -16,10 +16,13 @@
 
 #include <config.h>
 
+#include <cstdlib> // For random(), temporary until better forwarding is done
+
 #include <unistd.h>             // for some IPC/network system calls
 #include <sys/socket.h>
 #include <netinet/in.h>
 
+#include <vector>
 #include <asio.hpp>
 #include <boost/lexical_cast.hpp>
 #include <boost/bind.hpp>
@@ -48,7 +51,10 @@ private:
     IOServiceImpl& operator=(const IOService& source);
 public:
     /// \brief The constructor
-    IOServiceImpl() : io_service_() {};
+    IOServiceImpl() :
+        io_service_(),
+        work_(io_service_)
+    {};
     /// \brief The destructor.
     ~IOServiceImpl() {};
     //@}
@@ -80,6 +86,7 @@ public:
     asio::io_service& get_io_service() { return io_service_; };
 private:
     asio::io_service io_service_;
+    asio::io_service::work work_;
 };
 
 IOService::IOService() {
@@ -116,70 +123,75 @@ public:
                   const ip::address* v4addr, const ip::address* v6addr,
                   SimpleCallback* checkin, DNSLookup* lookup,
                   DNSAnswer* answer);
-    //asio::io_service io_service_;
 
-    void stop();
+    IOService& io_service_;
+
     typedef boost::shared_ptr<UDPServer> UDPServerPtr;
     typedef boost::shared_ptr<TCPServer> TCPServerPtr;
-    UDPServerPtr udp4_server_;
-    UDPServerPtr udp6_server_;
-    TCPServerPtr tcp4_server_;
-    TCPServerPtr tcp6_server_;
+    typedef boost::shared_ptr<DNSServer> DNSServerPtr;
+    vector<DNSServerPtr> servers_;
+    SimpleCallback *checkin_;
+    DNSLookup *lookup_;
+    DNSAnswer *answer_;
+
+    void addServer(uint16_t port, const ip::address& address) {
+        try {
+            TCPServerPtr tcpServer(new TCPServer(io_service_.get_io_service(),
+                address, port, checkin_, lookup_, answer_));
+            (*tcpServer)();
+            servers_.push_back(tcpServer);
+
+            UDPServerPtr udpServer(new UDPServer(io_service_.get_io_service(),
+                address, port, checkin_, lookup_, answer_));
+            (*udpServer)();
+            servers_.push_back(udpServer);
+        }
+        catch (const asio::system_error& err) {
+            // We need to catch and convert any ASIO level exceptions.
+            // This can happen for unavailable address, binding a privilege port
+            // without the privilege, etc.
+            isc_throw(IOError, "Failed to initialize network servers: " <<
+                      err.what());
+        }
+    }
+    void addServer(const char& port, const ip::address& address) {
+        uint16_t portnum;
+        try {
+            // XXX: SunStudio with stlport4 doesn't reject some invalid
+            // representation such as "-1" by lexical_cast<uint16_t>, so
+            // we convert it into a signed integer of a larger size and perform
+            // range check ourselves.
+            const int32_t portnum32 = boost::lexical_cast<int32_t>(&port);
+            if (portnum32 < 0 || portnum32 > 65535) {
+                isc_throw(IOError, "Invalid port number '" << &port);
+            }
+            portnum = portnum32;
+        } catch (const boost::bad_lexical_cast& ex) {
+            isc_throw(IOError, "Invalid port number '" << &port << "': " <<
+                      ex.what());
+        }
+        addServer(portnum, address);
+    }
 };
 
-DNSServiceImpl::DNSServiceImpl(IOService& io_service_,
+DNSServiceImpl::DNSServiceImpl(IOService& io_service,
                                const char& port,
                                const ip::address* const v4addr,
                                const ip::address* const v6addr,
                                SimpleCallback* checkin,
                                DNSLookup* lookup,
                                DNSAnswer* answer) :
-    udp4_server_(UDPServerPtr()), udp6_server_(UDPServerPtr()),
-    tcp4_server_(TCPServerPtr()), tcp6_server_(TCPServerPtr())
+    io_service_(io_service),
+    checkin_(checkin),
+    lookup_(lookup),
+    answer_(answer)
 {
-    uint16_t portnum;
-    try {
-        // XXX: SunStudio with stlport4 doesn't reject some invalid
-        // representation such as "-1" by lexical_cast<uint16_t>, so
-        // we convert it into a signed integer of a larger size and perform
-        // range check ourselves.
-        const int32_t portnum32 = boost::lexical_cast<int32_t>(&port);
-        if (portnum32 < 0 || portnum32 > 65535) {
-            isc_throw(IOError, "Invalid port number '" << &port);
-        }
-        portnum = portnum32;
-    } catch (const boost::bad_lexical_cast& ex) {
-        isc_throw(IOError, "Invalid port number '" << &port << "': " <<
-                  ex.what());
-    }
 
-    try {
-        if (v4addr != NULL) {
-            udp4_server_ = UDPServerPtr(new UDPServer(io_service_.get_io_service(),
-                                                      *v4addr, portnum,
-                                                      checkin, lookup, answer));
-            (*udp4_server_)();
-            tcp4_server_ = TCPServerPtr(new TCPServer(io_service_.get_io_service(),
-                                                      *v4addr, portnum,
-                                                      checkin, lookup, answer));
-            (*tcp4_server_)();
-        }
-        if (v6addr != NULL) {
-            udp6_server_ = UDPServerPtr(new UDPServer(io_service_.get_io_service(),
-                                                      *v6addr, portnum,
-                                                      checkin, lookup, answer));
-            (*udp6_server_)();
-            tcp6_server_ = TCPServerPtr(new TCPServer(io_service_.get_io_service(),
-                                                      *v6addr, portnum,
-                                                      checkin, lookup, answer));
-            (*tcp6_server_)();
-        }
-    } catch (const asio::system_error& err) {
-        // We need to catch and convert any ASIO level exceptions.
-        // This can happen for unavailable address, binding a privilege port
-        // without the privilege, etc.
-        isc_throw(IOError, "Failed to initialize network servers: " <<
-                  err.what());
+    if (v4addr) {
+        addServer(port, *v4addr);
+    }
+    if (v6addr) {
+        addServer(port, *v6addr);
     }
 }
 
@@ -188,19 +200,10 @@ DNSService::DNSService(IOService& io_service,
                        SimpleCallback* checkin,
                        DNSLookup* lookup,
                        DNSAnswer* answer) :
-    impl_(NULL), io_service_(io_service)
+    impl_(new DNSServiceImpl(io_service, port, NULL, NULL, checkin, lookup,
+        answer)), io_service_(io_service)
 {
-    error_code err;
-    const ip::address addr = ip::address::from_string(&address, err);
-    if (err) {
-        isc_throw(IOError, "Invalid IP address '" << &address << "': "
-                  << err.message());
-    }
-
-    impl_ = new DNSServiceImpl(io_service, port,
-                              addr.is_v4() ? &addr : NULL,
-                              addr.is_v6() ? &addr : NULL,
-                              checkin, lookup, answer);
+    addServer(port, &address);
 }
 
 DNSService::DNSService(IOService& io_service,
@@ -218,13 +221,52 @@ DNSService::DNSService(IOService& io_service,
     impl_ = new DNSServiceImpl(io_service, port, v4addrp, v6addrp, checkin, lookup, answer);
 }
 
+DNSService::DNSService(IOService& io_service, SimpleCallback* checkin,
+    DNSLookup* lookup, DNSAnswer *answer) :
+    impl_(new DNSServiceImpl(io_service, *"0", NULL, NULL, checkin, lookup,
+        answer)), io_service_(io_service)
+{
+}
+
 DNSService::~DNSService() {
     delete impl_;
 }
 
-RecursiveQuery::RecursiveQuery(DNSService& dns_service, const char& forward,
-                               uint16_t port) :
-    dns_service_(dns_service), ns_addr_(&forward), port_(port) 
+namespace {
+
+ip::address
+convertAddr(const string& address) {
+    error_code err;
+    ip::address addr = ip::address::from_string(address, err);
+    if (err) {
+        isc_throw(IOError, "Invalid IP address '" << &address << "': "
+            << err.message());
+    }
+    return addr;
+}
+
+}
+
+void
+DNSService::addServer(const char& port, const string& address) {
+    impl_->addServer(port, convertAddr(address));
+}
+
+void
+DNSService::addServer(uint16_t port, const string &address) {
+    impl_->addServer(port, convertAddr(address));
+}
+
+void
+DNSService::clearServers() {
+    // FIXME: This does not work, it does not close the socket.
+    // How is it done?
+    impl_->servers_.clear();
+}
+
+RecursiveQuery::RecursiveQuery(DNSService& dns_service,
+        const std::vector<std::pair<std::string, uint16_t> >& upstream) :
+    dns_service_(dns_service), upstream_(upstream)
 {}
 
 void
@@ -237,7 +279,10 @@ RecursiveQuery::sendQuery(const Question& question, OutputBufferPtr buffer,
     // UDP and then fall back to TCP on failure, but for the moment
     // we're only going to handle UDP.
     asio::io_service& io = dns_service_.get_io_service();
-    UDPQuery q(io, question, ns_addr_, port_, buffer, server);
+    // TODO: Better way to choose the server
+    int serverIndex(random() % upstream_.size());
+    UDPQuery q(io, question, upstream_[serverIndex].first,
+        upstream_[serverIndex].second, buffer, server);
     io.post(q);
 }
 

src/lib/asiolink/asiolink.h

@@ -25,6 +25,8 @@
 
 #include <functional>
 #include <string>
+#include <vector>
+#include <utility>
 
 #include <dns/buffer.h>
 #include <dns/message.h>
@@ -207,10 +209,21 @@ public:
                const bool use_ipv4, const bool use_ipv6,
                SimpleCallback* checkin, DNSLookup* lookup,
                DNSAnswer* answer);
+    /// \brief The constructor without any servers.
+    ///
+    /// Use addServer() to add some servers.
+    DNSService(IOService& io_service, SimpleCallback* checkin,
+               DNSLookup* lookup, DNSAnswer* answer);
     /// \brief The destructor.
     ~DNSService();
     //@}
 
+    /// \brief Add another server to the service
+    void addServer(uint16_t port, const std::string &address);
+    void addServer(const char &port, const std::string &address);
+    /// \brief Remove all servers from the service
+    void clearServers();
+
     /// \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
@@ -510,15 +523,15 @@ public:
     /// \brief Constructor for use when acting as a forwarder
     ///
     /// 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.
-    ///
+    /// object.  The addresses of the forward nameservers is specified,
+    /// and every upstream query will be sent to one random address.
     /// \param dns_service The DNS Service to perform the recursive
-    ///        query on
-    /// \param forward The address of the nameserver to forward to
-    /// \param port The remote port to send the dns query to
-    RecursiveQuery(DNSService& dns_service, const char& forward,
-                   uint16_t port = 53);
+    ///        query on.
+    /// \param upstream Addresses and ports of the upstream servers
+    ///        to forward queries to.
+    RecursiveQuery(DNSService& dns_service,
+                   const std::vector<std::pair<std::string, uint16_t> >&
+                   upstream);
     //@}
 
     /// \brief Initiates an upstream query in the \c RecursiveQuery object.
@@ -536,8 +549,7 @@ public:
                    DNSServer* server);
 private:
     DNSService& dns_service_;
-    IOAddress ns_addr_;
-    uint16_t port_;
+    std::vector<std::pair<std::string, uint16_t> > upstream_;
 };
 
 }      // asiolink

src/lib/asiolink/tests/asiolink_unittest.cc

@@ -365,10 +365,21 @@ protected:
                                       NULL, NULL);
     }
 
+    // Set up an IO Service queue without any addresses
+    void setDNSService() {
+        delete dns_service_;
+        dns_service_ = NULL;
+        delete io_service_;
+        io_service_ = NULL;
+        io_service_ = new IOService();
+        callback_ = new ASIOCallBack(this);
+        dns_service_ = new DNSService(*io_service_, callback_, NULL, NULL);
+    }
+
     // Run a simple server test, on either IPv4 or IPv6, and over either
     // UDP or TCP.  Calls the sendUDP() or sendTCP() methods, which will
     // start the IO Service queue.  The UDPServer or TCPServer that was
-    // created by setIOSerice() will receive the test packet and issue a
+    // created by setIOService() will receive the test packet and issue a
     // callback, which enables us to check that the data it received
     // matches what we sent.
     void doTest(const int family, const int protocol) {
@@ -543,6 +554,32 @@ TEST_F(ASIOLinkTest, v4TCPSendSpecific) {
     EXPECT_THROW(sendTCP(AF_INET6), IOError);
 }
 
+TEST_F(ASIOLinkTest, v6AddServer) {
+    setDNSService();
+    dns_service_->addServer(*TEST_SERVER_PORT, TEST_IPV6_ADDR);
+    doTest(AF_INET6, IPPROTO_TCP);
+
+    EXPECT_THROW(sendTCP(AF_INET), IOError);
+}
+
+TEST_F(ASIOLinkTest, v4AddServer) {
+    setDNSService();
+    dns_service_->addServer(*TEST_SERVER_PORT, TEST_IPV4_ADDR);
+    doTest(AF_INET, IPPROTO_TCP);
+
+    EXPECT_THROW(sendTCP(AF_INET6), IOError);
+}
+
+TEST_F(ASIOLinkTest, DISABLED_clearServers) {
+    // FIXME: Enable when clearServers actually close the sockets
+    //    See #388
+    setDNSService();
+    dns_service_->clearServers();
+
+    EXPECT_THROW(sendTCP(AF_INET), IOError);
+    EXPECT_THROW(sendTCP(AF_INET6), IOError);
+}
+
 TEST_F(ASIOLinkTest, v6TCPOnly) {
     // Open only IPv6 TCP socket.  A subsequent attempt of establishing an
     // IPv4/TCP connection should fail.  See above for why we only test this
@@ -556,16 +593,25 @@ TEST_F(ASIOLinkTest, v4TCPOnly) {
     EXPECT_THROW(sendTCP(AF_INET6), IOError);
 }
 
+vector<pair<string, uint16_t> >
+singleAddress(const string &address, uint16_t port) {
+    vector<pair<string, uint16_t> > result;
+    result.push_back(pair<string, uint16_t>(address, port));
+    return (result);
+}
+
 TEST_F(ASIOLinkTest, recursiveSetupV4) {
     setDNSService(true, false);
     uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
-    EXPECT_NO_THROW(RecursiveQuery(*dns_service_, *TEST_IPV4_ADDR, port));
+    EXPECT_NO_THROW(RecursiveQuery(*dns_service_, singleAddress(TEST_IPV6_ADDR,
+        port)));
 }
 
 TEST_F(ASIOLinkTest, recursiveSetupV6) {
     setDNSService(false, true);
     uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
-    EXPECT_NO_THROW(RecursiveQuery(*dns_service_, *TEST_IPV6_ADDR, port));
+    EXPECT_NO_THROW(RecursiveQuery(*dns_service_, singleAddress(TEST_IPV6_ADDR,
+        port)));
 }
 
 // XXX:
@@ -583,7 +629,7 @@ TEST_F(ASIOLinkTest, recursiveSend) {
     asio::ip::address addr = asio::ip::address::from_string(TEST_IPV4_ADDR);
 
     MockServer server(io, addr, port, NULL, NULL, NULL);
-    RecursiveQuery rq(*dns_service_, *TEST_IPV4_ADDR, port);
+    RecursiveQuery rq(*dns_service_, singleAddress(TEST_IPV4_ADDR, port));
 
     Question q(Name("example.com"), RRClass::IN(), RRType::TXT());
     OutputBufferPtr buffer(new OutputBuffer(0));

src/lib/cc/session.cc

@@ -171,7 +171,7 @@ SessionImpl::readData(void* data, size_t datalen) {
         asio::async_read(socket_, asio::buffer(data, datalen),
                          boost::bind(&setResult, &read_result, _1));
         asio::deadline_timer timer(socket_.io_service());
-    
+
         if (getTimeout() != 0) {
             timer.expires_from_now(boost::posix_time::milliseconds(getTimeout()));
             timer.async_wait(boost::bind(&setResult, &timer_result, _1));