diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 8c0077748..79cc6f4e1 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -690,6 +690,14 @@ func (d *Daemon) Start() error { if node, ok := d.clusterConfig.Nodes[d.clusterConfig.Node]; ok { dhcpBindBridge = node.VPCD.DhcpBindBridge } + // For DHCP-sourced pools, compute the gateway LRP MAC so per-VM + // DORA uses it as chaddr. This makes the upstream DHCP binding + // match the MAC OVN GARPs with, preventing ARP table conflicts on + // routers that derive their ARP table from DHCP leases. + gwMAC := "" + if d.clusterConfig.Bootstrap.VpcId != "" { + gwMAC = utils.HashMAC("gw-" + d.clusterConfig.Bootstrap.VpcId) + } for _, p := range d.clusterConfig.Network.ExternalPools { pools = append(pools, handlers_ec2_vpc.ExternalPoolConfig{ Name: p.Name, @@ -702,6 +710,7 @@ func (d *Daemon) Start() error { Region: p.Region, AZ: p.AZ, DhcpBindBridge: dhcpBindBridge, + GatewayMAC: gwMAC, }) } d.externalIPAM, err = handlers_ec2_vpc.NewExternalIPAM(d.natsConn, js, pools) diff --git a/spinifex/handlers/ec2/vpc/external_ipam.go b/spinifex/handlers/ec2/vpc/external_ipam.go index e96a9d721..6c551f581 100644 --- a/spinifex/handlers/ec2/vpc/external_ipam.go +++ b/spinifex/handlers/ec2/vpc/external_ipam.go @@ -62,6 +62,13 @@ type ExternalPoolConfig struct { Region string AZ string DhcpBindBridge string // Bridge where the DHCP AF_PACKET socket binds (e.g. "br-wan"). Linux bridge in veth mode; OVS bridge in direct mode. Never "br-ext". + // GatewayMAC, when set, is used as the chaddr in per-VM DHCP DORAs so + // that the upstream DHCP binding matches the MAC that OVN GARPs use (the + // gateway LRP MAC). Without this, the DHCP server binds the public IP to + // the generated ENI-derived MAC while OVN announces a different MAC via + // GARP, causing home routers that use their DHCP lease table as ARP to + // forward traffic to a non-existent interface. + GatewayMAC string } // IsDHCP returns true if this pool obtains IPs from router DHCP. @@ -165,6 +172,7 @@ func (m *ExternalIPAM) initPool(pool ExternalPoolConfig) error { "gateway-"+pool.Name, "mulga-spinifex-gw", pool.Name, + "", // no specific chaddr — this lease is a pool reservation, not a routable VM address ) if dhcpErr != nil { return fmt.Errorf("DHCP gateway lease for pool %q: %w", pool.Name, dhcpErr) @@ -253,7 +261,7 @@ func (m *ExternalIPAM) allocateFromPool(poolName, allocType, allocID, eniID, ins var dhcpLease *dhcpLeaseResult if pool != nil && pool.IsDHCP() { hostname, vendorClass := dhcpIdentityOptions(eniID, instanceID, poolName) - lease, err := ObtainDHCPLease(m.nc, pool.DhcpBindBridge, clientID, hostname, vendorClass, poolName) + lease, err := ObtainDHCPLease(m.nc, pool.DhcpBindBridge, clientID, hostname, vendorClass, poolName, pool.GatewayMAC) if err != nil { return "", fmt.Errorf("DHCP lease for %s: %w", clientID, err) } diff --git a/spinifex/handlers/ec2/vpc/external_ipam_dhcp.go b/spinifex/handlers/ec2/vpc/external_ipam_dhcp.go index ea3473c83..026eec16d 100644 --- a/spinifex/handlers/ec2/vpc/external_ipam_dhcp.go +++ b/spinifex/handlers/ec2/vpc/external_ipam_dhcp.go @@ -44,12 +44,17 @@ type dhcpLeaseResult struct { // ObtainDHCPLease asks spinifex-vpcd to acquire a DHCP lease on the given // bridge, identifying us with option 61 (client-id), option 12 (hostname) -// and option 60 (vendor class). Blocks until vpcd replies or -// dhcpNATSTimeout expires. The Manager-side handler is idempotent: a -// second call with the same clientID while a live lease exists returns -// the same lease without a fresh DORA, so CAS retry loops on the caller -// are safe. -func ObtainDHCPLease(nc *nats.Conn, bridge, clientID, hostname, vendorClass, poolName string) (dhcpLeaseResult, error) { +// and option 60 (vendor class). hwAddr, when non-empty, is used as the +// chaddr (layer-2 MAC) in the DHCP payload; when empty, vpcd derives one +// from clientID. Pass the OVN gateway LRP MAC here for per-VM allocations +// so that the upstream DHCP binding matches the MAC OVN GARPs with — without +// this, routers that use their DHCP lease table as ARP forward traffic to +// the generated chaddr instead of the gateway LRP, dropping all inbound traffic. +// Blocks until vpcd replies or dhcpNATSTimeout expires. The Manager-side +// handler is idempotent: a second call with the same clientID while a live +// lease exists returns the same lease without a fresh DORA, so CAS retry +// loops on the caller are safe. +func ObtainDHCPLease(nc *nats.Conn, bridge, clientID, hostname, vendorClass, poolName, hwAddr string) (dhcpLeaseResult, error) { if nc == nil { return dhcpLeaseResult{}, fmt.Errorf("DHCP lease: NATS connection is required") } @@ -66,6 +71,7 @@ func ObtainDHCPLease(nc *nats.Conn, bridge, clientID, hostname, vendorClass, poo Hostname: hostname, VendorClass: vendorClass, PoolName: poolName, + HWAddr: hwAddr, } data, err := json.Marshal(req) if err != nil { diff --git a/spinifex/handlers/ec2/vpc/external_ipam_test.go b/spinifex/handlers/ec2/vpc/external_ipam_test.go index 618696273..8e18f0dfe 100644 --- a/spinifex/handlers/ec2/vpc/external_ipam_test.go +++ b/spinifex/handlers/ec2/vpc/external_ipam_test.go @@ -612,7 +612,7 @@ func TestObtainDHCPLease_RequestTimeout(t *testing.T) { dhcpAcquireRetryDelay = prevDelay }) - _, err = ObtainDHCPLease(nc, "br-wan", "eni-timeout", "eni-timeout", "mulga-spinifex", "wan") + _, err = ObtainDHCPLease(nc, "br-wan", "eni-timeout", "eni-timeout", "mulga-spinifex", "wan", "") assert.ErrorContains(t, err, "dhcp acquire NATS request") } @@ -638,7 +638,7 @@ func TestObtainDHCPLease_MalformedReply(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { _ = sub.Unsubscribe() }) - _, err = ObtainDHCPLease(nc, "br-wan", "eni-malformed", "eni-malformed", "mulga-spinifex", "wan") + _, err = ObtainDHCPLease(nc, "br-wan", "eni-malformed", "eni-malformed", "mulga-spinifex", "wan", "") assert.ErrorContains(t, err, "unmarshal dhcp acquire reply") } @@ -668,14 +668,14 @@ func TestReleaseDHCPLease_ReplyError(t *testing.T) { } func TestObtainDHCPLease_Guards(t *testing.T) { - _, err := ObtainDHCPLease(nil, "br-wan", "eni-1", "eni-1", "mulga-spinifex", "wan") + _, err := ObtainDHCPLease(nil, "br-wan", "eni-1", "eni-1", "mulga-spinifex", "wan", "") assert.ErrorContains(t, err, "NATS connection is required") _, nc := testutil.StartTestNATS(t) - _, err = ObtainDHCPLease(nc, "", "eni-1", "eni-1", "mulga-spinifex", "wan") + _, err = ObtainDHCPLease(nc, "", "eni-1", "eni-1", "mulga-spinifex", "wan", "") assert.ErrorContains(t, err, "bridge name is required") - _, err = ObtainDHCPLease(nc, "br-wan", "", "eni-1", "mulga-spinifex", "wan") + _, err = ObtainDHCPLease(nc, "br-wan", "", "eni-1", "mulga-spinifex", "wan", "") assert.ErrorContains(t, err, "client ID is required") }