Better interface selection heuristic #3546

dadgar · 2017-11-13T23:12:11Z

This PR introduces a better interface selection heuristic such that we
select interfaces with globally routable unicast addresses over link
local addresses.

Fixes #3487

This PR introduces a better interface selection heuristic such that we select interfaces with globally routable unicast addresses over link local addresses. Fixes #3487

schmichael

I think this heuristic will work for the linked issue, but it will break on machine which worries me about its portability. When I go run ... this script I get:

1 lo
  0 127.0.0.1/8 (*net.IPNet)
    IsGlobalUnicast          = false
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = false
    IsLoopback               = true
    IsMulticast              = false
  1 ::1/128 (*net.IPNet)
    IsGlobalUnicast          = false
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = false
    IsLoopback               = true
    IsMulticast              = false

2 enp0s31f6

3 wlp4s0
  0 192.168.86.26/24 (*net.IPNet)
    IsGlobalUnicast          = true
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = false
    IsLoopback               = false
    IsMulticast              = false
  1 fe80::d896:c428:a4f2:e480/64 (*net.IPNet)
    IsGlobalUnicast          = false
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = true
    IsLoopback               = false
    IsMulticast              = false

4 lxcbr0
  0 10.0.3.1/24 (*net.IPNet)
    IsGlobalUnicast          = true
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = false
    IsLoopback               = false
    IsMulticast              = false

5 docker0
  0 172.18.0.1/16 (*net.IPNet)
    IsGlobalUnicast          = true
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = false
    IsLoopback               = false
    IsMulticast              = false
  1 fe80::42:a4ff:fed1:1f7d/64 (*net.IPNet)
    IsGlobalUnicast          = false
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = true
    IsLoopback               = false
    IsMulticast              = false

6 vmnet1
  0 172.16.203.1/24 (*net.IPNet)
    IsGlobalUnicast          = true
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = false
    IsLoopback               = false
    IsMulticast              = false
  1 fe80::250:56ff:fec0:1/64 (*net.IPNet)
    IsGlobalUnicast          = false
    IsInterfaceLocalMulticast= false
    IsLinkLocalMulticast     = false
    IsLinkLocalUnicast       = true
    IsLoopback               = false
    IsMulticast              = false

...two more vmnet's...

wlp4s0 is my wifi which I would expect to be my "best" interface. However because it has a linklocal unicast address (due to ipv6 being enabled locally but not available on my LAN) it gets a score of 1 while lxc's bridge (lxcbr0) gets a score of 2 for having a global unicast address but no link local.

I'm not sure what we can do other than blacklist or score interfaces based on well known names or attempt to use cloud-specific metadata before scraping the system's interfaces...

schmichael · 2017-11-13T23:39:53Z

client/fingerprint/network.go

+
+		addrs, err := f.interfaceDetector.Addrs(&intf)
+		if err != nil {
+			return nil, err


🤔 We could WARN and continue here, but after peeking at the Addrs(...) code it seems like all of the errors it returns indicate pretty bad syscall failures. Seems like just bailing is probably safest as users can manually set this anyway.

preetapan · 2017-11-14T22:51:43Z

client/fingerprint/network_test.go

-func TestNetworkFingerPrint_excludelo_down_interfaces(t *testing.T) {
+// This tests that we prefer skipping link local and down interfaces as well as
+// those without global unicast addresses
+func TestNetworkFingerPrint_preferential_interfaces(t *testing.T) {


I don't see how any of these tests are exercising the new code path with the scoring introduced above.
I realize its not possible to test in a portable way across machines, but I would try to refactor the code so the scoring/selection logic is testable

preetapan · 2017-11-14T23:08:29Z

client/fingerprint/network.go

+			}
+		}
+
+		if intfScore > bestScore {


If infscore stays at zero and bestScore at -1000 this will pick the first intf from the intfs list, is that acceptable?

preetapan · 2017-11-14T23:14:35Z

client/fingerprint/network.go

@@ -200,14 +199,57 @@ func (f *NetworkFingerprint) findInterface(deviceName string) (*net.Interface, e
 		return nil, err


One of the comments on the PR indicated that the returned list here is not stable (can return things out of order). What happens if there is more than one interface with a global unicast address, thus resulting in them getting the same score?
Reading the code below, it seems like it will choose the first one, but if the order changes when calling interfaceDetector.Interfaces() that will change the results.

dadgar · 2017-11-15T19:57:35Z

@preetapan @schmichael Would appreciate a new review. Code has changed.

preetapan · 2017-11-15T20:44:16Z

website/source/docs/agent/configuration/client.html.md

+  to force network fingerprinting on. When run in dev mode, this is defaulted
+  to the machine's loopback interface. If not in dev mode, the interface attached
+  to the default route is used. All addresses on the interface are fingerprinted
+  except those that are scoped local for IPv6. When allocating ports for tasks,


suggesting a small rewrite for brevity and continuity.

When run in dev mode, this defaults to the loopback interface. When not in dev mode, the interface attached to the default route is used. All IP addresses except those scoped local for IPV6 on the chosen interface are fingerprinted. The scheduler chooses from those IP addresses when allocating ports for tasks.

preetapan · 2017-11-15T20:44:35Z

client/fingerprint/network.go

+		if defaultIfName == "" {
+			return nil, fmt.Errorf("no network_interface given and failed to determine interface attached to default route")
+		}
+		deviceName = defaultIfName


Much nicer now 👍

github-actions · 2023-03-17T02:14:14Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

dadgar force-pushed the f-heuristic branch from 0523d93 to b83565a Compare November 13, 2017 23:12

Better interface selection heuristic

35cb143

This PR introduces a better interface selection heuristic such that we select interfaces with globally routable unicast addresses over link local addresses. Fixes #3487

dadgar force-pushed the f-heuristic branch from b83565a to 35cb143 Compare November 13, 2017 23:13

dadgar mentioned this pull request Nov 13, 2017

Failed to start allocation due to binding to non-existent address #3487

Closed

schmichael reviewed Nov 14, 2017

View reviewed changes

preetapan reviewed Nov 14, 2017

View reviewed changes

Use interface attached to default route

00557af

preetapan reviewed Nov 15, 2017

View reviewed changes

preetapan approved these changes Nov 15, 2017

View reviewed changes

dadgar added 2 commits November 15, 2017 12:49

use revised wording

8ebca4b

changelog

6a434bc

dadgar merged commit 167c81a into master Nov 15, 2017

dadgar deleted the f-heuristic branch November 15, 2017 20:51

nickethier mentioned this pull request May 22, 2018

Nomad can't use in windows7/2012r2 from version 0.7.3to0.8.3 #4309

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better interface selection heuristic #3546

Better interface selection heuristic #3546

dadgar commented Nov 13, 2017

schmichael left a comment

schmichael Nov 13, 2017

preetapan Nov 14, 2017

preetapan Nov 14, 2017 •

edited

Loading

preetapan Nov 14, 2017

dadgar commented Nov 15, 2017

preetapan Nov 15, 2017 •

edited

Loading

preetapan Nov 15, 2017

github-actions bot commented Mar 17, 2023

		@@ -200,14 +199,57 @@ func (f NetworkFingerprint) findInterface(deviceName string) (net.Interface, e
		return nil, err

Better interface selection heuristic #3546

Better interface selection heuristic #3546

Conversation

dadgar commented Nov 13, 2017

schmichael left a comment

Choose a reason for hiding this comment

schmichael Nov 13, 2017

Choose a reason for hiding this comment

preetapan Nov 14, 2017

Choose a reason for hiding this comment

preetapan Nov 14, 2017 • edited Loading

Choose a reason for hiding this comment

preetapan Nov 14, 2017

Choose a reason for hiding this comment

dadgar commented Nov 15, 2017

preetapan Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

preetapan Nov 15, 2017

Choose a reason for hiding this comment

github-actions bot commented Mar 17, 2023

preetapan Nov 14, 2017 •

edited

Loading

preetapan Nov 15, 2017 •

edited

Loading