[svn] commit: r3720 - in /branches/trac408/src/lib/nsas: ./ tests/
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Dec 4 13:50:08 UTC 2010
Author: vorner
Date: Sat Dec 4 13:50:07 2010
New Revision: 3720
Log:
Update the nameserver address store
And its tests
Fix one bug in zone entry on the way
Modified:
branches/trac408/src/lib/nsas/Makefile.am
branches/trac408/src/lib/nsas/TODO
branches/trac408/src/lib/nsas/nameserver_address_store.cc
branches/trac408/src/lib/nsas/nameserver_address_store.h
branches/trac408/src/lib/nsas/tests/Makefile.am
branches/trac408/src/lib/nsas/tests/nameserver_address_store_unittest.cc
branches/trac408/src/lib/nsas/tests/nsas_test.h
branches/trac408/src/lib/nsas/tests/zone_entry_unittest.cc
branches/trac408/src/lib/nsas/zone_entry.cc
branches/trac408/src/lib/nsas/zone_entry.h
Modified: branches/trac408/src/lib/nsas/Makefile.am
==============================================================================
--- branches/trac408/src/lib/nsas/Makefile.am (original)
+++ branches/trac408/src/lib/nsas/Makefile.am Sat Dec 4 13:50:07 2010
@@ -14,7 +14,7 @@
libnsas_la_SOURCES += hash_key.cc hash_key.h
libnsas_la_SOURCES += hash_table.h
libnsas_la_SOURCES += lru_list.h
-#libnsas_la_SOURCES += nameserver_address_store.cc nameserver_address_store.h
+libnsas_la_SOURCES += nameserver_address_store.cc nameserver_address_store.h
libnsas_la_SOURCES += nameserver_address.h
libnsas_la_SOURCES += nameserver_entry.cc nameserver_entry.h
libnsas_la_SOURCES += nsas_entry_compare.h
Modified: branches/trac408/src/lib/nsas/TODO
==============================================================================
--- branches/trac408/src/lib/nsas/TODO (original)
+++ branches/trac408/src/lib/nsas/TODO Sat Dec 4 13:50:07 2010
@@ -1,8 +1,9 @@
NameserverEntry:
* Reuse data on timeout/requery
-The NSAS itself:
-* Implement/recreate
+Global:
+* There are TODO notes in tests, some more tests should be added to stress
+ and test it more.
Long term:
* Make a mechanism the cache (which does not exist at the time of writing this
Modified: branches/trac408/src/lib/nsas/nameserver_address_store.cc
==============================================================================
--- branches/trac408/src/lib/nsas/nameserver_address_store.cc (original)
+++ branches/trac408/src/lib/nsas/nameserver_address_store.cc Sat Dec 4 13:50:07 2010
@@ -21,11 +21,14 @@
#include <config.h>
#include <dns/rdataclass.h>
+#include "hash_table.h"
+#include "lru_list.h"
#include "hash_deleter.h"
#include "nsas_entry_compare.h"
#include "nameserver_entry.h"
#include "nameserver_address_store.h"
#include "zone_entry.h"
+#include "address_request_callback.h"
using namespace isc::dns;
using namespace std;
@@ -39,183 +42,54 @@
// The LRU lists are set equal to three times the size of the respective
// hash table, on the assumption that three elements is the longest linear
// search we want to do when looking up names in the hash table.
-NameserverAddressStore::NameserverAddressStore(ResolverInterface& resolver,
- uint32_t zonehashsize, uint32_t nshashsize) :
- zone_hash_(new NsasEntryCompare<ZoneEntry>, zonehashsize),
- nameserver_hash_(new NsasEntryCompare<NameserverEntry>, nshashsize),
- zone_lru_((3 * zonehashsize), new HashDeleter<ZoneEntry>(zone_hash_)),
- nameserver_lru_((3 * nshashsize), new HashDeleter<NameserverEntry>(
- nameserver_hash_)),
- resolver_(resolver),
- callback_(*this)
-{
-}
+NameserverAddressStore::NameserverAddressStore(
+ shared_ptr<ResolverInterface> resolver, uint32_t zonehashsize,
+ uint32_t nshashsize) :
+ zone_hash_(new HashTable<ZoneEntry>(new NsasEntryCompare<ZoneEntry>,
+ zonehashsize)),
+ nameserver_hash_(new HashTable<NameserverEntry>(
+ new NsasEntryCompare<NameserverEntry>, nshashsize)),
+ zone_lru_(new LruList<ZoneEntry>((3 * zonehashsize),
+ new HashDeleter<ZoneEntry>(*zone_hash_))),
+ nameserver_lru_(new LruList<NameserverEntry>((3 * nshashsize),
+ new HashDeleter<NameserverEntry>(*nameserver_hash_))),
+ resolver_(resolver)
+{ }
namespace {
-// Often used types
-typedef shared_ptr<ZoneEntry> ZonePtr;
-typedef shared_ptr<NameserverEntry> NameserverPtr;
-typedef shared_ptr<AddressRequestCallback> CallbackPtr;
-
/*
- * Create a zone entry.
- * It is called inside the mutex so it is called and filled in attomically.
- * Pointers are used instead of references, because with references,
- * boost::bind copyes.
+ * We use pointers here so there's no call to any copy constructor.
+ * It is easier for the compiler to inline it and prove that there's
+ * no need to copy anything. In that case, the bind should not be
+ * called at all to create the object, just call the function.
*/
-ZonePtr
-newZone(const std::string* zone, uint16_t class_code,
- const AbstractRRset* authority, const vector<AbstractRRset>* additional,
- HashTable<NameserverEntry>* ns_hash, LruList<NameserverEntry>* ns_lru)
+shared_ptr<ZoneEntry>
+newZone(const shared_ptr<ResolverInterface>* resolver, const string* zone,
+ const RRClass* class_code,
+ const shared_ptr<HashTable<NameserverEntry> >* ns_hash,
+ const shared_ptr<LruList<NameserverEntry> >* ns_lru)
{
- ZonePtr zone_ptr(new ZoneEntry(*zone, class_code));
- // Sanitize the authority section and put the data there
- if (authority->getClass().getCode() != class_code) {
- isc_throw(InconsistentZone,
- "Authority section is for different class, expected: " <<
- RRClass(class_code).toText() << ", got: " <<
- authority->getClass().toText());
- }
- if (authority->getName() != Name(*zone)) {
- isc_throw(InconsistentZone,
- "Authority section is for different zone, expected: " <<
- *zone << ", got: " << authority->getName().toText());
- }
- if (authority->getType() != RRType::NS()) {
- isc_throw(NotNS, "Authority section with non-NS RR type: " <<
- authority->getType().toText());
- }
- // Make sure the name servers exist
- RdataIteratorPtr ns(authority->getRdataIterator());
- // TODO Remove the call to first on merge with #410
- for (ns->first(); !ns->isLast(); ns->next()) {
- Name ns_name(dynamic_cast<const rdata::generic::NS&>(
- ns->getCurrent()).getNSName());
- string ns_name_str(ns_name.toText());
- pair<bool, NameserverPtr> ns_lookup(
- ns_hash->getOrAdd(HashKey(ns_name_str, class_code),
- bind(newNs, &ns_name_str, class_code, additional)));
- if (ns_lookup.first) { // Is it a new nameserver?
- ns_lru->add(ns_lookup.second);
- } else {
- ns_lru->touch(ns_lookup.second);
- }
- zone_ptr->nameserverAdd(ns_lookup.second);
- }
-
- return zone_ptr;
+ shared_ptr<ZoneEntry> result(new ZoneEntry(*resolver, *zone, *class_code,
+ *ns_hash, *ns_lru));
+ return (result);
}
}
void
-NameserverAddressStore::lookup(const std::string& zone, uint16_t class_code,
- const AbstractRRset& authority, const vector<AbstractRRset>& additional,
- CallbackPtr callback)
+NameserverAddressStore::lookup(const string& zone, const RRClass& class_code,
+ shared_ptr<AddressRequestCallback> callback, AddressFamily family)
{
- // Try to look up the entry, or create and fill it with initial data
- pair<bool, ZonePtr> zone_lookup(
- zone_hash_.getOrAdd(HashKey(zone, class_code),
- bind(newZone, &zone, class_code, &authority, &additional,
+ pair<bool, shared_ptr<ZoneEntry> > zone_obj(zone_hash_->getOrAdd(HashKey(
+ zone, class_code), boost::bind(newZone, &resolver_, &zone, &class_code,
&nameserver_hash_, &nameserver_lru_)));
- ZonePtr zone_ptr(zone_lookup.second);
- if (zone_lookup.first) { // New value
- zone_lru_.add(zone_ptr);
- } else { // Was already here
- // FIXME Check the TTL, delete and retry if outdated
- zone_lru_.touch(zone_ptr);
+ if (zone_obj.first) {
+ zone_lru_->add(zone_obj.second);
+ } else {
+ zone_lru_->touch(zone_obj.second);
}
- zone_ptr->addCallback(callback);
- // TODO Do this only when there are no callbacks currently
- // if there are, it means this will be just added as well
- processZone(zone_ptr);
-}
-
-namespace {
-}
-
-// TODO Pass a nameserver that is responsible for this, as it is not
-// checked for TTL (might be 0)
-// TODO Move this to the zone
-void NameserverAddressStore::processZone(ZonePtr zone) {
- // Addresses of existing nameservers
- NameserverEntry::AddressVector addresses;
- // Current state
- Fetchable::State state;
- {
- // Nameservers we can still ask for their IP address
- vector<NameserverPtr> not_asked;
- // Are there any in-progress nameservers?
- bool pending(false);
- /**
- * This is inside a block so we can unlock the zone with nameservers
- * after we get all information from it. We do not need to keep the
- * lock on the nameservers when we run callbacks.
- */
- ZoneEntry::Lock lock(zone->getLock());
-
- if (zone->getState() != Fetchable::UNREACHABLE) {
- // Look into all the nameservers
-
- // FIXME This completely ignores TTLs, something shoud be done with them.
- // Maybe turn all outdated nameservers into NOT_ASKED?
-
- BOOST_FOREACH(NameserverPtr ns, *zone) {
- switch (ns->getState()) {
- case Fetchable::NOT_ASKED:
- not_asked.push_back(ns);
- break;
- case Fetchable::READY:
- ns->getAddresses(addresses);
- break;
- case Fetchable::IN_PROGRESS:
- pending = true;
- ns->ensureHasCallback(zone, callback_);
- break;
- case Fetchable::UNREACHABLE:
- // Nothing. We do not care about it.
- break;
- }
- }
- }
-
- /*
- * If there is no data, noone to ask and noone to expect answer from
- * then bad luck, but this zone can't be reached.
- */
- if (not_asked.empty() && addresses.empty() && !pending) {
- zone->setState(Fetchable::UNREACHABLE);
- }
-
- // Pick up to two nameservers and try to resolve them
- // TODO Any better way to choose one?
- for (int i(0); i < 2 && !not_asked.empty(); ++ i) {
- size_t index(randIndex(not_asked.size()));
- not_asked[index]->askIP(resolver_, zone, callback_,
- not_asked[index]);
- // Remove from the vector so we do not choose it again
- not_asked.erase(not_asked.begin() + index);
- }
-
- state = zone->getState();
- } // Release the lock (and some other resources as a bonus)
-
- // If we are unreachable, tell everyone
- if (state == Fetchable::UNREACHABLE) {
- while (zone->hasCallbacks()) {
- zone->popCallback()->unreachable();
- }
- } else if (!addresses.empty()) { // Give everyone an address
- while (zone->hasCallbacks()) {
- // Get the address first, so the callback is removed after we are
- // sure there's no exception
- asiolink::IOAddress address(chooseAddress(addresses));
- zone->popCallback()->success(address);
- }
- }
- // Otherwise, we are still waiting for more info to come, let the
- // callbacks rest
+ zone_obj.second->addCallback(callback, family);
}
} // namespace nsas
Modified: branches/trac408/src/lib/nsas/nameserver_address_store.h
==============================================================================
--- branches/trac408/src/lib/nsas/nameserver_address_store.h (original)
+++ branches/trac408/src/lib/nsas/nameserver_address_store.h Sat Dec 4 13:50:07 2010
@@ -20,44 +20,25 @@
#include <string>
#include <vector>
-#include <dns/rrset.h>
+#include <boost/shared_ptr.hpp>
-#include "address_request_callback.h"
-#include "hash_table.h"
-#include "nameserver_entry.h"
-#include "lru_list.h"
-#include "zone_entry.h"
-#include "resolver_interface.h"
#include "nsas_types.h"
namespace isc {
+// Some forward declarations, so we do not need to include so many headers
+
+namespace dns {
+class RRClass;
+}
+
namespace nsas {
-/**
- * \short Invalid referral information passed.
- *
- * This is thrown if the referral passed to NameserverAddressStore::lookup is
- * wrong. Has subexceptions for specific conditions.
- */
-struct InvalidReferral : public isc::BadValue {
- InvalidReferral(const char *file, size_t line, const char *what) :
- BadValue(file, line, what)
- { }
-};
-
-/// \short The referral is not for this zone.
-struct InconsistentZone : public InvalidReferral {
- InconsistentZone(const char *file, size_t line, const char *what) :
- InvalidReferral(file, line, what)
- { }
-};
-
-/// \short The authority zone contains something else than NS
-struct NotNS : public InvalidReferral {
- NotNS(const char *file, size_t line, const char *what) :
- InvalidReferral(file, line, what)
- { }
-};
+class ResolverInterface;
+template<class T> class HashTable;
+template<class T> class LruList;
+class ZoneEntry;
+class NameserverEntry;
+class AddressRequestCallback;
/// \brief Nameserver Address Store
///
@@ -80,7 +61,7 @@
/// \param zonehashsize Size of the zone hash table. The default value of
/// 1009 is the first prime number above 1000.
/// \param nshash size Size of the nameserver hash table. The default
- /// value of 2003 is the first prime number over 2000, and by implication,
+ /// value of 3001 is the first prime number over 3000, and by implication,
/// there is an assumption that there will be more nameservers than zones
/// in the store.
NameserverAddressStore(boost::shared_ptr<ResolverInterface> resolver,
@@ -92,22 +73,18 @@
virtual ~NameserverAddressStore()
{}
- // TODO Drop zone and class code, they can be found out from the authority
/// \brief Lookup Address for a Zone
///
/// Looks up the address of a nameserver in the zone.
///
/// \param zone Name of zone for which an address is required.
/// \param class_code Class of the zone.
- /// \param authority Authority RRset from the referral containing the
- /// nameservers that serve the zone.
/// \param callback Callback object used to pass the result back to the
/// caller.
- /// \param request Which address is requested.
- void lookup(const std::string& zone, uint16_t class_code,
- const isc::dns::AbstractRRset& authority,
- boost::shared_ptr<AddressRequestCallback> callback, AddressRequest
- request = ANY_OK);
+ /// \param family Which address is requested.
+ void lookup(const std::string& zone, const dns::RRClass& class_code,
+ boost::shared_ptr<AddressRequestCallback> callback, AddressFamily
+ family = ANY_OK);
/// \brief Protected Members
///
@@ -122,15 +99,16 @@
//@{
protected:
// Zone and nameserver hash tables
- HashTable<ZoneEntry> zone_hash_;
- HashTable<NameserverEntry> nameserver_hash_;
+ boost::shared_ptr<HashTable<ZoneEntry> > zone_hash_;
+ boost::shared_ptr<HashTable<NameserverEntry> > nameserver_hash_;
// ... and the LRU lists
- LruList<ZoneEntry> zone_lru_;
- LruList<NameserverEntry> nameserver_lru_;
- //}@
+ boost::shared_ptr<LruList<ZoneEntry> > zone_lru_;
+ boost::shared_ptr<LruList<NameserverEntry> > nameserver_lru_;
+ // The resolver we use
private:
boost::shared_ptr<ResolverInterface> resolver_;
+ //}@
};
} // namespace nsas
Modified: branches/trac408/src/lib/nsas/tests/Makefile.am
==============================================================================
--- branches/trac408/src/lib/nsas/tests/Makefile.am (original)
+++ branches/trac408/src/lib/nsas/tests/Makefile.am Sat Dec 4 13:50:07 2010
@@ -24,7 +24,7 @@
run_unittests_SOURCES += hash_unittest.cc
run_unittests_SOURCES += lru_list_unittest.cc
run_unittests_SOURCES += nameserver_address_unittest.cc
-#run_unittests_SOURCES += nameserver_address_store_unittest.cc
+run_unittests_SOURCES += nameserver_address_store_unittest.cc
run_unittests_SOURCES += nameserver_entry_unittest.cc
run_unittests_SOURCES += nsas_entry_compare_unittest.cc
run_unittests_SOURCES += nsas_test.h
Modified: branches/trac408/src/lib/nsas/tests/nameserver_address_store_unittest.cc
==============================================================================
--- branches/trac408/src/lib/nsas/tests/nameserver_address_store_unittest.cc (original)
+++ branches/trac408/src/lib/nsas/tests/nameserver_address_store_unittest.cc Sat Dec 4 13:50:07 2010
@@ -22,6 +22,7 @@
#include <dns/rrttl.h>
#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
#include <gtest/gtest.h>
#include <boost/shared_ptr.hpp>
@@ -35,6 +36,7 @@
#include "../nsas_entry_compare.h"
#include "../nameserver_entry.h"
#include "../zone_entry.h"
+#include "../address_request_callback.h"
#include "nsas_test.h"
using namespace isc::dns;
@@ -55,9 +57,10 @@
///
/// \param hashsize Size of the zone hash table
/// \param lrusize Size of the zone hash table
- DerivedNsas(ResolverInterface& resolver, uint32_t hashsize,
+ DerivedNsas(shared_ptr<TestResolver> resolver, uint32_t hashsize,
uint32_t lrusize) :
- NameserverAddressStore(resolver, hashsize, lrusize)
+ NameserverAddressStore(resolver, hashsize, lrusize),
+ resolver_(resolver)
{}
/// \brief Virtual Destructor
@@ -67,16 +70,39 @@
/// \brief Add Nameserver Entry to Hash and LRU Tables
void AddNameserverEntry(boost::shared_ptr<NameserverEntry>& entry) {
HashKey h = entry->hashKey();
- nameserver_hash_.add(entry, h);
- nameserver_lru_.add(entry);
+ nameserver_hash_->add(entry, h);
+ nameserver_lru_->add(entry);
}
/// \brief Add Zone Entry to Hash and LRU Tables
void AddZoneEntry(boost::shared_ptr<ZoneEntry>& entry) {
HashKey h = entry->hashKey();
- zone_hash_.add(entry, h);
- zone_lru_.add(entry);
- }
+ zone_hash_->add(entry, h);
+ zone_lru_->add(entry);
+ }
+ /**
+ * \short Just wraps the common lookup
+ *
+ * It calls the lookup and provides the authority section
+ * if it is asked for by the resolver.
+ */
+ void lookupAndAnswer(const string& name, const RRClass& class_code,
+ shared_ptr<AbstractRRset> authority,
+ shared_ptr<AddressRequestCallback> callback)
+ {
+ size_t size(resolver_->requests.size());
+ NameserverAddressStore::lookup(name, class_code, callback, ANY_OK);
+ // It asked something, the only thing it can ask is the NS list
+ if (size < resolver_->requests.size()) {
+ resolver_->provideNS(size, authority);
+ // Once answered, drop the request so noone else sees it
+ resolver_->requests.erase(resolver_->requests.begin() + size);
+ } else {
+ ADD_FAILURE() << "Not asked for NS";
+ }
+ }
+private:
+ shared_ptr<TestResolver> resolver_;
};
@@ -89,22 +115,31 @@
authority_(new RRset(Name("example.net."), RRClass::IN(), RRType::NS(),
RRTTL(128))),
empty_authority_(new RRset(Name("example.net."), RRClass::IN(),
- RRType::NS(), RRTTL(128)))
+ RRType::NS(), RRTTL(128))),
+ resolver_(new TestResolver)
{
- // Constructor - initialize a set of nameserver and zone objects. For convenience,
- // these are stored in vectors.
+ // Constructor - initialize a set of nameserver and zone objects. For
+ // convenience, these are stored in vectors.
for (int i = 1; i <= 9; ++i) {
- std::string name = "nameserver" + boost::lexical_cast<std::string>(i);
- nameservers_.push_back(boost::shared_ptr<NameserverEntry>(new NameserverEntry(name, (40 + i))));
+ std::string name = "nameserver" + boost::lexical_cast<std::string>(
+ i);
+ nameservers_.push_back(boost::shared_ptr<NameserverEntry>(
+ new NameserverEntry(name, RRClass(40 + i))));
}
+ // Some zones. They will not use the tables in this test, so it can be
+ // empty
for (int i = 1; i <= 9; ++i) {
std::string name = "zone" + boost::lexical_cast<std::string>(i);
- zones_.push_back(boost::shared_ptr<ZoneEntry>(new ZoneEntry(name, (40 + i))));
+ zones_.push_back(boost::shared_ptr<ZoneEntry>(new ZoneEntry(
+ resolver_, name, RRClass(40 + i),
+ shared_ptr<HashTable<NameserverEntry> >(),
+ shared_ptr<LruList<NameserverEntry> >())));
}
// A nameserver serving data
- authority_->addRdata(ConstRdataPtr(new rdata::generic::NS(Name("ns.example.com."))));
+ authority_->addRdata(ConstRdataPtr(new rdata::generic::NS(Name(
+ "ns.example.com."))));
// This is reused because of convenience, clear it just in case
NSASCallback::results.clear();
@@ -116,7 +151,7 @@
RRsetPtr authority_, empty_authority_;
- TestResolver defaultTestResolver;
+ shared_ptr<TestResolver> resolver_;
class NSASCallback : public AddressRequestCallback {
public:
@@ -148,7 +183,7 @@
// Create a NSAS with a hash size of three and a LRU size of 9 (both zone and
// nameserver tables).
- DerivedNsas nsas(defaultTestResolver, 2, 2);
+ DerivedNsas nsas(resolver_, 2, 2);
// Add six entries to the tables. After addition the reference count of each element
// should be 3 - one for the entry in the zones_ vector, and one each for the entries
@@ -178,7 +213,7 @@
// Create a NSAS with a hash size of three and a LRU size of 9 (both zone and
// nameserver tables).
- DerivedNsas nsas(defaultTestResolver, 2, 2);
+ DerivedNsas nsas(resolver_, 2, 2);
// Add six entries to the tables. After addition the reference count of each element
// should be 3 - one for the entry in the nameservers_ vector, and one each for the entries
@@ -205,37 +240,29 @@
* Check if it asks correct questions and it keeps correct internal state.
*/
TEST_F(NameserverAddressStoreTest, emptyLookup) {
- DerivedNsas nsas(defaultTestResolver, 10, 10);
+ DerivedNsas nsas(resolver_, 10, 10);
// Ask it a question
- nsas.lookup("example.net.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
- // It should ask for IP addresses for example.com.
- ASSERT_EQ(2, defaultTestResolver.requests.size());
- defaultTestResolver.asksIPs(Name("ns.example.com."), 0, 1);
+ nsas.lookupAndAnswer("example.net.", RRClass::IN(), authority_,
+ getCallback());
+ // It should ask for IP addresses for ns.example.com.
+ resolver_->asksIPs(Name("ns.example.com."), 0, 1);
// Ask another question for the same zone
- nsas.lookup("example.net.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookup("example.net.", RRClass::IN(), getCallback());
// It should ask no more questions now
- EXPECT_EQ(2, defaultTestResolver.requests.size());
+ EXPECT_EQ(2, resolver_->requests.size());
// Ask another question with different zone but the same nameserver
authority_->setName(Name("example.com."));
- nsas.lookup("example.com.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookupAndAnswer("example.com.", RRClass::IN(), authority_,
+ getCallback());
// It still should ask nothing
- EXPECT_EQ(2, defaultTestResolver.requests.size());
+ EXPECT_EQ(2, resolver_->requests.size());
// We provide IP address of one nameserver, it should generate all the
// results
- RRsetPtr answer(new RRset(Name("example.com."), RRClass::IN(), RRType::A(),
- RRTTL(100)));
- answer->addRdata(rdata::in::A("192.0.2.1"));
- Message address(Message::RENDER); // Not able to create different one
- address.addRRset(Section::ANSWER(), answer);
- address.addRRset(Section::AUTHORITY(), authority_);
- address.addQuestion(defaultTestResolver[0]);
- defaultTestResolver.requests[0].second->success(address);
+ resolver_->answer(0, Name("ns.example.com."), RRType::A(),
+ rdata::in::A("192.0.2.1"));
EXPECT_EQ(3, NSASCallback::results.size());
BOOST_FOREACH(const NSASCallback::Result& result, NSASCallback::results) {
EXPECT_TRUE(result.first);
@@ -249,12 +276,12 @@
* It should not ask anything and say it is unreachable right away.
*/
TEST_F(NameserverAddressStoreTest, zoneWithoutNameservers) {
- DerivedNsas nsas(defaultTestResolver, 10, 10);
+ DerivedNsas nsas(resolver_, 10, 10);
// Ask it a question
- nsas.lookup("example.net.", RRClass::IN().getCode(), *empty_authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookupAndAnswer("example.net.", RRClass::IN(), empty_authority_,
+ getCallback());
// There should be no questions, because there's nothing to ask
- EXPECT_EQ(0, defaultTestResolver.requests.size());
+ EXPECT_EQ(0, resolver_->requests.size());
// And there should be one „unreachable“ answer for the query
ASSERT_EQ(1, NSASCallback::results.size());
EXPECT_FALSE(NSASCallback::results[0].first);
@@ -268,35 +295,33 @@
* without further asking.
*/
TEST_F(NameserverAddressStoreTest, unreachableNS) {
- DerivedNsas nsas(defaultTestResolver, 10, 10);
+ DerivedNsas nsas(resolver_, 10, 10);
// Ask it a question
- nsas.lookup("example.net.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookupAndAnswer("example.net.", RRClass::IN(), authority_,
+ getCallback());
// It should ask for IP addresses for example.com.
- ASSERT_EQ(2, defaultTestResolver.requests.size());
- defaultTestResolver.asksIPs(Name("ns.example.com."), 0, 1);
+ ASSERT_EQ(2, resolver_->requests.size());
+ resolver_->asksIPs(Name("ns.example.com."), 0, 1);
// Ask another question with different zone but the same nameserver
authority_->setName(Name("example.com."));
- nsas.lookup("example.com.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookupAndAnswer("example.com.", RRClass::IN(), authority_,
+ getCallback());
// It should ask nothing more now
- EXPECT_EQ(2, defaultTestResolver.requests.size());
+ EXPECT_EQ(2, resolver_->requests.size());
// We say there are no addresses
- defaultTestResolver.requests[0].second->failure();
- defaultTestResolver.requests[1].second->failure();
+ resolver_->requests[0].second->failure();
+ resolver_->requests[1].second->failure();
// We should have 2 answers now
EXPECT_EQ(2, NSASCallback::results.size());
// When we ask one same and one other zone with the same nameserver,
// it should generate no questions and answer right away
- authority_->setName(Name("example.net."));
- nsas.lookup("example.net.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookup("example.net.", RRClass::IN(), getCallback());
authority_->setName(Name("example.org."));
- nsas.lookup("example.org.", RRClass::IN().getCode(), *authority_,
- vector<AbstractRRset>(), getCallback());
+ nsas.lookupAndAnswer("example.org.", RRClass::IN(), authority_,
+ getCallback());
// There should be 4 negative answers now
EXPECT_EQ(4, NSASCallback::results.size());
BOOST_FOREACH(const NSASCallback::Result& result, NSASCallback::results) {
@@ -304,27 +329,11 @@
}
}
-/// \short Test invalid authority section.
-TEST_F(NameserverAddressStoreTest, invalidAuthority) {
- DerivedNsas nsas(defaultTestResolver, 2, 2);
- EXPECT_THROW(nsas.lookup("example.net.", RRClass::CH().getCode(),
- *authority_, vector<AbstractRRset>(), getCallback()),
- InconsistentZone);
- EXPECT_EQ(0, defaultTestResolver.requests.size());
- EXPECT_EQ(0, NSASCallback::results.size());
- EXPECT_THROW(nsas.lookup("example.com.", RRClass::IN().getCode(),
- *authority_, vector<AbstractRRset>(), getCallback()),
- InconsistentZone);
- EXPECT_EQ(0, defaultTestResolver.requests.size());
- EXPECT_EQ(0, NSASCallback::results.size());
- BasicRRset aAuthority(Name("example.net."), RRClass::IN(), RRType::A(),
- RRTTL(128));
- EXPECT_THROW(nsas.lookup("example.net.", RRClass::IN().getCode(),
- aAuthority, vector<AbstractRRset>(),
- getCallback()), NotNS);
- EXPECT_EQ(0, defaultTestResolver.requests.size());
- EXPECT_EQ(0, NSASCallback::results.size());
-}
+/*
+ * TODO: More tests. Some eviction combined with lookups would make sense.
+ * Stressing the entries that some nameservers for zone are there and some
+ * are not, etc.
+ */
} // namespace nsas
} // namespace isc
Modified: branches/trac408/src/lib/nsas/tests/nsas_test.h
==============================================================================
--- branches/trac408/src/lib/nsas/tests/nsas_test.h (original)
+++ branches/trac408/src/lib/nsas/tests/nsas_test.h Sat Dec 4 13:50:07 2010
@@ -289,7 +289,9 @@
requests[index].second->success(set);
}
- void provideNS(size_t index, RRsetPtr nameservers) {
+ void provideNS(size_t index,
+ boost::shared_ptr<AbstractRRset> nameservers)
+ {
if (index >= requests.size()) {
throw NoSuchRequest();
}
Modified: branches/trac408/src/lib/nsas/tests/zone_entry_unittest.cc
==============================================================================
--- branches/trac408/src/lib/nsas/tests/zone_entry_unittest.cc (original)
+++ branches/trac408/src/lib/nsas/tests/zone_entry_unittest.cc Sat Dec 4 13:50:07 2010
@@ -279,6 +279,8 @@
* (provide it with some kind of cache-like thing, preconfigure answer, so
* the things start recursing).
* - Combine this with some timeouting.
+ * - Look what happens when the nameservers are already in some different
+ * states and not just newly created.
*/
} // namespace
Modified: branches/trac408/src/lib/nsas/zone_entry.cc
==============================================================================
--- branches/trac408/src/lib/nsas/zone_entry.cc (original)
+++ branches/trac408/src/lib/nsas/zone_entry.cc Sat Dec 4 13:50:07 2010
@@ -79,6 +79,8 @@
} else {
// Store the current ones so we can keep them
map<string, NameserverPtr> old;
+ set<NameserverPtr> old_not_asked;
+ old_not_asked.swap(entry_->nameservers_not_asked_);
BOOST_FOREACH(const NameserverPtr& ptr, entry_->nameservers_) {
old[ptr->getName()] = ptr;
}
@@ -102,16 +104,29 @@
entry_->nameserver_table_->getOrAdd(HashKey(
ns_name_str, entry_->class_code_), bind(
newNs, &ns_name_str, &entry_->class_code_)));
- // Touch it if it is not newly created
- if (!from_hash.first) {
+ // Make it at the front of the list
+ if (from_hash.first) {
+ entry_->nameserver_lru_->add(from_hash.second);
+ } else {
entry_->nameserver_lru_->touch(
from_hash.second);
}
// And add it at last
entry_->nameservers_.push_back(from_hash.second);
+ entry_->nameservers_not_asked_.insert(
+ from_hash.second);
} else {
// We have it, so just use it
entry_->nameservers_.push_back(old_ns->second);
+ // Did we ask it already? If not, it is still not
+ // asked (the one designing std interface must
+ // have been mad)
+ if (old_not_asked.find(old_ns->second) !=
+ old_not_asked.end())
+ {
+ entry_->nameservers_not_asked_.insert(
+ old_ns->second);
+ }
}
}
// OK, we skip this one it is not NS (log?)
@@ -330,6 +345,13 @@
switch (ns_state) {
case IN_PROGRESS:
pending = true;
+ // Someone asked it, but not us, we don't have
+ // callback
+ if (nameservers_not_asked_.find(ns) !=
+ nameservers_not_asked_.end())
+ {
+ to_ask.push_back(ns);
+ }
break;
case NOT_ASKED:
case EXPIRED:
@@ -344,16 +366,11 @@
// We have someone to ask, so do it
if (!to_ask.empty()) {
+ // We ask everything that makes sense now
+ nameservers_not_asked_.clear();
// We should not be locked, because this function can
// be called directly from the askIP again
lock->unlock();
- shared_ptr<NameserverCallback> ns_callbacks[ADDR_REQ_MAX];;
- ns_callbacks[ANY_OK].reset(new NameserverCallback(
- shared_from_this(), ANY_OK));
- ns_callbacks[V4_ONLY].reset(new NameserverCallback(
- shared_from_this(), V4_ONLY));
- ns_callbacks[V6_ONLY].reset(new NameserverCallback(
- shared_from_this(), V6_ONLY));
/*
* TODO: Possible place for an optimisation. We now ask
* everything we can. We should limit this to something like
@@ -367,9 +384,7 @@
// callback for different one.
// If they recurse back to us (call directly), we kill
// it by the in_process_
- ns->askIP(resolver_, ns_callbacks[V4_ONLY], V4_ONLY);
- ns->askIP(resolver_, ns_callbacks[V6_ONLY], V6_ONLY);
- ns->askIP(resolver_, ns_callbacks[ANY_OK], ANY_OK);
+ insertCallback(ns, ADDR_REQ_MAX);
}
// Retry with all the data that might have arrived
in_process_[family] = false;
@@ -401,5 +416,18 @@
}
}
+void
+ZoneEntry::insertCallback(NameserverPtr ns, AddressFamily family) {
+ if (family == ADDR_REQ_MAX) {
+ insertCallback(ns, ANY_OK);
+ insertCallback(ns, V4_ONLY);
+ insertCallback(ns, V6_ONLY);
+ } else {
+ shared_ptr<NameserverCallback> callback(new NameserverCallback(
+ shared_from_this(), family));
+ ns->askIP(resolver_, callback, family);
+ }
+}
+
}; // namespace nsas
}; // namespace isc
Modified: branches/trac408/src/lib/nsas/zone_entry.h
==============================================================================
--- branches/trac408/src/lib/nsas/zone_entry.h (original)
+++ branches/trac408/src/lib/nsas/zone_entry.h Sat Dec 4 13:50:07 2010
@@ -19,6 +19,7 @@
#include <string>
#include <vector>
+#include <set>
#include <boost/thread.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/enable_shared_from_this.hpp>
@@ -105,6 +106,8 @@
typedef boost::shared_ptr<NameserverEntry> NameserverPtr;
typedef std::vector<NameserverPtr> NameserverVector;
NameserverVector nameservers_; ///< Nameservers
+ // Which nameservers didn't have any of our callbacks yet
+ std::set<NameserverPtr> nameservers_not_asked_;
/*
* Callbacks. For each fimily type one vector, so we can process
* them separately.
@@ -153,6 +156,9 @@
// The lock is mandatory
void dispatchFailures(AddressFamily family,
boost::shared_ptr<boost::mutex::scoped_lock> lock);
+ // Put a callback into the nameserver entry. Same ADDR_REQ_MAX means for
+ // all families
+ void insertCallback(NameserverPtr nameserver, AddressFamily family);
};
} // namespace nsas
More information about the bind10-changes
mailing list