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 192 matching lines @@
 }  // unnamed namespace
 
 namespace cricket {
 
 // When the socket is unwritable, we will use 10 Kbps (ignoring IP+UDP headers)
 // for pinging.  When the socket is writable, we will use only 1 Kbps because
 // we don't want to degrade the quality on a modem.  These numbers should work
 // 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;
+// STRONG_PING_INTERVAL (2400ms) is applied to the Writable connections in the
+// channel when the best connection is both writable and receiving.
+static const int STRONG_PING_INTERVAL = 1000 * PING_PACKET_SIZE / 200;

[Taylor Brandstetter, 2016/05/04 17:58:08]

I don't think the strong ping interval should change. That means that if one
connection becomes writable, other connections will only be pinged every ~5
seconds.

 // 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).
-static const int MAX_CURRENT_STRONG_INTERVAL = 900;  // ms
+static const int MAX_CURRENT_STRONG_INTERVAL = 4500;  // ms

[Taylor Brandstetter, 2016/05/04 17:58:08]

This can probably change to 2500 though.

 
 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,
@@ skipping 1040 matching lines @@
 void P2PTransportChannel::OnSort() {
   // Resort the connections based on the new statistics.
   SortConnections();
 }
 
 // Handle queued up check-and-ping request
 void P2PTransportChannel::OnCheckAndPing() {
   // Make sure the states of the connections are up-to-date (since this affects
   // which ones are pingable).
   UpdateConnectionStates();
-  // 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) {
+
+  if (rtc::TimeMillis() >= last_ping_sent_ms_ + weak_ping_interval_) {

[honghaiz3, 2016/05/03 19:15:22]

Now after I see this, I realize it will ping all connections with a fast rate
until either they time out or become writable, which is good and bad.
This prevents the issue of missing finding a high-priority connection although
it may ping an un-connectable high-priority connection too often. 

[honghaiz3, 2016/05/03 19:59:17]

I think you remove the if condition here and take the check_and_ping delay as a
min of weak_ping_interval_ and checking_receiving_interval_. 

[Taylor Brandstetter, 2016/05/04 17:58:08]

I think the strong ping interval (if it stays ~500ms) should still be used here.

     Connection* conn = FindNextPingableConnection();
     if (conn) {
       PingConnection(conn);
       MarkConnectionPinged(conn);
     }
   }
+  // 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;
   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();
 }
@@ skipping 15 matching lines @@
   // reconnecting state so ping is needed.
   if (!conn->connected() && !conn->writable()) {
     return false;
   }
 
   // If the channel is weakly connected, ping all connections.
   if (weak()) {
     return true;
   }
 
+  // Writable connections are ping at a slower rate.
+  if (conn->writable()) {

[honghaiz3, 2016/05/03 19:15:22]

You may need to move this after the backupconnection check. 
Otherwise, once a backup connection is writable, it will keep on being pinged at
strong_ping_interval. 

+    return now >= conn->last_ping_sent() + STRONG_PING_INTERVAL;

[Taylor Brandstetter, 2016/05/04 17:58:08]

If STRONG_PING_INTERVAL is changed back to 500ms, this should use
MAX_CURRENT_STRONG_INTERVAL, or some other define that's also 2500ms.

+  }
+
   // 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);
   }
+
   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
 // 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.
@@ skipping 293 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