Increase the stun ping interval.

webrtc/p2p/base/p2ptransportchannel.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.
  */
 
@@ skipping 199 matching lines @@
 // well on a 28.8K modem, which is the slowest connection on which the voice
 // quality is reasonable at all.
 static const int PING_PACKET_SIZE = 60 * 8;
 // STRONG_PING_INTERVAL (480ms) is applied when the best connection is both
 // writable and receiving.
 static const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 1000;
 // WEAK_PING_INTERVAL (48ms) is applied when the best connection is either not
 // writable or not receiving.
 const int WEAK_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 10000;
 
-// If the current best connection is both writable and receiving, then we will
-// also try hard to make sure it is pinged at this rate (a little less than
-// 2 * STRONG_PING_INTERVAL).
+// If the current best connection is both writable and receiving, first several
+// pings will be sent at this rate by default. If it pings too fast, other
+// connection may get starved.

[Taylor Brandstetter, 2016/05/27 20:11:00]

"connection" -> "connections"
Also, may want to mention that the first several pings are sent at a faster rate
in order for the RTT to converge faster.

 static const int MAX_CURRENT_STRONG_INTERVAL = 900;  // ms
 
+// Writable connections are pinged at a slower rate.
+static const int WRITABLE_CONNECTION_PING_INTERVAL = 2500;  // ms
+
 static const int MIN_CHECK_RECEIVING_INTERVAL = 50;  // ms
 
 P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
                                          int component,
                                          P2PTransport* transport,
                                          PortAllocator* allocator)
     : P2PTransportChannel(transport_name, component, allocator) {}
 
 P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
                                          int component,
                                          PortAllocator* allocator)
     : TransportChannelImpl(transport_name, component),
       allocator_(allocator),
       worker_thread_(rtc::Thread::Current()),
       incoming_only_(false),
       error_(0),
       best_connection_(NULL),
       pending_best_connection_(NULL),
       sort_dirty_(false),
       remote_ice_mode_(ICEMODE_FULL),
       ice_role_(ICEROLE_UNKNOWN),
       tiebreaker_(0),
       gathering_state_(kIceGatheringNew),
       check_receiving_interval_(MIN_CHECK_RECEIVING_INTERVAL * 5),
       config_(MIN_CHECK_RECEIVING_INTERVAL * 50 /* receiving_timeout */,
               0 /* backup_connection_ping_interval */,
               false /* gather_continually */,
               false /* prioritize_most_likely_candidate_pairs */,
-              MAX_CURRENT_STRONG_INTERVAL /* max_strong_interval */) {
+              WRITABLE_CONNECTION_PING_INTERVAL) {
   uint32_t weak_ping_interval = ::strtoul(
       webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(),
       nullptr, 10);
   if (weak_ping_interval) {
     weak_ping_interval_ = static_cast<int>(weak_ping_interval);
   }
 }
 
 P2PTransportChannel::~P2PTransportChannel() {
   ASSERT(worker_thread_ == rtc::Thread::Current());
@@ skipping 162 matching lines @@
     }
     LOG(LS_INFO) << "Set ICE receiving timeout to " << config_.receiving_timeout
                  << " milliseconds";
   }
 
   config_.prioritize_most_likely_candidate_pairs =
       config.prioritize_most_likely_candidate_pairs;
   LOG(LS_INFO) << "Set ping most likely connection to "
                << config_.prioritize_most_likely_candidate_pairs;
 
-  if (config.max_strong_interval >= 0 &&
-      config_.max_strong_interval != config.max_strong_interval) {
-    config_.max_strong_interval = config.max_strong_interval;
-    LOG(LS_INFO) << "Set max strong interval to "
-                 << config_.max_strong_interval;
+  if (config.writable_connection_ping_interval >= 0 &&
+      config_.writable_connection_ping_interval !=
+          config.writable_connection_ping_interval) {
+    config_.writable_connection_ping_interval =
+        config.writable_connection_ping_interval;
+    LOG(LS_INFO) << "Set writable_connection_ping_interval to "
+                 << config_.writable_connection_ping_interval;
   }
 }
 
 const IceConfig& P2PTransportChannel::config() const {
   return config_;
 }
 
 // Go into the state of processing candidates, and running in general
 void P2PTransportChannel::Connect() {
   ASSERT(worker_thread_ == rtc::Thread::Current());
@@ skipping 849 matching lines @@
   // When the best connection is either not receiving or not writable,
   // switch to weak ping interval.
   int ping_interval = weak() ? weak_ping_interval_ : STRONG_PING_INTERVAL;
   if (rtc::TimeMillis() >= last_ping_sent_ms_ + ping_interval) {
     Connection* conn = FindNextPingableConnection();
     if (conn) {
       PingConnection(conn);
       MarkConnectionPinged(conn);
     }
   }
+
   int delay = std::min(ping_interval, check_receiving_interval_);
   thread()->PostDelayed(delay, this, MSG_CHECK_AND_PING);
 }
 
 // A connection is considered a backup connection if the channel state
 // is completed, the connection is not the best connection and it is active.
 bool P2PTransportChannel::IsBackupConnection(Connection* conn) const {
   return state_ == STATE_COMPLETED && conn != best_connection_ &&
          conn->active();
 }
 
+bool P2PTransportChannel::IsBestConnectionPingable() {
+  int64_t now = rtc::TimeMillis();
+  bool is_pingable = false;
+  if (best_connection_ && best_connection_->connected() &&
+      best_connection_->writable()) {
+    // The best_connection need to ping at a fast rate.

[pthatcher1, 2016/05/27 17:36:31]

need => needs

+    is_pingable =
+        (best_connection_->NeedToPingFast() &&
+         best_connection_->last_ping_sent() + MAX_CURRENT_STRONG_INTERVAL <=
+             now);
+
+    // After passing writable_connection_ping_interval, the best connection
+    // becomes pingable no matter if it needs to ping fast or not.
+    is_pingable =
+        is_pingable || (best_connection_->last_ping_sent() +
+                            config_.writable_connection_ping_interval <=
+                        now);

[pthatcher1, 2016/05/27 17:36:31]

This would be more clear with early returns and an interval parameter.

if (!best_connection_ || !best_connection_->connected() ||
!best_connection_->writable()) {
  return false;
}

int interval = best_connection_->NeedToPingFast() ? MAX_CURRENT_STRONG_INTERVAL
: config_.writable_connection_ping_interval;
return best_connection_->last_ping_sent() + interval <= rtc::TimeMillis();

[zhihuang1, 2016/06/01 21:15:01]

writable_connection_ping_interval may be smaller than the default value
MAX_CURRENT_STRONG_INTERVAL. 

So I changed it to:
int interval = best_connection_->NeedToPingFast() ?
min(MAX_CURRENT_STRONG_INTERVAL,  config_.writable_connection_ping_interval)
: config_.writable_connection_ping_interval;

+  }
+  return is_pingable;
+}
+
 // Is the connection in a state for us to even consider pinging the other side?
 // We consider a connection pingable even if it's not connected because that's
 // how a TCP connection is kicked into reconnecting on the active side.
 bool P2PTransportChannel::IsPingable(Connection* conn, int64_t now) {
   const Candidate& remote = conn->remote_candidate();
   // We should never get this far with an empty remote ufrag.
   ASSERT(!remote.username().empty());
   if (remote.username().empty() || remote.password().empty()) {
     // If we don't have an ICE ufrag and pwd, there's no way we can ping.
     return false;
@@ skipping 10 matching lines @@
   if (weak()) {
     return true;
   }
 
   // Always ping active connections regardless whether the channel is completed
   // or not, but backup connections are pinged at a slower rate.
   if (IsBackupConnection(conn)) {
     return (now >= conn->last_ping_response_received() +
                        config_.backup_connection_ping_interval);
   }
+
+  // The connect will ping at a slower rate with two conditions:
+  // 1. The connection is writable.
+  // 2. The connection is not sending first several pings

[Taylor Brandstetter, 2016/05/27 20:11:00]

"The connect" -> "The connection"
"is not sending first several pings" -> "has sent enough initial pings for the
RTT to converge".

+  // (kFirstNPingsWithFastRate)
+  if (conn->writable() && !conn->NeedToPingFast()) {
+    return (now >=
+            conn->last_ping_sent() + config_.writable_connection_ping_interval);
+  }
+
   return conn->active();
 }
 
 // Returns the next pingable connection to ping.  This will be the oldest
 // pingable connection unless we have a connected, writable connection that is
-// past the maximum acceptable ping interval. When reconnecting a TCP
+// past the writable ping interval. When reconnecting a TCP
 // connection, the best connection is disconnected, although still WRITABLE
 // while reconnecting. The newly created connection should be selected as the
 // ping target to become writable instead. See the big comment in
 // CompareConnections.
 Connection* P2PTransportChannel::FindNextPingableConnection() {
   int64_t now = rtc::TimeMillis();
   Connection* conn_to_ping = nullptr;
-  if (best_connection_ && best_connection_->connected() &&
-      best_connection_->writable() &&
-      (best_connection_->last_ping_sent() + config_.max_strong_interval <=
-       now)) {
+  if (IsBestConnectionPingable()) {
     conn_to_ping = best_connection_;
   } else {
     conn_to_ping = FindConnectionToPing(now);
   }
   return conn_to_ping;
 }
 
 void P2PTransportChannel::MarkConnectionPinged(Connection* conn) {
   if (conn && pinged_connections_.insert(conn).second) {
     unpinged_connections_.erase(conn);
@@ skipping 280 matching lines @@
 
   // During the initial state when nothing has been pinged yet, return the first
   // one in the ordered |connections_|.
   return *(std::find_if(connections_.begin(), connections_.end(),
                         [conn1, conn2](Connection* conn) {
                           return conn == conn1 || conn == conn2;
                         }));
 }
 
 }  // namespace cricket