From d6cde247234d51d29d9958cd8610045bc15e470e Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 28 Nov 2024 23:28:34 +0530 Subject: [PATCH 01/32] added reordering threshold in ack frequency --- src/core/ack_tracker.c | 11 ++++++----- src/core/connection.c | 7 +++++++ src/core/connection.h | 15 +++++++++++++++ src/core/frame.c | 8 ++++++-- src/core/frame.h | 1 + src/core/quicdef.h | 6 ++++++ src/core/send.c | 1 + 7 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index fe70fa57df..9114b52af9 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -182,10 +182,10 @@ QuicAckTrackerAckPacket( // 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 had 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 'IgnoreReordering' is TRUE or 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 @@ -196,11 +196,12 @@ QuicAckTrackerAckPacket( Connection->Settings.MaxAckDelayMs == 0 || (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) || (!Connection->State.IgnoreReordering && + Connection->ReorderingThreshold > 0 && (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. + QuicRangeSize(&Tracker->PacketNumbersToAck) - 1)->Count == Connection->ReorderingThreshold))) { // // Send the ACK immediately. // diff --git a/src/core/connection.c b/src/core/connection.c index 3dc7b4145f..938988d70e 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -118,6 +118,8 @@ QuicConnAlloc( 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); @@ -5245,6 +5247,11 @@ QuicConnRecvFrames( } 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, diff --git a/src/core/connection.h b/src/core/connection.h index 5a88b33dc9..e87379ccb6 100644 --- a/src/core/connection.h +++ b/src/core/connection.h @@ -463,6 +463,21 @@ typedef struct QUIC_CONNECTION { // uint64_t NextRecvAckFreqSeqNum; + // + // 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 sequence number to use for the next source CID. // diff --git a/src/core/frame.c b/src/core/frame.c index 7e1a923e11..8b1bb729da 100644 --- a/src/core/frame.c +++ b/src/core/frame.c @@ -1261,6 +1261,7 @@ QuicAckFrequencyFrameEncode( QuicVarIntSize(Frame->SequenceNumber) + QuicVarIntSize(Frame->PacketTolerance) + QuicVarIntSize(Frame->UpdateMaxAckDelay) + + QuicVarIntSize(Frame->ReorderingThreshold) + sizeof(QUIC_ACK_FREQUENCY_EXTRAS); if (BufferLength < *Offset + RequiredLength) { @@ -1279,6 +1280,7 @@ QuicAckFrequencyFrameEncode( Buffer = QuicVarIntEncode(Frame->SequenceNumber, Buffer); Buffer = QuicVarIntEncode(Frame->PacketTolerance, Buffer); Buffer = QuicVarIntEncode(Frame->UpdateMaxAckDelay, Buffer); + Buffer = QuicVarIntEncode(Frame->ReorderingThreshold, Buffer); QuicUint8Encode(Extras.Value, Buffer); *Offset += RequiredLength; @@ -1299,6 +1301,7 @@ QuicAckFrequencyFrameDecode( if (!QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->SequenceNumber) || !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->PacketTolerance) || !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->UpdateMaxAckDelay) || + !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->ReorderingThreshold) || !QuicUint8tDecode(BufferLength, Buffer, Offset, &Extras.Value)) { return FALSE; } @@ -1963,13 +1966,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 PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold: %llu IgnoreOrder:%hhu IgnoreCE:%hhu", PtkConnPre(Connection), PktRxPre(Rx), PacketNumber, Frame.SequenceNumber, Frame.PacketTolerance, Frame.UpdateMaxAckDelay, + Frame.ReorderingThreshold, Frame.IgnoreOrder, Frame.IgnoreCE); break; @@ -2006,7 +2010,7 @@ QuicFrameLog( Frame.Timestamp); break; } - + case QUIC_FRAME_RELIABLE_RESET_STREAM: { QUIC_RELIABLE_RESET_STREAM_EX Frame; if (!QuicReliableResetFrameDecode(PacketLength, Packet, Offset, &Frame)) { diff --git a/src/core/frame.h b/src/core/frame.h index 277177cbe9..e8aaf40e7f 100644 --- a/src/core/frame.h +++ b/src/core/frame.h @@ -846,6 +846,7 @@ typedef struct QUIC_ACK_FREQUENCY_EX { QUIC_VAR_INT SequenceNumber; QUIC_VAR_INT PacketTolerance; QUIC_VAR_INT UpdateMaxAckDelay; // In microseconds (us) + QUIC_VAR_INT ReorderingThreshold; BOOLEAN IgnoreOrder; BOOLEAN IgnoreCE; diff --git a/src/core/quicdef.h b/src/core/quicdef.h index 07956b50fb..10bec83f95 100644 --- a/src/core/quicdef.h +++ b/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. // diff --git a/src/core/send.c b/src/core/send.c index bbd1d0493b..b1bc38d606 100644 --- a/src/core/send.c +++ b/src/core/send.c @@ -896,6 +896,7 @@ QuicSendWriteFrames( Frame.SequenceNumber = Connection->SendAckFreqSeqNum; Frame.PacketTolerance = Connection->PeerPacketTolerance; Frame.UpdateMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection)); + Frame.ReorderingThreshold = Connection->PeerReorderingThreshold; Frame.IgnoreOrder = FALSE; Frame.IgnoreCE = FALSE; From bf614d944d33fdab4e704e494023f343ff53849f Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Mon, 2 Dec 2024 16:15:43 +0530 Subject: [PATCH 02/32] modified log filesd --- src/generated/linux/frame.c.clog.h | 16 +-- src/generated/linux/frame.c.clog.h.lttng.h | 124 +++++++++++---------- 2 files changed, 73 insertions(+), 67 deletions(-) diff --git a/src/generated/linux/frame.c.clog.h b/src/generated/linux/frame.c.clog.h index 99477a4200..413f5c5f97 100644 --- a/src/generated/linux/frame.c.clog.h +++ b/src/generated/linux/frame.c.clog.h @@ -1173,16 +1173,17 @@ tracepoint(CLOG_FRAME_C, FrameLogAckFrequencyInvalid , arg2, arg3, arg4);\ /*---------------------------------------------------------- // Decoder Ring for FrameLogAckFrequency -// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu +// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu // QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", + "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", PtkConnPre(Connection), PktRxPre(Rx), PacketNumber, Frame.SequenceNumber, Frame.PacketTolerance, Frame.UpdateMaxAckDelay, + Frame.ReorderingThreshold, Frame.IgnoreOrder, Frame.IgnoreCE); // arg2 = arg2 = PtkConnPre(Connection) = arg2 @@ -1191,12 +1192,13 @@ tracepoint(CLOG_FRAME_C, FrameLogAckFrequencyInvalid , arg2, arg3, arg4);\ // arg5 = arg5 = Frame.SequenceNumber = arg5 // arg6 = arg6 = Frame.PacketTolerance = arg6 // arg7 = arg7 = Frame.UpdateMaxAckDelay = arg7 -// arg8 = arg8 = Frame.IgnoreOrder = arg8 -// arg9 = arg9 = Frame.IgnoreCE = arg9 +// arg8 = arg8 = Frame.ReorderingThreshold = arg8 +// arg9 = arg9 = Frame.IgnoreOrder = arg9 +// arg10 = arg10 = Frame.IgnoreCE = arg10 ----------------------------------------------------------*/ -#ifndef _clog_10_ARGS_TRACE_FrameLogAckFrequency -#define _clog_10_ARGS_TRACE_FrameLogAckFrequency(uniqueId, encoded_arg_string, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9)\ -tracepoint(CLOG_FRAME_C, FrameLogAckFrequency , arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9);\ +#ifndef _clog_11_ARGS_TRACE_FrameLogAckFrequency +#define _clog_11_ARGS_TRACE_FrameLogAckFrequency(uniqueId, encoded_arg_string, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10)\ +tracepoint(CLOG_FRAME_C, FrameLogAckFrequency , arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10);\ #endif diff --git a/src/generated/linux/frame.c.clog.h.lttng.h b/src/generated/linux/frame.c.clog.h.lttng.h index 5c25f96c93..a62603b987 100644 --- a/src/generated/linux/frame.c.clog.h.lttng.h +++ b/src/generated/linux/frame.c.clog.h.lttng.h @@ -21,7 +21,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogUnknownType, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -52,7 +52,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPadding, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned short, arg5), + unsigned short, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -80,7 +80,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPing, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -107,7 +107,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -140,7 +140,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAck, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -172,7 +172,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckSingleBlock, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -206,7 +206,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckMultiBlock, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -235,7 +235,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckInvalidBlock, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -262,7 +262,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckEcnInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -298,7 +298,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckEcn, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned long long, arg7), + unsigned long long, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -328,7 +328,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogResetStreamInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -364,7 +364,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogResetStream, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned long long, arg7), + unsigned long long, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -394,7 +394,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStopSendingInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -427,7 +427,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStopSending, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -456,7 +456,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogCryptoInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -489,7 +489,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogCrypto, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned short, arg6), + unsigned short, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -518,7 +518,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewTokenInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -548,7 +548,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewToken, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -576,7 +576,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -612,7 +612,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamFin, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned short, arg7), + unsigned short, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -651,7 +651,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStream, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned short, arg7), + unsigned short, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -681,7 +681,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxDataInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -711,7 +711,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxData, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -739,7 +739,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreamDataInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -772,7 +772,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreamData, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -801,7 +801,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreamsInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -834,7 +834,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreams, unsigned char, arg3, unsigned long long, arg4, unsigned short, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -863,7 +863,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDataBlockedInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -893,7 +893,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDataBlocked, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -921,7 +921,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamDataBlockedInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -954,7 +954,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamDataBlocked, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -983,7 +983,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamsBlockedInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1016,7 +1016,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamsBlocked, unsigned char, arg3, unsigned long long, arg4, unsigned short, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1045,7 +1045,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewConnectionIDInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1084,7 +1084,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewConnectionID, unsigned long long, arg5, unsigned long long, arg6, const char *, arg7, - const char *, arg8), + const char *, arg8), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1115,7 +1115,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogRetireConnectionIDInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1145,7 +1145,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogRetireConnectionID, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1173,7 +1173,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathChallengeInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1203,7 +1203,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathChallenge, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1231,7 +1231,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathResponseInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1261,7 +1261,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathResponse, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1289,7 +1289,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogConnectionCloseInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1319,7 +1319,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogConnectionCloseApp, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1353,7 +1353,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogConnectionClose, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1382,7 +1382,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogHandshakeDone, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1409,7 +1409,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDatagramInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1439,7 +1439,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDatagram, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned short, arg5), + unsigned short, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1467,7 +1467,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1479,7 +1479,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, /*---------------------------------------------------------- // Decoder Ring for FrameLogAckFrequency -// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu +// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu // QuicTraceLogVerbose( FrameLogAckFrequency, "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", @@ -1489,6 +1489,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, Frame.SequenceNumber, Frame.PacketTolerance, Frame.UpdateMaxAckDelay, + Frame.ReorderingThreshold, Frame.IgnoreOrder, Frame.IgnoreCE); // arg2 = arg2 = PtkConnPre(Connection) = arg2 @@ -1497,8 +1498,9 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, // arg5 = arg5 = Frame.SequenceNumber = arg5 // arg6 = arg6 = Frame.PacketTolerance = arg6 // arg7 = arg7 = Frame.UpdateMaxAckDelay = arg7 -// arg8 = arg8 = Frame.IgnoreOrder = arg8 -// arg9 = arg9 = Frame.IgnoreCE = arg9 +// arg8 = arg8 = Frame.ReorderingThreshold = arg8 +// arg9 = arg9 = Frame.IgnoreOrder = arg9 +// arg10 = arg10 = Frame.IgnoreCE = arg10 ----------------------------------------------------------*/ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, TP_ARGS( @@ -1508,8 +1510,9 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, unsigned long long, arg5, unsigned long long, arg6, unsigned long long, arg7, - unsigned char, arg8, - unsigned char, arg9), + unsigned long long, arg8, + unsigned char, arg9, + unsigned char, arg10), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1517,8 +1520,9 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, ctf_integer(uint64_t, arg5, arg5) ctf_integer(uint64_t, arg6, arg6) ctf_integer(uint64_t, arg7, arg7) - ctf_integer(unsigned char, arg8, arg8) + ctf_integer(uint64_t, arg8, arg8) ctf_integer(unsigned char, arg9, arg9) + ctf_integer(unsigned char, arg10, arg10) ) ) @@ -1541,7 +1545,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogImmediateAck, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1568,7 +1572,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogTimestampInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1598,7 +1602,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogTimestamp, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1626,7 +1630,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogReliableResetStreamInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1665,7 +1669,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogReliableResetStream, unsigned long long, arg5, unsigned long long, arg6, unsigned long long, arg7, - unsigned long long, arg8), + unsigned long long, arg8), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1693,7 +1697,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogReliableResetStream, TRACEPOINT_EVENT(CLOG_FRAME_C, ConnError, TP_ARGS( const void *, arg2, - const char *, arg3), + const char *, arg3), TP_FIELDS( ctf_integer_hex(uint64_t, arg2, (uint64_t)arg2) ctf_string(arg3, arg3) From 542c7812478d532a8862f961621b940e10d42170 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 3 Dec 2024 00:02:59 +0530 Subject: [PATCH 03/32] few refactoring --- src/core/connection.c | 2 +- src/core/connection.h | 22 +++++++++++----------- src/core/unittest/FrameTest.cpp | 2 +- src/manifest/clog.sidecar | 10 +++++++--- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/core/connection.c b/src/core/connection.c index 938988d70e..52ecaf0f7d 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -5247,7 +5247,7 @@ QuicConnRecvFrames( } else { Connection->PacketTolerance = UINT8_MAX; // Cap to 0xFF for space savings. } - if(Frame.ReorderingThreshold < UINT8_MAX) { + if (Frame.ReorderingThreshold < UINT8_MAX) { Connection->ReorderingThreshold = (uint8_t)Frame.ReorderingThreshold; } else { Connection->ReorderingThreshold = UINT8_MAX; // Cap to 0xFF for space savings. diff --git a/src/core/connection.h b/src/core/connection.h index e87379ccb6..208819a8aa 100644 --- a/src/core/connection.h +++ b/src/core/connection.h @@ -452,17 +452,7 @@ typedef struct QUIC_CONNECTION { // be able to send to the peer. // uint8_t PeerPacketTolerance; - - // - // The ACK frequency sequence number we are currently using to send. - // - uint64_t SendAckFreqSeqNum; - - // - // The next ACK frequency sequence number we expect to receive. - // - uint64_t NextRecvAckFreqSeqNum; - + // // The maximum number of packets that can be out of order before an immediate // acknowledgment (ACK) is triggered. If no specific instructions (ACK_FREQUENCY @@ -478,6 +468,16 @@ typedef struct QUIC_CONNECTION { // uint8_t PeerReorderingThreshold; + // + // The ACK frequency sequence number we are currently using to send. + // + uint64_t SendAckFreqSeqNum; + + // + // The next ACK frequency sequence number we expect to receive. + // + uint64_t NextRecvAckFreqSeqNum; + // // The sequence number to use for the next source CID. // diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index 7daff1d250..99103b6a9b 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -257,7 +257,7 @@ struct ResetStreamFrameParams { uint8_t TestValue = (j & 1) ? 64 : 255; if (i & 1) { - Temp.Buffer[1] = TestValue; + Temp.Buffer[1] = TestValue; } else { Temp.Buffer[1] = 0; } diff --git a/src/manifest/clog.sidecar b/src/manifest/clog.sidecar index eacefe88b5..dbf18d6c44 100644 --- a/src/manifest/clog.sidecar +++ b/src/manifest/clog.sidecar @@ -4306,7 +4306,7 @@ }, "FrameLogAckFrequency": { "ModuleProperites": {}, - "TraceString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", + "TraceString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu, "UniqueId": "FrameLogAckFrequency", "splitArgs": [ { @@ -4334,12 +4334,16 @@ "MacroVariableName": "arg7" }, { - "DefinationEncoding": "hhu", + "DefinationEncoding": "llu", "MacroVariableName": "arg8" }, { "DefinationEncoding": "hhu", "MacroVariableName": "arg9" + }, + { + "DefinationEncoding": "hhu", + "MacroVariableName": "arg10" } ], "macroName": "QuicTraceLogVerbose" @@ -14711,7 +14715,7 @@ { "UniquenessHash": "7470e68a-8ad6-563a-4957-76dc22e5deeb", "TraceID": "FrameLogAckFrequency", - "EncodingString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu" + "EncodingString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu" }, { "UniquenessHash": "262b3f95-1062-851d-c2d8-c46867da793b", From 54a90dac6750c28df571050eb6d9140d7c93d4cf Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 3 Dec 2024 02:04:02 +0530 Subject: [PATCH 04/32] minor refactoring --- src/core/unittest/FrameTest.cpp | 2 +- src/manifest/clog.sidecar | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index 99103b6a9b..7daff1d250 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -257,7 +257,7 @@ struct ResetStreamFrameParams { uint8_t TestValue = (j & 1) ? 64 : 255; if (i & 1) { - Temp.Buffer[1] = TestValue; + Temp.Buffer[1] = TestValue; } else { Temp.Buffer[1] = 0; } diff --git a/src/manifest/clog.sidecar b/src/manifest/clog.sidecar index dbf18d6c44..a950abb5ff 100644 --- a/src/manifest/clog.sidecar +++ b/src/manifest/clog.sidecar @@ -4306,7 +4306,7 @@ }, "FrameLogAckFrequency": { "ModuleProperites": {}, - "TraceString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu, + "TraceString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", "UniqueId": "FrameLogAckFrequency", "splitArgs": [ { From 80973502c282f8c1fb9c61e2fa3eef96ee421291 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 3 Dec 2024 02:33:07 +0530 Subject: [PATCH 05/32] fixed clog --- src/core/frame.c | 2 +- src/generated/linux/frame.c.clog.h.lttng.h | 114 ++++++++++----------- src/manifest/clog.sidecar | 2 +- 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/src/core/frame.c b/src/core/frame.c index 8b1bb729da..5f645d9571 100644 --- a/src/core/frame.c +++ b/src/core/frame.c @@ -1966,7 +1966,7 @@ QuicFrameLog( QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold: %llu IgnoreOrder:%hhu IgnoreCE:%hhu", + "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", PtkConnPre(Connection), PktRxPre(Rx), PacketNumber, diff --git a/src/generated/linux/frame.c.clog.h.lttng.h b/src/generated/linux/frame.c.clog.h.lttng.h index a62603b987..fbe1de81cd 100644 --- a/src/generated/linux/frame.c.clog.h.lttng.h +++ b/src/generated/linux/frame.c.clog.h.lttng.h @@ -21,7 +21,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogUnknownType, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -52,7 +52,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPadding, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned short, arg5), + unsigned short, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -80,7 +80,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPing, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -107,7 +107,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -140,7 +140,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAck, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -172,7 +172,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckSingleBlock, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -206,7 +206,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckMultiBlock, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -235,7 +235,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckInvalidBlock, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -262,7 +262,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckEcnInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -298,7 +298,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckEcn, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned long long, arg7), + unsigned long long, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -328,7 +328,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogResetStreamInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -364,7 +364,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogResetStream, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned long long, arg7), + unsigned long long, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -394,7 +394,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStopSendingInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -427,7 +427,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStopSending, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -456,7 +456,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogCryptoInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -489,7 +489,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogCrypto, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned short, arg6), + unsigned short, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -518,7 +518,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewTokenInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -548,7 +548,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewToken, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -576,7 +576,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -612,7 +612,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamFin, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned short, arg7), + unsigned short, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -651,7 +651,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStream, unsigned long long, arg4, unsigned long long, arg5, unsigned long long, arg6, - unsigned short, arg7), + unsigned short, arg7), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -681,7 +681,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxDataInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -711,7 +711,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxData, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -739,7 +739,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreamDataInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -772,7 +772,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreamData, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -801,7 +801,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreamsInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -834,7 +834,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogMaxStreams, unsigned char, arg3, unsigned long long, arg4, unsigned short, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -863,7 +863,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDataBlockedInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -893,7 +893,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDataBlocked, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -921,7 +921,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamDataBlockedInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -954,7 +954,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamDataBlocked, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -983,7 +983,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamsBlockedInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1016,7 +1016,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogStreamsBlocked, unsigned char, arg3, unsigned long long, arg4, unsigned short, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1045,7 +1045,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewConnectionIDInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1084,7 +1084,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogNewConnectionID, unsigned long long, arg5, unsigned long long, arg6, const char *, arg7, - const char *, arg8), + const char *, arg8), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1115,7 +1115,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogRetireConnectionIDInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1145,7 +1145,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogRetireConnectionID, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1173,7 +1173,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathChallengeInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1203,7 +1203,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathChallenge, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1231,7 +1231,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathResponseInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1261,7 +1261,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogPathResponse, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1289,7 +1289,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogConnectionCloseInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1319,7 +1319,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogConnectionCloseApp, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1353,7 +1353,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogConnectionClose, unsigned char, arg3, unsigned long long, arg4, unsigned long long, arg5, - unsigned long long, arg6), + unsigned long long, arg6), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1382,7 +1382,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogHandshakeDone, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1409,7 +1409,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDatagramInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1439,7 +1439,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogDatagram, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned short, arg5), + unsigned short, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1467,7 +1467,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1482,7 +1482,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, // [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu // QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", + "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", PtkConnPre(Connection), PktRxPre(Rx), PacketNumber, @@ -1512,7 +1512,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, unsigned long long, arg7, unsigned long long, arg8, unsigned char, arg9, - unsigned char, arg10), + unsigned char, arg10), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1545,7 +1545,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogImmediateAck, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1572,7 +1572,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogTimestampInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1602,7 +1602,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogTimestamp, unsigned char, arg2, unsigned char, arg3, unsigned long long, arg4, - unsigned long long, arg5), + unsigned long long, arg5), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1630,7 +1630,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogReliableResetStreamInvalid, TP_ARGS( unsigned char, arg2, unsigned char, arg3, - unsigned long long, arg4), + unsigned long long, arg4), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1669,7 +1669,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogReliableResetStream, unsigned long long, arg5, unsigned long long, arg6, unsigned long long, arg7, - unsigned long long, arg8), + unsigned long long, arg8), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1697,9 +1697,9 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogReliableResetStream, TRACEPOINT_EVENT(CLOG_FRAME_C, ConnError, TP_ARGS( const void *, arg2, - const char *, arg3), + const char *, arg3), TP_FIELDS( - ctf_integer_hex(uint64_t, arg2, (uint64_t)arg2) + ctf_integer_hex(uint64_t, arg2, arg2) ctf_string(arg3, arg3) ) ) diff --git a/src/manifest/clog.sidecar b/src/manifest/clog.sidecar index a950abb5ff..3f95bb040c 100644 --- a/src/manifest/clog.sidecar +++ b/src/manifest/clog.sidecar @@ -14713,7 +14713,7 @@ "EncodingString": "[%c][%cX][%llu] ECN [Invalid]" }, { - "UniquenessHash": "7470e68a-8ad6-563a-4957-76dc22e5deeb", + "UniquenessHash": "f1eaafb0-988a-0711-b22f-e430cc97bbbb", "TraceID": "FrameLogAckFrequency", "EncodingString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu" }, From e7b4add5313b6ef2a6d0fbc1513031159de9e89e Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 3 Dec 2024 18:52:09 +0530 Subject: [PATCH 06/32] fixed clog --- src/generated/linux/frame.c.clog.h.lttng.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/generated/linux/frame.c.clog.h.lttng.h b/src/generated/linux/frame.c.clog.h.lttng.h index fbe1de81cd..d4f8c98abc 100644 --- a/src/generated/linux/frame.c.clog.h.lttng.h +++ b/src/generated/linux/frame.c.clog.h.lttng.h @@ -1699,7 +1699,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, ConnError, const void *, arg2, const char *, arg3), TP_FIELDS( - ctf_integer_hex(uint64_t, arg2, arg2) + ctf_integer_hex(uint64_t, arg2, (uint64_t)arg2) ctf_string(arg3, arg3) ) ) From 5fdcc8dc70e3a2d4b2b8cbc59c29bc8e686dc350 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 01:36:37 +0530 Subject: [PATCH 07/32] removed IgnoreOrder --- src/core/connection.c | 2 +- src/core/frame.c | 7 +------ src/core/frame.h | 1 - src/core/send.c | 1 - 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/core/connection.c b/src/core/connection.c index 52ecaf0f7d..65cff6d2bc 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -5233,7 +5233,7 @@ QuicConnRecvFrames( } Connection->NextRecvAckFreqSeqNum = Frame.SequenceNumber + 1; - Connection->State.IgnoreReordering = Frame.IgnoreOrder; + Connection->State.IgnoreReordering = Frame.ReorderingThreshold == 0; if (Frame.UpdateMaxAckDelay == 0) { Connection->Settings.MaxAckDelayMs = 0; } else if (Frame.UpdateMaxAckDelay < 1000) { diff --git a/src/core/frame.c b/src/core/frame.c index 5f645d9571..b9aa854f56 100644 --- a/src/core/frame.c +++ b/src/core/frame.c @@ -1238,7 +1238,6 @@ typedef struct QUIC_ACK_FREQUENCY_EXTRAS { union { struct { - uint8_t IgnoreOrder : 1; uint8_t IgnoreCE : 1; uint8_t Reserved : 6; }; @@ -1268,11 +1267,9 @@ QuicAckFrequencyFrameEncode( 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; @@ -1305,7 +1302,6 @@ QuicAckFrequencyFrameDecode( !QuicUint8tDecode(BufferLength, Buffer, Offset, &Extras.Value)) { return FALSE; } - Frame->IgnoreOrder = Extras.IgnoreOrder; Frame->IgnoreCE = Extras.IgnoreCE; return TRUE; } @@ -1966,7 +1962,7 @@ QuicFrameLog( QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", + "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreCE:%hhu", PtkConnPre(Connection), PktRxPre(Rx), PacketNumber, @@ -1974,7 +1970,6 @@ QuicFrameLog( Frame.PacketTolerance, Frame.UpdateMaxAckDelay, Frame.ReorderingThreshold, - Frame.IgnoreOrder, Frame.IgnoreCE); break; } diff --git a/src/core/frame.h b/src/core/frame.h index e8aaf40e7f..9e448d83f0 100644 --- a/src/core/frame.h +++ b/src/core/frame.h @@ -847,7 +847,6 @@ typedef struct QUIC_ACK_FREQUENCY_EX { QUIC_VAR_INT PacketTolerance; QUIC_VAR_INT UpdateMaxAckDelay; // In microseconds (us) QUIC_VAR_INT ReorderingThreshold; - BOOLEAN IgnoreOrder; BOOLEAN IgnoreCE; } QUIC_ACK_FREQUENCY_EX; diff --git a/src/core/send.c b/src/core/send.c index b1bc38d606..c0664bf718 100644 --- a/src/core/send.c +++ b/src/core/send.c @@ -897,7 +897,6 @@ QuicSendWriteFrames( Frame.PacketTolerance = Connection->PeerPacketTolerance; Frame.UpdateMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection)); Frame.ReorderingThreshold = Connection->PeerReorderingThreshold; - Frame.IgnoreOrder = FALSE; Frame.IgnoreCE = FALSE; if (QuicAckFrequencyFrameEncode( From 3f17438e95a36a00cf861cc1557e999a4bb793cf Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 01:48:18 +0530 Subject: [PATCH 08/32] removed IgnoreCE --- src/core/frame.c | 31 ++++--------------------------- src/core/frame.h | 2 -- src/core/send.c | 1 - 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/core/frame.c b/src/core/frame.c index b9aa854f56..3f45f6abfa 100644 --- a/src/core/frame.c +++ b/src/core/frame.c @@ -1234,18 +1234,6 @@ QuicDatagramFrameDecode( return TRUE; } -typedef struct QUIC_ACK_FREQUENCY_EXTRAS { - - union { - struct { - uint8_t IgnoreCE : 1; - uint8_t Reserved : 6; - }; - uint8_t Value; - }; - -} QUIC_ACK_FREQUENCY_EXTRAS; - _Success_(return != FALSE) BOOLEAN QuicAckFrequencyFrameEncode( @@ -1260,25 +1248,18 @@ QuicAckFrequencyFrameEncode( QuicVarIntSize(Frame->SequenceNumber) + QuicVarIntSize(Frame->PacketTolerance) + QuicVarIntSize(Frame->UpdateMaxAckDelay) + - QuicVarIntSize(Frame->ReorderingThreshold) + - sizeof(QUIC_ACK_FREQUENCY_EXTRAS); + QuicVarIntSize(Frame->ReorderingThreshold); if (BufferLength < *Offset + RequiredLength) { return FALSE; } - CXPLAT_DBG_ASSERT(Frame->IgnoreCE <= 1); // IgnoreCE should only be 0 or 1. - - QUIC_ACK_FREQUENCY_EXTRAS Extras = { .Value = 0 }; - 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); Buffer = QuicVarIntEncode(Frame->ReorderingThreshold, Buffer); - QuicUint8Encode(Extras.Value, Buffer); *Offset += RequiredLength; return TRUE; @@ -1294,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) || - !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->ReorderingThreshold) || - !QuicUint8tDecode(BufferLength, Buffer, Offset, &Extras.Value)) { + !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->ReorderingThreshold)) { return FALSE; } - Frame->IgnoreCE = Extras.IgnoreCE; return TRUE; } @@ -1962,15 +1940,14 @@ QuicFrameLog( QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreCE:%hhu", + "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu", PtkConnPre(Connection), PktRxPre(Rx), PacketNumber, Frame.SequenceNumber, Frame.PacketTolerance, Frame.UpdateMaxAckDelay, - Frame.ReorderingThreshold, - Frame.IgnoreCE); + Frame.ReorderingThreshold); break; } diff --git a/src/core/frame.h b/src/core/frame.h index 9e448d83f0..8ff32fb9c2 100644 --- a/src/core/frame.h +++ b/src/core/frame.h @@ -847,8 +847,6 @@ typedef struct QUIC_ACK_FREQUENCY_EX { QUIC_VAR_INT PacketTolerance; QUIC_VAR_INT UpdateMaxAckDelay; // In microseconds (us) QUIC_VAR_INT ReorderingThreshold; - BOOLEAN IgnoreCE; - } QUIC_ACK_FREQUENCY_EX; _Success_(return != FALSE) diff --git a/src/core/send.c b/src/core/send.c index c0664bf718..2552774e5d 100644 --- a/src/core/send.c +++ b/src/core/send.c @@ -897,7 +897,6 @@ QuicSendWriteFrames( Frame.PacketTolerance = Connection->PeerPacketTolerance; Frame.UpdateMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection)); Frame.ReorderingThreshold = Connection->PeerReorderingThreshold; - Frame.IgnoreCE = FALSE; if (QuicAckFrequencyFrameEncode( &Frame, From d4ed89057c134a838b7a74baa52747234d6967e2 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 02:12:23 +0530 Subject: [PATCH 09/32] updated names --- src/core/connection.c | 16 ++++++------- src/core/frame.c | 18 +++++++-------- src/core/frame.h | 4 ++-- src/core/send.c | 4 ++-- src/generated/linux/frame.c.clog.h | 24 +++++++++----------- src/generated/linux/frame.c.clog.h.lttng.h | 26 ++++++++-------------- src/manifest/clog.sidecar | 14 +++--------- 7 files changed, 43 insertions(+), 63 deletions(-) diff --git a/src/core/connection.c b/src/core/connection.c index 65cff6d2bc..9edf27522f 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -5213,12 +5213,12 @@ QuicConnRecvFrames( 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; } @@ -5234,16 +5234,16 @@ QuicConnRecvFrames( Connection->NextRecvAckFreqSeqNum = Frame.SequenceNumber + 1; Connection->State.IgnoreReordering = Frame.ReorderingThreshold == 0; - 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. } diff --git a/src/core/frame.c b/src/core/frame.c index 3f45f6abfa..66d94748fb 100644 --- a/src/core/frame.c +++ b/src/core/frame.c @@ -1246,8 +1246,8 @@ QuicAckFrequencyFrameEncode( uint16_t RequiredLength = QuicVarIntSize(QUIC_FRAME_ACK_FREQUENCY) + QuicVarIntSize(Frame->SequenceNumber) + - QuicVarIntSize(Frame->PacketTolerance) + - QuicVarIntSize(Frame->UpdateMaxAckDelay) + + QuicVarIntSize(Frame->AckElicitingThreshold) + + QuicVarIntSize(Frame->RequestedMaxAckDelay) + QuicVarIntSize(Frame->ReorderingThreshold); if (BufferLength < *Offset + RequiredLength) { @@ -1257,8 +1257,8 @@ QuicAckFrequencyFrameEncode( 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); + Buffer = QuicVarIntEncode(Frame->AckElicitingThreshold, Buffer); + Buffer = QuicVarIntEncode(Frame->RequestedMaxAckDelay, Buffer); Buffer = QuicVarIntEncode(Frame->ReorderingThreshold, Buffer); *Offset += RequiredLength; @@ -1276,8 +1276,8 @@ QuicAckFrequencyFrameDecode( ) { if (!QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->SequenceNumber) || - !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->PacketTolerance) || - !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->UpdateMaxAckDelay) || + !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->AckElicitingThreshold) || + !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->RequestedMaxAckDelay) || !QuicVarIntDecode(BufferLength, Buffer, Offset, &Frame->ReorderingThreshold)) { return FALSE; } @@ -1940,13 +1940,13 @@ QuicFrameLog( QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu", + "[%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.AckElicitingThreshold, + Frame.RequestedMaxAckDelay, Frame.ReorderingThreshold); break; } diff --git a/src/core/frame.h b/src/core/frame.h index 8ff32fb9c2..ddb5c995b8 100644 --- a/src/core/frame.h +++ b/src/core/frame.h @@ -844,8 +844,8 @@ QuicDatagramFrameDecode( typedef struct QUIC_ACK_FREQUENCY_EX { QUIC_VAR_INT SequenceNumber; - QUIC_VAR_INT PacketTolerance; - QUIC_VAR_INT UpdateMaxAckDelay; // In microseconds (us) + QUIC_VAR_INT AckElicitingThreshold; + QUIC_VAR_INT RequestedMaxAckDelay; // In microseconds (us) QUIC_VAR_INT ReorderingThreshold; } QUIC_ACK_FREQUENCY_EX; diff --git a/src/core/send.c b/src/core/send.c index 2552774e5d..306801d4bf 100644 --- a/src/core/send.c +++ b/src/core/send.c @@ -894,8 +894,8 @@ QuicSendWriteFrames( QUIC_ACK_FREQUENCY_EX Frame; Frame.SequenceNumber = Connection->SendAckFreqSeqNum; - Frame.PacketTolerance = Connection->PeerPacketTolerance; - Frame.UpdateMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection)); + Frame.AckElicitingThreshold = Connection->PeerPacketTolerance; + Frame.RequestedMaxAckDelay = MS_TO_US(QuicConnGetAckDelay(Connection)); Frame.ReorderingThreshold = Connection->PeerReorderingThreshold; if (QuicAckFrequencyFrameEncode( diff --git a/src/generated/linux/frame.c.clog.h b/src/generated/linux/frame.c.clog.h index 413f5c5f97..e8c8b30384 100644 --- a/src/generated/linux/frame.c.clog.h +++ b/src/generated/linux/frame.c.clog.h @@ -1173,32 +1173,28 @@ tracepoint(CLOG_FRAME_C, FrameLogAckFrequencyInvalid , arg2, arg3, arg4);\ /*---------------------------------------------------------- // Decoder Ring for FrameLogAckFrequency -// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu +// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu AckElicitThreshold:%llu MaxAckDelay:%llu ReorderThreshold:%llu // QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%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.ReorderingThreshold, - Frame.IgnoreOrder, - Frame.IgnoreCE); + Frame.AckElicitingThreshold, + Frame.RequestedMaxAckDelay, + Frame.ReorderingThreshold); // arg2 = arg2 = PtkConnPre(Connection) = arg2 // arg3 = arg3 = PktRxPre(Rx) = arg3 // arg4 = arg4 = PacketNumber = arg4 // arg5 = arg5 = Frame.SequenceNumber = arg5 -// arg6 = arg6 = Frame.PacketTolerance = arg6 -// arg7 = arg7 = Frame.UpdateMaxAckDelay = arg7 +// arg6 = arg6 = Frame.AckElicitingThreshold = arg6 +// arg7 = arg7 = Frame.RequestedMaxAckDelay = arg7 // arg8 = arg8 = Frame.ReorderingThreshold = arg8 -// arg9 = arg9 = Frame.IgnoreOrder = arg9 -// arg10 = arg10 = Frame.IgnoreCE = arg10 ----------------------------------------------------------*/ -#ifndef _clog_11_ARGS_TRACE_FrameLogAckFrequency -#define _clog_11_ARGS_TRACE_FrameLogAckFrequency(uniqueId, encoded_arg_string, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10)\ -tracepoint(CLOG_FRAME_C, FrameLogAckFrequency , arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10);\ +#ifndef _clog_9_ARGS_TRACE_FrameLogAckFrequency +#define _clog_9_ARGS_TRACE_FrameLogAckFrequency(uniqueId, encoded_arg_string, arg2, arg3, arg4, arg5, arg6, arg7, arg8)\ +tracepoint(CLOG_FRAME_C, FrameLogAckFrequency , arg2, arg3, arg4, arg5, arg6, arg7, arg8);\ #endif diff --git a/src/generated/linux/frame.c.clog.h.lttng.h b/src/generated/linux/frame.c.clog.h.lttng.h index d4f8c98abc..2552f75ebe 100644 --- a/src/generated/linux/frame.c.clog.h.lttng.h +++ b/src/generated/linux/frame.c.clog.h.lttng.h @@ -1479,28 +1479,24 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequencyInvalid, /*---------------------------------------------------------- // Decoder Ring for FrameLogAckFrequency -// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu +// [%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu AckElicitThreshold:%llu MaxAckDelay:%llu ReorderThreshold:%llu // QuicTraceLogVerbose( FrameLogAckFrequency, - "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%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.ReorderingThreshold, - Frame.IgnoreOrder, - Frame.IgnoreCE); + Frame.AckElicitingThreshold, + Frame.RequestedMaxAckDelay, + Frame.ReorderingThreshold); // arg2 = arg2 = PtkConnPre(Connection) = arg2 // arg3 = arg3 = PktRxPre(Rx) = arg3 // arg4 = arg4 = PacketNumber = arg4 // arg5 = arg5 = Frame.SequenceNumber = arg5 -// arg6 = arg6 = Frame.PacketTolerance = arg6 -// arg7 = arg7 = Frame.UpdateMaxAckDelay = arg7 +// arg6 = arg6 = Frame.AckElicitingThreshold = arg6 +// arg7 = arg7 = Frame.RequestedMaxAckDelay = arg7 // arg8 = arg8 = Frame.ReorderingThreshold = arg8 -// arg9 = arg9 = Frame.IgnoreOrder = arg9 -// arg10 = arg10 = Frame.IgnoreCE = arg10 ----------------------------------------------------------*/ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, TP_ARGS( @@ -1510,9 +1506,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, unsigned long long, arg5, unsigned long long, arg6, unsigned long long, arg7, - unsigned long long, arg8, - unsigned char, arg9, - unsigned char, arg10), + unsigned long long, arg8), TP_FIELDS( ctf_integer(unsigned char, arg2, arg2) ctf_integer(unsigned char, arg3, arg3) @@ -1521,8 +1515,6 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, FrameLogAckFrequency, ctf_integer(uint64_t, arg6, arg6) ctf_integer(uint64_t, arg7, arg7) ctf_integer(uint64_t, arg8, arg8) - ctf_integer(unsigned char, arg9, arg9) - ctf_integer(unsigned char, arg10, arg10) ) ) @@ -1699,7 +1691,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, ConnError, const void *, arg2, const char *, arg3), TP_FIELDS( - ctf_integer_hex(uint64_t, arg2, (uint64_t)arg2) + ctf_integer_hex(uint64_t, arg2, arg2) ctf_string(arg3, arg3) ) ) diff --git a/src/manifest/clog.sidecar b/src/manifest/clog.sidecar index 3f95bb040c..907d055946 100644 --- a/src/manifest/clog.sidecar +++ b/src/manifest/clog.sidecar @@ -4306,7 +4306,7 @@ }, "FrameLogAckFrequency": { "ModuleProperites": {}, - "TraceString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu", + "TraceString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu AckElicitThreshold:%llu MaxAckDelay:%llu ReorderThreshold:%llu", "UniqueId": "FrameLogAckFrequency", "splitArgs": [ { @@ -4336,14 +4336,6 @@ { "DefinationEncoding": "llu", "MacroVariableName": "arg8" - }, - { - "DefinationEncoding": "hhu", - "MacroVariableName": "arg9" - }, - { - "DefinationEncoding": "hhu", - "MacroVariableName": "arg10" } ], "macroName": "QuicTraceLogVerbose" @@ -14713,9 +14705,9 @@ "EncodingString": "[%c][%cX][%llu] ECN [Invalid]" }, { - "UniquenessHash": "f1eaafb0-988a-0711-b22f-e430cc97bbbb", + "UniquenessHash": "92b3c109-06f5-8316-5792-593a28f376c3", "TraceID": "FrameLogAckFrequency", - "EncodingString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu PktTolerance:%llu MaxAckDelay:%llu ReorderThreshold:%llu IgnoreOrder:%hhu IgnoreCE:%hhu" + "EncodingString": "[%c][%cX][%llu] ACK_FREQUENCY SeqNum:%llu AckElicitThreshold:%llu MaxAckDelay:%llu ReorderThreshold:%llu" }, { "UniquenessHash": "262b3f95-1062-851d-c2d8-c46867da793b", From d7d69b866d5d113230a419004996b9da610c95e1 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 02:20:59 +0530 Subject: [PATCH 10/32] updated the values --- src/core/crypto_tls.c | 2 +- src/core/frame.h | 2 +- src/generated/linux/frame.c.clog.h.lttng.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/crypto_tls.c b/src/core/crypto_tls.c index da1b459030..713405fef7 100644 --- a/src/core/crypto_tls.c +++ b/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 diff --git a/src/core/frame.h b/src/core/frame.h index ddb5c995b8..f538ecffbc 100644 --- a/src/core/frame.h +++ b/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, diff --git a/src/generated/linux/frame.c.clog.h.lttng.h b/src/generated/linux/frame.c.clog.h.lttng.h index 2552f75ebe..df916db905 100644 --- a/src/generated/linux/frame.c.clog.h.lttng.h +++ b/src/generated/linux/frame.c.clog.h.lttng.h @@ -1691,7 +1691,7 @@ TRACEPOINT_EVENT(CLOG_FRAME_C, ConnError, const void *, arg2, const char *, arg3), TP_FIELDS( - ctf_integer_hex(uint64_t, arg2, arg2) + ctf_integer_hex(uint64_t, arg2, (uint64_t)arg2) ctf_string(arg3, arg3) ) ) From 93dd676c87f6b32f5bd0b11c14ad63f4f70cd69a Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 17:04:58 +0530 Subject: [PATCH 11/32] added logic for reordering --- src/core/ack_tracker.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 9114b52af9..919d36dfb2 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -96,6 +96,20 @@ QuicAckTrackerAddPacketNumber( !RangeUpdated; } +BOOLEAN +QuicAckTrackerDidHitReorderingThreshold( + _In_ QUIC_ACK_TRACKER* Tracker, + _In_ uint8_t ReorderingThreshold + ) +{ + uint64_t LargestUnackedPacketNumber = QuicRangeGetMax(&Tracker->PacketNumbersToAck); + uint64_t SmallestUnreportedMissingPacketNumber = + QuicRangeGetHigh( + QuicRangeGet(&Tracker->PacketNumbersReceived, 0)) + 1; + + return LargestUnackedPacketNumber - SmallestUnreportedMissingPacketNumber >= ReorderingThreshold; +} + _IRQL_requires_max_(DISPATCH_LEVEL) void QuicAckTrackerAckPacket( @@ -198,10 +212,8 @@ QuicAckTrackerAckPacket( (!Connection->State.IgnoreReordering && Connection->ReorderingThreshold > 0 && (NewLargestPacketNumber && - QuicRangeSize(&Tracker->PacketNumbersToAck) > 1 && // There are more than two ranges, i.e. a gap somewhere. - QuicRangeGet( - &Tracker->PacketNumbersToAck, - QuicRangeSize(&Tracker->PacketNumbersToAck) - 1)->Count == Connection->ReorderingThreshold))) { + QuicRangeSize(&Tracker->PacketNumbersToAck) > 1 && // There are more than two ranges, i.e. a gap somewhere. + QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold)))) { // // Send the ACK immediately. // From 4a2c32796c006c6688bfa82dfa143122ee8edeb8 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 17:15:21 +0530 Subject: [PATCH 12/32] removed ignorereordering parameter in connection --- src/core/ack_tracker.c | 5 ++--- src/core/connection.c | 1 - src/core/connection.h | 8 +------- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 919d36dfb2..312ca8de66 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -199,7 +199,7 @@ QuicAckTrackerAckPacket( // 4. We had 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 'IgnoreReordering' is TRUE or the Reordering Threshold is 0. + // 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 @@ -209,8 +209,7 @@ QuicAckTrackerAckPacket( if (AckType == QUIC_ACK_TYPE_ACK_IMMEDIATE || Connection->Settings.MaxAckDelayMs == 0 || (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) || - (!Connection->State.IgnoreReordering && - Connection->ReorderingThreshold > 0 && + (Connection->ReorderingThreshold > 0 && (NewLargestPacketNumber && QuicRangeSize(&Tracker->PacketNumbersToAck) > 1 && // There are more than two ranges, i.e. a gap somewhere. QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold)))) { diff --git a/src/core/connection.c b/src/core/connection.c index 9edf27522f..b59d19ba45 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -5233,7 +5233,6 @@ QuicConnRecvFrames( } Connection->NextRecvAckFreqSeqNum = Frame.SequenceNumber + 1; - Connection->State.IgnoreReordering = Frame.ReorderingThreshold == 0; if (Frame.RequestedMaxAckDelay == 0) { Connection->Settings.MaxAckDelayMs = 0; } else if (Frame.RequestedMaxAckDelay < 1000) { diff --git a/src/core/connection.h b/src/core/connection.h index 208819a8aa..c754115f52 100644 --- a/src/core/connection.h +++ b/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). @@ -452,7 +446,7 @@ typedef struct QUIC_CONNECTION { // be able to send to the peer. // 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 From 77474a6363ce1b767b0985ae240bf12e0ad63cda Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 4 Dec 2024 17:54:21 +0530 Subject: [PATCH 13/32] added assert --- src/core/ack_tracker.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 312ca8de66..607aa10c4a 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -102,6 +102,9 @@ QuicAckTrackerDidHitReorderingThreshold( _In_ uint8_t ReorderingThreshold ) { + CXPLAT_DBG_ASSERT(ReorderingThreshold > 0); + CXPLAT_DBG_ASSERT(QuicRangeSize(&Tracker->PacketNumbersToAck) > 1); + uint64_t LargestUnackedPacketNumber = QuicRangeGetMax(&Tracker->PacketNumbersToAck); uint64_t SmallestUnreportedMissingPacketNumber = QuicRangeGetHigh( From d607245839f6f5e14fa4ea5c31e7d07c5b318789 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 5 Dec 2024 00:56:21 +0530 Subject: [PATCH 14/32] updated logic for the reordering threshold function --- src/core/ack_tracker.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 607aa10c4a..96c66934f6 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -99,11 +99,15 @@ QuicAckTrackerAddPacketNumber( BOOLEAN QuicAckTrackerDidHitReorderingThreshold( _In_ QUIC_ACK_TRACKER* Tracker, - _In_ uint8_t ReorderingThreshold + _In_ uint8_t ReorderingThreshold, + _In_ uint64_t PacketNumber ) { - CXPLAT_DBG_ASSERT(ReorderingThreshold > 0); - CXPLAT_DBG_ASSERT(QuicRangeSize(&Tracker->PacketNumbersToAck) > 1); + if (ReorderingThreshold == 0 || + QuicRangeSize(&Tracker->PacketNumbersToAck) <= 1 || + PacketNumber != QuicRangeGetMax(&Tracker->PacketNumbersToAck)) { // There are more than two ranges, i.e. a gap somewhere. + return FALSE; + } uint64_t LargestUnackedPacketNumber = QuicRangeGetMax(&Tracker->PacketNumbersToAck); uint64_t SmallestUnreportedMissingPacketNumber = @@ -212,10 +216,7 @@ QuicAckTrackerAckPacket( if (AckType == QUIC_ACK_TYPE_ACK_IMMEDIATE || Connection->Settings.MaxAckDelayMs == 0 || (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) || - (Connection->ReorderingThreshold > 0 && - (NewLargestPacketNumber && - QuicRangeSize(&Tracker->PacketNumbersToAck) > 1 && // There are more than two ranges, i.e. a gap somewhere. - QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold)))) { + QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold, PacketNumber)) { // // Send the ACK immediately. // From 7a8cbad712c7cb75c6b76d7a18ce0f3d20b296d6 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 5 Dec 2024 02:58:55 +0530 Subject: [PATCH 15/32] minor change --- src/core/ack_tracker.c | 2 +- src/core/ack_tracker.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 96c66934f6..08a4a295fc 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -112,7 +112,7 @@ QuicAckTrackerDidHitReorderingThreshold( uint64_t LargestUnackedPacketNumber = QuicRangeGetMax(&Tracker->PacketNumbersToAck); uint64_t SmallestUnreportedMissingPacketNumber = QuicRangeGetHigh( - QuicRangeGet(&Tracker->PacketNumbersReceived, 0)) + 1; + QuicRangeGet(&Tracker->PacketNumbersToAck, 0)) + 1; return LargestUnackedPacketNumber - SmallestUnreportedMissingPacketNumber >= ReorderingThreshold; } diff --git a/src/core/ack_tracker.h b/src/core/ack_tracker.h index 802c337ce9..f58437c76e 100644 --- a/src/core/ack_tracker.h +++ b/src/core/ack_tracker.h @@ -91,6 +91,13 @@ QuicAckTrackerAddPacketNumber( _In_ uint64_t PacketNumber ); +BOOLEAN +QuicAckTrackerDidHitReorderingThreshold( + _In_ QUIC_ACK_TRACKER* Tracker, + _In_ uint8_t ReorderingThreshold, + _In_ uint64_t PacketNumber + ); + typedef enum QUIC_ACK_TYPE { QUIC_ACK_TYPE_NON_ACK_ELICITING, QUIC_ACK_TYPE_ACK_ELICITING, From 4c0e62a3635c0a7a40d5554c4fe6e80018c960d3 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 5 Dec 2024 18:04:14 +0530 Subject: [PATCH 16/32] added unit test --- src/core/ack_tracker.h | 8 ++++++- src/core/unittest/FrameTest.cpp | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/core/ack_tracker.h b/src/core/ack_tracker.h index f58437c76e..490e34e1cf 100644 --- a/src/core/ack_tracker.h +++ b/src/core/ack_tracker.h @@ -4,7 +4,9 @@ Licensed under the MIT License. --*/ - +#if defined(__cplusplus) +extern "C" { +#endif typedef struct QUIC_ACK_TRACKER { // @@ -153,3 +155,7 @@ QuicAckTrackerHasPacketsToAck( !Tracker->AlreadyWrittenAckFrame && QuicRangeSize(&Tracker->PacketNumbersToAck) != 0; } + +#if defined(__cplusplus) +} +#endif \ No newline at end of file diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index 7daff1d250..274034661d 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -243,6 +243,43 @@ TEST(FrameTest, ReliableResetStreamFrameEncodeDecode) ASSERT_EQ(Frame.ReliableSize, DecodedFrame.ReliableSize); } +TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) +{ + QUIC_ACK_TRACKER Tracker; + uint8_t ReorderingThreshold; + + // Initialize the Tracker and other variables + + QuicAckTrackerInitialize(&Tracker); + + // Test case 1: ReorderingThreshold is 0 + ReorderingThreshold = 0; + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 100); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 100)); + + // Test case 2: The number of ranges is less than 2. + ReorderingThreshold = 3; + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 101); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 101)); + + // Test case 3: PacketNumber is not the largest unacked packet number + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 104); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 105); + + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 103)); + + // Test case 4: Gap between smallest unreported missing packet and largest unacked packet is less than ReorderingThreshold + ReorderingThreshold = 5; + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 105)); + + // Test case 5: Gap between smallest unreported missing packet and largest unacked packet is greater than or equal to ReorderingThreshold + ReorderingThreshold = 3; + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 105)); + + // Clean up + QuicAckTrackerUninitialize(&Tracker); +} + struct ResetStreamFrameParams { uint8_t Buffer[4]; uint16_t BufferLength = 4; From 6191f1fe508b3ba575cdde00a5325e712208e787 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 6 Dec 2024 02:36:17 +0530 Subject: [PATCH 17/32] added more unit test cases --- src/core/ack_tracker.h | 2 +- src/core/unittest/FrameTest.cpp | 68 ++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/core/ack_tracker.h b/src/core/ack_tracker.h index 490e34e1cf..309255ce3b 100644 --- a/src/core/ack_tracker.h +++ b/src/core/ack_tracker.h @@ -158,4 +158,4 @@ QuicAckTrackerHasPacketsToAck( #if defined(__cplusplus) } -#endif \ No newline at end of file +#endif diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index 274034661d..91999f3be1 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -252,30 +252,78 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) QuicAckTrackerInitialize(&Tracker); - // Test case 1: ReorderingThreshold is 0 + // ReorderingThreshold is 0 ReorderingThreshold = 0; QuicRangeAddValue(&Tracker.PacketNumbersToAck, 100); ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 100)); - // Test case 2: The number of ranges is less than 2. + // The number of ranges is less than 2. ReorderingThreshold = 3; QuicRangeAddValue(&Tracker.PacketNumbersToAck, 101); ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 101)); - // Test case 3: PacketNumber is not the largest unacked packet number + // PacketNumber is not the largest unacked packet number QuicRangeAddValue(&Tracker.PacketNumbersToAck, 104); QuicRangeAddValue(&Tracker.PacketNumbersToAck, 105); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 103)); - // Test case 4: Gap between smallest unreported missing packet and largest unacked packet is less than ReorderingThreshold + // Case 1 + QuicAckTrackerReset(&Tracker); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 0); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 0)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 1); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 1)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 3); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 3)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 4); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 4)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 5); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 5)); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 2); // 2 is reported missing + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 8)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 9); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 9)); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 6); // 6 is reported missing + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 10); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 10)); + + // Case 2 ReorderingThreshold = 5; - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 105)); + QuicAckTrackerReset(&Tracker); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 0); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 0)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 1); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 1)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 3); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 3)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 5); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 5)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 6); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 6)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 7); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 7)); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 2); // 2 is reported missing + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); + ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 8)); + + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 9); + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 9)); + - // Test case 5: Gap between smallest unreported missing packet and largest unacked packet is greater than or equal to ReorderingThreshold - ReorderingThreshold = 3; - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold, 105)); - // Clean up QuicAckTrackerUninitialize(&Tracker); } From 1d1e1306b33eb11cde1a41a35e78457a2e282cd1 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 12 Dec 2024 01:36:59 +0530 Subject: [PATCH 18/32] 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 From 4a9aae6ea5d3ee41460d3972bd20fcb7a661456f Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 12 Dec 2024 01:47:09 +0530 Subject: [PATCH 19/32] minor refactoring --- src/core/ack_tracker.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 2b16708392..4dd5b90407 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -123,11 +123,9 @@ QuicAckTrackerDidHitReorderingThreshold( if (result >= 0 && result < (int)QuicRangeSize(&Tracker->PacketNumbersToAck) - 1) { SmallestUnreportedMissingPacketNumber = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, result)) + 1; - } - else if (result < 0) { + } else if (result < 0) { SmallestUnreportedMissingPacketNumber = LargestReported; - } - else { + } else { return FALSE; } From f354d437c2e5bde3ad62a5e5da26935910fcd32d Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 13 Dec 2024 02:03:02 +0530 Subject: [PATCH 20/32] minor refactoring --- src/core/ack_tracker.c | 15 ++++++--------- src/core/ack_tracker.h | 2 ++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 4dd5b90407..09946b8a81 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -110,21 +110,18 @@ QuicAckTrackerDidHitReorderingThreshold( uint64_t LargestReported = 0; // The largest packet number that could be declared lost uint64_t SmallestUnreportedMissingPacketNumber; - if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold) { + if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold && + Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 <= LargestUnackedPacketNumber) { 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) { + if (result < 0) { + SmallestUnreportedMissingPacketNumber = LargestReported; + } else if (result < (int)QuicRangeSize(&Tracker->PacketNumbersToAck) - 1) { SmallestUnreportedMissingPacketNumber = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, result)) + 1; - } else if (result < 0) { - SmallestUnreportedMissingPacketNumber = LargestReported; } else { return FALSE; } @@ -218,7 +215,7 @@ QuicAckTrackerAckPacket( // 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 had received an ACK eliciting packet that is out of order and the + // 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. diff --git a/src/core/ack_tracker.h b/src/core/ack_tracker.h index 12a82441e5..041d86a778 100644 --- a/src/core/ack_tracker.h +++ b/src/core/ack_tracker.h @@ -4,9 +4,11 @@ Licensed under the MIT License. --*/ + #if defined(__cplusplus) extern "C" { #endif + typedef struct QUIC_ACK_TRACKER { // From 2cb10efe7ac6c4c48e1f9861bd8a780e2b5b51c2 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Mon, 16 Dec 2024 02:12:49 +0530 Subject: [PATCH 21/32] changed variable names --- src/core/ack_tracker.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 09946b8a81..a9368cf562 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -96,6 +96,10 @@ QuicAckTrackerAddPacketNumber( !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, @@ -106,27 +110,27 @@ QuicAckTrackerDidHitReorderingThreshold( return FALSE; } - uint64_t LargestUnackedPacketNumber = QuicRangeGetMax(&Tracker->PacketNumbersToAck); + uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); uint64_t LargestReported = 0; // The largest packet number that could be declared lost - uint64_t SmallestUnreportedMissingPacketNumber; + uint64_t SmallestUnreportedMissing; if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold && - Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 <= LargestUnackedPacketNumber) { + Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 <= LargestUnacked) { LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; } QUIC_RANGE_SEARCH_KEY Key = { LargestReported, LargestReported }; int result = QuicRangeSearch(&Tracker->PacketNumbersToAck, &Key); if (result < 0) { - SmallestUnreportedMissingPacketNumber = LargestReported; + SmallestUnreportedMissing = LargestReported; } else if (result < (int)QuicRangeSize(&Tracker->PacketNumbersToAck) - 1) { - SmallestUnreportedMissingPacketNumber = + SmallestUnreportedMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, result)) + 1; } else { return FALSE; } - return LargestUnackedPacketNumber - SmallestUnreportedMissingPacketNumber >= ReorderingThreshold; + return LargestUnacked - SmallestUnreportedMissing >= ReorderingThreshold; } _IRQL_requires_max_(DISPATCH_LEVEL) From 5e48229d6ede7dfb57b2161630dd7daf04bd0930 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 17 Dec 2024 02:39:56 +0530 Subject: [PATCH 22/32] modified way of implementing to be more optimised --- src/core/ack_tracker.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index a9368cf562..c3fcd51084 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -110,27 +110,42 @@ QuicAckTrackerDidHitReorderingThreshold( return FALSE; } - uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); + const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); uint64_t LargestReported = 0; // The largest packet number that could be declared lost - uint64_t SmallestUnreportedMissing; if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold && Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 <= LargestUnacked) { LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; } - QUIC_RANGE_SEARCH_KEY Key = { LargestReported, LargestReported }; - int result = QuicRangeSearch(&Tracker->PacketNumbersToAck, &Key); - if (result < 0) { - SmallestUnreportedMissing = LargestReported; - } else if (result < (int)QuicRangeSize(&Tracker->PacketNumbersToAck) - 1) { - SmallestUnreportedMissing = - QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, result)) + 1; - } else { - return FALSE; - } + // + // 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. + // - return LargestUnacked - SmallestUnreportedMissing >= ReorderingThreshold; + for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index > 0; --Index) { + uint64_t SmallestMissing = 0; // Smallest missing after this range + uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Highest missing in this range + 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) From d2baaa0807f6b16c7e7e589e987378239d3b28bc Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 18 Dec 2024 01:30:50 +0530 Subject: [PATCH 23/32] addressed comments --- src/core/ack_tracker.c | 22 ++++++++++++++-------- src/core/unittest/FrameTest.cpp | 7 +++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index c3fcd51084..7298babfef 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -113,8 +113,12 @@ QuicAckTrackerDidHitReorderingThreshold( const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); uint64_t LargestReported = 0; // The largest packet number that could be declared lost - if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold && - Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 <= LargestUnacked) { + if (Tracker->LargestPacketNumberAcknowledged != 0 && + Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 > LargestUnacked) { + return FALSE; + } + + if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold) { LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; } @@ -126,18 +130,20 @@ QuicAckTrackerDidHitReorderingThreshold( // acknowledgement. // - for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index > 0; --Index) { - uint64_t SmallestMissing = 0; // Smallest missing after this range - uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Highest missing in this range - if(Index != 0) { + 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 { + } else { return FALSE; } } diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index d649e39ddb..4681fef97c 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -316,6 +316,13 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); Tracker.LargestPacketNumberAcknowledged = 9; + // Case 3 + ReorderingThreshold = 3; + QuicAckTrackerReset(&Tracker); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 7); + QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); + Tracker.LargestPacketNumberAcknowledged = 5; + ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); // Clean up QuicAckTrackerUninitialize(&Tracker); From 95a3533095b4b6b33df1bfcb16300b99c363a6fe Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 18 Dec 2024 18:54:58 +0530 Subject: [PATCH 24/32] resolved comments --- src/core/ack_tracker.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 7298babfef..bd30f2bd9e 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -113,11 +113,6 @@ QuicAckTrackerDidHitReorderingThreshold( 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; } @@ -130,14 +125,14 @@ QuicAckTrackerDidHitReorderingThreshold( // acknowledgement. // - for (int Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index >= 0; --Index) { + for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck); 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 + uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)->Low; // Highest missing in the prevous gap if (HighestMissing == 0) { return FALSE; } - if (Index != 0) { - SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; + if (Index != 1) { + SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 2)) + 1; } if (LargestReported > SmallestMissing) { From a928db699e14f82661a8426c4eca81e7291101ad Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Thu, 19 Dec 2024 16:27:00 +0530 Subject: [PATCH 25/32] modified variable name --- src/core/ack_tracker.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index bd30f2bd9e..8889b13710 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -127,8 +127,8 @@ QuicAckTrackerDidHitReorderingThreshold( for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck); Index > 0; --Index) { uint64_t SmallestMissing = 0; // Smallest missing in the previous gap - uint64_t HighestMissing = QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)->Low; // Highest missing in the prevous gap - if (HighestMissing == 0) { + uint64_t RangeStart = QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)->Low; // Lowest Packet number in the subrange + if (RangeStart == 0) { return FALSE; } if (Index != 1) { @@ -136,7 +136,7 @@ QuicAckTrackerDidHitReorderingThreshold( } if (LargestReported > SmallestMissing) { - if (HighestMissing > LargestReported) { + if (RangeStart > LargestReported) { SmallestMissing = LargestReported; } else { return FALSE; From 70b564ecbc2873f4bb3869dd4b49291aefde73a8 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 24 Dec 2024 02:31:25 +0530 Subject: [PATCH 26/32] changed the alogrithm logic --- src/core/ack_tracker.c | 32 ++++++---- src/core/unittest/FrameTest.cpp | 107 ++++++++++---------------------- 2 files changed, 53 insertions(+), 86 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 8889b13710..49bfa6a869 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -111,10 +111,18 @@ QuicAckTrackerDidHitReorderingThreshold( } const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); - uint64_t LargestReported = 0; // The largest packet number that could be declared lost + uint64_t LargestReported; // The largest packet number that could be declared lost - if (Tracker->LargestPacketNumberAcknowledged >= ReorderingThreshold) { + // + // Largest Reported is equal to the largest packet number acknowledged minus the Reordering Threshold. + // If the difference between the largest packet number acknowledged and the Reordering Threshold is smaller than the + // smallest packet in the ack tracker, then the largest reported is the smallest packet in the ack tracker. + // + + if (Tracker->LargestPacketNumberAcknowledged >= QuicRangeGet(&Tracker->PacketNumbersToAck, 0)->Low + ReorderingThreshold) { LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; + } else { + LargestReported = QuicRangeGet(&Tracker->PacketNumbersToAck, 0)->Low; } // @@ -125,16 +133,13 @@ QuicAckTrackerDidHitReorderingThreshold( // acknowledgement. // - for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck); Index > 0; --Index) { - uint64_t SmallestMissing = 0; // Smallest missing in the previous gap - uint64_t RangeStart = QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)->Low; // Lowest Packet number in the subrange - if (RangeStart == 0) { - return FALSE; - } - if (Index != 1) { - SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 2)) + 1; - } + for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index > 0; --Index) { + uint64_t SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; // Smallest missing in the previous gap + uint64_t RangeStart = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Lowest Packet number in the subrange + // + // Check if largest reported packet is missing. In that case, the smalles missing packet becomes the largest reported packet. + // if (LargestReported > SmallestMissing) { if (RangeStart > LargestReported) { SmallestMissing = LargestReported; @@ -331,11 +336,12 @@ QuicAckTrackerOnAckFrameAcked( // // Drop all packet numbers less than or equal to the largest acknowledged - // packet number. + // packet number - Reordering Threshold + 1. This is so that we dont lose + // memory of packets that are missing and are yet to be reported. // QuicRangeSetMin( &Tracker->PacketNumbersToAck, - LargestAckedPacketNumber + 1); + LargestAckedPacketNumber - Connection->ReorderingThreshold + 2); if (!QuicAckTrackerHasPacketsToAck(Tracker) && Tracker->AckElicitingPacketsToAcknowledge) { diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index 4681fef97c..fa8b0e00a4 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -243,89 +243,50 @@ TEST(FrameTest, ReliableResetStreamFrameEncodeDecode) ASSERT_EQ(Frame.ReliableSize, DecodedFrame.ReliableSize); } -TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) +BOOLEAN DidHitReorderingThreshold( + uint8_t ReorderingThreshold, + uint64_t LargestPacketNumberAcknowledged, + const std::vector>& AckTrackerLists) { QUIC_ACK_TRACKER Tracker; - uint8_t ReorderingThreshold; - - // Initialize the Tracker and other variables + QuicAckTrackerInitialize(&Tracker);; + for (const auto& List : AckTrackerLists) { + for (int i : List) { + QuicRangeAddValue(&Tracker.PacketNumbersToAck, i); + } + } + Tracker.LargestPacketNumberAcknowledged = LargestPacketNumberAcknowledged; + BOOLEAN Result = QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold); + QuicAckTrackerUninitialize(&Tracker); + return Result; +} - QuicAckTrackerInitialize(&Tracker); +TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) +{ - // ReorderingThreshold is 0 - ReorderingThreshold = 0; - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 100); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + ASSERT_FALSE(DidHitReorderingThreshold(0, 0, {{100}})); // Case 1 - ReorderingThreshold = 3; - QuicAckTrackerReset(&Tracker); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 0); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 1); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 3); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 4); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 5); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - Tracker.LargestPacketNumberAcknowledged = 5; - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 9); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - Tracker.LargestPacketNumberAcknowledged = 9; - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 10); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - Tracker.LargestPacketNumberAcknowledged = 10; + ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0}})); + ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0, 1}})); + ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0, 1}, {3}})); + ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0, 1}, {3, 4}})); + ASSERT_TRUE(DidHitReorderingThreshold(3, 0, {{0, 1}, {3, 4, 5}})); + ASSERT_FALSE(DidHitReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8}})); + ASSERT_TRUE(DidHitReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8, 9}})); + ASSERT_TRUE(DidHitReorderingThreshold(3, 9, {{0, 1}, {3, 4, 5}, {8, 9, 10}})); // Case 2 - ReorderingThreshold = 5; - QuicAckTrackerReset(&Tracker); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 0); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 1); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 3); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 5); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 6); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); + ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0}})); + ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}})); + ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}})); + ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}, {5}})); + ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6}})); + ASSERT_TRUE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6, 7}})); + ASSERT_FALSE(DidHitReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8}})); + ASSERT_TRUE(DidHitReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8, 9}})); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 7); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - Tracker.LargestPacketNumberAcknowledged = 7; - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); - ASSERT_FALSE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 9); - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - Tracker.LargestPacketNumberAcknowledged = 9; - - // Case 3 - ReorderingThreshold = 3; - QuicAckTrackerReset(&Tracker); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 7); - QuicRangeAddValue(&Tracker.PacketNumbersToAck, 8); - Tracker.LargestPacketNumberAcknowledged = 5; - ASSERT_TRUE(QuicAckTrackerDidHitReorderingThreshold(&Tracker, ReorderingThreshold)); - - // Clean up - QuicAckTrackerUninitialize(&Tracker); } struct ResetStreamFrameParams { From f53a7fbf263a218e55f3cd54a260e594597fd122 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Tue, 24 Dec 2024 19:12:11 +0530 Subject: [PATCH 27/32] resolved comments --- src/core/ack_tracker.c | 44 +++++++++++++++----------- src/core/unittest/FrameTest.cpp | 56 +++++++++++++++++++-------------- 2 files changed, 58 insertions(+), 42 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 49bfa6a869..37b37e2330 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -111,18 +111,21 @@ QuicAckTrackerDidHitReorderingThreshold( } const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); + const uint64_t SmallestTracked = QuicRangeGet(&Tracker->PacketNumbersToAck, 0)->Low; uint64_t LargestReported; // The largest packet number that could be declared lost // - // Largest Reported is equal to the largest packet number acknowledged minus the Reordering Threshold. - // If the difference between the largest packet number acknowledged and the Reordering Threshold is smaller than the - // smallest packet in the ack tracker, then the largest reported is the smallest packet in the ack tracker. + // Largest Reported is equal to the largest packet number acknowledged minus the + // Reordering Threshold. If the difference between the largest packet number + // acknowledged and the Reordering Threshold is smaller than the smallest packet + // in the ack tracker, then the largest reported is the smallest packet in the ack + // tracker. // - if (Tracker->LargestPacketNumberAcknowledged >= QuicRangeGet(&Tracker->PacketNumbersToAck, 0)->Low + ReorderingThreshold) { + if (Tracker->LargestPacketNumberAcknowledged >= SmallestTracked + ReorderingThreshold) { LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; } else { - LargestReported = QuicRangeGet(&Tracker->PacketNumbersToAck, 0)->Low; + LargestReported = SmallestTracked; } // @@ -134,20 +137,26 @@ QuicAckTrackerDidHitReorderingThreshold( // for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index > 0; --Index) { - uint64_t SmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; // Smallest missing in the previous gap - uint64_t RangeStart = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // Lowest Packet number in the subrange + uint64_t PreviousSmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; + const uint64_t RangeStart = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; // - // Check if largest reported packet is missing. In that case, the smalles missing packet becomes the largest reported packet. + // Check if largest reported packet is missing. In that case, the smallest missing + // packet becomes the largest reported packet. // - if (LargestReported > SmallestMissing) { - if (RangeStart > LargestReported) { - SmallestMissing = LargestReported; - } else { - return FALSE; - } + + if (LargestReported >= RangeStart) { + // + // Since we are only looking for packets more than LargestReported, we return + // false here. + // + return FALSE; + } + + if (LargestReported > PreviousSmallestMissing) { + PreviousSmallestMissing = LargestReported; } - if (LargestUnacked - SmallestMissing >= ReorderingThreshold) { + if (LargestUnacked - PreviousSmallestMissing >= ReorderingThreshold) { return TRUE; } } @@ -336,12 +345,11 @@ QuicAckTrackerOnAckFrameAcked( // // Drop all packet numbers less than or equal to the largest acknowledged - // packet number - Reordering Threshold + 1. This is so that we dont lose - // memory of packets that are missing and are yet to be reported. + // packet number. // QuicRangeSetMin( &Tracker->PacketNumbersToAck, - LargestAckedPacketNumber - Connection->ReorderingThreshold + 2); + LargestAckedPacketNumber + 1); if (!QuicAckTrackerHasPacketsToAck(Tracker) && Tracker->AckElicitingPacketsToAcknowledge) { diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index fa8b0e00a4..da024b83ca 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -243,13 +243,20 @@ TEST(FrameTest, ReliableResetStreamFrameEncodeDecode) ASSERT_EQ(Frame.ReliableSize, DecodedFrame.ReliableSize); } -BOOLEAN DidHitReorderingThreshold( +// +// Tests if the reordering threshold has been hit. This function initializes a +// QUIC_ACK_TRACKER and populates it with the provided packet numbers. It then +// calls QuicAckTrackerDidHitReorderingThreshold to determine if the reordering +// threshold has been hit. +// +// +bool TestReorderingThreshold( uint8_t ReorderingThreshold, uint64_t LargestPacketNumberAcknowledged, const std::vector>& AckTrackerLists) { QUIC_ACK_TRACKER Tracker; - QuicAckTrackerInitialize(&Tracker);; + QuicAckTrackerInitialize(&Tracker); for (const auto& List : AckTrackerLists) { for (int i : List) { QuicRangeAddValue(&Tracker.PacketNumbersToAck, i); @@ -263,30 +270,31 @@ BOOLEAN DidHitReorderingThreshold( TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) { - - ASSERT_FALSE(DidHitReorderingThreshold(0, 0, {{100}})); - - // Case 1 - - ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0}})); - ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0, 1}})); - ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0, 1}, {3}})); - ASSERT_FALSE(DidHitReorderingThreshold(3, 0, {{0, 1}, {3, 4}})); - ASSERT_TRUE(DidHitReorderingThreshold(3, 0, {{0, 1}, {3, 4, 5}})); - ASSERT_FALSE(DidHitReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8}})); - ASSERT_TRUE(DidHitReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8, 9}})); - ASSERT_TRUE(DidHitReorderingThreshold(3, 9, {{0, 1}, {3, 4, 5}, {8, 9, 10}})); + ASSERT_FALSE(TestReorderingThreshold(0, 0, {{100}})); + + // Case 1 + ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0}})); + ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0, 1}})); + ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0, 1}, {3}})); + ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0, 1}, {3, 4}})); + ASSERT_TRUE(TestReorderingThreshold(3, 0, {{0, 1}, {3, 4, 5}})); + ASSERT_FALSE(TestReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8}})); + ASSERT_TRUE(TestReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8, 9}})); + ASSERT_TRUE(TestReorderingThreshold(3, 9, {{0, 1}, {3, 4, 5}, {8, 9, 10}})); // Case 2 - ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0}})); - ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}})); - ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}})); - ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}, {5}})); - ASSERT_FALSE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6}})); - ASSERT_TRUE(DidHitReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6, 7}})); - ASSERT_FALSE(DidHitReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8}})); - ASSERT_TRUE(DidHitReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8, 9}})); - + ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0}})); + ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}})); + ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}, {3}})); + ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5}})); + ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6}})); + ASSERT_TRUE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6, 7}})); + ASSERT_FALSE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8}})); + ASSERT_TRUE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8, 9}})); + + ASSERT_TRUE(TestReorderingThreshold(5, 4, {{1, 2}, {4}, {10}})); + ASSERT_FALSE(TestReorderingThreshold(5, 0, {{1, 2}, {4}})); + ASSERT_FALSE(TestReorderingThreshold(5, 2, {{1, 2}, {4}})); } struct ResetStreamFrameParams { From 56f3f007accbbe699af5a560f7a15a90c960345c Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 25 Dec 2024 03:16:33 +0530 Subject: [PATCH 28/32] addressed comments --- src/core/ack_tracker.c | 28 ++++++++++++++-------------- src/core/frame.c | 2 +- src/core/unittest/FrameTest.cpp | 28 ++++++++++++++++------------ 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 37b37e2330..e4a4287f07 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -106,27 +106,25 @@ QuicAckTrackerDidHitReorderingThreshold( _In_ uint8_t ReorderingThreshold ) { - if (ReorderingThreshold == 0) { + if (ReorderingThreshold == 0 || QuicRangeSize(&Tracker->PacketNumbersToAck) < 2) { return FALSE; } const uint64_t LargestUnacked = QuicRangeGetMax(&Tracker->PacketNumbersToAck); const uint64_t SmallestTracked = QuicRangeGet(&Tracker->PacketNumbersToAck, 0)->Low; - uint64_t LargestReported; // The largest packet number that could be declared lost // // Largest Reported is equal to the largest packet number acknowledged minus the // Reordering Threshold. If the difference between the largest packet number // acknowledged and the Reordering Threshold is smaller than the smallest packet // in the ack tracker, then the largest reported is the smallest packet in the ack - // tracker. + // tracker. // - if (Tracker->LargestPacketNumberAcknowledged >= SmallestTracked + ReorderingThreshold) { - LargestReported = Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1; - } else { - LargestReported = SmallestTracked; - } + const uint64_t LargestReported = + (Tracker->LargestPacketNumberAcknowledged >= SmallestTracked + ReorderingThreshold) ? + Tracker->LargestPacketNumberAcknowledged - ReorderingThreshold + 1 : + SmallestTracked; // // Loop through all previous ACK ranges (before last) to find the smallest missing @@ -137,13 +135,7 @@ QuicAckTrackerDidHitReorderingThreshold( // for (uint32_t Index = QuicRangeSize(&Tracker->PacketNumbersToAck) - 1; Index > 0; --Index) { - uint64_t PreviousSmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; const uint64_t RangeStart = QuicRangeGet(&Tracker->PacketNumbersToAck, Index)->Low; - - // - // Check if largest reported packet is missing. In that case, the smallest missing - // packet becomes the largest reported packet. - // if (LargestReported >= RangeStart) { // @@ -153,13 +145,21 @@ QuicAckTrackerDidHitReorderingThreshold( return FALSE; } + // + // Check if largest reported packet is missing. In that case, the smallest missing + // packet becomes the largest reported packet. + // + + uint64_t PreviousSmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; if (LargestReported > PreviousSmallestMissing) { PreviousSmallestMissing = LargestReported; } + if (LargestUnacked - PreviousSmallestMissing >= ReorderingThreshold) { return TRUE; } } + return FALSE; } diff --git a/src/core/frame.c b/src/core/frame.c index 66d94748fb..20b6d91eb5 100644 --- a/src/core/frame.c +++ b/src/core/frame.c @@ -1259,7 +1259,7 @@ QuicAckFrequencyFrameEncode( Buffer = QuicVarIntEncode(Frame->SequenceNumber, Buffer); Buffer = QuicVarIntEncode(Frame->AckElicitingThreshold, Buffer); Buffer = QuicVarIntEncode(Frame->RequestedMaxAckDelay, Buffer); - Buffer = QuicVarIntEncode(Frame->ReorderingThreshold, Buffer); + QuicVarIntEncode(Frame->ReorderingThreshold, Buffer); *Offset += RequiredLength; return TRUE; diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index da024b83ca..e79812800a 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -249,17 +249,20 @@ TEST(FrameTest, ReliableResetStreamFrameEncodeDecode) // calls QuicAckTrackerDidHitReorderingThreshold to determine if the reordering // threshold has been hit. // -// bool TestReorderingThreshold( uint8_t ReorderingThreshold, uint64_t LargestPacketNumberAcknowledged, - const std::vector>& AckTrackerLists) + const std::vector>& AckTrackerRanges) { QUIC_ACK_TRACKER Tracker; QuicAckTrackerInitialize(&Tracker); - for (const auto& List : AckTrackerLists) { - for (int i : List) { - QuicRangeAddValue(&Tracker.PacketNumbersToAck, i); + for (const auto& Range : AckTrackerRanges) { + if (Range.size() == 1) { + QuicRangeAddValue(&Tracker.PacketNumbersToAck, Range[0]); + } else { + for (int packet = Range[0]; packet <= Range[1]; ++packet) { + QuicRangeAddValue(&Tracker.PacketNumbersToAck, packet); + } } } Tracker.LargestPacketNumberAcknowledged = LargestPacketNumberAcknowledged; @@ -277,10 +280,10 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0, 1}})); ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0, 1}, {3}})); ASSERT_FALSE(TestReorderingThreshold(3, 0, {{0, 1}, {3, 4}})); - ASSERT_TRUE(TestReorderingThreshold(3, 0, {{0, 1}, {3, 4, 5}})); - ASSERT_FALSE(TestReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8}})); - ASSERT_TRUE(TestReorderingThreshold(3, 5, {{0, 1}, {3, 4, 5}, {8, 9}})); - ASSERT_TRUE(TestReorderingThreshold(3, 9, {{0, 1}, {3, 4, 5}, {8, 9, 10}})); + ASSERT_TRUE(TestReorderingThreshold(3, 0, {{0, 1}, {3, 5}})); + ASSERT_FALSE(TestReorderingThreshold(3, 5, {{0, 1}, {3, 5}, {8}})); + ASSERT_TRUE(TestReorderingThreshold(3, 5, {{0, 1}, {3, 5}, {8, 9}})); + ASSERT_TRUE(TestReorderingThreshold(3, 9, {{0, 1}, {3, 5}, {8, 10}})); // Case 2 ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0}})); @@ -288,10 +291,11 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}, {3}})); ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5}})); ASSERT_FALSE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6}})); - ASSERT_TRUE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 6, 7}})); - ASSERT_FALSE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8}})); - ASSERT_TRUE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 6, 7, 8, 9}})); + ASSERT_TRUE(TestReorderingThreshold(5, 0, {{0, 1}, {3}, {5, 7}})); + ASSERT_FALSE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 8}})); + ASSERT_TRUE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 9}})); + // Additional cases to test edge conditions ASSERT_TRUE(TestReorderingThreshold(5, 4, {{1, 2}, {4}, {10}})); ASSERT_FALSE(TestReorderingThreshold(5, 0, {{1, 2}, {4}})); ASSERT_FALSE(TestReorderingThreshold(5, 2, {{1, 2}, {4}})); From a8f238695be04e719df0281def61eaea96083c0a Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Wed, 25 Dec 2024 03:18:22 +0530 Subject: [PATCH 29/32] minor change --- src/core/unittest/FrameTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/unittest/FrameTest.cpp b/src/core/unittest/FrameTest.cpp index e79812800a..f3cde7e738 100644 --- a/src/core/unittest/FrameTest.cpp +++ b/src/core/unittest/FrameTest.cpp @@ -296,9 +296,9 @@ TEST(FrameTest, TestQuicAckTrackerDidHitReorderingThreshold) ASSERT_TRUE(TestReorderingThreshold(5, 7, {{0, 1}, {3}, {5, 9}})); // Additional cases to test edge conditions - ASSERT_TRUE(TestReorderingThreshold(5, 4, {{1, 2}, {4}, {10}})); ASSERT_FALSE(TestReorderingThreshold(5, 0, {{1, 2}, {4}})); ASSERT_FALSE(TestReorderingThreshold(5, 2, {{1, 2}, {4}})); + ASSERT_TRUE(TestReorderingThreshold(5, 4, {{1, 2}, {4}, {10}})); } struct ResetStreamFrameParams { From 7a37125c226a388f5d027f18d54a371223e733df Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 3 Jan 2025 14:20:07 +0530 Subject: [PATCH 30/32] minor change to fix the pipeline --- src/core/ack_tracker.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index e4a4287f07..4762fb573a 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -149,7 +149,7 @@ QuicAckTrackerDidHitReorderingThreshold( // Check if largest reported packet is missing. In that case, the smallest missing // packet becomes the largest reported packet. // - + uint64_t PreviousSmallestMissing = QuicRangeGetHigh(QuicRangeGet(&Tracker->PacketNumbersToAck, Index - 1)) + 1; if (LargestReported > PreviousSmallestMissing) { PreviousSmallestMissing = LargestReported; @@ -268,7 +268,7 @@ QuicAckTrackerAckPacket( // QuicSendSetSendFlag(&Connection->Send, QUIC_CONN_SEND_FLAG_ACK); - } else if (Tracker->AckElicitingPacketsToAcknowledge == 1) { + } else if (Tracker->AckElicitingPacketsToAcknowledge > 0) { // // We now have ACK eliciting payload to acknowledge but haven't met the // criteria to send an ACK frame immediately, so just ensure the delayed From d94d218c10395ce6d7fbf37185bb2bdbaa4147a6 Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 3 Jan 2025 16:06:18 +0530 Subject: [PATCH 31/32] minor change --- src/core/ack_tracker.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 4762fb573a..538930546e 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -268,7 +268,7 @@ QuicAckTrackerAckPacket( // QuicSendSetSendFlag(&Connection->Send, QUIC_CONN_SEND_FLAG_ACK); - } else if (Tracker->AckElicitingPacketsToAcknowledge > 0) { + } else if (Tracker->AckElicitingPacketsToAcknowledge == 1) { // // We now have ACK eliciting payload to acknowledge but haven't met the // criteria to send an ACK frame immediately, so just ensure the delayed From 140e0522a2ad42ec78d8eeba7414cb22af2a924b Mon Sep 17 00:00:00 2001 From: Gaurav Singh Date: Fri, 3 Jan 2025 17:59:20 +0530 Subject: [PATCH 32/32] New Largest Packet Number check shifted --- src/core/ack_tracker.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c index 538930546e..5ba46d3810 100644 --- a/src/core/ack_tracker.c +++ b/src/core/ack_tracker.c @@ -239,7 +239,7 @@ QuicAckTrackerAckPacket( Tracker->AckElicitingPacketsToAcknowledge++; - if ((Connection->Send.SendFlags & QUIC_CONN_SEND_FLAG_ACK) || !NewLargestPacketNumber) { + if (Connection->Send.SendFlags & QUIC_CONN_SEND_FLAG_ACK) { goto Exit; // Already queued to send an ACK, no more work to do. } @@ -262,7 +262,8 @@ QuicAckTrackerAckPacket( if (AckType == QUIC_ACK_TYPE_ACK_IMMEDIATE || Connection->Settings.MaxAckDelayMs == 0 || (Tracker->AckElicitingPacketsToAcknowledge >= (uint16_t)Connection->PacketTolerance) || - QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold)) { + (NewLargestPacketNumber && + QuicAckTrackerDidHitReorderingThreshold(Tracker, Connection->ReorderingThreshold))) { // // Send the ACK immediately. //