[svn] commit: r3804 - in /branches/trac408/src/lib/nsas: Makefile.am nameserver_address.cc nameserver_address.h nameserver_entry.cc nameserver_entry.h random_number_generator.h tests/nameserver_address_unittest.cc tests/nameserver_entry_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Dec 11 20:26:47 UTC 2010
Author: vorner
Date: Sat Dec 11 20:26:31 2010
New Revision: 3804
Log:
NameserverEntry provides NameserverAddress
And the NameserverAddress is made safe with regard to changing indices
inside the NameserverEntry.
Added:
branches/trac408/src/lib/nsas/nameserver_address.cc
Modified:
branches/trac408/src/lib/nsas/Makefile.am
branches/trac408/src/lib/nsas/nameserver_address.h
branches/trac408/src/lib/nsas/nameserver_entry.cc
branches/trac408/src/lib/nsas/nameserver_entry.h
branches/trac408/src/lib/nsas/random_number_generator.h
branches/trac408/src/lib/nsas/tests/nameserver_address_unittest.cc
branches/trac408/src/lib/nsas/tests/nameserver_entry_unittest.cc
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 11 20:26:31 2010
@@ -15,7 +15,7 @@
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.h
+libnsas_la_SOURCES += nameserver_address.h nameserver_address.cc
libnsas_la_SOURCES += nameserver_entry.cc nameserver_entry.h
libnsas_la_SOURCES += nsas_entry_compare.h
libnsas_la_SOURCES += nsas_entry.h nsas_types.h
Modified: branches/trac408/src/lib/nsas/nameserver_address.h
==============================================================================
--- branches/trac408/src/lib/nsas/nameserver_address.h (original)
+++ branches/trac408/src/lib/nsas/nameserver_address.h Sat Dec 11 20:26:31 2010
@@ -19,11 +19,17 @@
#include <boost/shared_ptr.hpp>
+#include <exceptions/exceptions.h>
+
#include "asiolink.h"
-#include "nameserver_entry.h"
+#include "address_entry.h"
+#include "nsas_types.h"
namespace isc {
namespace nsas {
+
+class ZoneEntry;
+class NameserverEntry;
/// \brief Empty \c NameserverEntry pointer exception
///
@@ -40,17 +46,20 @@
/// \brief Nameserver Address
///
/// This class implements the object that returned from NSAS when the resolver
-/// request an address for the name server. It contains one IOAddress object
+/// request an address for the name server. It contains one address
/// that can be used by resolver. When the resolver get query back from the name
-/// server, it should update the name server's RTT(Round Trip Time) with this
+/// server, it should update the name server's RTT(Round Trip Time) with this
/// object.
+///
+/// It is not thread safe, only reentrant. It is expected to be kept inside
+/// the resolver and used only once for the address and once for the update.
class NameserverAddress {
public:
/// \brief Constructor
///
/// The NameserverAddress object will contain one shared_ptr object that
- /// pointed to NameserverEntry which contains the address as well as it's
+ /// pointed to NameserverEntry which contains the address as well as it's
/// corresponding index. The user can update it's RTT with the index later.
///
/// \param namerserver A shared_ptr that points to a NameserverEntry object
@@ -59,32 +68,21 @@
/// \param index The address's index in NameserverEntry's addresses vector
/// \param family Address family, V4_ONLY or V6_ONLY
NameserverAddress(const boost::shared_ptr<NameserverEntry>& nameserver,
- size_t index, AddressFamily family):
- ns_(nameserver), index_(index), family_(family)
+ const AddressEntry& address, AddressFamily family):
+ ns_(nameserver), address_(address), family_(family)
{
- if(!ns_.get()) {
+ if(!ns_) {
isc_throw(NullNameserverEntryPointer, "NULL NameserverEntry pointer.");
}
}
/// \brief Default Constructor
- ///
- /// \todo Is it needed? This one seems to make no sense.
- NameserverAddress(): index_(0), family_(V4_ONLY)
- {
- }
-
- /// \brief Destructor
- ///
- /// Empty destructor.
- ~NameserverAddress()
- {
- }
+ NameserverAddress() : address_(asiolink::IOAddress("::1")) { }
/// \brief Return address
///
asiolink::IOAddress getAddress() const {
- return ns_.get()->getAddressAtIndex(index_, family_);
+ return (address_.getAddress());
}
/// \brief Update Round-trip Time
@@ -92,13 +90,26 @@
/// When the user get one request back from the name server, it should
/// update the address's RTT.
/// \param rtt The new Round-Trip Time
- void updateRTT(uint32_t rtt) {
- ns_.get()->updateAddressRTTAtIndex(rtt, index_, family_);
+ void updateRTT(uint32_t rtt) const;
+
+ /// Short access to the AddressEntry inside.
+ //@{
+ const AddressEntry& getAddressEntry() const {
+ return (address_);
}
+ AddressEntry& getAddressEntry() {
+ return (address_);
+ }
+ //@}
private:
+ /*
+ * Note: Previous implementation used index into the entry. That is wrong,
+ * as the list of addresses may change. Thil would cause setting a
+ * different address or a crash.
+ */
boost::shared_ptr<NameserverEntry> ns_; ///< Shared-pointer to NameserverEntry object
- size_t index_; ///< The address index in NameserverEntry
+ AddressEntry address_; ///< The address
AddressFamily family_; ///< The address family (V4_ONLY or V6_ONLY)
};
Modified: branches/trac408/src/lib/nsas/nameserver_entry.cc
==============================================================================
--- branches/trac408/src/lib/nsas/nameserver_entry.cc (original)
+++ branches/trac408/src/lib/nsas/nameserver_entry.cc Sat Dec 11 20:26:31 2010
@@ -49,7 +49,7 @@
namespace {
// Just shorter type alias
-typedef mutex::scoped_lock Lock;
+typedef recursive_mutex::scoped_lock Lock;
}
@@ -100,15 +100,19 @@
return (getState());
}
+ shared_ptr<NameserverEntry> self(shared_from_this());
// If any address is OK, just pass everything we have
if (family == ANY_OK) {
- addresses.insert(addresses.end(), addresses_[V4_ONLY].begin(),
- addresses_[V4_ONLY].end());
- addresses.insert(addresses.end(), addresses_[V6_ONLY].begin(),
- addresses_[V6_ONLY].end());
+ BOOST_FOREACH(const AddressEntry& entry, addresses_[V6_ONLY]) {
+ addresses.push_back(NameserverAddress(self, entry, V6_ONLY));
+ }
+ BOOST_FOREACH(const AddressEntry& entry, addresses_[V4_ONLY]) {
+ addresses.push_back(NameserverAddress(self, entry, V4_ONLY));
+ }
} else {
- addresses.insert(addresses.end(), addresses_[family].begin(),
- addresses_[family].end());
+ BOOST_FOREACH(const AddressEntry& entry, addresses_[family]) {
+ addresses.push_back(NameserverAddress(self, entry, family));
+ }
}
if (getState() == EXPIRED && expired_ok) {
return READY;
@@ -125,7 +129,7 @@
return (false);
} else {
address = NameserverAddress(shared_from_this(),
- address_selectors_[family](), family);
+ addresses_[family][address_selectors_[family]()], family);
return (true);
}
}
@@ -186,6 +190,19 @@
addresses_[family][index].setRTT(new_rtt);
updateAddressSelector(addresses_[family], address_selectors_[family]);
+}
+
+void
+NameserverEntry::updateAddressRTT(uint32_t rtt,
+ const asiolink::IOAddress& address, AddressFamily family)
+{
+ Lock lock(mutex_);
+ for (size_t i(0); i < addresses_[family].size(); ++ i) {
+ if (addresses_[family][i].getAddress().equal(address)) {
+ updateAddressRTTAtIndex(rtt, i, family);
+ return;
+ }
+ }
}
// Sets the address to be unreachable
Modified: branches/trac408/src/lib/nsas/nameserver_entry.h
==============================================================================
--- branches/trac408/src/lib/nsas/nameserver_entry.h (original)
+++ branches/trac408/src/lib/nsas/nameserver_entry.h Sat Dec 11 20:26:31 2010
@@ -35,6 +35,7 @@
#include "resolver_interface.h"
#include "nsas_entry.h"
#include "random_number_generator.h"
+#include "nameserver_address.h"
namespace isc {
namespace nsas {
@@ -82,13 +83,6 @@
/// for several zones (hence is pointed to by more than one zone entry), and
/// may have several addresses associated with it.
///
-/// When created, zero or more addresses may be given. At any time, the list
-/// of addresses may be updated. This may occur (a) after creation, either to
-/// to get the list of addresses when none have been supplied or to replace
-/// glue records, or (b) when the object has been accessed but found to be
-/// expired (the address records have reached their TTL).
-/// TODO: Add code for update of addresses
-///
/// The addresses expire after their TTL has been reached. For simplicity,
/// (and because it is unlikely that A and AAAA records from the same zone have
/// different TTLs) there is one expiration time for all address records.
@@ -103,7 +97,7 @@
class NameserverEntry : public NsasEntry<NameserverEntry>, public Fetchable {
public:
/// List of addresses associated with this nameserver
- typedef std::vector<AddressEntry> AddressVector;
+ typedef std::vector<NameserverAddress> AddressVector;
typedef AddressVector::iterator AddressVectorIterator;
/// \brief Constructor where no A records are supplied.
@@ -139,8 +133,7 @@
* arrived and is is returned, but the other is still on the way).
* \todo Should we sort out unreachable addresses as well?
*/
- Fetchable::State getAddresses(
- NameserverEntry::AddressVector& addresses,
+ Fetchable::State getAddresses(AddressVector& addresses,
AddressFamily family = ANY_OK, bool expired_ok = false);
/// \brief Return one address
@@ -169,10 +162,25 @@
/// \brief Update RTT of the address that corresponding to the index
///
+ /// Shouldn't probably be used directly. Use corresponding
+ /// NameserverAddress.
/// \param rtt Round-Trip Time
/// \param index The address's index in address vector
/// \param family The address family, V4_ONLY or V6_ONLY
void updateAddressRTTAtIndex(uint32_t rtt, size_t index,
+ AddressFamily family);
+ /**
+ * \short Update RTT of an address.
+ *
+ * This is similar to updateAddressRTTAtIndex, but you pass the address,
+ * not it's index. Passing the index might be unsafe, because the position
+ * of the address or the cound of addresses may change in time.
+ *
+ * \param rtt Round-Trip Time
+ * \param address The address whose RTT should be updated.
+ * \param family The address family, V4_ONLY or V6_ONLY
+ */
+ void updateAddressRTT(uint32_t rtt, const asiolink::IOAddress& address,
AddressFamily family);
/// \brief Set Address Unreachable
@@ -244,8 +252,7 @@
//@}
private:
- // TODO Read-write lock?
- mutable boost::mutex mutex_; ///< Mutex protecting this object
+ mutable boost::recursive_mutex mutex_; ///< Mutex protecting this object
std::string name_; ///< Canonical name of the nameserver
isc::dns::RRClass classCode_; ///< Class of the nameserver
/**
Modified: branches/trac408/src/lib/nsas/random_number_generator.h
==============================================================================
--- branches/trac408/src/lib/nsas/random_number_generator.h (original)
+++ branches/trac408/src/lib/nsas/random_number_generator.h Sat Dec 11 20:26:31 2010
@@ -83,7 +83,7 @@
// Init with the current time
rng_.seed(time(NULL));
}
-
+
/// \brief Default constructor
///
WeightedRandomIntegerGenerator():
@@ -120,11 +120,6 @@
{
return std::lower_bound(cumulative_.begin(), cumulative_.end(), uniform_real_gen_())
- cumulative_.begin() + min_;
- }
-
- /// \brief Destroctor
- ~WeightedRandomIntegerGenerator()
- {
}
private:
Modified: branches/trac408/src/lib/nsas/tests/nameserver_address_unittest.cc
==============================================================================
--- branches/trac408/src/lib/nsas/tests/nameserver_address_unittest.cc (original)
+++ branches/trac408/src/lib/nsas/tests/nameserver_address_unittest.cc Sat Dec 11 20:26:31 2010
@@ -23,6 +23,7 @@
#include <dns/rrttl.h>
#include "../nameserver_address.h"
+#include "../nameserver_entry.h"
#include "nsas_test.h"
namespace isc {
@@ -68,7 +69,7 @@
uint32_t getAddressRTTAtIndex(uint32_t index) {
NameserverEntry::AddressVector addresses;
ns_.get()->getAddresses(addresses);
- return addresses[index].getRTT();
+ return (addresses[index].getAddressEntry().getRTT());
}
private:
@@ -87,31 +88,26 @@
protected:
// Constructor
NameserverAddressTest():
- ns_address_(ns_sample_.getNameserverEntry(), TEST_ADDRESS_INDEX,
- V4_ONLY),
- invalid_ns_address_(ns_sample_.getNameserverEntry(),
- ns_sample_.getAddressesCount(), V4_ONLY)
+ ns_address_(ns_sample_.getNameserverEntry(),
+ ns_sample_.getNameserverEntry()->getAddressAtIndex(
+ TEST_ADDRESS_INDEX, V4_ONLY), V4_ONLY)
{
}
NameserverEntrySample ns_sample_;
// Valid NameserverAddress object
NameserverAddress ns_address_;
-
- // NameserverAddress object that constructed with invalid index
- NameserverAddress invalid_ns_address_;
};
// Test that the address is equal to the address in NameserverEntry
TEST_F(NameserverAddressTest, Address) {
EXPECT_TRUE(ns_address_.getAddress().equal( ns_sample_.getAddressAtIndex(TEST_ADDRESS_INDEX)));
- // It will trigger an assert with the invalid index
- ASSERT_DEATH(invalid_ns_address_.getAddress(), "");
-
boost::shared_ptr<NameserverEntry> empty_ne((NameserverEntry*)NULL);
// It will throw an NullNameserverEntryPointer exception with the empty NameserverEntry shared pointer
- ASSERT_THROW({NameserverAddress empty_ns_address(empty_ne, 0, V4_ONLY);}, NullNameserverEntryPointer);
+ ASSERT_THROW({NameserverAddress empty_ns_address(empty_ne,
+ asiolink::IOAddress("127.0.0.1"), V4_ONLY);},
+ NullNameserverEntryPointer);
}
// Test that the RTT is updated
Modified: branches/trac408/src/lib/nsas/tests/nameserver_entry_unittest.cc
==============================================================================
--- branches/trac408/src/lib/nsas/tests/nameserver_entry_unittest.cc (original)
+++ branches/trac408/src/lib/nsas/tests/nameserver_entry_unittest.cc Sat Dec 11 20:26:31 2010
@@ -122,10 +122,10 @@
// Check they are not 0 and they are all small, they should be some kind
// of randomish numbers, so we can't expect much more here
- BOOST_FOREACH(AddressEntry& entry, vec) {
- EXPECT_GT(entry.getRTT(), 0);
+ BOOST_FOREACH(NameserverAddress& entry, vec) {
+ EXPECT_GT(entry.getAddressEntry().getRTT(), 0);
// 20 is some arbitrary small value
- EXPECT_LT(entry.getRTT(), 20);
+ EXPECT_LT(entry.getAddressEntry().getRTT(), 20);
}
}
@@ -143,7 +143,7 @@
// Take the first address and change the RTT.
IOAddress first_address = vec[0].getAddress();
- uint32_t first_rtt = vec[0].getRTT();
+ uint32_t first_rtt = vec[0].getAddressEntry().getRTT();
uint32_t new_rtt = first_rtt + 42;
alpha->setAddressRTT(first_address, new_rtt);
@@ -156,7 +156,7 @@
i != newvec.end(); ++i) {
if (i->getAddress().equal(first_address)) {
++matchcount;
- EXPECT_EQ(i->getRTT(), new_rtt);
+ EXPECT_EQ(i->getAddressEntry().getRTT(), new_rtt);
}
}
@@ -178,7 +178,7 @@
// Take the first address and mark as unreachable.
IOAddress first_address = vec[0].getAddress();
- EXPECT_FALSE(vec[0].isUnreachable());
+ EXPECT_FALSE(vec[0].getAddressEntry().isUnreachable());
alpha->setAddressUnreachable(first_address);
@@ -191,7 +191,7 @@
i != newvec.end(); ++i) {
if (i->getAddress().equal(first_address)) {
++matchcount;
- EXPECT_TRUE(i->isUnreachable());
+ EXPECT_TRUE(i->getAddressEntry().isUnreachable());
}
}
@@ -450,7 +450,7 @@
EXPECT_EQ(Fetchable::READY, entry->getAddresses(addresses, V6_ONLY, true));
ASSERT_EQ(2, addresses.size());
EXPECT_EQ("2001:db8::1", addresses[1].getAddress().toText());
- BOOST_FOREACH(const AddressEntry& address, addresses) {
+ BOOST_FOREACH(const NameserverAddress& address, addresses) {
entry->setAddressRTT(address.getAddress(), 123);
}
@@ -475,8 +475,8 @@
ASSERT_EQ(2, addresses.size());
EXPECT_EQ("2001:db8::1", addresses[1].getAddress().toText());
// They should have the RTT we set to them
- BOOST_FOREACH(AddressEntry& address, addresses) {
- EXPECT_EQ(123, address.getRTT());
+ BOOST_FOREACH(NameserverAddress& address, addresses) {
+ EXPECT_EQ(123, address.getAddressEntry().getRTT());
}
}
@@ -574,22 +574,22 @@
uint32_t stable_rtt = 100;
// Update the rtt
- ns->updateAddressRTTAtIndex(stable_rtt, 0, V4_ONLY);
+ vec[0].updateRTT(stable_rtt);
vec.clear();
ns->getAddresses(vec);
- uint32_t new_rtt = vec[0].getRTT();
+ uint32_t new_rtt = vec[0].getAddressEntry().getRTT();
// The rtt should not close to new rtt immediately
EXPECT_TRUE((stable_rtt - new_rtt) > (new_rtt - init_rtt));
// Update the rtt for enough times
for(int i = 0; i < 10000; ++i){
- ns->updateAddressRTTAtIndex(stable_rtt, 0, V4_ONLY);
+ vec[0].updateRTT(stable_rtt);
}
vec.clear();
ns->getAddresses(vec);
- new_rtt = vec[0].getRTT();
+ new_rtt = vec[0].getAddressEntry().getRTT();
// The rtt should be close to stable rtt value
EXPECT_TRUE((stable_rtt - new_rtt) < (new_rtt - init_rtt));
More information about the bind10-changes
mailing list