Commit 1dbc33e9 authored by Jelte Jansen's avatar Jelte Jansen
Browse files

Merge remote branch 'origin/trac489'

Conflicts:
	src/bin/resolver/resolver.cc
	src/lib/asiolink/asiolink.cc
	src/lib/asiolink/asiolink.h
	src/lib/asiolink/tests/asiolink_unittest.cc
parents 5720ab60 6e9d5c8b
......@@ -64,7 +64,9 @@ private:
public:
ResolverImpl() :
config_session_(NULL),
timeout_(2000),
query_timeout_(2000),
client_timeout_(4000),
lookup_timeout_(30000),
retries_(3),
rec_query_(NULL)
{}
......@@ -76,7 +78,12 @@ public:
void querySetup(DNSService& dnss) {
assert(!rec_query_); // queryShutdown must be called first
dlog("Query setup");
rec_query_ = new RecursiveQuery(dnss, upstream_, upstream_root_, timeout_, retries_);
rec_query_ = new RecursiveQuery(dnss, upstream_,
upstream_root_,
query_timeout_,
client_timeout_,
lookup_timeout_,
retries_);
}
void queryShutdown() {
......@@ -143,8 +150,13 @@ public:
/// Addresses we listen on
vector<addr_t> listen_;
/// Time in milliseconds, to timeout
int timeout_;
/// Timeout for outgoing queries in milliseconds
int query_timeout_;
/// Timeout for incoming client queries in milliseconds
int client_timeout_;
/// Timeout for lookup processing in milliseconds
int lookup_timeout_;
/// Number of retries after timeout
unsigned retries_;
......@@ -487,16 +499,34 @@ Resolver::updateConfig(ConstElementPtr config) {
ConstElementPtr listenAddressesE(config->get("listen_on"));
vector<addr_t> listenAddresses(parseAddresses(listenAddressesE));
bool set_timeouts(false);
int timeout = impl_->timeout_;
int qtimeout = impl_->query_timeout_;
int ctimeout = impl_->client_timeout_;
int ltimeout = impl_->lookup_timeout_;
unsigned retries = impl_->retries_;
ConstElementPtr timeoutE(config->get("timeout")),
retriesE(config->get("retries"));
if (timeoutE) {
ConstElementPtr qtimeoutE(config->get("timeout_query")),
ctimeoutE(config->get("timeout_client")),
ltimeoutE(config->get("timeout_lookup")),
retriesE(config->get("retries"));
if (qtimeoutE) {
// It should be safe to just get it, the config manager should
// check for us
timeout = timeoutE->intValue();
if (timeout < -1) {
isc_throw(BadValue, "Timeout too small");
qtimeout = qtimeoutE->intValue();
if (qtimeout < -1) {
isc_throw(BadValue, "Query timeout too small");
}
set_timeouts = true;
}
if (ctimeoutE) {
ctimeout = ctimeoutE->intValue();
if (ctimeout < -1) {
isc_throw(BadValue, "Client timeout too small");
}
set_timeouts = true;
}
if (ltimeoutE) {
ltimeout = ltimeoutE->intValue();
if (ltimeout < -1) {
isc_throw(BadValue, "Lookup timeout too small");
}
set_timeouts = true;
}
......@@ -523,7 +553,7 @@ Resolver::updateConfig(ConstElementPtr config) {
setRootAddresses(rootAddresses);
}
if (set_timeouts) {
setTimeouts(timeout, retries);
setTimeouts(qtimeout, ctimeout, ltimeout, retries);
need_query_restart = true;
}
......@@ -610,15 +640,36 @@ Resolver::setListenAddresses(const vector<addr_t>& addresses) {
}
void
Resolver::setTimeouts(int timeout, unsigned retries) {
dlog("Setting timeout to " + boost::lexical_cast<string>(timeout) +
" and retry count to " + boost::lexical_cast<string>(retries));
impl_->timeout_ = timeout;
Resolver::setTimeouts(int query_timeout, int client_timeout,
int lookup_timeout, unsigned retries) {
dlog("Setting query timeout to " + boost::lexical_cast<string>(query_timeout) +
", client timeout to " + boost::lexical_cast<string>(client_timeout) +
", lookup timeout to " + boost::lexical_cast<string>(lookup_timeout) +
" and retry count to " + boost::lexical_cast<string>(retries));
impl_->query_timeout_ = query_timeout;
impl_->client_timeout_ = client_timeout;
impl_->lookup_timeout_ = lookup_timeout;
impl_->retries_ = retries;
}
pair<int, unsigned>
Resolver::getTimeouts() const {
return (pair<int, unsigned>(impl_->timeout_, impl_->retries_));
int
Resolver::getQueryTimeout() const {
return impl_->query_timeout_;
}
int
Resolver::getClientTimeout() const {
return impl_->client_timeout_;
}
int
Resolver::getLookupTimeout() const {
return impl_->lookup_timeout_;
}
int
Resolver::getRetries() const {
return impl_->retries_;
}
vector<addr_t>
......
......@@ -144,7 +144,10 @@ public:
* \param retries The number of retries (0 means try the first time only,
* do not retry).
*/
void setTimeouts(int timeout = -1, unsigned retries = 0);
void setTimeouts(int query_timeout = 2000,
int client_timeout = 4000,
int lookup_timeout = 30000,
unsigned retries = 3);
/**
* \short Get info about timeouts.
......@@ -153,6 +156,39 @@ public:
*/
std::pair<int, unsigned> getTimeouts() const;
/**
* \brief Get the timeout for outgoing queries
*
* \returns Timeout for outgoing queries
*/
int getQueryTimeout() const;
/**
* \brief Get the timeout for incoming client queries
*
* After this timeout, a SERVFAIL shall be sent back
* (internal resolving on the query will continue, see
* \c getLookupTimeout())
*
* \returns Timeout for outgoing queries
*/
int getClientTimeout() const;
/**
* \brief Get the timeout for lookups
*
* After this timeout, internal processing shall stop
*/
int getLookupTimeout() const;
/**
* \brief Get the number of retries for outgoing queries
*
* If a query times out (value of \c getQueryTimeout()), we
* will retry this number of times
*/
int getRetries() const;
private:
ResolverImpl* impl_;
asiolink::DNSService* dnss_;
......
......@@ -4,11 +4,23 @@
"module_description": "Recursive service",
"config_data": [
{
"item_name": "timeout",
"item_name": "timeout_query",
"item_type": "integer",
"item_optional": False,
"item_default": 2000
},
{
"item_name": "timeout_client",
"item_type": "integer",
"item_optional": False,
"item_default": 4000
},
{
"item_name": "timeout_lookup",
"item_type": "integer",
"item_optional": False,
"item_default": 30000
},
{
"item_name": "retries",
"item_type": "integer",
......
......@@ -237,31 +237,51 @@ TEST_F(ResolverConfig, invalidListenAddresses) {
// Just test it sets and gets the values correctly
TEST_F(ResolverConfig, timeouts) {
server.setTimeouts(0, 1);
EXPECT_EQ(0, server.getTimeouts().first);
EXPECT_EQ(1, server.getTimeouts().second);
server.setTimeouts(0, 1, 2, 3);
EXPECT_EQ(0, server.getQueryTimeout());
EXPECT_EQ(1, server.getClientTimeout());
EXPECT_EQ(2, server.getLookupTimeout());
EXPECT_EQ(3, server.getRetries());
server.setTimeouts();
EXPECT_EQ(-1, server.getTimeouts().first);
EXPECT_EQ(0, server.getTimeouts().second);
EXPECT_EQ(2000, server.getQueryTimeout());
EXPECT_EQ(4000, server.getClientTimeout());
EXPECT_EQ(30000, server.getLookupTimeout());
EXPECT_EQ(3, server.getRetries());
}
TEST_F(ResolverConfig, timeoutsConfig) {
ElementPtr config = Element::fromJSON("{"
"\"timeout\": 1000,"
"\"retries\": 3"
"\"timeout_query\": 1000,"
"\"timeout_client\": 2000,"
"\"timeout_lookup\": 3000,"
"\"retries\": 4"
"}");
ConstElementPtr result(server.updateConfig(config));
EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
EXPECT_EQ(1000, server.getTimeouts().first);
EXPECT_EQ(3, server.getTimeouts().second);
EXPECT_EQ(1000, server.getQueryTimeout());
EXPECT_EQ(2000, server.getClientTimeout());
EXPECT_EQ(3000, server.getLookupTimeout());
EXPECT_EQ(4, server.getRetries());
}
TEST_F(ResolverConfig, invalidTimeoutsConfig) {
invalidTest("{"
"\"timeout\": \"error\""
"\"timeout_query\": \"error\""
"}");
invalidTest("{"
"\"timeout\": -2"
"\"timeout_query\": -2"
"}");
invalidTest("{"
"\"timeout_client\": \"error\""
"}");
invalidTest("{"
"\"timeout_client\": -2"
"}");
invalidTest("{"
"\"timeout_lookup\": \"error\""
"}");
invalidTest("{"
"\"timeout_lookup\": -2"
"}");
invalidTest("{"
"\"retries\": \"error\""
......
......@@ -285,10 +285,11 @@ typedef std::vector<std::pair<std::string, uint16_t> > AddressVector;
RecursiveQuery::RecursiveQuery(DNSService& dns_service,
const AddressVector& upstream,
const AddressVector& upstream_root,
int timeout, unsigned retries) :
int query_timeout, int client_timeout, int lookup_timeout,
unsigned retries) :
dns_service_(dns_service), upstream_(new AddressVector(upstream)),
upstream_root_(new AddressVector(upstream_root)),
timeout_(timeout), retries_(retries)
query_timeout_(query_timeout), client_timeout_(client_timeout),
lookup_timeout_(lookup_timeout), retries_(retries)
{}
namespace {
......@@ -361,15 +362,11 @@ private:
* computation of average RTT, increase with each retry, etc.
*/
// Timeout information
int timeout_;
int query_timeout_;
unsigned retries_;
// normal query state
// if we change this to running and add a sent, we can do
// decoupled timeouts i think
bool done;
// Not using NSAS at this moment, so we keep a list
// of 'current' zone servers
vector<addr_t> zone_servers_;
......@@ -379,19 +376,28 @@ private:
question_ = new_question;
}
deadline_timer client_timer;
deadline_timer lookup_timer;
size_t queries_out_;
// If we timed out ourselves (lookup timeout), stop issuing queries
bool done_;
// (re)send the query to the server.
void send() {
const int uc = upstream_->size();
const int zs = zone_servers_.size();
buffer_->clear();
if (uc > 0) {
++queries_out_;
int serverIndex = rand() % uc;
dlog("Sending upstream query (" + question_.toText() +
") to " + upstream_->at(serverIndex).first);
UDPQuery query(io_, question_,
upstream_->at(serverIndex).first,
upstream_->at(serverIndex).second, buffer_, this,
timeout_);
query_timeout_);
io_.post(query);
} else if (zs > 0) {
int serverIndex = rand() % zs;
......@@ -400,7 +406,7 @@ private:
UDPQuery query(io_, question_,
zone_servers_.at(serverIndex).first,
zone_servers_.at(serverIndex).second, buffer_, this,
timeout_);
query_timeout_);
io_.post(query);
} else {
dlog("Error, no upstream servers to send to.");
......@@ -475,7 +481,8 @@ public:
RunningQuery(asio::io_service& io, const Question &question,
MessagePtr answer_message, shared_ptr<AddressVector> upstream,
shared_ptr<AddressVector> upstream_root,
OutputBufferPtr buffer, DNSServer* server, int timeout,
OutputBufferPtr buffer, DNSServer* server,
int query_timeout, int client_timeout, int lookup_timeout,
unsigned retries) :
io_(io),
question_(question),
......@@ -484,13 +491,27 @@ public:
upstream_root_(upstream_root),
buffer_(buffer),
server_(server->clone()),
timeout_(timeout),
query_timeout_(query_timeout),
retries_(retries),
zone_servers_()
client_timer(io),
lookup_timer(io),
queries_out_(0),
done_(false)
{
dlog("Started a new RunningQuery");
done = false;
// Setup the timer to stop trying (lookup_timeout)
if (lookup_timeout >= 0) {
lookup_timer.expires_from_now(
boost::posix_time::milliseconds(lookup_timeout));
lookup_timer.async_wait(boost::bind(&RunningQuery::stop, this, false));
}
// Setup the timer to send an answer (client_timeout)
if (client_timeout >= 0) {
client_timer.expires_from_now(
boost::posix_time::milliseconds(client_timeout));
client_timer.async_wait(boost::bind(&RunningQuery::clientTimeout, this));
}
// should use NSAS for root servers
// Adding root servers if not a forwarder
if (upstream_->empty()) {
......@@ -511,14 +532,43 @@ public:
}
}
}
send();
}
virtual void clientTimeout() {
// right now, just stop (should make SERVFAIL and send that
// back, but not stop)
stop(false);
}
virtual void stop(bool resume) {
// if we cancel our timers, we will still get an event for
// that, so we cannot delete ourselves just yet (those events
// would be bound to a deleted object)
// cancel them one by one, both cancels should get us back
// here again.
// same goes if we have an outstanding query (can't delete
// until that one comes back to us)
done_ = true;
server_->resume(resume);
if (lookup_timer.cancel() != 0) {
return;
}
if (client_timer.cancel() != 0) {
return;
}
if (queries_out_ > 0) {
return;
}
delete this;
}
// This function is used as callback from DNSQuery.
// This function is used as callback from DNSQuery.
virtual void operator()(UDPQuery::Result result) {
// XXX is this the place for TCP retry?
if (result != UDPQuery::TIME_OUT) {
if (!done_ && result != UDPQuery::TIME_OUT) {
// we got an answer
Message incoming(Message::PARSE);
InputBuffer ibuf(buffer_->getData(), buffer_->getLength());
......@@ -526,24 +576,22 @@ public:
if (upstream_->size() == 0 &&
incoming.getRcode() == Rcode::NOERROR()) {
done = handleRecursiveAnswer(incoming);
done_ = handleRecursiveAnswer(incoming);
} else {
copyAnswerMessage(incoming, answer_message_);
done = true;
done_ = true;
}
if (done) {
server_->resume(result == UDPQuery::SUCCESS);
delete this;
if (done_) {
stop(result == UDPQuery::SUCCESS);
}
} else if (retries_--) {
} else if (!done_ && retries_--) {
// We timed out, but we have some retries, so send again
dlog("Timeout, resending query");
send();
} else {
// out of retries, give up for now
server_->resume(false);
delete this;
// We are done
stop(false);
}
}
};
......@@ -563,7 +611,8 @@ RecursiveQuery::sendQuery(const Question& question,
asio::io_service& io = dns_service_.get_io_service();
// It will delete itself when it is done
new RunningQuery(io, question, answer_message, upstream_, upstream_root_,
buffer, server, timeout_, retries_);
buffer, server, query_timeout_, client_timeout_,
lookup_timeout_, retries_);
}
class IntervalTimerImpl {
......
......@@ -550,7 +550,10 @@ public:
upstream,
const std::vector<std::pair<std::string, uint16_t> >&
upstream_root,
int timeout = -1, unsigned retries = 0);
int query_timeout = 2000,
int client_timeout = 4000,
int lookup_timeout = 30000,
unsigned retries = 3);
//@}
/// \brief Initiates an upstream query in the \c RecursiveQuery object.
......@@ -573,7 +576,9 @@ private:
upstream_;
boost::shared_ptr<std::vector<std::pair<std::string, uint16_t> > >
upstream_root_;
int timeout_;
int query_timeout_;
int client_timeout_;
int lookup_timeout_;
unsigned retries_;
};
......
......@@ -698,20 +698,59 @@ TEST_F(ASIOLinkTest, forwarderSend) {
EXPECT_EQ(q.getClass(), q2->getClass());
}
// Test it tries the correct amount of times before giving up
TEST_F(ASIOLinkTest, recursiveTimeout) {
// Prepare the service (we do not use the common setup, we do not answer
setDNSService();
// Prepare the socket
res_ = resolveAddress(AF_INET, IPPROTO_UDP, true);
sock_ = socket(res_->ai_family, res_->ai_socktype, res_->ai_protocol);
int
createTestSocket()
{
struct addrinfo* res_ = resolveAddress(AF_INET, IPPROTO_UDP, true);
int sock_ = socket(res_->ai_family, res_->ai_socktype, res_->ai_protocol);
if (sock_ < 0) {
isc_throw(IOError, "failed to open test socket");
}
if (bind(sock_, res_->ai_addr, res_->ai_addrlen) < 0) {
isc_throw(IOError, "failed to bind test socket");
}
return sock_;
}
int
setSocketTimeout(int sock_, size_t tv_sec, size_t tv_usec) {
const struct timeval timeo = { tv_sec, tv_usec };
int recv_options = 0;
if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) {
if (errno == ENOPROTOOPT) { // see ASIOLinkTest::recvUDP()
recv_options = MSG_DONTWAIT;
} else {
isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno));
}
}
return recv_options;
}
// try to read from the socket max time
// *num is incremented for every succesfull read
// returns true if it can read max times, false otherwise
bool tryRead(int sock_, int recv_options, size_t max, int* num) {
size_t i = 0;
do {
char inbuff[512];
if (recv(sock_, inbuff, sizeof(inbuff), recv_options) < 0) {
return false;
} else {
++i;
++*num;
}
} while (i < max);
return true;
}
// Test it tries the correct amount of times before giving up
TEST_F(ASIOLinkTest, forwardQueryTimeout) {
// Prepare the service (we do not use the common setup, we do not answer
setDNSService();
// Prepare the socket
sock_ = createTestSocket();
// Prepare the server
bool done(true);
......@@ -722,7 +761,7 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
RecursiveQuery query(*dns_service_,
singleAddress(TEST_IPV4_ADDR, port),
singleAddress(TEST_IPV4_ADDR, port),
10, 2);
10, 4000, 3000, 2);
Question question(Name("example.net"), RRClass::IN(), RRType::A());
OutputBufferPtr buffer(new OutputBuffer(0));
MessagePtr answer(new Message(Message::RENDER));
......@@ -733,27 +772,103 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
// Read up to 3 packets. Use some ad hoc timeout to prevent an infinite
// block (see also recvUDP()).
const struct timeval timeo = { 10, 0 };
int recv_options = 0;
if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) {
if (errno == ENOPROTOOPT) { // see ASIOLinkTest::recvUDP()
recv_options = MSG_DONTWAIT;
} else {
isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno));
}
}
int recv_options = setSocketTimeout(sock_, 10, 0);
int num = 0;
do {
char inbuff[512];
if (recv(sock_, inbuff, sizeof(inbuff), recv_options) < 0) {
num = -1;
break;
}
} while (++num < 3);
bool read_success = tryRead(sock_, recv_options, 3, &num);
// The query should fail
EXPECT_FALSE(done);
EXPECT_EQ(3, num);
EXPECT_TRUE(read_success);
}
// If we set client timeout to lower than querytimeout, we should
// get a failure answer, but still see retries
// (no actual answer is given here yet)
TEST_F(ASIOLinkTest, forwardClientTimeout) {
// Prepare the service (we do not use the common setup, we do not answer
setDNSService();
sock_ = createTestSocket();
// Prepare the server
bool done(true);