Revert "Rollback new routing functionality (#1805)" (#1813)

This reverts commit 9f32ccd.

.github/workflows/golang-test-darwin.yml

@@ -32,6 +32,9 @@ jobs:
           restore-keys: |
             macos-go-
 
+      - name: Install libpcap
+        run: brew install libpcap
+
       - name: Install modules
         run: go mod tidy
 

.github/workflows/golang-test-linux.yml

@@ -36,7 +36,11 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Install dependencies
-        run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev gcc-multilib
+        run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev gcc-multilib libpcap-dev
+
+      - name: Install 32-bit libpcap
+        if: matrix.arch == '386'
+        run: sudo dpkg --add-architecture i386 && sudo apt update && sudo apt-get install -y libpcap0.8-dev:i386
 
       - name: Install modules
         run: go mod tidy
@@ -67,7 +71,7 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Install dependencies
-        run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev gcc-multilib
+        run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev gcc-multilib libpcap-dev
 
       - name: Install modules
         run: go mod tidy
@@ -82,7 +86,7 @@ jobs:
         run: CGO_ENABLED=0 go test -c -o sharedsock-testing.bin ./sharedsock
 
       - name: Generate RouteManager Test bin
-        run: CGO_ENABLED=0 go test -c -o routemanager-testing.bin ./client/internal/routemanager/...
+        run: CGO_ENABLED=1 go test -c -o routemanager-testing.bin -tags netgo -ldflags '-w -extldflags "-static -ldbus-1 -lpcap"' ./client/internal/routemanager/...
 
       - name: Generate nftables Manager Test bin
         run: CGO_ENABLED=0 go test -c -o nftablesmanager-testing.bin ./client/firewall/nftables/...

.github/workflows/golang-test-windows.yml

@@ -46,7 +46,7 @@ jobs:
       - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe env -w GOCACHE=C:\Users\runneradmin\AppData\Local\go-build
 
       - name: test
-        run: PsExec64 -s -w ${{ github.workspace }} cmd.exe /c "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe test -timeout 5m -p 1 ./... > test-out.txt 2>&1"
+        run: PsExec64 -s -w ${{ github.workspace }} cmd.exe /c "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe test -timeout 10m -p 1 ./... > test-out.txt 2>&1"
       - name: test output
         if: ${{ always() }}
         run: Get-Content test-out.txt

.github/workflows/golangci-lint.yml

@@ -40,7 +40,7 @@ jobs:
           cache: false
       - name: Install dependencies
         if: matrix.os == 'ubuntu-latest'
-        run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev
+        run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev libpcap-dev
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:

client/internal/engine.go

@@ -94,6 +94,9 @@ type Engine struct {
 	// peerConns is a map that holds all the peers that are known to this peer
 	peerConns map[string]*peer.Conn
 
+	beforePeerHook peer.BeforeAddPeerHookFunc
+	afterPeerHook  peer.AfterRemovePeerHookFunc
+
 	// rpManager is a Rosenpass manager
 	rpManager *rosenpass.Manager
 
@@ -261,6 +264,14 @@ func (e *Engine) Start() error {
 	e.dnsServer = dnsServer
 
 	e.routeManager = routemanager.NewManager(e.ctx, e.config.WgPrivateKey.PublicKey().String(), e.wgInterface, e.statusRecorder, initialRoutes)
+	beforePeerHook, afterPeerHook, err := e.routeManager.Init()
+	if err != nil {
+		log.Errorf("Failed to initialize route manager: %s", err)
+	} else {
+		e.beforePeerHook = beforePeerHook
+		e.afterPeerHook = afterPeerHook
+	}
+
 	e.routeManager.SetRouteChangeListener(e.mobileDep.NetworkChangeListener)
 
 	err = e.wgInterfaceCreate()
@@ -810,6 +821,11 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error {
 		}
 		e.peerConns[peerKey] = conn
 
+		if e.beforePeerHook != nil && e.afterPeerHook != nil {
+			conn.AddBeforeAddPeerHook(e.beforePeerHook)
+			conn.AddAfterRemovePeerHook(e.afterPeerHook)
+		}
+
 		err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn)
 		if err != nil {
 			log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err)

client/internal/peer/conn.go

@@ -20,6 +20,7 @@ import (
 	"github.com/netbirdio/netbird/iface/bind"
 	signal "github.com/netbirdio/netbird/signal/client"
 	sProto "github.com/netbirdio/netbird/signal/proto"
+	nbnet "github.com/netbirdio/netbird/util/net"
 	"github.com/netbirdio/netbird/version"
 )
 
@@ -100,6 +101,9 @@ type IceCredentials struct {
 	Pwd   string
 }
 
+type BeforeAddPeerHookFunc func(connID nbnet.ConnectionID, IP net.IP) error
+type AfterRemovePeerHookFunc func(connID nbnet.ConnectionID) error
+
 type Conn struct {
 	config ConnConfig
 	mu     sync.Mutex
@@ -138,6 +142,10 @@ type Conn struct {
 
 	remoteEndpoint *net.UDPAddr
 	remoteConn     *ice.Conn
+
+	connID               nbnet.ConnectionID
+	beforeAddPeerHooks   []BeforeAddPeerHookFunc
+	afterRemovePeerHooks []AfterRemovePeerHookFunc
 }
 
 // meta holds meta information about a connection
@@ -393,6 +401,14 @@ func isRelayCandidate(candidate ice.Candidate) bool {
 	return candidate.Type() == ice.CandidateTypeRelay
 }
 
+func (conn *Conn) AddBeforeAddPeerHook(hook BeforeAddPeerHookFunc) {
+	conn.beforeAddPeerHooks = append(conn.beforeAddPeerHooks, hook)
+}
+
+func (conn *Conn) AddAfterRemovePeerHook(hook AfterRemovePeerHookFunc) {
+	conn.afterRemovePeerHooks = append(conn.afterRemovePeerHooks, hook)
+}
+
 // configureConnection starts proxying traffic from/to local Wireguard and sets connection status to StatusConnected
 func (conn *Conn) configureConnection(remoteConn net.Conn, remoteWgPort int, remoteRosenpassPubKey []byte, remoteRosenpassAddr string) (net.Addr, error) {
 	conn.mu.Lock()
@@ -421,6 +437,13 @@ func (conn *Conn) configureConnection(remoteConn net.Conn, remoteWgPort int, rem
 	conn.remoteEndpoint = endpointUdpAddr
 	log.Debugf("Conn resolved IP for %s: %s", endpoint, endpointUdpAddr.IP)
 
+	conn.connID = nbnet.GenerateConnID()
+	for _, hook := range conn.beforeAddPeerHooks {
+		if err := hook(conn.connID, endpointUdpAddr.IP); err != nil {
+			log.Errorf("Before add peer hook failed: %v", err)
+		}
+	}
+
 	err = conn.config.WgConfig.WgInterface.UpdatePeer(conn.config.WgConfig.RemoteKey, conn.config.WgConfig.AllowedIps, defaultWgKeepAlive, endpointUdpAddr, conn.config.WgConfig.PreSharedKey)
 	if err != nil {
 		if conn.wgProxy != nil {
@@ -511,6 +534,15 @@ func (conn *Conn) cleanup() error {
 	// todo: is it problem if we try to remove a peer what is never existed?
 	err3 = conn.config.WgConfig.WgInterface.RemovePeer(conn.config.WgConfig.RemoteKey)
 
+	if conn.connID != "" {
+		for _, hook := range conn.afterRemovePeerHooks {
+			if err := hook(conn.connID); err != nil {
+				log.Errorf("After remove peer hook failed: %v", err)
+			}
+		}
+	}
+	conn.connID = ""
+
 	if conn.notifyDisconnected != nil {
 		conn.notifyDisconnected()
 		conn.notifyDisconnected = nil

client/internal/relay/relay.go

@@ -12,6 +12,7 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/netbirdio/netbird/client/internal/stdnet"
+	nbnet "github.com/netbirdio/netbird/util/net"
 )
 
 // ProbeResult holds the info about the result of a relay probe request
@@ -95,15 +96,13 @@ func ProbeTURN(ctx context.Context, uri *stun.URI) (addr string, probeErr error)
 	switch uri.Proto {
 	case stun.ProtoTypeUDP:
 		var err error
-		listener := &net.ListenConfig{}
-		conn, err = listener.ListenPacket(ctx, "udp", "")
+		conn, err = nbnet.NewListener().ListenPacket(ctx, "udp", "")
 		if err != nil {
 			probeErr = fmt.Errorf("listen: %w", err)
 			return
 		}
 	case stun.ProtoTypeTCP:
-		dialer := &net.Dialer{}
-		tcpConn, err := dialer.DialContext(ctx, "tcp", turnServerAddr)
+		tcpConn, err := nbnet.NewDialer().DialContext(ctx, "tcp", turnServerAddr)
 		if err != nil {
 			probeErr = fmt.Errorf("dial: %w", err)
 			return

client/internal/routemanager/client.go

@@ -41,6 +41,7 @@ type clientNetwork struct {
 
 func newClientNetworkWatcher(ctx context.Context, wgInterface *iface.WGIface, statusRecorder *peer.Status, network netip.Prefix) *clientNetwork {
 	ctx, cancel := context.WithCancel(ctx)
+
 	client := &clientNetwork{
 		ctx:                 ctx,
 		stop:                cancel,
@@ -72,6 +73,18 @@ func (c *clientNetwork) getRouterPeerStatuses() map[string]routerPeerStatus {
 	return routePeerStatuses
 }
 
+// getBestRouteFromStatuses determines the most optimal route from the available routes
+// within a clientNetwork, taking into account peer connection status, route metrics, and
+// preference for non-relayed and direct connections.
+//
+// It follows these prioritization rules:
+// * Connected peers: Only routes with connected peers are considered.
+// * Metric: Routes with lower metrics (better) are prioritized.
+// * 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.
+//
+// It returns the ID of the selected optimal route.
 func (c *clientNetwork) getBestRouteFromStatuses(routePeerStatuses map[string]routerPeerStatus) string {
 	chosen := ""
 	chosenScore := 0
@@ -158,7 +171,7 @@ func (c *clientNetwork) startPeersStatusChangeWatcher() {
 func (c *clientNetwork) removeRouteFromWireguardPeer(peerKey string) error {
 	state, err := c.statusRecorder.GetPeer(peerKey)
 	if err != nil {
-		return err
+		return fmt.Errorf("get peer state: %v", err)
 	}
 
 	delete(state.Routes, c.network.String())
@@ -172,59 +185,56 @@ func (c *clientNetwork) removeRouteFromWireguardPeer(peerKey string) error {
 
 	err = c.wgInterface.RemoveAllowedIP(peerKey, c.network.String())
 	if err != nil {
-		return fmt.Errorf("couldn't remove allowed IP %s removed for peer %s, err: %v",
+		return fmt.Errorf("remove allowed IP %s removed for peer %s, err: %v",
 			c.network, c.chosenRoute.Peer, err)
 	}
 	return nil
 }
 
 func (c *clientNetwork) removeRouteFromPeerAndSystem() error {
 	if c.chosenRoute != nil {
-		err := c.removeRouteFromWireguardPeer(c.chosenRoute.Peer)
-		if err != nil {
-			return err
+		if err := removeVPNRoute(c.network, c.wgInterface.Name()); err != nil {
+			return fmt.Errorf("remove route %s from system, err: %v", c.network, err)
 		}
-		err = removeFromRouteTableIfNonSystem(c.network, c.wgInterface.Address().IP.String())
-		if err != nil {
-			return fmt.Errorf("couldn't remove route %s from system, err: %v",
-				c.network, err)
+
+		if err := c.removeRouteFromWireguardPeer(c.chosenRoute.Peer); err != nil {
+			return fmt.Errorf("remove route: %v", err)
 		}
 	}
 	return nil
 }
 
 func (c *clientNetwork) recalculateRouteAndUpdatePeerAndSystem() error {
-
-	var err error
-
 	routerPeerStatuses := c.getRouterPeerStatuses()
 
 	chosen := c.getBestRouteFromStatuses(routerPeerStatuses)
+
+	// If no route is chosen, remove the route from the peer and system
 	if chosen == "" {
-		err = c.removeRouteFromPeerAndSystem()
-		if err != nil {
-			return err
+		if err := c.removeRouteFromPeerAndSystem(); err != nil {
+			return fmt.Errorf("remove route from peer and system: %v", err)
 		}
 
 		c.chosenRoute = nil
 
 		return nil
 	}
 
+	// If the chosen route is the same as the current route, do nothing
 	if c.chosenRoute != nil && c.chosenRoute.ID == chosen {
 		if c.chosenRoute.IsEqual(c.routes[chosen]) {
 			return nil
 		}
 	}
 
 	if c.chosenRoute != nil {
-		err = c.removeRouteFromWireguardPeer(c.chosenRoute.Peer)
-		if err != nil {
-			return err
+		// If a previous route exists, remove it from the peer
+		if err := c.removeRouteFromWireguardPeer(c.chosenRoute.Peer); err != nil {
+			return fmt.Errorf("remove route from peer: %v", err)
 		}
 	} else {
-		err = addToRouteTableIfNoExists(c.network, c.wgInterface.Address().IP.String())
-		if err != nil {
+		// otherwise add the route to the system
+		if err := addVPNRoute(c.network, c.wgInterface.Name()); err != nil {
 			return fmt.Errorf("route %s couldn't be added for peer %s, err: %v",
 				c.network.String(), c.wgInterface.Address().IP.String(), err)
 		}
@@ -245,8 +255,7 @@ func (c *clientNetwork) recalculateRouteAndUpdatePeerAndSystem() error {
 		}
 	}
 
-	err = c.wgInterface.AddAllowedIP(c.chosenRoute.Peer, c.network.String())
-	if err != nil {
+	if err := c.wgInterface.AddAllowedIP(c.chosenRoute.Peer, c.network.String()); err != nil {
 		log.Errorf("couldn't add allowed IP %s added for peer %s, err: %v",
 			c.network, c.chosenRoute.Peer, err)
 	}
@@ -287,29 +296,29 @@ func (c *clientNetwork) peersStateAndUpdateWatcher() {
 			log.Debugf("stopping watcher for network %s", c.network)
 			err := c.removeRouteFromPeerAndSystem()
 			if err != nil {
-				log.Error(err)
+				log.Errorf("Couldn't remove route from peer and system for network %s: %v", c.network, err)
 			}
 			return
 		case <-c.peerStateUpdate:
 			err := c.recalculateRouteAndUpdatePeerAndSystem()
 			if err != nil {
-				log.Error(err)
+				log.Errorf("Couldn't recalculate route and update peer and system: %v", err)
 			}
 		case update := <-c.routeUpdate:
 			if update.updateSerial < c.updateSerial {
-				log.Warnf("received a routes update with smaller serial number, ignoring it")
+				log.Warnf("Received a routes update with smaller serial number, ignoring it")
 				continue
 			}
 
-			log.Debugf("received a new client network route update for %s", c.network)
+			log.Debugf("Received a new client network route update for %s", c.network)
 
 			c.handleUpdate(update)
 
 			c.updateSerial = update.updateSerial
 
 			err := c.recalculateRouteAndUpdatePeerAndSystem()
 			if err != nil {
-				log.Error(err)
+				log.Errorf("Couldn't recalculate route and update peer and system for network %s: %v", c.network, err)
 			}
 
 			c.startPeersStatusChangeWatcher()