Reland "Surface ICE candidates that match an updated candidate filter."

This is a reland of cd8d1cf68e4eeed71fba51c97006a91bfd41813d

Original change's description:
> Surface ICE candidates that match an updated candidate filter.
> 
> After this change an ICE agent can surface candidates that do not match
> the previous filter but are allowed by the updated one. The candidate
> filter, as part of the internal implementation in the ICE transport,
> manifests the RTCIceTransportPolicy field in RTCConfiguration.
> 
> This new feature would allow an ICE agent to gather new candidates when
> the transport policy changes from e.g. 'relay' to 'all' without an ICE
> restart.
> 
> A caveat in the current implementation remains, and a candidate can
> surface multiple times if the transport policy, or the candidate filter
> directly, performs multiple transitions from a value that disallows to
> one that allows the underlying candidate type. For example, if the
> transport policy is updated by 'all' -> 'relay' -> 'all', the same host
> candidate can surface after the second update.
> 
> 
> Bug: webrtc:8939
> Change-Id: I92c2e07dafab225c702c5de28f47958a0d3270cc
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132282
> Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
> Reviewed-by: Jeroen de Borst <jeroendb@webrtc.org>
> Reviewed-by: Seth Hampson <shampson@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27674}

Bug: webrtc:8939
Change-Id: I9c32b1ea05028ecd937ab4912779dd958faf734f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133582
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Jeroen de Borst <jeroendb@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27694}
This commit is contained in:
Qingsi Wang 2019-04-18 10:41:58 -07:00 committed by Commit Bot
parent 3ba187226e
commit c129c359d7
13 changed files with 594 additions and 77 deletions

View File

@ -202,6 +202,7 @@ if (rtc_include_tests) {
"../rtc_base/network:sent_packet",
"../rtc_base/third_party/sigslot",
"../system_wrappers:metrics",
"../test:field_trial",
"../test:test_support",
"//testing/gtest",
"//third_party/abseil-cpp/absl/algorithm:container",

View File

@ -15,6 +15,7 @@
#include <utility>
#include "absl/algorithm/container.h"
#include "absl/memory/memory.h"
#include "api/candidate.h"
#include "logging/rtc_event_log/ice_logger.h"
#include "p2p/base/candidate_pair_interface.h"
@ -148,8 +149,13 @@ P2PTransportChannel::P2PTransportChannel(
webrtc::BasicRegatheringController::Config regathering_config(
config_.regather_all_networks_interval_range,
config_.regather_on_failed_networks_interval_or_default());
regathering_controller_.reset(new webrtc::BasicRegatheringController(
regathering_config, this, network_thread_));
regathering_controller_ =
absl::make_unique<webrtc::BasicRegatheringController>(
regathering_config, this, network_thread_);
// We populate the change in the candidate filter to the session taken by
// the transport.
allocator_->SignalCandidateFilterChanged.connect(
this, &P2PTransportChannel::OnCandidateFilterChanged);
ice_event_log_.set_event_log(event_log);
}
@ -999,6 +1005,16 @@ void P2PTransportChannel::OnUnknownAddress(
"a new candidate pair created from an unknown remote address");
}
void P2PTransportChannel::OnCandidateFilterChanged(uint32_t prev_filter,
uint32_t cur_filter) {
if (prev_filter == cur_filter || allocator_session() == nullptr) {
return;
}
if (webrtc::field_trial::IsEnabled("WebRTC-GatherOnCandidateFilterChanged")) {
allocator_session()->SetCandidateFilter(cur_filter);
}
}
void P2PTransportChannel::OnRoleConflict(PortInterface* port) {
SignalRoleConflict(this); // STUN ping will be sent when SetRole is called
// from Transport.

View File

@ -292,6 +292,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
IceMessage* stun_msg,
const std::string& remote_username,
bool port_muxed);
void OnCandidateFilterChanged(uint32_t prev_filter, uint32_t cur_filter);
// When a port is destroyed, remove it from both lists |ports_|
// and |pruned_ports_|.

View File

@ -37,6 +37,7 @@
#include "rtc_base/thread.h"
#include "rtc_base/virtual_socket_server.h"
#include "system_wrappers/include/metrics.h"
#include "test/field_trial.h"
namespace {
@ -50,6 +51,7 @@ using ::testing::InvokeWithoutArgs;
using ::testing::NiceMock;
using ::testing::Return;
using ::testing::SetArgPointee;
using ::testing::SizeIs;
// Default timeout for tests in this file.
// Should be large enough for slow buildbots to run the tests reliably.
@ -1606,8 +1608,8 @@ TEST_F(P2PTransportChannelTest,
kDefaultPortAllocatorFlags);
// Only gather relay candidates, so that when the prflx candidate arrives
// it's prioritized above the current candidate pair.
GetEndpoint(0)->allocator_->set_candidate_filter(CF_RELAY);
GetEndpoint(1)->allocator_->set_candidate_filter(CF_RELAY);
GetEndpoint(0)->allocator_->SetCandidateFilter(CF_RELAY);
GetEndpoint(1)->allocator_->SetCandidateFilter(CF_RELAY);
// Setting this allows us to control when SetRemoteIceParameters is called.
set_remote_ice_parameter_source(FROM_CANDIDATE);
CreateChannels();
@ -4587,7 +4589,8 @@ TEST(P2PTransportChannelResolverTest, HostnameCandidateIsResolved) {
EXPECT_CALL(mock_async_resolver_factory, Create())
.WillOnce(Return(&mock_async_resolver));
P2PTransportChannel channel("tn", 0, /*allocator*/ nullptr,
FakePortAllocator allocator(rtc::Thread::Current(), nullptr);
P2PTransportChannel channel("tn", 0, &allocator,
&mock_async_resolver_factory);
Candidate hostname_candidate;
SocketAddress hostname_address("fake.test", 1000);
@ -4888,4 +4891,197 @@ TEST_F(P2PTransportChannelTest,
DestroyChannels();
}
// Test that after changing the candidate filter from relay-only to allowing all
// types of candidates when doing continual gathering, we can gather without ICE
// restart the other types of candidates that are now enabled and form candidate
// pairs. Also, we verify that the relay candidates gathered previously are not
// removed and are still usable for necessary route switching.
TEST_F(P2PTransportChannelTest,
SurfaceHostCandidateOnCandidateFilterChangeFromRelayToAll) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-GatherOnCandidateFilterChanged/Enabled/");
rtc::ScopedFakeClock clock;
ConfigureEndpoints(
OPEN, OPEN,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET);
auto* ep1 = GetEndpoint(0);
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_RELAY);
ep2->allocator_->SetCandidateFilter(CF_RELAY);
IceConfig continual_gathering_config =
CreateIceConfig(1000, GATHER_CONTINUALLY);
CreateChannels(continual_gathering_config, continual_gathering_config);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->local_candidate().type());
EXPECT_EQ(RELAY_PORT_TYPE,
ep2_ch1()->selected_connection()->local_candidate().type());
// Loosen the candidate filter at ep1.
ep1->allocator_->SetCandidateFilter(CF_ALL);
EXPECT_TRUE_SIMULATED_WAIT(
ep1_ch1()->selected_connection() != nullptr &&
ep1_ch1()->selected_connection()->local_candidate().type() ==
LOCAL_PORT_TYPE,
kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type());
// Loosen the candidate filter at ep2.
ep2->allocator_->SetCandidateFilter(CF_ALL);
EXPECT_TRUE_SIMULATED_WAIT(
ep2_ch1()->selected_connection() != nullptr &&
ep2_ch1()->selected_connection()->local_candidate().type() ==
LOCAL_PORT_TYPE,
kDefaultTimeout, clock);
// We have migrated to a host-host candidate pair.
EXPECT_EQ(LOCAL_PORT_TYPE,
ep2_ch1()->selected_connection()->remote_candidate().type());
// Block the traffic over non-relay-to-relay routes and expect a route change.
fw()->AddRule(false, rtc::FP_ANY, kPublicAddrs[0], kPublicAddrs[1]);
fw()->AddRule(false, rtc::FP_ANY, kPublicAddrs[1], kPublicAddrs[0]);
fw()->AddRule(false, rtc::FP_ANY, kPublicAddrs[0], kTurnUdpExtAddr);
fw()->AddRule(false, rtc::FP_ANY, kPublicAddrs[1], kTurnUdpExtAddr);
// We should be able to reuse the previously gathered relay candidates.
EXPECT_EQ_SIMULATED_WAIT(
RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->local_candidate().type(),
kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type());
}
// A similar test as SurfaceHostCandidateOnCandidateFilterChangeFromRelayToAll,
// and we should surface server-reflexive candidates that are enabled after
// changing the candidate filter.
TEST_F(P2PTransportChannelTest,
SurfaceSrflxCandidateOnCandidateFilterChangeFromRelayToNoHost) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-GatherOnCandidateFilterChanged/Enabled/");
rtc::ScopedFakeClock clock;
// We need an actual NAT so that the host candidate is not equivalent to the
// srflx candidate; otherwise, the host candidate would still surface even
// though we disable it via the candidate filter below. This is a result of
// the following limitation in the current implementation:
// 1. We don't generate the srflx candidate when we have public IP.
// 2. We keep the host candidate in this case in CheckCandidateFilter even
// though we intend to filter them.
ConfigureEndpoints(
NAT_FULL_CONE, NAT_FULL_CONE,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET);
auto* ep1 = GetEndpoint(0);
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_RELAY);
ep2->allocator_->SetCandidateFilter(CF_RELAY);
IceConfig continual_gathering_config =
CreateIceConfig(1000, GATHER_CONTINUALLY);
CreateChannels(continual_gathering_config, continual_gathering_config);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
const uint32_t kCandidateFilterNoHost = CF_ALL & ~CF_HOST;
// Loosen the candidate filter at ep1.
ep1->allocator_->SetCandidateFilter(kCandidateFilterNoHost);
EXPECT_TRUE_SIMULATED_WAIT(
ep1_ch1()->selected_connection() != nullptr &&
ep1_ch1()->selected_connection()->local_candidate().type() ==
STUN_PORT_TYPE,
kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type());
// Loosen the candidate filter at ep2.
ep2->allocator_->SetCandidateFilter(kCandidateFilterNoHost);
EXPECT_TRUE_SIMULATED_WAIT(
ep2_ch1()->selected_connection() != nullptr &&
ep2_ch1()->selected_connection()->local_candidate().type() ==
STUN_PORT_TYPE,
kDefaultTimeout, clock);
// We have migrated to a srflx-srflx candidate pair.
EXPECT_EQ(STUN_PORT_TYPE,
ep2_ch1()->selected_connection()->remote_candidate().type());
// Block the traffic over non-relay-to-relay routes and expect a route change.
fw()->AddRule(false, rtc::FP_ANY, kPrivateAddrs[0], kPublicAddrs[1]);
fw()->AddRule(false, rtc::FP_ANY, kPrivateAddrs[1], kPublicAddrs[0]);
fw()->AddRule(false, rtc::FP_ANY, kPrivateAddrs[0], kTurnUdpExtAddr);
fw()->AddRule(false, rtc::FP_ANY, kPrivateAddrs[1], kTurnUdpExtAddr);
// We should be able to reuse the previously gathered relay candidates.
EXPECT_EQ_SIMULATED_WAIT(
RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->local_candidate().type(),
kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type());
}
// Test that when the candidate filter is updated to be more restrictive,
// candidates that 1) have already been gathered and signaled 2) but no longer
// match the filter, are not removed.
TEST_F(P2PTransportChannelTest,
RestrictingCandidateFilterDoesNotRemoveRegatheredCandidates) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-GatherOnCandidateFilterChanged/Enabled/");
rtc::ScopedFakeClock clock;
ConfigureEndpoints(
OPEN, OPEN,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_SHARED_SOCKET);
auto* ep1 = GetEndpoint(0);
auto* ep2 = GetEndpoint(1);
ep1->allocator_->SetCandidateFilter(CF_ALL);
ep2->allocator_->SetCandidateFilter(CF_ALL);
IceConfig continual_gathering_config =
CreateIceConfig(1000, GATHER_CONTINUALLY);
// Pause candidates so we can gather all types of candidates. See
// P2PTransportChannel::OnConnectionStateChange, where we would stop the
// gathering when we have a strongly connected candidate pair.
PauseCandidates(0);
PauseCandidates(1);
CreateChannels(continual_gathering_config, continual_gathering_config);
// We have gathered host, srflx and relay candidates.
EXPECT_TRUE_SIMULATED_WAIT(ep1->saved_candidates_.size() == 3u,
kDefaultTimeout, clock);
ResumeCandidates(0);
ResumeCandidates(1);
ASSERT_TRUE_SIMULATED_WAIT(ep1_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
ASSERT_TRUE_SIMULATED_WAIT(ep2_ch1()->selected_connection() != nullptr,
kDefaultTimeout, clock);
// Test that we have a host-host candidate pair selected and the number of
// candidates signaled to the remote peer stays the same.
auto test_invariants = [this]() {
EXPECT_EQ(LOCAL_PORT_TYPE,
ep1_ch1()->selected_connection()->local_candidate().type());
EXPECT_EQ(LOCAL_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type());
EXPECT_THAT(ep2_ch1()->remote_candidates(), SizeIs(3));
};
test_invariants();
// Set a more restrictive candidate filter at ep1.
ep1->allocator_->SetCandidateFilter(CF_HOST | CF_REFLEXIVE);
SIMULATED_WAIT(false, kDefaultTimeout, clock);
test_invariants();
ep1->allocator_->SetCandidateFilter(CF_HOST);
SIMULATED_WAIT(false, kDefaultTimeout, clock);
test_invariants();
ep1->allocator_->SetCandidateFilter(CF_NONE);
SIMULATED_WAIT(false, kDefaultTimeout, clock);
test_invariants();
}
} // namespace cricket

View File

@ -290,6 +290,16 @@ void PortAllocator::DiscardCandidatePool() {
pooled_sessions_.clear();
}
void PortAllocator::SetCandidateFilter(uint32_t filter) {
CheckRunOnValidThreadIfInitialized();
if (candidate_filter_ == filter) {
return;
}
uint32_t prev_filter = candidate_filter_;
candidate_filter_ = filter;
SignalCandidateFilterChanged(prev_filter, filter);
}
void PortAllocator::GetCandidateStatsFromPooledSessions(
CandidateStatsList* candidate_stats_list) {
CheckRunOnValidThreadAndInitialized();

View File

@ -528,10 +528,22 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> {
return candidate_filter_;
}
void set_candidate_filter(uint32_t filter) {
CheckRunOnValidThreadIfInitialized();
candidate_filter_ = filter;
}
// The new filter value will be populated to future allocation sessions, when
// they are created via CreateSession, and also pooled sessions when one is
// taken via TakePooledSession.
//
// A change in the candidate filter also fires a signal
// |SignalCandidateFilterChanged|, so that objects subscribed to this signal
// can, for example, update the candidate filter for sessions created by this
// allocator and already taken by the object.
//
// Specifically for the session taken by the ICE transport, we currently do
// not support removing candidate pairs formed with local candidates from this
// session that are disabled by the new candidate filter.
void SetCandidateFilter(uint32_t filter);
// Deprecated.
// TODO(qingsi): Remove this after Chromium migrates to the new method.
void set_candidate_filter(uint32_t filter) { SetCandidateFilter(filter); }
bool prune_turn_ports() const {
CheckRunOnValidThreadIfInitialized();
@ -565,6 +577,10 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> {
// Return IceParameters of the pooled sessions.
std::vector<IceParameters> GetPooledIceCredentials();
// Fired when |candidate_filter_| changes.
sigslot::signal2<uint32_t /* prev_filter */, uint32_t /* cur_filter */>
SignalCandidateFilterChanged;
protected:
virtual PortAllocatorSession* CreateSessionInternal(
const std::string& content_name,

View File

@ -100,7 +100,7 @@ TEST_F(PortAllocatorTest, TestDefaults) {
// Call CreateSession and verify that the parameters passed in and the
// candidate filter are applied as expected.
TEST_F(PortAllocatorTest, CreateSession) {
allocator_->set_candidate_filter(cricket::CF_RELAY);
allocator_->SetCandidateFilter(cricket::CF_RELAY);
auto session = CreateSession(kContentName, 1, kIceUfrag, kIcePwd);
ASSERT_NE(nullptr, session);
EXPECT_EQ(cricket::CF_RELAY, session->candidate_filter());
@ -258,7 +258,7 @@ TEST_F(PortAllocatorTest, TakePooledSessionUpdatesIceParameters) {
// session is taken. So a pooled session should gather candidates
// unfiltered until it's returned by TakePooledSession.
TEST_F(PortAllocatorTest, TakePooledSessionUpdatesCandidateFilter) {
allocator_->set_candidate_filter(cricket::CF_RELAY);
allocator_->SetCandidateFilter(cricket::CF_RELAY);
SetConfigurationWithPoolSize(1);
auto peeked_session = GetPooledSession();
ASSERT_NE(nullptr, peeked_session);

View File

@ -31,6 +31,7 @@
using rtc::CreateRandomId;
namespace cricket {
namespace {
enum {
@ -112,9 +113,37 @@ void FilterNetworks(NetworkList* networks, NetworkFilter filter) {
networks->erase(start_to_remove, networks->end());
}
bool IsAllowedByCandidateFilter(const Candidate& c, uint32_t filter) {
// When binding to any address, before sending packets out, the getsockname
// returns all 0s, but after sending packets, it'll be the NIC used to
// send. All 0s is not a valid ICE candidate address and should be filtered
// out.
if (c.address().IsAnyIP()) {
return false;
}
if (c.type() == RELAY_PORT_TYPE) {
return ((filter & CF_RELAY) != 0);
} else if (c.type() == STUN_PORT_TYPE) {
return ((filter & CF_REFLEXIVE) != 0);
} else if (c.type() == LOCAL_PORT_TYPE) {
if ((filter & CF_REFLEXIVE) && !c.address().IsPrivateIP()) {
// We allow host candidates if the filter allows server-reflexive
// candidates and the candidate is a public IP. Because we don't generate
// server-reflexive candidates if they have the same IP as the host
// candidate (i.e. when the host candidate is a public IP), filtering to
// only server-reflexive candidates won't work right when the host
// candidates have public IPs.
return true;
}
return ((filter & CF_HOST) != 0);
}
return false;
}
} // namespace
namespace cricket {
const uint32_t DISABLE_ALL_PHASES =
PORTALLOCATOR_DISABLE_UDP | PORTALLOCATOR_DISABLE_TCP |
PORTALLOCATOR_DISABLE_STUN | PORTALLOCATOR_DISABLE_RELAY;
@ -307,20 +336,57 @@ void BasicPortAllocatorSession::SetCandidateFilter(uint32_t filter) {
if (filter == candidate_filter_) {
return;
}
// We assume the filter will only change from "ALL" to something else.
RTC_DCHECK(candidate_filter_ == CF_ALL);
uint32_t prev_filter = candidate_filter_;
candidate_filter_ = filter;
for (PortData& port : ports_) {
if (!port.has_pairable_candidate()) {
for (PortData& port_data : ports_) {
if (port_data.error() || port_data.pruned()) {
continue;
}
PortData::State cur_state = port_data.state();
bool found_signalable_candidate = false;
bool found_pairable_candidate = false;
cricket::Port* port = port_data.port();
for (const auto& c : port->Candidates()) {
if (!IsStopped() && !IsAllowedByCandidateFilter(c, prev_filter) &&
IsAllowedByCandidateFilter(c, filter)) {
// This candidate was not signaled because of not matching the previous
// filter (see OnCandidateReady below). Let the Port to fire the signal
// again.
//
// Note that
// 1) we would need the Port to enter the state of in-progress of
// gathering to have candidates signaled;
//
// 2) firing the signal would also let the session set the port ready
// if needed, so that we could form candidate pairs with candidates
// from this port;
//
// * See again OnCandidateReady below for 1) and 2).
//
// 3) we only try to resurface candidates if we have not stopped
// getting ports, which is always true for the continual gathering.
if (!found_signalable_candidate) {
found_signalable_candidate = true;
port_data.set_state(PortData::STATE_INPROGRESS);
}
port->SignalCandidateReady(port, c);
}
if (CandidatePairable(c, port)) {
found_pairable_candidate = true;
}
}
// Restore the previous state.
port_data.set_state(cur_state);
// Setting a filter may cause a ready port to become non-ready
// if it no longer has any pairable candidates.
if (absl::c_none_of(port.port()->Candidates(),
[this, &port](const Candidate& candidate) {
return CandidatePairable(candidate, port.port());
})) {
port.set_has_pairable_candidate(false);
//
// Note that we only set for the negative case here, since a port would be
// set to have pairable candidates when it signals a ready candidate, which
// requires the port is still in the progress of gathering/surfacing
// candidates, and would be done in the firing of the signal above.
if (!found_pairable_candidate) {
port_data.set_has_pairable_candidate(false);
}
}
}
@ -597,6 +663,7 @@ void BasicPortAllocatorSession::UpdateIceParametersInternal() {
void BasicPortAllocatorSession::GetPortConfigurations() {
RTC_DCHECK_RUN_ON(network_thread_);
PortConfiguration* config =
new PortConfiguration(allocator_->stun_servers(), username(), password());
@ -633,7 +700,7 @@ void BasicPortAllocatorSession::OnConfigStop() {
if (it->inprogress()) {
// Updating port state to error, which didn't finish allocating candidates
// yet.
it->set_error();
it->set_state(PortData::STATE_ERROR);
send_signal = true;
}
}
@ -1025,7 +1092,7 @@ void BasicPortAllocatorSession::OnPortComplete(Port* port) {
}
// Moving to COMPLETE state.
data->set_complete();
data->set_state(PortData::STATE_COMPLETE);
// Send candidate allocation complete signal if this was the last port.
MaybeSignalCandidatesAllocationDone();
}
@ -1043,7 +1110,7 @@ void BasicPortAllocatorSession::OnPortError(Port* port) {
// SignalAddressError is currently sent from StunPort/TurnPort.
// But this signal itself is generic.
data->set_error();
data->set_state(PortData::STATE_ERROR);
// Send candidate allocation complete signal if this was the last port.
MaybeSignalCandidatesAllocationDone();
}
@ -1051,34 +1118,7 @@ void BasicPortAllocatorSession::OnPortError(Port* port) {
bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) const {
RTC_DCHECK_RUN_ON(network_thread_);
uint32_t filter = candidate_filter_;
// When binding to any address, before sending packets out, the getsockname
// returns all 0s, but after sending packets, it'll be the NIC used to
// send. All 0s is not a valid ICE candidate address and should be filtered
// out.
if (c.address().IsAnyIP()) {
return false;
}
if (c.type() == RELAY_PORT_TYPE) {
return ((filter & CF_RELAY) != 0);
} else if (c.type() == STUN_PORT_TYPE) {
return ((filter & CF_REFLEXIVE) != 0);
} else if (c.type() == LOCAL_PORT_TYPE) {
if ((filter & CF_REFLEXIVE) && !c.address().IsPrivateIP()) {
// We allow host candidates if the filter allows server-reflexive
// candidates and the candidate is a public IP. Because we don't generate
// server-reflexive candidates if they have the same IP as the host
// candidate (i.e. when the host candidate is a public IP), filtering to
// only server-reflexive candidates won't work right when the host
// candidates have public IPs.
return true;
}
return ((filter & CF_HOST) != 0);
}
return false;
return IsAllowedByCandidateFilter(c, candidate_filter_);
}
bool BasicPortAllocatorSession::CandidatePairable(const Candidate& c,
@ -1259,25 +1299,39 @@ void AllocationSequence::DisableEquivalentPhases(rtc::Network* network,
// This can happen if, say, there's a network change event right before an
// application-triggered ICE restart. Hopefully this problem will just go
// away if we get rid of the gathering "phases" though, which is planned.
//
//
// PORTALLOCATOR_DISABLE_UDP is used to disable a Port from gathering the host
// candidate (and srflx candidate if Port::SharedSocket()), and we do not want
// to disable the gathering of these candidates just becaue of an existing
// Port over PROTO_UDP, namely a TurnPort over UDP.
if (absl::c_any_of(session_->ports_,
[this](const BasicPortAllocatorSession::PortData& p) {
return p.port()->Network() == network_ &&
return !p.pruned() && p.port()->Network() == network_ &&
p.port()->GetProtocol() == PROTO_UDP &&
!p.error();
p.port()->Type() == LOCAL_PORT_TYPE && !p.error();
})) {
*flags |= PORTALLOCATOR_DISABLE_UDP;
}
// Similarly we need to check both the protocol used by an existing Port and
// its type.
if (absl::c_any_of(session_->ports_,
[this](const BasicPortAllocatorSession::PortData& p) {
return p.port()->Network() == network_ &&
return !p.pruned() && p.port()->Network() == network_ &&
p.port()->GetProtocol() == PROTO_TCP &&
!p.error();
p.port()->Type() == LOCAL_PORT_TYPE && !p.error();
})) {
*flags |= PORTALLOCATOR_DISABLE_TCP;
}
if (config_ && config) {
if (config_->StunServers() == config->StunServers()) {
// We need to regather srflx candidates if either of the following
// conditions occurs:
// 1. The STUN servers are different from the previous gathering.
// 2. We will regather host candidates, hence possibly inducing new NAT
// bindings.
if (config_->StunServers() == config->StunServers() &&
(*flags & PORTALLOCATOR_DISABLE_UDP)) {
// Already got this STUN servers covered.
*flags |= PORTALLOCATOR_DISABLE_STUN;
}

View File

@ -125,6 +125,15 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
rtc::Thread* network_thread() { return network_thread_; }
rtc::PacketSocketFactory* socket_factory() { return socket_factory_; }
// If the new filter allows new types of candidates compared to the previous
// filter, gathered candidates that were discarded because of not matching the
// previous filter will be signaled if they match the new one.
//
// We do not perform any regathering since the port allocator flags decide
// the type of candidates to gather and the candidate filter only controls the
// signaling of candidates. As a result, with the candidate filter changed
// alone, all newly allowed candidates for signaling should already be
// gathered by the respective cricket::Port.
void SetCandidateFilter(uint32_t filter) override;
void StartGettingPorts() override;
void StopGettingPorts() override;
@ -158,6 +167,14 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
private:
class PortData {
public:
enum State {
STATE_INPROGRESS, // Still gathering candidates.
STATE_COMPLETE, // All candidates allocated and ready for process.
STATE_ERROR, // Error in gathering candidates.
STATE_PRUNED // Pruned by higher priority ports on the same network
// interface. Only TURN ports may be pruned.
};
PortData() {}
PortData(Port* port, AllocationSequence* seq)
: port_(port), sequence_(seq) {}
@ -165,6 +182,7 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
Port* port() const { return port_; }
AllocationSequence* sequence() const { return sequence_; }
bool has_pairable_candidate() const { return has_pairable_candidate_; }
State state() const { return state_; }
bool complete() const { return state_ == STATE_COMPLETE; }
bool error() const { return state_ == STATE_ERROR; }
bool pruned() const { return state_ == STATE_PRUNED; }
@ -187,20 +205,12 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
}
has_pairable_candidate_ = has_pairable_candidate;
}
void set_complete() { state_ = STATE_COMPLETE; }
void set_error() {
RTC_DCHECK(state_ == STATE_INPROGRESS);
state_ = STATE_ERROR;
void set_state(State state) {
RTC_DCHECK(state != STATE_ERROR || state_ == STATE_INPROGRESS);
state_ = state;
}
private:
enum State {
STATE_INPROGRESS, // Still gathering candidates.
STATE_COMPLETE, // All candidates allocated and ready for process.
STATE_ERROR, // Error in gathering candidates.
STATE_PRUNED // Pruned by higher priority ports on the same network
// interface. Only TURN ports may be pruned.
};
Port* port_ = nullptr;
AllocationSequence* sequence_ = nullptr;
bool has_pairable_candidate_ = false;

View File

@ -1266,7 +1266,7 @@ TEST_F(BasicPortAllocatorTest, TestGetAllPortsNoAdapters) {
TEST_F(BasicPortAllocatorTest,
TestDisableAdapterEnumerationWithoutNatRelayTransportOnly) {
ResetWithStunServerNoNat(kStunAddr);
allocator().set_candidate_filter(CF_RELAY);
allocator().SetCandidateFilter(CF_RELAY);
// Expect to see no ports and no candidates.
CheckDisableAdapterEnumeration(0U, rtc::IPAddress(), rtc::IPAddress(),
rtc::IPAddress(), rtc::IPAddress());
@ -1527,7 +1527,7 @@ TEST_F(BasicPortAllocatorTest, TestSessionUsesOwnCandidateFilter) {
AddInterface(kClientAddr);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
// Set candidate filter *after* creating the session. Should have no effect.
allocator().set_candidate_filter(CF_RELAY);
allocator().SetCandidateFilter(CF_RELAY);
session_->StartGettingPorts();
// 7 candidates and 4 ports is what we would normally get (see the
// TestGetAllPorts* tests).
@ -1546,7 +1546,7 @@ TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithRelayOnly) {
AddInterface(kClientAddr);
// GTURN is not configured here.
ResetWithTurnServersNoNat(kTurnUdpIntAddr, rtc::SocketAddress());
allocator().set_candidate_filter(CF_RELAY);
allocator().SetCandidateFilter(CF_RELAY);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@ -1565,7 +1565,7 @@ TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithRelayOnly) {
TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithHostOnly) {
AddInterface(kClientAddr);
allocator().set_flags(PORTALLOCATOR_ENABLE_SHARED_SOCKET);
allocator().set_candidate_filter(CF_HOST);
allocator().SetCandidateFilter(CF_HOST);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@ -1583,7 +1583,7 @@ TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithReflexiveOnly) {
ResetWithStunServerAndNat(kStunAddr);
allocator().set_flags(PORTALLOCATOR_ENABLE_SHARED_SOCKET);
allocator().set_candidate_filter(CF_REFLEXIVE);
allocator().SetCandidateFilter(CF_REFLEXIVE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@ -1602,7 +1602,7 @@ TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithReflexiveOnly) {
TEST_F(BasicPortAllocatorTest, TestCandidateFilterWithReflexiveOnlyAndNoNAT) {
AddInterface(kClientAddr);
allocator().set_flags(PORTALLOCATOR_ENABLE_SHARED_SOCKET);
allocator().set_candidate_filter(CF_REFLEXIVE);
allocator().SetCandidateFilter(CF_REFLEXIVE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
@ -2133,7 +2133,7 @@ TEST_F(BasicPortAllocatorTest, TestSetCandidateFilterAfterCandidatesGathered) {
kDefaultAllocationTimeout, fake_clock);
size_t initial_candidates_size = peeked_session->ReadyCandidates().size();
size_t initial_ports_size = peeked_session->ReadyPorts().size();
allocator_->set_candidate_filter(CF_RELAY);
allocator_->SetCandidateFilter(CF_RELAY);
// Assume that when TakePooledSession is called, the candidate filter will be
// applied to the pooled session. This is tested by PortAllocatorTest.
session_ =
@ -2157,6 +2157,145 @@ TEST_F(BasicPortAllocatorTest, TestSetCandidateFilterAfterCandidatesGathered) {
}
}
// Test that candidates that do not match a previous candidate filter can be
// surfaced if they match the new one after setting the filter value.
TEST_F(BasicPortAllocatorTest,
SurfaceNewCandidatesAfterSetCandidateFilterToAddCandidateTypes) {
// We would still surface a host candidate if the IP is public, even though it
// is disabled by the candidate filter. See
// BasicPortAllocatorSession::CheckCandidateFilter. Use the private address so
// that the srflx candidate is not equivalent to the host candidate.
AddInterface(kPrivateAddr);
ResetWithStunServerAndNat(kStunAddr);
AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
allocator_->set_flags(allocator().flags() |
PORTALLOCATOR_ENABLE_SHARED_SOCKET |
PORTALLOCATOR_DISABLE_TCP);
allocator_->SetCandidateFilter(CF_NONE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
kDefaultAllocationTimeout, fake_clock);
EXPECT_TRUE(candidates_.empty());
EXPECT_TRUE(ports_.empty());
// Surface the relay candidate previously gathered but not signaled.
session_->SetCandidateFilter(CF_RELAY);
ASSERT_EQ_SIMULATED_WAIT(1u, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
EXPECT_EQ(RELAY_PORT_TYPE, candidates_.back().type());
EXPECT_EQ(1u, ports_.size());
// Surface the srflx candidate previously gathered but not signaled.
session_->SetCandidateFilter(CF_RELAY | CF_REFLEXIVE);
ASSERT_EQ_SIMULATED_WAIT(2u, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
EXPECT_EQ(STUN_PORT_TYPE, candidates_.back().type());
EXPECT_EQ(2u, ports_.size());
// Surface the srflx candidate previously gathered but not signaled.
session_->SetCandidateFilter(CF_ALL);
ASSERT_EQ_SIMULATED_WAIT(3u, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
EXPECT_EQ(LOCAL_PORT_TYPE, candidates_.back().type());
EXPECT_EQ(2u, ports_.size());
}
// This is a similar test as
// SurfaceNewCandidatesAfterSetCandidateFilterToAddCandidateTypes, and we
// test the transitions for which the new filter value is not a super set of the
// previous value.
TEST_F(
BasicPortAllocatorTest,
SurfaceNewCandidatesAfterSetCandidateFilterToAllowDifferentCandidateTypes) {
// We would still surface a host candidate if the IP is public, even though it
// is disabled by the candidate filter. See
// BasicPortAllocatorSession::CheckCandidateFilter. Use the private address so
// that the srflx candidate is not equivalent to the host candidate.
AddInterface(kPrivateAddr);
ResetWithStunServerAndNat(kStunAddr);
AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
allocator_->set_flags(allocator().flags() |
PORTALLOCATOR_ENABLE_SHARED_SOCKET |
PORTALLOCATOR_DISABLE_TCP);
allocator_->SetCandidateFilter(CF_NONE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
kDefaultAllocationTimeout, fake_clock);
EXPECT_TRUE(candidates_.empty());
EXPECT_TRUE(ports_.empty());
// Surface the relay candidate previously gathered but not signaled.
session_->SetCandidateFilter(CF_RELAY);
EXPECT_EQ_SIMULATED_WAIT(1u, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
EXPECT_EQ(RELAY_PORT_TYPE, candidates_.back().type());
EXPECT_EQ(1u, ports_.size());
// Surface the srflx candidate previously gathered but not signaled.
session_->SetCandidateFilter(CF_REFLEXIVE);
EXPECT_EQ_SIMULATED_WAIT(2u, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
EXPECT_EQ(STUN_PORT_TYPE, candidates_.back().type());
EXPECT_EQ(2u, ports_.size());
// Surface the host candidate previously gathered but not signaled.
session_->SetCandidateFilter(CF_HOST);
EXPECT_EQ_SIMULATED_WAIT(3u, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
EXPECT_EQ(LOCAL_PORT_TYPE, candidates_.back().type());
// We use a shared socket and cricket::UDPPort handles the srflx candidate.
EXPECT_EQ(2u, ports_.size());
}
// Test that after an allocation session has stopped getting ports, changing the
// candidate filter to allow new types of gathered candidates does not surface
// any candidate.
TEST_F(BasicPortAllocatorTest,
NoCandidateSurfacedWhenUpdatingCandidateFilterIfSessionStopped) {
AddInterface(kPrivateAddr);
ResetWithStunServerAndNat(kStunAddr);
AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
allocator_->set_flags(allocator().flags() |
PORTALLOCATOR_ENABLE_SHARED_SOCKET |
PORTALLOCATOR_DISABLE_TCP);
allocator_->SetCandidateFilter(CF_NONE);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
kDefaultAllocationTimeout, fake_clock);
auto test_invariants = [this]() {
EXPECT_TRUE(candidates_.empty());
EXPECT_TRUE(ports_.empty());
};
test_invariants();
session_->StopGettingPorts();
session_->SetCandidateFilter(CF_RELAY);
SIMULATED_WAIT(false, kDefaultAllocationTimeout, fake_clock);
test_invariants();
session_->SetCandidateFilter(CF_RELAY | CF_REFLEXIVE);
SIMULATED_WAIT(false, kDefaultAllocationTimeout, fake_clock);
test_invariants();
session_->SetCandidateFilter(CF_ALL);
SIMULATED_WAIT(false, kDefaultAllocationTimeout, fake_clock);
test_invariants();
}
TEST_F(BasicPortAllocatorTest, SetStunKeepaliveIntervalForPorts) {
const int pool_size = 1;
const int expected_stun_keepalive_interval = 123;

View File

@ -516,6 +516,7 @@ if (rtc_include_tests) {
"../rtc_base/third_party/base64",
"../rtc_base/third_party/sigslot",
"../system_wrappers:metrics",
"../test:field_trial",
"../test:fileutils",
"//third_party/abseil-cpp/absl/algorithm:container",
"//third_party/abseil-cpp/absl/memory",

View File

@ -5420,7 +5420,7 @@ PeerConnection::InitializePortAllocator_n(
port_allocator_->set_flags(port_allocator_flags);
// No step delay is used while allocating ports.
port_allocator_->set_step_delay(cricket::kMinimumStepDelay);
port_allocator_->set_candidate_filter(
port_allocator_->SetCandidateFilter(
ConvertIceTransportTypeToCandidateFilter(configuration.type));
port_allocator_->set_max_ipv6_networks(configuration.max_ipv6_networks);
@ -5450,7 +5450,7 @@ bool PeerConnection::ReconfigurePortAllocator_n(
webrtc::TurnCustomizer* turn_customizer,
absl::optional<int> stun_candidate_keepalive_interval,
bool have_local_description) {
port_allocator_->set_candidate_filter(
port_allocator_->SetCandidateFilter(
ConvertIceTransportTypeToCandidateFilter(type));
// According to JSEP, after setLocalDescription, changing the candidate pool
// size is not allowed, and changing the set of ICE servers will not result

View File

@ -69,6 +69,7 @@
#include "rtc_base/time_utils.h"
#include "rtc_base/virtual_socket_server.h"
#include "system_wrappers/include/metrics.h"
#include "test/field_trial.h"
#include "test/gmock.h"
namespace webrtc {
@ -560,6 +561,10 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
return event_log_factory_;
}
const cricket::Candidate& last_candidate_gathered() const {
return last_candidate_gathered_;
}
private:
explicit PeerConnectionWrapper(const std::string& debug_name)
: debug_name_(debug_name) {}
@ -942,6 +947,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
return;
}
SendIceMessage(candidate->sdp_mid(), candidate->sdp_mline_index(), ice_sdp);
last_candidate_gathered_ = candidate->candidate();
}
void OnDataChannel(
rtc::scoped_refptr<DataChannelInterface> data_channel) override {
@ -972,6 +978,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver,
SignalingMessageReceiver* signaling_message_receiver_ = nullptr;
int signaling_delay_ms_ = 0;
bool signal_ice_candidates_ = true;
cricket::Candidate last_candidate_gathered_;
// Store references to the video sources we've created, so that we can stop
// them, if required.
@ -5063,6 +5070,72 @@ TEST_P(PeerConnectionIntegrationTest,
EXPECT_LT(0, callee_ice_event_count);
}
TEST_P(PeerConnectionIntegrationTest, RegatherAfterChangingIceTransportType) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-GatherOnCandidateFilterChanged/Enabled/");
static const rtc::SocketAddress turn_server_internal_address{"88.88.88.0",
3478};
static const rtc::SocketAddress turn_server_external_address{"88.88.88.1", 0};
CreateTurnServer(turn_server_internal_address, turn_server_external_address);
webrtc::PeerConnectionInterface::IceServer ice_server;
ice_server.urls.push_back("turn:88.88.88.0:3478");
ice_server.username = "test";
ice_server.password = "test";
PeerConnectionInterface::RTCConfiguration caller_config;
caller_config.servers.push_back(ice_server);
caller_config.type = webrtc::PeerConnectionInterface::kRelay;
caller_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY;
PeerConnectionInterface::RTCConfiguration callee_config;
callee_config.servers.push_back(ice_server);
callee_config.type = webrtc::PeerConnectionInterface::kRelay;
callee_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY;
ASSERT_TRUE(
CreatePeerConnectionWrappersWithConfig(caller_config, callee_config));
// Do normal offer/answer and wait for ICE to complete.
ConnectFakeSignaling();
caller()->AddAudioVideoTracks();
callee()->AddAudioVideoTracks();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
// Since we are doing continual gathering, the ICE transport does not reach
// kIceGatheringComplete (see
// P2PTransportChannel::OnCandidatesAllocationDone), and consequently not
// kIceConnectionComplete.
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
caller()->ice_connection_state(), kDefaultTimeout);
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
callee()->ice_connection_state(), kDefaultTimeout);
// Note that we cannot use the metric
// |WebRTC.PeerConnection.CandidatePairType_UDP| in this test since this
// metric is only populated when we reach kIceConnectionComplete in the
// current implementation.
EXPECT_EQ(cricket::RELAY_PORT_TYPE,
caller()->last_candidate_gathered().type());
EXPECT_EQ(cricket::RELAY_PORT_TYPE,
callee()->last_candidate_gathered().type());
// Loosen the caller's candidate filter.
caller_config = caller()->pc()->GetConfiguration();
caller_config.type = webrtc::PeerConnectionInterface::kAll;
caller()->pc()->SetConfiguration(caller_config);
// We should have gathered a new host candidate.
EXPECT_EQ_WAIT(cricket::LOCAL_PORT_TYPE,
caller()->last_candidate_gathered().type(), kDefaultTimeout);
// Loosen the callee's candidate filter.
callee_config = callee()->pc()->GetConfiguration();
callee_config.type = webrtc::PeerConnectionInterface::kAll;
callee()->pc()->SetConfiguration(callee_config);
EXPECT_EQ_WAIT(cricket::LOCAL_PORT_TYPE,
callee()->last_candidate_gathered().type(), kDefaultTimeout);
}
INSTANTIATE_TEST_SUITE_P(PeerConnectionIntegrationTest,
PeerConnectionIntegrationTest,
Values(SdpSemantics::kPlanB,