New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARP/NDP should work correctly on machines with multiple interfaces #165

Closed
danderson opened this Issue Feb 18, 2018 · 3 comments

Comments

@danderson
Copy link
Member

danderson commented Feb 18, 2018

Is this a bug report or a feature request?:

Feature request.

What happened:

ARP speaker right now assumes that there's only one interface on the worker machines, and "locks" itself to the MAC address of the first interface it finds.

This doesn't work right for machines with multiple interfaces, where the allocated CIDRs may belong to different interfaces.

What you expected to happen:

The ARP speaker should just listen for ARP traffic on all interfaces, and respond to relevant queries on the correct interface (i.e. the interface that has an address/cidr compatible with the requested IP).

How to reproduce it (as minimally and precisely as possible):

Set up a cluster that straddles 2 networks, 192.168.42.0/24 and 192.168.200.0/24. Configure 2 ARP pools, for 192.168.42.128/25 and 192.168.200.128/25. Configure services in both pools. Try to use them. One of the two pools will never get any responses.

@danderson danderson added this to To Do in Layer 2 mode via automation Feb 18, 2018

@danderson

This comment has been minimized.

Copy link
Member

danderson commented Feb 18, 2018

Most of the changes for this would be in the internal/arp package. Currently, the ARP announcer is constructed by giving it a single net.Interface, and the announcer listens only on that interface.

I think what should happen instead is that the arp package should internally look up all interfaces, and create a listener for each "real" interface (i.e. interfaces that have "global" unicast IPs on them - in the parlance of go's net package, RFC1918 space is global as well).

Then the announcer will need one goroutine per listener, so that it can monitor all the interfaces at once. The announcer code already has a mutex protecting the "should we respond to this IP" code, so fortunately the concurrency headaches are already solved.

So, then, each goroutine just needs to do the same readPacket loop that already exists, but additionally needs to check if the IP in the ARP request is contained within the configured subnet for the current interface (e.g. "is 10.1.2.3 in 10.1.2.0/24 ? Yes, that means I should respond").

The tests probably also need adjusting - for testing purposes only, we may want to preserve a way to only run the announcer on specific interfaces, I'm not sure.

@danderson

This comment has been minimized.

Copy link
Member

danderson commented Feb 18, 2018

Oh, a bit more direction: internal/arp is being used from speaker/main.go, in the newController func. That's what initializes the ARP announcer. If you follow the code from there, you'll find the logic that decides which single interface to use in the current implementation.

@aphistic expressed interest in working on this. /cc @miekg @mdlayher FYI.

@miekg

This comment has been minimized.

Copy link
Collaborator

miekg commented Feb 19, 2018

sgtm, happy to review any PRs

@danderson danderson added this to To do in Work better with multiple interfaces via automation Mar 4, 2018

@danderson danderson added this to the v0.5.0 milestone Mar 4, 2018

@danderson danderson changed the title ARP should work correctly on machines with multiple interfaces ARP/NDP should work correctly on machines with multiple interfaces Mar 5, 2018

@danderson danderson self-assigned this Mar 5, 2018

@danderson danderson modified the milestones: v0.5.0, v0.6.0 Apr 1, 2018

@danderson danderson assigned danderson and unassigned danderson Apr 1, 2018

danderson added a commit to danderson/metallb that referenced this issue Apr 8, 2018

Listen for ARP requests on all interfaces. google#165
Every 10s, the layer2 controller scans interfaces, and runs an ARP
responder for each interface with a globally routable IPv4 address
(note this includes RFC1918).

danderson added a commit that referenced this issue Apr 8, 2018

Listen for ARP requests on all interfaces. #165
Every 10s, the layer2 controller scans interfaces, and runs an ARP
responder for each interface with a globally routable IPv4 address
(note this includes RFC1918).

danderson added a commit to danderson/metallb that referenced this issue Apr 8, 2018

danderson added a commit to danderson/metallb that referenced this issue Apr 8, 2018

Listen for NDP requests on all interfaces. google#165
Every 10s, the layer2 controller scans interfaces, and runs an NDP
responder for each interface with a globally routable IPv6 address.

danderson added a commit that referenced this issue Apr 8, 2018

Listen for NDP requests on all interfaces. #165
Every 10s, the layer2 controller scans interfaces, and runs an NDP
responder for each interface with a globally routable IPv6 address.

danderson added a commit to danderson/metallb that referenced this issue Apr 8, 2018

@danderson danderson closed this in 400deef Apr 8, 2018

Layer 2 mode automation moved this from To Do to Done Apr 8, 2018

Work better with multiple interfaces automation moved this from To do to Done Apr 8, 2018

danderson added a commit that referenced this issue Apr 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment