diff --git a/webrtc/base/network.h b/webrtc/base/network.h index a81bcabf66..680c005ea2 100644 --- a/webrtc/base/network.h +++ b/webrtc/base/network.h @@ -337,6 +337,8 @@ class Network { void set_ignored(bool ignored) { ignored_ = ignored; } AdapterType type() const { return type_; } + void set_type(AdapterType type) { type_ = type; } + int preference() const { return preference_; } void set_preference(int preference) { preference_ = preference; } diff --git a/webrtc/p2p/base/stunport.cc b/webrtc/p2p/base/stunport.cc index 8f37dd5218..e871a6c0cc 100644 --- a/webrtc/p2p/base/stunport.cc +++ b/webrtc/p2p/base/stunport.cc @@ -24,21 +24,23 @@ namespace cricket { // TODO: Move these to a common place (used in relayport too) const int KEEPALIVE_DELAY = 10 * 1000; // 10 seconds - sort timeouts -const int RETRY_TIMEOUT = 50 * 1000; // ICE says 50 secs -// Stop sending STUN binding requests after this amount of time -// (in milliseconds) because the connection binding requests should keep -// the NAT binding alive. -const int KEEP_ALIVE_TIMEOUT = 2 * 60 * 1000; // 2 minutes +const int RETRY_TIMEOUT = 50 * 1000; // 50 seconds +// Lifetime chosen for STUN ports on low-cost networks. +const int INFINITE_LIFETIME = -1; +// Lifetime for STUN ports on high-cost networks: 2 minutes +const int HIGH_COST_PORT_KEEPALIVE_LIFETIME = 2 * 60 * 1000; // Handles a binding request sent to the STUN server. class StunBindingRequest : public StunRequest { public: StunBindingRequest(UDPPort* port, const rtc::SocketAddress& addr, - uint32_t deadline) - : port_(port), server_addr_(addr), deadline_(deadline) { - start_time_ = rtc::Time(); - } + uint32_t start_time, + int lifetime) + : port_(port), + server_addr_(addr), + start_time_(start_time), + lifetime_(lifetime) {} virtual ~StunBindingRequest() { } @@ -62,11 +64,10 @@ class StunBindingRequest : public StunRequest { port_->OnStunBindingRequestSucceeded(server_addr_, addr); } - // We will do a keep-alive regardless of whether this request succeeds. - // It will be stopped after |deadline_| mostly to conserve the battery life. - if (rtc::Time() <= deadline_) { + // The keep-alive requests will be stopped after its lifetime has passed. + if (WithinLifetime(rtc::Time())) { port_->requests_.SendDelayed( - new StunBindingRequest(port_, server_addr_, deadline_), + new StunBindingRequest(port_, server_addr_, start_time_, lifetime_), port_->stun_keepalive_delay()); } } @@ -77,34 +78,41 @@ class StunBindingRequest : public StunRequest { LOG(LS_ERROR) << "Bad allocate response error code"; } else { LOG(LS_ERROR) << "Binding error response:" - << " class=" << attr->eclass() - << " number=" << attr->number() - << " reason='" << attr->reason() << "'"; + << " class=" << attr->eclass() + << " number=" << attr->number() << " reason='" + << attr->reason() << "'"; } port_->OnStunBindingOrResolveRequestFailed(server_addr_); uint32_t now = rtc::Time(); - if (now <= deadline_ && rtc::TimeDiff(now, start_time_) <= RETRY_TIMEOUT) { + if (WithinLifetime(now) && + rtc::TimeDiff(now, start_time_) < RETRY_TIMEOUT) { port_->requests_.SendDelayed( - new StunBindingRequest(port_, server_addr_, deadline_), + new StunBindingRequest(port_, server_addr_, start_time_, lifetime_), port_->stun_keepalive_delay()); } } - virtual void OnTimeout() override { LOG(LS_ERROR) << "Binding request timed out from " - << port_->GetLocalAddress().ToSensitiveString() - << " (" << port_->Network()->name() << ")"; + << port_->GetLocalAddress().ToSensitiveString() << " (" + << port_->Network()->name() << ")"; port_->OnStunBindingOrResolveRequestFailed(server_addr_); } private: + // Returns true if |now| is within the lifetime of the request (a negative + // lifetime means infinite). + bool WithinLifetime(uint32_t now) const { + return lifetime_ < 0 || rtc::TimeDiff(now, start_time_) <= lifetime_; + } UDPPort* port_; const rtc::SocketAddress server_addr_; + uint32_t start_time_; - uint32_t deadline_; + // The time duration for which this request will be rescheduled. + int lifetime_; }; UDPPort::AddressResolver::AddressResolver( @@ -212,6 +220,12 @@ UDPPort::UDPPort(rtc::Thread* thread, } bool UDPPort::Init() { + // If this is a zero-cost network, it will keep on sending STUN binding + // requests indefinitely to keep the NAT binding alive. Otherwise, stop + // sending STUN binding requests after HIGH_COST_PORT_KEEPALIVE_LIFETIME. + stun_keepalive_lifetime_ = (network_cost() == 0) + ? INFINITE_LIFETIME + : HIGH_COST_PORT_KEEPALIVE_LIFETIME; if (!SharedSocket()) { ASSERT(socket_ == NULL); socket_ = socket_factory()->CreateUdpSocket( @@ -397,8 +411,8 @@ void UDPPort::SendStunBindingRequest(const rtc::SocketAddress& stun_addr) { } else if (socket_->GetState() == rtc::AsyncPacketSocket::STATE_BOUND) { // Check if |server_addr_| is compatible with the port's ip. if (IsCompatibleAddress(stun_addr)) { - requests_.Send(new StunBindingRequest(this, stun_addr, - rtc::Time() + KEEP_ALIVE_TIMEOUT)); + requests_.Send(new StunBindingRequest(this, stun_addr, rtc::Time(), + stun_keepalive_lifetime_)); } else { // Since we can't send stun messages to the server, we should mark this // port ready. diff --git a/webrtc/p2p/base/stunport.h b/webrtc/p2p/base/stunport.h index ecf61a782d..a0eba51cd4 100644 --- a/webrtc/p2p/base/stunport.h +++ b/webrtc/p2p/base/stunport.h @@ -106,6 +106,16 @@ class UDPPort : public Port { return stun_keepalive_delay_; } + // Visible for testing. + int stun_keepalive_lifetime() const { return stun_keepalive_lifetime_; } + void set_stun_keepalive_lifetime(int lifetime) { + stun_keepalive_lifetime_ = lifetime; + } + // Returns true if there is a pending request with type |msg_type|. + bool HasPendingRequest(int msg_type) { + return requests_.HasRequest(msg_type); + } + protected: UDPPort(rtc::Thread* thread, rtc::PacketSocketFactory* factory, @@ -217,6 +227,7 @@ class UDPPort : public Port { rtc::scoped_ptr resolver_; bool ready_; int stun_keepalive_delay_; + int stun_keepalive_lifetime_; // This is true by default and false when // PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE is specified. diff --git a/webrtc/p2p/base/stunport_unittest.cc b/webrtc/p2p/base/stunport_unittest.cc index 037d448b9e..1926b93279 100644 --- a/webrtc/p2p/base/stunport_unittest.cc +++ b/webrtc/p2p/base/stunport_unittest.cc @@ -31,6 +31,8 @@ static const SocketAddress kBadHostnameAddr("not-a-real-hostname", 5000); static const int kTimeoutMs = 10000; // stun prio = 100 << 24 | 30 (IPV4) << 8 | 256 - 0 static const uint32_t kStunCandidatePriority = 1677729535; +static const int kInfiniteLifetime = -1; +static const int kHighCostPortKeepaliveLifetimeMs = 2 * 60 * 1000; // Tests connecting a StunPort to a fake STUN server (cricket::StunServer) // TODO: Use a VirtualSocketServer here. We have to use a @@ -44,17 +46,23 @@ class StunPortTest : public testing::Test, ss_scope_(ss_.get()), network_("unittest", "unittest", rtc::IPAddress(INADDR_ANY), 32), socket_factory_(rtc::Thread::Current()), - stun_server_1_(cricket::TestStunServer::Create( - rtc::Thread::Current(), kStunAddr1)), - stun_server_2_(cricket::TestStunServer::Create( - rtc::Thread::Current(), kStunAddr2)), - done_(false), error_(false), stun_keepalive_delay_(0) { - } + stun_server_1_(cricket::TestStunServer::Create(rtc::Thread::Current(), + kStunAddr1)), + stun_server_2_(cricket::TestStunServer::Create(rtc::Thread::Current(), + kStunAddr2)), + done_(false), + error_(false), + stun_keepalive_delay_(0), + stun_keepalive_lifetime_(-1) {} - const cricket::Port* port() const { return stun_port_.get(); } + cricket::UDPPort* port() const { return stun_port_.get(); } bool done() const { return done_; } bool error() const { return error_; } + void SetNetworkType(rtc::AdapterType adapter_type) { + network_.set_type(adapter_type); + } + void CreateStunPort(const rtc::SocketAddress& server_addr) { ServerAddresses stun_servers; stun_servers.insert(server_addr); @@ -67,6 +75,11 @@ class StunPortTest : public testing::Test, kLocalAddr.ipaddr(), 0, 0, rtc::CreateRandomString(16), rtc::CreateRandomString(22), stun_servers, std::string())); stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_); + // If |stun_keepalive_lifetime_| is negative, let the stun port + // choose its lifetime from the network type. + if (stun_keepalive_lifetime_ >= 0) { + stun_port_->set_stun_keepalive_lifetime(stun_keepalive_lifetime_); + } stun_port_->SignalPortComplete.connect(this, &StunPortTest::OnPortComplete); stun_port_->SignalPortError.connect(this, @@ -130,6 +143,10 @@ class StunPortTest : public testing::Test, stun_keepalive_delay_ = delay; } + void SetKeepaliveLifetime(int lifetime) { + stun_keepalive_lifetime_ = lifetime; + } + cricket::TestStunServer* stun_server_1() { return stun_server_1_.get(); } @@ -150,6 +167,7 @@ class StunPortTest : public testing::Test, bool done_; bool error_; int stun_keepalive_delay_; + int stun_keepalive_lifetime_; }; // Test that we can create a STUN port @@ -285,3 +303,53 @@ TEST_F(StunPortTest, TestTwoCandidatesWithTwoStunServersAcrossNat) { EXPECT_EQ(port()->Candidates()[0].relay_protocol(), ""); EXPECT_EQ(port()->Candidates()[1].relay_protocol(), ""); } + +// Test that the stun_keepalive_lifetime is set correctly based on the network +// type on a STUN port. +TEST_F(StunPortTest, TestStunPortGetStunKeepaliveLifetime) { + // Lifetime for the default network type is |kInfiniteLifetime|. + CreateStunPort(kStunAddr1); + EXPECT_EQ(kInfiniteLifetime, port()->stun_keepalive_lifetime()); + + // Lifetime for the cellular network is |kHighCostPortKeepaliveLifetimeMs| + SetNetworkType(rtc::ADAPTER_TYPE_CELLULAR); + CreateStunPort(kStunAddr2); + EXPECT_EQ(kHighCostPortKeepaliveLifetimeMs, + port()->stun_keepalive_lifetime()); +} + +// Test that the stun_keepalive_lifetime is set correctly based on the network +// type on a shared STUN port (UDPPort). +TEST_F(StunPortTest, TestUdpPortGetStunKeepaliveLifetime) { + // Lifetime for the default network type is |kInfiniteLifetime|. + CreateSharedStunPort(kStunAddr1); + EXPECT_EQ(kInfiniteLifetime, port()->stun_keepalive_lifetime()); + + // Lifetime for the cellular network is |kHighCostPortKeepaliveLifetimeMs| + SetNetworkType(rtc::ADAPTER_TYPE_CELLULAR); + CreateSharedStunPort(kStunAddr2); + EXPECT_EQ(kHighCostPortKeepaliveLifetimeMs, + port()->stun_keepalive_lifetime()); +} + +// Test that STUN binding requests will be stopped shortly if the keep-alive +// lifetime is short. +TEST_F(StunPortTest, TestStunBindingRequestShortLifetime) { + SetKeepaliveDelay(101); + SetKeepaliveLifetime(100); + CreateStunPort(kStunAddr1); + PrepareAddress(); + EXPECT_TRUE_WAIT(done(), kTimeoutMs); + EXPECT_TRUE_WAIT(!port()->HasPendingRequest(cricket::STUN_BINDING_REQUEST), + 2000); +} + +// Test that by default, the STUN binding requests will last for a long time. +TEST_F(StunPortTest, TestStunBindingRequestLongLifetime) { + SetKeepaliveDelay(101); + CreateStunPort(kStunAddr1); + PrepareAddress(); + EXPECT_TRUE_WAIT(done(), kTimeoutMs); + rtc::Thread::Current()->ProcessMessages(1000); + EXPECT_TRUE(port()->HasPendingRequest(cricket::STUN_BINDING_REQUEST)); +} diff --git a/webrtc/p2p/base/stunrequest.cc b/webrtc/p2p/base/stunrequest.cc index ce0364e8db..3338861cfb 100644 --- a/webrtc/p2p/base/stunrequest.cc +++ b/webrtc/p2p/base/stunrequest.cc @@ -63,6 +63,16 @@ void StunRequestManager::Flush(int msg_type) { } } +bool StunRequestManager::HasRequest(int msg_type) { + for (const auto kv : requests_) { + StunRequest* request = kv.second; + if (msg_type == kAllRequests || msg_type == request->type()) { + return true; + } + } + return false; +} + void StunRequestManager::Remove(StunRequest* request) { ASSERT(request->manager() == this); RequestMap::iterator iter = requests_.find(request->id()); diff --git a/webrtc/p2p/base/stunrequest.h b/webrtc/p2p/base/stunrequest.h index 44c1ebff56..4217a81f1f 100644 --- a/webrtc/p2p/base/stunrequest.h +++ b/webrtc/p2p/base/stunrequest.h @@ -39,6 +39,10 @@ class StunRequestManager { // Only for testing. void Flush(int msg_type); + // Returns true if at least one request with |msg_type| is scheduled for + // transmission. For testing only. + bool HasRequest(int msg_type); + // Removes a stun request that was added previously. This will happen // automatically when a request succeeds, fails, or times out. void Remove(StunRequest* request);