Refactor multipath handling to use negotiated state

microsoft · Dec 31, 2024 · 6103189 · 6103189
1 parent bf1a2f7
commit 6103189
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 31 deletions.
diff --git a/src/core/ack_tracker.c b/src/core/ack_tracker.c
@@ -248,7 +248,7 @@ QuicAckTrackerAckFrameEncode(
     }
 
     if (!QuicAckFrameEncode(
-            QuicConnIsMultipathEnabled(PacketSpace->Connection) &&
+            PacketSpace->Connection->State.MultipathNegotiated &&
                 Builder->EncryptLevel == QUIC_ENCRYPT_LEVEL_1_RTT,
             PacketSpace->PathID->ID,
             &Tracker->PacketNumbersToAck,

diff --git a/src/core/connection.c b/src/core/connection.c
@@ -2629,6 +2629,11 @@ QuicConnProcessPeerTransportParameters(
             QuicConnIndicateEvent(Connection, &Event);
         }
 
+        if (Connection->Settings.MultipathEnabled) {
+            Connection->State.MultipathNegotiated =
+                !!(Connection->PeerTransportParams.Flags & QUIC_TP_FLAG_INITIAL_MAX_PATH_ID);
+        }
+
         //
         // Fully validate all exchanged connection IDs.
         //
@@ -4580,7 +4585,7 @@ QuicConnRecvFrames(
                 return FALSE;
             }
 
-            if (QuicConnIsMultipathEnabled(Connection)) {
+            if (Connection->State.MultipathNegotiated) {
                 QuicConnAssignPathIDs(Connection);
             }
 
@@ -4704,7 +4709,7 @@ QuicConnRecvFrames(
                     !memcmp(Frame.Data, TempPath->Challenge, sizeof(Frame.Data))) {
                     QuicPerfCounterIncrement(QUIC_PERF_COUNTER_PATH_VALIDATED);
                     QuicPathSetValid(Connection, TempPath, QUIC_PATH_VALID_PATH_RESPONSE);
-                    if (QuicConnIsMultipathEnabled(Connection)) {
+                    if (Connection->State.MultipathNegotiated) {
                         QuicPathSetActive(Connection, TempPath);
                     }
                     break;
@@ -4716,7 +4721,7 @@ QuicConnRecvFrames(
         }
 
         case QUIC_FRAME_PATH_ABANDON: {
-            if (!QuicConnIsMultipathEnabled(Connection)) {
+            if (!Connection->State.MultipathNegotiated) {
                 QuicTraceEvent(
                     ConnError,
                     "[conn][%p] ERROR, %s.",
@@ -5171,7 +5176,7 @@ QuicConnRecvPostProcessing(
     }
 
     if (QuicConnIsServer(Connection) &&
-        !QuicConnIsMultipathEnabled(Connection) &&
+        !Connection->State.MultipathNegotiated &&
         Packet->HasNonProbingFrame &&
         Packet->NewLargestPacketNumber &&
         !(*Path)->IsActive) {
@@ -5974,7 +5979,7 @@ QuicConnOpenNewPath(
         CxPlatCopyMemory(&Path->Route.RemoteAddress,
             &Connection->Paths[0].Route.RemoteAddress,
             sizeof(QUIC_ADDR));
-        if (QuicConnIsMultipathEnabled(Connection)) {
+        if (Connection->State.MultipathNegotiated) {
             Path->PathID = QuicPathIDSetGetUnusedPathID(&Connection->PathIDs);
             if (Path->PathID != NULL) {
                 QuicPathIDAddRef(Path->PathID, QUIC_PATHID_REF_PATH);
@@ -6189,7 +6194,7 @@ QuicConnRemoveLocalAddress(
 
     QUIC_PATH* Path = &Connection->Paths[PathIndex];
 
-    if (!QuicConnIsMultipathEnabled(Connection)) {
+    if (!Connection->State.MultipathNegotiated) {
         if (Path->IsActive && Connection->State.Started) {
             return QUIC_STATUS_INVALID_STATE;
         }
@@ -7497,6 +7502,11 @@ QuicConnApplyNewSettings(
             QuicConnIndicateEvent(Connection, &Event);
         }
 
+        if (QuicConnIsServer(Connection) && Connection->Settings.MultipathEnabled) {
+            Connection->State.MultipathNegotiated =
+                !!(Connection->PeerTransportParams.Flags & QUIC_TP_FLAG_INITIAL_MAX_PATH_ID);
+        }
+
         if (Connection->Settings.EcnEnabled) {
             QUIC_PATH* Path = &Connection->Paths[0];
             Path->EcnValidationState = ECN_VALIDATION_TESTING;

diff --git a/src/core/connection.h b/src/core/connection.h
@@ -194,6 +194,11 @@ typedef union QUIC_CONNECTION_STATE {
         //
         BOOLEAN TimestampRecvNegotiated : 1;
 
+        //
+        // Multipath extension has been negotiated.
+        //
+        BOOLEAN MultipathNegotiated : 1;
+
         //
         // Indicates we received APPLICATION_ERROR transport error and are checking also
         // later packets in case they contain CONNECTION_CLOSE frame with application-layer error.
@@ -690,18 +695,6 @@ QuicConnIsClosed(
     return Connection->State.ClosedLocally || Connection->State.ClosedRemotely;
 }
 
-//
-// Helper to determine if a connection is multipath enabled
-//
-inline
-BOOLEAN
-QuicConnIsMultipathEnabled(
-    _In_ const QUIC_CONNECTION * const Connection
-    )
-{
-    return Connection->Settings.MultipathEnabled && Connection->PathIDs.Flags.InitialMaxPathRecvd;
-}
-
 //
 // Helper to get the owning QUIC_CONNECTION for the stream set module.
 //

diff --git a/src/core/frame.h b/src/core/frame.h
@@ -183,7 +183,7 @@ CXPLAT_STATIC_ASSERT(
       X == QUIC_FRAME_ACK_FREQUENCY || X == QUIC_FRAME_IMMEDIATE_ACK || \
       X == QUIC_FRAME_RELIABLE_RESET_STREAM || \
       X == QUIC_FRAME_TIMESTAMP || \
-      X == QUIC_FRAME_PATH_ACK || \
+     (X >= QUIC_FRAME_PATH_ACK && X <= QUIC_FRAME_PATH_ACK_1) || \
       X == QUIC_FRAME_PATH_ABANDON || \
       X == QUIC_FRAME_PATH_NEW_CONNECTION_ID || \
       X == QUIC_FRAME_PATH_RETIRE_CONNECTION_ID || \

diff --git a/src/core/inline.c b/src/core/inline.c
@@ -100,11 +100,6 @@ QuicConnIsClient(
     _In_ const QUIC_CONNECTION * const Connection
     );
 
-BOOLEAN
-QuicConnIsMultipathEnabled(
-    _In_ const QUIC_CONNECTION * const Connection
-    );
-
 _IRQL_requires_max_(PASSIVE_LEVEL)
 void
 QuicConnTransportError(

diff --git a/src/core/path.c b/src/core/path.c
@@ -311,7 +311,7 @@ QuicConnChoosePath(
 {
     QUIC_PATH* Path = &Connection->Paths[0];
 
-    if (QuicConnIsMultipathEnabled(Connection) && Connection->State.HandshakeConfirmed) {
+    if (Connection->State.MultipathNegotiated && Connection->State.HandshakeConfirmed) {
         QUIC_PATH* ActivePaths[QUIC_MAX_PATH_COUNT];
         uint8_t ActivePathCount = 0;
         for (uint8_t i = 0; i < Connection->PathsCount; ++i) {
@@ -346,7 +346,7 @@ QuicPathSetActive(
 {
     BOOLEAN UdpPortChangeOnly = FALSE;
 
-    if (QuicConnIsMultipathEnabled(Connection)) {
+    if (Connection->State.MultipathNegotiated) {
         Path->IsActive = TRUE;
     } else if (Path == &Connection->Paths[0]) {
         CXPLAT_DBG_ASSERT(!Path->IsActive);

diff --git a/src/core/pathid.c b/src/core/pathid.c
@@ -759,7 +759,7 @@ QuicPathIDWriteNewConnectionIDFrame(
     )
 {
     QUIC_FRAME_TYPE FrameType =
-        QuicConnIsMultipathEnabled(PathID->Connection) ?
+        PathID->Connection->State.MultipathNegotiated ?
             QUIC_FRAME_PATH_NEW_CONNECTION_ID : QUIC_FRAME_NEW_CONNECTION_ID;
     for (CXPLAT_SLIST_ENTRY* Entry = PathID->SourceCids.Next;
             Entry != NULL;
@@ -834,7 +834,7 @@ QuicPathIDWriteRetireConnectionIDFrame(
     )
 {
     QUIC_FRAME_TYPE FrameType =
-        QuicConnIsMultipathEnabled(PathID->Connection) ?
+        PathID->Connection->State.MultipathNegotiated ?
             QUIC_FRAME_PATH_RETIRE_CONNECTION_ID : QUIC_FRAME_RETIRE_CONNECTION_ID;
     for (CXPLAT_LIST_ENTRY* Entry = PathID->DestCids.Flink;
             Entry != &PathID->DestCids;

diff --git a/src/core/pathid_set.c b/src/core/pathid_set.c
@@ -320,7 +320,7 @@ QuicPathIDSetGenerateNewSourceCids(
     _In_ BOOLEAN ReplaceExistingCids
     )
 {
-    if (QuicConnIsMultipathEnabled(QuicPathIDSetGetConnection(PathIDSet))) {
+    if (QuicPathIDSetGetConnection(PathIDSet)->State.MultipathNegotiated) {
         uint16_t NewPathIDCount = 0;
         if (PathIDSet->CurrentPathIDCount < PathIDSet->MaxCurrentPathIDCount) {
             NewPathIDCount = PathIDSet->MaxCurrentPathIDCount - PathIDSet->CurrentPathIDCount;
@@ -596,7 +596,7 @@ QuicPathIDSetNewLocalPathID(
 
     if (NewPathIDBlocked) {
         if (Connection->State.PeerTransportParameterValid) {
-            QuicSendSetSendFlag(&Connection->Send, QUIC_CONN_SEND_FLAG_PATHS_BLOCKED);
+            // QuicSendSetSendFlag(&Connection->Send, QUIC_CONN_SEND_FLAG_PATHS_BLOCKED);
         }
         Status = QUIC_STATUS_PATHID_LIMIT_REACHED;
         goto Exit;