Support sender control of ack frequency step2: (1) add min_ack_delay transport parameters. (2) client send min_ack_delay by default. protected by gfe2_reloadable_flag_quic_record_received_min_ack_delay. PiperOrigin-RevId: 321468850 Change-Id: I71a9b8ab272400a80a1af091f92e440172885dc5
diff --git a/quic/core/crypto/transport_parameters.cc b/quic/core/crypto/transport_parameters.cc index 330a14d..d2a0edf 100644 --- a/quic/core/crypto/transport_parameters.cc +++ b/quic/core/crypto/transport_parameters.cc
@@ -60,6 +60,8 @@ kGoogleQuicParam = 18257, // Used for non-standard Google-specific params. kGoogleQuicVersion = 18258, // Used to transmit version and supported_versions. + + kMinAckDelay = 0xde1a // An IETF QUIC extension. }; namespace { @@ -127,6 +129,8 @@ return "google"; case TransportParameters::kGoogleQuicVersion: return "google-version"; + case TransportParameters::kMinAckDelay: + return "min_ack_delay_us"; } return "Unknown(" + quiche::QuicheTextUtils::Uint64ToString(param_id) + ")"; } @@ -158,6 +162,7 @@ case TransportParameters::kGoogleSupportHandshakeDone: case TransportParameters::kGoogleQuicParam: case TransportParameters::kGoogleQuicVersion: + case TransportParameters::kMinAckDelay: return true; } return false; @@ -435,6 +440,7 @@ rv += initial_max_streams_uni.ToString(/*for_use_in_list=*/true); rv += ack_delay_exponent.ToString(/*for_use_in_list=*/true); rv += max_ack_delay.ToString(/*for_use_in_list=*/true); + rv += min_ack_delay_us.ToString(/*for_use_in_list=*/true); if (disable_active_migration) { rv += " " + TransportParameterIdToString(kDisableActiveMigration); } @@ -513,6 +519,10 @@ kDefaultMaxAckDelayTransportParam, 0, kMaxMaxAckDelayTransportParam), + min_ack_delay_us(kMinAckDelay, + 0, + 0, + kMaxMaxAckDelayTransportParam * kNumMicrosPerMilli), disable_active_migration(false), active_connection_id_limit(kActiveConnectionIdLimit, kDefaultActiveConnectionIdLimitTransportParam, @@ -546,6 +556,7 @@ initial_max_streams_uni(other.initial_max_streams_uni), ack_delay_exponent(other.ack_delay_exponent), max_ack_delay(other.max_ack_delay), + min_ack_delay_us(other.min_ack_delay_us), disable_active_migration(other.disable_active_migration), active_connection_id_limit(other.active_connection_id_limit), initial_source_connection_id(other.initial_source_connection_id), @@ -587,6 +598,7 @@ rhs.initial_max_streams_uni.value() && ack_delay_exponent.value() == rhs.ack_delay_exponent.value() && max_ack_delay.value() == rhs.max_ack_delay.value() && + min_ack_delay_us.value() == rhs.min_ack_delay_us.value() && disable_active_migration == rhs.disable_active_migration && active_connection_id_limit.value() == rhs.active_connection_id_limit.value() && @@ -691,7 +703,7 @@ initial_max_stream_data_uni.IsValid() && initial_max_streams_bidi.IsValid() && initial_max_streams_uni.IsValid() && ack_delay_exponent.IsValid() && max_ack_delay.IsValid() && - active_connection_id_limit.IsValid() && + min_ack_delay_us.IsValid() && active_connection_id_limit.IsValid() && max_datagram_frame_size.IsValid() && initial_round_trip_time_us.IsValid(); if (!ok) { *error_details = "Invalid transport parameters " + this->ToString(); @@ -749,6 +761,7 @@ kIntegerParameterLength + // initial_max_streams_uni kIntegerParameterLength + // ack_delay_exponent kIntegerParameterLength + // max_ack_delay + kIntegerParameterLength + // min_ack_delay_us kTypeAndValueLength + // disable_active_migration kPreferredAddressParameterLength + // preferred_address kIntegerParameterLength + // active_connection_id_limit @@ -854,6 +867,7 @@ !in.initial_max_streams_uni.Write(&writer, version) || !in.ack_delay_exponent.Write(&writer, version) || !in.max_ack_delay.Write(&writer, version) || + !in.min_ack_delay_us.Write(&writer, version) || !in.active_connection_id_limit.Write(&writer, version) || !in.max_datagram_frame_size.Write(&writer, version) || !in.initial_round_trip_time_us.Write(&writer, version)) { @@ -1389,6 +1403,10 @@ } } } break; + case TransportParameters::kMinAckDelay: + parse_success = + out->min_ack_delay_us.Read(&value_reader, error_details); + break; default: if (out->custom_parameters.find(param_id) != out->custom_parameters.end()) {
diff --git a/quic/core/crypto/transport_parameters.h b/quic/core/crypto/transport_parameters.h index 093bb09..8c3b565 100644 --- a/quic/core/crypto/transport_parameters.h +++ b/quic/core/crypto/transport_parameters.h
@@ -170,6 +170,10 @@ // delay sending acknowledgments. IntegerParameter max_ack_delay; + // Minimum amount of time in microseconds by which the endpoint will + // delay sending acknowledgments. Used to enable sender control of ack delay. + IntegerParameter min_ack_delay_us; + // Indicates lack of support for connection migration. bool disable_active_migration;
diff --git a/quic/core/crypto/transport_parameters_test.cc b/quic/core/crypto/transport_parameters_test.cc index d47ffa3..84b50f3 100644 --- a/quic/core/crypto/transport_parameters_test.cc +++ b/quic/core/crypto/transport_parameters_test.cc
@@ -259,6 +259,7 @@ orig_params.initial_max_streams_uni.set_value(kFakeInitialMaxStreamsUni); orig_params.ack_delay_exponent.set_value(kAckDelayExponentForTest); orig_params.max_ack_delay.set_value(kMaxAckDelayForTest); + orig_params.min_ack_delay_us.set_value(kMinAckDelayUsForTest); orig_params.disable_active_migration = kFakeDisableMigration; orig_params.preferred_address = CreateFakePreferredAddress(); orig_params.active_connection_id_limit.set_value( @@ -294,6 +295,7 @@ orig_params.initial_max_streams_uni.set_value(kFakeInitialMaxStreamsUni); orig_params.ack_delay_exponent.set_value(kAckDelayExponentForTest); orig_params.max_ack_delay.set_value(kMaxAckDelayForTest); + orig_params.min_ack_delay_us.set_value(kMinAckDelayUsForTest); orig_params.disable_active_migration = kFakeDisableMigration; orig_params.active_connection_id_limit.set_value( kActiveConnectionIdLimitForTest); @@ -342,6 +344,7 @@ orig_params.initial_max_streams_uni.set_value(kFakeInitialMaxStreamsUni); orig_params.ack_delay_exponent.set_value(kAckDelayExponentForTest); orig_params.max_ack_delay.set_value(kMaxAckDelayForTest); + orig_params.min_ack_delay_us.set_value(kMinAckDelayUsForTest); orig_params.disable_active_migration = kFakeDisableMigration; orig_params.preferred_address = CreateFakePreferredAddress(); orig_params.active_connection_id_limit.set_value( @@ -477,7 +480,7 @@ TEST_P(TransportParametersTest, ParseClientParams) { // clang-format off const uint8_t kClientParamsOld[] = { - 0x00, 0x7A, // length of the parameters array that follows + 0x00, 0x80, // length of the parameters array that follows // max_idle_timeout 0x00, 0x01, // parameter id 0x00, 0x02, // length @@ -518,6 +521,10 @@ 0x00, 0x0b, // parameter id 0x00, 0x01, // length 0x33, // value + // min_ack_delay_us + 0xde, 0x1a, // parameter id + 0x00, 0x02, // length + 0x43, 0xe8, // value // disable_active_migration 0x00, 0x0c, // parameter id 0x00, 0x00, // length @@ -592,6 +599,10 @@ 0x0b, // parameter id 0x01, // length 0x33, // value + // min_ack_delay_us + 0x80, 0x00, 0xde, 0x1a, // parameter id + 0x02, // length + 0x43, 0xe8, // value // disable_active_migration 0x0c, // parameter id 0x00, // length @@ -661,6 +672,7 @@ new_params.initial_max_streams_uni.value()); EXPECT_EQ(kAckDelayExponentForTest, new_params.ack_delay_exponent.value()); EXPECT_EQ(kMaxAckDelayForTest, new_params.max_ack_delay.value()); + EXPECT_EQ(kMinAckDelayUsForTest, new_params.min_ack_delay_us.value()); EXPECT_EQ(kFakeDisableMigration, new_params.disable_active_migration); EXPECT_EQ(kActiveConnectionIdLimitForTest, new_params.active_connection_id_limit.value()); @@ -843,7 +855,7 @@ TEST_P(TransportParametersTest, ParseServerParams) { // clang-format off const uint8_t kServerParamsOld[] = { - 0x00, 0xd3, // length of parameters array that follows + 0x00, 0xd9, // length of parameters array that follows // original_destination_connection_id 0x00, 0x00, // parameter id 0x00, 0x08, // length @@ -893,6 +905,10 @@ 0x00, 0x0b, // parameter id 0x00, 0x01, // length 0x33, // value + // min_ack_delay_us + 0xde, 0x1a, // parameter id + 0x00, 0x02, // length + 0x43, 0xe8, // value // disable_active_migration 0x00, 0x0c, // parameter id 0x00, 0x00, // length @@ -987,6 +1003,10 @@ 0x0b, // parameter id 0x01, // length 0x33, // value + // min_ack_delay_us + 0x80, 0x00, 0xde, 0x1a, // parameter id + 0x02, // length + 0x43, 0xe8, // value // disable_active_migration 0x0c, // parameter id 0x00, // length @@ -1072,6 +1092,7 @@ new_params.initial_max_streams_uni.value()); EXPECT_EQ(kAckDelayExponentForTest, new_params.ack_delay_exponent.value()); EXPECT_EQ(kMaxAckDelayForTest, new_params.max_ack_delay.value()); + EXPECT_EQ(kMinAckDelayUsForTest, new_params.min_ack_delay_us.value()); EXPECT_EQ(kFakeDisableMigration, new_params.disable_active_migration); ASSERT_NE(nullptr, new_params.preferred_address.get()); EXPECT_EQ(CreateFakeV4SocketAddress(), @@ -1297,6 +1318,7 @@ kFakeInitialMaxStreamsUni); original_params_.ack_delay_exponent.set_value(kAckDelayExponentForTest); original_params_.max_ack_delay.set_value(kMaxAckDelayForTest); + original_params_.min_ack_delay_us.set_value(kMinAckDelayUsForTest); original_params_.disable_active_migration = kFakeDisableMigration; original_params_.preferred_address = CreateFakePreferredAddress(); original_params_.active_connection_id_limit.set_value(
diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc index d7a9f00..0050649 100644 --- a/quic/core/quic_config.cc +++ b/quic/core/quic_config.cc
@@ -452,6 +452,7 @@ alternate_server_address_ipv4_(kASAD, PRESENCE_OPTIONAL), stateless_reset_token_(kSRST, PRESENCE_OPTIONAL), max_ack_delay_ms_(kMAD, PRESENCE_OPTIONAL), + min_ack_delay_ms_(0, PRESENCE_OPTIONAL), ack_delay_exponent_(kADE, PRESENCE_OPTIONAL), max_udp_payload_size_(0, PRESENCE_OPTIONAL), max_datagram_frame_size_(0, PRESENCE_OPTIONAL), @@ -590,7 +591,7 @@ } void QuicConfig::SetMaxAckDelayToSendMs(uint32_t max_ack_delay_ms) { - return max_ack_delay_ms_.SetSendValue(max_ack_delay_ms); + max_ack_delay_ms_.SetSendValue(max_ack_delay_ms); } uint32_t QuicConfig::GetMaxAckDelayToSendMs() const { @@ -605,6 +606,22 @@ return max_ack_delay_ms_.GetReceivedValue(); } +void QuicConfig::SetMinAckDelayMs(uint32_t min_ack_delay_ms) { + min_ack_delay_ms_.SetSendValue(min_ack_delay_ms); +} + +uint32_t QuicConfig::GetMinAckDelayToSendMs() const { + return min_ack_delay_ms_.GetSendValue(); +} + +bool QuicConfig::HasReceivedMinAckDelayMs() const { + return min_ack_delay_ms_.HasReceivedValue(); +} + +uint32_t QuicConfig::ReceivedMinAckDelayMs() const { + return min_ack_delay_ms_.GetReceivedValue(); +} + void QuicConfig::SetAckDelayExponentToSend(uint32_t exponent) { ack_delay_exponent_.SetSendValue(exponent); } @@ -1166,6 +1183,10 @@ params->initial_max_streams_uni.set_value( GetMaxUnidirectionalStreamsToSend()); params->max_ack_delay.set_value(GetMaxAckDelayToSendMs()); + if (min_ack_delay_ms_.HasSendValue()) { + params->min_ack_delay_us.set_value(min_ack_delay_ms_.GetSendValue() * + kNumMicrosPerMilli); + } params->ack_delay_exponent.set_value(GetAckDelayExponentToSend()); params->disable_active_migration = connection_migration_disabled_.HasSendValue() && @@ -1310,6 +1331,17 @@ params.preferred_address->ipv4_socket_address); } } + if (GetQuicReloadableFlag(quic_record_received_min_ack_delay)) { + if (params.min_ack_delay_us.value() != 0) { + if (params.min_ack_delay_us.value() > + params.max_ack_delay.value() * kNumMicrosPerMilli) { + *error_details = "MinAckDelay is greater than MaxAckDelay."; + return IETF_QUIC_PROTOCOL_VIOLATION; + } + min_ack_delay_ms_.SetReceivedValue(params.min_ack_delay_us.value() / + kNumMicrosPerMilli); + } + } } if (params.disable_active_migration) {
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h index 6f041b8..1c329dc 100644 --- a/quic/core/quic_config.h +++ b/quic/core/quic_config.h
@@ -420,6 +420,14 @@ bool HasReceivedMaxAckDelayMs() const; uint32_t ReceivedMaxAckDelayMs() const; + // Manage the IETF QUIC extension Min Ack Delay transport parameter. + // An endpoint uses min_ack_delay to advsertise its support for + // AckFrequencyFrame sent by peer. + void SetMinAckDelayMs(uint32_t min_ack_delay_ms); + uint32_t GetMinAckDelayToSendMs() const; + bool HasReceivedMinAckDelayMs() const; + uint32_t ReceivedMinAckDelayMs() const; + void SetAckDelayExponentToSend(uint32_t exponent); uint32_t GetAckDelayExponentToSend() const; bool HasReceivedAckDelayExponent() const; @@ -596,6 +604,10 @@ // Uses the max_ack_delay transport parameter in IETF QUIC. QuicFixedUint32 max_ack_delay_ms_; + // Minimum ack delay. Used to enable sender control of max_ack_delay. + // Uses the min_ack_delay transport parameter in IETF QUIC extension. + QuicFixedUint32 min_ack_delay_ms_; + // The sent exponent is the exponent that this node uses when serializing an // ACK frame (and the peer should use when deserializing the frame); // the received exponent is the value the peer uses to serialize frames and
diff --git a/quic/core/quic_config_test.cc b/quic/core/quic_config_test.cc index 41dd2f8..6c1d829 100644 --- a/quic/core/quic_config_test.cc +++ b/quic/core/quic_config_test.cc
@@ -8,6 +8,7 @@ #include "net/third_party/quiche/src/quic/core/crypto/crypto_handshake_message.h" #include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h" +#include "net/third_party/quiche/src/quic/core/quic_constants.h" #include "net/third_party/quiche/src/quic/core/quic_packets.h" #include "net/third_party/quiche/src/quic/core/quic_time.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" @@ -428,6 +429,30 @@ config_.IdleNetworkTimeout()); } +TEST_P(QuicConfigTest, RecievedInvalidMinAckDelayInTransportParameter) { + if (!version_.UsesTls()) { + // TransportParameters are only used for QUIC+TLS. + return; + } + SetQuicReloadableFlag(quic_record_received_min_ack_delay, true); + TransportParameters params; + + params.max_ack_delay.set_value(25 /*ms*/); + params.min_ack_delay_us.set_value(25 * kNumMicrosPerMilli + 1); + std::string error_details = "foobar"; + EXPECT_THAT(config_.ProcessTransportParameters( + params, SERVER, /* is_resumption = */ false, &error_details), + IsError(IETF_QUIC_PROTOCOL_VIOLATION)); + EXPECT_EQ("MinAckDelay is greater than MaxAckDelay.", error_details); + + params.max_ack_delay.set_value(25 /*ms*/); + params.min_ack_delay_us.set_value(25 * kNumMicrosPerMilli); + EXPECT_THAT(config_.ProcessTransportParameters( + params, SERVER, /* is_resumption = */ false, &error_details), + IsQuicNoError()); + EXPECT_TRUE(error_details.empty()); +} + TEST_P(QuicConfigTest, FillTransportParams) { if (!version_.UsesTls()) { // TransportParameters are only used for QUIC+TLS. @@ -446,6 +471,7 @@ config_.SetOriginalConnectionIdToSend(TestConnectionId(0x1111)); config_.SetInitialSourceConnectionIdToSend(TestConnectionId(0x2222)); config_.SetRetrySourceConnectionIdToSend(TestConnectionId(0x3333)); + config_.SetMinAckDelayMs(kDefaultMinAckDelayTimeMs); TransportParameters params; config_.FillTransportParameters(¶ms); @@ -475,6 +501,10 @@ ASSERT_TRUE(params.retry_source_connection_id.has_value()); EXPECT_EQ(TestConnectionId(0x3333), params.retry_source_connection_id.value()); + + EXPECT_EQ( + static_cast<uint64_t>(kDefaultMinAckDelayTimeMs) * kNumMicrosPerMilli, + params.min_ack_delay_us.value()); } TEST_P(QuicConfigTest, ProcessTransportParametersServer) { @@ -495,6 +525,7 @@ params.initial_max_streams_bidi.set_value(kDefaultMaxStreamsPerConnection); params.stateless_reset_token = CreateStatelessResetTokenForTest(); params.max_ack_delay.set_value(kMaxAckDelayForTest); + params.min_ack_delay_us.set_value(kMinAckDelayUsForTest); params.ack_delay_exponent.set_value(kAckDelayExponentForTest); params.active_connection_id_limit.set_value(kActiveConnectionIdLimitForTest); params.original_destination_connection_id = TestConnectionId(0x1111); @@ -541,6 +572,7 @@ EXPECT_FALSE(config_.HasReceivedStatelessResetToken()); EXPECT_FALSE(config_.HasReceivedMaxAckDelayMs()); EXPECT_FALSE(config_.HasReceivedAckDelayExponent()); + EXPECT_FALSE(config_.HasReceivedMinAckDelayMs()); EXPECT_FALSE(config_.HasReceivedOriginalConnectionId()); EXPECT_FALSE(config_.HasReceivedInitialSourceConnectionId()); EXPECT_FALSE(config_.HasReceivedRetrySourceConnectionId()); @@ -597,9 +629,18 @@ EXPECT_TRUE(config_.PeerSupportsHandshakeDone()); ASSERT_TRUE(config_.HasReceivedStatelessResetToken()); + ASSERT_TRUE(config_.HasReceivedMaxAckDelayMs()); EXPECT_EQ(config_.ReceivedMaxAckDelayMs(), kMaxAckDelayForTest); + if (GetQuicReloadableFlag(quic_record_received_min_ack_delay)) { + ASSERT_TRUE(config_.HasReceivedMinAckDelayMs()); + EXPECT_EQ(config_.ReceivedMinAckDelayMs(), + kMinAckDelayUsForTest / kNumMicrosPerMilli); + } else { + ASSERT_FALSE(config_.HasReceivedMinAckDelayMs()); + } + ASSERT_TRUE(config_.HasReceivedAckDelayExponent()); EXPECT_EQ(config_.ReceivedAckDelayExponent(), kAckDelayExponentForTest);
diff --git a/quic/core/quic_constants.h b/quic/core/quic_constants.h index 51ad0f0..6da3151 100644 --- a/quic/core/quic_constants.h +++ b/quic/core/quic_constants.h
@@ -123,6 +123,10 @@ // in low-bandwidth (< ~384 kbps), where an ack is sent per packet. const int64_t kDefaultDelayedAckTimeMs = 25; +// Default minimum delayed ack time, in ms (used only for sender control of ack +// frequency). +const uint32_t kDefaultMinAckDelayTimeMs = 1; + // Default shift of the ACK delay in the IETF QUIC ACK frame. const uint32_t kDefaultAckDelayExponent = 3;
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index d63006c..0a9f872 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -120,11 +120,14 @@ connection_->SetSessionNotifier(this); connection_->SetDataProducer(this); connection_->SetFromConfig(config_); - if (GetQuicReloadableFlag(quic_support_handshake_done_in_t050) && - version().UsesTls() && !version().HasHandshakeDone() && - perspective_ == Perspective::IS_CLIENT) { - config_.SetSupportHandshakeDone(); + if (perspective() == Perspective::IS_CLIENT && version().UsesTls()) { + config_.SetMinAckDelayMs(kDefaultMinAckDelayTimeMs); + if (GetQuicReloadableFlag(quic_support_handshake_done_in_t050) && + !version().HasHandshakeDone()) { + config_.SetSupportHandshakeDone(); + } } + // On the server side, version negotiation has been done by the dispatcher, // and the server session is created with the right version. if (perspective() == Perspective::IS_SERVER) {
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 54c7bfb..63a2bb1 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -2207,6 +2207,14 @@ session_.OnStreamFrame(frame); } +TEST_P(QuicSessionTestClient, MinAckDelaySetOnTheClientQuicConfig) { + if (!VersionUsesHttp3(transport_version())) { + return; + } + ASSERT_EQ(session_.config()->GetMinAckDelayToSendMs(), + kDefaultMinAckDelayTimeMs); +} + TEST_P(QuicSessionTestServer, ZombieStreams) { TestStream* stream2 = session_.CreateOutgoingBidirectionalStream(); QuicStreamPeer::SetStreamBytesWritten(3, stream2);
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index 2b483b5..27799bb 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -63,8 +63,9 @@ enum : uint64_t { kAckDelayExponentForTest = 10, - kMaxAckDelayForTest = 51, + kMaxAckDelayForTest = 51, // ms kActiveConnectionIdLimitForTest = 52, + kMinAckDelayUsForTest = 1000 }; // Create an arbitrary stateless reset token, same across multiple calls.