Add field trial to reduce STUN pings.
This patch adds field trials for limiting no of STUN pings
- max_outstanding_pings
send this count of outstanding pings (pings w/o any response),
after that never send any before a reply is received.
NOTE:
1) This patch redoes https://webrtc.googlesource.com/src.git/+/0d28972d8f0659ab90cef7fd59ca54fb122b71bc
which was put into the StunRequestManager.
But that mechanism is not used for STUN pings.
2) This patch build on field-trial-parser added in https://webrtc-review.googlesource.com/c/src/+/156083
Bug: webrtc:10282
Change-Id: If2f22d2b61a28598a3aa93781c9857145576b7a1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156162
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Honghai Zhang <honghaiz@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29467}
diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc
index e50890b..1c55619 100644
--- a/p2p/base/connection.cc
+++ b/p2p/base/connection.cc
@@ -1119,6 +1119,18 @@
return waiting > 2 * rtt();
}
+bool Connection::TooManyOutstandingPings(
+ const absl::optional<int>& max_outstanding_pings) const {
+ if (!max_outstanding_pings.has_value()) {
+ return false;
+ }
+ if (static_cast<int>(pings_since_last_response_.size()) <
+ *max_outstanding_pings) {
+ return false;
+ }
+ return true;
+}
+
ProxyConnection::ProxyConnection(Port* port,
size_t index,
const Candidate& remote_candidate)
diff --git a/p2p/base/connection.h b/p2p/base/connection.h
index b872dbf..dc9333d 100644
--- a/p2p/base/connection.h
+++ b/p2p/base/connection.h
@@ -294,6 +294,9 @@
bool stable(int64_t now) const;
+ // Check if we sent |val| pings without receving a response.
+ bool TooManyOutstandingPings(const absl::optional<int>& val) const;
+
protected:
enum { MSG_DELETE = 0, MSG_FIRST_AVAILABLE };
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 6e68aa3..a1c1450 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -697,13 +697,19 @@
webrtc::StructParametersParser::Create(
"skip_relay_to_non_relay_connections",
- &field_trials_.skip_relay_to_non_relay_connections)
+ &field_trials_.skip_relay_to_non_relay_connections,
+ "max_outstanding_pings", &field_trials_.max_outstanding_pings)
->Parse(webrtc::field_trial::FindFullName("WebRTC-IceFieldTrials"));
if (field_trials_.skip_relay_to_non_relay_connections) {
RTC_LOG(LS_INFO) << "Set skip_relay_to_non_relay_connections";
}
+ if (field_trials_.max_outstanding_pings.has_value()) {
+ RTC_LOG(LS_INFO) << "Set max_outstanding_pings: "
+ << *field_trials_.max_outstanding_pings;
+ }
+
webrtc::BasicRegatheringController::Config regathering_config(
config_.regather_all_networks_interval_range,
config_.regather_on_failed_networks_interval_or_default());
@@ -1700,6 +1706,7 @@
return b_is_better;
}
}
+
return 0;
}
@@ -2202,6 +2209,12 @@
return false;
}
+ // If we sent a number of pings wo/ reply, skip sending more
+ // until we get one.
+ if (conn->TooManyOutstandingPings(field_trials_.max_outstanding_pings)) {
+ return false;
+ }
+
// If the channel is weakly connected, ping all connections.
if (weak()) {
return true;
diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h
index 1fe68ec..a7a1fbe 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/p2p/base/p2p_transport_channel.h
@@ -77,6 +77,7 @@
struct IceFieldTrials {
bool skip_relay_to_non_relay_connections = false;
+ absl::optional<int> max_outstanding_pings;
};
// P2PTransportChannel manages the candidates and connection process to keep
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 1f21bc2..5a060a8 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -4529,6 +4529,29 @@
EXPECT_EQ_SIMULATED_WAIT(nullptr, GetPrunedPort(&ch), 1, fake_clock);
}
+TEST_F(P2PTransportChannelPingTest, TestMaxOutstandingPingsFieldTrial) {
+ webrtc::test::ScopedFieldTrials field_trials(
+ "WebRTC-IceFieldTrials/max_outstanding_pings:3/");
+ FakePortAllocator pa(rtc::Thread::Current(), nullptr);
+ P2PTransportChannel ch("max", 1, &pa);
+ ch.SetIceConfig(ch.config());
+ PrepareChannel(&ch);
+ ch.MaybeStartGathering();
+ ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
+ ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 2));
+
+ Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
+ Connection* conn2 = WaitForConnectionTo(&ch, "2.2.2.2", 2);
+ ASSERT_TRUE(conn1 != nullptr);
+ ASSERT_TRUE(conn2 != nullptr);
+
+ EXPECT_TRUE_WAIT(conn1->num_pings_sent() == 3 && conn2->num_pings_sent() == 3,
+ kDefaultTimeout);
+
+ // Check that these connections don't send any more pings.
+ EXPECT_EQ(nullptr, ch.FindNextPingableConnection());
+}
+
class P2PTransportChannelMostLikelyToWorkFirstTest
: public P2PTransportChannelPingTest {
public:
