From 3ed2f08f3c5dd930a598a26f24cf028807816486 Mon Sep 17 00:00:00 2001 From: pascal-fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Tue, 9 Apr 2024 21:20:02 +0200 Subject: [PATCH] Add latency based routing (#1732) Now that we have the latency between peers available we can use this data to consider when choosing the best route. This way the route with the routing peer with the lower latency will be preferred over others with the same target network. --- client/internal/routemanager/client.go | 38 ++++-- client/internal/routemanager/client_test.go | 142 ++++++++++++++++++-- 2 files changed, 163 insertions(+), 17 deletions(-) diff --git a/client/internal/routemanager/client.go b/client/internal/routemanager/client.go index 38cf4bf6550..370ad5cf44b 100644 --- a/client/internal/routemanager/client.go +++ b/client/internal/routemanager/client.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/netip" + "time" log "github.com/sirupsen/logrus" @@ -18,6 +19,7 @@ type routerPeerStatus struct { connected bool relayed bool direct bool + latency time.Duration } type routesUpdate struct { @@ -68,6 +70,7 @@ func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus { connected: peerStatus.ConnStatus == peer.StatusConnected, relayed: peerStatus.Relayed, direct: peerStatus.Direct, + latency: peerStatus.Latency, } } return routePeerStatuses @@ -83,11 +86,13 @@ func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus { // * Non-relayed: Routes without relays are preferred. // * Direct connections: Routes with direct peer connections are favored. // * Stability: In case of equal scores, the currently active route (if any) is maintained. +// * Latency: Routes with lower latency are prioritized. // // It returns the ID of the selected optimal route. func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]routerPeerStatus) string { chosen := "" - chosenScore := 0 + chosenScore := float64(0) + currScore := float64(0) currID := "" if c.chosenRoute != nil { @@ -95,7 +100,7 @@ func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]ro } for _, r := range c.routes { - tempScore := 0 + tempScore := float64(0) peerStatus, found := routePeerStatuses[r.ID] if !found || !peerStatus.connected { continue @@ -103,9 +108,18 @@ func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]ro if r.Metric < route.MaxMetric { metricDiff := route.MaxMetric - r.Metric - tempScore = metricDiff * 10 + tempScore = float64(metricDiff) * 10 } + // in some temporal cases, latency can be 0, so we set it to 1s to not block but try to avoid this route + latency := time.Second + if peerStatus.latency != 0 { + latency = peerStatus.latency + } else { + log.Warnf("peer %s has 0 latency", r.Peer) + } + tempScore += 1 - latency.Seconds() + if !peerStatus.relayed { tempScore++ } @@ -114,7 +128,7 @@ func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]ro tempScore++ } - if tempScore > chosenScore || (tempScore == chosenScore && r.ID == currID) { + if tempScore > chosenScore || (tempScore == chosenScore && chosen == "") { chosen = r.ID chosenScore = tempScore } @@ -123,18 +137,26 @@ func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]ro chosen = r.ID chosenScore = tempScore } + + if r.ID == currID { + currScore = tempScore + } } - if chosen == "" { + switch { + case chosen == "": var peers []string for _, r := range c.routes { peers = append(peers, r.Peer) } log.Warnf("the network %s has not been assigned a routing peer as no peers from the list %s are currently connected", c.network, peers) - - } else if chosen != currID { - log.Infof("new chosen route is %s with peer %s with score %d for network %s", chosen, c.routes[chosen].Peer, chosenScore, c.network) + case chosen != currID: + if currScore != 0 && currScore < chosenScore+0.1 { + return currID + } else { + log.Infof("new chosen route is %s with peer %s with score %f for network %s", chosen, c.routes[chosen].Peer, chosenScore, c.network) + } } return chosen diff --git a/client/internal/routemanager/client_test.go b/client/internal/routemanager/client_test.go index 3700d72ecd7..d24d42b8eaf 100644 --- a/client/internal/routemanager/client_test.go +++ b/client/internal/routemanager/client_test.go @@ -3,6 +3,7 @@ package routemanager import ( "net/netip" "testing" + "time" "github.com/netbirdio/netbird/route" ) @@ -13,7 +14,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { name string statuses map[string]routerPeerStatus expectedRouteID string - currentRoute *route.Route + currentRoute string existingRoutes map[string]*route.Route }{ { @@ -32,7 +33,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer1", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "route1", }, { @@ -51,7 +52,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer1", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "route1", }, { @@ -70,7 +71,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer1", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "route1", }, { @@ -89,7 +90,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer1", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "", }, { @@ -118,7 +119,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer2", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "route1", }, { @@ -147,7 +148,7 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer2", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "route1", }, { @@ -176,18 +177,141 @@ func TestGetBestrouteFromStatuses(t *testing.T) { Peer: "peer2", }, }, - currentRoute: nil, + currentRoute: "", expectedRouteID: "route1", }, + { + name: "multiple connected peers with different latencies", + statuses: map[string]routerPeerStatus{ + "route1": { + connected: true, + latency: 300 * time.Millisecond, + }, + "route2": { + connected: true, + latency: 10 * time.Millisecond, + }, + }, + existingRoutes: map[string]*route.Route{ + "route1": { + ID: "route1", + Metric: route.MaxMetric, + Peer: "peer1", + }, + "route2": { + ID: "route2", + Metric: route.MaxMetric, + Peer: "peer2", + }, + }, + currentRoute: "", + expectedRouteID: "route2", + }, + { + name: "should ignore routes with latency 0", + statuses: map[string]routerPeerStatus{ + "route1": { + connected: true, + latency: 0 * time.Millisecond, + }, + "route2": { + connected: true, + latency: 10 * time.Millisecond, + }, + }, + existingRoutes: map[string]*route.Route{ + "route1": { + ID: "route1", + Metric: route.MaxMetric, + Peer: "peer1", + }, + "route2": { + ID: "route2", + Metric: route.MaxMetric, + Peer: "peer2", + }, + }, + currentRoute: "", + expectedRouteID: "route2", + }, + { + name: "current route with similar score and similar but slightly worse latency should not change", + statuses: map[string]routerPeerStatus{ + "route1": { + connected: true, + relayed: false, + direct: true, + latency: 12 * time.Millisecond, + }, + "route2": { + connected: true, + relayed: false, + direct: true, + latency: 10 * time.Millisecond, + }, + }, + existingRoutes: map[string]*route.Route{ + "route1": { + ID: "route1", + Metric: route.MaxMetric, + Peer: "peer1", + }, + "route2": { + ID: "route2", + Metric: route.MaxMetric, + Peer: "peer2", + }, + }, + currentRoute: "route1", + expectedRouteID: "route1", + }, + { + name: "current chosen route doesn't exist anymore", + statuses: map[string]routerPeerStatus{ + "route1": { + connected: true, + relayed: false, + direct: true, + latency: 20 * time.Millisecond, + }, + "route2": { + connected: true, + relayed: false, + direct: true, + latency: 10 * time.Millisecond, + }, + }, + existingRoutes: map[string]*route.Route{ + "route1": { + ID: "route1", + Metric: route.MaxMetric, + Peer: "peer1", + }, + "route2": { + ID: "route2", + Metric: route.MaxMetric, + Peer: "peer2", + }, + }, + currentRoute: "routeDoesntExistAnymore", + expectedRouteID: "route2", + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + currentRoute := &route.Route{ + ID: "routeDoesntExistAnymore", + } + if tc.currentRoute != "" { + currentRoute = tc.existingRoutes[tc.currentRoute] + } + // create new clientNetwork client := &clientNetwork{ network: netip.MustParsePrefix("192.168.0.0/24"), routes: tc.existingRoutes, - chosenRoute: tc.currentRoute, + chosenRoute: currentRoute, } chosenRoute := client.getBestRouteFromStatuses(tc.statuses)