Ping backup connection at a slower rate

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 169 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 uint32_t PING_PACKET_SIZE = 60 * 8;
+// TODO(honghaiz): Change the word DELAY to INTERVAL whenever appropriate.
 // STRONG_PING_DELAY (480ms) is applied when the best connection is both
 // writable and receiving.
 static const uint32_t STRONG_PING_DELAY = 1000 * PING_PACKET_SIZE / 1000;
 // WEAK_PING_DELAY (48ms) is applied when the best connection is either not
 // writable or not receiving.
 const uint32_t WEAK_PING_DELAY = 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_DELAY).
 static const uint32_t MAX_CURRENT_STRONG_DELAY = 900;
 
 static const int MIN_CHECK_RECEIVING_DELAY = 50;  // ms
 
-
 P2PTransportChannel::P2PTransportChannel(const std::string& transport_name,
                                          int component,
                                          P2PTransport* transport,
                                          PortAllocator* allocator)
     : TransportChannelImpl(transport_name, component),
       transport_(transport),
       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),
       remote_candidate_generation_(0),
       gathering_state_(kIceGatheringNew),
       check_receiving_delay_(MIN_CHECK_RECEIVING_DELAY * 5),
-      receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50) {
+      receiving_timeout_(MIN_CHECK_RECEIVING_DELAY * 50),
+      backup_connection_ping_interval_(0) {
   uint32_t weak_ping_delay = ::strtoul(
       webrtc::field_trial::FindFullName("WebRTC-StunInterPacketDelay").c_str(),
       nullptr, 10);
   if (weak_ping_delay) {
     weak_ping_delay_ =  weak_ping_delay;
   }
 }
 
 P2PTransportChannel::~P2PTransportChannel() {
   ASSERT(worker_thread_ == rtc::Thread::Current());
@@ skipping 54 matching lines @@
   ASSERT(worker_thread_ == rtc::Thread::Current());
   if (!ports_.empty()) {
     LOG(LS_ERROR)
         << "Attempt to change tiebreaker after Port has been allocated.";
     return;
   }
 
   tiebreaker_ = tiebreaker;
 }
 
+TransportChannelState P2PTransportChannel::GetState() const {

[pthatcher1, 2015/12/01 19:40:25]

Can we just change this to state()?

[honghaiz3, 2015/12/01 23:43:08]

This is a virtual method. 

+  return state_;
+}
+
 // A channel is considered ICE completed once there is at most one active
 // connection per network and at least one active connection.
-TransportChannelState P2PTransportChannel::GetState() const {
+TransportChannelState P2PTransportChannel::ComputeState() const {
   if (!had_connection_) {
     return TransportChannelState::STATE_INIT;
   }
 
   std::vector<Connection*> active_connections;
   for (Connection* connection : connections_) {
     if (connection->active()) {
       active_connections.push_back(connection);
     }
   }
@@ skipping 53 matching lines @@
 }
 
 void P2PTransportChannel::SetRemoteIceMode(IceMode mode) {
   remote_ice_mode_ = mode;
 }
 
 void P2PTransportChannel::SetIceConfig(const IceConfig& config) {
   gather_continually_ = config.gather_continually;
   LOG(LS_INFO) << "Set gather_continually to " << gather_continually_;
 
-  if (config.receiving_timeout_ms < 0) {
-    return;
+  if (config.backup_connection_ping_interval >= 0 &&
+      backup_connection_ping_interval_ !=
+          config.backup_connection_ping_interval) {
+    backup_connection_ping_interval_ = config.backup_connection_ping_interval;
+    LOG(LS_INFO) << "Set backup connection ping interval to "
+                 << backup_connection_ping_interval_ << " milliseconds.";
   }
-  receiving_timeout_ = config.receiving_timeout_ms;
-  check_receiving_delay_ =
-      std::max(MIN_CHECK_RECEIVING_DELAY, receiving_timeout_ / 10);
 
-  for (Connection* connection : connections_) {
-    connection->set_receiving_timeout(receiving_timeout_);
+  if (config.receiving_timeout_ms >= 0 &&
+      receiving_timeout_ != config.receiving_timeout_ms) {
+    receiving_timeout_ = config.receiving_timeout_ms;
+    check_receiving_delay_ =
+        std::max(MIN_CHECK_RECEIVING_DELAY, receiving_timeout_ / 10);
+
+    for (Connection* connection : connections_) {
+      connection->set_receiving_timeout(receiving_timeout_);
+    }
+    LOG(LS_INFO) << "Set ICE receiving timeout to " << receiving_timeout_
+                 << " milliseconds";
   }
-  LOG(LS_INFO) << "Set ICE receiving timeout to " << receiving_timeout_
-               << " milliseconds";
 }
 
 // Go into the state of processing candidates, and running in general
 void P2PTransportChannel::Connect() {
   ASSERT(worker_thread_ == rtc::Thread::Current());
   if (ice_ufrag_.empty() || ice_pwd_.empty()) {
     ASSERT(false);
     LOG(LS_ERROR) << "P2PTransportChannel::Connect: The ice_ufrag_ and the "
                   << "ice_pwd_ are not set.";
     return;
@@ skipping 647 matching lines @@
     }
     LOG_J(LS_INFO, this) << "New best connection: "
                          << best_connection_->ToString();
     SignalRouteChange(this, best_connection_->remote_candidate());
   } else {
     LOG_J(LS_INFO, this) << "No best connection";
   }
 }
 
 void P2PTransportChannel::UpdateChannelState() {
+  state_ = ComputeState();

[pthatcher1, 2015/12/01 19:40:25]

Since ComputeState is never called anywhere else, would it be more readable to
just move that logic here into UpdateChannelState()?  

Also, can we rename it to UpdateState()?  

[honghaiz3, 2015/12/01 23:43:08]

Changed to UpdateState
ComputeState has a lot of early returns. 
Although we can keep the early return by moving the updating to the end of the
method, it will have issues that the transportcontroller will get an old state
of the channel. 

+
   bool writable = best_connection_ && best_connection_->writable();
   set_writable(writable);
 
   bool receiving = false;
   for (const Connection* connection : connections_) {
     if (connection->receiving()) {
       receiving = true;
       break;
     }
   }
@@ skipping 79 matching lines @@
   if (rtc::Time() >= last_ping_sent_ms_ + ping_delay) {
     Connection* conn = FindNextPingableConnection();
     if (conn) {
       PingConnection(conn);
     }
   }
   int check_delay = std::min(ping_delay, check_receiving_delay_);
   thread()->PostDelayed(check_delay, this, MSG_CHECK_AND_PING);
 }
 
+// A connection is considered a backup connection if the channel state
+// is complete, the connection is not the best connection and it is active.

[pthatcher1, 2015/12/01 19:40:25]

complete => completed

[honghaiz3, 2015/12/01 23:43:08]

Done.

+bool P2PTransportChannel::IsBackupConnection(Connection* conn) const {
+  return state_ == STATE_COMPLETED && conn != best_connection_ &&
+         conn->active();
+}
+
 // 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) {
+bool P2PTransportChannel::IsPingable(Connection* conn, uint32_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;
   }
 
   // An never connected connection cannot be written to at all, so pinging is
   // out of the question. However, if it has become WRITABLE, it is in the
   // reconnecting state so ping is needed.
   if (!conn->connected() && !conn->writable()) {
     return false;
   }
 
-  // If the channel is weak, ping all candidates. Otherwise, we only
-  // want to ping connections that have not timed out on writing.
-  return weak() || conn->write_state() != Connection::STATE_WRITE_TIMEOUT;
+  // If the channel is weakly connected, ping all connections.
+  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() +
+                       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 delay. 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() {
   uint32_t now = rtc::Time();
   if (best_connection_ && best_connection_->connected() &&
       best_connection_->writable() &&
       (best_connection_->last_ping_sent() + MAX_CURRENT_STRONG_DELAY <= now)) {
     return best_connection_;
   }
 
   // First, find "triggered checks".  We ping first those connections
   // that have received a ping but have not sent a ping since receiving
   // it (last_received_ping > last_sent_ping).  But we shouldn't do
   // triggered checks if the connection is already writable.
   Connection* oldest_needing_triggered_check = nullptr;
   Connection* oldest = nullptr;
   for (Connection* conn : connections_) {
-    if (!IsPingable(conn)) {
+    if (!IsPingable(conn, now)) {
       continue;
     }
     bool needs_triggered_check =
         (!conn->writable() &&
          conn->last_ping_received() > conn->last_ping_sent());
     if (needs_triggered_check &&
         (!oldest_needing_triggered_check ||
          (conn->last_ping_received() <
           oldest_needing_triggered_check->last_ping_received()))) {
       oldest_needing_triggered_check = conn;
@@ skipping 91 matching lines @@
   // The call to SortConnections will pick a new one.  It looks at the current
   // best connection in order to avoid switching between fairly similar ones.
   // Since this connection is no longer an option, we can just set best to NULL
   // and re-choose a best assuming that there was no best connection.
   if (best_connection_ == connection) {
     LOG(LS_INFO) << "Best connection destroyed.  Will choose a new one.";
     SwitchBestConnectionTo(NULL);
     RequestSort();
   }
 
+  UpdateChannelState();
+  // SignalConnectionRemoved should be called after the channel state is
+  // updated because the receiver of the event may access the channel state.
   SignalConnectionRemoved(this);
 }
 
 // When a port is destroyed remove it from our list of ports to use for
 // connection attempts.
 void P2PTransportChannel::OnPortDestroyed(PortInterface* port) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   // Remove this port from the list (if we didn't drop it already).
   std::vector<PortInterface*>::iterator iter =
@@ skipping 34 matching lines @@
   SignalSentPacket(this, sent_packet);
 }
 
 void P2PTransportChannel::OnReadyToSend(Connection* connection) {
   if (connection == best_connection_ && writable()) {
     SignalReadyToSend(this);
   }
 }
 
 }  // namespace cricket