Move local_delayed_ack_time from QuicSentPacketManager to QuicReceivedPacketManager, set the delayed ack time to 1ms for Initial and Handshake packet number spaces when using v99, and use peer_delayed_ack_time instead of local_delayed_ack_time when with the MAD1 connection option.
gfe-relnote: n/a (Refactor or v99 only)
PiperOrigin-RevId: 261378188
Change-Id: I4bfed0bde6fa58506250b766294221f1aaf9fc29
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index df0d069..0b648e2 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1363,8 +1363,7 @@
uber_received_packet_manager_.MaybeUpdateAckTimeout(
should_last_packet_instigate_acks_, last_decrypted_packet_level_,
last_header_.packet_number, time_of_last_received_packet_,
- clock_->ApproximateNow(), sent_packet_manager_.GetRttStats(),
- sent_packet_manager_.local_max_ack_delay());
+ clock_->ApproximateNow(), sent_packet_manager_.GetRttStats());
} else {
QUIC_DLOG(INFO) << ENDPOINT << "Not updating ACK timeout for "
<< QuicUtils::EncryptionLevelToString(
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 45d31be..118e13b 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -683,6 +683,10 @@
// Returns the underlying sent packet manager.
QuicSentPacketManager& sent_packet_manager() { return sent_packet_manager_; }
+ UberReceivedPacketManager& received_packet_manager() {
+ return uber_received_packet_manager_;
+ }
+
bool CanWrite(HasRetransmittableData retransmittable);
// When the flusher is out of scope, only the outermost flusher will cause a
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 2994748..e21f4c4 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -58,6 +58,8 @@
ack_decimation_delay_(kAckDecimationDelay),
unlimited_ack_decimation_(false),
fast_ack_after_quiescence_(false),
+ local_max_ack_delay_(
+ QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)),
ack_timeout_(QuicTime::Zero()),
time_of_previous_received_packet_(QuicTime::Zero()),
was_last_packet_missing_(false) {
@@ -218,8 +220,7 @@
QuicPacketNumber last_received_packet_number,
QuicTime time_of_last_received_packet,
QuicTime now,
- const RttStats* rtt_stats,
- QuicTime::Delta local_max_ack_delay) {
+ const RttStats* rtt_stats) {
if (!ack_frame_updated_) {
// ACK frame has not been updated, nothing to do.
return;
@@ -251,7 +252,7 @@
// Wait for the minimum of the ack decimation delay or the delayed ack time
// before sending an ack.
QuicTime::Delta ack_delay = std::min(
- local_max_ack_delay, rtt_stats->min_rtt() * ack_decimation_delay_);
+ local_max_ack_delay_, rtt_stats->min_rtt() * ack_decimation_delay_);
if (fast_ack_after_quiescence_ && now - time_of_previous_received_packet_ >
rtt_stats->SmoothedOrInitialRtt()) {
// Ack the first packet out of queiscence faster, because QUIC does
@@ -273,7 +274,7 @@
// or TLP packets, which we'd like to acknowledge quickly.
MaybeUpdateAckTimeoutTo(now + QuicTime::Delta::FromMilliseconds(1));
} else {
- MaybeUpdateAckTimeoutTo(now + local_max_ack_delay);
+ MaybeUpdateAckTimeoutTo(now + local_max_ack_delay_);
}
}
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index aaae7f1..293b03c 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -64,8 +64,7 @@
QuicPacketNumber last_received_packet_number,
QuicTime time_of_last_received_packet,
QuicTime now,
- const RttStats* rtt_stats,
- QuicTime::Delta local_max_ack_delay);
+ const RttStats* rtt_stats);
// Resets ACK related states, called after an ACK is successfully sent.
void ResetAckStates();
@@ -119,6 +118,11 @@
ack_frequency_before_ack_decimation_ = new_value;
}
+ QuicTime::Delta local_max_ack_delay() const { return local_max_ack_delay_; }
+ void set_local_max_ack_delay(QuicTime::Delta local_max_ack_delay) {
+ local_max_ack_delay_ = local_max_ack_delay;
+ }
+
QuicTime ack_timeout() const { return ack_timeout_; }
private:
@@ -172,6 +176,9 @@
// was received.
bool fast_ack_after_quiescence_;
+ // The local node's maximum ack delay time. This is the maximum amount of
+ // time to wait before sending an acknowledgement.
+ QuicTime::Delta local_max_ack_delay_;
// Time that an ACK needs to be sent. 0 means no ACK is pending. Used when
// decide_when_to_send_acks_ is true.
QuicTime ack_timeout_;
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc
index 86ac933..db5e260 100644
--- a/quic/core/quic_received_packet_manager_test.cc
+++ b/quic/core/quic_received_packet_manager_test.cc
@@ -90,7 +90,7 @@
received_manager_.MaybeUpdateAckTimeout(
should_last_packet_instigate_acks,
QuicPacketNumber(last_received_packet_number), clock_.ApproximateNow(),
- clock_.ApproximateNow(), &rtt_stats_, kDelayedAckTime);
+ clock_.ApproximateNow(), &rtt_stats_);
}
void CheckAckTimeout(QuicTime time) {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 7427b73..fb0e8de 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -111,8 +111,6 @@
ietf_style_2x_tlp_(false),
largest_mtu_acked_(0),
handshake_confirmed_(false),
- local_max_ack_delay_(
- QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)),
peer_max_ack_delay_(
QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)),
rtt_updated_(false),
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index b08704d..010181c 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -355,17 +355,11 @@
return pending_timer_transmission_count_;
}
- QuicTime::Delta local_max_ack_delay() const { return local_max_ack_delay_; }
-
- void set_local_max_ack_delay(QuicTime::Delta local_max_ack_delay) {
- // The delayed ack time should never be more than one half the min RTO time.
- DCHECK_LE(local_max_ack_delay, (min_rto_timeout_ * 0.5));
- local_max_ack_delay_ = local_max_ack_delay;
- }
-
QuicTime::Delta peer_max_ack_delay() const { return peer_max_ack_delay_; }
void set_peer_max_ack_delay(QuicTime::Delta peer_max_ack_delay) {
+ // The delayed ack time should never be more than one half the min RTO time.
+ DCHECK_LE(peer_max_ack_delay, (min_rto_timeout_ * 0.5));
peer_max_ack_delay_ = peer_max_ack_delay;
}
@@ -615,10 +609,6 @@
QuicPacketNumber
largest_packets_peer_knows_is_acked_[NUM_PACKET_NUMBER_SPACES];
- // The local node's maximum ack delay time. This is the maximum amount of
- // time to wait before sending an acknowledgement.
- QuicTime::Delta local_max_ack_delay_;
-
// The maximum ACK delay time that the peer uses. Initialized to be the
// same as local_max_ack_delay_, may be changed via transport parameter
// negotiation.
diff --git a/quic/core/uber_received_packet_manager.cc b/quic/core/uber_received_packet_manager.cc
index 6df7490..78cf359 100644
--- a/quic/core/uber_received_packet_manager.cc
+++ b/quic/core/uber_received_packet_manager.cc
@@ -4,6 +4,7 @@
#include "net/third_party/quiche/src/quic/core/uber_received_packet_manager.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"
@@ -77,19 +78,18 @@
QuicPacketNumber last_received_packet_number,
QuicTime time_of_last_received_packet,
QuicTime now,
- const RttStats* rtt_stats,
- QuicTime::Delta local_max_ack_delay) {
+ const RttStats* rtt_stats) {
if (!supports_multiple_packet_number_spaces_) {
received_packet_managers_[0].MaybeUpdateAckTimeout(
should_last_packet_instigate_acks, last_received_packet_number,
- time_of_last_received_packet, now, rtt_stats, local_max_ack_delay);
+ time_of_last_received_packet, now, rtt_stats);
return;
}
received_packet_managers_[QuicUtils::GetPacketNumberSpace(
decrypted_packet_level)]
- .MaybeUpdateAckTimeout(
- should_last_packet_instigate_acks, last_received_packet_number,
- time_of_last_received_packet, now, rtt_stats, local_max_ack_delay);
+ .MaybeUpdateAckTimeout(should_last_packet_instigate_acks,
+ last_received_packet_number,
+ time_of_last_received_packet, now, rtt_stats);
}
void UberReceivedPacketManager::ResetAckStates(
@@ -112,6 +112,12 @@
"packet has been received.";
return;
}
+ // In IETF QUIC, the peer is expected to acknowledge packets in Initial and
+ // Handshake packets with minimal delay.
+ received_packet_managers_[INITIAL_DATA].set_local_max_ack_delay(
+ QuicTime::Delta::FromMilliseconds(1));
+ received_packet_managers_[HANDSHAKE_DATA].set_local_max_ack_delay(
+ QuicTime::Delta::FromMilliseconds(1));
supports_multiple_packet_number_spaces_ = true;
}
@@ -207,6 +213,23 @@
}
}
+QuicTime::Delta UberReceivedPacketManager::max_ack_delay() {
+ if (!supports_multiple_packet_number_spaces_) {
+ return received_packet_managers_[0].local_max_ack_delay();
+ }
+ return received_packet_managers_[APPLICATION_DATA].local_max_ack_delay();
+}
+
+void UberReceivedPacketManager::set_max_ack_delay(
+ QuicTime::Delta max_ack_delay) {
+ if (!supports_multiple_packet_number_spaces_) {
+ received_packet_managers_[0].set_local_max_ack_delay(max_ack_delay);
+ return;
+ }
+ received_packet_managers_[APPLICATION_DATA].set_local_max_ack_delay(
+ max_ack_delay);
+}
+
void UberReceivedPacketManager::set_save_timestamps(bool save_timestamps) {
for (auto& received_packet_manager : received_packet_managers_) {
received_packet_manager.set_save_timestamps(save_timestamps);
diff --git a/quic/core/uber_received_packet_manager.h b/quic/core/uber_received_packet_manager.h
index c442804..14ba868 100644
--- a/quic/core/uber_received_packet_manager.h
+++ b/quic/core/uber_received_packet_manager.h
@@ -48,8 +48,7 @@
QuicPacketNumber last_received_packet_number,
QuicTime time_of_last_received_packet,
QuicTime now,
- const RttStats* rtt_stats,
- QuicTime::Delta local_max_ack_delay);
+ const RttStats* rtt_stats);
// Resets ACK related states, called after an ACK is successfully sent.
void ResetAckStates(EncryptionLevel encryption_level);
@@ -89,6 +88,10 @@
void set_max_ack_ranges(size_t max_ack_ranges);
+ // Get and set the max ack delay to use for application data.
+ QuicTime::Delta max_ack_delay();
+ void set_max_ack_delay(QuicTime::Delta max_ack_delay);
+
void set_save_timestamps(bool save_timestamps);
private:
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc
index 64dbb17..9372da8 100644
--- a/quic/core/uber_received_packet_manager_test.cc
+++ b/quic/core/uber_received_packet_manager_test.cc
@@ -98,7 +98,7 @@
manager_->MaybeUpdateAckTimeout(
should_last_packet_instigate_acks, decrypted_packet_level,
QuicPacketNumber(last_received_packet_number), clock_.ApproximateNow(),
- clock_.ApproximateNow(), &rtt_stats_, kDelayedAckTime);
+ clock_.ApproximateNow(), &rtt_stats_);
}
void CheckAckTimeout(QuicTime time) {
@@ -378,6 +378,20 @@
CheckAckTimeout(ack_time);
}
+TEST_F(UberReceivedPacketManagerTest, SendDelayedMaxAckDelay) {
+ EXPECT_FALSE(HasPendingAck());
+ QuicTime::Delta max_ack_delay = QuicTime::Delta::FromMilliseconds(100);
+ manager_->set_max_ack_delay(max_ack_delay);
+ QuicTime ack_time = clock_.ApproximateNow() + max_ack_delay;
+
+ RecordPacketReceipt(1, clock_.ApproximateNow());
+ MaybeUpdateAckTimeout(kInstigateAck, 1);
+ CheckAckTimeout(ack_time);
+ // Simulate delayed ack alarm firing.
+ clock_.AdvanceTime(max_ack_delay);
+ CheckAckTimeout(clock_.ApproximateNow());
+}
+
TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimation) {
EXPECT_FALSE(HasPendingAck());
UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION);
@@ -765,7 +779,12 @@
MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_HANDSHAKE, 3);
EXPECT_TRUE(HasPendingAck());
// Delayed ack is scheduled.
- CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
+ CheckAckTimeout(clock_.ApproximateNow() +
+ QuicTime::Delta::FromMilliseconds(1));
+ // Send delayed handshake data ACK.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
+ CheckAckTimeout(clock_.ApproximateNow());
+ EXPECT_FALSE(HasPendingAck());
RecordPacketReceipt(ENCRYPTION_FORWARD_SECURE, 3);
MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_FORWARD_SECURE, 3);
@@ -777,12 +796,6 @@
MaybeUpdateAckTimeout(kInstigateAck, ENCRYPTION_FORWARD_SECURE, 2);
// Application data ACK should be sent immediately.
CheckAckTimeout(clock_.ApproximateNow());
- // Delayed ACK of handshake data is pending.
- CheckAckTimeout(clock_.ApproximateNow() + kDelayedAckTime);
-
- // Send delayed handshake data ACK.
- clock_.AdvanceTime(kDelayedAckTime);
- CheckAckTimeout(clock_.ApproximateNow());
EXPECT_FALSE(HasPendingAck());
}
diff --git a/quic/quartc/quartc_factory.cc b/quic/quartc/quartc_factory.cc
index 8a69ab5..6742b89 100644
--- a/quic/quartc/quartc_factory.cc
+++ b/quic/quartc/quartc_factory.cc
@@ -8,6 +8,7 @@
#include "net/third_party/quiche/src/quic/core/quic_utils.h"
#include "net/third_party/quiche/src/quic/core/tls_client_handshaker.h"
#include "net/third_party/quiche/src/quic/core/tls_server_handshaker.h"
+#include "net/third_party/quiche/src/quic/core/uber_received_packet_manager.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_socket_address.h"
#include "net/third_party/quiche/src/quic/quartc/quartc_connection_helper.h"
@@ -46,8 +47,8 @@
// Quartc sets its own ack delay; get that ack delay and copy it over
// to the QuicConfig so that it can be properly advertised to the peer
// via transport parameter negotiation.
- quic_config.SetMaxAckDelayToSendMs(quic_connection->sent_packet_manager()
- .local_max_ack_delay()
+ quic_config.SetMaxAckDelayToSendMs(quic_connection->received_packet_manager()
+ .max_ack_delay()
.ToMilliseconds());
return QuicMakeUnique<QuartcClientSession>(
@@ -202,6 +203,8 @@
QuicSentPacketManager& sent_packet_manager =
quic_connection->sent_packet_manager();
+ UberReceivedPacketManager& received_packet_manager =
+ quic_connection->received_packet_manager();
// Default delayed ack time is 25ms.
// If data packets are sent less often (e.g. because p-time was modified),
@@ -213,7 +216,7 @@
// The p-time can go up to as high as 120ms, and when it does, it's
// when the low overhead is the most important thing. Ideally it should be
// above 120ms, but it cannot be higher than 0.5*RTO, which equals to 100ms.
- sent_packet_manager.set_local_max_ack_delay(
+ received_packet_manager.set_max_ack_delay(
QuicTime::Delta::FromMilliseconds(100));
sent_packet_manager.set_peer_max_ack_delay(
QuicTime::Delta::FromMilliseconds(100));
