ip route conflicts and race conditions when resolving best routes in clientNetwork.getBestRouteFromStatuses()

[nazarewk]

Describe the problem

I have noticed the client-side route selection feature so during today's Netbird workshop for our team I have enabled both routes clashing on 10.4/16 and later today teammates started reporting problems with accessing the primary system so I switched the secondary off and dug into the code.

First thing I have noticed using ip route metric was superseded by:

• best peer selection algorithm, which takes Route.Metric as the most significant scoring,
• using dedicated routing table 7120 aka netbird (the one I discovered in Make multiple Netbird client instances cooperate on the ip route table level on Linux (possibly other OS too?) #2023 )
• not providing a metric parameter to created ip route at all

The method is scoped to *clientNetwork, which I guess does not consider a possibility of another NetworkID providing the same CIDR (10.4/16 in our case) resulting in a conflict/race condition on the created ip route entry.

To Reproduce

Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior

Either of:

• only the best/highest metric route across all networks is selected
• create the ip route with the metric argument (seems to be netlink.Route.Priority) which would properly prioritize routes on the ip route level even in case of a clash
  • watch our not to deregister the wrong route during updates

Are you using NetBird Cloud?

Yes

NetBird version

0.27.7

NetBird status -d output:

If applicable, add the `netbird status -d' command output.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

[nazarewk]

I've noticed that I was connected to the primary network and the ip route entry disappeared completely after i disabled the secondary network in the web ui.

I had to do netbird down && netbird up for the client to re-register the primary route.

[nazarewk]

a single clientNetwork seems to represent a very specific <network-id>-<network-range> grouping:

1. netbird/client/internal/routemanager/manager.go

   Line 46 in f176807

   clientNetworks map[route.HAUniqueID]*clientNetwork

2. netbird/route/hauniqueid.go

   Lines 8 to 10 in f176807

   	func GetHAUniqueID(input *Route) HAUniqueID {
   	return HAUniqueID(string(input.NetID) + "-" + input.Network.String())
   	}

making it impossible to know about other input.NetID providing the same input.Network.String() on completely different site/datacenter

[nazarewk]

theoretically using input.Network.String() instead of HAUniqueID here could help with my issue:

netbird/client/internal/routemanager/manager.go

Lines 243 to 251 in f176807

	for _, newRoute := range newRoutes {
	haID := route.GetHAUniqueID(newRoute)
	if !ownNetworkIDs[haID] {
	if !isPrefixSupported(newRoute.Network) {
	continue
	}
	newClientRoutesIDMap[haID] = append(newClientRoutesIDMap[haID], newRoute)
	}
	}

but I am not sure about the wider consequences

[nazarewk]

I've noticed that I was connected to the primary network and the ip route entry disappeared completely after i disabled the secondary network in the web ui.

I had to do netbird down && netbird up for the client to re-register the primary route.

@mlsmaycon noted that this part of the issue (removing routes still in use) will be fixed by #1943 , since it is a big refactor of (previously) linked code in this issue it will be worth analyzing and revisiting again after #1943 is merged