Revert "Reland "Enable any address ports by default.""
This reverts commit b89ac622f3f5a7bb065a08cb1efba10a0e8cae23.
Reason for revert: Speculative revert.
Original change's description:
> Reland "Enable any address ports by default."
>
> This reverts commit 1165949341b6f61c5d728999bfbdaf68fd5c15aa.
>
> Reason for revert: Speculative reland (the revert breaks a downstream project).
>
> Original change's description:
> > Revert "Reland "Enable any address ports by default.""
> >
> > This reverts commit ac5bbd940ed31f8a58095952f4dcdcbb1b58203c.
> >
> > Reason for revert: Speculative revert, possibly breaking downstream projects
> >
> > Original change's description:
> > > Reland "Enable any address ports by default."
> > >
> > > This reverts commit 056a68da896d9a578b9ea83e56d261648ea0adc6.
> > >
> > > Reason for revert: Trying to reland.
> > >
> > > Original change's description:
> > > > Revert "Enable any address ports by default."
> > > >
> > > > This reverts commit f04148c810aad2a0809dc8978650c55308381c47.
> > > >
> > > > Reason for revert: Speculative revert. I suspect this is breaking a
> > > > downstream test (I'll reland if it is not the culprit).
> > > >
> > > > Original change's description:
> > > > > Enable any address ports by default.
> > > > >
> > > > > Ports not bound to any specific network interface are allocated by
> > > > > default. These any address ports are pruned after allocation,
> > > > > conditional on the allocation results of normal ports that are bound to
> > > > > the enumerated interfaces.
> > > > >
> > > > > Bug: webrtc:9313
> > > > > Change-Id: I3ce12eeab0cf3547224e5f8c188d061fc530e145
> > > > > Reviewed-on: https://webrtc-review.googlesource.com/78383
> > > > > Commit-Queue: Qingsi Wang <qingsi@google.com>
> > > > > Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> > > > > Cr-Commit-Position: refs/heads/master@{#23673}
> > > >
> > > > TBR=deadbeef@webrtc.org,pthatcher@webrtc.org,qingsi@google.com
> > > >
> > > > Change-Id: I3b3dc42c7de46d198d4b9c270020dcf1100dd907
> > > > No-Presubmit: true
> > > > No-Tree-Checks: true
> > > > No-Try: true
> > > > Bug: webrtc:9313
> > > > Reviewed-on: https://webrtc-review.googlesource.com/84300
> > > > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> > > > Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> > > > Cr-Commit-Position: refs/heads/master@{#23678}
> > >
> > > TBR=deadbeef@webrtc.org,mbonadei@webrtc.org,pthatcher@webrtc.org,qingsi@google.com
> > >
> > > # Not skipping CQ checks because original CL landed > 1 day ago.
> > >
> > > Bug: webrtc:9313
> > > Change-Id: I98442346babb5d8953d37dc5825efaf79804ed7f
> > > Reviewed-on: https://webrtc-review.googlesource.com/85000
> > > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> > > Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
> > > Cr-Commit-Position: refs/heads/master@{#23720}
> >
> > TBR=deadbeef@webrtc.org,mbonadei@webrtc.org,pthatcher@webrtc.org,qingsi@google.com,qingsi@webrtc.org
> >
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> >
> > Bug: webrtc:9313
> > Change-Id: Ie5da4133a371532f717af144f183e299e759f152
> > Reviewed-on: https://webrtc-review.googlesource.com/95340
> > Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> > Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#24374}
>
> TBR=deadbeef@webrtc.org,mbonadei@webrtc.org,pthatcher@webrtc.org,qingsi@google.com,qingsi@webrtc.org
>
> Change-Id: I52bf487d441ce8ccedee7e348b9ed9ade0fd9d1c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:9313
> Reviewed-on: https://webrtc-review.googlesource.com/95440
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#24379}
TBR=deadbeef@webrtc.org,mbonadei@webrtc.org,pthatcher@webrtc.org,qingsi@google.com,qingsi@webrtc.org
Change-Id: I6db41f092c55be74f6594eb729ad5f15c718fe34
Bug: webrtc:9313
Reviewed-on: https://webrtc-review.googlesource.com/95520
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24412}diff --git a/p2p/client/basicportallocator_unittest.cc b/p2p/client/basicportallocator_unittest.cc
index b3e52ff..b49be8a 100644
--- a/p2p/client/basicportallocator_unittest.cc
+++ b/p2p/client/basicportallocator_unittest.cc
@@ -1150,20 +1150,15 @@
allocator_->set_step_delay(kDefaultStepDelay);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
- // Host and STUN candidates from kClientAddr.
ASSERT_EQ_SIMULATED_WAIT(2U, candidates_.size(), 1000, fake_clock);
- // UDP and STUN ports on kClientAddr.
EXPECT_EQ(2U, ports_.size());
- // Host, STUN and relay candidates from kClientAddr.
ASSERT_EQ_SIMULATED_WAIT(6U, candidates_.size(), 2000, fake_clock);
- // UDP, STUN and relay ports on kClientAddr.
EXPECT_EQ(3U, ports_.size());
EXPECT_TRUE(HasCandidate(candidates_, "relay", "udp", kRelayUdpIntAddr));
EXPECT_TRUE(HasCandidate(candidates_, "relay", "udp", kRelayUdpExtAddr));
EXPECT_TRUE(HasCandidate(candidates_, "relay", "tcp", kRelayTcpIntAddr));
EXPECT_TRUE(
HasCandidate(candidates_, "relay", "ssltcp", kRelaySslTcpIntAddr));
- // One more TCP candidate from kClientAddr.
ASSERT_EQ_SIMULATED_WAIT(7U, candidates_.size(), 1500, fake_clock);
EXPECT_TRUE(HasCandidate(candidates_, "local", "tcp", kClientAddr));
EXPECT_EQ(4U, ports_.size());
@@ -1473,20 +1468,17 @@
// Testing STUN timeout.
TEST_F(BasicPortAllocatorTest, TestGetAllPortsNoUdpAllowed) {
fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kClientAddr);
- fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kAnyAddr);
AddInterface(kClientAddr);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
EXPECT_EQ_SIMULATED_WAIT(2U, candidates_.size(), kDefaultAllocationTimeout,
fake_clock);
- // UDP and TCP ports on kClientAddr.
EXPECT_EQ(2U, ports_.size());
EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr));
EXPECT_TRUE(HasCandidate(candidates_, "local", "tcp", kClientAddr));
// RelayPort connection timeout is 3sec. TCP connection with RelayServer
// will be tried after about 3 seconds.
EXPECT_EQ_SIMULATED_WAIT(6U, candidates_.size(), 3500, fake_clock);
- // UDP, TCP and relay ports on kClientAddr.
EXPECT_EQ(3U, ports_.size());
EXPECT_TRUE(HasCandidate(candidates_, "relay", "udp", kRelayUdpIntAddr));
EXPECT_TRUE(HasCandidate(candidates_, "relay", "tcp", kRelayTcpIntAddr));
@@ -1993,14 +1985,12 @@
PORTALLOCATOR_DISABLE_TCP |
PORTALLOCATOR_ENABLE_SHARED_SOCKET);
fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kClientAddr);
- fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kAnyAddr);
AddInterface(kClientAddr);
ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
session_->StartGettingPorts();
- ASSERT_EQ_SIMULATED_WAIT(1U, candidates_.size(), kDefaultAllocationTimeout,
+ ASSERT_EQ_SIMULATED_WAIT(1U, ports_.size(), kDefaultAllocationTimeout,
fake_clock);
- // UDP ports on kClientAddr.
- EXPECT_EQ(1U, ports_.size());
+ EXPECT_EQ(1U, candidates_.size());
EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr));
// STUN timeout is 9.5sec. We need to wait to get candidate done signal.
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_, kStunTimeoutMs,
@@ -2008,137 +1998,6 @@
EXPECT_EQ(1U, candidates_.size());
}
-// Test that any address ports that are redundant do not surface.
-TEST_F(BasicPortAllocatorTest, RedundantAnyAddressPortsDoNotSurface) {
- allocator().set_flags(allocator().flags() | PORTALLOCATOR_DISABLE_RELAY |
- PORTALLOCATOR_DISABLE_TCP |
- PORTALLOCATOR_ENABLE_SHARED_SOCKET);
- AddInterface(kClientAddr);
- // The any address ports will be duplicates of kClientAddr.
- vss_->SetAlternativeLocalAddress(kAnyAddr.ipaddr(), kClientAddr.ipaddr());
- ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
- session_->StartGettingPorts();
- EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_, kStunTimeoutMs,
- fake_clock);
- EXPECT_EQ(1U, ports_.size());
- EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
-}
-
-// Test that candidates from the any address ports are not pruned if the
-// explicit binding to enumerated networks fails.
-TEST_F(BasicPortAllocatorTest,
- CandidatesFromAnyAddressPortsCanSurfaceWhenExplicitBindingFails) {
- allocator().set_flags(allocator().flags() | PORTALLOCATOR_DISABLE_RELAY |
- PORTALLOCATOR_DISABLE_TCP |
- PORTALLOCATOR_ENABLE_SHARED_SOCKET);
- AddInterface(kClientAddr);
- fss_->SetUnbindableIps({kClientAddr.ipaddr()});
- // The any address ports will be duplicates of kClientAddr, but the explict
- // binding will fail.
- vss_->SetAlternativeLocalAddress(kAnyAddr.ipaddr(), kClientAddr.ipaddr());
- ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
- session_->StartGettingPorts();
- EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_, kStunTimeoutMs,
- fake_clock);
- EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kAnyAddr));
- EXPECT_EQ(1U, candidates_.size());
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr));
-}
-
-// Test that for an endpoint whose network enumeration only reveals one address
-// (kClientAddr), it can observe a different address when binding to the "any"
-// address. BasicPortAllocator should detect this and surface candidates for
-// each address.
-TEST_F(BasicPortAllocatorTest,
- CandidatesFromAnyAddressPortsCanSurfaceIfNotRedundant) {
- allocator().set_flags(allocator().flags() | PORTALLOCATOR_DISABLE_RELAY |
- PORTALLOCATOR_DISABLE_TCP |
- PORTALLOCATOR_ENABLE_SHARED_SOCKET);
- AddInterface(kClientAddr);
- // When bound to the any address, the port allocator should discover the
- // alternative local address.
- vss_->SetAlternativeLocalAddress(kAnyAddr.ipaddr(), kClientAddr2.ipaddr());
- ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
- session_->StartGettingPorts();
- EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_, kStunTimeoutMs,
- fake_clock);
- EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kAnyAddr));
- EXPECT_EQ(2U, candidates_.size());
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr));
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr2));
-}
-
-// Test that any address ports and their candidates are eventually signaled
-// after the maximum wait interval for the completion of candidate allocation,
-// when the any address ports and candidates are not redundant.
-TEST_F(BasicPortAllocatorTest,
- GetAnyAddressPortsAfterMaximumWaitForCandidateAllocationDone) {
- ResetWithTurnServersNoNat(kTurnUdpIntAddr, rtc::SocketAddress());
- AddInterface(kClientAddr);
- vss_->SetAlternativeLocalAddress(kAnyAddr.ipaddr(), kClientAddr2.ipaddr());
- // STUN binding request and TURN allocation request will time out.
- fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kClientAddr);
- ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
- session_->StartGettingPorts();
- SIMULATED_WAIT(false, kMaxWaitMsBeforeSignalingAnyAddressPortsAndCandidates,
- fake_clock);
- EXPECT_EQ(2U, candidates_.size());
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr));
- EXPECT_TRUE(HasCandidate(candidates_, "local", "tcp", kClientAddr));
- SIMULATED_WAIT(false, 1, fake_clock);
- EXPECT_FALSE(candidate_allocation_done_);
- // Candidates from the any address ports.
- EXPECT_EQ(6U, candidates_.size());
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr2));
- EXPECT_TRUE(HasCandidate(candidates_, "stun", "udp", kClientAddr2));
- EXPECT_TRUE(HasCandidate(candidates_, "relay", "udp", kTurnUdpExtAddr));
- EXPECT_TRUE(HasCandidate(candidates_, "local", "tcp", kClientAddr2));
-}
-
-// Test that the TCP port with the wildcard address is signaled if no ports are
-// bound to enuemrated networks.
-TEST_F(BasicPortAllocatorTest,
- GetAnyAddressTcpPortWhenNoPortsBoundToEnumeratedNetworks) {
- ResetWithTurnServersNoNat(kTurnUdpIntAddr, rtc::SocketAddress());
- AddInterface(kClientAddr);
- fss_->SetUnbindableIps({kClientAddr.ipaddr()});
- ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
- session_->StartGettingPorts();
- SIMULATED_WAIT(false,
- kMaxWaitMsBeforeSignalingAnyAddressPortsAndCandidates + 1,
- fake_clock);
- EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kAnyAddr));
-}
-
-// Test that the STUN candidate from the any address port can still surface, if
-// it is gathered after this port is signaled, .
-TEST_F(BasicPortAllocatorTest,
- StunCandidateFromAnyAddressPortsGatheredLateCanBeSignaled) {
- ResetWithTurnServersNoNat(kTurnUdpIntAddr, rtc::SocketAddress());
- AddInterface(kClientAddr);
- vss_->SetAlternativeLocalAddress(kAnyAddr.ipaddr(), kClientAddr2.ipaddr());
- fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kClientAddr);
- fss_->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kClientAddr2);
- ASSERT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
- session_->StartGettingPorts();
- SIMULATED_WAIT(false,
- kMaxWaitMsBeforeSignalingAnyAddressPortsAndCandidates + 1000,
- fake_clock);
- EXPECT_FALSE(candidate_allocation_done_);
- EXPECT_EQ(4U, candidates_.size());
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr));
- EXPECT_TRUE(HasCandidate(candidates_, "local", "tcp", kClientAddr));
- EXPECT_TRUE(HasCandidate(candidates_, "local", "udp", kClientAddr2));
- EXPECT_TRUE(HasCandidate(candidates_, "local", "tcp", kClientAddr2));
- fss_->ClearRules();
- EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
- kDefaultAllocationTimeout, fake_clock);
- EXPECT_EQ(7U, candidates_.size());
- EXPECT_TRUE(HasCandidate(candidates_, "stun", "udp", kClientAddr));
- EXPECT_TRUE(HasCandidate(candidates_, "stun", "udp", kClientAddr2));
- EXPECT_TRUE(HasCandidate(candidates_, "relay", "udp", kTurnUdpExtAddr));
-}
-
// Test that when the NetworkManager doesn't have permission to enumerate
// adapters, the PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION is specified
// automatically.
