gfe-relnote: Add a connection option(1ACK) to send only one immediate ACK in QUIC after reordering, instead of 4. Protected by gfe2_reloadable_flag_quic_one_immediate_ack
PiperOrigin-RevId: 290743392
Change-Id: I0c1a557d48f3b0b0c3e58c235d6c97dd79973c68
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index 9400a5e..70cd234 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -138,6 +138,7 @@
const QuicTag kMAD3 = TAG('M', 'A', 'D', '3'); // No min RTO
const QuicTag kMAD4 = TAG('M', 'A', 'D', '4'); // IETF style TLP
const QuicTag kMAD5 = TAG('M', 'A', 'D', '5'); // IETF style TLP with 2x mult
+const QuicTag k1ACK = TAG('1', 'A', 'C', 'K'); // 1 fast ack for reordering
const QuicTag kACD0 = TAG('A', 'D', 'D', '0'); // Disable ack decimation
const QuicTag kACKD = TAG('A', 'C', 'K', 'D'); // Ack decimation style acking.
const QuicTag kAKD2 = TAG('A', 'K', 'D', '2'); // Ack decimation tolerating
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 9e33ea4..4333323 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -12,6 +12,7 @@
#include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h"
#include "net/third_party/quiche/src/quic/core/quic_connection_stats.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
namespace quic {
@@ -50,6 +51,7 @@
ack_decimation_delay_(kAckDecimationDelay),
unlimited_ack_decimation_(false),
fast_ack_after_quiescence_(false),
+ one_immediate_ack_(false),
local_max_ack_delay_(
QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)),
ack_timeout_(QuicTime::Zero()),
@@ -88,6 +90,11 @@
if (config.HasClientSentConnectionOption(kACKQ, perspective)) {
fast_ack_after_quiescence_ = true;
}
+ if (GetQuicReloadableFlag(quic_one_immediate_ack) &&
+ config.HasClientSentConnectionOption(k1ACK, perspective)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_one_immediate_ack);
+ one_immediate_ack_ = true;
+ }
}
void QuicReceivedPacketManager::RecordPacketReceived(
@@ -315,6 +322,9 @@
}
bool QuicReceivedPacketManager::HasNewMissingPackets() const {
+ if (one_immediate_ack_) {
+ return HasMissingPackets() && ack_frame_.packets.LastIntervalLength() == 1;
+ }
return HasMissingPackets() &&
ack_frame_.packets.LastIntervalLength() <= kMaxPacketsAfterNewMissing;
}
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index 293b03c..92a9e2f 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -175,6 +175,8 @@
// When true, use a 1ms delayed ack timer if it's been an SRTT since a packet
// was received.
bool fast_ack_after_quiescence_;
+ // When true, only send 1 immediate ACK when reordering is detected.
+ bool one_immediate_ack_;
// The local node's maximum ack delay time. This is the maximum amount of
// time to wait before sending an acknowledgement.
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc
index 7355847..bfdf338 100644
--- a/quic/core/quic_received_packet_manager_test.cc
+++ b/quic/core/quic_received_packet_manager_test.cc
@@ -31,6 +31,11 @@
manager->fast_ack_after_quiescence_ = fast_ack_after_quiescence;
}
+ static void SetOneImmediateAck(QuicReceivedPacketManager* manager,
+ bool one_immediate_ack) {
+ manager->one_immediate_ack_ = one_immediate_ack;
+ }
+
static void SetAckDecimationDelay(QuicReceivedPacketManager* manager,
float ack_decimation_delay) {
manager->ack_decimation_delay_ = ack_decimation_delay;
@@ -247,6 +252,16 @@
// Delayed ack is scheduled.
CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
+ RecordPacketReceipt(5, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 5);
+ // Immediate ack is sent.
+ CheckAckTimeout(clock_.ApproximateNow());
+
+ RecordPacketReceipt(6, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 6);
+ // Immediate ack is scheduled, because 4 is still missing.
+ CheckAckTimeout(clock_.ApproximateNow());
+
RecordPacketReceipt(2, clock_.ApproximateNow());
MaybeUpdateAckTimeout(kInstigateAck, 2);
CheckAckTimeout(clock_.ApproximateNow());
@@ -256,10 +271,44 @@
// Should ack immediately, since this fills the last hole.
CheckAckTimeout(clock_.ApproximateNow());
- RecordPacketReceipt(4, clock_.ApproximateNow());
- MaybeUpdateAckTimeout(kInstigateAck, 4);
+ RecordPacketReceipt(7, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 7);
+ // Immediate ack is scheduled, because 4 is still missing.
+ CheckAckTimeout(clock_.ApproximateNow());
+}
+
+TEST_P(QuicReceivedPacketManagerTest, OutOfOrderReceiptCausesAckSent1Ack) {
+ QuicReceivedPacketManagerPeer::SetOneImmediateAck(&received_manager_, true);
+ EXPECT_FALSE(HasPendingAck());
+
+ RecordPacketReceipt(3, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 3);
// Delayed ack is scheduled.
CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
+
+ RecordPacketReceipt(5, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 5);
+ // Immediate ack is sent.
+ CheckAckTimeout(clock_.ApproximateNow());
+
+ RecordPacketReceipt(6, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 6);
+ // Delayed ack is scheduled.
+ CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
+
+ RecordPacketReceipt(2, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 2);
+ CheckAckTimeout(clock_.ApproximateNow());
+
+ RecordPacketReceipt(1, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 1);
+ // Should ack immediately, since this fills the last hole.
+ CheckAckTimeout(clock_.ApproximateNow());
+
+ RecordPacketReceipt(7, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 7);
+ // Delayed ack is scheduled, even though 4 is still missing.
+ CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
}
TEST_P(QuicReceivedPacketManagerTest, OutOfOrderAckReceiptCausesNoAck) {
