When retransmittable-on-wire timeout, re-send 1st 1-RTT packet or random bytes (which are behind client connection options ROWF and ROWR, respectively). PiperOrigin-RevId: 443493674
diff --git a/quiche/quic/core/crypto/crypto_protocol.h b/quiche/quic/core/crypto/crypto_protocol.h index aef3ff1..6ca1797 100644 --- a/quiche/quic/core/crypto/crypto_protocol.h +++ b/quiche/quic/core/crypto/crypto_protocol.h
@@ -426,6 +426,10 @@ const QuicTag kSRWP = TAG('S', 'R', 'W', 'P'); // Enable retransmittable on // wire PING (ROWP) on the // server side. +const QuicTag kROWF = TAG('R', 'O', 'W', 'F'); // Send first 1-RTT packet on + // ROWP timeout. +const QuicTag kROWR = TAG('R', 'O', 'W', 'R'); // Send random bytes on ROWP + // timeout. const QuicTag kGSR0 = TAG('G', 'S', 'R', '0'); // Selective Resumption const QuicTag kNRES = TAG('N', 'R', 'E', 'S'); // No resumption
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index db2597b..e0956f4 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -589,6 +589,16 @@ if (config.HasClientRequestedIndependentOption(kFIDT, perspective_)) { idle_network_detector_.enable_shorter_idle_timeout_on_sent_packet(); } + if (perspective_ == Perspective::IS_CLIENT && version().HasIetfQuicFrames()) { + // Only conduct those experiments in IETF QUIC because random packets may + // elicit reset and gQUIC PUBLIC_RESET will cause connection close. + if (config.HasClientRequestedIndependentOption(kROWF, perspective_)) { + retransmittable_on_wire_behavior_ = SEND_FIRST_FORWARD_SECURE_PACKET; + } + if (config.HasClientRequestedIndependentOption(kROWR, perspective_)) { + retransmittable_on_wire_behavior_ = SEND_RANDOM_BYTES; + } + } if (config.HasClientRequestedIndependentOption(k3AFF, perspective_)) { anti_amplification_factor_ = 3; } @@ -3925,6 +3935,12 @@ } else { consecutive_num_packets_with_no_retransmittable_frames_ = 0; } + if (retransmittable_on_wire_behavior_ == SEND_FIRST_FORWARD_SECURE_PACKET && + first_serialized_one_rtt_packet_ == nullptr && + serialized_packet.encryption_level == ENCRYPTION_FORWARD_SECURE) { + first_serialized_one_rtt_packet_ = std::make_unique<BufferedPacket>( + serialized_packet, self_address(), peer_address()); + } SendOrQueuePacket(std::move(serialized_packet)); } @@ -4929,6 +4945,17 @@ memcpy(data.get(), encrypted_buffer, encrypted_length); } +QuicConnection::BufferedPacket::BufferedPacket( + QuicRandom& random, QuicPacketLength encrypted_length, + const QuicSocketAddress& self_address, + const QuicSocketAddress& peer_address) + : length(encrypted_length), + self_address(self_address), + peer_address(peer_address) { + data = std::make_unique<char[]>(encrypted_length); + random.RandBytes(data.get(), encrypted_length); +} + HasRetransmittableData QuicConnection::IsRetransmittable( const SerializedPacket& packet) { // Retransmitted packets retransmittable frames are owned by the unacked @@ -6278,6 +6305,42 @@ !visitor_->ShouldKeepConnectionAlive()) { return; } + bool packet_buffered = false; + switch (retransmittable_on_wire_behavior_) { + case DEFAULT: + break; + case SEND_FIRST_FORWARD_SECURE_PACKET: + if (first_serialized_one_rtt_packet_ != nullptr) { + buffered_packets_.emplace_back( + first_serialized_one_rtt_packet_->data.get(), + first_serialized_one_rtt_packet_->length, self_address(), + peer_address()); + packet_buffered = true; + } + break; + case SEND_RANDOM_BYTES: + const QuicPacketLength random_bytes_length = + std::max(QuicFramer::GetMinStatelessResetPacketLength() + 1, + random_generator_->RandUint64() % + packet_creator_.max_packet_length()); + buffered_packets_.emplace_back(*random_generator_, random_bytes_length, + self_address(), peer_address()); + packet_buffered = true; + break; + } + if (packet_buffered) { + if (!writer_->IsWriteBlocked()) { + WriteQueuedPackets(); + } + if (connected_) { + // Always reset PING alarm with has_in_flight_packets=true. This is used + // to avoid re-arming the alarm in retransmittable-on-wire mode. + ping_manager_.SetAlarm(clock_->ApproximateNow(), + visitor_->ShouldKeepConnectionAlive(), + /*has_in_flight_packets=*/true); + } + return; + } SendPingAtLevel(framer().GetEncryptionLevelToSendApplicationData()); }
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index b73f5c4..d7b83e5 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1338,6 +1338,12 @@ private: friend class test::QuicConnectionPeer; + enum RetransmittableOnWireBehavior { + DEFAULT, // Send packet containing a PING frame. + SEND_FIRST_FORWARD_SECURE_PACKET, // Send 1st 1-RTT packet. + SEND_RANDOM_BYTES // Send random bytes which is an unprocessable packet. + }; + struct QUIC_EXPORT_PRIVATE PendingPathChallenge { QuicPathFrameBuffer received_path_challenge; QuicSocketAddress peer_address; @@ -1399,6 +1405,11 @@ QuicPacketLength encrypted_length, const QuicSocketAddress& self_address, const QuicSocketAddress& peer_address); + // Please note, this buffered packet contains random bytes (and is not + // *actually* a QUIC packet). + BufferedPacket(QuicRandom& random, QuicPacketLength encrypted_length, + const QuicSocketAddress& self_address, + const QuicSocketAddress& peer_address); BufferedPacket(const BufferedPacket& other) = delete; BufferedPacket(const BufferedPacket&& other) = delete; @@ -2252,6 +2263,11 @@ QuicPingManager ping_manager_; + // Records first serialized 1-RTT packet. + std::unique_ptr<BufferedPacket> first_serialized_one_rtt_packet_; + + RetransmittableOnWireBehavior retransmittable_on_wire_behavior_ = DEFAULT; + // TODO(b/205023946) Debug-only fields, to be deprecated after the bug is // fixed. absl::optional<QuicWallTime> quic_bug_10511_43_timestamp_;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 97f69f3..b77cc15 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -1285,6 +1285,10 @@ void SimulateNextPacketTooLarge() { writer_->SimulateNextPacketTooLarge(); } + void ExpectNextPacketUnprocessable() { + writer_->ExpectNextPacketUnprocessable(); + } + void AlwaysGetPacketTooLarge() { writer_->AlwaysGetPacketTooLarge(); } void SetWritePauseTimeDelta(QuicTime::Delta delta) { @@ -7646,6 +7650,150 @@ connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); } +TEST_P(QuicConnectionTest, RetransmittableOnWireSendFirstPacket) { + if (!GetQuicReloadableFlag(quic_use_ping_manager) || + !VersionHasIetfQuicFrames(connection_.version().transport_version)) { + return; + } + EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) + .WillRepeatedly(Return(true)); + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + + const QuicTime::Delta kRetransmittableOnWireTimeout = + QuicTime::Delta::FromMilliseconds(200); + const QuicTime::Delta kTestRtt = QuicTime::Delta::FromMilliseconds(100); + + connection_.set_initial_retransmittable_on_wire_timeout( + kRetransmittableOnWireTimeout); + + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + QuicConfig config; + QuicTagVector connection_options; + connection_options.push_back(kROWF); + config.SetClientConnectionOptions(connection_options); + connection_.SetFromConfig(config); + + // Send a request. + connection_.SendStreamDataWithString(3, "foo", 0, NO_FIN); + // Receive an ACK after 1-RTT. + clock_.AdvanceTime(kTestRtt); + EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); + QuicAckFrame frame = + InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(2)}}); + ProcessAckPacket(&frame); + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(kRetransmittableOnWireTimeout, + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + EXPECT_EQ(1u, writer_->packets_write_attempts()); + + // Fire retransmittable-on-wire alarm. + clock_.AdvanceTime(kRetransmittableOnWireTimeout); + connection_.GetPingAlarm()->Fire(); + EXPECT_EQ(2u, writer_->packets_write_attempts()); + // Verify alarm is set in keep-alive mode. + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(QuicTime::Delta::FromSeconds(kPingTimeoutSecs), + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); +} + +TEST_P(QuicConnectionTest, RetransmittableOnWireSendRandomBytes) { + if (!GetQuicReloadableFlag(quic_use_ping_manager) || + !VersionHasIetfQuicFrames(connection_.version().transport_version)) { + return; + } + EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) + .WillRepeatedly(Return(true)); + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + + const QuicTime::Delta kRetransmittableOnWireTimeout = + QuicTime::Delta::FromMilliseconds(200); + const QuicTime::Delta kTestRtt = QuicTime::Delta::FromMilliseconds(100); + + connection_.set_initial_retransmittable_on_wire_timeout( + kRetransmittableOnWireTimeout); + + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + QuicConfig config; + QuicTagVector connection_options; + connection_options.push_back(kROWR); + config.SetClientConnectionOptions(connection_options); + connection_.SetFromConfig(config); + + // Send a request. + connection_.SendStreamDataWithString(3, "foo", 0, NO_FIN); + // Receive an ACK after 1-RTT. + clock_.AdvanceTime(kTestRtt); + EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); + QuicAckFrame frame = + InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(2)}}); + ProcessAckPacket(&frame); + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(kRetransmittableOnWireTimeout, + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + EXPECT_EQ(1u, writer_->packets_write_attempts()); + + // Fire retransmittable-on-wire alarm. + clock_.AdvanceTime(kRetransmittableOnWireTimeout); + // Next packet is not processable by the framer in the test writer. + ExpectNextPacketUnprocessable(); + connection_.GetPingAlarm()->Fire(); + EXPECT_EQ(2u, writer_->packets_write_attempts()); + // Verify alarm is set in keep-alive mode. + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(QuicTime::Delta::FromSeconds(kPingTimeoutSecs), + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); +} + +TEST_P(QuicConnectionTest, + RetransmittableOnWireSendRandomBytesWithWriterBlocked) { + if (!GetQuicReloadableFlag(quic_use_ping_manager) || + !VersionHasIetfQuicFrames(connection_.version().transport_version)) { + return; + } + EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) + .WillRepeatedly(Return(true)); + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); + + const QuicTime::Delta kRetransmittableOnWireTimeout = + QuicTime::Delta::FromMilliseconds(200); + const QuicTime::Delta kTestRtt = QuicTime::Delta::FromMilliseconds(100); + + connection_.set_initial_retransmittable_on_wire_timeout( + kRetransmittableOnWireTimeout); + + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + QuicConfig config; + QuicTagVector connection_options; + connection_options.push_back(kROWR); + config.SetClientConnectionOptions(connection_options); + connection_.SetFromConfig(config); + + // Send a request. + connection_.SendStreamDataWithString(3, "foo", 0, NO_FIN); + // Receive an ACK after 1-RTT. + clock_.AdvanceTime(kTestRtt); + EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); + QuicAckFrame frame = + InitAckFrame({{QuicPacketNumber(1), QuicPacketNumber(2)}}); + ProcessAckPacket(&frame); + ASSERT_TRUE(connection_.GetPingAlarm()->IsSet()); + EXPECT_EQ(kRetransmittableOnWireTimeout, + connection_.GetPingAlarm()->deadline() - clock_.ApproximateNow()); + EXPECT_EQ(1u, writer_->packets_write_attempts()); + // Receive an out of order data packet and block the ACK packet. + BlockOnNextWrite(); + ProcessDataPacket(3); + EXPECT_EQ(2u, writer_->packets_write_attempts()); + EXPECT_EQ(1u, connection_.NumQueuedPackets()); + + // Fire retransmittable-on-wire alarm. + clock_.AdvanceTime(kRetransmittableOnWireTimeout); + connection_.GetPingAlarm()->Fire(); + // Verify the random bytes packet gets queued. + EXPECT_EQ(2u, connection_.NumQueuedPackets()); +} + // 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.
diff --git a/quiche/quic/test_tools/quic_test_utils.cc b/quiche/quic/test_tools/quic_test_utils.cc index b182dc5..741159a 100644 --- a/quiche/quic/test_tools/quic_test_utils.cc +++ b/quiche/quic/test_tools/quic_test_utils.cc
@@ -1335,9 +1335,10 @@ ENCRYPTION_FORWARD_SECURE, std::make_unique<NullDecrypter>(framer_.framer()->perspective())); } - EXPECT_TRUE(framer_.ProcessPacket(packet)) + EXPECT_EQ(next_packet_processable_, framer_.ProcessPacket(packet)) << framer_.framer()->detailed_error() << " perspective " << framer_.framer()->perspective(); + next_packet_processable_ = true; if (block_on_next_write_) { write_blocked_ = true; block_on_next_write_ = false;
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h index b7bda22..e074008 100644 --- a/quiche/quic/test_tools/quic_test_utils.h +++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -1777,6 +1777,8 @@ void SimulateNextPacketTooLarge() { next_packet_too_large_ = true; } + void ExpectNextPacketUnprocessable() { next_packet_processable_ = false; } + void AlwaysGetPacketTooLarge() { always_get_packet_too_large_ = true; } // Sets the amount of time that the writer should before the actual write. @@ -1923,6 +1925,7 @@ bool block_on_next_flush_ = false; bool block_on_next_write_ = false; bool next_packet_too_large_ = false; + bool next_packet_processable_ = true; bool always_get_packet_too_large_ = false; bool is_write_blocked_data_buffered_ = false; bool is_batch_mode_ = false;