Internal QUICHE change PiperOrigin-RevId: 337213860 Change-Id: I01f6717bf203f64bf23b5af7d7f2905162514db8
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h index de9d9d3..ab0ef01 100644 --- a/quic/core/crypto/crypto_protocol.h +++ b/quic/core/crypto/crypto_protocol.h
@@ -163,7 +163,8 @@ // received before acking const QuicTag kACKQ = TAG('A', 'C', 'K', 'Q'); // Send an immediate ack after // 1 RTT of not receiving. -const QuicTag kAFFE = TAG('A', 'F', 'F', 'E'); // AckFrequencyFrame Enabled. +const QuicTag kAFFE = TAG('A', 'F', 'F', 'E'); // Enable client receiving + // AckFrequencyFrame. const QuicTag kSSLR = TAG('S', 'S', 'L', 'R'); // Slow Start Large Reduction. const QuicTag kNPRR = TAG('N', 'P', 'R', 'R'); // Pace at unity instead of PRR const QuicTag k2RTO = TAG('2', 'R', 'T', 'O'); // Close connection on 2 RTOs
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index f3b0eda..52992d0 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2631,6 +2631,15 @@ } const QuicFrames QuicConnection::MaybeBundleAckOpportunistically() { + if (!ack_frequency_sent_ && sent_packet_manager_.CanSendAckFrequency()) { + if (packet_creator_.NextSendingPacketNumber() >= + FirstSendingPacketNumber() + kMinReceivedBeforeAckDecimation) { + ack_frequency_sent_ = true; + auto frame = sent_packet_manager_.GetUpdatedAckFrequencyFrame(); + visitor_->SendAckFrequency(frame); + } + } + QuicFrames frames; const bool has_pending_ack = uber_received_packet_manager_
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index e02b573..3e74191 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -29,6 +29,7 @@ #include "net/third_party/quiche/src/quic/core/crypto/quic_decrypter.h" #include "net/third_party/quiche/src/quic/core/crypto/quic_encrypter.h" #include "net/third_party/quiche/src/quic/core/crypto/transport_parameters.h" +#include "net/third_party/quiche/src/quic/core/frames/quic_ack_frequency_frame.h" #include "net/third_party/quiche/src/quic/core/frames/quic_max_streams_frame.h" #include "net/third_party/quiche/src/quic/core/proto/cached_network_parameters_proto.h" #include "net/third_party/quiche/src/quic/core/quic_alarm.h" @@ -158,6 +159,9 @@ // Called when a ping needs to be sent. virtual void SendPing() = 0; + // Called when an AckFrequency frame need to be sent. + virtual void SendAckFrequency(const QuicAckFrequencyFrame& frame) = 0; + // Called to ask if the visitor wants to schedule write resumption as it both // has pending data to write, and is able to write (e.g. based on flow control // limits). @@ -1870,6 +1874,9 @@ // Indicate whether any ENCRYPTION_HANDSHAKE packet has been sent. bool handshake_packet_sent_ = false; + // Indicate whether AckFrequency frame has been sent. + bool ack_frequency_sent_ = false; + const bool fix_missing_initial_keys_ = GetQuicReloadableFlag(quic_fix_missing_initial_keys2);
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 132bee2..178eb2f 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -12135,6 +12135,47 @@ EXPECT_FALSE(connection_.GetDiscardPreviousOneRttKeysAlarm()->IsSet()); } +TEST_P(QuicConnectionTest, SendAckFrequencyFrame) { + if (!version().HasIetfQuicFrames()) { + return; + } + SetQuicReloadableFlag(quic_can_send_ack_frequency, true); + set_perspective(Perspective::IS_SERVER); + EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _)) + .Times(AnyNumber()); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(AnyNumber()); + + QuicConfig config; + QuicConfigPeer::SetReceivedMinAckDelayMs(&config, /*min_ack_delay_ms=*/1); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + connection_.SetFromConfig(config); + QuicConnectionPeer::SetAddressValidated(&connection_); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); + + connection_.OnHandshakeComplete(); + + writer_->SetWritable(); + QuicPacketCreatorPeer::SetPacketNumber(creator_, 99); + // Send packet 100 + SendStreamDataToPeer(/*id=*/1, "foo", /*offset=*/0, NO_FIN, nullptr); + + QuicAckFrequencyFrame captured_frame; + EXPECT_CALL(visitor_, SendAckFrequency(_)) + .WillOnce(Invoke([&captured_frame](const QuicAckFrequencyFrame& frame) { + captured_frame = frame; + })); + // Send packet 101. + SendStreamDataToPeer(/*id=*/1, "bar", /*offset=*/3, NO_FIN, nullptr); + + EXPECT_EQ(captured_frame.packet_tolerance, 10u); + EXPECT_EQ(captured_frame.max_ack_delay, + QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)); + + // Sending packet 102 does not trigger sending another AckFrequencyFrame. + SendStreamDataToPeer(/*id=*/1, "baz", /*offset=*/6, NO_FIN, nullptr); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_control_frame_manager.cc b/quic/core/quic_control_frame_manager.cc index 28b3446..fddf087 100644 --- a/quic/core/quic_control_frame_manager.cc +++ b/quic/core/quic_control_frame_manager.cc
@@ -120,13 +120,16 @@ } void QuicControlFrameManager::WriteOrBufferAckFrequency( - uint64_t packet_tolerance, - QuicTime::Delta max_ack_delay) { + const QuicAckFrequencyFrame& ack_frequency_frame) { QUIC_DVLOG(1) << "Writing ACK_FREQUENCY frame"; QuicControlFrameId control_frame_id = ++last_control_frame_id_; - WriteOrBufferQuicFrame(QuicFrame(new QuicAckFrequencyFrame( - control_frame_id, - /*sequence_number=*/control_frame_id, packet_tolerance, max_ack_delay))); + // Using the control_frame_id for sequence_number here leaves gaps in + // sequence_number. + WriteOrBufferQuicFrame( + QuicFrame(new QuicAckFrequencyFrame(control_frame_id, + /*sequence_number=*/control_frame_id, + ack_frequency_frame.packet_tolerance, + ack_frequency_frame.max_ack_delay))); } void QuicControlFrameManager::WritePing() {
diff --git a/quic/core/quic_control_frame_manager.h b/quic/core/quic_control_frame_manager.h index d79b679..493e7f0 100644 --- a/quic/core/quic_control_frame_manager.h +++ b/quic/core/quic_control_frame_manager.h
@@ -86,8 +86,8 @@ // Tries to send an AckFrequencyFrame. The frame is buffered if it cannot be // sent immediately. - void WriteOrBufferAckFrequency(uint64_t packet_tolerance, - QuicTime::Delta max_ack_delay); + void WriteOrBufferAckFrequency( + const QuicAckFrequencyFrame& ack_frequency_frame); // Sends a PING_FRAME. Do not send PING if there is buffered frames. void WritePing();
diff --git a/quic/core/quic_control_frame_manager_test.cc b/quic/core/quic_control_frame_manager_test.cc index 6817699..cf90e90 100644 --- a/quic/core/quic_control_frame_manager_test.cc +++ b/quic/core/quic_control_frame_manager_test.cc
@@ -243,16 +243,19 @@ manager_->OnCanWrite(); // Send AckFrequencyFrame as frame 6. - QuicAckFrequencyFrame ack_frequency = {6, 6, 10, - QuicTime::Delta::FromMilliseconds(24)}; - manager_->WriteOrBufferAckFrequency(10, - QuicTime::Delta::FromMilliseconds(24)); + QuicAckFrequencyFrame frame_to_send; + frame_to_send.packet_tolerance = 10; + frame_to_send.max_ack_delay = QuicTime::Delta::FromMilliseconds(24); + manager_->WriteOrBufferAckFrequency(frame_to_send); EXPECT_CALL(*connection_, SendControlFrame(_)) .WillOnce(Invoke(&ClearControlFrame)); manager_->OnCanWrite(); // Ack AckFrequencyFrame. - EXPECT_TRUE(manager_->OnControlFrameAcked(QuicFrame(&ack_frequency))); + QuicAckFrequencyFrame expected_ack_frequency = { + 6, 6, 10, QuicTime::Delta::FromMilliseconds(24)}; + EXPECT_TRUE( + manager_->OnControlFrameAcked(QuicFrame(&expected_ack_frequency))); } TEST_F(QuicControlFrameManagerTest, DonotRetransmitOldWindowUpdates) {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 4dc2582..106393b 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -5,13 +5,17 @@ #include "net/third_party/quiche/src/quic/core/quic_sent_packet_manager.h" #include <algorithm> +#include <cstddef> #include <string> #include "net/third_party/quiche/src/quic/core/congestion_control/general_loss_algorithm.h" #include "net/third_party/quiche/src/quic/core/congestion_control/pacing_sender.h" #include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h" +#include "net/third_party/quiche/src/quic/core/frames/quic_ack_frequency_frame.h" #include "net/third_party/quiche/src/quic/core/proto/cached_network_parameters_proto.h" #include "net/third_party/quiche/src/quic/core/quic_connection_stats.h" +#include "net/third_party/quiche/src/quic/core/quic_constants.h" +#include "net/third_party/quiche/src/quic/core/quic_packet_number.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" @@ -55,6 +59,11 @@ // losses. static const uint32_t kConservativeUnpacedBurst = 2; +// TODO(haoyuewang) Unify this constant and kAckDecimationDelay in +// quic_received_packet_manager.cc. +// Ack delay as (RTT >> kAckDelayShift). +static const int kAckDelayShift = 2; + } // namespace #define ENDPOINT \ @@ -140,6 +149,13 @@ peer_max_ack_delay_ = QuicTime::Delta::FromMilliseconds(config.ReceivedMaxAckDelayMs()); } + if (GetQuicReloadableFlag(quic_can_send_ack_frequency) && + perspective == Perspective::IS_SERVER) { + if (config.HasReceivedMinAckDelayMs()) { + peer_min_ack_delay_ = + QuicTime::Delta::FromMilliseconds(config.ReceivedMinAckDelayMs()); + } + } if (config.HasClientSentConnectionOption(kMAD0, perspective)) { rtt_stats_.set_ignore_max_ack_delay(true); } @@ -700,6 +716,27 @@ info->state = ACKED; } +bool QuicSentPacketManager::CanSendAckFrequency() const { + return !peer_min_ack_delay_.IsInfinite() && handshake_finished_; +} + +QuicAckFrequencyFrame QuicSentPacketManager::GetUpdatedAckFrequencyFrame() + const { + QuicAckFrequencyFrame frame; + if (!CanSendAckFrequency()) { + QUIC_BUG << "New AckFrequencyFrame is created while it shouldn't."; + return frame; + } + + QUIC_RELOADABLE_FLAG_COUNT(quic_can_send_ack_frequency); + frame.packet_tolerance = kMaxRetransmittablePacketsBeforeAck; + auto rtt = rtt_stats_.MinOrInitialRtt(); + frame.max_ack_delay = rtt >> kAckDelayShift; + frame.max_ack_delay = std::max(frame.max_ack_delay, peer_min_ack_delay_); + + return frame; +} + bool QuicSentPacketManager::OnPacketSent( SerializedPacket* mutable_packet, QuicTime sent_time,
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index b332034..ef5972a 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -22,6 +22,7 @@ #include "net/third_party/quiche/src/quic/core/quic_circular_deque.h" #include "net/third_party/quiche/src/quic/core/quic_packets.h" #include "net/third_party/quiche/src/quic/core/quic_sustained_bandwidth_recorder.h" +#include "net/third_party/quiche/src/quic/core/quic_time.h" #include "net/third_party/quiche/src/quic/core/quic_transmission_info.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_unacked_packet_map.h" @@ -197,6 +198,10 @@ HasRetransmittableData has_retransmittable_data, bool measure_rtt); + bool CanSendAckFrequency() const; + + QuicAckFrequencyFrame GetUpdatedAckFrequencyFrame() const; + // Called when the retransmission timer expires and returns the retransmission // mode. RetransmissionTimeoutMode OnRetransmissionTimeout(); @@ -650,6 +655,10 @@ // negotiation or subsequently by AckFrequencyFrame. QuicTime::Delta peer_max_ack_delay_; + // Peer sends min_ack_delay in TransportParameter to advertise its support for + // AckFrequencyFrame. + QuicTime::Delta peer_min_ack_delay_ = QuicTime::Delta::Infinite(); + // The history of outstanding max_ack_delays sent to peer. Outstanding means // a max_ack_delay is sent as part of the last acked AckFrequencyFrame or // an unacked AckFrequencyFrame after that.
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index ef651a7..4bb16a3 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -10,6 +10,7 @@ #include "absl/strings/string_view.h" #include "net/third_party/quiche/src/quic/core/frames/quic_ack_frequency_frame.h" #include "net/third_party/quiche/src/quic/core/quic_time.h" +#include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" @@ -4419,6 +4420,33 @@ EXPECT_EQ(message_frame->message_length, 0); } +TEST_F(QuicSentPacketManagerTest, SendAckFrequencyFrame) { + SetQuicReloadableFlag(quic_can_send_ack_frequency, true); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); + QuicConfig config; + QuicConfigPeer::SetReceivedMinAckDelayMs(&config, /*min_ack_delay_ms=*/1); + manager_.SetFromConfig(config); + manager_.SetHandshakeConfirmed(); + + // Set up RTTs. + auto* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); + rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(8), + /*ack_delay=*/QuicTime::Delta::Zero(), + /*now=*/QuicTime::Zero()); + // Make sure srtt and min_rtt are different. + rtt_stats->UpdateRtt( + QuicTime::Delta::FromMilliseconds(16), + /*ack_delay=*/QuicTime::Delta::Zero(), + /*now=*/QuicTime::Zero() + QuicTime::Delta::FromMilliseconds(24)); + + auto frame = manager_.GetUpdatedAckFrequencyFrame(); + EXPECT_EQ(frame.max_ack_delay, + std::max(rtt_stats->min_rtt() * 0.25, + QuicTime::Delta::FromMilliseconds(1u))); + EXPECT_EQ(frame.packet_tolerance, 10u); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 84f0a7f..9705710 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -9,6 +9,7 @@ #include <utility> #include "absl/strings/string_view.h" +#include "net/third_party/quiche/src/quic/core/frames/quic_ack_frequency_frame.h" #include "net/third_party/quiche/src/quic/core/quic_connection.h" #include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_flow_controller.h" @@ -2003,6 +2004,10 @@ control_frame_manager_.WritePing(); } +void QuicSession::SendAckFrequency(const QuicAckFrequencyFrame& frame) { + control_frame_manager_.WriteOrBufferAckFrequency(frame); +} + bool QuicSession::IsConnectionFlowControlBlocked() const { return flow_controller_.IsBlocked(); }
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index e84124e..3c04c77 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -16,6 +16,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "net/third_party/quiche/src/quic/core/frames/quic_ack_frequency_frame.h" #include "net/third_party/quiche/src/quic/core/handshaker_delegate_interface.h" #include "net/third_party/quiche/src/quic/core/legacy_quic_stream_id_manager.h" #include "net/third_party/quiche/src/quic/core/quic_connection.h" @@ -123,6 +124,7 @@ // Adds a connection level WINDOW_UPDATE frame. void OnAckNeedsRetransmittableFrame() override; void SendPing() override; + void SendAckFrequency(const QuicAckFrequencyFrame& frame) override; bool WillingAndAbleToWrite() const override; std::string GetStreamsInfoForLogging() const override; void OnPathDegrading() override;
diff --git a/quic/test_tools/quic_config_peer.cc b/quic/test_tools/quic_config_peer.cc index 52325d1..b10453e 100644 --- a/quic/test_tools/quic_config_peer.cc +++ b/quic/test_tools/quic_config_peer.cc
@@ -99,6 +99,12 @@ } // static +void QuicConfigPeer::SetReceivedMinAckDelayMs(QuicConfig* config, + uint32_t min_ack_delay_ms) { + config->min_ack_delay_ms_.SetReceivedValue(min_ack_delay_ms); +} + +// static void QuicConfigPeer::SetNegotiated(QuicConfig* config, bool negotiated) { config->negotiated_ = negotiated; }
diff --git a/quic/test_tools/quic_config_peer.h b/quic/test_tools/quic_config_peer.h index 109bd64..98c35a1 100644 --- a/quic/test_tools/quic_config_peer.h +++ b/quic/test_tools/quic_config_peer.h
@@ -60,6 +60,9 @@ static void SetReceivedMaxPacketSize(QuicConfig* config, uint32_t max_udp_payload_size); + static void SetReceivedMinAckDelayMs(QuicConfig* config, + uint32_t min_ack_delay_ms); + static void SetNegotiated(QuicConfig* config, bool negotiated); static void SetReceivedOriginalConnectionId(
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index 30798c7..3539628 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -560,6 +560,10 @@ (override)); MOCK_METHOD(void, OnAckNeedsRetransmittableFrame, (), (override)); MOCK_METHOD(void, SendPing, (), (override)); + MOCK_METHOD(void, + SendAckFrequency, + (const QuicAckFrequencyFrame& frame), + (override)); MOCK_METHOD(bool, AllowSelfAddressChange, (), (const, override)); MOCK_METHOD(HandshakeState, GetHandshakeState, (), (const, override)); MOCK_METHOD(bool,
diff --git a/quic/test_tools/simulator/quic_endpoint.h b/quic/test_tools/simulator/quic_endpoint.h index faf2bfd..7968900 100644 --- a/quic/test_tools/simulator/quic_endpoint.h +++ b/quic/test_tools/simulator/quic_endpoint.h
@@ -80,6 +80,7 @@ void OnForwardProgressMadeAfterPathDegrading() override {} void OnAckNeedsRetransmittableFrame() override {} void SendPing() override {} + void SendAckFrequency(const QuicAckFrequencyFrame& /*frame*/) override {} bool AllowSelfAddressChange() const override; HandshakeState GetHandshakeState() const override; bool OnMaxStreamsFrame(const QuicMaxStreamsFrame& /*frame*/) override {