Skip to content

Conversation

@hurricanehrndz
Copy link
Contributor

Describe your changes

Network monitor should only trigger engine restarts when route get actually changes. There are many other instances when unspecified address get added to the system without actually affecting the real default route.

** change depends on **
netbirdio/go-netroute#5

Issue ticket number and link

fixes #3352

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

@hurricanehrndz
Copy link
Contributor Author

Please do not merge until go-netroute gets updated and merged.

Copy link
Collaborator

@lixmal lixmal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/netbirdio/netbird/pull/3355/files#diff-0f6b5bd1fd7206f0c6c1450e86746586e21d8a9bf4eda7d356e65d096893ac98R61

With this change RTM_CHANGE could also make sense. Would you mind testing that with route change ...?

Can you also add some comments why we're doing this differently on bsd (multiple 0.0.0.0/0 etc)

@hurricanehrndz hurricanehrndz force-pushed the fix_networkmonitor_false_enginerestarts branch from a238f3e to 91dd571 Compare February 20, 2025 17:40
@hurricanehrndz
Copy link
Contributor Author

With the newest changes, this is the log when I pull out my usb-c wired connection

2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: default route changed, v4GatewayChanged: true v4Gateway: "invalid IP", v6GatewayChanged: true, v6Gateway: "fe80::2d0:4cff:fe10:15d2%en0", IntfV4Changed: true, v4Intf: (*net.Interface)(nil),  v6IntfChanged: true, v6Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33}
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: failed to check next hop, assuming no network connection available: route not found
2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: default route changed, v4GatewayChanged: true v4Gateway: "invalid IP", v6GatewayChanged: true, v6Gateway: "fe80::2d0:4cff:fe10:15d2%en0", IntfV4Changed: true, v4Intf: (*net.Interface)(nil),  v6IntfChanged: true, v6Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33}
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: failed to check next hop, assuming no network connection available: route not found
2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: default route changed, v4GatewayChanged: true v4Gateway: "invalid IP", v6GatewayChanged: true, v6Gateway: "fe80::2d0:4cff:fe10:15d2%en0", IntfV4Changed: true, v4Intf: (*net.Interface)(nil),  v6IntfChanged: true, v6Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33}
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: default route changed, v4GatewayChanged: false v4Gateway: "172.28.250.1", v6GatewayChanged: true, v6Gateway: "fe80::2d0:4cff:fe10:15d2%en0", IntfV4Changed: true, v4Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33},  v6IntfChanged: true, v6Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33}
2025-02-20T12:23:52-07:00       info    client/internal/networkmonitor.checkChange      Network monitor: default route changed, v4GatewayChanged: false v4Gateway: "172.28.250.1", v6GatewayChanged: true, v6Gateway: "fe80::2d0:4cff:fe10:15d2%en0", IntfV4Changed: true, v4Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33},  v6IntfChanged: true, v6Intf: &net.Interface{Index:15, MTU:1500, Name:"en0", HardwareAddr:net.HardwareAddr{0x2e, 0xcf, 0x11, 0xd3, 0x8d, 0xc3}, Flags:0x33}
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:52-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1   Network monitor: detected network change, reset debounceTimer
2025-02-20T12:23:54-07:00       info    client/internal.(*Engine).startNetworkMonitor.func1.1.1 Network monitor: detected network change, restarting engine

@hurricanehrndz
Copy link
Contributor Author

Once the go-netroute PR is merge I can update the go.mod file

Currently, check_change registers a change whenever an unspecified
address is removed or added on any interface. It should only register a
change when it is the default route that is affected by route changes.
@hurricanehrndz hurricanehrndz force-pushed the fix_networkmonitor_false_enginerestarts branch from 973363f to 0190ed4 Compare March 13, 2025 12:53
@sonarqubecloud
Copy link

@hurricanehrndz
Copy link
Contributor Author

hurricanehrndz commented Mar 13, 2025

@pappz @lixmal I have updated the PR to resolve any conflicts. Can you please review. This patch still needs this PR: netbirdio/go-netroute#5

@hurricanehrndz hurricanehrndz requested a review from lixmal March 19, 2025 01:25
…false_enginerestarts

* upstream/main: (228 commits)
  [management] add breakdown of network map calculation metrics (netbirdio#4020)
  [client] Don't open cmd.exe during MSI actions (netbirdio#4041)
  [misc] Specify netbird binary location in Dockerfiles (netbirdio#4024)
  [management] check and log on new management version (netbirdio#4029)
  [misc] add additional metrics (netbirdio#4028)
  [client] close windows when process needs to exit (netbirdio#4027)
  [client] Refactor showLoginURL to improve error handling and connection status checks (netbirdio#4026)
  [client] Handle lazy routing peers that are part of HA groups (netbirdio#3943)
  [management] add transaction for integrated validator groups update and primary account update (netbirdio#4014)
  [management] export ephemeral peer flag on api (netbirdio#4004)
  [management] Add backward compatibility for older clients without firewall rules port range support (netbirdio#4003)
  [client] Fix port range squashing (netbirdio#4007)
  [client] Add more Android advanced settings (netbirdio#4001)
  Fix route notification
  [management] Refactor routes to use store methods  (netbirdio#2928)
  [client] fix connection state handling (netbirdio#3995)
  [client] Fix logic in updateStatus to correctly handle connection state (netbirdio#3994)
  [management] Avoid recalculating next peer expiration (netbirdio#3991)
  [client] Fix DNS Interceptor Build Error (netbirdio#3993)
  [client] Tighten allowed domains for dns forwarder (netbirdio#3978)
  ...
@hurricanehrndz hurricanehrndz force-pushed the fix_networkmonitor_false_enginerestarts branch from b8eff53 to 96cd3e0 Compare June 25, 2025 19:28
@sonarqubecloud
Copy link

@hurricanehrndz
Copy link
Contributor Author

@pappz @lixmal this is ready for review, instead of patching your fork of go-netroute, I fixed upstream

@hurricanehrndz hurricanehrndz deleted the fix_networkmonitor_false_enginerestarts branch November 7, 2025 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On macOS the route monitor triggers on non gw changes

2 participants