Skip to content

Commit 3c535cd

Browse files
authored
[client] Add lazy connections to routed networks (#3908)
1 parent 0f050e5 commit 3c535cd

File tree

11 files changed

+918
-147
lines changed

11 files changed

+918
-147
lines changed

client/internal/conn_mgr.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/netbirdio/netbird/client/internal/peer"
1515
"github.com/netbirdio/netbird/client/internal/peer/dispatcher"
1616
"github.com/netbirdio/netbird/client/internal/peerstore"
17+
"github.com/netbirdio/netbird/route"
1718
)
1819

1920
// ConnMgr coordinates both lazy connections (established on-demand) and permanent peer connections.
@@ -97,6 +98,16 @@ func (e *ConnMgr) UpdatedRemoteFeatureFlag(ctx context.Context, enabled bool) er
9798
}
9899
}
99100

101+
// UpdateRouteHAMap updates the route HA mappings in the lazy connection manager
102+
func (e *ConnMgr) UpdateRouteHAMap(haMap route.HAMap) {
103+
if !e.isStartedWithLazyMgr() {
104+
log.Debugf("lazy connection manager is not started, skipping UpdateRouteHAMap")
105+
return
106+
}
107+
108+
e.lazyConnMgr.UpdateRouteHAMap(haMap)
109+
}
110+
100111
// SetExcludeList sets the list of peer IDs that should always have permanent connections.
101112
func (e *ConnMgr) SetExcludeList(peerIDs map[string]bool) {
102113
if e.lazyConnMgr == nil {

client/internal/engine.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,15 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error {
10071007

10081008
// apply routes first, route related actions might depend on routing being enabled
10091009
routes := toRoutes(networkMap.GetRoutes())
1010-
if err := e.routeManager.UpdateRoutes(serial, routes, dnsRouteFeatureFlag); err != nil {
1010+
serverRoutes, clientRoutes := e.routeManager.ClassifyRoutes(routes)
1011+
1012+
// lazy mgr needs to be aware of which routes are available before they are applied
1013+
if e.connMgr != nil {
1014+
e.connMgr.UpdateRouteHAMap(clientRoutes)
1015+
log.Debugf("updated lazy connection manager with %d HA groups", len(clientRoutes))
1016+
}
1017+
1018+
if err := e.routeManager.UpdateRoutes(serial, serverRoutes, clientRoutes, dnsRouteFeatureFlag); err != nil {
10111019
log.Errorf("failed to update routes: %v", err)
10121020
}
10131021

@@ -1067,7 +1075,7 @@ func (e *Engine) updateNetworkMap(networkMap *mgmProto.NetworkMap) error {
10671075
}
10681076

10691077
// must set the exclude list after the peers are added. Without it the manager can not figure out the peers parameters from the store
1070-
excludedLazyPeers := e.toExcludedLazyPeers(routes, forwardingRules, networkMap.GetRemotePeers())
1078+
excludedLazyPeers := e.toExcludedLazyPeers(forwardingRules, networkMap.GetRemotePeers())
10711079
e.connMgr.SetExcludeList(excludedLazyPeers)
10721080

10731081
e.networkSerial = serial
@@ -1933,18 +1941,8 @@ func (e *Engine) updateForwardRules(rules []*mgmProto.ForwardingRule) ([]firewal
19331941
return forwardingRules, nberrors.FormatErrorOrNil(merr)
19341942
}
19351943

1936-
func (e *Engine) toExcludedLazyPeers(routes []*route.Route, rules []firewallManager.ForwardRule, peers []*mgmProto.RemotePeerConfig) map[string]bool {
1944+
func (e *Engine) toExcludedLazyPeers(rules []firewallManager.ForwardRule, peers []*mgmProto.RemotePeerConfig) map[string]bool {
19371945
excludedPeers := make(map[string]bool)
1938-
for _, r := range routes {
1939-
if r.Peer == "" {
1940-
continue
1941-
}
1942-
if !excludedPeers[r.Peer] {
1943-
log.Infof("exclude router peer from lazy connection: %s", r.Peer)
1944-
excludedPeers[r.Peer] = true
1945-
}
1946-
}
1947-
19481946
for _, r := range rules {
19491947
ip := r.TranslatedAddress
19501948
for _, p := range peers {

client/internal/engine_test.go

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -643,12 +643,12 @@ func TestEngine_Sync(t *testing.T) {
643643

644644
func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
645645
testCases := []struct {
646-
name string
647-
inputErr error
648-
networkMap *mgmtProto.NetworkMap
649-
expectedLen int
650-
expectedRoutes []*route.Route
651-
expectedSerial uint64
646+
name string
647+
inputErr error
648+
networkMap *mgmtProto.NetworkMap
649+
expectedLen int
650+
expectedClientRoutes route.HAMap
651+
expectedSerial uint64
652652
}{
653653
{
654654
name: "Routes Config Should Be Passed To Manager",
@@ -676,22 +676,26 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
676676
},
677677
},
678678
expectedLen: 2,
679-
expectedRoutes: []*route.Route{
680-
{
681-
ID: "a",
682-
Network: netip.MustParsePrefix("192.168.0.0/24"),
683-
NetID: "n1",
684-
Peer: "p1",
685-
NetworkType: 1,
686-
Masquerade: false,
679+
expectedClientRoutes: route.HAMap{
680+
"n1|192.168.0.0/24": []*route.Route{
681+
{
682+
ID: "a",
683+
Network: netip.MustParsePrefix("192.168.0.0/24"),
684+
NetID: "n1",
685+
Peer: "p1",
686+
NetworkType: 1,
687+
Masquerade: false,
688+
},
687689
},
688-
{
689-
ID: "b",
690-
Network: netip.MustParsePrefix("192.168.1.0/24"),
691-
NetID: "n2",
692-
Peer: "p1",
693-
NetworkType: 1,
694-
Masquerade: false,
690+
"n2|192.168.1.0/24": []*route.Route{
691+
{
692+
ID: "b",
693+
Network: netip.MustParsePrefix("192.168.1.0/24"),
694+
NetID: "n2",
695+
Peer: "p1",
696+
NetworkType: 1,
697+
Masquerade: false,
698+
},
695699
},
696700
},
697701
expectedSerial: 1,
@@ -704,9 +708,9 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
704708
RemotePeersIsEmpty: false,
705709
Routes: nil,
706710
},
707-
expectedLen: 0,
708-
expectedRoutes: []*route.Route{},
709-
expectedSerial: 1,
711+
expectedLen: 0,
712+
expectedClientRoutes: nil,
713+
expectedSerial: 1,
710714
},
711715
{
712716
name: "Error Shouldn't Break Engine",
@@ -717,9 +721,9 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
717721
RemotePeersIsEmpty: false,
718722
Routes: nil,
719723
},
720-
expectedLen: 0,
721-
expectedRoutes: []*route.Route{},
722-
expectedSerial: 1,
724+
expectedLen: 0,
725+
expectedClientRoutes: nil,
726+
expectedSerial: 1,
723727
},
724728
}
725729

@@ -762,16 +766,29 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
762766
engine.wgInterface, err = iface.NewWGIFace(opts)
763767
assert.NoError(t, err, "shouldn't return error")
764768
input := struct {
765-
inputSerial uint64
766-
inputRoutes []*route.Route
769+
inputSerial uint64
770+
clientRoutes route.HAMap
767771
}{}
768772

769773
mockRouteManager := &routemanager.MockManager{
770-
UpdateRoutesFunc: func(updateSerial uint64, newRoutes []*route.Route) error {
774+
UpdateRoutesFunc: func(updateSerial uint64, serverRoutes map[route.ID]*route.Route, clientRoutes route.HAMap, useNewDNSRoute bool) error {
771775
input.inputSerial = updateSerial
772-
input.inputRoutes = newRoutes
776+
input.clientRoutes = clientRoutes
773777
return testCase.inputErr
774778
},
779+
ClassifyRoutesFunc: func(newRoutes []*route.Route) (map[route.ID]*route.Route, route.HAMap) {
780+
if len(newRoutes) == 0 {
781+
return nil, nil
782+
}
783+
784+
// Classify all routes as client routes (not matching our public key)
785+
clientRoutes := make(route.HAMap)
786+
for _, r := range newRoutes {
787+
haID := r.GetHAUniqueID()
788+
clientRoutes[haID] = append(clientRoutes[haID], r)
789+
}
790+
return nil, clientRoutes
791+
},
775792
}
776793

777794
engine.routeManager = mockRouteManager
@@ -789,8 +806,8 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) {
789806
err = engine.updateNetworkMap(testCase.networkMap)
790807
assert.NoError(t, err, "shouldn't return error")
791808
assert.Equal(t, testCase.expectedSerial, input.inputSerial, "serial should match")
792-
assert.Len(t, input.inputRoutes, testCase.expectedLen, "clientRoutes len should match")
793-
assert.Equal(t, testCase.expectedRoutes, input.inputRoutes, "clientRoutes should match")
809+
assert.Len(t, input.clientRoutes, testCase.expectedLen, "clientRoutes len should match")
810+
assert.Equal(t, testCase.expectedClientRoutes, input.clientRoutes, "clientRoutes should match")
794811
})
795812
}
796813
}
@@ -951,7 +968,7 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) {
951968
assert.NoError(t, err, "shouldn't return error")
952969

953970
mockRouteManager := &routemanager.MockManager{
954-
UpdateRoutesFunc: func(updateSerial uint64, newRoutes []*route.Route) error {
971+
UpdateRoutesFunc: func(updateSerial uint64, serverRoutes map[route.ID]*route.Route, clientRoutes route.HAMap, useNewDNSRoute bool) error {
955972
return nil
956973
},
957974
}

0 commit comments

Comments
 (0)