diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index d83726eab5..c55b310208 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -217,10 +217,11 @@ static const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 1000; // writable or not receiving. const int WEAK_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 10000; -// If the current best connection is both writable and receiving, then we will -// also try hard to make sure it is pinged at this rate (a little less than -// 2 * STRONG_PING_INTERVAL). -static const int MAX_CURRENT_STRONG_INTERVAL = 900; // ms +// Writable connections are pinged at a faster rate while stabilizing. +const int STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL = 900; // ms + +// Writable connections are pinged at a slower rate once stabilized. +const int STABLE_WRITABLE_CONNECTION_PING_INTERVAL = 2500; // ms static const int MIN_CHECK_RECEIVING_INTERVAL = 50; // ms @@ -250,7 +251,7 @@ P2PTransportChannel::P2PTransportChannel(const std::string& transport_name, 0 /* backup_connection_ping_interval */, false /* gather_continually */, false /* prioritize_most_likely_candidate_pairs */, - MAX_CURRENT_STRONG_INTERVAL /* max_strong_interval */) { + STABLE_WRITABLE_CONNECTION_PING_INTERVAL) { uint32_t weak_ping_interval = ::strtoul( webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(), nullptr, 10); @@ -432,11 +433,13 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) { LOG(LS_INFO) << "Set ping most likely connection to " << config_.prioritize_most_likely_candidate_pairs; - if (config.max_strong_interval >= 0 && - config_.max_strong_interval != config.max_strong_interval) { - config_.max_strong_interval = config.max_strong_interval; - LOG(LS_INFO) << "Set max strong interval to " - << config_.max_strong_interval; + if (config.stable_writable_connection_ping_interval >= 0 && + config_.stable_writable_connection_ping_interval != + config.stable_writable_connection_ping_interval) { + config_.stable_writable_connection_ping_interval = + config.stable_writable_connection_ping_interval; + LOG(LS_INFO) << "Set stable_writable_connection_ping_interval to " + << config_.stable_writable_connection_ping_interval; } } @@ -1381,12 +1384,50 @@ bool P2PTransportChannel::IsPingable(Connection* conn, int64_t now) { return (now >= conn->last_ping_response_received() + config_.backup_connection_ping_interval); } - return conn->active(); + // Don't ping inactive non-backup connections. + if (!conn->active()) { + return false; + } + + // Do ping unwritable, active connections. + if (!conn->writable()) { + return true; + } + + // Ping writable, active connections if it's been long enough since the last + // ping. + int ping_interval = CalculateActiveWritablePingInterval(conn, now); + return (now >= conn->last_ping_sent() + ping_interval); +} + +bool P2PTransportChannel::IsBestConnectionPingable(int64_t now) { + if (!best_connection_ || !best_connection_->connected() || + !best_connection_->writable()) { + return false; + } + + int interval = CalculateActiveWritablePingInterval(best_connection_, now); + return best_connection_->last_ping_sent() + interval <= now; +} + +int P2PTransportChannel::CalculateActiveWritablePingInterval(Connection* conn, + int64_t now) { + // Ping each connection at a higher rate at least + // MIN_PINGS_AT_WEAK_PING_INTERVAL times. + if (conn->num_pings_sent() < MIN_PINGS_AT_WEAK_PING_INTERVAL) { + return weak_ping_interval_; + } + + int stable_interval = config_.stable_writable_connection_ping_interval; + int stablizing_interval = + std::min(stable_interval, STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL); + + return conn->stable(now) ? stable_interval : stablizing_interval; } // Returns the next pingable connection to ping. This will be the oldest // pingable connection unless we have a connected, writable connection that is -// past the maximum acceptable ping interval. When reconnecting a TCP +// past the writable ping interval. When reconnecting a TCP // connection, the best connection is disconnected, although still WRITABLE // while reconnecting. The newly created connection should be selected as the // ping target to become writable instead. See the big comment in @@ -1394,10 +1435,7 @@ bool P2PTransportChannel::IsPingable(Connection* conn, int64_t now) { Connection* P2PTransportChannel::FindNextPingableConnection() { int64_t now = rtc::TimeMillis(); Connection* conn_to_ping = nullptr; - if (best_connection_ && best_connection_->connected() && - best_connection_->writable() && - (best_connection_->last_ping_sent() + config_.max_strong_interval <= - now)) { + if (IsBestConnectionPingable(now)) { conn_to_ping = best_connection_; } else { conn_to_ping = FindConnectionToPing(now); diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index ada3ecfb8b..bac7093f4c 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -39,6 +39,8 @@ namespace cricket { extern const int WEAK_PING_INTERVAL; +extern const int STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL; +extern const int STABLE_WRITABLE_CONNECTION_PING_INTERVAL; static const int MIN_PINGS_AT_WEAK_PING_INTERVAL = 3; struct IceParameters { @@ -227,6 +229,8 @@ class P2PTransportChannel : public TransportChannelImpl, void RememberRemoteCandidate(const Candidate& remote_candidate, PortInterface* origin_port); bool IsPingable(Connection* conn, int64_t now); + bool IsBestConnectionPingable(int64_t now); + int CalculateActiveWritablePingInterval(Connection* conn, int64_t now); void PingConnection(Connection* conn); void AddAllocatorSession(std::unique_ptr session); void AddConnection(Connection* connection); diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 587f971f1e..227e913c2e 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -44,6 +44,7 @@ static const int kDefaultTimeout = 1000; static const int kOnlyLocalPorts = cricket::PORTALLOCATOR_DISABLE_STUN | cricket::PORTALLOCATOR_DISABLE_RELAY | cricket::PORTALLOCATOR_DISABLE_TCP; +static const int LOW_RTT = 20; // Addresses on the public internet. static const SocketAddress kPublicAddrs[2] = { SocketAddress("11.11.11.11", 0), SocketAddress("22.22.22.22", 0) }; @@ -2077,13 +2078,103 @@ TEST_F(P2PTransportChannelPingTest, TestAllConnectionsPingedSufficiently) { // Low-priority connection becomes writable so that the other connection // is not pruned. - conn1->ReceivedPingResponse(); + conn1->ReceivedPingResponse(LOW_RTT); EXPECT_TRUE_WAIT( conn1->num_pings_sent() >= MIN_PINGS_AT_WEAK_PING_INTERVAL && conn2->num_pings_sent() >= MIN_PINGS_AT_WEAK_PING_INTERVAL, kDefaultTimeout); } +// Verify that the connections are pinged at the right time. +TEST_F(P2PTransportChannelPingTest, TestStunPingIntervals) { + rtc::ScopedFakeClock clock; + int RTT_RATIO = 4; + int SCHEDULING_RANGE = 200; + int RTT_RANGE = 10; + + cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr); + cricket::P2PTransportChannel ch("TestChannel", 1, &pa); + PrepareChannel(&ch); + ch.Connect(); + ch.MaybeStartGathering(); + ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1)); + cricket::Connection* conn = WaitForConnectionTo(&ch, "1.1.1.1", 1); + + ASSERT_TRUE(conn != nullptr); + SIMULATED_WAIT(conn->num_pings_sent() == 1, kDefaultTimeout, clock); + + // Initializing. + + int64_t start = clock.TimeNanos(); + SIMULATED_WAIT(conn->num_pings_sent() >= MIN_PINGS_AT_WEAK_PING_INTERVAL, + kDefaultTimeout, clock); + int64_t ping_interval_ms = (clock.TimeNanos() - start) / + rtc::kNumNanosecsPerMillisec / + (MIN_PINGS_AT_WEAK_PING_INTERVAL - 1); + EXPECT_EQ(ping_interval_ms, cricket::WEAK_PING_INTERVAL); + + // Stabilizing. + + conn->ReceivedPingResponse(LOW_RTT); + int ping_sent_before = conn->num_pings_sent(); + start = clock.TimeNanos(); + // The connection becomes strong but not stable because we haven't been able + // to converge the RTT. + SIMULATED_WAIT(conn->num_pings_sent() == ping_sent_before + 1, 3000, clock); + ping_interval_ms = (clock.TimeNanos() - start) / rtc::kNumNanosecsPerMillisec; + EXPECT_GE(ping_interval_ms, + cricket::STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL); + EXPECT_LE(ping_interval_ms, + cricket::STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL + + SCHEDULING_RANGE); + + // Stabilized. + + // The connection becomes stable after receiving more than RTT_RATIO rtt + // samples. + for (int i = 0; i < RTT_RATIO; i++) { + conn->ReceivedPingResponse(LOW_RTT); + } + ping_sent_before = conn->num_pings_sent(); + start = clock.TimeNanos(); + SIMULATED_WAIT(conn->num_pings_sent() == ping_sent_before + 1, 3000, clock); + ping_interval_ms = (clock.TimeNanos() - start) / rtc::kNumNanosecsPerMillisec; + EXPECT_GE(ping_interval_ms, + cricket::STABLE_WRITABLE_CONNECTION_PING_INTERVAL); + EXPECT_LE( + ping_interval_ms, + cricket::STABLE_WRITABLE_CONNECTION_PING_INTERVAL + SCHEDULING_RANGE); + + // Destabilized. + + conn->ReceivedPingResponse(LOW_RTT); + // Create a in-flight ping. + conn->Ping(clock.TimeNanos() / rtc::kNumNanosecsPerMillisec); + start = clock.TimeNanos(); + // In-flight ping timeout and the connection will be unstable. + SIMULATED_WAIT( + !conn->stable(clock.TimeNanos() / rtc::kNumNanosecsPerMillisec), 3000, + clock); + int64_t duration_ms = + (clock.TimeNanos() - start) / rtc::kNumNanosecsPerMillisec; + EXPECT_GE(duration_ms, 2 * conn->rtt() - RTT_RANGE); + EXPECT_LE(duration_ms, 2 * conn->rtt() + RTT_RANGE); + // The connection become unstable due to not receiving ping responses. + ping_sent_before = conn->num_pings_sent(); + SIMULATED_WAIT(conn->num_pings_sent() == ping_sent_before + 1, 3000, clock); + // The interval is expected to be + // STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL. + start = clock.TimeNanos(); + ping_sent_before = conn->num_pings_sent(); + SIMULATED_WAIT(conn->num_pings_sent() == ping_sent_before + 1, 3000, clock); + ping_interval_ms = (clock.TimeNanos() - start) / rtc::kNumNanosecsPerMillisec; + EXPECT_GE(ping_interval_ms, + cricket::STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL); + EXPECT_LE(ping_interval_ms, + cricket::STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL + + SCHEDULING_RANGE); +} + TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) { cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr); cricket::P2PTransportChannel ch("trigger checks", 1, &pa); @@ -2100,7 +2191,7 @@ TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) { EXPECT_EQ(conn2, FindNextPingableConnectionAndPingIt(&ch)); EXPECT_EQ(conn1, FindNextPingableConnectionAndPingIt(&ch)); - conn1->ReceivedPingResponse(); + conn1->ReceivedPingResponse(LOW_RTT); ASSERT_TRUE(conn1->writable()); conn1->ReceivedPing(); @@ -2200,7 +2291,7 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) { cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2); ASSERT_TRUE(conn2 != nullptr); conn2->ReceivedPing(); - conn2->ReceivedPingResponse(); + conn2->ReceivedPingResponse(LOW_RTT); // Wait for conn1 to be pruned. EXPECT_TRUE_WAIT(conn1->pruned(), 3000); @@ -2303,7 +2394,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { ASSERT_TRUE(conn3 != nullptr); // Because it has a lower priority, the best connection is still conn2. EXPECT_EQ(conn2, ch.best_connection()); - conn3->ReceivedPingResponse(); // Become writable. + conn3->ReceivedPingResponse(LOW_RTT); // Become writable. // But if it is nominated via use_candidate, it is chosen as the best // connection. conn3->set_nominated(true); @@ -2329,7 +2420,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { EXPECT_EQ(conn3, ch.best_connection()); reset_channel_ready_to_send(); // The best connection switches after conn4 becomes writable. - conn4->ReceivedPingResponse(); + conn4->ReceivedPingResponse(LOW_RTT); EXPECT_EQ(conn4, ch.best_connection()); EXPECT_EQ(conn4, last_selected_candidate_pair()); EXPECT_EQ(last_packet_id, last_sent_packet_id()); @@ -2364,7 +2455,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { ASSERT_TRUE(conn1 != nullptr); EXPECT_TRUE(port->sent_binding_response()); EXPECT_EQ(conn1, ch.best_connection()); - conn1->ReceivedPingResponse(); + conn1->ReceivedPingResponse(LOW_RTT); EXPECT_EQ(conn1, ch.best_connection()); port->set_sent_binding_response(false); @@ -2376,7 +2467,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { EXPECT_EQ(conn1, ch.best_connection()); // When it is nominated via use_candidate and writable, it is chosen as the // best connection. - conn2->ReceivedPingResponse(); // Become writable. + conn2->ReceivedPingResponse(LOW_RTT); // Become writable. conn2->set_nominated(true); conn2->SignalNominated(conn2); EXPECT_EQ(conn2, ch.best_connection()); @@ -2389,7 +2480,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3); ASSERT_TRUE(conn3 != nullptr); EXPECT_TRUE(port->sent_binding_response()); - conn3->ReceivedPingResponse(); // Become writable. + conn3->ReceivedPingResponse(LOW_RTT); // Become writable. EXPECT_EQ(conn2, ch.best_connection()); port->set_sent_binding_response(false); @@ -2404,7 +2495,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { EXPECT_TRUE(port->sent_binding_response()); // conn4 is not the best connection yet because it is not writable. EXPECT_EQ(conn2, ch.best_connection()); - conn4->ReceivedPingResponse(); // Become writable. + conn4->ReceivedPingResponse(LOW_RTT); // Become writable. EXPECT_EQ(conn4, ch.best_connection()); // Test that the request from an unknown address contains a ufrag from an old @@ -2447,7 +2538,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) { conn2->OnReadPacket("ABC", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(conn1, ch.best_connection()); - conn2->ReceivedPingResponse(); // Become writable. + conn2->ReceivedPingResponse(LOW_RTT); // Become writable. // Switch because it is writable. conn2->OnReadPacket("DEF", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(conn2, ch.best_connection()); @@ -2469,13 +2560,13 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) { cricket::Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3); ASSERT_TRUE(conn3 != nullptr); EXPECT_EQ(conn2, ch.best_connection()); // Not writable yet. - conn3->ReceivedPingResponse(); // Become writable. + conn3->ReceivedPingResponse(LOW_RTT); // Become writable. EXPECT_EQ(conn3, ch.best_connection()); // Now another data packet will not switch the best connection because the // best connection was nominated by the controlling side. conn2->ReceivedPing(); - conn2->ReceivedPingResponse(); + conn2->ReceivedPingResponse(LOW_RTT); conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(conn3, ch.best_connection()); } @@ -2532,14 +2623,14 @@ TEST_F(P2PTransportChannelPingTest, TestDontPruneWhenWeak) { cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1); ASSERT_TRUE(conn1 != nullptr); EXPECT_EQ(conn1, ch.best_connection()); - conn1->ReceivedPingResponse(); // Becomes writable and receiving + conn1->ReceivedPingResponse(LOW_RTT); // Becomes writable and receiving // When a higher-priority, nominated candidate comes in, the connections with // lower-priority are pruned. ch.AddRemoteCandidate(CreateHostCandidate("2.2.2.2", 2, 10)); cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2); ASSERT_TRUE(conn2 != nullptr); - conn2->ReceivedPingResponse(); // Becomes writable and receiving + conn2->ReceivedPingResponse(LOW_RTT); // Becomes writable and receiving conn2->set_nominated(true); conn2->SignalNominated(conn2); EXPECT_TRUE_WAIT(conn1->pruned(), 3000); @@ -2575,7 +2666,7 @@ TEST_F(P2PTransportChannelPingTest, TestGetState) { // Now there are two connections, so the transport channel is connecting. EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState()); // |conn1| becomes writable and receiving; it then should prune |conn2|. - conn1->ReceivedPingResponse(); + conn1->ReceivedPingResponse(LOW_RTT); EXPECT_TRUE_WAIT(conn2->pruned(), 1000); EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState()); conn1->Prune(); // All connections are pruned. @@ -2597,7 +2688,7 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) { cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1); ASSERT_TRUE(conn1 != nullptr); EXPECT_EQ(conn1, ch.best_connection()); - conn1->ReceivedPingResponse(); // Becomes writable and receiving + conn1->ReceivedPingResponse(LOW_RTT); // Becomes writable and receiving // Add a low-priority connection |conn2|, which will be pruned, but it will // not be deleted right away. Once the current best connection becomes not @@ -2616,12 +2707,12 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) { conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2); ASSERT_TRUE(conn2 != nullptr); EXPECT_EQ_WAIT(cricket::Connection::STATE_INPROGRESS, conn2->state(), 1000); - conn2->ReceivedPingResponse(); + conn2->ReceivedPingResponse(LOW_RTT); EXPECT_EQ_WAIT(conn2, ch.best_connection(), 1000); EXPECT_EQ(cricket::TransportChannelState::STATE_CONNECTING, ch.GetState()); // When |conn1| comes back again, |conn2| will be pruned again. - conn1->ReceivedPingResponse(); + conn1->ReceivedPingResponse(LOW_RTT); EXPECT_EQ_WAIT(conn1, ch.best_connection(), 1000); EXPECT_TRUE_WAIT(!conn2->active(), 1000); EXPECT_EQ(cricket::TransportChannelState::STATE_COMPLETED, ch.GetState()); @@ -2672,7 +2763,7 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 100)); cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1); ASSERT_TRUE(conn1 != nullptr); - conn1->ReceivedPingResponse(); // Becomes writable and receiving + conn1->ReceivedPingResponse(LOW_RTT); // Becomes writable and receiving EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts()); // Start a new session. Even though conn1, which belongs to an older @@ -2681,7 +2772,7 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]); ch.MaybeStartGathering(); conn1->Prune(); - conn1->ReceivedPingResponse(); + conn1->ReceivedPingResponse(LOW_RTT); EXPECT_TRUE(ch.allocator_session()->IsGettingPorts()); // But if a new connection created from the new session becomes writable, @@ -2689,7 +2780,7 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { ch.AddRemoteCandidate(CreateHostCandidate("2.2.2.2", 2, 100)); cricket::Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2); ASSERT_TRUE(conn2 != nullptr); - conn2->ReceivedPingResponse(); // Becomes writable and receiving + conn2->ReceivedPingResponse(LOW_RTT); // Becomes writable and receiving EXPECT_TRUE(!ch.allocator_session()->IsGettingPorts()); } @@ -2715,13 +2806,14 @@ class P2PTransportChannelMostLikelyToWorkFirstTest cricket::P2PTransportChannel& StartTransportChannel( bool prioritize_most_likely_to_work, - int max_strong_interval) { + int stable_writable_connection_ping_interval) { channel_.reset( new cricket::P2PTransportChannel("checks", 1, nullptr, allocator())); cricket::IceConfig config = channel_->config(); config.prioritize_most_likely_candidate_pairs = prioritize_most_likely_to_work; - config.max_strong_interval = max_strong_interval; + config.stable_writable_connection_ping_interval = + stable_writable_connection_ping_interval; channel_->SetIceConfig(config); PrepareChannel(channel_.get()); channel_->Connect(); @@ -2797,7 +2889,7 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, cricket::Connection* conn3 = WaitForConnectionTo(&ch, "1.1.1.1", 1); EXPECT_EQ(conn3->local_candidate().type(), cricket::LOCAL_PORT_TYPE); EXPECT_EQ(conn3->remote_candidate().type(), cricket::RELAY_PORT_TYPE); - conn3->ReceivedPingResponse(); + conn3->ReceivedPingResponse(LOW_RTT); ASSERT_TRUE(conn3->writable()); conn3->ReceivedPing(); @@ -2814,7 +2906,7 @@ TEST_F(P2PTransportChannelMostLikelyToWorkFirstTest, // pingable connection. EXPECT_TRUE_WAIT(conn3 == ch.best_connection(), 5000); WAIT(false, max_strong_interval + 100); - conn3->ReceivedPingResponse(); + conn3->ReceivedPingResponse(LOW_RTT); ASSERT_TRUE(conn3->writable()); EXPECT_EQ(conn3, FindNextPingableConnectionAndPingIt(&ch)); diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index 0add358ec9..c0e8d9770d 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -1188,7 +1188,7 @@ void Connection::ReceivedPing() { last_ping_received_ = rtc::TimeMillis(); } -void Connection::ReceivedPingResponse() { +void Connection::ReceivedPingResponse(int rtt) { // We've already validated that this is a STUN binding response with // the correct local and remote username for this connection. // So if we're not already, become writable. We may be bringing a pruned @@ -1199,6 +1199,8 @@ void Connection::ReceivedPingResponse() { set_state(STATE_SUCCEEDED); pings_since_last_response_.clear(); last_ping_response_received_ = rtc::TimeMillis(); + rtt_samples_++; + rtt_ = (RTT_RATIO * rtt_ + rtt) / (RTT_RATIO + 1); } bool Connection::dead(int64_t now) const { @@ -1226,6 +1228,14 @@ bool Connection::dead(int64_t now) const { return now > (time_created_ms_ + MIN_CONNECTION_LIFETIME); } +bool Connection::stable(int64_t now) { + // A connection is stable if it's RTT has converged and it isn't missing any + // responses. We should send pings at a higher rate until the RTT converges + // and whenever a ping response is missing (so that we can detect + // unwritability faster) + return rtt_converged() && !missing_responses(now); +} + std::string Connection::ToDebugId() const { std::stringstream ss; ss << std::hex << this; @@ -1296,7 +1306,7 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request, int rtt = request->Elapsed(); - ReceivedPingResponse(); + ReceivedPingResponse(rtt); if (LOG_CHECK_LEVEL_V(sev)) { bool use_candidate = ( @@ -1311,7 +1321,6 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request, << ", pings_since_last_response=" << pings; } - rtt_ = (RTT_RATIO * rtt_ + rtt) / (RTT_RATIO + 1); stats_.recv_ping_responses++; MaybeAddPrflxCandidate(request, response); @@ -1490,6 +1499,19 @@ void Connection::MaybeAddPrflxCandidate(ConnectionRequest* request, SignalStateChange(this); } +bool Connection::rtt_converged() { + return rtt_samples_ > (RTT_RATIO + 1); +} + +bool Connection::missing_responses(int64_t now) { + if (pings_since_last_response_.empty()) { + return false; + } + + int64_t waiting = now - pings_since_last_response_[0].sent_time; + return waiting > 2 * rtt(); +} + ProxyConnection::ProxyConnection(Port* port, size_t index, const Candidate& remote_candidate) diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 06efd2d3c2..65932b9c9f 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -530,7 +530,7 @@ class Connection : public CandidatePairInterface, // Called when this connection should try checking writability again. int64_t last_ping_sent() const { return last_ping_sent_; } void Ping(int64_t now); - void ReceivedPingResponse(); + void ReceivedPingResponse(int rtt); int64_t last_ping_response_received() const { return last_ping_response_received_; } @@ -585,6 +585,8 @@ class Connection : public CandidatePairInterface, // response in milliseconds int64_t last_received() const; + bool stable(int64_t now); + protected: enum { MSG_DELETE = 0, MSG_FIRST_AVAILABLE }; @@ -602,6 +604,12 @@ class Connection : public CandidatePairInterface, void OnConnectionRequestTimeout(ConnectionRequest* req); void OnConnectionRequestSent(ConnectionRequest* req); + bool rtt_converged(); + + // If the response is not received within 2 * RTT, the response is assumed to + // be missing. + bool missing_responses(int64_t now); + // Changes the state and signals if necessary. void set_write_state(WriteState value); void set_receiving(bool value); @@ -628,6 +636,7 @@ class Connection : public CandidatePairInterface, IceMode remote_ice_mode_; StunRequestManager requests_; int rtt_; + int rtt_samples_ = 0; int64_t last_ping_sent_; // last time we sent a ping to the other side int64_t last_ping_received_; // last time we received a ping from the other // side diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index 9b6cf519f8..d50b8e5d36 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -165,23 +165,22 @@ struct IceConfig { // is writable yet. bool prioritize_most_likely_candidate_pairs = false; - // If the current best connection is both writable and receiving, - // then we will also try hard to make sure it is pinged at this rate - // (Default value is a little less than 2 * STRONG_PING_INTERVAL). - int max_strong_interval = -1; + // Writable connections are pinged at a slower rate once stablized. + int stable_writable_connection_ping_interval = -1; IceConfig() {} IceConfig(int receiving_timeout_ms, int backup_connection_ping_interval, bool gather_continually, bool prioritize_most_likely_candidate_pairs, - int max_strong_interval_ms) + int stable_writable_connection_ping_interval_ms) : receiving_timeout(receiving_timeout_ms), backup_connection_ping_interval(backup_connection_ping_interval), gather_continually(gather_continually), prioritize_most_likely_candidate_pairs( prioritize_most_likely_candidate_pairs), - max_strong_interval(max_strong_interval_ms) {} + stable_writable_connection_ping_interval( + stable_writable_connection_ping_interval_ms) {} }; bool BadTransportDescription(const std::string& desc, std::string* err_desc);