From 1d1e1306b33eb11cde1a41a35e78457a2e282cd1 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 12 Dec 2024 01:36:59 +0530 Subject: [PATCH] changed the reordering logic --- src/core/ack_tracker.c | 39 +++++++++++++++++------- src/core/ack_tracker.h | 3 +- src/core/unittest/FrameTest.cpp | 53 ++++++++++++++------------------- 3 files changed, 52 insertions(+), 43 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 08a4a295fc..2b16708392 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -99,21 +99,38 @@ QuicAckTrackerAddPacketNumber( BOOLEAN QuicAckTrackerDidHitReorderingThreshold( _In_ QUIC_ACK_TRACKER* Tracker, - _In_ uint8_t ReorderingThreshold, - _In_ uint64_t PacketNumber + _In_ uint8_t ReorderingThreshold ) { - if (ReorderingThreshold == 0 || - QuicRangeSize(&Tracker->PacketNumbersToAck) <= 1 || - PacketNumber != QuicRangeGetMax(&Tracker->PacketNumbersToAck)) { // There are more than two ranges, i.e. a gap somewhere. + if (ReorderingThreshold == 0) { return FALSE; } uint64_t LargestUnackedPacketNumber = QuicRangeGetMax(&Tracker->PacketNumbersToAck); - uint64_t SmallestUnreportedMissingPacketNumber = - QuicRangeGetHigh( - QuicRangeGet(&Tracker->PacketNumbersToAck, 0)) + 1; - + uint64_t LargestReported = 0; // The largest packet number that could be declared lost + uint64_t SmallestUnreportedMissingPacketNumber; + + if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold) { + LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; + } + + if (LargestReported > LargestUnackedPacketNumber) { + return FALSE; + } + + QUIC_RANGE_SEARCH_KEY Key = { LargestReported, LargestReported }; + int result = QuicRangeSearch(&Tracker->PacketNumbersToAck, &Key); + if (result >= 0 && result < (int)QuicRangeSize(&Tracker->PacketNumbersToAck) - 1) { + SmallestUnreportedMissingPacketNumber = + QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, result)) + 1; + } + else if (result < 0) { + SmallestUnreportedMissingPacketNumber = LargestReported; + } + else { + return FALSE; + } + return LargestUnackedPacketNumber - SmallestUnreportedMissingPacketNumber >= ReorderingThreshold; } @@ -193,7 +210,7 @@ QuicAckTrackerAckPacket( 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. } @@ -216,7 +233,7 @@ QuicAckTrackerAckPacket( if (AckType == QUIC_ACK_TYPE_ACK_IMMEDIATE || Connection->Settings.MaxAckDelayMs == 0 || (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) || - QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold, PacketNumber)) { + QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold)) { // // Send the ACK immediately. // diff --git a/src/core/ack_tracker.h b/src/core/ack_tracker.h index 309255ce3b..12a82441e5 100644 --- a/src/core/ack_tracker.h +++ b/src/core/ack_tracker.h @@ -96,8 +96,7 @@ QuicAckTrackerAddPacketNumber( BOOLEAN QuicAckTrackerDidHitReorderingThreshold( _In_ QUIC_ACK_TRACKER* Tracker, - _In_ uint8_t ReorderingThreshold, - _In_ uint64_t PacketNumber + _In_ uint8_t ReorderingThreshold ); typedef enum QUIC_ACK_TYPE { diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index 91999f3be1..d649e39ddb 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -255,73 +255,66 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) // ReorderingThreshold is 0 ReorderingThreshold = 0; QuicRangeAddValue(&Tracker.PacketNumbersToAck, 100); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 100)); - - // The number of ranges is less than 2. - ReorderingThreshold = 3; - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 101); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 101)); - - // PacketNumber is not the largest unacked packet number - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 104); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 105); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 103)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); // Case 1 + ReorderingThreshold = 3; QuicAckTrackerReset(&Tracker); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 0); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 0)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 1); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 1)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 3); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 3)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 4); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 4)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 5); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 5)); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 2); // 2 is reported missing + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + Tracker.LargestPacketNumberAcknowledged = 5; QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 8)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 9); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 9)); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 6); // 6 is reported missing + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + Tracker.LargestPacketNumberAcknowledged = 9; QuicRangeAddValue(&Tracker.PacketNumbersToAck, 10); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 10)); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + Tracker.LargestPacketNumberAcknowledged = 10; // Case 2 ReorderingThreshold = 5; QuicAckTrackerReset(&Tracker); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 0); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 0)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 1); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 1)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 3); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 3)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 5); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 5)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 6); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 6)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 7); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 7)); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 2); // 2 is reported missing + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + Tracker.LargestPacketNumberAcknowledged = 7; QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 8)); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 9); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 9)); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + Tracker.LargestPacketNumberAcknowledged = 9; // Clean up