diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 6e34cc9f57..e0df5b047c 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -179,19 +179,21 @@ QuicAckTrackerAckPacket( // // There are several conditions where we decide to send an ACK immediately: // - // 1. We have received 'PacketTolerance' ACK eliciting packets. - // 2. We received an ACK eliciting packet that doesn't directly follow the + // 1. The packet included an IMMEDIATE_ACK frame. + // 2. ACK delay is disabled (MaxAckDelayMs == 0). + // 3. We have received 'PacketTolerance' ACK eliciting packets. + // 4. We received an ACK eliciting packet that doesn't directly follow the // previously received packet number. So we assume there might have // been loss and should indicate this info to the peer. This logic is // disabled if 'IgnoreReordering' is TRUE. - // 3. The delayed ACK timer fires after the configured time. - // 4. The packet included an IMMEDIATE_ACK frame. + // 5. The delayed ACK timer fires after the configured time. // // If we don't queue an immediate ACK and this is the first ACK eliciting // packet received, we make sure the ACK delay timer is started. // if (AckType == QUIC_ACK_TYPE_ACK_IMMEDIATE || + Connection->Settings.MaxAckDelayMs == 0 || (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) || (!Connection->State.IgnoreReordering && (NewLargestPacketNumber && @@ -200,9 +202,7 @@ QuicAckTrackerAckPacket( &Tracker->PacketNumbersToAck, QuicRangeSize(&Tracker->PacketNumbersToAck) - 1)->Count == 1))) { // The gap is right before the last packet number. // - // Always send an ACK immediately if we have received enough ACK - // eliciting packets OR the latest one indicate a gap in the packet - // numbers, which likely means there was loss. + // Send the ACK immediately. // QuicSendSetSendFlag(&Connection->Send, QUIC_CONN_SEND_FLAG_ACK); diff --git a/src/core/connection.c b/src/core/connection.c index 96f8ec7b70..24720f8fc6 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -2349,9 +2349,11 @@ QuicConnGenerateLocalTransportParameters( MaxUdpPayloadSizeFromMTU( CxPlatSocketGetLocalMtu( Connection->Paths[0].Binding->Socket)); - LocalTP->MaxAckDelay = - Connection->Settings.MaxAckDelayMs + MsQuicLib.TimerResolutionMs; - LocalTP->MinAckDelay = MS_TO_US(MsQuicLib.TimerResolutionMs); + LocalTP->MaxAckDelay = QuicConnGetAckDelay(Connection); + LocalTP->MinAckDelay = + MsQuicLib.ExecutionConfig != NULL && + MsQuicLib.ExecutionConfig->PollingIdleTimeoutUs != 0 ? + 0 : MS_TO_US(MsQuicLib.TimerResolutionMs); LocalTP->ActiveConnectionIdLimit = QUIC_ACTIVE_CONNECTION_ID_LIMIT; LocalTP->Flags = QUIC_TP_FLAG_INITIAL_MAX_DATA | @@ -5296,7 +5298,9 @@ QuicConnRecvFrames( Connection->NextRecvAckFreqSeqNum = Frame.SequenceNumber + 1; Connection->State.IgnoreReordering = Frame.IgnoreOrder; - if (Frame.UpdateMaxAckDelay < 1000) { + if (Frame.UpdateMaxAckDelay == 0) { + Connection->Settings.MaxAckDelayMs = 0; + } else if (Frame.UpdateMaxAckDelay < 1000) { Connection->Settings.MaxAckDelayMs = 1; } else { CXPLAT_DBG_ASSERT(US_TO_MS(Frame.UpdateMaxAckDelay) <= UINT32_MAX); @@ -5318,7 +5322,7 @@ QuicConnRecvFrames( case QUIC_FRAME_IMMEDIATE_ACK: // Always accept the frame, because we always enable support. AckImmediately = TRUE; break; - + case QUIC_FRAME_TIMESTAMP: { // Always accept the frame, because we always enable support. if (!Connection->State.TimestampRecvNegotiated) { QuicTraceEvent( @@ -5345,7 +5349,7 @@ QuicConnRecvFrames( Packet->SendTimestamp = Frame.Timestamp; break; } - + default: // // No default case necessary, as we have already validated the frame diff --git a/src/core/connection.h b/src/core/connection.h index 977e70cdb4..4cb58c69f2 100644 --- a/src/core/connection.h +++ b/src/core/connection.h @@ -1353,6 +1353,26 @@ QuicConnTimerExpired( _In_ uint64_t TimeNow ); +_IRQL_requires_max_(PASSIVE_LEVEL) +inline +uint64_t +QuicConnGetAckDelay( + _In_ const QUIC_CONNECTION* Connection + ) +{ + if (Connection->Settings.MaxAckDelayMs && + (MsQuicLib.ExecutionConfig == NULL || + Connection->Settings.MaxAckDelayMs > US_TO_MS(MsQuicLib.ExecutionConfig->PollingIdleTimeoutUs))) { + // + // If we are using delayed ACKs, and the ACK delay is greater than the + // polling timeout, then we need to account for delay resulting from + // from the timer resolution. + // + return (uint64_t)Connection->Settings.MaxAckDelayMs + (uint64_t)MsQuicLib.TimerResolutionMs; + } + return (uint64_t)Connection->Settings.MaxAckDelayMs; +} + // // Called when the QUIC version is set. // diff --git a/src/core/inline.c b/src/core/inline.c index fbc50207ef..df2825c02b 100644 --- a/src/core/inline.c +++ b/src/core/inline.c @@ -779,6 +779,11 @@ QuicConnTimerSet( _In_ uint64_t DelayUs ); +uint64_t +QuicConnGetAckDelay( + _In_ const QUIC_CONNECTION* Connection + ); + uint8_t QuicPacketTraceType( _In_ const QUIC_SENT_PACKET_METADATA* Metadata diff --git a/src/core/send.c b/src/core/send.c index 0fcc615c7a..6b439482c8 100644 --- a/src/core/send.c +++ b/src/core/send.c @@ -870,10 +870,7 @@ QuicSendWriteFrames( QUIC_ACK_FREQUENCY_EX Frame; Frame.SequenceNumber = Connection->SendAckFreqSeqNum; Frame.PacketTolerance = Connection->PeerPacketTolerance; - Frame.UpdateMaxAckDelay = - MS_TO_US( - (uint64_t)Connection->Settings.MaxAckDelayMs + - (uint64_t)MsQuicLib.TimerResolutionMs); + Frame.UpdateMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection)); Frame.IgnoreOrder = FALSE; Frame.IgnoreCE = FALSE; @@ -1489,6 +1486,7 @@ QuicSendStartDelayedAckTimer( { QUIC_CONNECTION* Connection = QuicSendGetConnection(Send); + CXPLAT_DBG_ASSERT(Connection->Settings.MaxAckDelayMs != 0); if (!Send->DelayedAckTimerActive && !(Send->SendFlags & QUIC_CONN_SEND_FLAG_ACK) && !Connection->State.ClosedLocally &&