After receiving a packet in QuicConnection, do not schedule a send alarm unless
- There is data to send immediately, or
- There is data to send after a delay specified by the pacer.
Protected by quic_reloadable_flag_quic_no_send_alarm_unless_necessary.
PiperOrigin-RevId: 556886175
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 886d884..11d59d1 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -2350,15 +2350,81 @@
return;
}
- // Now that we have received an ack, we might be able to send packets which
- // are queued locally, or drain streams which are blocked.
- if (defer_send_in_response_to_packets_) {
- send_alarm_->Update(clock_->ApproximateNow() +
- sent_packet_manager_.GetDeferredSendAlarmDelay(),
- QuicTime::Delta::Zero());
- } else {
- WriteIfNotBlocked();
+ if (!GetQuicReloadableFlag(quic_no_send_alarm_unless_necessary)) {
+ // Now that we have received an ack, we might be able to send packets which
+ // are queued locally, or drain streams which are blocked.
+ if (defer_send_in_response_to_packets_) {
+ send_alarm_->Update(clock_->ApproximateNow() +
+ sent_packet_manager_.GetDeferredSendAlarmDelay(),
+ QuicTime::Delta::Zero());
+ } else {
+ WriteIfNotBlocked();
+ }
+ return;
}
+
+ if (!defer_send_in_response_to_packets_) {
+ WriteIfNotBlocked();
+ return;
+ }
+
+ if (!visitor_->WillingAndAbleToWrite()) {
+ QUIC_DVLOG(1)
+ << "No send alarm after processing packet. !WillingAndAbleToWrite.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 1, 7);
+ return;
+ }
+
+ // If the send alarm is already armed. Record its deadline in |max_deadline|
+ // and cancel the alarm temporarily. The rest of this function will ensure
+ // the alarm deadline is no later than |max_deadline| when the function exits.
+ QuicTime max_deadline = QuicTime::Infinite();
+ if (send_alarm_->IsSet()) {
+ QUIC_DVLOG(1) << "Send alarm already set to " << send_alarm_->deadline();
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 2, 7);
+ max_deadline = send_alarm_->deadline();
+ send_alarm_->Cancel();
+ }
+
+ if (CanWrite(HAS_RETRANSMITTABLE_DATA)) {
+ // Some data can be written immediately. Register for immediate resumption
+ // so we'll keep writing after other connections.
+ QUIC_BUG_IF(quic_send_alarm_set_with_data_to_send, send_alarm_->IsSet());
+ QUIC_DVLOG(1) << "Immediate send alarm scheduled after processing packet.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 3, 7);
+ send_alarm_->Set(clock_->ApproximateNow() +
+ sent_packet_manager_.GetDeferredSendAlarmDelay());
+ return;
+ }
+
+ if (send_alarm_->IsSet()) {
+ // Pacing limited: CanWrite returned false, and it has scheduled a send
+ // alarm before it returns.
+ if (send_alarm_->deadline() > max_deadline) {
+ QUIC_BUG(quic_send_alarm_postponed)
+ << "previous deadline:" << max_deadline
+ << ", deadline from CanWrite:" << send_alarm_->deadline();
+ QUIC_DVLOG(1) << "Send alarm restored after processing packet.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 4, 7);
+ // Restore to the previous, earlier deadline.
+ send_alarm_->Update(max_deadline, QuicTime::Delta::Zero());
+ } else {
+ QUIC_DVLOG(1) << "Future send alarm scheduled after processing packet.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 5, 7);
+ }
+ return;
+ }
+
+ if (max_deadline != QuicTime::Infinite()) {
+ QUIC_DVLOG(1) << "Send alarm restored after processing packet.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 6, 7);
+ send_alarm_->Set(max_deadline);
+ return;
+ }
+ // Can not send data due to other reasons: congestion blocked, anti
+ // amplification throttled, etc.
+ QUIC_DVLOG(1) << "No send alarm after processing packet. Other reasons.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_no_send_alarm_unless_necessary, 7, 7);
}
size_t QuicConnection::SendCryptoData(EncryptionLevel level,
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index e394d5c..daa49b3 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -689,7 +689,9 @@
EXPECT_CALL(*send_algorithm_, OnApplicationLimited(_)).Times(AnyNumber());
EXPECT_CALL(*send_algorithm_, GetCongestionControlType())
.Times(AnyNumber());
- EXPECT_CALL(visitor_, WillingAndAbleToWrite()).Times(AnyNumber());
+ EXPECT_CALL(visitor_, WillingAndAbleToWrite())
+ .WillRepeatedly(
+ Invoke(¬ifier_, &SimpleSessionNotifier::WillingToWrite));
EXPECT_CALL(visitor_, OnPacketDecrypted(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnCanWrite())
.WillRepeatedly(Invoke(¬ifier_, &SimpleSessionNotifier::OnCanWrite));
@@ -1520,6 +1522,18 @@
QuicConnectionPeer::GetReceivedServerPreferredAddress(&connection_));
}
+ // If defer sending is enabled, tell |visitor_| to return true on the next
+ // call to WillingAndAbleToWrite().
+ // This function can be used before a call to ProcessXxxPacket, to allow the
+ // process function to schedule and fire the send alarm at the end.
+ void ForceWillingAndAbleToWriteOnceForDeferSending() {
+ if (GetParam().ack_response == AckResponse::kDefer) {
+ EXPECT_CALL(visitor_, WillingAndAbleToWrite())
+ .WillOnce(Return(true))
+ .RetiresOnSaturation();
+ }
+ }
+
void TestClientRetryHandling(bool invalid_retry_tag,
bool missing_original_id_in_config,
bool wrong_original_id_in_config,
@@ -1849,7 +1863,7 @@
OnPacketSent(_, _, _, _, NO_RETRANSMITTABLE_DATA))
.Times(0);
// Do not propagate OnCanWrite() to session notifier.
- EXPECT_CALL(visitor_, OnCanWrite()).Times(AtLeast(1u));
+ EXPECT_CALL(visitor_, OnCanWrite()).Times(AnyNumber());
QuicFrames frames2;
frames2.push_back(QuicFrame(frame2_));
@@ -1981,7 +1995,7 @@
peer_creator_.SetServerConnectionId(server_cid1);
EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE)).Times(1);
// Do not propagate OnCanWrite() to session notifier.
- EXPECT_CALL(visitor_, OnCanWrite()).Times(testing::AtMost(1u));
+ EXPECT_CALL(visitor_, OnCanWrite()).Times(AnyNumber());
QuicFrames frames2;
frames2.push_back(QuicFrame(frame2_));
@@ -3657,7 +3671,7 @@
QuicAckFrame frame2 = InitAckFrame(2);
ProcessAckPacket(&frame2);
- EXPECT_CALL(visitor_, OnCanWrite());
+ EXPECT_CALL(visitor_, OnCanWrite()).Times(AnyNumber());
ProcessAckPacket(&frame1);
}
@@ -3951,6 +3965,7 @@
SetDecrypter(
ENCRYPTION_FORWARD_SECURE,
std::make_unique<StrictTaggingDecrypter>(ENCRYPTION_FORWARD_SECURE));
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessDataPacket(2);
EXPECT_EQ(0u, connection_.NumQueuedPackets());
@@ -4529,6 +4544,7 @@
size_t encrypted_length =
peer_framer_.EncryptPayload(level, QuicPacketNumber(received_packet_num),
*packet, buffer, kMaxOutgoingPacketSize);
+ EXPECT_CALL(visitor_, WillingAndAbleToWrite()).WillRepeatedly(Return(true));
connection_.ProcessUdpPacket(
kSelfAddress, kPeerAddress,
QuicReceivedPacket(buffer, encrypted_length, clock_.Now(), false));
@@ -6501,6 +6517,7 @@
} else {
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1);
}
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL);
// Check that ack is sent and that delayed ack alarm is reset.
EXPECT_EQ(3u, writer_->frame_count());
@@ -6539,6 +6556,7 @@
.WillOnce(IgnoreResult(InvokeWithoutArgs(
&connection_, &TestConnection::SendCryptoStreamData)));
}
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL);
}
// Check that ack is sent and that delayed ack alarm is reset.
@@ -6596,6 +6614,7 @@
EXPECT_CALL(visitor_, OnCanWrite())
.WillOnce(IgnoreResult(InvokeWithoutArgs(
&connection_, &TestConnection::EnsureWritableAndSendStreamData5)));
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessAckPacket(&ack);
// Check that ack is bundled with outgoing data and the delayed ack
@@ -9625,6 +9644,7 @@
// Receives packet 1.
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
const size_t anti_amplification_factor =
@@ -9644,6 +9664,7 @@
// Receives packet 2.
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL);
// Verify more packets can be sent.
for (size_t i = anti_amplification_factor + 1;
@@ -9657,6 +9678,7 @@
2 * anti_amplification_factor * 3);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessPacket(3);
// Verify anti-amplification limit is gone after address validation.
for (size_t i = 0; i < 100; ++i) {
@@ -9695,6 +9717,7 @@
// Receives packet 1.
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
const size_t anti_amplification_factor = 3;
@@ -9713,6 +9736,7 @@
// Receives packet 2.
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL);
// Verify more packets can be sent.
for (size_t i = anti_amplification_factor + 1;
@@ -9726,6 +9750,7 @@
2 * anti_amplification_factor * 3);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessPacket(3);
// Verify anti-amplification limit is gone after address validation.
for (size_t i = 0; i < 100; ++i) {
@@ -9764,6 +9789,7 @@
// Receives packet 1.
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
const size_t anti_amplification_factor = 10;
@@ -9782,6 +9808,7 @@
// Receives packet 2.
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL);
// Verify more packets can be sent.
for (size_t i = anti_amplification_factor + 1;
@@ -9795,6 +9822,7 @@
2 * anti_amplification_factor * 3);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessPacket(3);
// Verify anti-amplification limit is gone after address validation.
for (size_t i = 0; i < 100; ++i) {
@@ -16039,6 +16067,7 @@
EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(1);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
.Times(anti_amplification_factor);
+ ForceWillingAndAbleToWriteOnceForDeferSending();
ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL);
connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
std::make_unique<TaggingEncrypter>(0x02));
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index f95556e..b56e7fd 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -19,6 +19,8 @@
QUIC_FLAG(quic_reloadable_flag_quic_block_until_settings_received_copt, false)
// If trrue, early return before write control frame in OnCanWrite() if the connection is already closed.
QUIC_FLAG(quic_reloadable_flag_quic_no_write_control_frame_upon_connection_close, true)
+// If true and defer_send_in_response_to_packets is enabled, QuicConnection will schedule send alarms at the end of packet processing only if it\'s necessary.
+QUIC_FLAG(quic_reloadable_flag_quic_no_send_alarm_unless_necessary, false)
// If true, HTTP/3 client will allow host header in HTTP/3 response.
QUIC_FLAG(quic_reloadable_flag_quic_allow_host_header_in_response, true)
// If true, QUIC BBR2 will ignore non-positive RTT samples.
