merged changes from RC_1_0

ChangeLog

@@ -70,6 +70,7 @@
 	* almost completely changed the storage interface (for custom storage)
 	* added support for hashing pieces in multiple threads
 
+	* improve ip_voter to avoid flapping
 	* fixed bug when max_peerlist_size was set to 0
 	* fix issues with missing exported symbols when building dll
 	* fix division by zero bug in edge case while connecting peers

include/libtorrent/ip_voter.hpp

@@ -61,11 +61,13 @@ namespace libtorrent
 			external_ip_t(): sources(0), num_votes(0) {}
 
 			bool add_vote(sha1_hash const& k, int type);
+
+			// we want to sort decending
 			bool operator<(external_ip_t const& rhs) const
 			{
-				if (num_votes < rhs.num_votes) return true;
-				if (num_votes > rhs.num_votes) return false;
-				return sources < rhs.sources;
+				if (num_votes > rhs.num_votes) return true;
+				if (num_votes < rhs.num_votes) return false;
+				return sources > rhs.sources;
 			}
 
 			// this is a bloom filter of the IPs that have

src/ip_voter.cpp

@@ -58,17 +58,35 @@ namespace libtorrent
 		// this is the inverse condition, since this is the case
 		// were we exit, without rotating
 		if (m_total_votes < 50
-			&& (now - m_last_rotate < minutes(5) || m_total_votes == 0))
+			&& (now - m_last_rotate < minutes(5) || m_total_votes == 0)
+			&& m_valid_external)
 			return false;
 
 		// this shouldn't really happen if we have at least one
 		// vote.
 		if (m_external_addresses.empty()) return false;
 
-		// rotate
-		std::vector<external_ip_t>::iterator i = std::max_element(
-			m_external_addresses.begin(), m_external_addresses.end());
-		TORRENT_ASSERT(i != m_external_addresses.end());
+		// if there's just one vote, go with that
+		std::vector<external_ip_t>::iterator i;
+		if (m_external_addresses.size() == 1)
+		{
+			// avoid flapping. We need more votes to change our mind on the
+			// external IP
+			if (m_external_addresses[0].num_votes < 2) return false;
+		}
+		else
+		{
+			// find the top two votes.
+			std::partial_sort(m_external_addresses.begin()
+				, m_external_addresses.begin() + 2, m_external_addresses.end());
+
+			// if we don't have enough of a majority voting for the winning
+			// IP, don't rotate. This avoids flapping
+			if (m_external_addresses[0].num_votes * 2 / 3 <= m_external_addresses[1].num_votes)
+				return false;
+		}
+
+		i = m_external_addresses.begin();
 
 		bool ret = m_external_address != i->addr;
 		m_external_address = i->addr;
@@ -119,12 +137,9 @@ namespace libtorrent
 				// is a sort of weighted LRU.
 				std::stable_sort(m_external_addresses.begin(), m_external_addresses.end());
 
-				// erase the first element, since this is the
-				// oldest entry and the one with lowst number
-				// of votes. This makes sense because the oldest
-				// entry has had the longest time to receive more
-				// votes to be bumped up
-				m_external_addresses.erase(m_external_addresses.begin());
+				// erase the last element, since it is one of the
+				// ones with the fewest votes
+				m_external_addresses.erase(m_external_addresses.end() - 1);
 			}
 			m_external_addresses.push_back(external_ip_t());
 			i = m_external_addresses.end() - 1;
@@ -136,11 +151,18 @@ namespace libtorrent
 		
 		if (m_valid_external) return maybe_rotate();
 
-		i = std::max_element(m_external_addresses.begin(), m_external_addresses.end());
+		i = std::min_element(m_external_addresses.begin(), m_external_addresses.end());
 		TORRENT_ASSERT(i != m_external_addresses.end());
 
 		if (i->addr == m_external_address) return maybe_rotate();
 
+		if (m_external_address != address_v4())
+		{
+			// we have a temporary external address. As soon as we have
+			// more than 25 votes, consider deciding which one to settle for
+			return (m_total_votes >= 25) ? maybe_rotate() : false;
+		}
+
 		m_external_address = i->addr;
 
 		return true;

test/Jamfile

@@ -102,6 +102,7 @@ test-suite libtorrent :
 	[ run test_resolve_links.cpp ]
 	[ run test_crc32.cpp ]
 	[ run test_heterogeneous_queue.cpp ]
+	[ run test_ip_voter.cpp ]
 	[ run test_resume.cpp ]
 	[ run test_sliding_average.cpp ]
 	[ run test_socket_io.cpp ]

test/Makefile.am

@@ -23,6 +23,7 @@ test_programs = \
   test_heterogeneous_queue   \
   test_http_connection       \
   test_ip_filter             \
+  test_ip_voter              \
   test_dht                   \
   test_lsd                   \
   test_metadata_extension    \
@@ -164,6 +165,7 @@ test_hasher_SOURCES = test_hasher.cpp
 test_heterogeneous_queue_SOURCES = test_heterogeneous_queue.cpp
 test_http_connection_SOURCES = test_http_connection.cpp
 test_ip_filter_SOURCES = test_ip_filter.cpp
+test_ip_voter_SOURCES = test_ip_voter.cpp
 test_lsd_SOURCES = test_lsd.cpp
 test_metadata_extension_SOURCES = test_metadata_extension.cpp
 test_peer_priority_SOURCES = test_peer_priority.cpp

test/test_ip_voter.cpp

@@ -0,0 +1,120 @@
+/*
+
+Copyright (c) 2015, Arvid Norberg
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the distribution.
+    * Neither the name of the author nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#include "test.hpp"
+#include "libtorrent/ip_voter.hpp"
+#include "libtorrent/address.hpp"
+#include "libtorrent/socket.hpp"
+#include "libtorrent/random.hpp"
+#include "setup_transfer.hpp" // for rand_v4
+
+using namespace libtorrent;
+
+// test the case where every time we get a new IP. Make sure
+// we don't flap
+void test_random()
+{
+	ip_voter ipv;
+
+	bool new_ip = ipv.cast_vote(rand_v4(), 1, rand_v4());
+	TEST_CHECK(new_ip);
+	for (int i = 0; i < 1000; ++i)
+	{
+		new_ip = ipv.cast_vote(rand_v4(), 1, rand_v4());
+		TEST_CHECK(!new_ip);
+	}
+}
+
+void test_two_ips()
+{
+	ip_voter ipv;
+
+	address_v4 addr1(address_v4::from_string("51.1.1.1"));
+	address_v4 addr2(address_v4::from_string("53.3.3.3"));
+
+	// addr1 is the first address we see, which is the one we pick. Even though
+	// we'll have as many votes for addr2, we shouldn't flap, since addr2 never
+	// gets an overwhelming majority.
+	bool new_ip = ipv.cast_vote(addr1, 1, rand_v4());
+	TEST_CHECK(new_ip);
+	for (int i = 0; i < 1000; ++i)
+	{
+		fprintf(stderr, "%d\n", i);
+		new_ip = ipv.cast_vote(addr2, 1, rand_v4());
+		TEST_CHECK(!new_ip);
+		new_ip = ipv.cast_vote(rand_v4(), 1, rand_v4());
+		TEST_CHECK(!new_ip);
+		new_ip = ipv.cast_vote(addr1, 1, rand_v4());
+		TEST_CHECK(!new_ip);
+
+		TEST_CHECK(ipv.external_address() == addr1);
+	}
+}
+
+void test_one_ip()
+{
+	ip_voter ipv;
+
+	address_v4 addr1(address_v4::from_string("51.1.1.1"));
+	address_v4 addr2(address_v4::from_string("53.3.3.3"));
+
+	bool new_ip = ipv.cast_vote(rand_v4(), 1, rand_v4());
+	TEST_CHECK(new_ip);
+	bool switched_ip = false;
+	for (int i = 0; i < 1000; ++i)
+	{
+		new_ip = ipv.cast_vote(addr2, 1, rand_v4());
+		TEST_CHECK(!new_ip);
+		new_ip = ipv.cast_vote(rand_v4(), 1, rand_v4());
+		TEST_CHECK(!new_ip);
+		new_ip = ipv.cast_vote(addr1, 1, rand_v4());
+		if (new_ip) switched_ip = true;
+		new_ip = ipv.cast_vote(addr1, 1, rand_v4());
+		if (new_ip) switched_ip = true;
+
+		if (switched_ip)
+		{
+			TEST_CHECK(ipv.external_address() == addr1);
+		}
+	}
+	TEST_CHECK(switched_ip);
+	TEST_CHECK(ipv.external_address() == addr1);
+}
+
+int test_main()
+{
+	test_random();
+	test_two_ips();
+	test_one_ip();
+	return 0;
+}
+