ACK non-consecutive QUIC CE packets immediately.

The arrival of a Congestion Experienced-marked packet currently has no effect on the ACK timeout. RFC 9000 recommends sending an ACK after every CE. draft-ietf-quic-ack-frequency-10 recommends sending after every non-consecutive CE, which is better advice.

This CL implements the latter.

Protected by FLAGS_quic_reloadable_flag_quic_ack_ce_immediately.

PiperOrigin-RevId: 712679547

quiche/common/quiche_feature_flags_list.h

diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 065699e..2b2bee8 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -9,6 +9,7 @@
 #if defined(QUICHE_FLAG)
 
 QUICHE_FLAG(bool, quiche_reloadable_flag_enable_h3_origin_frame, false, true, "If true, enables support for parsing HTTP/3 ORIGIN frames.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_ack_ce_immediately, false, false, "When true, acks a CE-marked packet immediately, unless the previous packet was also CE-marked.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_act_upon_invalid_header, true, true, "If true, reject or send error response code upon receiving invalid request or response headers.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_add_stream_info_to_idle_close_detail, false, true, "If true, include stream information in idle timeout connection close detail.")
 QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_client_enabled_bbr_v2, true, true, "If true, allow client to enable BBRv2 on server via connection option 'B2ON'.")

quiche/quic/core/quic_received_packet_manager.cc

diff --git a/quiche/quic/core/quic_received_packet_manager.cc b/quiche/quic/core/quic_received_packet_manager.cc
index 8d56d51..fa0f64b 100644
--- a/quiche/quic/core/quic_received_packet_manager.cc
+++ b/quiche/quic/core/quic_received_packet_manager.cc
@@ -124,6 +124,13 @@
     }
   }
 
+  if (GetQuicReloadableFlag(quic_ack_ce_immediately)) {
+    if (ecn == ECN_CE && !last_packet_was_ce_marked_) {
+      QUIC_RELOADABLE_FLAG_COUNT_N(quic_ack_ce_immediately, 1, 2);
+      changed_to_ce_marked_ = true;
+    }
+    last_packet_was_ce_marked_ = ecn == ECN_CE;
+  }
   if (ecn != ECN_NOT_ECT) {
     if (!ack_frame_.ecn_counters.has_value()) {
       ack_frame_.ecn_counters = QuicEcnCounts();
@@ -291,6 +298,15 @@
     return;
   }
 
+  if (changed_to_ce_marked_) {
+    // changed_to_ce_marked_ is always false if quic_ack_ce_immediately is
+    // false, so there's no need to check the feature flag here.
+    ack_timeout_ = now;
+    changed_to_ce_marked_ = false;
+    QUIC_RELOADABLE_FLAG_COUNT_N(quic_ack_ce_immediately, 2, 2);
+    return;
+  }
+
   if (!should_last_packet_instigate_acks) {
     return;
   }

quiche/quic/core/quic_received_packet_manager.h

diff --git a/quiche/quic/core/quic_received_packet_manager.h b/quiche/quic/core/quic_received_packet_manager.h
index 1d24651..c08bb70 100644
--- a/quiche/quic/core/quic_received_packet_manager.h
+++ b/quiche/quic/core/quic_received_packet_manager.h
@@ -206,6 +206,12 @@
   // Whether the most recent packet was missing before it was received.
   bool was_last_packet_missing_;
 
+  // Was the previous received packet CE-marked?
+  bool last_packet_was_ce_marked_ = false;
+  // The current packet is CE-marked, and the previous packet was not. This
+  // condition should trigger an immediate ACK.
+  bool changed_to_ce_marked_ = false;
+
   // Last sent largest acked, which gets updated when ACK was successfully sent.
   QuicPacketNumber last_sent_largest_acked_;
 

quiche/quic/core/quic_received_packet_manager_test.cc

diff --git a/quiche/quic/core/quic_received_packet_manager_test.cc b/quiche/quic/core/quic_received_packet_manager_test.cc
index ffc8862..da189ac 100644
--- a/quiche/quic/core/quic_received_packet_manager_test.cc
+++ b/quiche/quic/core/quic_received_packet_manager_test.cc
@@ -6,13 +6,12 @@
 
 #include <algorithm>
 #include <cstddef>
-#include <ostream>
-#include <vector>
 
 #include "quiche/quic/core/congestion_control/rtt_stats.h"
 #include "quiche/quic/core/crypto/crypto_protocol.h"
 #include "quiche/quic/core/quic_connection_stats.h"
 #include "quiche/quic/core/quic_constants.h"
+#include "quiche/quic/core/quic_time.h"
 #include "quiche/quic/core/quic_types.h"
 #include "quiche/quic/platform/api/quic_expect_bug.h"
 #include "quiche/quic/platform/api/quic_flags.h"
@@ -709,6 +708,37 @@
   EXPECT_EQ(ack.ack_frame->ecn_counters->ce, 1);
 }
 
+TEST_F(QuicReceivedPacketManagerTest, NewCeTriggersImmediateAck) {
+  SetQuicReloadableFlag(quic_ack_ce_immediately, true);
+  EXPECT_FALSE(HasPendingAck());
+  RecordPacketReceipt(3, QuicTime::Zero(), ECN_ECT1);
+  MaybeUpdateAckTimeout(kInstigateAck, 3);
+  EXPECT_TRUE(HasPendingAck());
+  EXPECT_GT(received_manager_.ack_timeout(), clock_.ApproximateNow());
+  // Ack is triggered by kDefaultRetransmittablePacketsBeforeAck.
+  RecordPacketReceipt(4, QuicTime::Zero(), ECN_ECT1);
+  MaybeUpdateAckTimeout(kInstigateAck, 4);
+  CheckAckTimeout(clock_.ApproximateNow());
+  // New CE triggers immediate ACK.
+  RecordPacketReceipt(5, QuicTime::Zero(), ECN_CE);
+  MaybeUpdateAckTimeout(kInstigateAck, 5);
+  CheckAckTimeout(clock_.ApproximateNow());
+  // Do not ack consecutive CE.
+  RecordPacketReceipt(6, QuicTime::Zero(), ECN_CE);
+  MaybeUpdateAckTimeout(kInstigateAck, 6);
+  EXPECT_TRUE(HasPendingAck());
+  CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
+  // Non-CE packet is second, triggers ACK.
+  RecordPacketReceipt(7, QuicTime::Zero(), ECN_ECT1);
+  MaybeUpdateAckTimeout(kInstigateAck, 7);
+  CheckAckTimeout(clock_.ApproximateNow());
+  // New non-consecutive CE triggers immediate ACK.
+  RecordPacketReceipt(8, QuicTime::Zero(), ECN_CE);
+  MaybeUpdateAckTimeout(kInstigateAck, 8);
+  CheckAckTimeout(clock_.ApproximateNow());
+}
+
 }  // namespace
+
 }  // namespace test
 }  // namespace quic