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.