diff --git a/src/linux/netlinkutil/RoutingTable.cpp b/src/linux/netlinkutil/RoutingTable.cpp index 8deb15f691..8b7efe1668 100644 --- a/src/linux/netlinkutil/RoutingTable.cpp +++ b/src/linux/netlinkutil/RoutingTable.cpp @@ -92,6 +92,11 @@ void RoutingTable::ModifyRouteImpl(const Route& route, Operation action) int operation = 0; if (action == Update) { + // Note: NLM_F_REPLACE matches an existing route by + // (destination prefix, tos, metric) alone - ignoring the next hop / output interface - + // so replacing a route can silently overwrite a *different* interface's route that happens + // to share the same key (e.g. two interfaces with a default route at the same metric), + // deleting the second route. flags = NLM_F_CREATE | NLM_F_REPLACE; operation = RTM_NEWROUTE; } diff --git a/src/windows/service/exe/WslMirroredNetworking.cpp b/src/windows/service/exe/WslMirroredNetworking.cpp index 28b8d3137e..158cf50571 100644 --- a/src/windows/service/exe/WslMirroredNetworking.cpp +++ b/src/windows/service/exe/WslMirroredNetworking.cpp @@ -1396,24 +1396,26 @@ _Check_return_ bool wsl::core::networking::WslMirroredNetworkManager::SyncIpStat std::optional hr{}; switch (trackedRoute.SyncStatus) { + // Use ModifyRequestType::Add rather than ModifyRequestType::Update for PendingUpdate. + // An Update in GNS uses NLM_F_REPLACE and replaces by + // (destination prefix, tos, metric) key alone and would silently overwrite a same-key route + // belonging to a *different* interface (e.g. another interface's default at the same metric). + // An ModifyRequestType::Add plumbs a new route or leaves an existing one in place (EEXIST is ignored), + // which is the desired behavior for PendingUpdate. + // + // The route synchronization logic never attempts an in-place update of a route. Whenever + // a route change occurs on the host (ProcessRouteChange), we mark for removal the routes that are known + // to have been synced in Linux but are no longer part of the latest set of host routes, we do not + // do a diff of the route properties to determine if an in-place update is needed. case PendingAdd: + case PendingUpdate: hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Add); - if (FAILED(hr.value())) - { - // try to update it instead if it already exists - hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Update); - } break; case Synced: if (refreshAllRoutes) { - hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Update); - if (FAILED(hr.value())) - { - // try to add it - hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Add); - } + hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Add); if (FAILED(hr.value())) { trackedRoute.SyncStatus = PendingUpdate; @@ -1422,15 +1424,6 @@ _Check_return_ bool wsl::core::networking::WslMirroredNetworkManager::SyncIpStat } break; - case PendingUpdate: - hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Update); - if (FAILED(hr.value())) - { - // try to add it - hr = SendRouteRequestToGns(endpoint, trackedRoute, hns::ModifyRequestType::Add); - } - break; - case PendingRemoval: // This route is still slated for removal, which we'll try again later. continue;