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.
