diff --git a/src/bin/auth/Makefile.am b/src/bin/auth/Makefile.am index e9097f21df..36de53dc6e 100644 --- a/src/bin/auth/Makefile.am +++ b/src/bin/auth/Makefile.am @@ -50,6 +50,7 @@ b10_auth_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la b10_auth_LDADD += $(top_builddir)/src/lib/cc/libcc.la b10_auth_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la b10_auth_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la +b10_auth_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la b10_auth_LDADD += $(SQLITE_LIBS) diff --git a/src/bin/auth/benchmarks/Makefile.am b/src/bin/auth/benchmarks/Makefile.am index 05ab7540b1..653d5025f3 100644 --- a/src/bin/auth/benchmarks/Makefile.am +++ b/src/bin/auth/benchmarks/Makefile.am @@ -21,5 +21,6 @@ query_bench_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la query_bench_LDADD += $(top_builddir)/src/lib/cc/libcc.la query_bench_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la query_bench_LDADD += $(top_builddir)/src/lib/log/liblog.la +query_bench_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la query_bench_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la query_bench_LDADD += $(SQLITE_LIBS) diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am index a1114e4694..def99b0564 100644 --- a/src/bin/auth/tests/Makefile.am +++ b/src/bin/auth/tests/Makefile.am @@ -45,6 +45,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la +run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la endif noinst_PROGRAMS = $(TESTS) diff --git a/src/bin/resolver/Makefile.am b/src/bin/resolver/Makefile.am index 9e86ddde1e..75d6249ffc 100644 --- a/src/bin/resolver/Makefile.am +++ b/src/bin/resolver/Makefile.am @@ -48,6 +48,8 @@ b10_resolver_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la b10_resolver_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la b10_resolver_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la +b10_resolver_LDADD += $(top_builddir)/src/lib/cache/libcache.la +b10_resolver_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o b10_resolver_LDFLAGS = -pthread diff --git a/src/bin/resolver/resolver.cc b/src/bin/resolver/resolver.cc index dfd84a661a..95a417d285 100644 --- a/src/bin/resolver/resolver.cc +++ b/src/bin/resolver/resolver.cc @@ -182,6 +182,8 @@ public: MessagePtr message_; }; + +// TODO: REMOVE, USE isc::resolve::MakeErrorMessage? void makeErrorMessage(MessagePtr message, OutputBufferPtr buffer, const Rcode& rcode) @@ -256,25 +258,16 @@ public: const qid_t qid = query_message->getQid(); const bool rd = query_message->getHeaderFlag(Message::HEADERFLAG_RD); const bool cd = query_message->getHeaderFlag(Message::HEADERFLAG_CD); - const Opcode& opcode = query_message->getOpcode(); - - // Fill in the final details of the answer message + + // The opcode and question section should have already been set, + // fill in the final details of the answer message answer_message->setQid(qid); - answer_message->setOpcode(opcode); answer_message->setHeaderFlag(Message::HEADERFLAG_QR); answer_message->setHeaderFlag(Message::HEADERFLAG_RA); - if (rd) { - answer_message->setHeaderFlag(Message::HEADERFLAG_RD); - } - if (cd) { - answer_message->setHeaderFlag(Message::HEADERFLAG_CD); - } + answer_message->setHeaderFlag(Message::HEADERFLAG_RD, rd); + answer_message->setHeaderFlag(Message::HEADERFLAG_CD, cd); - vector questions; - questions.assign(query_message->beginQuestion(), query_message->endQuestion()); - for_each(questions.begin(), questions.end(), QuestionInserter(answer_message)); - // Now we can clear the buffer and render the new message into it buffer->clear(); MessageRenderer renderer(*buffer); diff --git a/src/bin/resolver/tests/Makefile.am b/src/bin/resolver/tests/Makefile.am index a03439c75f..3dc6b3b5a1 100644 --- a/src/bin/resolver/tests/Makefile.am +++ b/src/bin/resolver/tests/Makefile.am @@ -37,6 +37,8 @@ run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la +run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la +run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la # Note the ordering matters: -Wno-... must follow -Wextra (defined in # B10_CXXFLAGS diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index 8ab93ba611..d5486a0f7d 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -1,2 +1,2 @@ SUBDIRS = exceptions dns cc config datasrc python xfr bench log \ - resolve asiolink testutils nsas cache + resolve nsas cache asiolink testutils diff --git a/src/lib/asiolink/Makefile.am b/src/lib/asiolink/Makefile.am index 72f4046cc3..b7138ef3ee 100644 --- a/src/lib/asiolink/Makefile.am +++ b/src/lib/asiolink/Makefile.am @@ -47,3 +47,5 @@ endif libasiolink_la_CPPFLAGS = $(AM_CPPFLAGS) libasiolink_la_LIBADD = $(top_builddir)/src/lib/log/liblog.la libasiolink_la_LIBADD += $(top_builddir)/src/lib/resolve/libresolve.la +libasiolink_la_LIBADD += $(top_builddir)/src/lib/cache/libcache.la +libasiolink_la_LIBADD += $(top_builddir)/src/lib/nsas/libnsas.la diff --git a/src/lib/asiolink/recursive_query.cc b/src/lib/asiolink/recursive_query.cc index 4568b0d0e4..5b767a8980 100644 --- a/src/lib/asiolink/recursive_query.cc +++ b/src/lib/asiolink/recursive_query.cc @@ -26,8 +26,10 @@ #include #include +#include #include +#include #include #include @@ -123,7 +125,7 @@ private: asio::deadline_timer lookup_timer; size_t queries_out_; - + // If we timed out ourselves (lookup timeout), stop issuing queries bool done_; @@ -132,6 +134,26 @@ private: // answer if we do find one later (or if we have a lookup_timeout) bool answer_sent_; + // Reference to our cache + isc::cache::ResolverCache& cache_; + + // perform a single lookup; first we check the cache to see + // if we have a response for our query stored already. if + // so, call handlerecursiveresponse(), if not, we call send() + void doLookup() { + dlog("doLookup: try cache"); + Message cached_message(Message::RENDER); + isc::resolve::initResponseMessage(question_, cached_message); + if (cache_.lookup(question_.getName(), question_.getType(), + question_.getClass(), cached_message)) { + dlog("Message found in cache, returning that"); + handleRecursiveAnswer(cached_message); + } else { + send(); + } + + } + // (re)send the query to the server. void send() { const int uc = upstream_->size(); @@ -184,6 +206,11 @@ private: question_, incoming, cname_target, cname_count_, true); bool found_ns_address = false; + + // If the packet is OK, store it in the cache + if (!isc::resolve::ResponseClassifier::error(category)) { + cache_.update(incoming); + } switch (category) { case isc::resolve::ResponseClassifier::ANSWER: @@ -213,7 +240,7 @@ private: question_.getType()); dlog("Following CNAME chain to " + question_.toText()); - send(); + doLookup(); return false; break; case isc::resolve::ResponseClassifier::NXDOMAIN: @@ -245,11 +272,16 @@ private: // TODO should use NSAS zone_servers_.push_back(addr_t(addr_str, 53)); found_ns_address = true; + break; } } } if (found_ns_address) { // next resolver round + // we do NOT use doLookup() here, but send() (i.e. we + // skip the cache), since if we had the final answer + // instead of a delegation cached, we would have been + // there by now. send(); return false; } else { @@ -291,7 +323,8 @@ public: OutputBufferPtr buffer, isc::resolve::ResolverInterface::CallbackPtr cb, int query_timeout, int client_timeout, int lookup_timeout, - unsigned retries) : + unsigned retries, + isc::cache::ResolverCache& cache) : io_(io), question_(question), answer_message_(answer_message), @@ -306,7 +339,8 @@ public: lookup_timer(io.get_io_service()), queries_out_(0), done_(false), - answer_sent_(false) + answer_sent_(false), + cache_(cache) { // Setup the timer to stop trying (lookup_timeout) if (lookup_timeout >= 0) { @@ -328,7 +362,7 @@ public: setZoneServersToRoot(); } - send(); + doLookup(); } void setZoneServersToRoot() { @@ -369,6 +403,26 @@ public: done_ = true; if (resume && !answer_sent_) { answer_sent_ = true; + + // There are two types of messages we could store in the + // cache; + // 1. answers to our fetches from authoritative servers, + // exactly as we receive them, and + // 2. answers to queries we received from clients, which + // have received additional processing (following CNAME + // chains, for instance) + // + // Doing only the first would mean we would have to re-do + // processing when we get data from our cache, and doing + // only the second would miss out on the side-effect of + // having nameserver data in our cache. + // + // So right now we do both. Since the cache (currently) + // stores Messages on their question section only, this + // does mean that we overwrite the messages we stored in + // the previous iteration if we are following a delegation. + cache_.update(*answer_message_); + resolvercallback_->success(answer_message_); } else { resolvercallback_->failure(); @@ -426,12 +480,26 @@ RecursiveQuery::resolve(const QuestionPtr& question, IOService& io = dns_service_.getIOService(); MessagePtr answer_message(new Message(Message::RENDER)); + isc::resolve::initResponseMessage(*question, *answer_message); + OutputBufferPtr buffer(new OutputBuffer(0)); - - // It will delete itself when it is done - new RunningQuery(io, *question, answer_message, upstream_, - upstream_root_, buffer, callback, query_timeout_, - client_timeout_, lookup_timeout_, retries_); + + dlog("Try out cache first (direct call to resolve)"); + // First try to see if we have something cached in the messagecache + if (cache_.lookup(question->getName(), question->getType(), + question->getClass(), *answer_message)) { + dlog("Message found in cache, returning that"); + // TODO: err, should cache set rcode as well? + answer_message->setRcode(Rcode::NOERROR()); + callback->success(answer_message); + } else { + dlog("Message not found in cache, starting recursive query"); + // It will delete itself when it is done + new RunningQuery(io, *question, answer_message, upstream_, + upstream_root_, buffer, callback, query_timeout_, + client_timeout_, lookup_timeout_, retries_, + cache_); + } } void @@ -448,11 +516,26 @@ RecursiveQuery::resolve(const Question& question, isc::resolve::ResolverInterface::CallbackPtr crs( new isc::resolve::ResolverCallbackServer(server)); + + // TODO: general 'prepareinitialanswer' + answer_message->setOpcode(isc::dns::Opcode::QUERY()); + answer_message->addQuestion(question); - // It will delete itself when it is done - new RunningQuery(io, question, answer_message, upstream_, upstream_root_, - buffer, crs, query_timeout_, client_timeout_, - lookup_timeout_, retries_); + // First try to see if we have something cached in the messagecache + dlog("Try out cache first (started by incoming event)"); + if (cache_.lookup(question.getName(), question.getType(), + question.getClass(), *answer_message)) { + dlog("Message found in cache, returning that"); + // TODO: err, should cache set rcode as well? + answer_message->setRcode(Rcode::NOERROR()); + crs->success(answer_message); + } else { + dlog("Message not found in cache, starting recursive query"); + // It will delete itself when it is done + new RunningQuery(io, question, answer_message, upstream_, upstream_root_, + buffer, crs, query_timeout_, client_timeout_, + lookup_timeout_, retries_, cache_); + } } diff --git a/src/lib/asiolink/recursive_query.h b/src/lib/asiolink/recursive_query.h index 0660c0904a..6ef0069483 100644 --- a/src/lib/asiolink/recursive_query.h +++ b/src/lib/asiolink/recursive_query.h @@ -18,6 +18,7 @@ #include #include #include +#include namespace asiolink { /// \brief The \c RecursiveQuery class provides a layer of abstraction around @@ -107,6 +108,9 @@ private: int client_timeout_; int lookup_timeout_; unsigned retries_; + // Cache. TODO: I think we want this initialized in Resolver class, + // not here + isc::cache::ResolverCache cache_; }; } // namespace asiolink diff --git a/src/lib/asiolink/tests/Makefile.am b/src/lib/asiolink/tests/Makefile.am index 354fc0215f..d47527874f 100644 --- a/src/lib/asiolink/tests/Makefile.am +++ b/src/lib/asiolink/tests/Makefile.am @@ -36,6 +36,8 @@ run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la +run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la +run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) $(LOG4CXX_LDFLAGS) diff --git a/src/lib/asiolink/udp_server.cc b/src/lib/asiolink/udp_server.cc index 7a0e146c6d..9a5f00ebac 100644 --- a/src/lib/asiolink/udp_server.cc +++ b/src/lib/asiolink/udp_server.cc @@ -28,6 +28,8 @@ #include #include +#include + using namespace asio; using asio::ip::udp; using isc::log::dlog; diff --git a/src/lib/cache/message_cache.cc b/src/lib/cache/message_cache.cc index 70e7c67fc7..3f2c37ab02 100644 --- a/src/lib/cache/message_cache.cc +++ b/src/lib/cache/message_cache.cc @@ -57,6 +57,7 @@ bool MessageCache::update(const Message& msg) { QuestionIterator iter = msg.beginQuestion(); std::string entry_name = genCacheEntryName((*iter)->getName(), (*iter)->getType()); + std::cout << msg.toText(); HashKey entry_key = HashKey(entry_name, RRClass(message_class_)); // The simplest way to update is removing the old message entry directly. diff --git a/src/lib/cache/rrset_cache.h b/src/lib/cache/rrset_cache.h index 2062757378..15084c9bf6 100644 --- a/src/lib/cache/rrset_cache.h +++ b/src/lib/cache/rrset_cache.h @@ -15,7 +15,7 @@ #ifndef __RRSET_CACHE_H #define __RRSET_CACHE_H -#include +#include #include #include diff --git a/src/lib/resolve/resolve.cc b/src/lib/resolve/resolve.cc index 312c89d57b..f741121464 100644 --- a/src/lib/resolve/resolve.cc +++ b/src/lib/resolve/resolve.cc @@ -14,6 +14,9 @@ #include +#include +#include + using namespace isc::dns; namespace { @@ -44,6 +47,23 @@ makeErrorMessage(MessagePtr answer_message, answer_message->setRcode(error_code); } +void initResponseMessage(const isc::dns::Message& query_message, + isc::dns::Message& response_message) +{ + response_message.setOpcode(query_message.getOpcode()); + response_message.setQid(query_message.getQid()); + assert(response_message.getRRCount(Message::SECTION_QUESTION) == 0); + response_message.appendSection(Message::SECTION_QUESTION, + query_message); +} + +void initResponseMessage(const isc::dns::Question& question, + isc::dns::Message& response_message) +{ + response_message.setOpcode(isc::dns::Opcode::QUERY()); + response_message.addQuestion(question); +} + void copyResponseMessage(const Message& source, MessagePtr target) { target->setRcode(source.getRcode()); diff --git a/src/lib/resolve/resolve.h b/src/lib/resolve/resolve.h index b08c30c2ff..550b6204e4 100644 --- a/src/lib/resolve/resolve.h +++ b/src/lib/resolve/resolve.h @@ -42,6 +42,42 @@ namespace resolve { void makeErrorMessage(isc::dns::MessagePtr answer_message, const isc::dns::Rcode& error_code); + +/// \brief Initialize a response message +/// +/// Based on the given query message, this fills in the very +/// first details of the response (i.e. the Question section and +/// the Opcode). This allows for direct usage of makeErrorMessage(), +/// as well as ResolveCache.lookup(). +/// +/// Raises an isc::dns::InvalidMessageOperation if reponse_message is +/// not in RENDER mode. +/// +/// \param query_message The query message to take the Question, Qid, +/// and Opcode from. +/// \param response_message The fresh response message to initialize +/// (must be in RENDER mode) +void initResponseMessage(const isc::dns::Message& query_message, + isc::dns::Message& response_message); + + +/// \brief Initialize a response message +/// +/// Based on the given question, this fills in the very +/// first details of the response (i.e. the Question section and the +/// Opcode Query). This allows for direct usage of makeErrorMessage(), +/// as well as ResolveCache.lookup(). +/// +/// Raises an isc::dns::InvalidMessageOperation if reponse_message is +/// not in RENDER mode. +/// +/// \param question The question to place in the Question section +/// \param response_message The fresh response message to initialize +/// (must be in RENDER mode) +void initResponseMessage(const isc::dns::Question& question, + isc::dns::Message& response_message); + + /// \brief Copies the parts relevant for a DNS response to the /// target message /// diff --git a/src/lib/resolve/tests/Makefile.am b/src/lib/resolve/tests/Makefile.am index 3b5f41afbb..e669384f65 100644 --- a/src/lib/resolve/tests/Makefile.am +++ b/src/lib/resolve/tests/Makefile.am @@ -19,6 +19,7 @@ run_unittests_SOURCES += resolver_callback_unittest.cc run_unittests_SOURCES += response_classifier_unittest.cc run_unittests_LDADD = $(GTEST_LDADD) +run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la run_unittests_LDADD += $(top_builddir)/src/lib/resolve/libresolve.la run_unittests_LDADD += $(top_builddir)/src/lib/dns/libdns++.la diff --git a/src/lib/resolve/tests/resolve_unittest.cc b/src/lib/resolve/tests/resolve_unittest.cc index fd68f16033..85d264dd12 100644 --- a/src/lib/resolve/tests/resolve_unittest.cc +++ b/src/lib/resolve/tests/resolve_unittest.cc @@ -122,21 +122,47 @@ TEST_F(ResolveHelperFunctionsTest, makeErrorMessageNonEmptyMessage) { } void -compareSections(const MessagePtr message_a, const MessagePtr message_b, +compareSections(const Message& message_a, const Message& message_b, Message::Section section) { - RRsetIterator rrs_a = message_a->beginSection(section); - RRsetIterator rrs_b = message_b->beginSection(section); - while (rrs_a != message_a->endSection(section) && - rrs_b != message_b->endSection(section) + RRsetIterator rrs_a = message_a.beginSection(section); + RRsetIterator rrs_b = message_b.beginSection(section); + while (rrs_a != message_a.endSection(section) && + rrs_b != message_b.endSection(section) ) { EXPECT_EQ(*rrs_a, *rrs_b); ++rrs_a; ++rrs_b; } // can't use EXPECT_EQ here, no eqHelper for endsection comparison - EXPECT_TRUE(rrs_a == message_a->endSection(section)); - EXPECT_TRUE(rrs_b == message_b->endSection(section)); + EXPECT_TRUE(rrs_a == message_a.endSection(section)); + EXPECT_TRUE(rrs_b == message_b.endSection(section)); +} + +TEST_F(ResolveHelperFunctionsTest, initResponseMessage) { + Message response_parse(Message::PARSE); + EXPECT_THROW(isc::resolve::initResponseMessage(*message_a_, + response_parse), + isc::dns::InvalidMessageOperation); + EXPECT_THROW(isc::resolve::initResponseMessage(*question_, + response_parse), + isc::dns::InvalidMessageOperation); + + Message response1(Message::RENDER); + isc::resolve::initResponseMessage(*message_a_, response1); + ASSERT_EQ(message_a_->getOpcode(), response1.getOpcode()); + ASSERT_EQ(message_a_->getQid(), response1.getQid()); + isc::dns::QuestionIterator qi = response1.beginQuestion(); + ASSERT_EQ(*question_, **qi); + ASSERT_TRUE(++qi == response1.endQuestion()); + + Message response2(Message::RENDER); + isc::resolve::initResponseMessage(*question_, response2); + ASSERT_EQ(Opcode::QUERY(), response2.getOpcode()); + ASSERT_EQ(0, response2.getQid()); + qi = response2.beginQuestion(); + ASSERT_EQ(*question_, **qi); + ASSERT_TRUE(++qi == response2.endQuestion()); } TEST_F(ResolveHelperFunctionsTest, copyAnswerMessage) { @@ -164,9 +190,9 @@ TEST_F(ResolveHelperFunctionsTest, copyAnswerMessage) { message_a_->getRRCount(Message::SECTION_ADDITIONAL)); - compareSections(message_a_, message_b_, Message::SECTION_ANSWER); - compareSections(message_a_, message_b_, Message::SECTION_AUTHORITY); - compareSections(message_a_, message_b_, Message::SECTION_ADDITIONAL); + compareSections(*message_a_, *message_b_, Message::SECTION_ANSWER); + compareSections(*message_a_, *message_b_, Message::SECTION_AUTHORITY); + compareSections(*message_a_, *message_b_, Message::SECTION_ADDITIONAL); } } // Anonymous namespace