Add config to prune TURN ports

webrtc/p2p/client/basicportallocator_unittest.cc

Index: webrtc/p2p/client/basicportallocator_unittest.cc
diff --git a/webrtc/p2p/client/basicportallocator_unittest.cc b/webrtc/p2p/client/basicportallocator_unittest.cc
index 6d5d1086a9322e26ff9b1cf6d134f4862ce05507..63ea6bb71db6546d64640eb158b138ba3f4a43ed 100644
--- a/webrtc/p2p/client/basicportallocator_unittest.cc
+++ b/webrtc/p2p/client/basicportallocator_unittest.cc
@@ -39,12 +39,15 @@ using rtc::SocketAddress;
 using rtc::Thread;
 
 static const SocketAddress kClientAddr("11.11.11.11", 0);
+static const SocketAddress kClientAddr2("22.22.22.22", 0);
 static const SocketAddress kLoopbackAddr("127.0.0.1", 0);
 static const SocketAddress kPrivateAddr("192.168.1.11", 0);
 static const SocketAddress kPrivateAddr2("192.168.1.12", 0);
 static const SocketAddress kClientIPv6Addr("2401:fa00:4:1000:be30:5bff:fee5:c3",
                                            0);
-static const SocketAddress kClientAddr2("22.22.22.22", 0);
+static const SocketAddress kClientIPv6Addr2(
+    "2401:fa00:4:2000:be30:5bff:fee5:c3",
+    0);
 static const SocketAddress kNatUdpAddr("77.77.77.77", rtc::NAT_SERVER_UDP_PORT);
 static const SocketAddress kNatTcpAddr("77.77.77.77", rtc::NAT_SERVER_TCP_PORT);
 static const SocketAddress kRemoteClientAddr("22.22.22.22", 0);
@@ -56,7 +59,13 @@ static const SocketAddress kRelayTcpExtAddr("99.99.99.3", 5003);
 static const SocketAddress kRelaySslTcpIntAddr("99.99.99.2", 5004);
 static const SocketAddress kRelaySslTcpExtAddr("99.99.99.3", 5005);
 static const SocketAddress kTurnUdpIntAddr("99.99.99.4", 3478);
+static const SocketAddress kTurnUdpIntIPv6Addr(
+    "2402:fb00:4:1000:be30:5bff:fee5:c3",
+    3479);
 static const SocketAddress kTurnTcpIntAddr("99.99.99.5", 3478);
+static const SocketAddress kTurnTcpIntIPv6Addr(
+    "2402:fb00:4:2000:be30:5bff:fee5:c3",
+    3479);
 static const SocketAddress kTurnUdpExtAddr("99.99.99.6", 0);
 
 // Minimum and maximum port for port range tests.
@@ -184,12 +193,10 @@ class BasicPortAllocatorTest : public testing::Test,
     turn_server.credentials = credentials;
 
     if (!udp_turn.IsNil()) {
-      turn_server.ports.push_back(
-          ProtocolAddress(kTurnUdpIntAddr, PROTO_UDP, false));
+      turn_server.ports.push_back(ProtocolAddress(udp_turn, PROTO_UDP, false));
     }
     if (!tcp_turn.IsNil()) {
-      turn_server.ports.push_back(
-          ProtocolAddress(kTurnTcpIntAddr, PROTO_TCP, false));
+      turn_server.ports.push_back(ProtocolAddress(tcp_turn, PROTO_TCP, false));
     }
     allocator_->AddTurnServer(turn_server);
   }
@@ -232,6 +239,8 @@ class BasicPortAllocatorTest : public testing::Test,
         sid, content_name, component, ice_ufrag, ice_pwd);
     session->SignalPortReady.connect(this,
                                      &BasicPortAllocatorTest::OnPortReady);
+    session->SignalPortPruned.connect(this,
+                                      &BasicPortAllocatorTest::OnPortPruned);
     session->SignalCandidatesReady.connect(
         this, &BasicPortAllocatorTest::OnCandidatesReady);
     session->SignalCandidatesAllocationDone.connect(
@@ -251,6 +260,18 @@ class BasicPortAllocatorTest : public testing::Test,
             (pattern.port() != 0 && address.port() == pattern.port()));
   }
 
+  static int CountPorts(const std::vector<PortInterface*>& ports,

[Taylor Brandstetter, 2016/06/29 20:57:36]

nit: CountPortsWithAddress?

[honghaiz3, 2016/06/29 22:53:30]

Ideally we may want CountPortsWithTypeProtocolAndAddress, more like the IOS
function names. 
But with C++ style, it is perhaps not necessary to include all arguments in the
method name, right? 
I add a comment.  

+                        const std::string& type,
+                        ProtocolType protocol,
+                        const SocketAddress& client_addr) {
+    return std::count_if(
+        ports.begin(), ports.end(),
+        [type, protocol, client_addr](PortInterface* port) {
+          return port->Type() == type && port->GetProtocol() == protocol &&
+                 port->Network()->GetBestIP() == client_addr.ipaddr();
+        });
+  }
+
   // Find a candidate and return it.
   static bool FindCandidate(const std::vector<Candidate>& candidates,
                             const std::string& type,
@@ -392,6 +413,15 @@ class BasicPortAllocatorTest : public testing::Test,
     EXPECT_NE(ready_ports.end(),
               std::find(ready_ports.begin(), ready_ports.end(), port));
   }
+  void OnPortPruned(PortAllocatorSession* ses, PortInterface* port) {
+    LOG(LS_INFO) << "OnPortPruned: " << port->ToString();
+    ports_.erase(std::remove(ports_.begin(), ports_.end(), port), ports_.end());
+    // Make sure the pruned port is not in ReadyPorts.
+    auto ready_ports = ses->ReadyPorts();
+    EXPECT_EQ(ready_ports.end(),
+              std::find(ready_ports.begin(), ready_ports.end(), port));
+  }
+
   void OnCandidatesReady(PortAllocatorSession* ses,
                          const std::vector<Candidate>& candidates) {
     for (const Candidate& candidate : candidates) {
@@ -1174,6 +1204,145 @@ TEST_F(BasicPortAllocatorTest, TestSharedSocketWithoutNatUsingTurn) {
   EXPECT_EQ(3U, candidates_.size());
 }
 
+// Test that if prune_turn_ports is set, TCP TurnPort will not
+// be used if UDP TurnPort is used.
+TEST_F(BasicPortAllocatorTest, TestUdpTurnPortDisablesTcpTurnPorts) {
+  turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
+  AddInterface(kClientAddr);
+  allocator_.reset(new BasicPortAllocator(&network_manager_));
+  allocator_->SetConfiguration(allocator_->stun_servers(),
+                               allocator_->turn_servers(), 0, true);
+  AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
+  allocator_->set_step_delay(kMinimumStepDelay);
+  allocator_->set_flags(allocator().flags() |
+                        PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+                        PORTALLOCATOR_DISABLE_TCP);
+
+  EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+  // Only 2 ports (one STUN and one TURN) are actually being used.
+  EXPECT_EQ(2U, session_->ReadyPorts().size());
+  // We have verified that each port, when it is added to |ports_|, it is found
+  // in |ready_ports|, and when it is pruned, it is not found in |ready_ports|,
+  // so we only need to verify the content in one of them.
+  EXPECT_EQ(2U, ports_.size());
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
+  EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientAddr));
+  EXPECT_EQ(0, CountPorts(ports_, "relay", PROTO_TCP, kClientAddr));
+
+  // We don't remove candidates, so the size of |candidates_| will depend on
+  // when the TCP TURN port becomes ready. If it is ready after the UDP TURN
+  // port becomes ready, its candidates will be used there will be 3 candidates.
+  // Otherwise there will be only 2 candidates.

[Taylor Brandstetter, 2016/06/29 20:57:36]

This seems less than optimal. I understand keeping the candidates/connections
when a port is removed because the network is inactive, but if a port is removed
because it's pruned, it seems safe to remove the candidates/connections (unless
the connection is nominated on the controlled side). Do you have any plans to do
this or something similar?

[honghaiz3, 2016/06/29 22:53:30]

We do have a plan to remove candidates for v1.1 as Justin suggested (this is
v1.0).  
I do agree it is fairly easy to remove the candidates, except that we need to
make sure that the other side will handle it correctly (otherwise it should be a
no-op). 

Regarding removing the connection. It is pretty hard to know whether a
connection may be used by the other (controlling) side in the near future. So I
think we'd just leave it there until they are used or removed.

[Taylor Brandstetter, 2016/06/29 23:04:57]

Well in theory, if a connection has not been nominated it should be safe to
assume it's not being used. But I'm fine with doing this further optimization
later.

+  EXPECT_LE(2U, candidates_.size());
+  // There will only be 2 candidates in |ready_candidates| because it only
+  // includes the candidates in the ready ports.
+  const std::vector<Candidate>& ready_candidates = session_->ReadyCandidates();
+  EXPECT_EQ(2U, ready_candidates.size());
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp",
+               rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+}
+
+// Tests that if prune_turn_ports is set, IPv4 TurnPort will not
+// be used if IPv6 TurnPort is used.
+TEST_F(BasicPortAllocatorTest, TestIPv6TurnPortDisablesIPv4TurnPorts) {
+  turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP);
+  // Add two IP addresses on the same interface.
+  AddInterface(kClientAddr, "net1");
+  AddInterface(kClientIPv6Addr, "net1");
+  allocator_.reset(new BasicPortAllocator(&network_manager_));
+  allocator_->SetConfiguration(allocator_->stun_servers(),
+                               allocator_->turn_servers(), 0, true);
+  AddTurnServers(kTurnUdpIntIPv6Addr, rtc::SocketAddress());
+
+  allocator_->set_step_delay(kMinimumStepDelay);
+  allocator_->set_flags(allocator().flags() |
+                        PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+                        PORTALLOCATOR_ENABLE_IPV6 | PORTALLOCATOR_DISABLE_TCP);
+
+  EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+  rtc::Thread::Current()->ProcessMessages(1000);
+  // Three ports (one IPv4 STUN, one IPv6 STUN and one TURN) will be ready.
+  EXPECT_EQ(3U, session_->ReadyPorts().size());
+  EXPECT_EQ(3U, ports_.size());
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr));
+  EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr));
+  EXPECT_EQ(0, CountPorts(ports_, "relay", PROTO_UDP, kClientAddr));
+
+  // We don't remove candidates, so there may be more than 3 elemenets in
+  // |candidates_|, although |ready_candidates| only includes the candidates
+  // in |ready_ports|.
+  EXPECT_LE(3U, candidates_.size());
+  const std::vector<Candidate>& ready_candidates = session_->ReadyCandidates();
+  EXPECT_EQ(3U, ready_candidates.size());
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp",
+               rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+}
+
+// Tests that if prune_turn_ports is set, each network interface
+// will has its own set of TurnPorts based on their priorities.
+TEST_F(BasicPortAllocatorTest, TestEachInterfaceHasItsOwnTurnPorts) {
+  turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
+  turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP);
+  turn_server_.AddInternalSocket(kTurnTcpIntIPv6Addr, PROTO_TCP);
+  // Add two interfaces both having IPv4 and IPv6 addresses.
+  AddInterface(kClientAddr, "net1");
+  AddInterface(kClientIPv6Addr, "net1");
+  AddInterface(kClientAddr2, "net2");
+  AddInterface(kClientIPv6Addr2, "net2");
+  allocator_.reset(new BasicPortAllocator(&network_manager_));
+  allocator_->SetConfiguration(allocator_->stun_servers(),
+                               allocator_->turn_servers(), 0, true);
+  // Have both UDP/TCP and IPv4/IPv6 TURN ports.
+  AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
+  AddTurnServers(kTurnUdpIntIPv6Addr, kTurnTcpIntIPv6Addr);
+
+  allocator_->set_step_delay(kMinimumStepDelay);
+  allocator_->set_flags(allocator().flags() |
+                        PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+                        PORTALLOCATOR_ENABLE_IPV6);
+  EXPECT_TRUE(CreateSession(ICE_CANDIDATE_COMPONENT_RTP));
+  session_->StartGettingPorts();
+  EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+  // 10 ports (4 STUN and 1 TURN ports on each interface) will be ready to use.
+  EXPECT_EQ(10U, session_->ReadyPorts().size());
+  EXPECT_EQ(10U, ports_.size());
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientAddr2));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_UDP, kClientIPv6Addr2));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientAddr));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientAddr2));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientIPv6Addr));
+  EXPECT_EQ(1, CountPorts(ports_, "local", PROTO_TCP, kClientIPv6Addr2));
+  EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr));
+  EXPECT_EQ(1, CountPorts(ports_, "relay", PROTO_UDP, kClientIPv6Addr2));
+
+  // We don't remove candidates, so there may be more than 10 candidates
+  // in |candidates_|.
+  EXPECT_LE(10U, candidates_.size());
+  const std::vector<Candidate>& ready_candidates = session_->ReadyCandidates();
+  EXPECT_EQ(10U, ready_candidates.size());
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientAddr2);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp", kClientIPv6Addr);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "udp",
+               kClientIPv6Addr2);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientAddr);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientAddr2);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp", kClientIPv6Addr);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "local", "tcp",
+               kClientIPv6Addr2);
+  EXPECT_PRED4(HasCandidate, ready_candidates, "relay", "udp",
+               rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+}
+
 // Testing DNS resolve for the TURN server, this will test AllocationSequence
 // handling the unresolved address signal from TurnPort.
 TEST_F(BasicPortAllocatorTest, TestSharedSocketWithServerAddressResolve) {
@@ -1469,7 +1638,7 @@ TEST_F(BasicPortAllocatorTest, TestTransportInformationUpdated) {
   AddInterface(kClientAddr);
   int pool_size = 1;
   allocator_->SetConfiguration(allocator_->stun_servers(),
-                               allocator_->turn_servers(), pool_size);
+                               allocator_->turn_servers(), pool_size, false);
   const PortAllocatorSession* peeked_session = allocator_->GetPooledSession();
   ASSERT_NE(nullptr, peeked_session);
   EXPECT_EQ_WAIT(true, peeked_session->CandidatesAllocationDone(),
@@ -1505,7 +1674,7 @@ TEST_F(BasicPortAllocatorTest, TestSetCandidateFilterAfterCandidatesGathered) {
   AddInterface(kClientAddr);
   int pool_size = 1;
   allocator_->SetConfiguration(allocator_->stun_servers(),
-                               allocator_->turn_servers(), pool_size);
+                               allocator_->turn_servers(), pool_size, false);
   const PortAllocatorSession* peeked_session = allocator_->GetPooledSession();
   ASSERT_NE(nullptr, peeked_session);
   EXPECT_EQ_WAIT(true, peeked_session->CandidatesAllocationDone(),