Fix tracked routes being collapsed by incomplete comparison

src/windows/service/exe/WslCoreTcpIpStateTracking.h

@@ -161,44 +161,23 @@ struct TrackedRoute
 
     bool operator<(const TrackedRoute& other) const noexcept
     {
-        // return true if 'this' is less than (i.e. is ordered before) the input argument
-        if (Route.IsAutoGeneratedPrefixRoute || other.Route.IsAutoGeneratedPrefixRoute)
-        {
-            if (Route.IsAutoGeneratedPrefixRoute && other.Route.IsAutoGeneratedPrefixRoute)
+        const auto routeClass = [](const EndpointRoute& route) noexcept {
+            if (route.IsAutoGeneratedPrefixRoute)
             {
-                // if both are effectively equivalent, sort by their addresses
-                if (Route.DestinationPrefixString == other.Route.DestinationPrefixString)
-                {
-                    return Route.Metric < other.Route.Metric;
-                }
-                return Route.DestinationPrefixString < other.Route.DestinationPrefixString;
+                return 0;
             }
-            // else return true if it's the left that's IsAutoGeneratedPrefixRoute
-            return Route.IsAutoGeneratedPrefixRoute;
-        }
 
-        if (Route.IsNextHopOnlink() || other.Route.IsNextHopOnlink())
-        {
-            if (Route.IsNextHopOnlink() && other.Route.IsNextHopOnlink())
-            {
-                // if both are effectively equivalent, sort by their addresses
-                if (Route.DestinationPrefixString == other.Route.DestinationPrefixString)
-                {
-                    return Route.Metric < other.Route.Metric;
-                }
-                return Route.DestinationPrefixString < other.Route.DestinationPrefixString;
-            }
-            // else return true if it's the left that's IsNextHopOnlink()
-            return Route.IsNextHopOnlink();
-        }
+            return route.IsNextHopOnlink() ? 1 : 2;
+        };
 
-        // else it's an Add or Update for a route that's not an auto-generated route
-        // and whose next-hop address is not on-link
-        if (Route.DestinationPrefixString == other.Route.DestinationPrefixString)
+        const auto thisRouteClass = routeClass(Route);
+        const auto otherRouteClass = routeClass(other.Route);
+        if (thisRouteClass != otherRouteClass)
         {
-            return Route.Metric < other.Route.Metric;
+            return thisRouteClass < otherRouteClass;
         }
-        return Route.DestinationPrefixString < other.Route.DestinationPrefixString;
+
+        return Route < other.Route;
     }
 };
 

test/windows/CMakeLists.txt

@@ -35,6 +35,8 @@ endif ()
 target_include_directories(wsltests PRIVATE
     ${CMAKE_SOURCE_DIR}/src/windows/WslcSDK
     ${CMAKE_BINARY_DIR}/src/windows/WslcSDK/winrt/${TARGET_PLATFORM}/${CMAKE_BUILD_TYPE})
+set_property(SOURCE NetworkTests.cpp APPEND PROPERTY INCLUDE_DIRECTORIES
+    ${CMAKE_SOURCE_DIR}/src/windows/service/exe)
 target_link_directories(wsltests PRIVATE ${BIN})
 target_precompile_headers(wsltests REUSE_FROM common)
 target_link_libraries(wsltests

test/windows/NetworkTests.cpp

@@ -18,6 +18,7 @@ Module Name:
 #include "wslpolicies.h"
 #include "hns_schema.h"
 #include "WslCoreNetworkEndpointSettings.h"
+#include "WslCoreTcpIpStateTracking.h"
 
 #include <mstcpip.h>
 #include <winhttp.h>
@@ -607,6 +608,36 @@ class NetworkTests
         VERIFY_IS_TRUE(state.Routes.empty());
     }
 
+    TEST_METHOD(TrackedRouteOrderingPreservesRouteIdentity)
+    {
+        const auto makeRoute = [](const wchar_t* destination, uint8_t prefixLength, const wchar_t* nextHop, ULONG metric = 10) {
+            MIB_IPFORWARD_ROW2 routeRow{};
+            routeRow.DestinationPrefix.Prefix = wsl::windows::common::string::StringToSockAddrInet(destination);
+            routeRow.DestinationPrefix.PrefixLength = prefixLength;
+            routeRow.NextHop = wsl::windows::common::string::StringToSockAddrInet(nextHop);
+            routeRow.Metric = metric;
+            return wsl::core::networking::EndpointRoute(routeRow);
+        };
+
+        std::set<wsl::core::networking::TrackedRoute> routes;
+        routes.emplace(makeRoute(L"10.0.0.0", 8, L"192.168.0.1"));
+        routes.emplace(makeRoute(L"10.0.0.0", 24, L"192.168.0.1"));
+        routes.emplace(makeRoute(L"10.0.0.0", 24, L"192.168.0.2"));
+        routes.emplace(makeRoute(L"10.0.0.0", 24, L"192.168.0.2", 20));
+        routes.emplace(makeRoute(L"10.0.0.0", 24, L"192.168.0.2"));
+
+        VERIFY_ARE_EQUAL(static_cast<size_t>(4), routes.size());
+
+        auto autoGeneratedRoute = makeRoute(L"192.168.0.0", 24, L"0.0.0.0");
+        autoGeneratedRoute.IsAutoGeneratedPrefixRoute = true;
+        const wsl::core::networking::TrackedRoute trackedAutoGeneratedRoute(autoGeneratedRoute);
+        const wsl::core::networking::TrackedRoute trackedOnlinkRoute(makeRoute(L"192.168.1.0", 24, L"0.0.0.0"));
+        const wsl::core::networking::TrackedRoute trackedOfflinkRoute(makeRoute(L"192.168.2.0", 24, L"192.168.0.1"));
+
+        VERIFY_IS_TRUE(trackedAutoGeneratedRoute < trackedOnlinkRoute);
+        VERIFY_IS_TRUE(trackedOnlinkRoute < trackedOfflinkRoute);
+    }
+
     WSL2_TEST_METHOD(UpdateIpAddress)
     {
         TestCase({{L"eth0", {{L"192.168.0.2", 24}}, L"192.168.0.1", {{L"fc00::2", 64}}, L"fc00::1"}});