From f7446c69c6a9ace5b00113a240108a439045da75 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Mon, 8 Jun 2026 21:04:08 +1000 Subject: [PATCH] fix(network): prime host neigh with gateway port MAC on centralised EIP attach Centralised-mode EIP attach only flushed the host neighbour entry and relied on OVN re-answering the host's ARP. On OVN builds whose ovn-controller lacks inject-garp, no node answers a same-chassis recycled IP's re-ARP, so the entry decays to FAILED and the external IP is unreachable ("no route to host"). This sank the EKS e2e suite: the cluster NLB front-end EIP never resolved on br-wan, so kubectl could not reach the published apiserver endpoint. Mirror the distributed-mode prime: resolve the VPC gateway router port MAC (the L2 owner of a centralised EIP on the WAN segment) and program the host neighbour entry directly, falling back to a flush only when no MAC resolves. --- spinifex/network/policy/nat.go | 59 +++++++++++++++++++---------- spinifex/network/policy/nat_test.go | 41 ++++++++++++++++++-- spinifex/vpcd/vpcd.go | 13 ++++--- 3 files changed, 84 insertions(+), 29 deletions(-) diff --git a/spinifex/network/policy/nat.go b/spinifex/network/policy/nat.go index 40e8541a5..c7d2d050d 100644 --- a/spinifex/network/policy/nat.go +++ b/spinifex/network/policy/nat.go @@ -91,13 +91,13 @@ type GARPEmitter func(EIPSpec) error // Best-effort: implementations return errors but callers warn and proceed. type NeighFlusher func(externalIP string) error -// NeighPrimer installs the host neighbour entry for a distributed EIP directly, -// mapping ExternalIP to its external_mac. Preferred over NeighFlusher on attach -// when the MAC is known (distributed mode): a flush merely re-arms ARP, but on -// OVN builds without inject-garp no node answers the host's re-ARP for a -// same-chassis recycled IP, so the entry would just decay back to FAILED. -// Programming the known MAC makes the recycled IP reachable without an ARP -// round-trip. +// NeighPrimer installs the host neighbour entry for an EIP directly, mapping +// ExternalIP to the MAC that owns it on the WAN segment — the per-VM external_mac +// in distributed mode, the VPC gateway router-port MAC in centralised mode. +// Preferred over NeighFlusher on attach: a flush merely re-arms ARP, but on OVN +// builds without inject-garp no node answers the host's re-ARP for a same-chassis +// recycled IP, so the entry would just decay back to FAILED. Programming the +// known MAC makes the recycled IP reachable without an ARP round-trip. // // Best-effort: implementations return errors but callers warn and proceed. type NeighPrimer func(eip EIPSpec) error @@ -126,9 +126,8 @@ func WithGARPEmitter(g GARPEmitter) Option { } // WithNeighFlusher injects the host neighbour-flush hook fired on EIP detach (and -// on attach when the external_mac is unknown). Without it, recycled external IPs -// rely solely on the GARPEmitter, which is a no-op on OVN builds lacking -// inject-garp. +// on attach when no owning MAC resolves). Without it, recycled external IPs rely +// solely on the GARPEmitter, which is a no-op on OVN builds lacking inject-garp. func WithNeighFlusher(f NeighFlusher) Option { return func(m *natManager) { if f != nil { @@ -137,10 +136,10 @@ func WithNeighFlusher(f NeighFlusher) Option { } } -// WithNeighPrimer injects the host neighbour-prime hook fired on EIP attach in -// distributed mode (external_mac known). Preferred over the flusher there: it -// programs the recycled IP's MAC directly instead of waiting on an ARP reply -// that no node sends when inject-garp is unavailable. +// WithNeighPrimer injects the host neighbour-prime hook fired on EIP attach when +// the owning MAC is known. Preferred over the flusher: it programs the recycled +// IP's MAC directly instead of waiting on an ARP reply that no node sends when +// inject-garp is unavailable. func WithNeighPrimer(p NeighPrimer) Option { return func(m *natManager) { if p != nil { @@ -231,12 +230,21 @@ func (m *natManager) AddEIP(ctx context.Context, eip EIPSpec) error { if err := m.garp(eip); err != nil { slog.Warn("policy: AddEIP gratuitous-ARP emission failed", "external_ip", eip.ExternalIP, "logical_ip", eip.LogicalIP, "err", err) } - // Distributed mode: the external_mac is known, so program the host - // neighbour entry directly instead of flushing and waiting on an ARP reply - // no node sends when inject-garp is unavailable. Fall back to a flush when - // the MAC is absent (centralised shape). - if distributed { - if err := m.neighPrime(eip); err != nil { + // Program the host neighbour entry with the MAC that owns the external IP + // on the WAN segment so a recycled EIP is reachable immediately, instead of + // flushing and waiting on an ARP reply no node sends for a same-chassis + // rebind when inject-garp is unavailable. Distributed mode owns the EIP on + // the VM's own LSP (external_mac); centralised mode owns it on the VPC + // gateway router port, so resolve that port's MAC. Fall back to a flush only + // when no MAC resolves. + primeMAC := eip.MAC + if !distributed { + primeMAC = m.gatewayPortMAC(ctx, eip.VPCID) + } + if primeMAC != "" { + primed := eip + primed.MAC = primeMAC + if err := m.neighPrime(primed); err != nil { slog.Warn("policy: AddEIP neighbour prime failed", "external_ip", eip.ExternalIP, "logical_ip", eip.LogicalIP, "err", err) } } else if err := m.neigh(eip.ExternalIP); err != nil { @@ -245,6 +253,17 @@ func (m *natManager) AddEIP(ctx context.Context, eip EIPSpec) error { return nil } +// gatewayPortMAC returns the MAC of the VPC gateway router's external port, the +// L2 owner of a centralised EIP on the WAN segment. Empty on lookup miss so the +// caller falls back to an ARP flush. +func (m *natManager) gatewayPortMAC(ctx context.Context, vpcID string) string { + lrp, err := m.ovn.GetLogicalRouterPort(ctx, topology.GatewayRouterPort(vpcID)) + if err != nil || lrp == nil { + return "" + } + return lrp.MAC +} + func (m *natManager) DeleteEIP(ctx context.Context, vpcID, externalIP, logicalIP string) error { router := topology.VPCRouter(vpcID) if err := m.ovn.DeleteNAT(ctx, router, "dnat_and_snat", logicalIP); err != nil { diff --git a/spinifex/network/policy/nat_test.go b/spinifex/network/policy/nat_test.go index 9720899d5..3630628b3 100644 --- a/spinifex/network/policy/nat_test.go +++ b/spinifex/network/policy/nat_test.go @@ -20,6 +20,13 @@ func seedRouter(t *testing.T, m *mock.Client, vpcID string) string { return name } +func seedGatewayPort(t *testing.T, m *mock.Client, vpcID, mac string) { + t.Helper() + require.NoError(t, m.CreateLogicalRouterPort(context.Background(), + topology.VPCRouter(vpcID), + &nbdb.LogicalRouterPort{Name: topology.GatewayRouterPort(vpcID), MAC: mac})) +} + func findNAT(m *mock.Client, natType, logicalIP string) *nbdb.NAT { for _, n := range m.NATs { if n.Type == natType && n.LogicalIP == logicalIP { @@ -170,10 +177,11 @@ func TestNATManager_NeighPrime_OnDistributedAttach_FlushOnDetach(t *testing.T) { "detach must still flush the released external IP") } -func TestNATManager_NeighFlush_OnCentralizedAttach(t *testing.T) { +func TestNATManager_NeighPrime_OnCentralizedAttach(t *testing.T) { ctx := context.Background() m := mock.New() seedRouter(t, m, "vpc-1") + seedGatewayPort(t, m, "vpc-1", "02:gw:00:00:00:01") var primed []EIPSpec var flushed []string nm, err := NewNATManager(m, NATModeCentralized, @@ -190,9 +198,36 @@ func TestNATManager_NeighFlush_OnCentralizedAttach(t *testing.T) { require.NoError(t, nm.AddEIP(ctx, EIPSpec{ VPCID: "vpc-1", ExternalIP: "1.2.3.4", LogicalIP: "10.0.0.5", })) - assert.Empty(t, primed, "centralized attach has no external_mac to prime") + require.Len(t, primed, 1, + "centralized attach must prime the host neighbour with the gateway port MAC") + assert.Equal(t, "02:gw:00:00:00:01", primed[0].MAC, + "centralized prime must use the VPC gateway router-port MAC") + assert.Empty(t, flushed, "a successful prime must not also flush") +} + +func TestNATManager_NeighFlush_CentralizedNoGatewayMAC(t *testing.T) { + ctx := context.Background() + m := mock.New() + seedRouter(t, m, "vpc-1") // no gateway port seeded → MAC unresolvable + var primed []EIPSpec + var flushed []string + nm, err := NewNATManager(m, NATModeCentralized, + WithNeighPrimer(func(eip EIPSpec) error { + primed = append(primed, eip) + return nil + }), + WithNeighFlusher(func(externalIP string) error { + flushed = append(flushed, externalIP) + return nil + })) + require.NoError(t, err) + + require.NoError(t, nm.AddEIP(ctx, EIPSpec{ + VPCID: "vpc-1", ExternalIP: "1.2.3.4", LogicalIP: "10.0.0.5", + })) + assert.Empty(t, primed, "no gateway MAC to prime") assert.Equal(t, []string{"1.2.3.4"}, flushed, - "centralized attach must fall back to a neighbour flush") + "centralized attach must fall back to a neighbour flush when no MAC resolves") } func TestNATManager_NeighHooks_FailureNonFatal(t *testing.T) { diff --git a/spinifex/vpcd/vpcd.go b/spinifex/vpcd/vpcd.go index c18d5a376..33f259485 100644 --- a/spinifex/vpcd/vpcd.go +++ b/spinifex/vpcd/vpcd.go @@ -835,12 +835,13 @@ func neighFlusher(wanBridge string) policy.NeighFlusher { } } -// neighPrimer builds the post-AddEIP host ARP-prime hook for distributed EIPs. -// It programs the kernel neighbour entry for the external IP to the EIP's -// external_mac on the Linux WAN bridge, so a recycled IP is reachable -// immediately without an ARP reply — needed on OVN builds whose ovn-controller -// lacks inject-garp and therefore never advertises the new MAC. No-op when -// wanBridge or the MAC is unset. +// neighPrimer builds the post-AddEIP host ARP-prime hook. It programs the kernel +// neighbour entry for the external IP to the MAC carried on the EIPSpec on the +// Linux WAN bridge — the per-VM external_mac in distributed mode, the VPC gateway +// router-port MAC in centralised mode — so a recycled IP is reachable immediately +// without an ARP reply, needed on OVN builds whose ovn-controller lacks +// inject-garp and therefore never advertises the new MAC. No-op when wanBridge or +// the MAC is unset. func neighPrimer(wanBridge string) policy.NeighPrimer { return func(eip policy.EIPSpec) error { if wanBridge == "" || eip.MAC == "" {