Updated ACK_FREQUENCY according to draft-ietf-quic-ack-frequency-10

src/core/ack_tracker.c

@@ -96,6 +96,64 @@
         !RangeUpdated;
 }
 
+//
+// This function implements the logic defined in Section 6.2 [draft-ietf-quic-frequence-10]
+// to determine if the reordering threshold has been hit.
+//
+BOOLEAN
+QuicAckTrackerDidHitReorderingThreshold(
+    _In_ QUIC_ACK_TRACKER* Tracker,
+    _In_ uint8_t ReorderingThreshold
+    )
+{
+    if (ReorderingThreshold == 0) {
+        return FALSE;
+    }
+
+    const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck);
+    uint64_t LargestReported = 0; // The largest packet number that could be declared lost 
+
+    if (Tracker->LargestPacketNumberAcknowledged != 0 &&
+        Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 > LargestUnacked) {
+        return FALSE;
+    }
+
+    if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold) {
+        LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1;
+    }
+
+    //
+    // Loop through all previous ACK ranges (before last) to find the smallest missing
+    // packet number that is after the largest reported packet number. If the difference
+    // between that missing number and the largest unack'ed number is more than the
+    // reordering threshold, then the condition has been met to send an immediate
+    // acknowledgement.
+    //
+
+    for (int Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index >= 0; --Index) {
+        uint64_t SmallestMissing = 0; // Smallest missing in the previous gap
+        uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Highest missing in the prevous gap
+        if (HighestMissing == 0) {
+            return FALSE;
+        }
+        if (Index != 0) {
+            SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1;
+        }
+
+        if (LargestReported > SmallestMissing) {
+            if (HighestMissing > LargestReported) {
+                SmallestMissing = LargestReported;
+            } else {
+                return FALSE;
+            }
+        }
+        if (LargestUnacked - SmallestMissing >= ReorderingThreshold) {
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
 _IRQL_requires_max_(DISPATCH_LEVEL)
 void
 QuicAckTrackerAckPacket(
@@ -172,7 +230,7 @@
 
     Tracker->AckElicitingPacketsToAcknowledge++;
 
-    if (Connection->Send.SendFlags & QUIC_CONN_SEND_FLAG_ACK) {
+    if ((Connection->Send.SendFlags & QUIC_CONN_SEND_FLAG_ACK) || !NewLargestPacketNumber) {
         goto Exit; // Already queued to send an ACK, no more work to do.
     }
 
@@ -182,10 +240,10 @@
     //   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.
+    //   4. We have received an ACK eliciting packet that is out of order and the
+    //      gap between the smallest Unreported Missing packet and the Largest
+    //      Unacked is greater than or equal to the Reordering Threshold value. This logic is
+    //      disabled if the Reordering Threshold is 0.
     //   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
@@ -195,12 +253,7 @@
     if (AckType == QUIC_ACK_TYPE_ACK_IMMEDIATE ||
         Connection->Settings.MaxAckDelayMs == 0 ||
         (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) ||
-        (!Connection->State.IgnoreReordering &&
-         (NewLargestPacketNumber &&
-          QuicRangeSize(&Tracker->PacketNumbersToAck) > 1 && // There are more than two ranges, i.e. a gap somewhere.
-            QuicRangeGet(
-            &Tracker->PacketNumbersToAck,
-          QuicRangeSize(&Tracker->PacketNumbersToAck) - 1)->Count == 1))) { // The gap is right before the last packet number.
+        QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold)) {
         //
         // Send the ACK immediately.
         //

src/core/ack_tracker.h

@@ -5,6 +5,10 @@
 
 --*/
 
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
 typedef struct QUIC_ACK_TRACKER {
 
     //
@@ -91,6 +95,12 @@ QuicAckTrackerAddPacketNumber(
     _In_ uint64_t PacketNumber
     );
 
+BOOLEAN
+QuicAckTrackerDidHitReorderingThreshold(
+    _In_ QUIC_ACK_TRACKER* Tracker,
+    _In_ uint8_t ReorderingThreshold
+    );
+
 typedef enum QUIC_ACK_TYPE {
     QUIC_ACK_TYPE_NON_ACK_ELICITING,
     QUIC_ACK_TYPE_ACK_ELICITING,
@@ -146,3 +156,7 @@ QuicAckTrackerHasPacketsToAck(
         !Tracker->AlreadyWrittenAckFrame &&
         QuicRangeSize(&Tracker->PacketNumbersToAck) != 0;
 }
+
+#if defined(__cplusplus)
+}
+#endif

src/core/connection.c

@@ -118,6 +118,8 @@
     Connection->AckDelayExponent = QUIC_ACK_DELAY_EXPONENT;
     Connection->PacketTolerance = QUIC_MIN_ACK_SEND_NUMBER;
     Connection->PeerPacketTolerance = QUIC_MIN_ACK_SEND_NUMBER;
+    Connection->ReorderingThreshold = QUIC_MIN_REORDERING_THRESHOLD;
+    Connection->PeerReorderingThreshold = QUIC_MIN_REORDERING_THRESHOLD;
     Connection->PeerTransportParams.AckDelayExponent = QUIC_TP_ACK_DELAY_EXPONENT_DEFAULT;
     Connection->ReceiveQueueTail = &Connection->ReceiveQueue;
     QuicSettingsCopy(&Connection->Settings, &MsQuicLib.Settings);
@@ -5211,12 +5213,12 @@
                 return FALSE;
             }
 
-            if (Frame.UpdateMaxAckDelay < MS_TO_US(MsQuicLib.TimerResolutionMs)) {
+            if (Frame.RequestedMaxAckDelay < MS_TO_US(MsQuicLib.TimerResolutionMs)) {
                 QuicTraceEvent(
                     ConnError,
                     "[conn][%p] ERROR, %s.",
                     Connection,
-                    "UpdateMaxAckDelay is less than TimerResolution");
+                    "RequestedMaxAckDelay is less than TimerResolution");
                 QuicConnTransportError(Connection, QUIC_ERROR_PROTOCOL_VIOLATION);
                 return FALSE;
             }
@@ -5231,20 +5233,24 @@
             }
 
             Connection->NextRecvAckFreqSeqNum = Frame.SequenceNumber + 1;
-            Connection->State.IgnoreReordering = Frame.IgnoreOrder;
-            if (Frame.UpdateMaxAckDelay == 0) {
+            if (Frame.RequestedMaxAckDelay == 0) {
                 Connection->Settings.MaxAckDelayMs = 0;
-            } else if (Frame.UpdateMaxAckDelay < 1000) {
+            } else if (Frame.RequestedMaxAckDelay < 1000) {
                 Connection->Settings.MaxAckDelayMs = 1;
             } else {
-                CXPLAT_DBG_ASSERT(US_TO_MS(Frame.UpdateMaxAckDelay) <= UINT32_MAX);
-                Connection->Settings.MaxAckDelayMs = (uint32_t)US_TO_MS(Frame.UpdateMaxAckDelay);
+                CXPLAT_DBG_ASSERT(US_TO_MS(Frame.RequestedMaxAckDelay) <= UINT32_MAX);
+                Connection->Settings.MaxAckDelayMs = (uint32_t)US_TO_MS(Frame.RequestedMaxAckDelay);
             }
-            if (Frame.PacketTolerance < UINT8_MAX) {
-                Connection->PacketTolerance = (uint8_t)Frame.PacketTolerance;
+            if (Frame.AckElicitingThreshold < UINT8_MAX) {
+                Connection->PacketTolerance = (uint8_t)Frame.AckElicitingThreshold;
             } else {
                 Connection->PacketTolerance = UINT8_MAX; // Cap to 0xFF for space savings.
             }
+            if (Frame.ReorderingThreshold < UINT8_MAX) {
+                Connection->ReorderingThreshold = (uint8_t)Frame.ReorderingThreshold;
+            } else {
+                Connection->ReorderingThreshold = UINT8_MAX; // Cap to 0xFF for space savings.
+            }
             QuicTraceLogConnInfo(
                 UpdatePacketTolerance,
                 Connection,

src/core/connection.h

@@ -146,12 +146,6 @@ typedef union QUIC_CONNECTION_STATE {
         //
         BOOLEAN ResumptionEnabled : 1;
 
-        //
-        // When true,acknowledgment that reordering shouldn't elict an
-        // immediate acknowledgement.
-        //
-        BOOLEAN IgnoreReordering : 1;
-
         //
         // When true, this indicates that the connection is currently executing
         // an API call inline (from a reentrant call on a callback).
@@ -453,6 +447,21 @@ typedef struct QUIC_CONNECTION {
     //
     uint8_t PeerPacketTolerance;
 
+    //
+    // The maximum number of packets that can be out of order before an immediate
+    // acknowledgment (ACK) is triggered. If no specific instructions (ACK_FREQUENCY
+    // frames) are received from the peer, the receiver will immediately acknowledge
+    // any out-of-order packets, which means the default value is 1. A value of 0
+    // means out-of-order packets do not trigger an immediate ACK.
+    //
+    uint8_t ReorderingThreshold;
+
+    //
+    // The maximum number of packets that the peer can be out of order before an immediate
+    // acknowledgment (ACK) is triggered.
+    //
+    uint8_t PeerReorderingThreshold;
+
     //
     // The ACK frequency sequence number we are currently using to send.
     //

src/core/crypto_tls.c

@@ -63,7 +63,7 @@ typedef enum eSniNameType {
 #define QUIC_TP_ID_MAX_DATAGRAM_FRAME_SIZE                  32              // varint
 #define QUIC_TP_ID_DISABLE_1RTT_ENCRYPTION                  0xBAAD          // N/A
 #define QUIC_TP_ID_VERSION_NEGOTIATION_EXT                  0x11            // Blob
-#define QUIC_TP_ID_MIN_ACK_DELAY                            0xFF03DE1AULL   // varint
+#define QUIC_TP_ID_MIN_ACK_DELAY                            0xFF04DE1BULL   // varint
 #define QUIC_TP_ID_CIBIR_ENCODING                           0x1000          // {varint, varint}
 #define QUIC_TP_ID_GREASE_QUIC_BIT                          0x2AB2          // N/A
 #define QUIC_TP_ID_RELIABLE_RESET_ENABLED                   0x17f7586d2cb570   // varint

src/core/frame.c

@@ -1234,19 +1234,6 @@ QuicDatagramFrameDecode(
     return TRUE;
 }
 
-typedef struct QUIC_ACK_FREQUENCY_EXTRAS {
-
-    union {
-        struct {
-            uint8_t IgnoreOrder : 1;
-            uint8_t IgnoreCE    : 1;
-            uint8_t Reserved    : 6;
-        };
-        uint8_t Value;
-    };
-
-} QUIC_ACK_FREQUENCY_EXTRAS;
-
 _Success_(return != FALSE)
 BOOLEAN
 QuicAckFrequencyFrameEncode(
@@ -1259,27 +1246,20 @@ QuicAckFrequencyFrameEncode(
     uint16_t RequiredLength =
         QuicVarIntSize(QUIC_FRAME_ACK_FREQUENCY) +
         QuicVarIntSize(Frame->SequenceNumber) +
-        QuicVarIntSize(Frame->PacketTolerance) +
-        QuicVarIntSize(Frame->UpdateMaxAckDelay) +
-        sizeof(QUIC_ACK_FREQUENCY_EXTRAS);
+        QuicVarIntSize(Frame->AckElicitingThreshold) +
+        QuicVarIntSize(Frame->RequestedMaxAckDelay) +
+        QuicVarIntSize(Frame->ReorderingThreshold);
 
     if (BufferLength < *Offset + RequiredLength) {
         return FALSE;
     }
 
-    CXPLAT_DBG_ASSERT(Frame->IgnoreOrder <= 1); // IgnoreOrder should only be 0 or 1.
-    CXPLAT_DBG_ASSERT(Frame->IgnoreCE <= 1);    // IgnoreCE should only be 0 or 1.
-
-    QUIC_ACK_FREQUENCY_EXTRAS Extras = { .Value = 0 };
-    Extras.IgnoreOrder = Frame->IgnoreOrder;
-    Extras.IgnoreCE = Frame->IgnoreCE;
-
     Buffer = Buffer + *Offset;
     Buffer = QuicVarIntEncode(QUIC_FRAME_ACK_FREQUENCY, Buffer);
     Buffer = QuicVarIntEncode(Frame->SequenceNumber, Buffer);
-    Buffer = QuicVarIntEncode(Frame->PacketTolerance, Buffer);
-    Buffer = QuicVarIntEncode(Frame->UpdateMaxAckDelay, Buffer);
-    QuicUint8Encode(Extras.Value, Buffer);
+    Buffer = QuicVarIntEncode(Frame->AckElicitingThreshold, Buffer);
+    Buffer = QuicVarIntEncode(Frame->RequestedMaxAckDelay, Buffer);
+    Buffer = QuicVarIntEncode(Frame->ReorderingThreshold, Buffer);
     *Offset += RequiredLength;
 
     return TRUE;
@@ -1295,15 +1275,12 @@ QuicAckFrequencyFrameDecode(
     _Out_ QUIC_ACK_FREQUENCY_EX* Frame
     )
 {
-    QUIC_ACK_FREQUENCY_EXTRAS Extras;
     if (!QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->SequenceNumber) ||
-        !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->PacketTolerance) ||
-        !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->UpdateMaxAckDelay) ||
-        !QuicUint8tDecode(BufferLength, Buffer, Offset, &Extras.Value)) {
+        !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->AckElicitingThreshold) ||
+        !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->RequestedMaxAckDelay) ||
+        !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->ReorderingThreshold)) {
         return FALSE;
     }
-    Frame->IgnoreOrder = Extras.IgnoreOrder;
-    Frame->IgnoreCE = Extras.IgnoreCE;
     return TRUE;
 }
 
@@ -1963,15 +1940,14 @@ QuicFrameLog(
 
         QuicTraceLogVerbose(
             FrameLogAckFrequency,
-            "[%c][%cX][%llu]   ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu",
+            "[%c][%cX][%llu]   ACK_FREQUENCY SeqNum:%llu AckElicitThreshold:%llu MaxAckDelay:%llu ReorderThreshold:%llu",
             PtkConnPre(Connection),
             PktRxPre(Rx),
             PacketNumber,
             Frame.SequenceNumber,
-            Frame.PacketTolerance,
-            Frame.UpdateMaxAckDelay,
-            Frame.IgnoreOrder,
-            Frame.IgnoreCE);
+            Frame.AckElicitingThreshold,
+            Frame.RequestedMaxAckDelay,
+            Frame.ReorderingThreshold);
         break;
     }
 
@@ -2006,7 +1982,7 @@ QuicFrameLog(
             Frame.Timestamp);
         break;
     }
-    
+
     case QUIC_FRAME_RELIABLE_RESET_STREAM: {
         QUIC_RELIABLE_RESET_STREAM_EX Frame;
         if (!QuicReliableResetFrameDecode(PacketLength, Packet, Offset, &Frame)) {

src/core/frame.h

@@ -156,7 +156,7 @@ typedef enum QUIC_FRAME_TYPE {
     QUIC_FRAME_DATAGRAM_1           = 0x31ULL,
     /* 0x32 to 0xad are unused currently */
     QUIC_FRAME_ACK_FREQUENCY        = 0xafULL,
-    QUIC_FRAME_IMMEDIATE_ACK        = 0xacULL,
+    QUIC_FRAME_IMMEDIATE_ACK        = 0x1fULL,
     /* 0xaf to 0x2f4 are unused currently */
     QUIC_FRAME_TIMESTAMP            = 0x2f5ULL,
 
@@ -844,11 +844,9 @@ QuicDatagramFrameDecode(
 typedef struct QUIC_ACK_FREQUENCY_EX {
 
     QUIC_VAR_INT SequenceNumber;
-    QUIC_VAR_INT PacketTolerance;
-    QUIC_VAR_INT UpdateMaxAckDelay; // In microseconds (us)
-    BOOLEAN IgnoreOrder;
-    BOOLEAN IgnoreCE;
-
+    QUIC_VAR_INT AckElicitingThreshold;
+    QUIC_VAR_INT RequestedMaxAckDelay; // In microseconds (us)
+    QUIC_VAR_INT ReorderingThreshold;
 } QUIC_ACK_FREQUENCY_EX;
 
 _Success_(return != FALSE)

src/core/quicdef.h

@@ -103,6 +103,12 @@ typedef struct QUIC_RX_PACKET QUIC_RX_PACKET;
 //
 #define QUIC_MIN_ACK_SEND_NUMBER                2
 
+//
+// The value for Reordering threshold when no ACK_FREQUENCY frame is received.
+// This means that the receiver will immediately acknowledge any out-of-order packets.
+//
+#define QUIC_MIN_REORDERING_THRESHOLD           1
+
 //
 // The size of the stateless reset token.
 //

src/core/send.c

@@ -894,10 +894,9 @@ QuicSendWriteFrames(
 
             QUIC_ACK_FREQUENCY_EX Frame;
             Frame.SequenceNumber = Connection->SendAckFreqSeqNum;
-            Frame.PacketTolerance = Connection->PeerPacketTolerance;
-            Frame.UpdateMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection));
-            Frame.IgnoreOrder = FALSE;
-            Frame.IgnoreCE = FALSE;
+            Frame.AckElicitingThreshold = Connection->PeerPacketTolerance;
+            Frame.RequestedMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection));
+            Frame.ReorderingThreshold = Connection->PeerReorderingThreshold;
 
             if (QuicAckFrequencyFrameEncode(
                     &Frame,