From 6e24bbe77a921b39db30d18edf22201d3a60d995 Mon Sep 17 00:00:00 2001 From: Steven Siloti Date: Mon, 19 Sep 2016 20:47:17 -0700 Subject: [PATCH 1/3] only send a write token if we have storage space available --- include/libtorrent/kademlia/dht_storage.hpp | 4 ++-- include/libtorrent/kademlia/node.hpp | 4 +--- src/kademlia/dht_storage.cpp | 7 +++---- src/kademlia/node.cpp | 9 +++++---- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/include/libtorrent/kademlia/dht_storage.hpp b/include/libtorrent/kademlia/dht_storage.hpp index e817846cf..9c20ca602 100644 --- a/include/libtorrent/kademlia/dht_storage.hpp +++ b/include/libtorrent/kademlia/dht_storage.hpp @@ -121,8 +121,8 @@ namespace dht // consider the value of dht_settings::max_peers_reply. // If noseed is true only peers marked as no seed should be included. // - // returns true if an entry with the info_hash is found and - // the data is returned inside the (entry) out parameter peers. + // returns true if the maximum number of peers are stored + // for this info_hash. // virtual bool get_peers(sha1_hash const& info_hash, udp protocol , bool noseed, bool scrape diff --git a/include/libtorrent/kademlia/node.hpp b/include/libtorrent/kademlia/node.hpp index 4a33fddae..c4e5d6a8f 100644 --- a/include/libtorrent/kademlia/node.hpp +++ b/include/libtorrent/kademlia/node.hpp @@ -219,10 +219,8 @@ private: void send_single_refresh(udp::endpoint const& ep, int bucket , node_id const& id = node_id()); - void lookup_peers(sha1_hash const& info_hash, entry& reply + bool lookup_peers(sha1_hash const& info_hash, entry& reply , bool noseed, bool scrape) const; - bool lookup_torrents(sha1_hash const& target, entry& reply - , char* tags) const; libtorrent::dht_settings const& m_settings; diff --git a/src/kademlia/dht_storage.cpp b/src/kademlia/dht_storage.cpp index 2c4f46867..e8ca2d56e 100644 --- a/src/kademlia/dht_storage.cpp +++ b/src/kademlia/dht_storage.cpp @@ -189,9 +189,8 @@ namespace , bool const noseed, bool const scrape , entry& peers) const override { - auto const i = m_map.lower_bound(info_hash); - if (i == m_map.end()) return false; - if (i->first != info_hash) return false; + auto const i = m_map.find(info_hash); + if (i == m_map.end()) return int(m_map.size()) >= m_settings.max_torrents; torrent_entry const& v = i->second; @@ -257,7 +256,7 @@ namespace ++m; } } - return true; + return int(i->second.peers.size()) >= m_settings.max_peers; } void announce_peer(sha1_hash const& info_hash diff --git a/src/kademlia/node.cpp b/src/kademlia/node.cpp index f78e1ca7d..73d85971c 100644 --- a/src/kademlia/node.cpp +++ b/src/kademlia/node.cpp @@ -750,13 +750,13 @@ void node::status(session_status& s) } #endif -void node::lookup_peers(sha1_hash const& info_hash, entry& reply +bool node::lookup_peers(sha1_hash const& info_hash, entry& reply , bool noseed, bool scrape) const { if (m_observer) m_observer->get_peers(info_hash); - m_storage.get_peers(info_hash, protocol(), noseed, scrape, reply); + return m_storage.get_peers(info_hash, protocol(), noseed, scrape, reply); } entry write_nodes_entry(std::vector const& nodes) @@ -848,7 +848,6 @@ void node::incoming_request(msg const& m, entry& e) } sha1_hash const info_hash(msg_keys[0].string_ptr()); - reply["token"] = generate_token(m.addr, info_hash); m_counters.inc_stats_counter(counters::dht_get_peers_in); @@ -859,7 +858,9 @@ void node::incoming_request(msg const& m, entry& e) bool scrape = false; if (msg_keys[1] && msg_keys[1].int_value() != 0) noseed = true; if (msg_keys[2] && msg_keys[2].int_value() != 0) scrape = true; - lookup_peers(info_hash, reply, noseed, scrape); + bool full = lookup_peers(info_hash, reply, noseed, scrape); + if (!full) reply["token"] = generate_token(m.addr, info_hash); + #ifndef TORRENT_DISABLE_LOGGING if (reply.find_key("values") && m_observer) { From e2fefb074a5d2366e248db007b25d61e27c52c19 Mon Sep 17 00:00:00 2001 From: Steven Siloti Date: Mon, 19 Sep 2016 21:07:39 -0700 Subject: [PATCH 2/3] always drop the announcing peer when at capacity --- src/kademlia/dht_storage.cpp | 31 ++++++------------------------- test/test_dht_storage.cpp | 6 ++++-- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/kademlia/dht_storage.cpp b/src/kademlia/dht_storage.cpp index e8ca2d56e..1adadc5f8 100644 --- a/src/kademlia/dht_storage.cpp +++ b/src/kademlia/dht_storage.cpp @@ -267,26 +267,12 @@ namespace torrent_entry* v; if (ti == m_map.end()) { - // we don't have this torrent, add it - // do we need to remove another one first? - if (!m_map.empty() && int(m_map.size()) >= m_settings.max_torrents) + if (int(m_map.size()) >= m_settings.max_torrents) { - // we need to remove some. Remove the ones with the - // fewest peers - int num_peers = int(m_map.begin()->second.peers.size()); - auto candidate = m_map.begin(); - for (auto i = m_map.begin() - , end(m_map.end()); i != end; ++i) - { - if (int(i->second.peers.size()) > num_peers) continue; - if (i->first == info_hash) continue; - num_peers = int(i->second.peers.size()); - candidate = i; - } - m_map.erase(candidate); - m_counters.peers -= num_peers; - m_counters.torrents -= 1; + // we're at capacity, drop the announce + return; } + m_counters.torrents += 1; v = &m_map[info_hash]; } @@ -314,13 +300,8 @@ namespace } else if (v->peers.size() >= m_settings.max_peers) { - // when we're at capacity, there's a 50/50 chance of dropping the - // announcing peer or an existing peer - if (random(1)) return; - i = v->peers.lower_bound(peer); - if (i == v->peers.end()) --i; - v->peers.erase(i++); - m_counters.peers -= 1; + // we're at capacity, drop the announce + return; } v->peers.insert(i, peer); m_counters.peers += 1; diff --git a/test/test_dht_storage.cpp b/test/test_dht_storage.cpp index 1c7e16b12..12466238a 100644 --- a/test/test_dht_storage.cpp +++ b/test/test_dht_storage.cpp @@ -111,6 +111,7 @@ TORRENT_TEST(announce_peer) tcp::endpoint const p4 = ep("124.31.75.24", 1); s->announce_peer(n1, p1, "torrent_name", false); + peers = entry(); s->get_peers(n1, udp::v4(), false, false, peers); TEST_EQUAL(peers["n"].string(), "torrent_name") TEST_EQUAL(peers["values"].list().size(), 1) @@ -118,8 +119,9 @@ TORRENT_TEST(announce_peer) s->announce_peer(n2, p2, "torrent_name1", false); s->announce_peer(n2, p3, "torrent_name1", false); s->announce_peer(n3, p4, "torrent_name2", false); - bool r = s->get_peers(n1, udp::v4(), false, false, peers); - TEST_CHECK(!r); + peers = entry(); + s->get_peers(n3, udp::v4(), false, false, peers); + TEST_CHECK(!peers.find_key("values")); } TORRENT_TEST(dual_stack) From dad00000151fe34c45e1237c1da77dd1c0869b81 Mon Sep 17 00:00:00 2001 From: Steven Siloti Date: Tue, 20 Sep 2016 10:45:13 -0700 Subject: [PATCH 3/3] send write token if peer is alraedy stored --- include/libtorrent/kademlia/dht_storage.hpp | 2 +- include/libtorrent/kademlia/node.hpp | 2 +- src/kademlia/dht_storage.cpp | 16 ++++++++++++++-- src/kademlia/node.cpp | 9 ++++++--- test/test_dht_storage.cpp | 10 +++++----- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/include/libtorrent/kademlia/dht_storage.hpp b/include/libtorrent/kademlia/dht_storage.hpp index 9c20ca602..823747c7e 100644 --- a/include/libtorrent/kademlia/dht_storage.hpp +++ b/include/libtorrent/kademlia/dht_storage.hpp @@ -125,7 +125,7 @@ namespace dht // for this info_hash. // virtual bool get_peers(sha1_hash const& info_hash, udp protocol - , bool noseed, bool scrape + , bool noseed, bool scrape, address const& requester , entry& peers) const = 0; // This function is named announce_peer for consistency with the diff --git a/include/libtorrent/kademlia/node.hpp b/include/libtorrent/kademlia/node.hpp index c4e5d6a8f..721898ceb 100644 --- a/include/libtorrent/kademlia/node.hpp +++ b/include/libtorrent/kademlia/node.hpp @@ -220,7 +220,7 @@ private: void send_single_refresh(udp::endpoint const& ep, int bucket , node_id const& id = node_id()); bool lookup_peers(sha1_hash const& info_hash, entry& reply - , bool noseed, bool scrape) const; + , bool noseed, bool scrape, address const& requester) const; libtorrent::dht_settings const& m_settings; diff --git a/src/kademlia/dht_storage.cpp b/src/kademlia/dht_storage.cpp index 1adadc5f8..0b9575a6b 100644 --- a/src/kademlia/dht_storage.cpp +++ b/src/kademlia/dht_storage.cpp @@ -186,7 +186,7 @@ namespace } bool get_peers(sha1_hash const& info_hash, udp const protocol - , bool const noseed, bool const scrape + , bool const noseed, bool const scrape, address const& requester , entry& peers) const override { auto const i = m_map.find(info_hash); @@ -256,7 +256,19 @@ namespace ++m; } } - return int(i->second.peers.size()) >= m_settings.max_peers; + + if (int(i->second.peers.size()) < m_settings.max_peers) + return false; + + // we're at the max peers stored for this torrent + // only send a write token if the requester is already in the set + // only check for a match on IP because the peer may be announcing + // a different port than the one it is using to send DHT messages + peer_entry requester_entry; + requester_entry.addr.address(requester); + auto requester_iter = i->second.peers.lower_bound(requester_entry); + return requester_iter == i->second.peers.end() + || requester_iter->addr.address() != requester; } void announce_peer(sha1_hash const& info_hash diff --git a/src/kademlia/node.cpp b/src/kademlia/node.cpp index 73d85971c..b4b46f7b3 100644 --- a/src/kademlia/node.cpp +++ b/src/kademlia/node.cpp @@ -751,12 +751,12 @@ void node::status(session_status& s) #endif bool node::lookup_peers(sha1_hash const& info_hash, entry& reply - , bool noseed, bool scrape) const + , bool noseed, bool scrape, address const& requester) const { if (m_observer) m_observer->get_peers(info_hash); - return m_storage.get_peers(info_hash, protocol(), noseed, scrape, reply); + return m_storage.get_peers(info_hash, protocol(), noseed, scrape, requester, reply); } entry write_nodes_entry(std::vector const& nodes) @@ -858,7 +858,10 @@ void node::incoming_request(msg const& m, entry& e) bool scrape = false; if (msg_keys[1] && msg_keys[1].int_value() != 0) noseed = true; if (msg_keys[2] && msg_keys[2].int_value() != 0) scrape = true; - bool full = lookup_peers(info_hash, reply, noseed, scrape); + // If our storage is full we want to withhold the write token so that + // announces will spill over to our neighbors. This widens the + // perimeter of nodes which store peers for this torrent + bool full = lookup_peers(info_hash, reply, noseed, scrape, m.addr.address()); if (!full) reply["token"] = generate_token(m.addr, info_hash); #ifndef TORRENT_DISABLE_LOGGING diff --git a/test/test_dht_storage.cpp b/test/test_dht_storage.cpp index 12466238a..b52d0fcce 100644 --- a/test/test_dht_storage.cpp +++ b/test/test_dht_storage.cpp @@ -100,7 +100,7 @@ TORRENT_TEST(announce_peer) std::unique_ptr s(create_default_dht_storage(sett)); entry peers; - s->get_peers(n1, udp::v4(), false, false, peers); + s->get_peers(n1, udp::v4(), false, false, address(), peers); TEST_CHECK(peers["n"].string().empty()) TEST_CHECK(peers["values"].list().empty()); @@ -112,7 +112,7 @@ TORRENT_TEST(announce_peer) s->announce_peer(n1, p1, "torrent_name", false); peers = entry(); - s->get_peers(n1, udp::v4(), false, false, peers); + s->get_peers(n1, udp::v4(), false, false, address(), peers); TEST_EQUAL(peers["n"].string(), "torrent_name") TEST_EQUAL(peers["values"].list().size(), 1) @@ -120,7 +120,7 @@ TORRENT_TEST(announce_peer) s->announce_peer(n2, p3, "torrent_name1", false); s->announce_peer(n3, p4, "torrent_name2", false); peers = entry(); - s->get_peers(n3, udp::v4(), false, false, peers); + s->get_peers(n3, udp::v4(), false, false, address(), peers); TEST_CHECK(!peers.find_key("values")); } @@ -142,11 +142,11 @@ TORRENT_TEST(dual_stack) s->announce_peer(n1, p5, "torrent_name", false); entry peers4; - s->get_peers(n1, udp::v4(), false, false, peers4); + s->get_peers(n1, udp::v4(), false, false, address(), peers4); TEST_EQUAL(peers4["values"].list().size(), 3); entry peers6; - s->get_peers(n1, udp::v6(), false, false, peers6); + s->get_peers(n1, udp::v6(), false, false, address(), peers6); TEST_EQUAL(peers6["values"].list().size(), 2); }