Add WebRTC-IceFieldTrial send_ping_on_selected_ice_controlling

This patch adds send_ping_on_selected_ice_controlling that is
the same like send_ping_on_switch_ice_controlling except that it
also sends the ping on initial selection (i.e not a switch(?)).
I.e it sends "extra" pings when selecting a connection.

This lets the peer know that this candidate pair has been selected,
which causes some peers to unblock sending on the candidate pair.

TODO deprecate send_ping_on_switch_ice_controlling in favor of this,
i.e I should have done it like this on first attempt :(

Bug: webrtc:10273
Change-Id: I0e02cd5fd6d677720c1fa863af5d7c2334d02d4f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182780
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32006}
This commit is contained in:
Jonas Oreland
2020-08-27 12:43:10 +02:00
committed by Commit Bot
parent a8327d4415
commit 3d3b531b64
3 changed files with 40 additions and 3 deletions

View File

@ -696,6 +696,9 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
// Make sure that nomination reaching ICE controlled asap. // Make sure that nomination reaching ICE controlled asap.
"send_ping_on_switch_ice_controlling", "send_ping_on_switch_ice_controlling",
&field_trials_.send_ping_on_switch_ice_controlling, &field_trials_.send_ping_on_switch_ice_controlling,
// Make sure that nomination reaching ICE controlled asap.
"send_ping_on_selected_ice_controlling",
&field_trials_.send_ping_on_selected_ice_controlling,
// Reply to nomination ASAP. // Reply to nomination ASAP.
"send_ping_on_nomination_ice_controlled", "send_ping_on_nomination_ice_controlled",
&field_trials_.send_ping_on_nomination_ice_controlled, &field_trials_.send_ping_on_nomination_ice_controlled,
@ -1768,9 +1771,10 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
RTC_LOG(LS_INFO) << ToString() << ": No selected connection"; RTC_LOG(LS_INFO) << ToString() << ": No selected connection";
} }
if (field_trials_.send_ping_on_switch_ice_controlling && if (conn != nullptr && ice_role_ == ICEROLE_CONTROLLING &&
ice_role_ == ICEROLE_CONTROLLING && old_selected_connection != nullptr && ((field_trials_.send_ping_on_switch_ice_controlling &&
conn != nullptr) { old_selected_connection != nullptr) ||
field_trials_.send_ping_on_selected_ice_controlling)) {
PingConnection(conn); PingConnection(conn);
MarkConnectionPinged(conn); MarkConnectionPinged(conn);
} }

View File

@ -44,8 +44,14 @@ struct IceFieldTrials {
int rtt_estimate_halftime_ms = 500; int rtt_estimate_halftime_ms = 500;
// Sending a PING directly after a switch on ICE_CONTROLLING-side. // Sending a PING directly after a switch on ICE_CONTROLLING-side.
// TODO(jonaso) : Deprecate this in favor of
// |send_ping_on_selected_ice_controlling|.
bool send_ping_on_switch_ice_controlling = false; bool send_ping_on_switch_ice_controlling = false;
// Sending a PING directly after selecting a connection
// (i.e either a switch or the inital selection).
bool send_ping_on_selected_ice_controlling = false;
// Sending a PING directly after a nomination on ICE_CONTROLLED-side. // Sending a PING directly after a nomination on ICE_CONTROLLED-side.
bool send_ping_on_nomination_ice_controlled = false; bool send_ping_on_nomination_ice_controlled = false;

View File

@ -3826,6 +3826,33 @@ TEST_F(P2PTransportChannelPingTest, TestPingOnSwitch) {
EXPECT_EQ(conn2->num_pings_sent(), before + 1); EXPECT_EQ(conn2->num_pings_sent(), before + 1);
} }
// Test the field trial send_ping_on_switch_ice_controlling
// that sends a ping directly when selecteing a new connection
// on the ICE_CONTROLLING-side (i.e also initial selection).
TEST_F(P2PTransportChannelPingTest, TestPingOnSelected) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-IceFieldTrials/send_ping_on_selected_ice_controlling:true/");
FakePortAllocator pa(rtc::Thread::Current(), nullptr);
P2PTransportChannel ch("receiving state change", 1, &pa);
PrepareChannel(&ch);
ch.SetIceConfig(ch.config());
ch.SetIceRole(ICEROLE_CONTROLLING);
ch.MaybeStartGathering();
ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "1.1.1.1", 1, 1));
Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
ASSERT_TRUE(conn1 != nullptr);
const int before = conn1->num_pings_sent();
// A connection needs to be writable before it is selected for transmission.
conn1->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1));
// And the additional ping should have been sent directly.
EXPECT_EQ(conn1->num_pings_sent(), before + 1);
}
// The controlled side will select a connection as the "selected connection" // The controlled side will select a connection as the "selected connection"
// based on requests from an unknown address before the controlling side // based on requests from an unknown address before the controlling side
// nominates a connection, and will nominate a connection from an unknown // nominates a connection, and will nominate a connection from an unknown