Make the connection to remember path degrading status.

When a conenction has path degrading timer fired, the connection will remember
that it's path degrading until forward progress is made, i.e., new ack has
been received on the degrading path. It will then spin timer to detect future
path degrading.

Flag protected by a new flag --quic_path_degrading_alarm2.

Manual merge internal change: 193990670

Bug: 835444
Change-Id: Idf487fafb10ccd61551f4a9905714ff83bd91cb9
Reviewed-on: https://chromium-review.googlesource.com/1025143Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553315}

net/quic/core/quic_connection.cc

@@ -313,6 +313,7 @@ QuicConnection::QuicConnection(
       stateless_reset_token_received_(false),
       received_stateless_reset_token_(0),
       last_control_frame_id_(kInvalidControlFrameId),
+      is_path_degrading_(false),
       negotiate_version_early_(
           GetQuicReloadableFlag(quic_server_early_version_negotiation)),
       always_discard_packets_after_close_(
@@ -320,7 +321,7 @@ QuicConnection::QuicConnection(
       handle_write_results_for_connectivity_probe_(GetQuicReloadableFlag(
           quic_handle_write_results_for_connectivity_probe)),
       use_path_degrading_alarm_(
-          GetQuicReloadableFlag(quic_path_degrading_alarm)),
+          GetQuicReloadableFlag(quic_path_degrading_alarm2)),
       enable_server_proxy_(GetQuicReloadableFlag(quic_enable_server_proxy)) {
   QUIC_DLOG(INFO) << ENDPOINT
                   << "Created connection with connection_id: " << connection_id
@@ -2111,9 +2112,9 @@ bool QuicConnection::WritePacket(SerializedPacket* packet) {
     // |retransmittable_on_wire_alarm_| to possibly send a PING.
     retransmittable_on_wire_alarm_->Cancel();
     if (use_path_degrading_alarm_) {
-      if (!path_degrading_alarm_->IsSet()) {
-        // This is the first retransmittable packet on the wire after having
-        // none on the wire. Start the path degrading alarm.
+      if (!is_path_degrading_ && !path_degrading_alarm_->IsSet()) {
+        // This is the first retransmittable packet on the working path.
+        // Start the path degrading alarm to detect new path degrading.
         SetPathDegradingAlarm();
       }
     }
@@ -2255,7 +2256,7 @@ void QuicConnection::OnCongestionChange() {
 }
 
 // TODO(b/77267845): remove this method once
-// FLAGS_quic_reloadable_flag_quic_path_degrading_alarm is deprecated.
+// FLAGS_quic_reloadable_flag_quic_path_degrading_alarm2 is deprecated.
 void QuicConnection::OnPathDegrading() {
   DCHECK(!use_path_degrading_alarm_);
   visitor_->OnPathDegrading();
@@ -2324,7 +2325,8 @@ void QuicConnection::SendAck() {
 }
 
 void QuicConnection::OnPathDegradingTimeout() {
-  QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm, 2, 4);
+  QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm2, 2, 5);
+  is_path_degrading_ = true;
   visitor_->OnPathDegrading();
 }
 
@@ -2529,7 +2531,7 @@ void QuicConnection::CancelAllAlarms() {
   mtu_discovery_alarm_->Cancel();
   retransmittable_on_wire_alarm_->Cancel();
   if (use_path_degrading_alarm_) {
-    QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm, 4, 4);
+    QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm2, 5, 5);
     path_degrading_alarm_->Cancel();
   }
 }
@@ -2673,7 +2675,7 @@ void QuicConnection::SetRetransmissionAlarm() {
 
 void QuicConnection::SetPathDegradingAlarm() {
   DCHECK(use_path_degrading_alarm_);
-  QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm, 1, 4);
+  QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm2, 1, 5);
   const QuicTime::Delta delay = sent_packet_manager_.GetPathDegradingDelay();
   path_degrading_alarm_->Update(clock_->ApproximateNow() + delay,
                                 QuicTime::Delta::FromMilliseconds(1));
@@ -3254,13 +3256,16 @@ void QuicConnection::PostProcessAfterAckFrame(bool send_stop_waiting,
     // There are no retransmittable packets on the wire, so it's impossible to
     // say if the connection has degraded.
     if (use_path_degrading_alarm_) {
-      QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm, 3, 4);
+      QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm2, 3, 5);
       path_degrading_alarm_->Cancel();
     }
   } else if (acked_new_packet) {
     // A previously-unacked packet has been acked, which means forward progress
-    // has been made. Push back the path degrading alarm.
+    // has been made. Unset |is_path_degrading| if the path was considered as
+    // degrading previously. Set/update the path degrading alarm.
     if (use_path_degrading_alarm_) {
+      QUIC_FLAG_COUNT_N(quic_reloadable_flag_quic_path_degrading_alarm2, 4, 5);
+      is_path_degrading_ = false;
       SetPathDegradingAlarm();
     }
   }

net/quic/core/quic_connection.h

@@ -495,7 +495,7 @@ class QUIC_EXPORT_PRIVATE QuicConnection
   // QuicSentPacketManager::NetworkChangeVisitor
   void OnCongestionChange() override;
   // TODO(b/76462614): remove OnPathDegrading() once
-  // FLAGS_quic_reloadable_flag_quic_path_degrading_alarm is deprecated.
+  // FLAGS_quic_reloadable_flag_quic_path_degrading_alarm2 is deprecated.
   void OnPathDegrading() override;
   void OnPathMtuIncreased(QuicPacketLength packet_size) override;
 
@@ -783,6 +783,8 @@ class QUIC_EXPORT_PRIVATE QuicConnection
 
   bool IsServerProxyEnabled() const { return enable_server_proxy_; }
 
+  bool IsPathDegrading() const { return is_path_degrading_; }
+
  protected:
   // Calls cancel() on all the alarms owned by this connection.
   void CancelAllAlarms();
@@ -1304,6 +1306,9 @@ class QUIC_EXPORT_PRIVATE QuicConnection
   // Id of latest sent control frame. 0 if no control frame has been sent.
   QuicControlFrameId last_control_frame_id_;
 
+  // True if the peer is unreachable on the current path.
+  bool is_path_degrading_;
+
   // Latched value of
   // quic_reloadable_flag_quic_server_early_version_negotiation.
   const bool negotiate_version_early_;
@@ -1316,7 +1321,7 @@ class QUIC_EXPORT_PRIVATE QuicConnection
   const bool handle_write_results_for_connectivity_probe_;
 
   // Latched value of
-  // quic_reloadable_flag_quic_path_degrading_alarm
+  // quic_reloadable_flag_quic_path_degrading_alarm2.
   const bool use_path_degrading_alarm_;
 
   // Latched value of quic_reloadable_flag_quic_enable_server_proxy.

net/quic/core/quic_connection_test.cc

@@ -789,7 +789,7 @@ class QuicConnectionTest : public QuicTestWithParam<TestParams> {
         connection_id_length_(PACKET_8BYTE_CONNECTION_ID),
         notifier_(&connection_),
         use_path_degrading_alarm_(
-            GetQuicReloadableFlag(quic_path_degrading_alarm)) {
+            GetQuicReloadableFlag(quic_path_degrading_alarm2)) {
     SetQuicFlag(&FLAGS_quic_supports_tls_handshake, true);
     connection_.set_defer_send_in_response_to_packets(GetParam().ack_response ==
                                                       AckResponse::kDefer);
@@ -1208,7 +1208,7 @@ class QuicConnectionTest : public QuicTestWithParam<TestParams> {
   SimpleSessionNotifier notifier_;
 
   // Latched value of
-  // quic_reloadable_flag_quic_path_degrading_alarm
+  // quic_reloadable_flag_quic_path_degrading_alarm2.
   bool use_path_degrading_alarm_;
 
  private:
@@ -6011,6 +6011,7 @@ TEST_P(QuicConnectionTest, PathDegradingAlarm) {
 
   EXPECT_TRUE(connection_.connected());
   EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_FALSE(connection_.IsPathDegrading());
 
   const char data[] = "data";
   size_t data_size = strlen(data);
@@ -6073,6 +6074,7 @@ TEST_P(QuicConnectionTest, PathDegradingAlarm) {
       EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
     }
   }
+  EXPECT_TRUE(connection_.IsPathDegrading());
 }
 
 TEST_P(QuicConnectionTest, RetransmittableOnWireSetsPathDegradingAlarm) {
@@ -6088,6 +6090,7 @@ TEST_P(QuicConnectionTest, RetransmittableOnWireSetsPathDegradingAlarm) {
   EXPECT_CALL(visitor_, HasOpenDynamicStreams()).WillRepeatedly(Return(true));
 
   EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_FALSE(connection_.IsPathDegrading());
   EXPECT_FALSE(connection_.GetRetransmittableOnWireAlarm()->IsSet());
 
   const char data[] = "data";
@@ -6137,6 +6140,152 @@ TEST_P(QuicConnectionTest, RetransmittableOnWireSetsPathDegradingAlarm) {
             connection_.GetPathDegradingAlarm()->deadline());
 }
 
+// This test verifies that the connection marks path as degrading and does not
+// spin timer to detect path degrading when a new packet is sent on the
+// degraded path.
+TEST_P(QuicConnectionTest, NoPathDegradingAlarmIfPathIsDegrading) {
+  if (!use_path_degrading_alarm_) {
+    return;
+  }
+
+  EXPECT_TRUE(connection_.connected());
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_FALSE(connection_.IsPathDegrading());
+
+  const char data[] = "data";
+  size_t data_size = strlen(data);
+  QuicStreamOffset offset = 0;
+
+  // Send the first packet. Now there's a retransmittable packet on the wire, so
+  // the path degrading alarm should be set.
+  connection_.SendStreamDataWithString(1, data, offset, NO_FIN);
+  offset += data_size;
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+  // Check the deadline of the path degrading alarm.
+  QuicTime::Delta delay = QuicConnectionPeer::GetSentPacketManager(&connection_)
+                              ->GetPathDegradingDelay();
+  EXPECT_EQ(clock_.ApproximateNow() + delay,
+            connection_.GetPathDegradingAlarm()->deadline());
+
+  // Send a second packet. The path degrading alarm's deadline should remain
+  // the same.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  QuicTime prev_deadline = connection_.GetPathDegradingAlarm()->deadline();
+  connection_.SendStreamDataWithString(1, data, offset, NO_FIN);
+  offset += data_size;
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_EQ(prev_deadline, connection_.GetPathDegradingAlarm()->deadline());
+
+  // Now receive an ACK of the first packet. This should advance the path
+  // degrading alarm's deadline since forward progress has been made.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+  EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _));
+  QuicAckFrame frame = InitAckFrame({{1u, 2u}});
+  ProcessAckPacket(&frame);
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+  // Check the deadline of the path degrading alarm.
+  delay = QuicConnectionPeer::GetSentPacketManager(&connection_)
+              ->GetPathDegradingDelay();
+  EXPECT_EQ(clock_.ApproximateNow() + delay,
+            connection_.GetPathDegradingAlarm()->deadline());
+
+  // Advance time to the path degrading alarm's deadline and simulate
+  // firing the path degrading alarm. This path will be considered as
+  // degrading.
+  clock_.AdvanceTime(delay);
+  EXPECT_CALL(visitor_, OnPathDegrading()).Times(1);
+  connection_.GetPathDegradingAlarm()->Fire();
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_TRUE(connection_.IsPathDegrading());
+
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  // Send a third packet. The path degrading alarm is no longer set but path
+  // should still be marked as degrading.
+  connection_.SendStreamDataWithString(1, data, offset, NO_FIN);
+  offset += data_size;
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_TRUE(connection_.IsPathDegrading());
+}
+
+// This test verifies that the connection unmarks path as degrarding and spins
+// the timer to detect future path degrading when forward progress is made
+// after path has been marked degrading.
+TEST_P(QuicConnectionTest, UnmarkPathDegradingOnForwardProgress) {
+  if (!use_path_degrading_alarm_) {
+    return;
+  }
+
+  EXPECT_TRUE(connection_.connected());
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_FALSE(connection_.IsPathDegrading());
+
+  const char data[] = "data";
+  size_t data_size = strlen(data);
+  QuicStreamOffset offset = 0;
+
+  // Send the first packet. Now there's a retransmittable packet on the wire, so
+  // the path degrading alarm should be set.
+  connection_.SendStreamDataWithString(1, data, offset, NO_FIN);
+  offset += data_size;
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+  // Check the deadline of the path degrading alarm.
+  QuicTime::Delta delay = QuicConnectionPeer::GetSentPacketManager(&connection_)
+                              ->GetPathDegradingDelay();
+  EXPECT_EQ(clock_.ApproximateNow() + delay,
+            connection_.GetPathDegradingAlarm()->deadline());
+
+  // Send a second packet. The path degrading alarm's deadline should remain
+  // the same.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  QuicTime prev_deadline = connection_.GetPathDegradingAlarm()->deadline();
+  connection_.SendStreamDataWithString(1, data, offset, NO_FIN);
+  offset += data_size;
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_EQ(prev_deadline, connection_.GetPathDegradingAlarm()->deadline());
+
+  // Now receive an ACK of the first packet. This should advance the path
+  // degrading alarm's deadline since forward progress has been made.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+  EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _));
+  QuicAckFrame frame = InitAckFrame({{1u, 2u}});
+  ProcessAckPacket(&frame);
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+  // Check the deadline of the path degrading alarm.
+  delay = QuicConnectionPeer::GetSentPacketManager(&connection_)
+              ->GetPathDegradingDelay();
+  EXPECT_EQ(clock_.ApproximateNow() + delay,
+            connection_.GetPathDegradingAlarm()->deadline());
+
+  // Advance time to the path degrading alarm's deadline and simulate
+  // firing the alarm.
+  clock_.AdvanceTime(delay);
+  EXPECT_CALL(visitor_, OnPathDegrading()).Times(1);
+  connection_.GetPathDegradingAlarm()->Fire();
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_TRUE(connection_.IsPathDegrading());
+
+  // Send a third packet. The path degrading alarm is no longer set but path
+  // should still be marked as degrading.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  connection_.SendStreamDataWithString(1, data, offset, NO_FIN);
+  offset += data_size;
+  EXPECT_FALSE(connection_.GetPathDegradingAlarm()->IsSet());
+  EXPECT_TRUE(connection_.IsPathDegrading());
+
+  // Now receive an ACK of the second packet. This should unmark the path as
+  // degrading. And will set a timer to detect new path degrading.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _));
+  frame = InitAckFrame({{2, 3}});
+  ProcessAckPacket(&frame);
+  EXPECT_FALSE(connection_.IsPathDegrading());
+  EXPECT_TRUE(connection_.GetPathDegradingAlarm()->IsSet());
+}
+
 TEST_P(QuicConnectionTest, MultipleCallsToCloseConnection) {
   // Verifies that multiple calls to CloseConnection do not
   // result in multiple attempts to close the connection - it will be marked as

net/quic/core/quic_flags_list.h

@@ -216,7 +216,7 @@ QUIC_FLAG(
 
 // If true, a separate QuicAlarm in QuicConnection is used to trigger
 // OnPathDegrading() instead of using retransmission_alarm_.
-QUIC_FLAG(bool, FLAGS_quic_reloadable_flag_quic_path_degrading_alarm, true)
+QUIC_FLAG(bool, FLAGS_quic_reloadable_flag_quic_path_degrading_alarm2, true)
 
 // Remove special logic for headers stream from QuicWriteBlockedList and
 // replace it with a static streams map.

net/quic/core/quic_sent_packet_manager.cc

@@ -94,7 +94,7 @@ QuicSentPacketManager::QuicSentPacketManager(
       rtt_updated_(false),
       acked_packets_iter_(last_ack_frame_.packets.rbegin()),
       use_path_degrading_alarm_(
-          GetQuicReloadableFlag(quic_path_degrading_alarm)),
+          GetQuicReloadableFlag(quic_path_degrading_alarm2)),
       use_better_crypto_retransmission_(
           GetQuicReloadableFlag(quic_better_crypto_retransmission)) {
   QUIC_FLAG_COUNT(quic_reloadable_flag_quic_better_crypto_retransmission);

net/quic/core/quic_sent_packet_manager_test.cc

@@ -88,7 +88,7 @@ class QuicSentPacketManagerTest : public QuicTestWithParam<bool> {
         send_algorithm_(new StrictMock<MockSendAlgorithm>),
         network_change_visitor_(new StrictMock<MockNetworkChangeVisitor>),
         use_path_degrading_alarm_(
-            GetQuicReloadableFlag(quic_path_degrading_alarm)) {
+            GetQuicReloadableFlag(quic_path_degrading_alarm2)) {
     QuicSentPacketManagerPeer::SetSendAlgorithm(&manager_, send_algorithm_);
     // Disable tail loss probes for most tests.
     QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 0);
@@ -322,7 +322,7 @@ class QuicSentPacketManagerTest : public QuicTestWithParam<bool> {
   StrictMock<MockSessionNotifier> notifier_;
 
   // Latched value of
-  // quic_reloadable_flag_quic_path_degrading_alarm
+  // quic_reloadable_flag_quic_path_degrading_alarm2.
   bool use_path_degrading_alarm_;
 };