Do not switch best connection on the controlled side too frequently

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 182 matching lines @@
   }
 
   int prefs_cmp = CompareConnectionCandidates(a_conn, b_conn);
   if (prefs_cmp != 0) {
     return prefs_cmp < 0;
   }
 
   return b_conn->rtt() <= a_conn->rtt() + kMinImprovement;
 }
 
+// Determines whether we should switch the best connection to |new_connection|
+// based the writable state, the last nominated connection, and the last
+// connection receiving data. This is to prevent that the controlled side may
+// switch the best connection too frequently when the controlling side is doing
+// aggressive nominations. With this implementation, the controlled side will
+// switch the best connection if the new nominated connection is writable, and
+// i) the best connection is nullptr, or
+// ii) the best connection was not nominated, or
+// iii) the new connection has been nominated twice consecutively, or
+// iv) the new connection is nominated and is receiving data, or
+// v) the new connection has lower cost or high priority.
+// TODO(honghaiz): Stop the aggressive nomination on the controlling side and
+// implement the ice-renomination option.
+bool ShouldSwitchOnNominatedOrDataReceived(
+    cricket::Connection* best_connection,
+    cricket::Connection* new_connection,
+    cricket::Connection* last_nominated,
+    cricket::Connection* last_data_received) {
+  RTC_CHECK(new_connection != nullptr);
+  if (!new_connection->writable()) {
+    return false;
+  }
+  if (best_connection == nullptr || !best_connection->nominated()) {
+    return true;
+  }
+  if (new_connection == last_nominated ||
+      new_connection == last_data_received) {
+    return true;
+  }
+  // Compare the network cost and priority.
+  return CompareConnectionCandidates(best_connection, new_connection) < 0;
+}
+
 }  // 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;
@@ skipping 18 matching lines @@
     : 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 */,
@@ skipping 469 matching lines @@
     return nullptr;
   }
   *generation = params.rend() - it - 1;
   return &(*it);
 }
 
 void P2PTransportChannel::OnNominated(Connection* conn) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
   ASSERT(ice_role_ == ICEROLE_CONTROLLED);
 
-  if (conn->write_state() == Connection::STATE_WRITABLE) {
-    if (best_connection_ != conn) {
-      pending_best_connection_ = NULL;
-      LOG(LS_INFO) << "Switching best connection on controlled side: "
-                   << conn->ToString();
-      SwitchBestConnectionTo(conn);
-      // Now we have selected the best connection, time to prune other existing
-      // connections and update the read/write state of the channel.
-      RequestSort();
-    }
+  if (best_connection_ == conn) {
+    return;
+  }
+
+  if (ShouldSwitchOnNominatedOrDataReceived(best_connection_, conn,
+                                            last_nominated_conn_,
+                                            last_data_received_conn_)) {
+    last_nominated_conn_ = nullptr;
+    last_data_received_conn_ = nullptr;
+    LOG(LS_INFO) << "Switching best connection on controlled side: "
+                 << conn->ToString();
+    SwitchBestConnectionTo(conn);
+    // Now we have selected the best connection, time to prune other existing
+    // connections and update the read/write state of the channel.
+    RequestSort();
   } else {
-    LOG(LS_INFO) << "Not switching the best connection on controlled side yet,"
-                 << " because it's not writable: " << conn->ToString();
-    pending_best_connection_ = conn;
+    LOG(LS_INFO) << "Not switching the best connection on controlled side yet: "
+                 << conn->ToString();
+    last_nominated_conn_ = conn;
   }
 }
 
 void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   uint32_t generation = GetRemoteCandidateGeneration(candidate);
   // If a remote candidate with a previous generation arrives, drop it.
   if (generation < remote_ice_generation()) {
     LOG(LS_WARNING) << "Dropping a remote candidate because its ufrag "
@@ skipping 681 matching lines @@
   conn->Ping(last_ping_sent_ms_);
 }
 
 // When a connection's state changes, we need to figure out who to use as
 // the best connection again.  It could have become usable, or become unusable.
 void P2PTransportChannel::OnConnectionStateChange(Connection* connection) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   // Update the best connection if the state change is from pending best
   // connection and role is controlled.
-  if (ice_role_ == ICEROLE_CONTROLLED) {
-    if (connection == pending_best_connection_ && connection->writable()) {
-      pending_best_connection_ = NULL;
+  if (ice_role_ == ICEROLE_CONTROLLED && connection == last_nominated_conn_) {
+    // This essentially requires that the connection is both last nominated
+    // and receiving data, or the best connection is nullptr, in order
+    // to switch to |connection|.
+    if (connection == best_connection_) {
+      // If it is already the best connection, reset the last_nominated_conn_
+      last_nominated_conn_ = nullptr;
+    } else if (ShouldSwitchOnNominatedOrDataReceived(
+                   best_connection_, connection, nullptr,
+                   last_data_received_conn_)) {
+      last_nominated_conn_ = nullptr;
+      last_data_received_conn_ = nullptr;
       LOG(LS_INFO) << "Switching best connection on controlled side"
                    << " because it's now writable: " << connection->ToString();
       SwitchBestConnectionTo(connection);
     }
   }
 
   // May stop the allocator session when at least one connection becomes
   // strongly connected after starting to get ports and the local candidate of
   // the connection is at the latest generation. It is not enough to check
   // that the connection becomes weakly connected because the connection may be
@@ skipping 22 matching lines @@
   std::vector<Connection*>::iterator iter =
       std::find(connections_.begin(), connections_.end(), connection);
   ASSERT(iter != connections_.end());
   pinged_connections_.erase(*iter);
   unpinged_connections_.erase(*iter);
   connections_.erase(iter);
 
   LOG_J(LS_INFO, this) << "Removed connection ("
     << static_cast<int>(connections_.size()) << " remaining)";
 
-  if (pending_best_connection_ == connection) {
-    pending_best_connection_ = NULL;
+  if (last_nominated_conn_ == connection) {
+    last_nominated_conn_ = nullptr;
   }
 
   // If this is currently the best connection, then we need to pick a new one.
   // 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);
@@ skipping 48 matching lines @@
 
   // Do not deliver, if packet doesn't belong to the correct transport channel.
   if (!FindConnection(connection))
     return;
 
   // Let the client know of an incoming packet
   SignalReadPacket(this, data, len, packet_time, 0);
 
   // May need to switch the sending connection based on the receiving media path
   // if this is the controlled side.
-  if (ice_role_ == ICEROLE_CONTROLLED && !best_nominated_connection() &&
-      connection->writable() && best_connection_ != connection) {
+  if (ice_role_ != ICEROLE_CONTROLLED || best_connection_ == connection) {
+    return;
+  }
+
+  if (ShouldSwitchOnNominatedOrDataReceived(best_connection_, connection,
+                                            last_nominated_conn_, nullptr)) {
     SwitchBestConnectionTo(connection);
+    last_data_received_conn_ = nullptr;
+    last_nominated_conn_ = nullptr;
+  } else {
+    last_data_received_conn_ = connection;
   }
 }
 
 void P2PTransportChannel::OnSentPacket(const rtc::SentPacket& sent_packet) {
   ASSERT(worker_thread_ == rtc::Thread::Current());
 
   SignalSentPacket(this, sent_packet);
 }
 
 void P2PTransportChannel::OnReadyToSend(Connection* connection) {
@@ skipping 113 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