admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add feature to not discard candidates after connection is established

Browse Source 
In P2PTransportChannel::OnConnectionStateChange there is
code that stop port allocation sessions if the modified
connection is stronly connected.

This means that local candidates are discarded (they are still
gathered, only not surfaced).

The implication of this is that if e.g doing a TURN allocation
slower than P2P is established, the TURN allocation will not be
added to list of local candidates => no TURN connection will be
created.

NOTE: If first connecting kRelay (only RELAY ONLY) then this
patch does matter that much...until an ICE restart happens :)

I discovered this when adding the emulated TURN server
to tests, and being surprised that the TURN allocations
never got used. These test does not (currently) use kRelay
as start.

Bug: webrtc:12210
Change-Id: I78a67201cf421b0e6fdd2ea684a00d740e063f5e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/194141
Reviewed-by: Taylor <deadbeef@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32647}
This commit is contained in:
Jonas Oreland 2020-11-19 15:46:50 +01:00 committed by Commit Bot
parent 39c172d2a5
commit 715d02334b
3 changed files with 144 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
p2p/base/p2p_transport_channel.cc
View File

@ -703,7 +703,10 @@ void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
"send_ping_on_nomination_ice_controlled",
&field_trials_.send_ping_on_nomination_ice_controlled,
// Allow connections to live untouched longer that 30s.
"dead_connection_timeout_ms", &field_trials_.dead_connection_timeout_ms)
"dead_connection_timeout_ms", &field_trials_.dead_connection_timeout_ms,
// Stop gathering on strongly connected.
"stop_gather_on_strongly_connected",
&field_trials_.stop_gather_on_strongly_connected)
->Parse(webrtc::field_trial::FindFullName("WebRTC-IceFieldTrials"));
if (field_trials_.dead_connection_timeout_ms < 30000) {
@ -2015,11 +2018,13 @@ void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
// the connection is at the latest generation. It is not enough to check
// that the connection becomes weakly connected because the connection may be
// changing from (writable, receiving) to (writable, not receiving).
bool strongly_connected = !connection->weak();
bool latest_generation = connection->local_candidate().generation() >=
allocator_session()->generation();
if (strongly_connected && latest_generation) {
MaybeStopPortAllocatorSessions();
if (field_trials_.stop_gather_on_strongly_connected) {
bool strongly_connected = !connection->weak();
bool latest_generation = connection->local_candidate().generation() >=
allocator_session()->generation();
if (strongly_connected && latest_generation) {
MaybeStopPortAllocatorSessions();
}
}
// We have to unroll the stack before doing this because we may be changing
// the state of connections while sorting.

3
p2p/base/p2p_transport_channel_ice_field_trials.h
View File

@ -58,6 +58,9 @@ struct IceFieldTrials {
// The timeout after which the connection will be considered dead if no
// traffic is received.
int dead_connection_timeout_ms = 30000;
// Stop gathering when having a strong connection.
bool stop_gather_on_strongly_connected = true;
};
} // namespace cricket

130
p2p/base/p2p_transport_channel_unittest.cc
View File

@ -5978,4 +5978,134 @@ TEST_F(P2PTransportChannelTest, EnableDnsLookupsWithTransportPolicyNoHost) {
DestroyChannels();
}
class GatherAfterConnectedTest : public P2PTransportChannelTest,
public ::testing::WithParamInterface<bool> {};
TEST_P(GatherAfterConnectedTest, GatherAfterConnected) {
const bool stop_gather_on_strongly_connected = GetParam();
const std::string field_trial =
std::string("WebRTC-IceFieldTrials/stop_gather_on_strongly_connected:") +
(stop_gather_on_strongly_connected ? "true/" : "false/");
webrtc::test::ScopedFieldTrials field_trials(field_trial);
rtc::ScopedFakeClock clock;
// Use local + relay
constexpr uint32_t flags =
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET |
PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_TCP;
ConfigureEndpoints(OPEN, OPEN, flags, flags);
auto* ep1 = GetEndpoint(0);
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_ALL);
ep2->allocator_->SetCandidateFilter(CF_ALL);
// Use step delay 3s which is long enough for
// connection to be established before managing to gather relay candidates.
int delay = 3000;
SetAllocationStepDelay(0, delay);
SetAllocationStepDelay(1, delay);
IceConfig ice_config = CreateIceConfig(1000, GATHER_CONTINUALLY);
CreateChannels(ice_config, ice_config);
PauseCandidates(0);
PauseCandidates(1);
// We have gathered host candidates but not relay.
ASSERT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 1u &&
ep2->saved_candidates_.size() == 1u,
kDefaultTimeout, clock);
ResumeCandidates(0);
ResumeCandidates(1);
PauseCandidates(0);
PauseCandidates(1);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->remote_candidates().size() == 1 &&
ep2_ch1()->remote_candidates().size() == 1,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(
ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection(),
kDefaultTimeout, clock);
clock.AdvanceTime(webrtc::TimeDelta::Millis(10 * delay));
if (stop_gather_on_strongly_connected) {
// The relay candiates gathered has not been propagated to channel.
EXPECT_EQ(ep1->saved_candidates_.size(), 0u);
EXPECT_EQ(ep2->saved_candidates_.size(), 0u);
} else {
// The relay candiates gathered has been propagated to channel.
EXPECT_EQ(ep1->saved_candidates_.size(), 1u);
EXPECT_EQ(ep2->saved_candidates_.size(), 1u);
}
}
TEST_P(GatherAfterConnectedTest, GatherAfterConnectedMultiHomed) {
const bool stop_gather_on_strongly_connected = GetParam();
const std::string field_trial =
std::string("WebRTC-IceFieldTrials/stop_gather_on_strongly_connected:") +
(stop_gather_on_strongly_connected ? "true/" : "false/");
webrtc::test::ScopedFieldTrials field_trials(field_trial);
rtc::ScopedFakeClock clock;
// Use local + relay
constexpr uint32_t flags =
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET |
PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_TCP;
AddAddress(0, kAlternateAddrs[0]);
ConfigureEndpoints(OPEN, OPEN, flags, flags);
auto* ep1 = GetEndpoint(0);
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_ALL);
ep2->allocator_->SetCandidateFilter(CF_ALL);
// Use step delay 3s which is long enough for
// connection to be established before managing to gather relay candidates.
int delay = 3000;
SetAllocationStepDelay(0, delay);
SetAllocationStepDelay(1, delay);
IceConfig ice_config = CreateIceConfig(1000, GATHER_CONTINUALLY);
CreateChannels(ice_config, ice_config);
PauseCandidates(0);
PauseCandidates(1);
// We have gathered host candidates but not relay.
ASSERT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 2u &&
ep2->saved_candidates_.size() == 1u,
kDefaultTimeout, clock);
ResumeCandidates(0);
ResumeCandidates(1);
PauseCandidates(0);
PauseCandidates(1);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->remote_candidates().size() == 1 &&
ep2_ch1()->remote_candidates().size() == 2,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(
ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection(),
kDefaultTimeout, clock);
clock.AdvanceTime(webrtc::TimeDelta::Millis(10 * delay));
if (stop_gather_on_strongly_connected) {
// The relay candiates gathered has not been propagated to channel.
EXPECT_EQ(ep1->saved_candidates_.size(), 0u);
EXPECT_EQ(ep2->saved_candidates_.size(), 0u);
} else {
// The relay candiates gathered has been propagated.
EXPECT_EQ(ep1->saved_candidates_.size(), 2u);
EXPECT_EQ(ep2->saved_candidates_.size(), 1u);
}
}
INSTANTIATE_TEST_SUITE_P(GatherAfterConnectedTest,
GatherAfterConnectedTest,
::testing::Values(true, false));
} // namespace cricket