Clear out rebound paths upon path exhaustion

CMakeLists.txt

@@ -128,7 +128,7 @@ option(QUIC_TLS_SECRETS_SUPPORT "Enable export of TLS secrets" OFF)
 option(QUIC_TELEMETRY_ASSERTS "Enable telemetry asserts in release builds" OFF)
 option(QUIC_USE_SYSTEM_LIBCRYPTO "Use system libcrypto if openssl TLS" OFF)
 option(QUIC_DISABLE_POSIX_GSO "Disable GSO for systems that say they support it but don't" OFF)
-option(QUIC_FOLDER_PREFIX "Optional prefix for source group folders when using an IDE generator" "")
+set(QUIC_FOLDER_PREFIX "" CACHE STRING "Optional prefix for source group folders when using an IDE generator")
 
 set(BUILD_SHARED_LIBS ${QUIC_BUILD_SHARED})
 
@@ -628,7 +628,7 @@ if(QUIC_BUILD_TEST)
     include(FetchContent)
 
     enable_testing()
-    
+
     # Build the googletest framework.
 
     # Enforce static builds for test artifacts

src/core/path.c

@@ -56,7 +56,6 @@ QuicPathRemove(
 
 #if DEBUG
     if (Path->DestCid) {
-        CXPLAT_DBG_ASSERT(Path->DestCid->AssignedPath != NULL);
         QUIC_CID_CLEAR_PATH(Path->DestCid);
     }
 #endif
@@ -196,9 +195,27 @@ QuicConnGetPathForDatagram(
 
     if (Connection->PathsCount == QUIC_MAX_PATH_COUNT) {
         //
-        // Already tracking the maximum number of paths.
+        // See if any old paths share the same remote address, and is just a rebind.
+        // If so, remove the old paths.
+        // NB: Traversing the array backwards is simpler and more efficient here due
+        // to the array shifting that happens in QuicPathRemove.
         //
-        return NULL;
+        for (uint8_t i = Connection->PathsCount - 1; i > 0; i--) {
+            if (!Connection->Paths[i].IsActive
+                && QuicAddrGetFamily(&Datagram->Tuple->RemoteAddress) == QuicAddrGetFamily(&Connection->Paths[i].RemoteAddress)
+                && QuicAddrCompareIp(&Datagram->Tuple->RemoteAddress, &Connection->Paths[i].RemoteAddress)
+                && QuicAddrCompare(&Datagram->Tuple->LocalAddress, &Connection->Paths[i].LocalAddress)) {
+                QuicPathRemove(Connection, i);
+            }
+        }
+
+        if (Connection->PathsCount == QUIC_MAX_PATH_COUNT) {
+            //
+            // Already tracking the maximum number of paths, and can't free
+            // any more.
+            //
+            return NULL;
+        }
     }
 
     if (Connection->PathsCount > 1) {

src/inc/msquic.hpp

@@ -379,6 +379,7 @@ class MsQuicSettings : public QUIC_SETTINGS {
     MsQuicSettings& SetMinimumMtu(uint16_t Mtu) { MinimumMtu = Mtu; IsSet.MinimumMtu = TRUE; return *this; }
     MsQuicSettings& SetMtuDiscoverySearchCompleteTimeoutUs(uint64_t Time) { MtuDiscoverySearchCompleteTimeoutUs = Time; IsSet.MtuDiscoverySearchCompleteTimeoutUs = TRUE; return *this; }
     MsQuicSettings& SetMtuDiscoveryMissingProbeCount(uint8_t Count) { MtuDiscoveryMissingProbeCount = Count; IsSet.MtuDiscoveryMissingProbeCount = TRUE; return *this; }
+    MsQuicSettings& SetKeepAlive(uint32_t Time) { KeepAliveIntervalMs = Time; IsSet.KeepAliveIntervalMs = TRUE; return *this; }
 
     QUIC_STATUS
     SetGlobal() const noexcept {
@@ -779,6 +780,17 @@ struct MsQuicConnection {
                 &Addr.SockAddr);
     }
 
+    QUIC_STATUS
+    GetRemoteAddr(_Out_ QuicAddr& Addr) {
+        uint32_t Size = sizeof(Addr.SockAddr);
+        return
+            GetParam(
+                QUIC_PARAM_LEVEL_CONNECTION,
+                QUIC_PARAM_CONN_REMOTE_ADDRESS,
+                &Size,
+                &Addr.SockAddr);
+    }
+
     QUIC_STATUS
     SetLocalAddr(_In_ const QuicAddr& Addr) noexcept {
         return

src/test/MsQuicTests.h

@@ -72,6 +72,14 @@ QuicTestMtuDiscovery(
     _In_ BOOLEAN RaiseMinimumMtu
     );
 
+//
+// Path tests
+//
+void
+QuicTestLocalPathChanges(
+    _In_ int Family
+    );
+
 //
 // Handshake Tests
 //
@@ -868,4 +876,8 @@ typedef struct {
 #define IOCTL_QUIC_RUN_VALIDATE_PARAM_API \
     QUIC_CTL_CODE(71, METHOD_BUFFERED, FILE_WRITE_DATA)
 
-#define QUIC_MAX_IOCTL_FUNC_CODE 71
+#define IOCTL_QUIC_RUN_CLIENT_LOCAL_PATH_CHANGES \
+    QUIC_CTL_CODE(72, METHOD_BUFFERED, FILE_WRITE_DATA)
+    // int - Family
+
+#define QUIC_MAX_IOCTL_FUNC_CODE 72

src/test/bin/quic_gtest.cpp

@@ -338,6 +338,15 @@ TEST(Basic, CreateConnection) {
 
 #ifdef QUIC_TEST_DATAPATH_HOOKS_ENABLED
 
+TEST_P(WithFamilyArgs, LocalPathChanges) {
+    TestLoggerT<ParamType> Logger("QuicTestLocalPathChanges", GetParam());
+    if (TestingKernelMode) {
+        ASSERT_TRUE(DriverClient.Run(IOCTL_QUIC_RUN_CLIENT_LOCAL_PATH_CHANGES, GetParam().Family));
+    } else {
+        QuicTestLocalPathChanges(GetParam().Family);
+    }
+}
+
 TEST(Mtu, Settings) {
     TestLogger Logger("QuicTestMtuSettings");
     if (TestingKernelMode) {

src/test/bin/winkernel/control.cpp

@@ -441,7 +441,8 @@ size_t QUIC_IOCTL_BUFFER_SIZES[] =
     sizeof(QUIC_RUN_MTU_DISCOVERY_PARAMS),
     sizeof(INT32),
     sizeof(INT32),
-    0
+    0,
+    sizeof(INT32)
 };
 
 CXPLAT_STATIC_ASSERT(
@@ -1078,6 +1079,11 @@ QuicTestCtlEvtIoDeviceControl(
         QuicTestCtlRun(QuicTestValidateParamApi());
         break;
 
+    case IOCTL_QUIC_RUN_CLIENT_LOCAL_PATH_CHANGES:
+        CXPLAT_FRE_ASSERT(Params != nullptr);
+        QuicTestCtlRun(QuicTestLocalPathChanges(Params->Family));
+        break;
+
     default:
         Status = STATUS_NOT_IMPLEMENTED;
         break;

src/test/lib/CMakeLists.txt

@@ -10,6 +10,7 @@ set(SOURCES
     EventTest.cpp
     HandshakeTest.cpp
     MtuTest.cpp
+    PathTest.cpp
     QuicDrill.cpp
     TestConnection.cpp
     TestListener.cpp

src/test/lib/PathTest.cpp

@@ -0,0 +1,86 @@
+/*++
+
+    Copyright (c) Microsoft Corporation.
+    Licensed under the MIT License.
+
+Abstract:
+
+    MsQuic Path Unittest
+
+--*/
+
+#include "precomp.h"
+#ifdef QUIC_CLOG
+#include "PathTest.cpp.clog.h"
+#endif
+
+struct PathTestContext {
+    CxPlatEvent HandshakeCompleteEvent;
+    CxPlatEvent ShutdownEvent;
+    MsQuicConnection* Connection {nullptr};
+    CxPlatEvent PeerAddrChangedEvent;
+
+    static QUIC_STATUS ConnCallback(_In_ MsQuicConnection* Conn, _In_opt_ void* Context, _Inout_ QUIC_CONNECTION_EVENT* Event) {
+        PathTestContext* Ctx = static_cast<PathTestContext*>(Context);
+        Ctx->Connection = Conn;
+        if (Event->Type == QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE) {
+            Ctx->Connection = nullptr;
+            Ctx->PeerAddrChangedEvent.Set();
+            Ctx->ShutdownEvent.Set();
+            Ctx->HandshakeCompleteEvent.Set();
+        } else if (Event->Type == QUIC_CONNECTION_EVENT_CONNECTED) {
+            Ctx->HandshakeCompleteEvent.Set();
+        } else if (Event->Type == QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED) {
+            Ctx->PeerAddrChangedEvent.Set();
+        }
+        return QUIC_STATUS_SUCCESS;
+    }
+};
+
+void
+QuicTestLocalPathChanges(
+    _In_ int Family
+    )
+{
+    MsQuicRegistration Registration{true};
+    TEST_QUIC_SUCCEEDED(Registration.GetInitStatus());
+
+    MsQuicConfiguration ServerConfiguration(Registration, "MsQuicTest", ServerSelfSignedCredConfig);
+    TEST_QUIC_SUCCEEDED(ServerConfiguration.GetInitStatus());
+
+    MsQuicConfiguration ClientConfiguration(Registration, "MsQuicTest", MsQuicCredentialConfig{});
+    TEST_QUIC_SUCCEEDED(ClientConfiguration.GetInitStatus());
+
+    PathTestContext Context;
+    MsQuicAutoAcceptListener Listener(Registration, ServerConfiguration, PathTestContext::ConnCallback, &Context);
+    TEST_QUIC_SUCCEEDED(Listener.GetInitStatus());
+    QUIC_ADDRESS_FAMILY QuicAddrFamily = (Family == 4) ? QUIC_ADDRESS_FAMILY_INET : QUIC_ADDRESS_FAMILY_INET6;
+    QuicAddr ServerLocalAddr(QuicAddrFamily);
+    TEST_QUIC_SUCCEEDED(Listener.Start("MsQuicTest", &ServerLocalAddr.SockAddr));
+    TEST_QUIC_SUCCEEDED(Listener.GetLocalAddr(ServerLocalAddr));
+
+    MsQuicConnection Connection(Registration);
+    TEST_QUIC_SUCCEEDED(Connection.GetInitStatus());
+
+    TEST_QUIC_SUCCEEDED(Connection.StartLocalhost(ClientConfiguration, ServerLocalAddr));
+    TEST_TRUE(Connection.HandshakeCompleteEvent.WaitTimeout(TestWaitTimeout));
+    TEST_NOT_EQUAL(nullptr, Context.Connection);
+    TEST_TRUE(Context.Connection->HandshakeCompleteEvent.WaitTimeout(TestWaitTimeout));
+
+    QuicAddr OrigLocalAddr;
+    TEST_QUIC_SUCCEEDED(Connection.GetLocalAddr(OrigLocalAddr));
+    QuicAddr NewLocalAddr(OrigLocalAddr);
+    ReplaceAddressHelper AddrHelper(OrigLocalAddr.SockAddr, NewLocalAddr.SockAddr);
+
+    for (int i = 0; i < 50; i++) {
+        QuicAddrSetPort(&AddrHelper.New, QuicAddrGetPort(&AddrHelper.New) + 1);
+        Connection.SetSettings(MsQuicSettings{}.SetKeepAlive(25));
+
+        TEST_TRUE(Context.PeerAddrChangedEvent.WaitTimeout(1000))
+        Context.PeerAddrChangedEvent.Reset();
+        QuicAddr ServerRemoteAddr;
+        TEST_QUIC_SUCCEEDED(Context.Connection->GetRemoteAddr(ServerRemoteAddr));
+        TEST_TRUE(QuicAddrCompare(&AddrHelper.New, &ServerRemoteAddr.SockAddr));
+        Connection.SetSettings(MsQuicSettings{}.SetKeepAlive(0));
+    }
+}

src/test/lib/testlib.kernel.vcxproj

@@ -43,6 +43,7 @@
     <ClCompile Include="EventTest.cpp" />
     <ClCompile Include="HandshakeTest.cpp" />
     <ClCompile Include="MtuTest.cpp" />
+    <ClCompile Include="PathTest.cpp" />
     <ClCompile Include="QuicDrill.cpp" />
     <ClCompile Include="TestConnection.cpp" />
     <ClCompile Include="TestListener.cpp" />