In QUIC, do not check amplification limit if there is pending timer credit. This would guarantee CRYPTO frame be retransmitted because of
1) PTO fires
2) bundled with outgoing ACKs.
Protected by FLAGS_quic_reloadable_flag_quic_donot_check_amplification_limit_with_pending_timer_credit.
PiperOrigin-RevId: 408392907
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 7c4f004..87b1c86 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -3300,7 +3300,12 @@
return packet_creator_.HasSoftMaxPacketLength();
}
- if (LimitedByAmplificationFactor()) {
+ const bool donot_check_amplification_limit_with_pending_timer_credit =
+ GetQuicReloadableFlag(
+ quic_donot_check_amplification_limit_with_pending_timer_credit);
+
+ if (!donot_check_amplification_limit_with_pending_timer_credit &&
+ LimitedByAmplificationFactor()) {
// Server is constrained by the amplification restriction.
QUIC_CODE_COUNT(quic_throttled_by_amplification_limit);
QUIC_DVLOG(1) << ENDPOINT
@@ -3314,10 +3319,31 @@
}
if (sent_packet_manager_.pending_timer_transmission_count() > 0) {
- // Force sending the retransmissions for HANDSHAKE, TLP, RTO, PROBING cases.
+ // Allow sending if there are pending tokens, which occurs when:
+ // 1) firing PTO,
+ // 2) bundling CRYPTO data with ACKs,
+ // 3) coalescing CRYPTO data of higher space.
return true;
}
+ if (donot_check_amplification_limit_with_pending_timer_credit) {
+ QUIC_RELOADABLE_FLAG_COUNT(
+ quic_donot_check_amplification_limit_with_pending_timer_credit);
+ if (LimitedByAmplificationFactor()) {
+ // Server is constrained by the amplification restriction.
+ QUIC_CODE_COUNT(quic_throttled_by_amplification_limit);
+ QUIC_DVLOG(1)
+ << ENDPOINT
+ << "Constrained by amplification restriction to peer address "
+ << default_path_.peer_address << " bytes received "
+ << default_path_.bytes_received_before_address_validation
+ << ", bytes sent"
+ << default_path_.bytes_sent_before_address_validation;
+ ++stats_.num_amplification_throttling;
+ return false;
+ }
+ }
+
if (HandleWriteBlocked()) {
return false;
}
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index a6ed2a1..8879a32 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -763,7 +763,6 @@
std::unique_ptr<QuicDecrypter> decrypter) {
if (connection_.version().KnowsWhichDecrypterToUse()) {
connection_.InstallDecrypter(level, std::move(decrypter));
- connection_.RemoveDecrypter(ENCRYPTION_INITIAL);
} else {
connection_.SetDecrypter(level, std::move(decrypter));
}
@@ -898,7 +897,6 @@
connection_.InstallDecrypter(
QuicPacketCreatorPeer::GetEncryptionLevel(&peer_creator_),
std::make_unique<StrictTaggingDecrypter>(0x01));
- connection_.RemoveDecrypter(ENCRYPTION_INITIAL);
} else {
connection_.SetDecrypter(
QuicPacketCreatorPeer::GetEncryptionLevel(&peer_creator_),
@@ -15560,6 +15558,94 @@
}
}
+// Regression test for b/201643321.
+TEST_P(QuicConnectionTest, FailedToRetransmitShlo) {
+ if (!version().HasIetfQuicFrames()) {
+ return;
+ }
+ set_perspective(Perspective::IS_SERVER);
+ EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber());
+ EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
+ use_tagging_decrypter();
+ // Received INITIAL 1.
+ ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
+ EXPECT_TRUE(connection_.HasPendingAcks());
+
+ peer_framer_.SetEncrypter(ENCRYPTION_ZERO_RTT,
+ std::make_unique<TaggingEncrypter>(0x02));
+
+ connection_.SetEncrypter(ENCRYPTION_INITIAL,
+ std::make_unique<TaggingEncrypter>(0x01));
+ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<TaggingEncrypter>(0x03));
+ SetDecrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<StrictTaggingDecrypter>(0x02));
+ SetDecrypter(ENCRYPTION_ZERO_RTT,
+ std::make_unique<StrictTaggingDecrypter>(0x02));
+ connection_.SetEncrypter(ENCRYPTION_FORWARD_SECURE,
+ std::make_unique<TaggingEncrypter>(0x04));
+ // Received ENCRYPTION_ZERO_RTT 1.
+ ProcessDataPacketAtLevel(1, !kHasStopWaiting, ENCRYPTION_ZERO_RTT);
+ {
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
+ // Send INITIAL 1.
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_INITIAL);
+ connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_INITIAL);
+ // Send HANDSHAKE 2.
+ EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+ connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE);
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ // Send half RTT data to exhaust amplification credit.
+ connection_.SendStreamDataWithString(0, std::string(100 * 1024, 'a'), 0,
+ NO_FIN);
+ }
+ // Received INITIAL 2.
+ ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL);
+ ASSERT_TRUE(connection_.HasPendingAcks());
+ // Verify ACK delay is 1ms.
+ EXPECT_EQ(clock_.Now() + kAlarmGranularity,
+ connection_.GetAckAlarm()->deadline());
+ if (!GetQuicReloadableFlag(
+ quic_donot_check_amplification_limit_with_pending_timer_credit)) {
+ // ACK is not sent because of amplification limit throttled.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0);
+ } else {
+ // ACK is not throttled by amplification limit, and SHLO is bundled. Also
+ // HANDSHAKE packet gets coalesced.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
+ }
+ // ACK alarm fires.
+ clock_.AdvanceTime(kAlarmGranularity);
+ connection_.GetAckAlarm()->Fire();
+ if (GetQuicReloadableFlag(
+ quic_donot_check_amplification_limit_with_pending_timer_credit)) {
+ // Verify HANDSHAKE packet is coalesced with INITIAL ACK + SHLO.
+ EXPECT_EQ(0x03030303u, writer_->final_bytes_of_last_packet());
+ // Only the first packet in the coalesced packet has been processed,
+ // verify SHLO is bundled with INITIAL ACK.
+ EXPECT_EQ(1u, writer_->ack_frames().size());
+ EXPECT_EQ(1u, writer_->crypto_frames().size());
+ // Process the coalesced HANDSHAKE packet.
+ ASSERT_TRUE(writer_->coalesced_packet() != nullptr);
+ auto packet = writer_->coalesced_packet()->Clone();
+ writer_->framer()->ProcessPacket(*packet);
+ EXPECT_EQ(0u, writer_->ack_frames().size());
+ EXPECT_EQ(1u, writer_->crypto_frames().size());
+ ASSERT_TRUE(writer_->coalesced_packet() == nullptr);
+ }
+
+ // Received INITIAL 3.
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(AnyNumber());
+ ProcessCryptoPacketAtLevel(3, ENCRYPTION_INITIAL);
+ if (!GetQuicReloadableFlag(
+ quic_donot_check_amplification_limit_with_pending_timer_credit)) {
+ EXPECT_FALSE(connection_.HasPendingAcks());
+ } else {
+ EXPECT_TRUE(connection_.HasPendingAcks());
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 5105d90..d625fd0 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -15,6 +15,8 @@
QUIC_FLAG(FLAGS_quic_restart_flag_quic_testonly_default_false, false)
// A testonly restart flag that will always default to true.
QUIC_FLAG(FLAGS_quic_restart_flag_quic_testonly_default_true, true)
+// Donot check amplification limit if there is available pending_timer_transmission_count.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_donot_check_amplification_limit_with_pending_timer_credit, false)
// If bytes in flight has dipped below 1.25*MaxBW in the last round, do not exit PROBE_UP due to excess queue buildup.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bbr2_no_probe_up_exit_if_no_queue, true)
// If true and connection option B201 is used, check if cwnd limited before aggregation epoch, instead of ack event, in PROBE_UP phase.
