[client] Ignore irrelevant route changes to tracked network monitor routes (#3796)

client/internal/networkmonitor/check_change_bsd.go

@@ -19,7 +19,7 @@ import (
 func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop) error {
 	fd, err := unix.Socket(syscall.AF_ROUTE, syscall.SOCK_RAW, syscall.AF_UNSPEC)
 	if err != nil {
-		return fmt.Errorf("failed to open routing socket: %v", err)
+		return fmt.Errorf("open routing socket: %v", err)
 	}
 	defer func() {
 		err := unix.Close(fd)

client/internal/networkmonitor/check_change_windows.go

@@ -13,7 +13,7 @@ import (
 func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop) error {
 	routeMonitor, err := systemops.NewRouteMonitor(ctx)
 	if err != nil {
-		return fmt.Errorf("failed to create route monitor: %w", err)
+		return fmt.Errorf("create route monitor: %w", err)
 	}
 	defer func() {
 		if err := routeMonitor.Stop(); err != nil {
@@ -38,35 +38,49 @@ func checkChange(ctx context.Context, nexthopv4, nexthopv6 systemops.Nexthop) er
 }
 
 func routeChanged(route systemops.RouteUpdate, nexthopv4, nexthopv6 systemops.Nexthop) bool {
-	intf := "<nil>"
+	if intf := route.NextHop.Intf; intf != nil && isSoftInterface(intf.Name) {
-	if route.Interface != nil {
+		log.Debugf("Network monitor: ignoring default route change for next hop with soft interface %s", route.NextHop)
-		intf = route.Interface.Name
-		if isSoftInterface(intf) {
-			log.Debugf("Network monitor: ignoring default route change for soft interface %s", intf)
 		return false
 	}
+
+	// TODO: for the empty nexthop ip (on-link), determine the family differently
+	nexthop := nexthopv4
+	if route.NextHop.IP.Is6() {
+		nexthop = nexthopv6
 	}
 
 	switch route.Type {
-	case systemops.RouteModified:
+	case systemops.RouteModified, systemops.RouteAdded:
-		// TODO: get routing table to figure out if our route is affected for modified routes
+		return handleRouteAddedOrModified(route, nexthop)
-		log.Infof("Network monitor: default route changed: via %s, interface %s", route.NextHop, intf)
-		return true
-	case systemops.RouteAdded:
-		if route.NextHop.Is4() && route.NextHop != nexthopv4.IP || route.NextHop.Is6() && route.NextHop != nexthopv6.IP {
-			log.Infof("Network monitor: default route added: via %s, interface %s", route.NextHop, intf)
-			return true
-		}
 	case systemops.RouteDeleted:
-		if nexthopv4.Intf != nil && route.NextHop == nexthopv4.IP || nexthopv6.Intf != nil && route.NextHop == nexthopv6.IP {
+		return handleRouteDeleted(route, nexthop)
-			log.Infof("Network monitor: default route removed: via %s, interface %s", route.NextHop, intf)
-			return true
-		}
 	}
 
 	return false
 }
 
+func handleRouteAddedOrModified(route systemops.RouteUpdate, nexthop systemops.Nexthop) bool {
+	// For added/modified routes, we care about different next hops
+	if !nexthop.Equal(route.NextHop) {
+		action := "changed"
+		if route.Type == systemops.RouteAdded {
+			action = "added"
+		}
+		log.Infof("Network monitor: default route %s: via %s", action, route.NextHop)
+		return true
+	}
+	return false
+}
+
+func handleRouteDeleted(route systemops.RouteUpdate, nexthop systemops.Nexthop) bool {
+	// For deleted routes, we care about our tracked next hop being deleted
+	if nexthop.Equal(route.NextHop) {
+		log.Infof("Network monitor: default route removed: via %s", route.NextHop)
+		return true
+	}
+	return false
+}
+
 func isSoftInterface(name string) bool {
 	return strings.Contains(strings.ToLower(name), "isatap") || strings.Contains(strings.ToLower(name), "teredo")
 }

client/internal/networkmonitor/check_change_windows_test.go

@@ -0,0 +1,404 @@
+package networkmonitor
+
+import (
+	"net"
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/netbirdio/netbird/client/internal/routemanager/systemops"
+)
+
+func TestRouteChanged(t *testing.T) {
+	tests := []struct {
+		name      string
+		route     systemops.RouteUpdate
+		nexthopv4 systemops.Nexthop
+		nexthopv6 systemops.Nexthop
+		expected  bool
+	}{
+		{
+			name: "soft interface should be ignored",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Name: "ISATAP-Interface", // isSoftInterface checks name
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.2"),
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: false,
+		},
+		{
+			name: "modified route with different v4 nexthop IP should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.2"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: true,
+		},
+		{
+			name: "modified route with same v4 nexthop (IP and Intf Index) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: false,
+		},
+		{
+			name: "added route with different v6 nexthop IP should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteAdded,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("2001:db8::2"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "added route with same v6 nexthop (IP and Intf Index) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteAdded,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "deleted route matching tracked v4 nexthop (IP and Intf Index) should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: true,
+		},
+		{
+			name: "deleted route not matching tracked v4 nexthop (different IP) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP: netip.MustParseAddr("192.168.1.3"), // Different IP
+					Intf: &net.Interface{
+						Index: 1, Name: "eth0",
+					},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP: netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{
+					Index: 1, Name: "eth0",
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP: netip.MustParseAddr("2001:db8::1"),
+			},
+			expected: false,
+		},
+		{
+			name: "modified v4 route with same IP, different Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "modified v4 route with same IP, one Intf nil, other non-nil should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: nil, // Intf is nil
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"}, // Tracked Intf is not nil
+			},
+			expected: true,
+		},
+		{
+			name: "added v4 route with same IP, different Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteAdded,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "deleted v4 route with same IP, different Intf Index should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{ // This is the route being deleted
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv4: systemops.Nexthop{ // This is our tracked nexthop
+				IP:   netip.MustParseAddr("192.168.1.1"),
+				Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+			},
+			expected: false, // Because nexthopv4.Equal(route.NextHop) will be false
+		},
+		{
+			name: "modified v6 route with different IP, same Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::3"), // Different IP
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "modified v6 route with same IP, different Intf Index should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "modified v6 route with same IP, same Intf Index should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteModified,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: false,
+		},
+		{
+			name: "deleted v6 route matching tracked nexthop (IP and Intf Index) should return true",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: true,
+		},
+		{
+			name: "deleted v6 route not matching tracked nexthop (different IP) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("2001:db8::3"), // Different IP
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: false,
+		},
+		{
+			name: "deleted v6 route not matching tracked nexthop (same IP, different Intf Index) should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteDeleted,
+				Destination: netip.PrefixFrom(netip.IPv6Unspecified(), 0),
+				NextHop: systemops.Nexthop{ // This is the route being deleted
+					IP:   netip.MustParseAddr("2001:db8::1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv6: systemops.Nexthop{ // This is our tracked nexthop
+				IP:   netip.MustParseAddr("2001:db8::1"),
+				Intf: &net.Interface{Index: 2, Name: "eth1"}, // Different Intf Index
+			},
+			expected: false,
+		},
+		{
+			name: "unknown route type should return false",
+			route: systemops.RouteUpdate{
+				Type:        systemops.RouteUpdateType(99), // Unknown type
+				Destination: netip.PrefixFrom(netip.IPv4Unspecified(), 0),
+				NextHop: systemops.Nexthop{
+					IP:   netip.MustParseAddr("192.168.1.1"),
+					Intf: &net.Interface{Index: 1, Name: "eth0"},
+				},
+			},
+			nexthopv4: systemops.Nexthop{
+				IP:   netip.MustParseAddr("192.168.1.2"), // Different from route.NextHop
+				Intf: &net.Interface{Index: 1, Name: "eth0"},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := routeChanged(tt.route, tt.nexthopv4, tt.nexthopv6)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestIsSoftInterface(t *testing.T) {
+	tests := []struct {
+		name     string
+		ifname   string
+		expected bool
+	}{
+		{
+			name:     "ISATAP interface should be detected",
+			ifname:   "ISATAP tunnel adapter",
+			expected: true,
+		},
+		{
+			name:     "lowercase soft interface should be detected",
+			ifname:   "isatap.{14A5CF17-CA72-43EC-B4EA-B4B093641B7D}",
+			expected: true,
+		},
+		{
+			name:     "Teredo interface should be detected",
+			ifname:   "Teredo Tunneling Pseudo-Interface",
+			expected: true,
+		},
+		{
+			name:     "regular interface should not be detected as soft",
+			ifname:   "eth0",
+			expected: false,
+		},
+		{
+			name:     "another regular interface should not be detected as soft",
+			ifname:   "wlan0",
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := isSoftInterface(tt.ifname)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

client/internal/networkmonitor/monitor.go

@@ -118,9 +118,12 @@ func (nw *NetworkMonitor) Stop() {
 }
 
 func (nw *NetworkMonitor) checkChanges(ctx context.Context, event chan struct{}, nexthop4 systemops.Nexthop, nexthop6 systemops.Nexthop) {
+	defer close(event)
 	for {
 		if err := checkChangeFn(ctx, nexthop4, nexthop6); err != nil {
-			close(event)
+			if !errors.Is(err, context.Canceled) {
+				log.Errorf("Network monitor: failed to check for changes: %v", err)
+			}
 			return
 		}
 		// prevent blocking

client/internal/routemanager/systemops/systemops.go

@@ -1,6 +1,7 @@
 package systemops
 
 import (
+	"fmt"
 	"net"
 	"net/netip"
 	"sync"
@@ -15,6 +16,20 @@ type Nexthop struct {
 	Intf *net.Interface
 }
 
+// Equal checks if two nexthops are equal.
+func (n Nexthop) Equal(other Nexthop) bool {
+	return n.IP == other.IP && (n.Intf == nil && other.Intf == nil ||
+		n.Intf != nil && other.Intf != nil && n.Intf.Index == other.Intf.Index)
+}
+
+// String returns a string representation of the nexthop.
+func (n Nexthop) String() string {
+	if n.Intf == nil {
+		return n.IP.String()
+	}
+	return fmt.Sprintf("%s @ %d (%s)", n.IP.String(), n.Intf.Index, n.Intf.Name)
+}
+
 type ExclusionCounter = refcounter.Counter[netip.Prefix, struct{}, Nexthop]
 
 type SysOps struct {

client/internal/routemanager/systemops/systemops_windows.go

@@ -33,8 +33,7 @@ type RouteUpdateType int
 type RouteUpdate struct {
 	Type        RouteUpdateType
 	Destination netip.Prefix
-	NextHop     netip.Addr
+	NextHop     Nexthop
-	Interface   *net.Interface
 }
 
 // RouteMonitor provides a way to monitor changes in the routing table.
@@ -231,15 +230,15 @@ func (rm *RouteMonitor) parseUpdate(row *MIB_IPFORWARD_ROW2, notificationType MI
 		intf, err := net.InterfaceByIndex(idx)
 		if err != nil {
 			log.Warnf("failed to get interface name for index %d: %v", idx, err)
-			update.Interface = &net.Interface{
+			update.NextHop.Intf = &net.Interface{
 				Index: idx,
 			}
 		} else {
-			update.Interface = intf
+			update.NextHop.Intf = intf
 		}
 	}
 
-	log.Tracef("Received route update with destination %v, next hop %v, interface %v", row.DestinationPrefix, row.NextHop, update.Interface)
+	log.Tracef("Received route update with destination %v, next hop %v, interface %v", row.DestinationPrefix, row.NextHop, update.NextHop.Intf)
 	dest := parseIPPrefix(row.DestinationPrefix, idx)
 	if !dest.Addr().IsValid() {
 		return RouteUpdate{}, fmt.Errorf("invalid destination: %v", row)
@@ -262,7 +261,7 @@ func (rm *RouteMonitor) parseUpdate(row *MIB_IPFORWARD_ROW2, notificationType MI
 
 	update.Type = updateType
 	update.Destination = dest
-	update.NextHop = nexthop
+	update.NextHop.IP = nexthop
 
 	return update, nil
 }