Enable shared socket for TurnPort.
In AllocationSequence::OnReadPacket, we now hand the packet to both the TurnPort and StunPort if the remote address matches the server address.
TESTED=AppRtc loopback call generates both turn and stun candidates.
BUG=1746
R=juberti@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/19189004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@7138 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/p2p/base/portallocator.h b/talk/p2p/base/portallocator.h
index a3ff031..5bc389e 100644
--- a/talk/p2p/base/portallocator.h
+++ b/talk/p2p/base/portallocator.h
@@ -55,7 +55,6 @@
PORTALLOCATOR_ENABLE_SHARED_UFRAG = 0x80,
PORTALLOCATOR_ENABLE_SHARED_SOCKET = 0x100,
PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE = 0x200,
- PORTALLOCATOR_ENABLE_TURN_SHARED_SOCKET = 0x400,
};
const uint32 kDefaultPortAllocatorFlags = 0;
@@ -170,7 +169,6 @@
uint32 step_delay() const { return step_delay_; }
void set_step_delay(uint32 delay) {
- ASSERT(delay >= kMinimumStepDelay);
step_delay_ = delay;
}
diff --git a/talk/p2p/base/stunport.h b/talk/p2p/base/stunport.h
index 0a49b67..b3b6d5b 100644
--- a/talk/p2p/base/stunport.h
+++ b/talk/p2p/base/stunport.h
@@ -82,7 +82,7 @@
return socket_->GetLocalAddress();
}
- const ServerAddresses server_addresses() const {
+ const ServerAddresses& server_addresses() const {
return server_addresses_;
}
void
diff --git a/talk/p2p/client/basicportallocator.cc b/talk/p2p/client/basicportallocator.cc
index 6c184b4..eda0b4b 100644
--- a/talk/p2p/client/basicportallocator.cc
+++ b/talk/p2p/client/basicportallocator.cc
@@ -149,9 +149,6 @@
const rtc::PacketTime& packet_time);
void OnPortDestroyed(PortInterface* port);
- void OnResolvedTurnServerAddress(
- TurnPort* port, const rtc::SocketAddress& server_address,
- const rtc::SocketAddress& resolved_server_address);
BasicPortAllocatorSession* session_;
rtc::Network* network_;
@@ -163,8 +160,7 @@
rtc::scoped_ptr<rtc::AsyncPacketSocket> udp_socket_;
// There will be only one udp port per AllocationSequence.
UDPPort* udp_port_;
- // Keeping a map for turn ports keyed with server addresses.
- std::map<rtc::SocketAddress, Port*> turn_ports_;
+ std::vector<TurnPort*> turn_ports_;
int phase_;
};
@@ -1054,26 +1050,15 @@
// don't pass shared socket for ports which will create TCP sockets.
// TODO(mallinath) - Enable shared socket mode for TURN ports. Disabled
// due to webrtc bug https://code.google.com/p/webrtc/issues/detail?id=3537
- if (IsFlagSet(PORTALLOCATOR_ENABLE_TURN_SHARED_SOCKET) &&
+ if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET) &&
relay_port->proto == PROTO_UDP) {
port = TurnPort::Create(session_->network_thread(),
session_->socket_factory(),
network_, udp_socket_.get(),
session_->username(), session_->password(),
*relay_port, config.credentials, config.priority);
- // If we are using shared socket for TURN and udp ports, we need to
- // find a way to demux the packets to the correct port when received.
- // Mapping against server_address is one way of doing this. When packet
- // is received the remote_address will be checked against the map.
- // If server address is not resolved, a signal will be sent from the port
- // after the address is resolved. The map entry will updated with the
- // resolved address when the signal is received from the port.
- if ((*relay_port).address.IsUnresolved()) {
- // If server address is not resolved then listen for signal from port.
- port->SignalResolvedServerAddress.connect(
- this, &AllocationSequence::OnResolvedTurnServerAddress);
- }
- turn_ports_[(*relay_port).address] = port;
+
+ turn_ports_.push_back(port);
// Listen to the port destroyed signal, to allow AllocationSequence to
// remove entrt from it's map.
port->SignalDestroyed.connect(this, &AllocationSequence::OnPortDestroyed);
@@ -1097,51 +1082,45 @@
const rtc::SocketAddress& remote_addr,
const rtc::PacketTime& packet_time) {
ASSERT(socket == udp_socket_.get());
- // If the packet is received from one of the TURN server in the config, then
- // pass down the packet to that port, otherwise it will be handed down to
- // the local udp port.
- Port* port = NULL;
- std::map<rtc::SocketAddress, Port*>::iterator iter =
- turn_ports_.find(remote_addr);
- if (iter != turn_ports_.end()) {
- port = iter->second;
- } else if (udp_port_) {
- port = udp_port_;
+
+ bool turn_port_found = false;
+
+ // Try to find the TurnPort that matches the remote address. Note that the
+ // message could be a STUN binding response if the TURN server is also used as
+ // a STUN server. We don't want to parse every message here to check if it is
+ // a STUN binding response, so we pass the message to TurnPort regardless of
+ // the message type. The TurnPort will just ignore the message since it will
+ // not find any request by transaction ID.
+ for (std::vector<TurnPort*>::const_iterator it = turn_ports_.begin();
+ it != turn_ports_.end(); ++it) {
+ TurnPort* port = *it;
+ if (port->server_address().address == remote_addr) {
+ port->HandleIncomingPacket(socket, data, size, remote_addr, packet_time);
+ turn_port_found = true;
+ break;
+ }
}
- ASSERT(port != NULL);
- if (port) {
- port->HandleIncomingPacket(socket, data, size, remote_addr, packet_time);
+
+ if (udp_port_) {
+ const ServerAddresses& stun_servers = udp_port_->server_addresses();
+
+ // Pass the packet to the UdpPort if there is no matching TurnPort, or if
+ // the TURN server is also a STUN server.
+ if (!turn_port_found ||
+ stun_servers.find(remote_addr) != stun_servers.end()) {
+ udp_port_->HandleIncomingPacket(
+ socket, data, size, remote_addr, packet_time);
+ }
}
}
void AllocationSequence::OnPortDestroyed(PortInterface* port) {
if (udp_port_ == port) {
udp_port_ = NULL;
- } else {
- std::map<rtc::SocketAddress, Port*>::iterator iter;
- for (iter = turn_ports_.begin(); iter != turn_ports_.end(); ++iter) {
- if (iter->second == port) {
- turn_ports_.erase(iter);
- break;
- }
- }
- }
-}
-
-void AllocationSequence::OnResolvedTurnServerAddress(
- TurnPort* port, const rtc::SocketAddress& server_address,
- const rtc::SocketAddress& resolved_server_address) {
- std::map<rtc::SocketAddress, Port*>::iterator iter;
- iter = turn_ports_.find(server_address);
- if (iter == turn_ports_.end()) {
- LOG(LS_INFO) << "TurnPort entry is not found in the map.";
return;
}
- ASSERT(iter->second == port);
- // Remove old entry and then insert using the resolved address as key.
- turn_ports_.erase(iter);
- turn_ports_[resolved_server_address] = port;
+ turn_ports_.erase(std::find(turn_ports_.begin(), turn_ports_.end(), port));
}
// PortConfiguration
diff --git a/talk/p2p/client/portallocator_unittest.cc b/talk/p2p/client/portallocator_unittest.cc
index c383032..e793064 100644
--- a/talk/p2p/client/portallocator_unittest.cc
+++ b/talk/p2p/client/portallocator_unittest.cc
@@ -133,9 +133,32 @@
bool SetPortRange(int min_port, int max_port) {
return allocator_->SetPortRange(min_port, max_port);
}
- rtc::NATServer* CreateNatServer(const SocketAddress& addr,
- rtc::NATType type) {
- return new rtc::NATServer(type, vss_.get(), addr, vss_.get(), addr);
+ void ResetWithNatServer(const rtc::SocketAddress& stun_server) {
+ nat_server_.reset(new rtc::NATServer(
+ rtc::NAT_OPEN_CONE, vss_.get(), kNatAddr, vss_.get(), kNatAddr));
+
+ ServerAddresses stun_servers;
+ stun_servers.insert(stun_server);
+ allocator_.reset(new cricket::BasicPortAllocator(
+ &network_manager_, &nat_socket_factory_, stun_servers));
+ allocator().set_step_delay(cricket::kMinimumStepDelay);
+ }
+
+ void AddTurnServers(const rtc::SocketAddress& udp_turn,
+ const rtc::SocketAddress& tcp_turn) {
+ cricket::RelayServerConfig relay_server(cricket::RELAY_TURN);
+ cricket::RelayCredentials credentials(kTurnUsername, kTurnPassword);
+ relay_server.credentials = credentials;
+
+ if (!udp_turn.IsNil()) {
+ relay_server.ports.push_back(cricket::ProtocolAddress(
+ kTurnUdpIntAddr, cricket::PROTO_UDP, false));
+ }
+ if (!tcp_turn.IsNil()) {
+ relay_server.ports.push_back(cricket::ProtocolAddress(
+ kTurnTcpIntAddr, cricket::PROTO_TCP, false));
+ }
+ allocator_->AddRelay(relay_server);
}
bool CreateSession(int component) {
@@ -254,6 +277,7 @@
rtc::scoped_ptr<rtc::VirtualSocketServer> vss_;
rtc::scoped_ptr<rtc::FirewallSocketServer> fss_;
rtc::SocketServerScope ss_scope_;
+ rtc::scoped_ptr<rtc::NATServer> nat_server_;
rtc::NATSocketFactory nat_factory_;
rtc::BasicPacketSocketFactory nat_socket_factory_;
cricket::TestStunServer stun_server_;
@@ -580,13 +604,8 @@
// Host is behind the NAT.
TEST_F(PortAllocatorTest, TestCandidateFilterWithReflexiveOnly) {
AddInterface(kPrivateAddr);
- rtc::scoped_ptr<rtc::NATServer> nat_server(
- CreateNatServer(kNatAddr, rtc::NAT_OPEN_CONE));
- ServerAddresses stun_servers;
- stun_servers.insert(kStunAddr);
- allocator_.reset(new cricket::BasicPortAllocator(
- &network_manager_, &nat_socket_factory_, stun_servers));
- allocator().set_step_delay(cricket::kMinimumStepDelay);
+ ResetWithNatServer(kStunAddr);
+
allocator().set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET);
allocator().set_candidate_filter(cricket::CF_REFLEXIVE);
@@ -771,13 +790,8 @@
// local candidates as client behind a nat.
TEST_F(PortAllocatorTest, TestSharedSocketWithNat) {
AddInterface(kClientAddr);
- rtc::scoped_ptr<rtc::NATServer> nat_server(
- CreateNatServer(kNatAddr, rtc::NAT_OPEN_CONE));
- ServerAddresses stun_servers;
- stun_servers.insert(kStunAddr);
- allocator_.reset(new cricket::BasicPortAllocator(
- &network_manager_, &nat_socket_factory_, stun_servers));
- allocator_->set_step_delay(cricket::kMinimumStepDelay);
+ ResetWithNatServer(kStunAddr);
+
allocator_->set_flags(allocator().flags() |
cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET);
@@ -799,14 +813,8 @@
turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
AddInterface(kClientAddr);
allocator_.reset(new cricket::BasicPortAllocator(&network_manager_));
- cricket::RelayServerConfig relay_server(cricket::RELAY_TURN);
- cricket::RelayCredentials credentials(kTurnUsername, kTurnPassword);
- relay_server.credentials = credentials;
- relay_server.ports.push_back(cricket::ProtocolAddress(
- kTurnUdpIntAddr, cricket::PROTO_UDP, false));
- relay_server.ports.push_back(cricket::ProtocolAddress(
- kTurnTcpIntAddr, cricket::PROTO_TCP, false));
- allocator_->AddRelay(relay_server);
+
+ AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr);
allocator_->set_step_delay(cricket::kMinimumStepDelay);
allocator_->set_flags(allocator().flags() |
@@ -863,20 +871,10 @@
// stun and turn candidates.
TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurn) {
AddInterface(kClientAddr);
- rtc::scoped_ptr<rtc::NATServer> nat_server(
- CreateNatServer(kNatAddr, rtc::NAT_OPEN_CONE));
- ServerAddresses stun_servers;
- stun_servers.insert(kStunAddr);
- allocator_.reset(new cricket::BasicPortAllocator(
- &network_manager_, &nat_socket_factory_, stun_servers));
- cricket::RelayServerConfig relay_server(cricket::RELAY_TURN);
- cricket::RelayCredentials credentials(kTurnUsername, kTurnPassword);
- relay_server.credentials = credentials;
- relay_server.ports.push_back(cricket::ProtocolAddress(
- kTurnUdpIntAddr, cricket::PROTO_UDP, false));
- allocator_->AddRelay(relay_server);
+ ResetWithNatServer(kStunAddr);
- allocator_->set_step_delay(cricket::kMinimumStepDelay);
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
allocator_->set_flags(allocator().flags() |
cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
@@ -902,6 +900,45 @@
EXPECT_EQ(1U, ports_[1]->Candidates().size());
}
+// Test that when PORTALLOCATOR_ENABLE_SHARED_SOCKET is enabled and the TURN
+// server is also used as the STUN server, we should get 'local', 'stun', and
+// 'relay' candidates.
+TEST_F(PortAllocatorTest, TestSharedSocketWithNatUsingTurnAsStun) {
+ AddInterface(kClientAddr);
+ ResetWithNatServer(kTurnUdpIntAddr);
+ AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress());
+
+ // Must set the step delay to 0 to make sure the relay allocation phase is
+ // started before the STUN candidates are obtained, so that the STUN binding
+ // response is processed when both StunPort and TurnPort exist to reproduce
+ // webrtc issue 3537.
+ allocator_->set_step_delay(0);
+ allocator_->set_flags(allocator().flags() |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG |
+ cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
+ cricket::PORTALLOCATOR_DISABLE_TCP);
+
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+
+ ASSERT_EQ_WAIT(3U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_PRED5(CheckCandidate, candidates_[0],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp", kClientAddr);
+ EXPECT_PRED5(CheckCandidate, candidates_[1],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp",
+ rtc::SocketAddress(kNatAddr.ipaddr(), 0));
+ EXPECT_PRED5(CheckCandidate, candidates_[2],
+ cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp",
+ rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
+ EXPECT_EQ(candidates_[2].related_address(), candidates_[1].address());
+
+ EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout);
+ EXPECT_EQ(3U, candidates_.size());
+ // Local port will be created first and then TURN port.
+ EXPECT_EQ(2U, ports_[0]->Candidates().size());
+ EXPECT_EQ(1U, ports_[1]->Candidates().size());
+}
+
// This test verifies when PORTALLOCATOR_ENABLE_SHARED_SOCKET flag is enabled
// and fail to generate STUN candidate, local UDP candidate is generated
// properly.
