mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-05 22:41:30 +02:00
[client] Renew the Android TUN only when the routes it carries change (#7396)
This commit is contained in:
@@ -4,8 +4,6 @@ package notifier
|
||||
|
||||
import (
|
||||
"net/netip"
|
||||
"slices"
|
||||
"sort"
|
||||
"sync"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/listener"
|
||||
@@ -75,19 +73,3 @@ func (n *Notifier) notifyLocked() {
|
||||
func (n *Notifier) Close() {
|
||||
// unused
|
||||
}
|
||||
|
||||
func routesToStrings(routes []*route.Route) []string {
|
||||
nets := make([]string, 0, len(routes))
|
||||
for _, r := range routes {
|
||||
nets = append(nets, r.NetString())
|
||||
}
|
||||
return nets
|
||||
}
|
||||
|
||||
func hasRouteDiff(a []*route.Route, b []*route.Route) bool {
|
||||
as := routesToStrings(a)
|
||||
bs := routesToStrings(b)
|
||||
sort.Strings(as)
|
||||
sort.Strings(bs)
|
||||
return !slices.Equal(as, bs)
|
||||
}
|
||||
|
||||
27
client/internal/routemanager/notifier/route_diff.go
Normal file
27
client/internal/routemanager/notifier/route_diff.go
Normal file
@@ -0,0 +1,27 @@
|
||||
package notifier
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"sort"
|
||||
|
||||
"github.com/netbirdio/netbird/route"
|
||||
)
|
||||
|
||||
// routePrefixes returns the distinct prefixes a route set covers, sorted.
|
||||
// Duplicates are dropped deliberately: an HA group hands us one route per
|
||||
// peer serving the same prefix, and the platform is given the prefix, not the
|
||||
// candidates. Counting them would report a change every time a peer joins or
|
||||
// leaves a group, and on Android each report renews the TUN.
|
||||
func routePrefixes(routes []*route.Route) []string {
|
||||
nets := make([]string, 0, len(routes))
|
||||
for _, r := range routes {
|
||||
nets = append(nets, r.NetString())
|
||||
}
|
||||
sort.Strings(nets)
|
||||
return slices.Compact(nets)
|
||||
}
|
||||
|
||||
// hasRouteDiff reports whether the prefixes the two route sets cover differ.
|
||||
func hasRouteDiff(a []*route.Route, b []*route.Route) bool {
|
||||
return !slices.Equal(routePrefixes(a), routePrefixes(b))
|
||||
}
|
||||
88
client/internal/routemanager/notifier/route_diff_test.go
Normal file
88
client/internal/routemanager/notifier/route_diff_test.go
Normal file
@@ -0,0 +1,88 @@
|
||||
package notifier
|
||||
|
||||
import (
|
||||
"net/netip"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/netbirdio/netbird/route"
|
||||
)
|
||||
|
||||
func routeFor(id route.ID, prefix string) *route.Route {
|
||||
return &route.Route{
|
||||
ID: id,
|
||||
NetID: "net",
|
||||
Network: netip.MustParsePrefix(prefix),
|
||||
}
|
||||
}
|
||||
|
||||
// TestHasRouteDiff_IgnoresHACandidateCount is the reason the comparison
|
||||
// deduplicates. Every notification renews the TUN, and a renewed TUN
|
||||
// invalidates the sockets the embedded servers are listening on, so a peer
|
||||
// joining or leaving an HA group must not count as a route change when the
|
||||
// prefixes the TUN carries are identical.
|
||||
func TestHasRouteDiff_IgnoresHACandidateCount(t *testing.T) {
|
||||
onePeer := []*route.Route{routeFor("a", "10.0.0.0/24")}
|
||||
twoPeers := []*route.Route{
|
||||
routeFor("a", "10.0.0.0/24"),
|
||||
routeFor("b", "10.0.0.0/24"),
|
||||
}
|
||||
|
||||
assert.False(t, hasRouteDiff(onePeer, twoPeers),
|
||||
"a second peer serving the same prefix is not a route change")
|
||||
assert.False(t, hasRouteDiff(twoPeers, onePeer),
|
||||
"losing one of two peers serving the same prefix is not a route change")
|
||||
}
|
||||
|
||||
func TestHasRouteDiff_ReportsRealChanges(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
a []*route.Route
|
||||
b []*route.Route
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "added prefix",
|
||||
a: []*route.Route{routeFor("a", "10.0.0.0/24")},
|
||||
b: []*route.Route{routeFor("a", "10.0.0.0/24"), routeFor("b", "10.0.1.0/24")},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "removed prefix",
|
||||
a: []*route.Route{routeFor("a", "10.0.0.0/24"), routeFor("b", "10.0.1.0/24")},
|
||||
b: []*route.Route{routeFor("a", "10.0.0.0/24")},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "replaced prefix",
|
||||
a: []*route.Route{routeFor("a", "10.0.0.0/24")},
|
||||
b: []*route.Route{routeFor("a", "10.0.1.0/24")},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "same prefix, different order",
|
||||
a: []*route.Route{routeFor("a", "10.0.1.0/24"), routeFor("b", "10.0.0.0/24")},
|
||||
b: []*route.Route{routeFor("b", "10.0.0.0/24"), routeFor("a", "10.0.1.0/24")},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "all routes gone",
|
||||
a: []*route.Route{routeFor("a", "10.0.0.0/24")},
|
||||
b: nil,
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "both empty",
|
||||
a: nil,
|
||||
b: nil,
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
assert.Equal(t, tc.want, hasRouteDiff(tc.a, tc.b),
|
||||
"route diff for %s", tc.name)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user