mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-06 05:29:07 +02:00
[client] Resolve the Android network route peer by HA unique ID instead of scanning the full status (#7705)
* [client] Build the Android network list from a single peer snapshot Networks() called GetFullStatus() once per network to find the peer that serves it, copying every peer state and taking eight recorder locks each time. With 100+ peers and the UI calling Networks() from every peer list change, this queued hundreds of callers on the status recorder lock. Take one snapshot per call and index it by route. * [client] Track the active route peer by HA unique ID in the status recorder The Android network list resolved the route owner by scanning peer state route keys. Those keys are the handler string: a prefix for static routes and the domain pattern for dynamic ones, so the prefix-based lookup never matched dynamic routes, and two networks sharing a prefix resolved to the same owner. The route watcher now records the chosen route peer under the route's HA unique ID in the status recorder, and the Android binding looks the owner up by that ID. The key is unique per network and independent of the handler string format, so both anomalies are gone. The prefix-based routeOwners helper is removed. * [client] Record the active route peer before notifying listeners AddPeerStateRoute and RemovePeerStateRoute fire the peer list change callback and wake the status subscribers. The active route peer mapping was written after those calls, so a Networks() call landing in between found no mapping for the network and fell back to the first connected peer, or kept showing the previous peer on removal. Nothing re-notified after the mapping write, so the wrong peer stayed until the next peer list change. Write and delete the mapping before the notifying calls so a listener reacting to the notification always reads the current owner.
This commit is contained in:
@@ -196,6 +196,7 @@ type Status struct {
|
||||
muxRelays sync.RWMutex
|
||||
peers map[string]State
|
||||
ipToKey map[string]string
|
||||
activeRoutePeers map[route.HAUniqueID]string
|
||||
changeNotify map[string]map[string]*StatusChangeSubscription // map[peerID]map[subscriptionID]*StatusChangeSubscription
|
||||
signalState bool
|
||||
signalError error
|
||||
@@ -257,6 +258,7 @@ func NewRecorder(mgmAddress string) *Status {
|
||||
return &Status{
|
||||
peers: make(map[string]State),
|
||||
ipToKey: make(map[string]string),
|
||||
activeRoutePeers: make(map[route.HAUniqueID]string),
|
||||
changeNotify: make(map[string]map[string]*StatusChangeSubscription),
|
||||
eventStreams: make(map[string]chan *proto.SystemEvent),
|
||||
eventQueue: NewEventQueue(eventQueueSize),
|
||||
@@ -481,6 +483,24 @@ func (d *Status) RemovePeerStateRoute(peer string, route string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (d *Status) AddActiveRoutePeer(haID route.HAUniqueID, peer string) {
|
||||
d.mux.Lock()
|
||||
defer d.mux.Unlock()
|
||||
d.activeRoutePeers[haID] = peer
|
||||
}
|
||||
|
||||
func (d *Status) RemoveActiveRoutePeer(haID route.HAUniqueID) {
|
||||
d.mux.Lock()
|
||||
defer d.mux.Unlock()
|
||||
delete(d.activeRoutePeers, haID)
|
||||
}
|
||||
|
||||
func (d *Status) GetActiveRoutePeers() map[route.HAUniqueID]string {
|
||||
d.mux.RLock()
|
||||
defer d.mux.RUnlock()
|
||||
return maps.Clone(d.activeRoutePeers)
|
||||
}
|
||||
|
||||
// CheckRoutes checks if the source and destination addresses are within the same route
|
||||
// and returns the resource ID of the route that contains the addresses
|
||||
func (d *Status) CheckRoutes(ip netip.Addr) ([]byte, bool) {
|
||||
|
||||
@@ -9,6 +9,8 @@ import (
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/route"
|
||||
)
|
||||
|
||||
func TestAddPeer(t *testing.T) {
|
||||
@@ -372,3 +374,24 @@ func TestMarkServerStateDoesNotNotifyWhenUnchanged(t *testing.T) {
|
||||
status.MarkManagementDisconnected(err)
|
||||
assert.False(t, notified(ch), "redundant disconnect should not notify")
|
||||
}
|
||||
|
||||
func TestActiveRoutePeers(t *testing.T) {
|
||||
status := NewRecorder("https://mgm")
|
||||
netA := route.HAUniqueID("net-a-10.0.0.0/24")
|
||||
netB := route.HAUniqueID("net-b-10.0.0.0/24")
|
||||
|
||||
status.AddActiveRoutePeer(netA, "peerA")
|
||||
status.AddActiveRoutePeer(netB, "peerB")
|
||||
|
||||
active := status.GetActiveRoutePeers()
|
||||
assert.Equal(t, "peerA", active[netA])
|
||||
assert.Equal(t, "peerB", active[netB])
|
||||
|
||||
status.RemoveActiveRoutePeer(netA)
|
||||
delete(active, netB)
|
||||
|
||||
active = status.GetActiveRoutePeers()
|
||||
_, ok := active[netA]
|
||||
assert.False(t, ok)
|
||||
assert.Equal(t, "peerB", active[netB])
|
||||
}
|
||||
|
||||
@@ -294,6 +294,7 @@ func (w *Watcher) addAllowedIPs(route *route.Route) error {
|
||||
return fmt.Errorf("add allowed IPs for peer %s: %w", route.Peer, err)
|
||||
}
|
||||
|
||||
w.statusRecorder.AddActiveRoutePeer(route.GetHAUniqueID(), route.Peer)
|
||||
if err := w.statusRecorder.AddPeerStateRoute(route.Peer, w.handler.String(), route.GetResourceID()); err != nil {
|
||||
log.Warnf("Failed to update peer state: %v", err)
|
||||
}
|
||||
@@ -303,6 +304,7 @@ func (w *Watcher) addAllowedIPs(route *route.Route) error {
|
||||
}
|
||||
|
||||
func (w *Watcher) removeAllowedIPs(route *route.Route, rsn reason) error {
|
||||
w.statusRecorder.RemoveActiveRoutePeer(route.GetHAUniqueID())
|
||||
if err := w.statusRecorder.RemovePeerStateRoute(route.Peer, w.handler.String()); err != nil {
|
||||
log.Warnf("Failed to update peer state: %v", err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user