ARP/NDP should work correctly on machines with multiple interfaces

[danderson]

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]

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]

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]

sgtm, happy to review any PRs