Don't try to bind on address from inactive interface #948
Conversation
|
Will probably resolve #928 as well. |
|
We seem to rely on resolving loopback (or an interface without multicast or broadcast) on Tarvis, maybe we want to bind to loopback if nothing else can be found. |
| return nil, fmt.Errorf("Failed to get interfaces: %v", err) | ||
| } | ||
|
|
||
| reqFlags := net.FlagUp | net.FlagBroadcast | net.FlagMulticast |
There was a problem hiding this comment.
We don't actually require Broadcast or Multicast support. Any reason to check for it? Also how does this method affect Windows?
There was a problem hiding this comment.
Reasoning for filtering on broadcast and multicast was to filter out loopback and other dummy interfaces, could change that to be a explicit filter on FlagLoopback instead.
Windows support all these flags, see https://golang.org/src/net/interface_windows.go.
|
Alright, we will now only require interfaces to be up and use loopback if nothing else can be found. |
|
@tiwilliam My only concern now is Windows. I have a suspicion it will not play nice... |
|
Alright, let me find some testing units and play around. |
|
Someone with easy access to Windows, feel free to test this out. I've not gathered energy enough for installing Windows in my VM. |
|
Works fine for me on Windows: This was with 2 virtualbox adapters (enabled) and my LAN adapter disabled. |
|
I get this error on Windows if I try to bind with all 3 adapters disabled: That makes sense though - there are NO interfaces at that point, and there is not a dedicated "loopback interface" in Windows so this makes sense. |
|
@highlyunavailable Thank you! I wonder if it works with the Windows Loopback Adapter installed. |
|
No, it fails with However, this is because privateBlocks does not contain 169.254.0.0/16, and that's what the loopback adapter binds to since it can't find another IP. Interestingly, that file also does not contain 127.0.0.0/8, so you might run into a problem there. |
|
@highlyunavailable Good catch, I've added I guess you can change address of your Windows Loopback Adapter manually to 127.0.0.1 if you really want that? |
|
You can't, because 127.0.0.1 would get added as part of the adapter. I strongly suggest adding 169.254.0.0/16 anyway, since it is defined as a private range in RFC 3927. |
|
Alright, makes sense. My only concern would be that we now might end up using a non-working DHCP interface if that's first in the interface list. |
|
True. I'd say if you're going to support 169.254 then it should be the absolute 100% last thing to bind to - all other non-loopback adapters should be tried first. |
|
I'm happy with current state of this now when it's tested on Windows. A final sign-off before merge would be appreciated. |
tiwilliam
force-pushed
the
iface-down-fix
branch
2 times, most recently
from
10f5f53 to
adf8d07
Compare
tiwilliam
force-pushed
the
iface-down-fix
branch
from
aea472e to
54ea982
Compare
|
It doesn't look like the 169.254 case is a problem since Consul will bail out if it is presented with multiple private IPs. This could only be a problem if the machine a. has 1 APIPA-addressed DHCP adapter b. public IP adapter(s) - this would mean that consul would try to auto-bind to the APIPA adapter, but that's already a thing - Consul won't bind to a public IP. I'm fine with how it works in Windows right now as well. if @armon approves it would be cool to get this merged - there are multiple open bugs that this would fix it looks like: #934, #928. |
|
Sorry for the delay on this one - looks good! |
Don't try to bind on address from inactive interface
So tests don't show ``` Received Transferred Average Speed Time Time Time Current Dload Upload Total Spent Left Speed ```
Build our own network address list, we now make sure the interface is up and supports both broadcast and multicast. Resolves #934.