Forget the host neighbor entry of a reused guest address

lib/network/allocate.go

@@ -50,6 +50,19 @@ func (m *manager) CreateAllocation(ctx context.Context, req AllocateRequest) (*N
 		return nil, err
 	}
 
+	// Addresses are reused. Drop what the host remembers about this one so the
+	// first packet to the new guest is resolved, not sent to the previous
+	// owner's MAC until that entry times out.
+	_, neighborSpanEnd := startNetworkStep(ctx, "network.forget_neighbor",
+		attribute.String("operation", "forget_neighbor"),
+		attribute.String("instance_id", req.InstanceID),
+	)
+	err = m.forgetNeighbor(network.Bridge, netConfig.IP)
+	neighborSpanEnd(err)
+	if err != nil {
+		log.WarnContext(ctx, "failed to forget neighbor entry for allocated address", "ip", netConfig.IP, "error", err)
+	}
+
 	// Create the TAP synchronously because the VMM restore depends on it.
 	tapCtx, tapSpanEnd := startNetworkStep(ctx, "network.create_tap",
 		attribute.String("operation", "create_tap"),

lib/network/bridge_darwin.go

@@ -58,6 +58,11 @@ func (m *manager) tapDeviceExists(tapName string) bool {
 	return true
 }
 
+// forgetNeighbor is a no-op on macOS: vmnet owns the host side of the network.
+func (m *manager) forgetNeighbor(bridgeName, ip string) error {
+	return nil
+}
+
 // queryNetworkState returns the default vmnet shared-NAT network targeted by
 // Hypeman. Host-level Shared_Net_Address overrides are not currently supported.
 func (m *manager) queryNetworkState(bridgeName string) (*Network, error) {

lib/network/bridge_linux.go

@@ -1093,6 +1093,24 @@ func (m *manager) tapDeviceExists(tapName string) bool {
 	return err == nil
 }
 
+// forgetNeighbor removes the host's neighbor entry for a guest address on the
+// bridge. A missing entry is not an error.
+func (m *manager) forgetNeighbor(bridgeName, ip string) error {
+	addr := net.ParseIP(ip).To4()
+	if addr == nil {
+		return fmt.Errorf("parse IPv4 address %q", ip)
+	}
+	bridge, err := netlink.LinkByName(bridgeName)
+	if err != nil {
+		return fmt.Errorf("get bridge: %w", err)
+	}
+	err = netlink.NeighDel(&netlink.Neigh{LinkIndex: bridge.Attrs().Index, Family: netlink.FAMILY_V4, IP: addr})
+	if err != nil && !errors.Is(err, unix.ENOENT) {
+		return fmt.Errorf("delete neighbor entry: %w", err)
+	}
+	return nil
+}
+
 // queryNetworkState queries kernel for bridge state.
 func (m *manager) queryNetworkState(bridgeName string) (*Network, error) {
 	link, err := netlink.LinkByName(bridgeName)

lib/network/bridge_linux_test.go

@@ -3,9 +3,15 @@
 package network
 
 import (
+	"context"
 	"errors"
+	"fmt"
+	"net"
+	"os"
 	"testing"
 
+	"github.com/kernel/hypeman/cmd/api/config"
+	"github.com/kernel/hypeman/lib/paths"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/vishvananda/netlink"
@@ -107,3 +113,116 @@ func TestRetryNetlinkDumpGivesUpAfterBudget(t *testing.T) {
 	require.ErrorIs(t, err, netlink.ErrDumpInterrupted)
 	assert.Equal(t, netlinkDumpRetryCount, calls)
 }
+
+func TestForgetNeighborDropsReusedAddressEntry(t *testing.T) {
+	bridge := newNeighborTestBridge(t)
+	ip := net.ParseIP("198.51.100.7").To4()
+	addStaleNeighbor(t, bridge, ip)
+	require.True(t, hasUsableNeighbor(t, bridge, ip))
+
+	m := &manager{}
+	require.NoError(t, m.forgetNeighbor(bridge.Name, ip.String()))
+	assert.False(t, hasUsableNeighbor(t, bridge, ip))
+
+	require.NoError(t, m.forgetNeighbor(bridge.Name, ip.String()), "a missing entry is not an error")
+	assert.Error(t, m.forgetNeighbor("hm-no-such-bridge", ip.String()))
+	assert.Error(t, m.forgetNeighbor(bridge.Name, "not-an-address"))
+}
+
+// A new guest must not inherit the neighbor entry of whichever guest held its
+// address before: the host would keep sending to the old MAC.
+func TestCreateAllocationForgetsNeighborOfReusedAddress(t *testing.T) {
+	bridge := newNeighborTestBridge(t)
+	const subnet = "198.51.100.0/29"
+	var candidates []net.IP
+	for last := byte(2); last <= 6; last++ {
+		ip := net.IPv4(198, 51, 100, last).To4()
+		candidates = append(candidates, ip)
+		addStaleNeighbor(t, bridge, ip)
+	}
+
+	m := &manager{
+		paths:              paths.New(t.TempDir()),
+		config:             &config.Config{Network: config.NetworkConfig{BridgeName: bridge.Name}},
+		pendingAllocations: make(map[string]pendingAllocation),
+	}
+	m.setDefaultNetwork(newDefaultNetwork(bridge.Name, subnet, "198.51.100.1"))
+
+	// The TAP is named after the first eight characters of the instance ID.
+	instanceID := fmt.Sprintf("n%07d", os.Getpid())
+	allocated, err := m.CreateAllocation(context.Background(), AllocateRequest{InstanceID: instanceID, InstanceName: instanceID})
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		if tap, err := netlink.LinkByName(allocated.TAPDevice); err == nil {
+			_ = netlink.LinkDel(tap)
+		}
+	})
+
+	for _, ip := range candidates {
+		assert.Equal(t, ip.String() != allocated.IP, hasUsableNeighbor(t, bridge, ip), "neighbor entry for %s", ip)
+	}
+}
+
+func newNeighborTestBridge(t *testing.T) *netlink.Bridge {
+	t.Helper()
+	if os.Geteuid() != 0 {
+		t.Skip("managing neighbor entries requires CAP_NET_ADMIN")
+	}
+	bridge := &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: fmt.Sprintf("hmn%d", os.Getpid())}}
+	require.NoError(t, netlink.LinkAdd(bridge))
+	t.Cleanup(func() { _ = netlink.LinkDel(bridge) })
+
+	// The kernel drops every neighbor entry of a bridge whose address changes
+	// or that has no carrier when a port is added. Pin the address and give
+	// the bridge a running port, like a host with guests has, so only the
+	// entry under test goes away. The TAP has carrier while this process
+	// holds it open.
+	address, err := net.ParseMAC("02:00:00:00:00:01")
+	require.NoError(t, err)
+	require.NoError(t, netlink.LinkSetHardwareAddr(bridge, address))
+	port := &netlink.Tuntap{
+		LinkAttrs:  netlink.LinkAttrs{Name: fmt.Sprintf("hmp%d", os.Getpid())},
+		Mode:       netlink.TUNTAP_MODE_TAP,
+		Queues:     1,
+		NonPersist: true,
+	}
+	require.NoError(t, netlink.LinkAdd(port))
+	t.Cleanup(func() {
+		for _, fd := range port.Fds {
+			_ = fd.Close()
+		}
+	})
+	require.NoError(t, netlink.LinkSetMaster(port, bridge))
+	require.NoError(t, netlink.LinkSetUp(port))
+	require.NoError(t, netlink.LinkSetUp(bridge))
+	return bridge
+}
+
+func addStaleNeighbor(t *testing.T, bridge *netlink.Bridge, ip net.IP) {
+	t.Helper()
+	previousOwner, err := net.ParseMAC("02:00:00:aa:bb:cc")
+	require.NoError(t, err)
+	require.NoError(t, netlink.NeighSet(&netlink.Neigh{
+		LinkIndex:    bridge.Attrs().Index,
+		Family:       netlink.FAMILY_V4,
+		State:        netlink.NUD_STALE,
+		IP:           ip,
+		HardwareAddr: previousOwner,
+	}))
+}
+
+func hasUsableNeighbor(t *testing.T, bridge *netlink.Bridge, ip net.IP) bool {
+	t.Helper()
+	var neighbors []netlink.Neigh
+	require.NoError(t, retryNetlinkDump(func() error {
+		var err error
+		neighbors, err = netlink.NeighList(bridge.Attrs().Index, netlink.FAMILY_V4)
+		return err
+	}))
+	for _, neighbor := range neighbors {
+		if neighbor.IP.Equal(ip) && neighbor.State != netlink.NUD_FAILED {
+			return true
+		}
+	}
+	return false
+}