Batch standalone QUIC ACK-only packets across a receive batch with ScopedAckDeferrer. When a client processes a batch of incoming packets (for example, up to 16 packets from a single recvmmsg call), each per-packet flush can send a standalone ACK-only packet mid-batch. Testing on Cobalt showed the best performance when ACKs were held until the whole batch had been processed, but the earlier approach of blocking the writer caused other problems. Add `QuicConnection::ScopedAckDeferrer`, an RAII helper for IETF QUIC connections. While it is in scope, standalone ACK-only packets are not sent, but pending ACKs can still be bundled with any outgoing data or control frames. When the outermost deferrer goes out of scope, it sends any ACKs that are due, leaves them to an imminent send alarm, or arms the ACK alarm for the earliest future ACK timeout. `QuicClientDefaultNetworkHelper` wraps its socket read loop in a deferrer. Deferral is enabled only on clients that set the new `DAOP` connection option, via either client connection options or connection options to send, so client experiments such as Chrome or Cobalt Finch can opt in. Servers ignore `DAOP`. PiperOrigin-RevId: 1015665961
diff --git a/quiche/quic/core/crypto/crypto_protocol.h b/quiche/quic/core/crypto/crypto_protocol.h index 98d21a6..009b032 100644 --- a/quiche/quic/core/crypto/crypto_protocol.h +++ b/quiche/quic/core/crypto/crypto_protocol.h
@@ -327,6 +327,7 @@ // Allows disabling defer_send_in_response_to_packets in QuicConnection. DEFINE_STATIC_QUIC_TAG(DFER); // Do not defer sending. DEFINE_STATIC_QUIC_TAG(CDFR); // Defer sending on client. +DEFINE_STATIC_QUIC_TAG(DAOP); // Defer sending ACK-only packets on client. // Pacing options. DEFINE_STATIC_QUIC_TAG(NPCO); // No pacing offload.
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index d32e051..3be2642 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -623,6 +623,12 @@ coalesce_ping_with_last_packet_ = true; } + if (perspective_ == Perspective::IS_CLIENT && version().IsIetfQuic() && + (config.HasClientRequestedIndependentOption(kDAOP, perspective_) || + config.HasClientSentConnectionOption(kDAOP, perspective_))) { + allow_deferring_ack_only_packets_ = true; + } + if (config.HasClientRequestedIndependentOption(kINVC, perspective_)) { send_connection_close_for_invalid_version_ = true; } @@ -3036,7 +3042,8 @@ } const QuicTime ack_timeout = uber_received_packet_manager_.GetEarliestAckTimeout(); - if (ack_timeout.IsInitialized() && + // ScopedAckDeferrer sends due ACKs when it goes out of scope. + if (!defer_ack_only_packets_ && ack_timeout.IsInitialized() && ack_timeout <= clock_->ApproximateNow()) { // Send an ACK now because either 1) we were write blocked when we last // tried to send an ACK, or 2) both ack alarm and send alarm were set to @@ -3049,7 +3056,8 @@ } else { const QuicTime ack_timeout = uber_received_packet_manager_.GetEarliestAckTimeout(); - if (ack_timeout.IsInitialized() && + // ScopedAckDeferrer sends due ACKs when it goes out of scope. + if (!defer_ack_only_packets_ && ack_timeout.IsInitialized() && ack_timeout <= clock_->ApproximateNow()) { // Send an ACK now because either 1) we were write blocked when we last // tried to send an ACK, or 2) both ack alarm and send alarm were set to @@ -5198,32 +5206,35 @@ } if (active_) { - const QuicTime ack_timeout = - connection_->uber_received_packet_manager_.GetEarliestAckTimeout(); - if (ack_timeout.IsInitialized()) { - if (ack_timeout <= connection_->clock_->ApproximateNow() && - !connection_->CanWrite(NO_RETRANSMITTABLE_DATA)) { - // Cancel ACK alarm if connection is write blocked, and ACK will be - // sent when connection gets unblocked. - connection_->ack_alarm().Cancel(); - } else if (!connection_->ack_alarm().IsSet() || - connection_->ack_alarm().deadline() > ack_timeout) { - connection_->ack_alarm().Update(ack_timeout, QuicTime::Delta::Zero()); + // ScopedAckDeferrer handles pending ACKs when it goes out of scope. + if (!connection_->defer_ack_only_packets_) { + const QuicTime ack_timeout = + connection_->uber_received_packet_manager_.GetEarliestAckTimeout(); + if (ack_timeout.IsInitialized()) { + if (ack_timeout <= connection_->clock_->ApproximateNow() && + !connection_->CanWrite(NO_RETRANSMITTABLE_DATA)) { + // Cancel ACK alarm if connection is write blocked, and ACK will be + // sent when connection gets unblocked. + connection_->ack_alarm().Cancel(); + } else if (!connection_->ack_alarm().IsSet() || + connection_->ack_alarm().deadline() > ack_timeout) { + connection_->ack_alarm().Update(ack_timeout, QuicTime::Delta::Zero()); + } } - } - if (connection_->ack_alarm().IsSet() && - connection_->ack_alarm().deadline() <= - connection_->clock_->ApproximateNow()) { - // An ACK needs to be sent right now. This ACK did not get bundled - // because either there was no data to write or packets were marked as - // received after frames were queued in the generator. - if (connection_->send_alarm().IsSet() && - connection_->send_alarm().deadline() <= + if (connection_->ack_alarm().IsSet() && + connection_->ack_alarm().deadline() <= connection_->clock_->ApproximateNow()) { - // If send alarm will go off soon, let send alarm send the ACK. - connection_->ack_alarm().Cancel(); - } else { - connection_->SendAllPendingAcks(); + // An ACK needs to be sent right now. This ACK did not get bundled + // because either there was no data to write or packets were marked as + // received after frames were queued in the generator. + if (connection_->send_alarm().IsSet() && + connection_->send_alarm().deadline() <= + connection_->clock_->ApproximateNow()) { + // If send alarm will go off soon, let send alarm send the ACK. + connection_->ack_alarm().Cancel(); + } else { + connection_->SendAllPendingAcks(); + } } } @@ -5290,6 +5301,35 @@ !connection_->packet_creator_.PacketFlusherAttached()); } +QuicConnection::ScopedAckDeferrer::ScopedAckDeferrer(QuicConnection* connection) + : connection_(connection), active_(false) { + if (connection_ == nullptr || !connection_->connected() || + !connection_->SupportsMultiplePacketNumberSpaces() || + !connection_->allow_deferring_ack_only_packets_) { + return; + } + if (!connection_->defer_ack_only_packets_) { + active_ = true; + connection_->defer_ack_only_packets_ = true; + } +} + +QuicConnection::ScopedAckDeferrer::~ScopedAckDeferrer() { + if (!active_) { + return; + } + connection_->defer_ack_only_packets_ = false; + if (!connection_->connected() || + connection_->packet_creator_.PacketFlusherAttached() || + !connection_->uber_received_packet_manager_.GetEarliestAckTimeout() + .IsInitialized()) { + return; + } + // Now that deferral has ended, ~ScopedPacketFlusher() sends any due ACKs or + // updates the ACK alarm for the whole batch. + ScopedPacketFlusher flusher(connection_); +} + QuicConnection::ScopedEncryptionLevelContext::ScopedEncryptionLevelContext( QuicConnection* connection, EncryptionLevel encryption_level) : connection_(connection), latched_encryption_level_(ENCRYPTION_INITIAL) {
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index ad1cdd3..d320e89 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1090,6 +1090,24 @@ const bool handshake_packet_sent_; }; + // While in scope on an IETF QUIC connection, suppresses sending standalone + // ACK-only packets (e.g. while processing a batch of packets from recvmmsg) + // while still allowing outgoing data or control packets to opportunistically + // bundle pending ACKs. When the outermost deferrer goes out of scope, sends + // any due ACKs or updates the ACK alarm once for the batch. |connection| may + // be nullptr; if non-null, it must outlive the ScopedAckDeferrer. + class QUICHE_EXPORT ScopedAckDeferrer { + public: + explicit ScopedAckDeferrer(QuicConnection* connection); + ScopedAckDeferrer(const ScopedAckDeferrer&) = delete; + ScopedAckDeferrer& operator=(const ScopedAckDeferrer&) = delete; + ~ScopedAckDeferrer(); + + private: + QuicConnection* connection_; + bool active_; + }; + class QUICHE_EXPORT ScopedEncryptionLevelContext { public: ScopedEncryptionLevelContext(QuicConnection* connection, @@ -2613,6 +2631,9 @@ bool fix_mtu_discovery_ : 1 = GetQuicReloadableFlag(quic_fix_mtu_discovery); bool send_acks_after_on_can_write_ : 1 = false; bool coalesce_ping_with_last_packet_ : 1 = false; + // Set by the client-only DAOP connection option. + bool allow_deferring_ack_only_packets_ : 1 = false; + bool defer_ack_only_packets_ : 1 = false; }; } // namespace quic
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index b56e8b0..512b0d7 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -20613,6 +20613,377 @@ EXPECT_LT(writer_->last_packet_size(), connection_.max_packet_length()); } +TEST_P(QuicConnectionTest, ScopedAckDeferrerDisabledWithoutConnectionOption) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(4); + + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t i = 1; i <= 4; ++i) { + ProcessDataPacket(i); + } + // Without DAOP, ACK-only packets are still sent mid-batch. + EXPECT_EQ(2u, writer_->packets_write_attempts()); + } + + EXPECT_EQ(2u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(4), LargestAcked(writer_->ack_frames()[0])); +} + +TEST_P(QuicConnectionTest, ScopedAckDeferrerBatchesAckOnlyPackets) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + + constexpr uint64_t kNumPackets = 16; + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(kNumPackets); + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t i = 1; i <= kNumPackets; ++i) { + ProcessDataPacket(i); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + } + + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(kNumPackets), + LargestAcked(writer_->ack_frames()[0])); + EXPECT_EQ(1u, writer_->ack_frames()[0].packets.NumIntervals()); + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); +} + +TEST_P(QuicConnectionTest, + ScopedAckDeferrerOutOfOrderBatchSendsSingleContiguousAck) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + + const uint64_t kPacketOrder[] = {1, 2, 4, 3, 5, 6}; + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(6); + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t packet_number : kPacketOrder) { + ProcessDataPacket(packet_number); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + } + + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(6), LargestAcked(writer_->ack_frames()[0])); + EXPECT_EQ(1u, writer_->ack_frames()[0].packets.NumIntervals()); +} + +TEST_P(QuicConnectionTest, ScopedAckDeferrerArmsAckAlarmWhenTimeoutInFuture) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + ProcessDataPacket(1); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + + // Only one packet arrived, so no immediate ACK is due; the delayed-ACK alarm + // must be armed for the future. + EXPECT_EQ(0u, writer_->packets_write_attempts()); + ASSERT_TRUE(connection_.GetAckAlarm()->IsSet()); + EXPECT_GT(connection_.GetAckAlarm()->deadline(), clock_.ApproximateNow()); + + clock_.AdvanceTime(connection_.GetAckAlarm()->deadline() - + clock_.ApproximateNow()); + connection_.GetAckAlarm()->Fire(); + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(1), LargestAcked(writer_->ack_frames()[0])); +} + +TEST_P(QuicConnectionTest, + ScopedAckDeferrerAllowsOpportunisticAckBundlingMidBatch) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + + int stream_frames_received = 0; + EXPECT_CALL(visitor_, OnStreamFrame(_)) + .Times(4) + .WillRepeatedly([&](const QuicStreamFrame& /*frame*/) { + ++stream_frames_received; + if (stream_frames_received == 4) { + connection_.SendStreamData3(); + } + }); + + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t i = 1; i <= 3; ++i) { + ProcessDataPacket(i); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + ProcessDataPacket(4); + // Outgoing STREAM data mid-batch is flushed immediately with the pending + // ACK bundled. + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(4), LargestAcked(writer_->ack_frames()[0])); + EXPECT_EQ(1u, writer_->stream_frames().size()); + } + + // No redundant ACK-only packet is sent when ScopedAckDeferrer exits. + EXPECT_EQ(1u, writer_->packets_write_attempts()); + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); +} + +TEST_P(QuicConnectionTest, + ScopedAckDeferrerDefersToPendingSendAlarmAtBatchEnd) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + connection_.set_defer_send_in_response_to_packets(true); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(4); + + auto receive_without_firing_send_alarm = [this](uint64_t packet_number) { + std::unique_ptr<QuicPacket> packet(ConstructDataPacket( + packet_number, /*has_stop_waiting=*/false, ENCRYPTION_FORWARD_SECURE)); + char buffer[kMaxOutgoingPacketSize]; + peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); + size_t encrypted_length = peer_framer_.EncryptPayload( + ENCRYPTION_FORWARD_SECURE, QuicPacketNumber(packet_number), *packet, + buffer, kMaxOutgoingPacketSize); + connection_.ProcessUdpPacket( + kSelfAddress, kPeerAddress, + QuicReceivedPacket(buffer, encrypted_length, clock_.Now(), false)); + }; + + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + receive_without_firing_send_alarm(1); + receive_without_firing_send_alarm(2); + // Simulate session having data ready to write via the send alarm. + EXPECT_CALL(visitor_, WillingAndAbleToWrite()).WillRepeatedly(Return(true)); + receive_without_firing_send_alarm(3); + receive_without_firing_send_alarm(4); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + EXPECT_TRUE(connection_.GetSendAlarm()->IsSet()); + } + + // Because send_alarm is due immediately, ~ScopedAckDeferrer() leaves the ACK + // for OnSendAlarm() to bundle with outgoing data rather than sending a + // separate ACK-only packet first. + EXPECT_EQ(0u, writer_->packets_write_attempts()); + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); + + EXPECT_CALL(visitor_, WillingAndAbleToWrite()).WillRepeatedly(Return(false)); + EXPECT_CALL(visitor_, OnCanWrite()).WillOnce([this]() { + connection_.SendStreamData3(); + }); + connection_.GetSendAlarm()->Fire(); + + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(4), LargestAcked(writer_->ack_frames()[0])); + EXPECT_EQ(1u, writer_->stream_frames().size()); +} + +TEST_P(QuicConnectionTest, + ScopedAckDeferrerMultiplePacketNumberSpacesAndNesting) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + QuicConnection::ScopedAckDeferrer null_deferrer(nullptr); + + connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, + std::make_unique<TaggingEncrypter>(0x02)); + peer_framer_.SetEncrypter(ENCRYPTION_HANDSHAKE, + std::make_unique<TaggingEncrypter>(0x02)); + writer_->framer()->framer()->InstallDecrypter( + ENCRYPTION_HANDSHAKE, std::make_unique<StrictTaggingDecrypter>(0x02)); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + + EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(2); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); + { + QuicConnection::ScopedAckDeferrer outer_deferrer(&connection_); + { + QuicConnection::ScopedAckDeferrer inner_deferrer(&connection_); + ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL); + ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL); + // Inner deferrer going out of scope must not flush ACKs early. + } + EXPECT_EQ(0u, writer_->packets_write_attempts()); + ProcessDataPacket(1); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + + // Outer deferrer sends the due INITIAL ACK and arms the future ACK alarm for + // APPLICATION_DATA packet 1. + EXPECT_EQ(1u, writer_->packets_write_attempts()); + EXPECT_TRUE(connection_.GetAckAlarm()->IsSet()); + EXPECT_GT(connection_.GetAckAlarm()->deadline(), clock_.ApproximateNow()); +} + +TEST_P(QuicConnectionTest, ScopedAckDeferrerHandlesConnectionCloseMidBatch) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConnectionPeer::SetAllowDeferringAckOnlyPackets(&connection_, true); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(2); + EXPECT_CALL(visitor_, + OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF)); + + size_t packets_after_close = 0; + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + ProcessDataPacket(1); + ProcessDataPacket(2); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + connection_.CloseConnection( + QUIC_NO_ERROR, "closing mid-batch", + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + packets_after_close = writer_->packets_write_attempts(); + EXPECT_GE(packets_after_close, 1u); + EXPECT_FALSE(writer_->connection_close_frames().empty()); + } + + EXPECT_FALSE(connection_.connected()); + // Exiting ScopedAckDeferrer after CloseConnection must not send any extra + // packets. + EXPECT_EQ(packets_after_close, writer_->packets_write_attempts()); +} + +TEST_P(QuicConnectionTest, ScopedAckDeferrerEnabledByClientConnectionOption) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConfig config; + config.SetClientConnectionOptions({kDAOP}); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + EXPECT_CALL(*send_algorithm_, EnableECT1()).WillOnce(Return(false)); + EXPECT_CALL(*send_algorithm_, EnableECT0()).WillOnce(Return(false)); + connection_.SetFromConfig(config); + + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(4); + + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t i = 1; i <= 4; ++i) { + ProcessDataPacket(i); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + } + + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(4), LargestAcked(writer_->ack_frames()[0])); +} + +TEST_P(QuicConnectionTest, + ScopedAckDeferrerEnabledByClientSentConnectionOption) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + QuicConfig config; + config.SetConnectionOptionsToSend({kDAOP}); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + EXPECT_CALL(*send_algorithm_, EnableECT1()).WillOnce(Return(false)); + EXPECT_CALL(*send_algorithm_, EnableECT0()).WillOnce(Return(false)); + connection_.SetFromConfig(config); + + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(4); + + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t i = 1; i <= 4; ++i) { + ProcessDataPacket(i); + EXPECT_EQ(0u, writer_->packets_write_attempts()); + } + } + + EXPECT_EQ(1u, writer_->packets_write_attempts()); + ASSERT_EQ(1u, writer_->ack_frames().size()); + EXPECT_EQ(QuicPacketNumber(4), LargestAcked(writer_->ack_frames()[0])); +} + +TEST_P(QuicConnectionTest, ScopedAckDeferrerIgnoredOnServerForClientOption) { + if (!connection_.SupportsMultiplePacketNumberSpaces()) { + return; + } + set_perspective(Perspective::IS_SERVER); + writer_->set_perspective(Perspective::IS_SERVER); + QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false); + QuicConnectionPeer::SetAddressValidated(&connection_); + + QuicConfig config; + config.SetInitialReceivedConnectionOptions({kDAOP}); + EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); + EXPECT_CALL(*send_algorithm_, EnableECT1()).WillOnce(Return(false)); + EXPECT_CALL(*send_algorithm_, EnableECT0()).WillOnce(Return(false)); + connection_.SetFromConfig(config); + + connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.OnHandshakeComplete(); + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(4); + + { + QuicConnection::ScopedAckDeferrer ack_deferrer(&connection_); + for (uint64_t i = 1; i <= 4; ++i) { + ProcessDataPacket(i); + } + EXPECT_EQ(2u, writer_->packets_write_attempts()); + } + EXPECT_EQ(2u, writer_->packets_write_attempts()); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc index 865e2b7..b458cd0 100644 --- a/quiche/quic/test_tools/quic_connection_peer.cc +++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -669,6 +669,12 @@ } // static +void QuicConnectionPeer::SetAllowDeferringAckOnlyPackets( + QuicConnection* connection, bool allow) { + connection->allow_deferring_ack_only_packets_ = allow; +} + +// static QuicConnection::ReceivedPacketInfo& QuicConnectionPeer::GetLastReceivedPacketInfo(QuicConnection* connection) { return connection->last_received_packet_info_;
diff --git a/quiche/quic/test_tools/quic_connection_peer.h b/quiche/quic/test_tools/quic_connection_peer.h index 7ff6b4e..7aa14ef 100644 --- a/quiche/quic/test_tools/quic_connection_peer.h +++ b/quiche/quic/test_tools/quic_connection_peer.h
@@ -274,6 +274,9 @@ static bool GetSpinBitEnabled(QuicConnection* connection); + static void SetAllowDeferringAckOnlyPackets(QuicConnection* connection, + bool allow); + static QuicConnection::ReceivedPacketInfo& GetLastReceivedPacketInfo( QuicConnection* connection); };
diff --git a/quiche/quic/tools/quic_client_default_network_helper.cc b/quiche/quic/tools/quic_client_default_network_helper.cc index 539b02c..d08b5ab 100644 --- a/quiche/quic/tools/quic_client_default_network_helper.cc +++ b/quiche/quic/tools/quic_client_default_network_helper.cc
@@ -12,6 +12,7 @@ #include "absl/cleanup/cleanup.h" #include "quiche/quic/core/io/quic_event_loop.h" #include "quiche/quic/core/io/socket.h" +#include "quiche/quic/core/quic_connection.h" #include "quiche/quic/core/quic_default_packet_writer.h" #include "quiche/quic/core/quic_packets.h" #include "quiche/quic/core/quic_types.h" @@ -154,11 +155,15 @@ int times_to_read = max_reads_per_event_loop_; bool more_to_read = true; QuicPacketCount packets_dropped = 0; - while (client_->connected() && more_to_read && times_to_read > 0) { - more_to_read = packet_reader_->ReadAndDispatchPackets( - fd, GetLatestClientAddress().port(), *client_->helper()->GetClock(), - this, overflow_supported_ ? &packets_dropped : nullptr); - --times_to_read; + { + QuicConnection::ScopedAckDeferrer ack_deferrer( + client_->connected() ? client_->session()->connection() : nullptr); + while (client_->connected() && more_to_read && times_to_read > 0) { + more_to_read = packet_reader_->ReadAndDispatchPackets( + fd, GetLatestClientAddress().port(), *client_->helper()->GetClock(), + this, overflow_supported_ ? &packets_dropped : nullptr); + --times_to_read; + } } if (packets_dropped_ < packets_dropped) { QUIC_LOG(ERROR)