In quic, consider discarding keys as forward progress. protected by gfe2_reloadable_flag_quic_default_enable_5rto_blackhole_detection2 which replaces gfe2_reloadable_flag_quic_default_enable_5rto_blackhole_detection.
PiperOrigin-RevId: 315548676
Change-Id: Ia3db7908a74941108c20fa31d5cfe0c21e252bb9
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index e332a89..e7990d9 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2338,6 +2338,13 @@
sent_packet_manager_.NeuterUnencryptedPackets();
// This may have changed the retransmission timer, so re-arm it.
SetRetransmissionAlarm();
+ if (default_enable_5rto_blackhole_detection_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_enable_5rto_blackhole_detection2,
+ 1, 3);
+ // Consider this as forward progress since this is called when initial key
+ // gets discarded (or previous unencrypted data is not needed anymore).
+ OnForwardProgressMade();
+ }
if (SupportsMultiplePacketNumberSpaces()) {
// Stop sending ack of initial packet number space.
uber_received_packet_manager_.ResetAckStates(ENCRYPTION_INITIAL);
@@ -2889,6 +2896,11 @@
sent_packet_manager_.SetHandshakeConfirmed();
// This may have changed the retransmission timer, so re-arm it.
SetRetransmissionAlarm();
+ if (default_enable_5rto_blackhole_detection_) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_enable_5rto_blackhole_detection2,
+ 2, 3);
+ OnForwardProgressMade();
+ }
if (!SupportsMultiplePacketNumberSpaces()) {
// The client should immediately ack the SHLO to confirm the handshake is
// complete with the server.
@@ -4040,15 +4052,7 @@
// have a better estimate of the current rtt than when it was set.
SetRetransmissionAlarm();
if (acked_new_packet) {
- is_path_degrading_ = false;
- if (sent_packet_manager_.HasInFlightPackets()) {
- // Restart detections if forward progress has been made.
- blackhole_detector_.RestartDetection(GetPathDegradingDeadline(),
- GetNetworkBlackholeDeadline());
- } else {
- // Stop detections in quiecense.
- blackhole_detector_.StopDetection();
- }
+ OnForwardProgressMade();
}
if (send_stop_waiting) {
@@ -4346,6 +4350,18 @@
}
}
+void QuicConnection::OnForwardProgressMade() {
+ is_path_degrading_ = false;
+ if (sent_packet_manager_.HasInFlightPackets()) {
+ // Restart detections if forward progress has been made.
+ blackhole_detector_.RestartDetection(GetPathDegradingDeadline(),
+ GetNetworkBlackholeDeadline());
+ } else {
+ // Stop detections in quiecense.
+ blackhole_detector_.StopDetection();
+ }
+}
+
QuicPacketNumber QuicConnection::GetLargestReceivedPacketWithAck() const {
if (SupportsMultiplePacketNumberSpaces()) {
return largest_seen_packets_with_ack_[QuicUtils::GetPacketNumberSpace(
@@ -4545,7 +4561,8 @@
}
// No blackhole detection before handshake completes.
if (default_enable_5rto_blackhole_detection_) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_default_enable_5rto_blackhole_detection);
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_enable_5rto_blackhole_detection2,
+ 3, 3);
return IsHandshakeComplete();
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 5d149b4..56f84c2 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1261,6 +1261,10 @@
// received packet number which contains an ACK frame.
void SetLargestReceivedPacketWithAck(QuicPacketNumber new_value);
+ // Called when new packets have been acknowledged or old keys have been
+ // discarded.
+ void OnForwardProgressMade();
+
// Returns largest received packet number which contains an ACK frame.
QuicPacketNumber GetLargestReceivedPacketWithAck() const;
@@ -1701,7 +1705,7 @@
: GetQuicFlag(FLAGS_quic_anti_amplification_factor);
const bool default_enable_5rto_blackhole_detection_ =
- GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection);
+ GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2);
};
} // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 8d096eb..6602a8c 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -6117,7 +6117,7 @@
connection_options.push_back(k5RTO);
config.SetConnectionOptionsToSend(connection_options);
QuicConfigPeer::SetNegotiated(&config, true);
- if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
EXPECT_CALL(visitor_, GetHandshakeState())
.WillRepeatedly(Return(HANDSHAKE_COMPLETE));
}
@@ -9930,7 +9930,7 @@
}
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
connection_.SetFromConfig(config);
- if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
EXPECT_CALL(visitor_, GetHandshakeState())
.WillRepeatedly(Return(HANDSHAKE_COMPLETE));
}
@@ -9981,7 +9981,7 @@
}
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
connection_.SetFromConfig(config);
- if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
EXPECT_CALL(visitor_, GetHandshakeState())
.WillRepeatedly(Return(HANDSHAKE_COMPLETE));
}
@@ -10031,7 +10031,7 @@
config.SetConnectionOptionsToSend(connection_options);
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
connection_.SetFromConfig(config);
- if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
EXPECT_CALL(visitor_, GetHandshakeState())
.WillRepeatedly(Return(HANDSHAKE_COMPLETE));
}
@@ -10962,7 +10962,7 @@
}
TEST_P(QuicConnectionTest, ClientOnlyBlackholeDetectionClient) {
- if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
return;
}
QuicConfig config;
@@ -10983,7 +10983,7 @@
}
TEST_P(QuicConnectionTest, ClientOnlyBlackholeDetectionServer) {
- if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
return;
}
set_perspective(Perspective::IS_SERVER);
@@ -11009,7 +11009,7 @@
}
TEST_P(QuicConnectionTest, 2RtoBlackholeDetection) {
- if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
return;
}
QuicConfig config;
@@ -11036,7 +11036,7 @@
}
TEST_P(QuicConnectionTest, 3RtoBlackholeDetection) {
- if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
return;
}
QuicConfig config;
@@ -11063,7 +11063,7 @@
}
TEST_P(QuicConnectionTest, 4RtoBlackholeDetection) {
- if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
return;
}
QuicConfig config;
@@ -11090,7 +11090,7 @@
}
TEST_P(QuicConnectionTest, 6RtoBlackholeDetection) {
- if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection)) {
+ if (!GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
return;
}
QuicConfig config;
@@ -11116,6 +11116,49 @@
QuicConnectionPeer::GetBlackholeDetectionDeadline(&connection_));
}
+// Regresstion test for b/158491591.
+TEST_P(QuicConnectionTest, MadeForwardProgressOnDiscardingKeys) {
+ if (!connection_.SupportsMultiplePacketNumberSpaces()) {
+ return;
+ }
+ use_tagging_decrypter();
+ // Send handshake packet.
+ connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
+ std::make_unique<TaggingEncrypter>(0x02));
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+ EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
+ QuicConfig config;
+ QuicTagVector connection_options;
+ connection_options.push_back(k5RTO);
+ config.SetConnectionOptionsToSend(connection_options);
+ QuicConfigPeer::SetNegotiated(&config, true);
+ if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
+ EXPECT_CALL(visitor_, GetHandshakeState())
+ .WillRepeatedly(Return(HANDSHAKE_COMPLETE));
+ }
+ if (connection_.version().AuthenticatesHandshakeConnectionIds()) {
+ QuicConfigPeer::SetReceivedOriginalConnectionId(
+ &config, connection_.connection_id());
+ QuicConfigPeer::SetReceivedInitialSourceConnectionId(
+ &config, connection_.connection_id());
+ }
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ connection_.SetFromConfig(config);
+
+ connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE);
+ EXPECT_TRUE(connection_.BlackholeDetectionInProgress());
+ // Discard handshake keys.
+ connection_.OnHandshakeComplete();
+ if (GetQuicReloadableFlag(quic_default_enable_5rto_blackhole_detection2)) {
+ // Verify blackhole detection stops.
+ EXPECT_FALSE(connection_.BlackholeDetectionInProgress());
+ } else {
+ // Problematic: although there is nothing in flight, blackhole detection is
+ // still in progress.
+ EXPECT_TRUE(connection_.BlackholeDetectionInProgress());
+ }
+}
+
} // namespace
} // namespace test
} // namespace quic
