BIND 10 trac491, updated. dee286dad1ba1b47d89659572fc2ce8fd1a94af6 [trac491] rest of review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Feb 22 10:13:55 UTC 2011
The branch, trac491 has been updated
via dee286dad1ba1b47d89659572fc2ce8fd1a94af6 (commit)
via d22c96099aacc094be00c1fe51433d4b39b9656f (commit)
from c9a06623e3e6041342046d52f502ce5e478ef29c (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit dee286dad1ba1b47d89659572fc2ce8fd1a94af6
Author: Jelte Jansen <jelte at isc.org>
Date: Tue Feb 22 10:59:25 2011 +0100
[trac491] rest of review comments
updated a comment description, and moved caching of answers to after the response classifier has run
commit d22c96099aacc094be00c1fe51433d4b39b9656f
Author: Jelte Jansen <jelte at isc.org>
Date: Tue Feb 22 10:23:48 2011 +0100
[trac491] review comments, the smaller issues
-----------------------------------------------------------------------
Summary of changes:
src/bin/resolver/resolver.cc | 8 ++------
src/lib/asiolink/recursive_query.cc | 31 +++++++++++++++++++++++--------
src/lib/cache/message_cache.cc | 3 ---
src/lib/cache/resolver_cache.cc | 1 -
src/lib/resolve/resolve.cc | 1 +
src/lib/resolve/resolve.h | 4 ++--
6 files changed, 28 insertions(+), 20 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/resolver/resolver.cc b/src/bin/resolver/resolver.cc
index 74eddfd..95a417d 100644
--- a/src/bin/resolver/resolver.cc
+++ b/src/bin/resolver/resolver.cc
@@ -265,12 +265,8 @@ public:
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);
// Now we can clear the buffer and render the new message into it
buffer->clear();
diff --git a/src/lib/asiolink/recursive_query.cc b/src/lib/asiolink/recursive_query.cc
index ed923a6..33b1391 100644
--- a/src/lib/asiolink/recursive_query.cc
+++ b/src/lib/asiolink/recursive_query.cc
@@ -207,6 +207,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:
@@ -268,6 +273,7 @@ private:
// TODO should use NSAS
zone_servers_.push_back(addr_t(addr_str, 53));
found_ns_address = true;
+ break;
}
}
}
@@ -395,11 +401,23 @@ public:
// until that one comes back to us)
done_ = true;
if (resume && !answer_sent_) {
- // If we have a full successful answer, let's store that
- // as well
- // (note: we can either do this or only cache answers
- // we receive, but in that case we'd need to re-do all
- // answer processing, e.g. cname handling etc)
+ // 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_);
@@ -428,9 +446,6 @@ public:
InputBuffer ibuf(buffer_->getData(), buffer_->getLength());
incoming.fromWire(ibuf);
- // let's first dunk it into our cache
- cache_.update(incoming);
-
if (upstream_->size() == 0 &&
incoming.getRcode() == Rcode::NOERROR()) {
done_ = handleRecursiveAnswer(incoming);
diff --git a/src/lib/cache/message_cache.cc b/src/lib/cache/message_cache.cc
index 3e447a9..f8e5910 100644
--- a/src/lib/cache/message_cache.cc
+++ b/src/lib/cache/message_cache.cc
@@ -45,7 +45,6 @@ MessageCache::lookup(const isc::dns::Name& qname,
isc::dns::Message& response)
{
std::string entry_name = genCacheEntryName(qname, qtype);
- std::cout << "[XX] MESSAGECACHE LOOKUP: " << entry_name << std::endl;
HashKey entry_key = HashKey(entry_name, RRClass(message_class_));
MessageEntryPtr msg_entry = message_table_.get(entry_key);
if(msg_entry) {
@@ -60,8 +59,6 @@ bool
MessageCache::update(const Message& msg) {
QuestionIterator iter = msg.beginQuestion();
std::string entry_name = genCacheEntryName((*iter)->getName(), (*iter)->getType());
- std::cout << "[XX] MESSAGECACHE UPDATE: " << entry_name << std::endl;
- std::cout << "[XX] FOR MESSAGE:" << std::endl;
std::cout << msg.toText();
HashKey entry_key = HashKey(entry_name, RRClass(message_class_));
diff --git a/src/lib/cache/resolver_cache.cc b/src/lib/cache/resolver_cache.cc
index c9183b0..04b6346 100644
--- a/src/lib/cache/resolver_cache.cc
+++ b/src/lib/cache/resolver_cache.cc
@@ -202,7 +202,6 @@ ResolverCache::lookupClosestRRset(const isc::dns::Name& qname,
bool
ResolverCache::update(const isc::dns::Message& msg) {
QuestionIterator iter = msg.beginQuestion();
- RRClass c = (*iter)->getClass();
ResolverClassCache* cc = getClassCache((*iter)->getClass());
if (cc) {
return (cc->update(msg));
diff --git a/src/lib/resolve/resolve.cc b/src/lib/resolve/resolve.cc
index c2bbe5c..f741121 100644
--- a/src/lib/resolve/resolve.cc
+++ b/src/lib/resolve/resolve.cc
@@ -52,6 +52,7 @@ void initResponseMessage(const isc::dns::Message& query_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);
}
diff --git a/src/lib/resolve/resolve.h b/src/lib/resolve/resolve.h
index 4544eb5..550b620 100644
--- a/src/lib/resolve/resolve.h
+++ b/src/lib/resolve/resolve.h
@@ -56,7 +56,7 @@ void makeErrorMessage(isc::dns::MessagePtr answer_message,
/// \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 type Message::RENDER)
+/// (must be in RENDER mode)
void initResponseMessage(const isc::dns::Message& query_message,
isc::dns::Message& response_message);
@@ -73,7 +73,7 @@ void initResponseMessage(const isc::dns::Message& query_message,
///
/// \param question The question to place in the Question section
/// \param response_message The fresh response message to initialize
-/// (must be type Message::RENDER)
+/// (must be in RENDER mode)
void initResponseMessage(const isc::dns::Question& question,
isc::dns::Message& response_message);
More information about the bind10-changes
mailing list