Keep on sending stun binding requests on zero-cost networks.

webrtc/p2p/base/stunport.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/p2p/base/stunport.h"
 
 #include "webrtc/p2p/base/common.h"
 #include "webrtc/p2p/base/portallocator.h"
 #include "webrtc/p2p/base/stun.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/ipaddress.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/nethelpers.h"
 
 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 KEEPALIVE_DELAY_MS = 10 * 1000;   // 10 seconds - sort timeouts
+const int RETRY_TIMEOUT_US = 50 * 1000000;  // 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 int32_t HIGH_COST_PORT_KEEPALIVE_LIFETIME_MS = 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();
-  }
+                     uint64_t start_time_us,
+                     int64_t lifetime_us)
+      : port_(port),
+        server_addr_(addr),
+        start_time_us_(start_time_us),
+        lifetime_us_(lifetime_us) {}
 
   virtual ~StunBindingRequest() {
   }
 
   const rtc::SocketAddress& server_addr() const { return server_addr_; }
 
   virtual void Prepare(StunMessage* request) override {
     request->SetType(STUN_BINDING_REQUEST);
   }
 
   virtual void OnResponse(StunMessage* response) override {
     const StunAddressAttribute* addr_attr =
         response->GetAddress(STUN_ATTR_MAPPED_ADDRESS);
     if (!addr_attr) {
       LOG(LS_ERROR) << "Binding response missing mapped address.";
     } else if (addr_attr->family() != STUN_ADDRESS_IPV4 &&
                addr_attr->family() != STUN_ADDRESS_IPV6) {
       LOG(LS_ERROR) << "Binding address has bad family";
     } else {
       rtc::SocketAddress addr(addr_attr->ipaddr(), addr_attr->port());
       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::TimeMicros())) {
       port_->requests_.SendDelayed(
-          new StunBindingRequest(port_, server_addr_, deadline_),
+          new StunBindingRequest(port_, server_addr_, start_time_us_,
+                                 lifetime_us_),
           port_->stun_keepalive_delay());
     }
   }
 
   virtual void OnErrorResponse(StunMessage* response) override {
     const StunErrorCodeAttribute* attr = response->GetErrorCode();
     if (!attr) {
       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) {
+    uint64_t now = rtc::TimeMicros();
+    if (WithinLifetime(now) && now - start_time_us_ <= RETRY_TIMEOUT_US) {
       port_->requests_.SendDelayed(
-          new StunBindingRequest(port_, server_addr_, deadline_),
+          new StunBindingRequest(port_, server_addr_, start_time_us_,
+                                 lifetime_us_),
           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(uint64_t now) const {
+    return lifetime_us_ < 0 || now <= start_time_us_ + lifetime_us_;

[honghaiz3, 2016/03/02 20:20:41]

Used the negative check instead of exactly -1 so that if the lifetime in
udp_port is -1, the lifetime in the request can be set just as -1 * 1000. 

+  }
   UDPPort* port_;
   const rtc::SocketAddress server_addr_;
-  uint32_t start_time_;
-  uint32_t deadline_;
+  uint64_t start_time_us_;
+  // The time duration for which this request will be rescheduled.
+  int64_t lifetime_us_;
 };
 
 UDPPort::AddressResolver::AddressResolver(
     rtc::PacketSocketFactory* factory)
     : socket_factory_(factory) {}
 
 UDPPort::AddressResolver::~AddressResolver() {
   for (ResolverMap::iterator it = resolvers_.begin();
        it != resolvers_.end(); ++it) {
     // TODO(guoweis): Change to asynchronous DNS resolution to prevent the hang
@@ skipping 53 matching lines @@
     : Port(thread,
            factory,
            network,
            socket->GetLocalAddress().ipaddr(),
            username,
            password),
       requests_(thread),
       socket_(socket),
       error_(0),
       ready_(false),
-      stun_keepalive_delay_(KEEPALIVE_DELAY),
+      stun_keepalive_delay_(KEEPALIVE_DELAY_MS),
       emit_local_for_anyaddress_(emit_local_for_anyaddress) {
   requests_.set_origin(origin);
 }
 
 UDPPort::UDPPort(rtc::Thread* thread,
                  rtc::PacketSocketFactory* factory,
                  rtc::Network* network,
                  const rtc::IPAddress& ip,
                  uint16_t min_port,
                  uint16_t max_port,
                  const std::string& username,
                  const std::string& password,
                  const std::string& origin,
                  bool emit_local_for_anyaddress)
     : Port(thread,
            LOCAL_PORT_TYPE,
            factory,
            network,
            ip,
            min_port,
            max_port,
            username,
            password),
       requests_(thread),
       socket_(NULL),
       error_(0),
       ready_(false),
-      stun_keepalive_delay_(KEEPALIVE_DELAY),
+      stun_keepalive_delay_(KEEPALIVE_DELAY_MS),
       emit_local_for_anyaddress_(emit_local_for_anyaddress) {
   requests_.set_origin(origin);
 }
 
 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_MS.
+  stun_keepalive_lifetime_ = (network_cost() == 0)
+                                 ? INFINITE_LIFETIME
+                                 : HIGH_COST_PORT_KEEPALIVE_LIFETIME_MS;
   if (!SharedSocket()) {
     ASSERT(socket_ == NULL);
     socket_ = socket_factory()->CreateUdpSocket(
         rtc::SocketAddress(ip(), 0), min_port(), max_port());
     if (!socket_) {
       LOG_J(LS_WARNING, this) << "UDP socket creation failed";
       return false;
     }
     socket_->SignalReadPacket.connect(this, &UDPPort::OnReadPacket);
   }
@@ skipping 165 matching lines @@
   }
 }
 
 void UDPPort::SendStunBindingRequest(const rtc::SocketAddress& stun_addr) {
   if (stun_addr.IsUnresolvedIP()) {
     ResolveStunAddress(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));
+      // The |stun_keepalive_lifetime_| is in milliseconds while the lifetime
+      // in the StunBindingRequest is in microseconds.

[pthatcher1, 2016/03/02 20:25:59]

Can we just make everything in this file milliseconds?  It's confusing having
both.

[honghaiz3, 2016/03/02 23:22:51]

It is hard to all use milliseconds or microseconds. 
The keep_alive_delay is used to schedule the next event, which has to be
milliseconds. 
The keep_alive_lifetime has to be in microseconds to avoid time wraparound. 
What I am doing now is 
Everything in UDPPort uses milliseconds. This also saves a few bytes. 
Everything in StunBindingRequest uses microseconds.

+      requests_.Send(new StunBindingRequest(this, stun_addr, rtc::TimeMicros(),
+                                            stun_keepalive_lifetime_ * 1000));
     } else {
       // Since we can't send stun messages to the server, we should mark this
       // port ready.
       LOG(LS_WARNING) << "STUN server address is incompatible.";
       OnStunBindingOrResolveRequestFailed(stun_addr);
     }
   }
 }
 
 bool UDPPort::MaybeSetDefaultLocalAddress(rtc::SocketAddress* addr) const {
@@ skipping 93 matching lines @@
   const std::vector<Candidate>& existing_candidates = Candidates();
   std::vector<Candidate>::const_iterator it = existing_candidates.begin();
   for (; it != existing_candidates.end(); ++it) {
     if (it->address() == addr)
       return true;
   }
   return false;
 }
 
 }  // namespace cricket