From 8e5130cda726f93d5115945448f41118081ba26e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Mon, 1 Jun 2026 10:55:07 +0200 Subject: [PATCH] [client] remove exit-node v6 DIAG logging and tidy routeselector Drop the temporary DIAG diagnostics added to trace the leaking ::/0 route (the root cause is fixed and confirmed). Also reorganize routeselector.go so the exit-node helpers (clearPairedV6Locked, isExitNode) sit next to the exit-node code paths and MarshalJSON/UnmarshalJSON are grouped together. --- client/internal/routemanager/manager.go | 18 -- .../internal/routeselector/routeselector.go | 226 ++++++++---------- 2 files changed, 94 insertions(+), 150 deletions(-) diff --git a/client/internal/routemanager/manager.go b/client/internal/routemanager/manager.go index 9fadf2795..a9a1f5eed 100644 --- a/client/internal/routemanager/manager.go +++ b/client/internal/routemanager/manager.go @@ -428,16 +428,10 @@ func (m *DefaultManager) UpdateRoutes( var merr *multierror.Error if !m.disableClientRoutes { - log.Debugf("DIAG UpdateRoutes: incoming %d client networks: %v", len(clientRoutes), haKeysList(clientRoutes)) - log.Debugf("DIAG UpdateRoutes: selector BEFORE management update: %s", m.routeSelector.MarshalSummary()) - // Update route selector based on management server's isSelected status m.updateRouteSelectorFromManagement(clientRoutes) - log.Debugf("DIAG UpdateRoutes: selector AFTER management update: %s", m.routeSelector.MarshalSummary()) - filteredClientRoutes := m.routeSelector.FilterSelectedExitNodes(clientRoutes) - log.Debugf("DIAG UpdateRoutes: AFTER filter, %d networks remain: %v", len(filteredClientRoutes), haKeysList(filteredClientRoutes)) if err := m.updateSystemRoutes(filteredClientRoutes); err != nil { merr = multierror.Append(merr, fmt.Errorf("update system routes: %w", err)) @@ -772,8 +766,6 @@ func (m *DefaultManager) checkManagementSelection(routes []*route.Route, netID r func (m *DefaultManager) updateExitNodeSelections(info exitNodeInfo) { routesToDeselect := m.getRoutesToDeselect(info.allIDs) - log.Debugf("DIAG updateExitNodeSelections: allIDs=%v userSelected=%v userDeselected=%v selectedByManagement=%v -> routesToDeselect(no user selection)=%v", - info.allIDs, info.userSelected, info.userDeselected, info.selectedByManagement, routesToDeselect) m.deselectExitNodes(routesToDeselect) m.selectExitNodesByManagement(info.selectedByManagement, info.allIDs) } @@ -813,14 +805,4 @@ func (m *DefaultManager) selectExitNodesByManagement(selectedByManagement []rout func (m *DefaultManager) logExitNodeUpdate(info exitNodeInfo) { log.Debugf("Updated route selector: %d exit nodes available, %d selected by management, %d user-selected, %d user-deselected", len(info.allIDs), len(info.selectedByManagement), len(info.userSelected), len(info.userDeselected)) - log.Debugf("DIAG logExitNodeUpdate: allIDs=%v selectedByManagement=%v userSelected=%v userDeselected=%v", - info.allIDs, info.selectedByManagement, info.userSelected, info.userDeselected) -} - -func haKeysList(m route.HAMap) []route.HAUniqueID { - out := make([]route.HAUniqueID, 0, len(m)) - for k := range m { - out = append(out, k) - } - return out } diff --git a/client/internal/routeselector/routeselector.go b/client/internal/routeselector/routeselector.go index 63a29b3b7..7f211304b 100644 --- a/client/internal/routeselector/routeselector.go +++ b/client/internal/routeselector/routeselector.go @@ -8,7 +8,6 @@ import ( "sync" "github.com/hashicorp/go-multierror" - log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/client/errors" "github.com/netbirdio/netbird/route" @@ -110,23 +109,6 @@ func (rs *RouteSelector) DeselectRoutes(routes []route.NetID, allRoutes []route. return errors.FormatErrorOrNil(err) } -// clearPairedV6Locked removes any explicit selected/deselected state on the "-v6" -// sibling of a v4 exit-node NetID, so the synthesized v6 entry resolves through -// effectiveNetID to its v4 base. No-op for IDs that already carry the "-v6" suffix, -// or when the sibling is itself part of the current batch (the caller is setting it -// deliberately, e.g. via ExpandV6ExitPairs). Must be called with rs.mu held. -func (rs *RouteSelector) clearPairedV6Locked(id route.NetID, batch []route.NetID) { - if strings.HasSuffix(string(id), route.V6ExitSuffix) { - return - } - v6ID := route.NetID(string(id) + route.V6ExitSuffix) - if slices.Contains(batch, v6ID) { - return - } - delete(rs.selectedRoutes, v6ID) - delete(rs.deselectedRoutes, v6ID) -} - // DeselectAllRoutes deselects all routes, effectively disabling route selection. func (rs *RouteSelector) DeselectAllRoutes() { rs.mu.Lock() @@ -162,13 +144,6 @@ func (rs *RouteSelector) IsSelectedForExitNode(routeID route.NetID) bool { return rs.isSelectedLocked(rs.effectiveNetID(routeID)) } -// MarshalSummary returns a short human-readable description of the selector state for diagnostics. -func (rs *RouteSelector) MarshalSummary() string { - rs.mu.RLock() - defer rs.mu.RUnlock() - return fmt.Sprintf("deselectAll=%v selected=%v deselected=%v", rs.deselectAll, keysOf(rs.selectedRoutes), keysOf(rs.deselectedRoutes)) -} - // FilterSelected removes unselected routes from the provided map. func (rs *RouteSelector) FilterSelected(routes route.HAMap) route.HAMap { rs.mu.RLock() @@ -206,131 +181,24 @@ func (rs *RouteSelector) FilterSelectedExitNodes(routes route.HAMap) route.HAMap return route.HAMap{} } - log.Debugf("DIAG FilterSelectedExitNodes: incoming %d networks, deselected=%v selected=%v deselectAll=%v", - len(routes), keysOf(rs.deselectedRoutes), keysOf(rs.selectedRoutes), rs.deselectAll) - filtered := make(route.HAMap, len(routes)) for id, rt := range routes { netID := id.NetID() if rs.isDeselectedLocked(netID) { - log.Debugf("DIAG FilterSelectedExitNodes: SKIP id=%q netID=%q (literally deselected)", id, netID) continue } if !isExitNode(rt) { - log.Debugf("DIAG FilterSelectedExitNodes: KEEP id=%q netID=%q (not an exit node)", id, netID) filtered[id] = rt continue } - log.Debugf("DIAG FilterSelectedExitNodes: EXITNODE id=%q netID=%q -> applyExitNodeFilter", id, netID) rs.applyExitNodeFilter(id, netID, rt, filtered) } - log.Debugf("DIAG FilterSelectedExitNodes: result keeps %d networks: %v", len(filtered), haKeysOf(filtered)) return filtered } -func keysOf(m map[route.NetID]struct{}) []route.NetID { - out := make([]route.NetID, 0, len(m)) - for k := range m { - out = append(out, k) - } - return out -} - -func haKeysOf(m route.HAMap) []route.HAUniqueID { - out := make([]route.HAUniqueID, 0, len(m)) - for k := range m { - out = append(out, k) - } - return out -} - -// effectiveNetID returns the v4 base for a "-v6" exit pair entry that has no explicit -// state of its own, so selections made on the v4 entry govern the v6 entry automatically. -// Only call this from exit-node-specific code paths: applying it to a non-exit "-v6" route -// would make it inherit unrelated v4 state. Must be called with rs.mu held. -func (rs *RouteSelector) effectiveNetID(id route.NetID) route.NetID { - name := string(id) - if !strings.HasSuffix(name, route.V6ExitSuffix) { - return id - } - if _, ok := rs.selectedRoutes[id]; ok { - return id - } - if _, ok := rs.deselectedRoutes[id]; ok { - return id - } - return route.NetID(strings.TrimSuffix(name, route.V6ExitSuffix)) -} - -func (rs *RouteSelector) isSelectedLocked(routeID route.NetID) bool { - if rs.deselectAll { - return false - } - _, deselected := rs.deselectedRoutes[routeID] - return !deselected -} - -func (rs *RouteSelector) isDeselectedLocked(netID route.NetID) bool { - if rs.deselectAll { - return true - } - _, deselected := rs.deselectedRoutes[netID] - return deselected -} - -func (rs *RouteSelector) hasUserSelectionForRouteLocked(routeID route.NetID) bool { - _, selected := rs.selectedRoutes[routeID] - _, deselected := rs.deselectedRoutes[routeID] - return selected || deselected -} - -func isExitNode(rt []*route.Route) bool { - return len(rt) > 0 && (route.IsV4DefaultRoute(rt[0].Network) || route.IsV6DefaultRoute(rt[0].Network)) -} - -func (rs *RouteSelector) applyExitNodeFilter( - id route.HAUniqueID, - netID route.NetID, - rt []*route.Route, - out route.HAMap, -) { - // Exit-node path: apply the v4/v6 pair mirror so a deselect on the v4 base also - // drops the synthesized v6 entry that lacks its own explicit state. - effective := rs.effectiveNetID(netID) - log.Debugf("DIAG applyExitNodeFilter: id=%q netID=%q effective=%q hasUserSel=%v isSelected=%v", - id, netID, effective, rs.hasUserSelectionForRouteLocked(effective), rs.isSelectedLocked(effective)) - if rs.hasUserSelectionForRouteLocked(effective) { - if rs.isSelectedLocked(effective) { - log.Debugf("DIAG applyExitNodeFilter: KEEP id=%q (effective %q is selected)", id, effective) - out[id] = rt - } else { - log.Debugf("DIAG applyExitNodeFilter: DROP id=%q (effective %q is deselected)", id, effective) - } - return - } - - // no explicit selection for this route: defer to management's SkipAutoApply flag - sel := collectSelected(rt) - log.Debugf("DIAG applyExitNodeFilter: no user selection for effective %q; SkipAutoApply filter kept %d/%d routes for id=%q", - effective, len(sel), len(rt), id) - if len(sel) > 0 { - out[id] = sel - } -} - -func collectSelected(rt []*route.Route) []*route.Route { - var sel []*route.Route - for _, r := range rt { - if !r.SkipAutoApply { - sel = append(sel, r) - } - } - return sel -} - // MarshalJSON implements the json.Marshaler interface func (rs *RouteSelector) MarshalJSON() ([]byte, error) { rs.mu.RLock() @@ -384,3 +252,97 @@ func (rs *RouteSelector) UnmarshalJSON(data []byte) error { return nil } + +// effectiveNetID returns the v4 base for a "-v6" exit pair entry that has no explicit +// state of its own, so selections made on the v4 entry govern the v6 entry automatically. +// Only call this from exit-node-specific code paths: applying it to a non-exit "-v6" route +// would make it inherit unrelated v4 state. Must be called with rs.mu held. +func (rs *RouteSelector) effectiveNetID(id route.NetID) route.NetID { + name := string(id) + if !strings.HasSuffix(name, route.V6ExitSuffix) { + return id + } + if _, ok := rs.selectedRoutes[id]; ok { + return id + } + if _, ok := rs.deselectedRoutes[id]; ok { + return id + } + return route.NetID(strings.TrimSuffix(name, route.V6ExitSuffix)) +} + +func (rs *RouteSelector) isSelectedLocked(routeID route.NetID) bool { + if rs.deselectAll { + return false + } + _, deselected := rs.deselectedRoutes[routeID] + return !deselected +} + +func (rs *RouteSelector) isDeselectedLocked(netID route.NetID) bool { + if rs.deselectAll { + return true + } + _, deselected := rs.deselectedRoutes[netID] + return deselected +} + +func (rs *RouteSelector) hasUserSelectionForRouteLocked(routeID route.NetID) bool { + _, selected := rs.selectedRoutes[routeID] + _, deselected := rs.deselectedRoutes[routeID] + return selected || deselected +} + +// clearPairedV6Locked removes any explicit selected/deselected state on the "-v6" +// sibling of a v4 exit-node NetID, so the synthesized v6 entry resolves through +// effectiveNetID to its v4 base. No-op for IDs that already carry the "-v6" suffix, +// or when the sibling is itself part of the current batch (the caller is setting it +// deliberately, e.g. via ExpandV6ExitPairs). Must be called with rs.mu held. +func (rs *RouteSelector) clearPairedV6Locked(id route.NetID, batch []route.NetID) { + if strings.HasSuffix(string(id), route.V6ExitSuffix) { + return + } + v6ID := route.NetID(string(id) + route.V6ExitSuffix) + if slices.Contains(batch, v6ID) { + return + } + delete(rs.selectedRoutes, v6ID) + delete(rs.deselectedRoutes, v6ID) +} + +func (rs *RouteSelector) applyExitNodeFilter( + id route.HAUniqueID, + netID route.NetID, + rt []*route.Route, + out route.HAMap, +) { + // Exit-node path: apply the v4/v6 pair mirror so a deselect on the v4 base also + // drops the synthesized v6 entry that lacks its own explicit state. + effective := rs.effectiveNetID(netID) + if rs.hasUserSelectionForRouteLocked(effective) { + if rs.isSelectedLocked(effective) { + out[id] = rt + } + return + } + + // no explicit selection for this route: defer to management's SkipAutoApply flag + sel := collectSelected(rt) + if len(sel) > 0 { + out[id] = sel + } +} + +func isExitNode(rt []*route.Route) bool { + return len(rt) > 0 && (route.IsV4DefaultRoute(rt[0].Network) || route.IsV6DefaultRoute(rt[0].Network)) +} + +func collectSelected(rt []*route.Route) []*route.Route { + var sel []*route.Route + for _, r := range rt { + if !r.SkipAutoApply { + sel = append(sel, r) + } + } + return sel +}