diff --git a/docs/cni/conflist-reference.md b/docs/cni/conflist-reference.md index 16fa5074c..f0faa03d2 100644 --- a/docs/cni/conflist-reference.md +++ b/docs/cni/conflist-reference.md @@ -106,6 +106,14 @@ prefix allocated to the attachment, so the guest kernel has nothing telling it the gateway is reachable and rejects the route as unreachable otherwise. The host side of the tap is given the same treatment. +Neither side has to discover the other. The file gives the guest a permanent +neighbor entry for each gateway, pointing at the tap's host MAC, and +`galactic-bgp` reads the guest MAC back from the file and installs a permanent +neighbor entry for the guest's addresses on the host side, as it does for a +veth. Without them, a new guest's first packets wait for neighbor discovery to +complete in both directions, which can hold its first DNS lookups for several +seconds. + A failed write fails ADD. Kata treats a missing file as "discover the network yourself", which produces a healthy-looking sandbox on the wrong network — a far worse outcome than an attachment that does not come up. diff --git a/internal/cnibgp/config.go b/internal/cnibgp/config.go index fb4c0e2da..ab69d288a 100644 --- a/internal/cnibgp/config.go +++ b/internal/cnibgp/config.go @@ -185,6 +185,7 @@ func parseConf(data []byte) (*PluginConf, error) { EgressShardSIDs: hostConf.EgressShardSIDs, NAT64Prefix: hostConf.NAT64Prefix, EBPFInterfaces: hostConf.EBPFInterfaces, + DANDir: hostConf.DANDir, }) if cniConfig.NodeName == "" { diff --git a/internal/cnibgp/ops_add.go b/internal/cnibgp/ops_add.go index 53e31456c..d7a595e2c 100644 --- a/internal/cnibgp/ops_add.go +++ b/internal/cnibgp/ops_add.go @@ -81,6 +81,9 @@ func cmdAdd(args *skel.CmdArgs) (err error) { vpc: pluginConf.VPC, vpcAttachment: pluginConf.VPCAttachment, ifaceType: ifaceType, guestHWAddr: guestHWAddrFrom(prevResult), } + if cfg.guestHWAddr == nil && ifaceType == ifaceTypeTap { + cfg.guestHWAddr = tapGuestHWAddr(args.ContainerID, pluginConf.VPC, pluginConf.VPCAttachment) + } result, err := publishBGPState(args, cfg, nodeName, namespace, ipamResult, vpcHex, k8sClient) tracker.publishResult = result if err != nil { diff --git a/internal/cnibgp/prevresult.go b/internal/cnibgp/prevresult.go index 7eff09312..78260b7af 100644 --- a/internal/cnibgp/prevresult.go +++ b/internal/cnibgp/prevresult.go @@ -8,12 +8,15 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "net" "github.com/containernetworking/cni/pkg/types" type100 "github.com/containernetworking/cni/pkg/types/100" "go.datum.net/galactic/internal/cniipam" + "go.datum.net/galactic/internal/plumbing/dan" + "go.datum.net/galactic/internal/plumbing/intf" ) // inferFromPrevResult reconstructs the IPAM result and infers whether the @@ -114,3 +117,26 @@ func guestHWAddrFrom(prevResult types.Result) net.HardwareAddr { } return nil } + +// tapGuestHWAddr returns the guest MAC a DAN file gives the tap of this +// attachment, or nil when the tap was not handed to its guest by one. With it, +// the host gateway primes a permanent neighbor entry for the guest, as for a +// veth, so the first packet toward a new guest is delivered without waiting +// for the guest to make itself known. Without a DAN file the guest picks its +// own MAC, which the host cannot know, so nothing is primed. +func tapGuestHWAddr(sandboxID, vpc, vpcAttachment string) net.HardwareAddr { + if cniConfig == nil || cniConfig.DANDir == "" { + return nil + } + tapName := intf.GenerateInterfaceNameHost(vpc, vpcAttachment) + mac, ok, err := dan.GuestMAC(cniConfig.DANDir, sandboxID, tapName) + if err != nil { + slog.Warn("ADD: could not read the tap's guest MAC from its DAN file; not priming its neighbor entry", + "err", err, "tap", tapName) + return nil + } + if !ok { + return nil + } + return mac +} diff --git a/internal/cnibgp/tapguest_test.go b/internal/cnibgp/tapguest_test.go new file mode 100644 index 000000000..1048de7aa --- /dev/null +++ b/internal/cnibgp/tapguest_test.go @@ -0,0 +1,38 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package cnibgp + +import ( + "testing" + + "go.datum.net/galactic/internal/config" + "go.datum.net/galactic/internal/plumbing/dan" + "go.datum.net/galactic/internal/plumbing/intf" +) + +// A tap handed to its guest by a DAN file has a MAC the host knows, so the +// host gateway can prime the guest's neighbor entry; any other tap has none. +func TestTapGuestHWAddrReadsTheDANFile(t *testing.T) { + saved := cniConfig + t.Cleanup(func() { cniConfig = saved }) + cniConfig = config.NewCNIConfig() + cniConfig.DANDir = t.TempDir() + + const vpc, attachment = "7nXb3k", "2fLq9w" + tap := intf.GenerateInterfaceNameHost(vpc, attachment) + if err := dan.Write(cniConfig.DANDir, "sandbox-1", &dan.Document{Devices: []dan.Device{{ + Name: "guest0", GuestMAC: "02:a7:de:f3:26:ef", + Device: dan.DeviceSpec{Type: dan.DeviceTypeHostTap, TapName: tap}, + }}}); err != nil { + t.Fatalf("write DAN file: %v", err) + } + + if mac := tapGuestHWAddr("sandbox-1", vpc, attachment); mac.String() != "02:a7:de:f3:26:ef" { + t.Errorf("guest MAC = %v, want 02:a7:de:f3:26:ef", mac) + } + if mac := tapGuestHWAddr("sandbox-2", vpc, attachment); mac != nil { + t.Errorf("a sandbox with no DAN file got guest MAC %v, want none", mac) + } +} diff --git a/internal/cnitap/dan.go b/internal/cnitap/dan.go index 83e8636db..4961c61f2 100644 --- a/internal/cnitap/dan.go +++ b/internal/cnitap/dan.go @@ -22,8 +22,8 @@ const ipv4TapMask = 25 // it in the sandbox's DAN file. // // Callers must treat an error as fatal to CNI ADD. See dan.Write. -func writeDANFile(dir, sandboxID, tapName string, res *cniipam.IPAMResult, mtu int) error { - document, err := buildDANDocument(tapName, res, mtu) +func writeDANFile(dir, sandboxID, tapName string, hostMAC net.HardwareAddr, res *cniipam.IPAMResult, mtu int) error { + document, err := buildDANDocument(tapName, hostMAC, res, mtu) if err != nil { return fmt.Errorf("build DAN document: %w", err) } @@ -33,22 +33,40 @@ func writeDANFile(dir, sandboxID, tapName string, res *cniipam.IPAMResult, mtu i // buildDANDocument describes this attachment to a Kata runtime-rs shim. It // names the tap to adopt and what the guest should configure on it. // +// Each gateway gets a permanent neighbor entry for hostMAC, the tap's host +// side, so the guest's first packet goes out without a Neighbor Solicitation +// or ARP request and the entry is never reconfirmed. +// // Every value comes from state this plugin already owns, so the shim discovers // nothing. The document carries exactly one device, because an attachment is // one interface. -func buildDANDocument(tapName string, res *cniipam.IPAMResult, mtu int) (*dan.Document, error) { +func buildDANDocument( + tapName string, hostMAC net.HardwareAddr, res *cniipam.IPAMResult, mtu int, +) (*dan.Document, error) { if res == nil { return nil, fmt.Errorf("no addresses were allocated for tap %q", tapName) } var addresses []string routes := []dan.Route{} + neighbors := []dan.Neighbor{} + gatewayNeighbor := func(gateway net.IP, bits int) { + if len(hostMAC) == 0 { + return + } + neighbors = append(neighbors, dan.Neighbor{ + IPAddress: (&net.IPNet{IP: gateway, Mask: net.CIDRMask(bits, bits)}).String(), + HardwareAddr: hostMAC.String(), + State: dan.NeighborStatePermanent, + }) + } if res.IPv6Subnet != nil { addresses = append(addresses, res.IPv6Subnet.String()) } if res.IPv6Gateway != nil { routes = append(routes, defaultRoute(res.IPv6Gateway)) + gatewayNeighbor(res.IPv6Gateway, 128) } if res.IPv4Address != nil { ipv4 := net.IPNet{IP: res.IPv4Address, Mask: net.CIDRMask(ipv4TapMask, 32)} @@ -56,6 +74,7 @@ func buildDANDocument(tapName string, res *cniipam.IPAMResult, mtu int) (*dan.Do } if res.IPv4Gateway != nil { routes = append(routes, defaultRoute(res.IPv4Gateway)) + gatewayNeighbor(res.IPv4Gateway.To4(), 32) } if len(addresses) == 0 { @@ -77,7 +96,7 @@ func buildDANDocument(tapName string, res *cniipam.IPAMResult, mtu int) (*dan.Do Ntype: dan.InterfaceTypeTap, }, Routes: routes, - Neighbors: []any{}, + Neighbors: neighbors, }, }}, }, nil diff --git a/internal/cnitap/dan_test.go b/internal/cnitap/dan_test.go index 0cef37361..68e895478 100644 --- a/internal/cnitap/dan_test.go +++ b/internal/cnitap/dan_test.go @@ -31,7 +31,7 @@ func ipv6Result() *cniipam.IPAMResult { // values the plugin already holds, including the guest address and default // route the guest is told to configure. func TestBuildDANDocumentDescribesTheAttachment(t *testing.T) { - document, err := buildDANDocument("G00000000bayhH", ipv6Result(), 1460) + document, err := buildDANDocument("G00000000bayhH", nil, ipv6Result(), 1460) if err != nil { t.Fatalf("buildDANDocument: %v", err) } @@ -70,7 +70,7 @@ func TestBuildDANDocumentOrdersDualStackAddresses(t *testing.T) { res.IPv4Address = net.ParseIP("10.244.1.5") res.IPv4Gateway = net.ParseIP("10.244.1.1") - document, err := buildDANDocument("G00000000bayhH", res, 1460) + document, err := buildDANDocument("G00000000bayhH", nil, res, 1460) if err != nil { t.Fatalf("buildDANDocument: %v", err) } @@ -100,7 +100,7 @@ func TestBuildDANDocumentMarksTheDefaultRouteOnLink(t *testing.T) { res.IPv4Address = net.ParseIP("10.244.1.5") res.IPv4Gateway = net.ParseIP("10.244.1.1") - document, err := buildDANDocument("G00000000bayhH", res, 1460) + document, err := buildDANDocument("G00000000bayhH", nil, res, 1460) if err != nil { t.Fatalf("buildDANDocument: %v", err) } @@ -144,7 +144,7 @@ func TestBuildDANDocumentRequiresAddresses(t *testing.T) { "empty result": {}, } { t.Run(name, func(t *testing.T) { - if _, err := buildDANDocument("G00000000bayhH", res, 1460); err == nil { + if _, err := buildDANDocument("G00000000bayhH", nil, res, 1460); err == nil { t.Error("buildDANDocument succeeded, want an error") } }) @@ -155,7 +155,7 @@ func TestBuildDANDocumentRequiresAddresses(t *testing.T) { // to bytes on disk against the document observed to work on a real node. func TestWriteDANFileProducesTheProvenShape(t *testing.T) { dir := t.TempDir() - if err := writeDANFile(dir, "sandbox-1", "G00000000bayhH", ipv6Result(), 1460); err != nil { + if err := writeDANFile(dir, "sandbox-1", "G00000000bayhH", nil, ipv6Result(), 1460); err != nil { t.Fatalf("writeDANFile: %v", err) } @@ -196,7 +196,7 @@ func TestWriteDANFileProducesTheProvenShape(t *testing.T) { // TestWriteDANFileFailsOnTheGoShimDirectory checks that ADD's write reaches the // guardrail rather than routing around it. func TestWriteDANFileFailsOnTheGoShimDirectory(t *testing.T) { - if err := writeDANFile(dan.GoShimDir, "sandbox-1", "G00000000bayhH", ipv6Result(), 1460); err == nil { + if err := writeDANFile(dan.GoShimDir, "sandbox-1", "G00000000bayhH", nil, ipv6Result(), 1460); err == nil { t.Error("writeDANFile succeeded on the Go shim's directory, want refusal") } } @@ -209,7 +209,7 @@ func TestWriteDANFileReportsWriteFailures(t *testing.T) { if err := os.WriteFile(blocked, []byte("not a directory"), 0o600); err != nil { t.Fatalf("create blocking file: %v", err) } - if err := writeDANFile(blocked, "sandbox-1", "G00000000bayhH", ipv6Result(), 1460); err == nil { + if err := writeDANFile(blocked, "sandbox-1", "G00000000bayhH", nil, ipv6Result(), 1460); err == nil { t.Error("writeDANFile succeeded where the directory cannot exist, want an error") } } @@ -234,3 +234,25 @@ func TestDANRequestedIsOptIn(t *testing.T) { }) } } + +// TestBuildDANDocumentPinsGatewayNeighbors checks that each gateway gets a +// permanent neighbor entry for the tap's host MAC, so the guest's first packet +// needs no Neighbor Solicitation or ARP request and is never held for one. +func TestBuildDANDocumentPinsGatewayNeighbors(t *testing.T) { + res := ipv6Result() + res.IPv4Address = net.ParseIP("10.244.1.5") + res.IPv4Gateway = net.ParseIP("10.244.1.1") + hostMAC := net.HardwareAddr{0x42, 0x2e, 0x8a, 0xa2, 0x3c, 0x4d} + + document, err := buildDANDocument("G00000000bayhH", hostMAC, res, 1460) + if err != nil { + t.Fatalf("buildDANDocument: %v", err) + } + want := []dan.Neighbor{ + {IPAddress: "fd20:0:f::1/128", HardwareAddr: "42:2e:8a:a2:3c:4d", State: dan.NeighborStatePermanent}, + {IPAddress: "10.244.1.1/32", HardwareAddr: "42:2e:8a:a2:3c:4d", State: dan.NeighborStatePermanent}, + } + if got := document.Devices[0].NetworkInfo.Neighbors; !reflect.DeepEqual(got, want) { + t.Errorf("neighbors = %+v, want %+v", got, want) + } +} diff --git a/internal/cnitap/ops_add.go b/internal/cnitap/ops_add.go index 5becbc2d3..5261b75fa 100644 --- a/internal/cnitap/ops_add.go +++ b/internal/cnitap/ops_add.go @@ -208,7 +208,8 @@ func cmdAdd(args *skel.CmdArgs) (err error) { // best-effort. See dan.Write. if danRequested(args.StdinData) { tracker.danDir, tracker.danSandboxID = cniConfig.DANDir, args.ContainerID - if err := writeDANFile(cniConfig.DANDir, args.ContainerID, hostName, ipamResult, hostMTU); err != nil { + if err := writeDANFile(cniConfig.DANDir, args.ContainerID, hostName, hostLink.Attrs().HardwareAddr, + ipamResult, hostMTU); err != nil { return fmt.Errorf("emit DAN file: %w", err) } slog.Debug("ADD: DAN file written", "containerID", args.ContainerID, diff --git a/internal/hostgw/tapneigh.go b/internal/hostgw/tapneigh.go index 672f09307..e8ff9c782 100644 --- a/internal/hostgw/tapneigh.go +++ b/internal/hostgw/tapneigh.go @@ -43,9 +43,12 @@ const ( // picking its own address, so no entry was ever installed and every packet // dropped. // -// So resolve it the only way available, by asking the guest. This deliberately -// does not install a permanent entry the way the veth path does: the host never -// learns a tap guest's MAC authoritatively, and a permanent entry holding a +// A tap handed to its guest by a DAN file is the exception: the file sets the +// guest's MAC, so galactic-bgp primes a permanent entry at ADD as for a veth, +// and this sweep finds it already resolved. For any other tap, resolve it the +// only way available, by asking the guest. This deliberately does not install +// a permanent entry for those: the host never learns such a guest's MAC +// authoritatively, and a permanent entry holding a // stale one after a guest reboots is worse than none, since nothing would // correct it. A dynamically resolved entry is what the lookup needs, any valid // state satisfying it, the kernel refreshes it from the guest's own diff --git a/internal/plumbing/dan/dan.go b/internal/plumbing/dan/dan.go index 2611a04f5..e6a9fad26 100644 --- a/internal/plumbing/dan/dan.go +++ b/internal/plumbing/dan/dan.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "net" "os" "path/filepath" "strings" @@ -77,9 +78,22 @@ type DeviceSpec struct { // NetworkInfo is what the guest is told to configure on the interface. type NetworkInfo struct { - Interface Interface `json:"interface"` - Routes []Route `json:"routes"` - Neighbors []any `json:"neighbors"` + Interface Interface `json:"interface"` + Routes []Route `json:"routes"` + Neighbors []Neighbor `json:"neighbors"` +} + +// NeighborStatePermanent is the kernel's NUD_PERMANENT: the guest never +// solicits or reconfirms the entry. +const NeighborStatePermanent uint32 = 0x80 + +// Neighbor is a neighbor entry the guest installs on the interface at boot. +type Neighbor struct { + // IPAddress is the neighbor's address, as a CIDR with a full-length mask. + IPAddress string `json:"ip_address"` + HardwareAddr string `json:"hardware_addr"` + State uint32 `json:"state"` + Flags uint32 `json:"flags"` } // Interface carries the guest-side addressing for one device. @@ -196,3 +210,36 @@ func Remove(dir, sandboxID string) error { } return nil } + +// GuestMAC returns the guest MAC that sandboxID's DAN file in dir gives the +// device adopting tapName. ok is false, with a nil error, when there is no +// such file or no such device: the tap is not handed to a guest by a DAN +// file, so its guest picks its own MAC. +func GuestMAC(dir, sandboxID, tapName string) (mac net.HardwareAddr, ok bool, err error) { + path, err := Path(dir, sandboxID) + if err != nil { + return nil, false, err + } + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, false, nil + } + return nil, false, fmt.Errorf("read DAN file %q: %w", path, err) + } + var doc Document + if err := json.Unmarshal(data, &doc); err != nil { + return nil, false, fmt.Errorf("parse DAN file %q: %w", path, err) + } + for _, device := range doc.Devices { + if device.Device.TapName != tapName { + continue + } + mac, err := net.ParseMAC(device.GuestMAC) + if err != nil { + return nil, false, fmt.Errorf("DAN file %q: guest MAC of %q: %w", path, tapName, err) + } + return mac, true, nil + } + return nil, false, nil +} diff --git a/internal/plumbing/dan/dan_test.go b/internal/plumbing/dan/dan_test.go index 37db57626..3dd0d89df 100644 --- a/internal/plumbing/dan/dan_test.go +++ b/internal/plumbing/dan/dan_test.go @@ -31,7 +31,7 @@ func sampleDocument() *Document { Ntype: InterfaceTypeTap, }, Routes: []Route{{Gateway: "fd20:0:f::1", Flags: RouteFlagOnLink}}, - Neighbors: []any{}, + Neighbors: []Neighbor{}, }, }}, } @@ -260,3 +260,29 @@ func TestDefaultDirIsNotTheGoShimDir(t *testing.T) { t.Fatalf("DefaultDir is the Go shim's directory %q", GoShimDir) } } + +// TestGuestMACReadsTheDeviceForTheTap checks that GuestMAC returns the MAC the +// file gives the device adopting the tap, and reports a missing file or device +// as not found rather than as an error. +func TestGuestMACReadsTheDeviceForTheTap(t *testing.T) { + dir := t.TempDir() + doc := &Document{Devices: []Device{{ + Name: "guest0", + GuestMAC: "02:a7:de:f3:26:ef", + Device: DeviceSpec{Type: DeviceTypeHostTap, TapName: "Gtap"}, + }}} + if err := Write(dir, "sandbox-1", doc); err != nil { + t.Fatalf("Write: %v", err) + } + + mac, ok, err := GuestMAC(dir, "sandbox-1", "Gtap") + if err != nil || !ok || mac.String() != "02:a7:de:f3:26:ef" { + t.Fatalf("GuestMAC = %v, %v, %v; want 02:a7:de:f3:26:ef, true, nil", mac, ok, err) + } + if _, ok, err := GuestMAC(dir, "sandbox-1", "Gother"); ok || err != nil { + t.Errorf("another tap: ok=%v err=%v, want false, nil", ok, err) + } + if _, ok, err := GuestMAC(dir, "sandbox-2", "Gtap"); ok || err != nil { + t.Errorf("no file: ok=%v err=%v, want false, nil", ok, err) + } +}