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

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

Browse Source 
This reverts commit cd8d1cf68e4eeed71fba51c97006a91bfd41813d.

Reason for revert: breaks an internal project

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}

TBR=shampson@webrtc.org,qingsi@webrtc.org,jeroendb@webrtc.org,sukhanov@webrtc.org

Change-Id: Idd51a640e55a612b42fe8b69e05dff57a22d021a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8939
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133581
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27677}
This commit is contained in:
Qingsi Wang 2019-04-17 21:21:54 +00:00 committed by Commit Bot
parent e0ab81c8c6
commit 797ede8e71
11 changed files with 77 additions and 581 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
p2p/base/p2p_transport_channel.cc
View File

@ -15,7 +15,6 @@
#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"
@ -149,14 +148,8 @@ P2PTransportChannel::P2PTransportChannel(
webrtc::BasicRegatheringController::Config regathering_config(
config_.regather_all_networks_interval_range,
config_.regather_on_failed_networks_interval_or_default());
regathering_controller_ =
absl::make_unique<webrtc::BasicRegatheringController>(
regathering_config, this, network_thread_);
RTC_DCHECK(allocator_ != nullptr);
// We populate the change in the candidate filter to the session taken by
// the transport.
allocator_->SignalCandidateFilterChanged.connect(
this, &P2PTransportChannel::OnCandidateFilterChanged);
regathering_controller_.reset(new webrtc::BasicRegatheringController(
regathering_config, this, network_thread_));
ice_event_log_.set_event_log(event_log);
}
@ -1006,14 +999,6 @@ 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;
}
allocator_session()->SetCandidateFilter(cur_filter);
}
void P2PTransportChannel::OnRoleConflict(PortInterface* port) {
SignalRoleConflict(this); // STUN ping will be sent when SetRole is called
// from Transport.

1
p2p/base/p2p_transport_channel.h
View File

@ -292,7 +292,6 @@ 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_|.

195
p2p/base/p2p_transport_channel_unittest.cc
View File

@ -50,7 +50,6 @@ 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.
@ -1607,8 +1606,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_->SetCandidateFilter(CF_RELAY);
GetEndpoint(1)->allocator_->SetCandidateFilter(CF_RELAY);
GetEndpoint(0)->allocator_->set_candidate_filter(CF_RELAY);
GetEndpoint(1)->allocator_->set_candidate_filter(CF_RELAY);
// Setting this allows us to control when SetRemoteIceParameters is called.
set_remote_ice_parameter_source(FROM_CANDIDATE);
CreateChannels();
@ -4588,8 +4587,7 @@ TEST(P2PTransportChannelResolverTest, HostnameCandidateIsResolved) {
EXPECT_CALL(mock_async_resolver_factory, Create())
.WillOnce(Return(&mock_async_resolver));
FakePortAllocator allocator(rtc::Thread::Current(), nullptr);
P2PTransportChannel channel("tn", 0, &allocator,
P2PTransportChannel channel("tn", 0, /*allocator*/ nullptr,
&mock_async_resolver_factory);
Candidate hostname_candidate;
SocketAddress hostname_address("fake.test", 1000);
@ -4890,191 +4888,4 @@ 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) {
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) {
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) {
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

10
p2p/base/port_allocator.cc
View File

@ -290,16 +290,6 @@ 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();

24
p2p/base/port_allocator.h
View File

@ -528,22 +528,10 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> {
return candidate_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); }
void set_candidate_filter(uint32_t filter) {
CheckRunOnValidThreadIfInitialized();
candidate_filter_ = filter;
}
bool prune_turn_ports() const {
CheckRunOnValidThreadIfInitialized();
@ -577,10 +565,6 @@ 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,

4
p2p/base/port_allocator_unittest.cc
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_->SetCandidateFilter(cricket::CF_RELAY);
allocator_->set_candidate_filter(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_->SetCandidateFilter(cricket::CF_RELAY);
allocator_->set_candidate_filter(cricket::CF_RELAY);
SetConfigurationWithPoolSize(1);
auto peeked_session = GetPooledSession();
ASSERT_NE(nullptr, peeked_session);

146
p2p/client/basic_port_allocator.cc
View File

@ -31,7 +31,6 @@
using rtc::CreateRandomId;
namespace cricket {
namespace {
enum {
@ -113,37 +112,9 @@ 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;
@ -336,57 +307,20 @@ void BasicPortAllocatorSession::SetCandidateFilter(uint32_t filter) {
if (filter == candidate_filter_) {
return;
}
uint32_t prev_filter = candidate_filter_;
// We assume the filter will only change from "ALL" to something else.
RTC_DCHECK(candidate_filter_ == CF_ALL);
candidate_filter_ = filter;
for (PortData& port_data : ports_) {
if (port_data.error() || port_data.pruned()) {
for (PortData& port : ports_) {
if (!port.has_pairable_candidate()) {
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.
//
// 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);
if (absl::c_none_of(port.port()->Candidates(),
[this, &port](const Candidate& candidate) {
return CandidatePairable(candidate, port.port());
})) {
port.set_has_pairable_candidate(false);
}
}
}
@ -663,7 +597,6 @@ void BasicPortAllocatorSession::UpdateIceParametersInternal() {
void BasicPortAllocatorSession::GetPortConfigurations() {
RTC_DCHECK_RUN_ON(network_thread_);
PortConfiguration* config =
new PortConfiguration(allocator_->stun_servers(), username(), password());
@ -700,7 +633,7 @@ void BasicPortAllocatorSession::OnConfigStop() {
if (it->inprogress()) {
// Updating port state to error, which didn't finish allocating candidates
// yet.
it->set_state(PortData::STATE_ERROR);
it->set_error();
send_signal = true;
}
}
@ -1092,7 +1025,7 @@ void BasicPortAllocatorSession::OnPortComplete(Port* port) {
}
// Moving to COMPLETE state.
data->set_state(PortData::STATE_COMPLETE);
data->set_complete();
// Send candidate allocation complete signal if this was the last port.
MaybeSignalCandidatesAllocationDone();
}
@ -1110,7 +1043,7 @@ void BasicPortAllocatorSession::OnPortError(Port* port) {
// SignalAddressError is currently sent from StunPort/TurnPort.
// But this signal itself is generic.
data->set_state(PortData::STATE_ERROR);
data->set_error();
// Send candidate allocation complete signal if this was the last port.
MaybeSignalCandidatesAllocationDone();
}
@ -1118,7 +1051,34 @@ void BasicPortAllocatorSession::OnPortError(Port* port) {
bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) const {
RTC_DCHECK_RUN_ON(network_thread_);
return IsAllowedByCandidateFilter(c, candidate_filter_);
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;
}
bool BasicPortAllocatorSession::CandidatePairable(const Candidate& c,
@ -1299,39 +1259,25 @@ 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.pruned() && p.port()->Network() == network_ &&
return p.port()->Network() == network_ &&
p.port()->GetProtocol() == PROTO_UDP &&
p.port()->Type() == LOCAL_PORT_TYPE && !p.error();
!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.pruned() && p.port()->Network() == network_ &&
return p.port()->Network() == network_ &&
p.port()->GetProtocol() == PROTO_TCP &&
p.port()->Type() == LOCAL_PORT_TYPE && !p.error();
!p.error();
})) {
*flags |= PORTALLOCATOR_DISABLE_TCP;
}
if (config_ && config) {
// 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)) {
if (config_->StunServers() == config->StunServers()) {
// Already got this STUN servers covered.
*flags |= PORTALLOCATOR_DISABLE_STUN;
}

32
p2p/client/basic_port_allocator.h
View File

@ -125,15 +125,6 @@ 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;
@ -167,14 +158,6 @@ 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) {}
@ -182,7 +165,6 @@ 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; }
@ -205,12 +187,20 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession,
}
has_pairable_candidate_ = has_pairable_candidate;
}
void set_state(State state) {
RTC_DCHECK(state != STATE_ERROR || state_ == STATE_INPROGRESS);
state_ = state;
void set_complete() { state_ = STATE_COMPLETE; }
void set_error() {
RTC_DCHECK(state_ == STATE_INPROGRESS);
state_ = STATE_ERROR;
}
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;

153
p2p/client/basic_port_allocator_unittest.cc
View File

@ -1266,7 +1266,7 @@ TEST_F(BasicPortAllocatorTest, TestGetAllPortsNoAdapters) {
TEST_F(BasicPortAllocatorTest,
TestDisableAdapterEnumerationWithoutNatRelayTransportOnly) {
ResetWithStunServerNoNat(kStunAddr);
allocator().SetCandidateFilter(CF_RELAY);
allocator().set_candidate_filter(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().SetCandidateFilter(CF_RELAY);
allocator().set_candidate_filter(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().SetCandidateFilter(CF_RELAY);
allocator().set_candidate_filter(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().SetCandidateFilter(CF_HOST);
allocator().set_candidate_filter(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().SetCandidateFilter(CF_REFLEXIVE);
allocator().set_candidate_filter(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().SetCandidateFilter(CF_REFLEXIVE);
allocator().set_candidate_filter(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_->SetCandidateFilter(CF_RELAY);
allocator_->set_candidate_filter(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,145 +2157,6 @@ 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;

4
pc/peer_connection.cc
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_->SetCandidateFilter(
port_allocator_->set_candidate_filter(
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_->SetCandidateFilter(
port_allocator_->set_candidate_filter(
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

70
pc/peer_connection_integrationtest.cc
View File

@ -560,10 +560,6 @@ 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) {}
@ -946,7 +942,6 @@ 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 {
@ -977,7 +972,6 @@ 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.
@ -5069,70 +5063,6 @@ TEST_P(PeerConnectionIntegrationTest,
EXPECT_LT(0, callee_ice_event_count);
}
TEST_P(PeerConnectionIntegrationTest, RegatherAfterChangingIceTransportType) {
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,