diff --git a/src/core/connection.c b/src/core/connection.c index bf57403b5f..dbb73b18d4 100644 --- a/src/core/connection.c +++ b/src/core/connection.c @@ -1047,6 +1047,10 @@ QuicConnRetireCid( _In_ QUIC_CID_LIST_ENTRY* DestCid ) { + if (DestCid->CID.Retired) { + return; + } + QuicTraceEvent( ConnDestCidRemoved, "[conn][%p] (SeqNum=%llu) Removed Destination CID: %!CID!", @@ -1076,6 +1080,10 @@ QuicConnRetireCurrentDestCid( _In_ QUIC_PATH* Path ) { + if (Path->DestCid == NULL) { + return TRUE; + } + if (Path->DestCid->CID.Length == 0) { QuicTraceLogConnVerbose( ZeroLengthCidRetire, @@ -1096,7 +1104,7 @@ QuicConnRetireCurrentDestCid( CXPLAT_DBG_ASSERT(Path->DestCid != NewDestCid); QUIC_CID_LIST_ENTRY* OldDestCid = Path->DestCid; QUIC_CID_CLEAR_PATH(Path->DestCid); - QuicConnRetireCid(Connection, Path->DestCid); + QuicConnRetireCid(Connection, OldDestCid); Path->DestCid = NewDestCid; QUIC_CID_SET_PATH(Connection, Path->DestCid, Path); QUIC_CID_VALIDATE_NULL(Connection, OldDestCid); @@ -1112,7 +1120,7 @@ QuicConnOnRetirePriorToUpdated( _In_ QUIC_CONNECTION* Connection ) { - BOOLEAN ReplaceRetiredCids = FALSE; + BOOLEAN RetiredUsedCid = FALSE; for (CXPLAT_LIST_ENTRY* Entry = Connection->DestCids.Flink; Entry != &Connection->DestCids; @@ -1127,78 +1135,11 @@ QuicConnOnRetirePriorToUpdated( continue; } - if (DestCid->CID.UsedLocally) { - ReplaceRetiredCids = TRUE; - } - - QUIC_CID_CLEAR_PATH(DestCid); + RetiredUsedCid |= DestCid->CID.UsedLocally; QuicConnRetireCid(Connection, DestCid); } - return ReplaceRetiredCids; -} - -_IRQL_requires_max_(PASSIVE_LEVEL) -BOOLEAN -QuicConnReplaceRetiredCids( - _In_ QUIC_CONNECTION* Connection - ) -{ - QUIC_PATH_SET* PathSet = &Connection->Paths; - CXPLAT_DBG_ASSERT(PathSet->Count <= QUIC_MAX_PATH_COUNT); - for (uint8_t i = 0; i < PathSet->Count; ++i) { - QUIC_PATH* Path = &PathSet->Paths[i]; - if (Path->DestCid == NULL || !Path->DestCid->CID.Retired) { - continue; - } - - QUIC_CID_VALIDATE_NULL(Connection, Path->DestCid); // Previously cleared on retire. - QUIC_CID_LIST_ENTRY* NewDestCid = QuicConnGetUnusedDestCid(Connection); - if (NewDestCid == NULL) { - if (Path->IsActive) { - QuicTraceEvent( - ConnError, - "[conn][%p] ERROR, %s.", - Connection, - "Active path has no replacement for retired CID"); - QuicConnSilentlyAbort(Connection); // Must silently abort because we can't send anything now. - return FALSE; - } - QuicTraceLogConnWarning( - NonActivePathCidRetired, - Connection, - "Non-active path has no replacement for retired CID."); - // - // A path pending deferred activation is still considered non-active here and - // may be removed. CID replacement will be deferred in the next stack layer. - // - CXPLAT_DBG_ASSERT(i != 0); - QuicPathRemove(Connection, i--); - continue; - } - - CXPLAT_DBG_ASSERT(NewDestCid != Path->DestCid); - Path->DestCid = NewDestCid; - QUIC_CID_SET_PATH(Connection, NewDestCid, Path); - Path->DestCid->CID.UsedLocally = TRUE; - Path->InitiatedCidUpdate = TRUE; - QuicPathValidate(Path); - } - -#if DEBUG - for (CXPLAT_LIST_ENTRY* Entry = Connection->DestCids.Flink; - Entry != &Connection->DestCids; - Entry = Entry->Flink) { - QUIC_CID_LIST_ENTRY* DestCid = - CXPLAT_CONTAINING_RECORD( - Entry, - QUIC_CID_LIST_ENTRY, - Link); - CXPLAT_DBG_ASSERT(!DestCid->CID.Retired || DestCid->AssignedPath == NULL); - } -#endif - - return TRUE; + return RetiredUsedCid; } _IRQL_requires_max_(DISPATCH_LEVEL) @@ -5058,10 +4999,10 @@ QuicConnRecvFrames( break; // Ignore frame if we are closed. } - BOOLEAN ReplaceRetiredCids = FALSE; + BOOLEAN RetiredUsedCid = FALSE; if (Connection->RetirePriorTo < Frame.RetirePriorTo) { Connection->RetirePriorTo = Frame.RetirePriorTo; - ReplaceRetiredCids = QuicConnOnRetirePriorToUpdated(Connection); + RetiredUsedCid = QuicConnOnRetirePriorToUpdated(Connection); } if (QuicConnGetDestCidFromSeq(Connection, Frame.Sequence, FALSE) == NULL) { @@ -5076,7 +5017,7 @@ QuicConnRecvFrames( "Allocation of '%s' failed. (%llu bytes)", "new DestCid", sizeof(QUIC_CID_LIST_ENTRY) + Frame.Length); - if (ReplaceRetiredCids) { + if (RetiredUsedCid) { QuicConnSilentlyAbort(Connection); } else { QuicConnFatalError(Connection, QUIC_STATUS_OUT_OF_MEMORY, NULL); @@ -5109,7 +5050,7 @@ QuicConnRecvFrames( "[conn][%p] ERROR, %s.", Connection, "Peer exceeded CID limit"); - if (ReplaceRetiredCids) { + if (RetiredUsedCid) { QuicConnSilentlyAbort(Connection); } else { QuicConnTransportError(Connection, QUIC_ERROR_PROTOCOL_VIOLATION); @@ -5118,10 +5059,6 @@ QuicConnRecvFrames( } } - if (ReplaceRetiredCids && !QuicConnReplaceRetiredCids(Connection)) { - return FALSE; - } - AckEliciting = TRUE; break; } @@ -5527,31 +5464,6 @@ QuicConnRecvPostProcessing( // sent back out. // - if (CurrentPath->DestCid == NULL || - (PeerUpdatedCid && CurrentPath->DestCid->CID.Length != 0)) { - // - // TODO - What if the peer (client) only sends a single CID and - // rebinding happens? Should we support using the same CID over? - // - QUIC_CID_LIST_ENTRY* NewDestCid = QuicConnGetUnusedDestCid(Connection); - if (NewDestCid == NULL) { - QuicTraceEvent( - ConnError, - "[conn][%p] ERROR, %s.", - Connection, - "No unused CID for new path"); - CurrentPath->GotValidPacket = FALSE; // Don't have a new CID to use!!! - CurrentPath->DestCid = NULL; - return; - } - CXPLAT_DBG_ASSERT(NewDestCid != CurrentPath->DestCid); - CurrentPath->DestCid = NewDestCid; - QUIC_CID_SET_PATH(Connection, CurrentPath->DestCid, CurrentPath); - CurrentPath->DestCid->CID.UsedLocally = TRUE; - } - - CXPLAT_DBG_ASSERT(CurrentPath->DestCid != NULL); - QuicPathValidate(CurrentPath); CurrentPath->SendChallenge = TRUE; CurrentPath->PathValidationStartTime = CxPlatTimeUs64(); @@ -5990,6 +5902,8 @@ QuicConnRecvDatagrams( // QuicPathUpdateActive(Connection); + QuicPathUpdateDestCids(PathSet, Connection); + if (!Connection->State.UpdateWorker && Connection->State.Connected && !Connection->State.ShutdownComplete && RecvState.UpdatePartitionId) { // diff --git a/src/core/connection.h b/src/core/connection.h index 3ce4668125..66728ef786 100644 --- a/src/core/connection.h +++ b/src/core/connection.h @@ -1260,6 +1260,15 @@ QuicConnGenerateNewSourceCids( _In_ BOOLEAN ReplaceExistingCids ); +// +// Get an unused destination CID from the list provided by the peer. +// +_IRQL_requires_max_(PASSIVE_LEVEL) +QUIC_CID_LIST_ENTRY* +QuicConnGetUnusedDestCid( + _In_ const QUIC_CONNECTION* Connection + ); + // // Retires the currently used destination connection ID. // diff --git a/src/core/path.c b/src/core/path.c index 6411af6bb9..eb4e21fe1e 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -76,6 +76,36 @@ QuicPathSetActive( _In_ uint32_t PathId ); +_IRQL_requires_max_(PASSIVE_LEVEL) +static +BOOLEAN +QuicPathUpdateDestCid( + _In_ QUIC_CONNECTION* Connection, + _Inout_ QUIC_PATH* Path + ) +{ + if (Path->DestCid != NULL && !Path->DestCid->CID.Retired) { + return TRUE; + } + + QUIC_CID_LIST_ENTRY* NewDestCid = QuicConnGetUnusedDestCid(Connection); + if (NewDestCid == NULL) { + return FALSE; + } + + if (Path->DestCid != NULL) { + QUIC_CID_CLEAR_PATH(Path->DestCid); + Path->DestCid = NULL; + } + + Path->DestCid = NewDestCid; + QUIC_CID_SET_PATH(Connection, NewDestCid, Path); + Path->DestCid->CID.UsedLocally = TRUE; + Path->InitiatedCidUpdate = TRUE; + QuicPathValidate(Path); + return TRUE; +} + _IRQL_requires_max_(PASSIVE_LEVEL) void QuicPathUpdateActive( @@ -90,6 +120,18 @@ QuicPathUpdateActive( return; } + // + // A path needs a usable destination CID before it can become active. + // + uint8_t NextActivePathIndex; + QUIC_PATH* NextActivePath = + QuicConnGetPathByID(Connection, PathSet->NextActivePathId, &NextActivePathIndex); + CXPLAT_DBG_ASSERT(NextActivePath != NULL); + if (!QuicPathUpdateDestCid(Connection, NextActivePath)) { + PathSet->NextActivePathId = QuicPathGetActive(PathSet)->ID; + return; + } + QuicPathSetActive(Connection, PathSet->NextActivePathId); QUIC_PATH* ActivePath = QuicPathGetActive(PathSet); @@ -206,6 +248,55 @@ QuicPathRemove( return TRUE; } +_IRQL_requires_max_(PASSIVE_LEVEL) +void +QuicPathUpdateDestCids( + _In_ QUIC_PATH_SET* PathSet, + _In_ QUIC_CONNECTION* Connection + ) +{ + for (uint8_t i = 0; i < PathSet->Count; ++i) { + QUIC_PATH* Path = &PathSet->Paths[i]; + if (QuicPathUpdateDestCid(Connection, Path)) { + continue; + } + + if (Path->IsActive) { + QuicTraceEvent( + ConnError, + "[conn][%p] ERROR, %s.", + Connection, + "Active path has no replacement for retired CID"); + QuicConnSilentlyAbort(Connection); + return; + } + + QuicTraceLogConnWarning( + NonActivePathCidRetired, + Connection, + "Non-active path has no replacement for retired CID."); + CXPLAT_DBG_ASSERT(i != 0); + QuicPathRemove(Connection, i); + // + // Reprocess this index because removal shifted the remaining paths down. + // + --i; + } + +#if DEBUG + for (CXPLAT_LIST_ENTRY* Entry = Connection->DestCids.Flink; + Entry != &Connection->DestCids; + Entry = Entry->Flink) { + QUIC_CID_LIST_ENTRY* DestCid = + CXPLAT_CONTAINING_RECORD( + Entry, + QUIC_CID_LIST_ENTRY, + Link); + CXPLAT_DBG_ASSERT(!DestCid->CID.Retired || DestCid->AssignedPath == NULL); + } +#endif +} + _IRQL_requires_max_(PASSIVE_LEVEL) void QuicPathSetAllowance( @@ -446,8 +537,8 @@ QuicPathSetActive( QuicCongestionControlReset(&Connection->CongestionControl, FALSE); } Connection->Paths.NextActivePathId = ActivePath->ID; - CXPLAT_DBG_ASSERT(Path->DestCid != NULL); - CXPLAT_DBG_ASSERT(!Path->DestCid->CID.Retired); + CXPLAT_DBG_ASSERT(ActivePath->DestCid != NULL); + CXPLAT_DBG_ASSERT(!ActivePath->DestCid->CID.Retired); } _IRQL_requires_max_(PASSIVE_LEVEL) diff --git a/src/core/path.h b/src/core/path.h index 2cf1e4b92c..97139223fa 100644 --- a/src/core/path.h +++ b/src/core/path.h @@ -268,6 +268,13 @@ QuicPathRemove( _In_ uint8_t Index ); +_IRQL_requires_max_(PASSIVE_LEVEL) +void +QuicPathUpdateDestCids( + _In_ QUIC_PATH_SET* PathSet, + _In_ QUIC_CONNECTION* Connection + ); + _IRQL_requires_max_(PASSIVE_LEVEL) void QuicPathSetAllowance( diff --git a/src/generated/linux/connection.c.clog.h b/src/generated/linux/connection.c.clog.h index feca261a1c..e5acdf5917 100644 --- a/src/generated/linux/connection.c.clog.h +++ b/src/generated/linux/connection.c.clog.h @@ -375,24 +375,6 @@ tracepoint(CLOG_CONNECTION_C, NoReplacementCidForRetire , arg1);\ -/*---------------------------------------------------------- -// Decoder Ring for NonActivePathCidRetired -// [conn][%p] Non-active path has no replacement for retired CID. -// QuicTraceLogConnWarning( - NonActivePathCidRetired, - Connection, - "Non-active path has no replacement for retired CID."); -// arg1 = arg1 = Connection = arg1 -----------------------------------------------------------*/ -#ifndef _clog_3_ARGS_TRACE_NonActivePathCidRetired -#define _clog_3_ARGS_TRACE_NonActivePathCidRetired(uniqueId, arg1, encoded_arg_string)\ -tracepoint(CLOG_CONNECTION_C, NonActivePathCidRetired , arg1);\ - -#endif - - - - /*---------------------------------------------------------- // Decoder Ring for IgnoreUnreachable // [conn][%p] Ignoring received unreachable event (inline) diff --git a/src/generated/linux/connection.c.clog.h.lttng.h b/src/generated/linux/connection.c.clog.h.lttng.h index 2ec9abb6a3..f12d233843 100644 --- a/src/generated/linux/connection.c.clog.h.lttng.h +++ b/src/generated/linux/connection.c.clog.h.lttng.h @@ -381,25 +381,6 @@ TRACEPOINT_EVENT(CLOG_CONNECTION_C, NoReplacementCidForRetire, -/*---------------------------------------------------------- -// Decoder Ring for NonActivePathCidRetired -// [conn][%p] Non-active path has no replacement for retired CID. -// QuicTraceLogConnWarning( - NonActivePathCidRetired, - Connection, - "Non-active path has no replacement for retired CID."); -// arg1 = arg1 = Connection = arg1 -----------------------------------------------------------*/ -TRACEPOINT_EVENT(CLOG_CONNECTION_C, NonActivePathCidRetired, - TP_ARGS( - const void *, arg1), - TP_FIELDS( - ctf_integer_hex(uint64_t, arg1, (uint64_t)arg1) - ) -) - - - /*---------------------------------------------------------- // Decoder Ring for IgnoreUnreachable // [conn][%p] Ignoring received unreachable event (inline) diff --git a/src/generated/linux/path.c.clog.h b/src/generated/linux/path.c.clog.h index 92057814a5..36e0c3966e 100644 --- a/src/generated/linux/path.c.clog.h +++ b/src/generated/linux/path.c.clog.h @@ -14,6 +14,10 @@ #include "path.c.clog.h.lttng.h" #endif #include +#ifndef _clog_MACRO_QuicTraceLogConnWarning +#define _clog_MACRO_QuicTraceLogConnWarning 1 +#define QuicTraceLogConnWarning(a, ...) _clog_CAT(_clog_ARGN_SELECTOR(__VA_ARGS__), _clog_CAT(_,a(#a, __VA_ARGS__))) +#endif #ifndef _clog_MACRO_QuicTraceLogConnInfo #define _clog_MACRO_QuicTraceLogConnInfo 1 #define QuicTraceLogConnInfo(a, ...) _clog_CAT(_clog_ARGN_SELECTOR(__VA_ARGS__), _clog_CAT(_,a(#a, __VA_ARGS__))) @@ -29,6 +33,24 @@ #ifdef __cplusplus extern "C" { #endif +/*---------------------------------------------------------- +// Decoder Ring for NonActivePathCidRetired +// [conn][%p] Non-active path has no replacement for retired CID. +// QuicTraceLogConnWarning( + NonActivePathCidRetired, + Connection, + "Non-active path has no replacement for retired CID."); +// arg1 = arg1 = Connection = arg1 +----------------------------------------------------------*/ +#ifndef _clog_3_ARGS_TRACE_NonActivePathCidRetired +#define _clog_3_ARGS_TRACE_NonActivePathCidRetired(uniqueId, arg1, encoded_arg_string)\ +tracepoint(CLOG_PATH_C, NonActivePathCidRetired , arg1);\ + +#endif + + + + /*---------------------------------------------------------- // Decoder Ring for PathActiveFallback // [conn][%p] Path[%u] removed; falling back to Path[%u] @@ -173,6 +195,26 @@ tracepoint(CLOG_PATH_C, ConnPathRemoved , arg2, arg3);\ +/*---------------------------------------------------------- +// Decoder Ring for ConnError +// [conn][%p] ERROR, %s. +// QuicTraceEvent( + ConnError, + "[conn][%p] ERROR, %s.", + Connection, + "Active path has no replacement for retired CID"); +// arg2 = arg2 = Connection = arg2 +// arg3 = arg3 = "Active path has no replacement for retired CID" = arg3 +----------------------------------------------------------*/ +#ifndef _clog_4_ARGS_TRACE_ConnError +#define _clog_4_ARGS_TRACE_ConnError(uniqueId, encoded_arg_string, arg2, arg3)\ +tracepoint(CLOG_PATH_C, ConnError , arg2, arg3);\ + +#endif + + + + /*---------------------------------------------------------- // Decoder Ring for ConnPathValidated // [conn][%p] Path[%u] Validated (%hhu) diff --git a/src/generated/linux/path.c.clog.h.lttng.h b/src/generated/linux/path.c.clog.h.lttng.h index 761590dbcd..5ea63225d7 100644 --- a/src/generated/linux/path.c.clog.h.lttng.h +++ b/src/generated/linux/path.c.clog.h.lttng.h @@ -1,6 +1,25 @@ +/*---------------------------------------------------------- +// Decoder Ring for NonActivePathCidRetired +// [conn][%p] Non-active path has no replacement for retired CID. +// QuicTraceLogConnWarning( + NonActivePathCidRetired, + Connection, + "Non-active path has no replacement for retired CID."); +// arg1 = arg1 = Connection = arg1 +----------------------------------------------------------*/ +TRACEPOINT_EVENT(CLOG_PATH_C, NonActivePathCidRetired, + TP_ARGS( + const void *, arg1), + TP_FIELDS( + ctf_integer_hex(uint64_t, arg1, (uint64_t)arg1) + ) +) + + + /*---------------------------------------------------------- // Decoder Ring for PathActiveFallback // [conn][%p] Path[%u] removed; falling back to Path[%u] @@ -168,6 +187,29 @@ TRACEPOINT_EVENT(CLOG_PATH_C, ConnPathRemoved, +/*---------------------------------------------------------- +// Decoder Ring for ConnError +// [conn][%p] ERROR, %s. +// QuicTraceEvent( + ConnError, + "[conn][%p] ERROR, %s.", + Connection, + "Active path has no replacement for retired CID"); +// arg2 = arg2 = Connection = arg2 +// arg3 = arg3 = "Active path has no replacement for retired CID" = arg3 +----------------------------------------------------------*/ +TRACEPOINT_EVENT(CLOG_PATH_C, ConnError, + TP_ARGS( + const void *, arg2, + const char *, arg3), + TP_FIELDS( + ctf_integer_hex(uint64_t, arg2, (uint64_t)arg2) + ctf_string(arg3, arg3) + ) +) + + + /*---------------------------------------------------------- // Decoder Ring for ConnPathValidated // [conn][%p] Path[%u] Validated (%hhu)