mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-07 14:09:07 +02:00
routeselector: make exit-node reconciliation atomic
enforceSingleExitNode took the RouteSelector lock three separate times (IsDeselectAll, then DeselectRoutes, then SelectRoutes), so a concurrent DeselectAllRoutes could interleave and be silently undone: SelectRoutes on its deselectAll branch clears the flag and re-selects the preferred exit node, overriding the user's "all off". Move the whole reconciliation into a single locked RouteSelector method (SetExclusiveExitNode) that checks deselectAll inside the critical section, so a deselect-all either fully precedes the reconcile (left untouched) or fully follows it (honoured). No interleaving is possible.
This commit is contained in:
@@ -836,29 +836,12 @@ func pickPreferredExitNode(info exitNodeInfo) route.NetID {
|
|||||||
|
|
||||||
// enforceSingleExitNode makes preferred the only selected exit node: every other
|
// enforceSingleExitNode makes preferred the only selected exit node: every other
|
||||||
// available exit node is deselected and preferred (if any) is selected, without
|
// available exit node is deselected and preferred (if any) is selected, without
|
||||||
// disturbing non-exit route selections. A global deselect-all is left untouched
|
// disturbing non-exit route selections. The whole reconciliation runs under a
|
||||||
// so the user's "all off" stays in effect.
|
// single RouteSelector lock (SetExclusiveExitNode) so a concurrent deselect-all
|
||||||
|
// cannot interleave and get undone; a global deselect-all is left untouched so
|
||||||
|
// the user's "all off" stays in effect.
|
||||||
func (m *DefaultManager) enforceSingleExitNode(preferred route.NetID, allIDs []route.NetID) {
|
func (m *DefaultManager) enforceSingleExitNode(preferred route.NetID, allIDs []route.NetID) {
|
||||||
if m.routeSelector.IsDeselectAll() {
|
m.routeSelector.SetExclusiveExitNode(preferred, allIDs)
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
others := make([]route.NetID, 0, len(allIDs))
|
|
||||||
for _, id := range allIDs {
|
|
||||||
if id != preferred {
|
|
||||||
others = append(others, id)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if len(others) > 0 {
|
|
||||||
if err := m.routeSelector.DeselectRoutes(others, allIDs); err != nil {
|
|
||||||
log.Warnf("deselect other exit nodes: %v", err)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if preferred != "" {
|
|
||||||
if err := m.routeSelector.SelectRoutes([]route.NetID{preferred}, true, allIDs); err != nil {
|
|
||||||
log.Warnf("select preferred exit node %q: %v", preferred, err)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func (m *DefaultManager) logExitNodeUpdate(info exitNodeInfo, preferred route.NetID) {
|
func (m *DefaultManager) logExitNodeUpdate(info exitNodeInfo, preferred route.NetID) {
|
||||||
|
|||||||
@@ -115,6 +115,35 @@ func (rs *RouteSelector) DeselectAllRoutes() {
|
|||||||
clear(rs.selectedRoutes)
|
clear(rs.selectedRoutes)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetExclusiveExitNode atomically makes preferred the only selected exit node
|
||||||
|
// among exitIDs: every other ID in exitIDs is deselected and preferred (when
|
||||||
|
// non-empty) is selected, all under a single lock. Holding the lock across the
|
||||||
|
// whole reconciliation prevents a concurrent DeselectAllRoutes from interleaving
|
||||||
|
// between the deselect and select steps and being silently undone. A global
|
||||||
|
// deselect-all is left untouched so the user's "all off" stays in effect;
|
||||||
|
// non-exit routes are never referenced, so their selection is preserved.
|
||||||
|
func (rs *RouteSelector) SetExclusiveExitNode(preferred route.NetID, exitIDs []route.NetID) {
|
||||||
|
rs.mu.Lock()
|
||||||
|
defer rs.mu.Unlock()
|
||||||
|
|
||||||
|
if rs.deselectAll {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, id := range exitIDs {
|
||||||
|
if id == preferred {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
rs.deselectedRoutes[id] = struct{}{}
|
||||||
|
delete(rs.selectedRoutes, id)
|
||||||
|
}
|
||||||
|
|
||||||
|
if preferred != "" {
|
||||||
|
delete(rs.deselectedRoutes, preferred)
|
||||||
|
rs.selectedRoutes[preferred] = struct{}{}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// IsDeselectAll reports whether the global "deselect all" flag is set, i.e. the
|
// IsDeselectAll reports whether the global "deselect all" flag is set, i.e. the
|
||||||
// user explicitly disabled every route. Callers enforcing per-route invariants
|
// user explicitly disabled every route. Callers enforcing per-route invariants
|
||||||
// (e.g. single exit node) should leave the selection untouched when it is.
|
// (e.g. single exit node) should leave the selection untouched when it is.
|
||||||
|
|||||||
Reference in New Issue
Block a user