diff --git a/cmd/galactic-gateway/dispatch_test.go b/cmd/galactic-gateway/dispatch_test.go index a4338d92..6907a2a0 100644 --- a/cmd/galactic-gateway/dispatch_test.go +++ b/cmd/galactic-gateway/dispatch_test.go @@ -19,7 +19,6 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/natattach" "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" "go.datum.net/galactic/internal/plumbing/ebpf/xdpdispatch" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) func requireRoot(t *testing.T) { @@ -95,7 +94,7 @@ type gatewayProcess struct { func startGateway(t *testing.T, d pinDirs) gatewayProcess { t.Helper() ctx, cancel := context.WithCancel(context.Background()) - objs, dispatcher, public, _, leaseDone, err := joinDispatcher(ctx, d.dispatch, d.edge, xdpmode.Native, false, nil) + objs, dispatcher, public, _, leaseDone, err := joinDispatcher(ctx, d.dispatch, d.edge, false, nil) if err != nil { cancel() t.Fatalf("gateway joinDispatcher: %v", err) @@ -127,7 +126,7 @@ func startShard(t *testing.T, d pinDirs, uplink string) func() error { if err := natattach.PopulateProgArray(objs); err != nil { t.Fatalf("populate nat_progs: %v", err) } - dispatcher, err := xdpdispatch.Open(context.Background(), d.dispatch, xdpmode.Native) + dispatcher, err := xdpdispatch.Open(context.Background(), d.dispatch) if err != nil { t.Fatalf("shard open dispatcher: %v", err) } @@ -166,7 +165,7 @@ func TestGatewayAndShardShareAnUplink(t *testing.T) { shardCovered := startShard(t, d, uplink) root := xdpProgID(t, uplink) - probe, err := xdpdispatch.Open(context.Background(), d.dispatch, xdpmode.Native) + probe, err := xdpdispatch.Open(context.Background(), d.dispatch) if err != nil { t.Fatalf("open dispatcher: %v", err) } @@ -203,7 +202,7 @@ func TestAttachDirect_RefusesAnUplinkTheShardRunsOn(t *testing.T) { startShard(t, d, uplink) root := xdpProgID(t, uplink) - _, _, _, err := attachDirect(context.Background(), d.dispatch, d.edge, xdpmode.Native, []string{uplink}, nil) + _, _, _, err := attachDirect(context.Background(), d.dispatch, d.edge, []string{uplink}, nil) if !errors.Is(err, xdpattach.ErrDispatcherInUse) { t.Fatalf("attachDirect over a live shard slot = %v, want ErrDispatcherInUse", err) } diff --git a/cmd/galactic-gateway/gateway.go b/cmd/galactic-gateway/gateway.go index 768ebb56..d85b64f0 100644 --- a/cmd/galactic-gateway/gateway.go +++ b/cmd/galactic-gateway/gateway.go @@ -23,7 +23,6 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/edgeprog" "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" "go.datum.net/galactic/internal/plumbing/ebpf/xdpdispatch" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" "go.datum.net/galactic/internal/plumbing/sysctl" ) @@ -110,20 +109,6 @@ func setupGatewayDatapath( overlap, publicInterface) } - // One attach mode for every XDP attachment on the host, the public and - // internal targets alike (xdpmode). A target that does not exist, or a - // NIC whose driver cannot be read, fails startup rather than settling the - // mode without it. - modeDecision, err := xdpmode.Select(append(slices.Clone(targets), returnTargets...)...) - if err != nil { - return nil, fmt.Errorf("select the XDP attach mode for this host: %w", err) - } - mode := modeDecision.Mode - slog.Info("Selected the XDP attach mode for this host", "mode", mode.String(), "reason", modeDecision.Reason) - for _, skipped := range modeDecision.Skipped { - slog.Warn("Left a NIC with no driver out of the XDP attach mode decision", "reason", skipped) - } - // Required for the FIB lookup in the datapath's header push to succeed // on the interface the program actually runs on. The lookup uses the // ingress interface, meaning whichever resolved target the packet @@ -166,7 +151,7 @@ func setupGatewayDatapath( return nil, fmt.Errorf("register dispatcher lease metrics: %w", err) } objs, dispatcher, publicSet, returnSet, leaseDone, err = joinDispatcher(leaseCtx, xdpdispatch.PinDir, - edgeattach.PinDir, mode, len(returnTargets) > 0, lease) + edgeattach.PinDir, len(returnTargets) > 0, lease) if err != nil { stopLeases() return nil, err @@ -180,7 +165,7 @@ func setupGatewayDatapath( } } } else { - objs, publicSet, returnSet, err = attachDirect(ctx, xdpdispatch.PinDir, edgeattach.PinDir, mode, targets, + objs, publicSet, returnSet, err = attachDirect(ctx, xdpdispatch.PinDir, edgeattach.PinDir, targets, returnTargets) } if err != nil { @@ -239,14 +224,11 @@ func (s dispatchSet) reconcile(ctx context.Context, targets []string) []string { // them to the targets themselves, unpinned, after releasing an idle // dispatcher from those targets. A dispatcher still serving the egress shard // there is refused rather than taken: detaching it would cut egress. -// Attachment is all-or-nothing, as it has always been in this mode, and every -// attach is in mode. -func attachDirect(ctx context.Context, dispatchDir, edgeDir string, mode xdpmode.Mode, - targets, returnTargets []string, -) ( +// Attachment is all-or-nothing, as it has always been in this mode. +func attachDirect(ctx context.Context, dispatchDir, edgeDir string, targets, returnTargets []string) ( *edgeprog.EdgedsrObjects, targetSet, targetSet, error, ) { - if err := xdpattach.ReleaseIdleDispatcher(ctx, dispatchDir, mode, append(slices.Clone(targets), returnTargets...), + if err := xdpattach.ReleaseIdleDispatcher(ctx, dispatchDir, append(slices.Clone(targets), returnTargets...), xdpdispatch.SlotGatewayLB, xdpdispatch.SlotGatewayReturn); err != nil { return nil, nil, nil, fmt.Errorf("%w; set %s=%s", err, config.EnvGatewayXDPAttach, config.GatewayXDPAttachDispatch) @@ -257,12 +239,12 @@ func attachDirect(ctx context.Context, dispatchDir, edgeDir string, mode xdpmode return nil, nil, nil, fmt.Errorf("load edge gateway eBPF datapath: %w", err) } - xdpLinks, err := edgeattach.Attach(objs.EdgeLb, targets, mode) + xdpLinks, err := edgeattach.Attach(objs.EdgeLb, targets) if err != nil { _ = objs.Close() return nil, nil, nil, fmt.Errorf("attach edge gateway datapath to public interfaces %v: %w", targets, err) } - publicSet, err := xdpattach.NewSet(objs.EdgeLb, targets, xdpLinks, mode) + publicSet, err := xdpattach.NewSet(objs.EdgeLb, targets, xdpLinks) if err != nil { closeAll(xdpLinks) _ = objs.Close() @@ -272,14 +254,14 @@ func attachDirect(ctx context.Context, dispatchDir, edgeDir string, mode xdpmode return objs, directSet{publicSet}, nil, nil } - returnLinks, err := edgeattach.Attach(objs.EdgeReturn, returnTargets, mode) + returnLinks, err := edgeattach.Attach(objs.EdgeReturn, returnTargets) if err != nil { closeAll(xdpLinks) _ = objs.Close() return nil, nil, nil, fmt.Errorf("attach edge gateway return datapath to internal interfaces %v: %w", returnTargets, err) } - returnSet, err := xdpattach.NewSet(objs.EdgeReturn, returnTargets, returnLinks, mode) + returnSet, err := xdpattach.NewSet(objs.EdgeReturn, returnTargets, returnLinks) if err != nil { closeAll(append(xdpLinks, returnLinks...)) _ = objs.Close() @@ -295,14 +277,13 @@ func attachDirect(ctx context.Context, dispatchDir, edgeDir string, mode xdpmode // without the role on any interface it never runs. Nothing is attached here; // the returned sets put the dispatcher on the targets and read coverage back. // The returned channel closes once both lease loops have stopped, which a -// caller waits on before closing the dispatcher. mode is the host's XDP attach -// mode, which the dispatcher attaches in and migrates its links to. -func joinDispatcher(ctx context.Context, dispatchDir, edgeDir string, mode xdpmode.Mode, withReturn bool, +// caller waits on before closing the dispatcher. +func joinDispatcher(ctx context.Context, dispatchDir, edgeDir string, withReturn bool, lease *xdpdispatch.LeaseMetrics, ) ( *edgeprog.EdgedsrObjects, *xdpdispatch.Dispatcher, targetSet, targetSet, <-chan struct{}, error, ) { - dispatcher, err := xdpdispatch.Open(ctx, dispatchDir, mode) + dispatcher, err := xdpdispatch.Open(ctx, dispatchDir) if err != nil { return nil, nil, nil, nil, nil, fmt.Errorf("open the node's XDP dispatcher: %w", err) } @@ -363,7 +344,7 @@ func joinDispatcher(ctx context.Context, dispatchDir, edgeDir string, mode xdpmo leases.Wait() close(leaseDone) }() - slog.Info("Edge gateway datapath installed in the XDP dispatcher", "pinDir", dispatchDir, "mode", mode.String()) + slog.Info("Edge gateway datapath installed in the XDP dispatcher", "pinDir", dispatchDir) return objs, dispatcher, dispatchSet{public}, ret, leaseDone, nil } diff --git a/cmd/galactic-nat/dispatch_test.go b/cmd/galactic-nat/dispatch_test.go index 9e6c85a5..d2c0a2be 100644 --- a/cmd/galactic-nat/dispatch_test.go +++ b/cmd/galactic-nat/dispatch_test.go @@ -19,7 +19,6 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/natattach" "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" "go.datum.net/galactic/internal/plumbing/ebpf/xdpdispatch" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // testPinDirs returns per-test bpffs directories for the dispatcher and the @@ -95,7 +94,7 @@ func joinForTest(t *testing.T, dispatchDir string, program *ebpf.Program) (*xdpd ) { t.Helper() ctx, cancel := context.WithCancel(context.Background()) - d, set, leaseDone, err := joinDispatcher(ctx, dispatchDir, xdpmode.Native, program, nil) + d, set, leaseDone, err := joinDispatcher(ctx, dispatchDir, program, nil) if err != nil { cancel() t.Fatalf("joinDispatcher: %v", err) @@ -157,13 +156,13 @@ func TestReleaseIdleDispatcher_FreesTheUplinkForADirectAttach(t *testing.T) { t.Fatalf("missing %v", missing) } - if err := releaseIdleDispatcher(context.Background(), dispatchDir, xdpmode.Native, []string{uplink}); err != nil { + if err := releaseIdleDispatcher(context.Background(), dispatchDir, []string{uplink}); err != nil { t.Fatalf("releaseIdleDispatcher: %v", err) } if got := xdpProgID(t, uplink); got != 0 { t.Fatalf("uplink still runs program %d after release", got) } - links, err := natattach.Attach(program, []string{uplink}, xdpmode.Native) + links, err := natattach.Attach(program, []string{uplink}) if err != nil { t.Fatalf("direct attach after release: %v", err) } @@ -194,7 +193,7 @@ func TestReleaseIdleDispatcher_RefusesWhileAnotherSlotIsLive(t *testing.T) { } root := xdpProgID(t, uplink) - if err := releaseIdleDispatcher(context.Background(), dispatchDir, xdpmode.Native, []string{uplink}); err == nil { + if err := releaseIdleDispatcher(context.Background(), dispatchDir, []string{uplink}); err == nil { t.Fatal("releaseIdleDispatcher succeeded with the gateway's slot live") } if got := xdpProgID(t, uplink); got != root { diff --git a/cmd/galactic-nat/nat.go b/cmd/galactic-nat/nat.go index 051cf8b5..90a2cefd 100644 --- a/cmd/galactic-nat/nat.go +++ b/cmd/galactic-nat/nat.go @@ -25,7 +25,6 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/natprog" "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" "go.datum.net/galactic/internal/plumbing/ebpf/xdpdispatch" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" "go.datum.net/galactic/internal/plumbing/nicstats" "go.datum.net/galactic/internal/plumbing/sysctl" ) @@ -286,20 +285,6 @@ func setupNatDatapath(ctx context.Context, cfg *config.NATConfig, return nil, err } - // One attach mode for every XDP attachment on the host, shared with the - // edge gateway's (xdpmode). An uplink that does not exist, or a NIC whose - // driver cannot be read, fails startup rather than settling the mode - // without it. - modeDecision, err := xdpmode.Select(uplinks...) - if err != nil { - return nil, fmt.Errorf("select the XDP attach mode for this host: %w", err) - } - mode := modeDecision.Mode - slog.Info("Selected the XDP attach mode for this host", "mode", mode.String(), "reason", modeDecision.Reason) - for _, skipped := range modeDecision.Skipped { - slog.Warn("Left a NIC with no driver out of the XDP attach mode decision", "reason", skipped) - } - // Required for the FIB lookup in both the forward and return paths to // succeed on the interface the program actually runs on. The lookup uses // the ingress interface, meaning whichever uplink the packet arrived on, so @@ -349,23 +334,22 @@ func setupNatDatapath(ctx context.Context, cfg *config.NATConfig, _ = objs.Close() return nil, fmt.Errorf("register dispatcher lease metrics: %w", err) } - if dispatcher, dispatchSet, _, err = joinDispatcher(ctx, xdpdispatch.PinDir, mode, objs.NatIngress, - lease); err != nil { + if dispatcher, dispatchSet, _, err = joinDispatcher(ctx, xdpdispatch.PinDir, objs.NatIngress, lease); err != nil { _ = objs.Close() return nil, err } d.missing = dispatchSet.Reconcile(ctx, uplinks) default: - if err := releaseIdleDispatcher(ctx, xdpdispatch.PinDir, mode, uplinks); err != nil { + if err := releaseIdleDispatcher(ctx, xdpdispatch.PinDir, uplinks); err != nil { _ = objs.Close() return nil, err } - xdpLinks, err = natattach.Attach(objs.NatIngress, uplinks, mode) + xdpLinks, err = natattach.Attach(objs.NatIngress, uplinks) if err != nil { _ = objs.Close() return nil, fmt.Errorf("attach egress translation datapath to uplink interfaces %v: %w", uplinks, err) } - if set, err = xdpattach.NewSet(objs.NatIngress, uplinks, xdpLinks, mode); err != nil { + if set, err = xdpattach.NewSet(objs.NatIngress, uplinks, xdpLinks); err != nil { closeAll(xdpLinks) _ = objs.Close() return nil, err @@ -430,14 +414,11 @@ func registerUplinkQueueStats(metricsReg prometheus.Registerer, d *natDatapath) // each renewal in lease, which may be nil. It // returns the set that keeps the dispatcher on the uplinks, and a channel // closed once the lease loop has stopped, which a caller waits on before -// closing the dispatcher. mode is the host's XDP attach mode, which the -// dispatcher attaches in and migrates its links to. -func joinDispatcher(ctx context.Context, pinDir string, mode xdpmode.Mode, program *ebpf.Program, - lease *xdpdispatch.LeaseMetrics, -) ( +// closing the dispatcher. +func joinDispatcher(ctx context.Context, pinDir string, program *ebpf.Program, lease *xdpdispatch.LeaseMetrics) ( *xdpdispatch.Dispatcher, *xdpattach.DispatchSet, <-chan struct{}, error, ) { - dispatcher, err := xdpdispatch.Open(ctx, pinDir, mode) + dispatcher, err := xdpdispatch.Open(ctx, pinDir) if err != nil { return nil, nil, nil, fmt.Errorf("open the node's XDP dispatcher: %w", err) } @@ -468,15 +449,15 @@ func joinDispatcher(ctx context.Context, pinDir string, mode xdpmode.Mode, progr "the dispatcher skips the slot once it lapses", "err", err) })) }() - slog.Info("Egress translation datapath installed in the XDP dispatcher", "pinDir", pinDir, "mode", mode.String()) + slog.Info("Egress translation datapath installed in the XDP dispatcher", "pinDir", pinDir) return dispatcher, set, leaseDone, nil } // releaseIdleDispatcher detaches an idle XDP dispatcher from uplinks before a // direct attach (xdpattach.ReleaseIdleDispatcher), refusing while another // datapath's slot is live there. -func releaseIdleDispatcher(ctx context.Context, pinDir string, mode xdpmode.Mode, uplinks []string) error { - if err := xdpattach.ReleaseIdleDispatcher(ctx, pinDir, mode, uplinks, xdpdispatch.SlotNAT); err != nil { +func releaseIdleDispatcher(ctx context.Context, pinDir string, uplinks []string) error { + if err := xdpattach.ReleaseIdleDispatcher(ctx, pinDir, uplinks, xdpdispatch.SlotNAT); err != nil { return fmt.Errorf("%w; set %s=%s", err, config.EnvNATXDPAttach, config.NATXDPAttachDispatch) } return nil diff --git a/config/galactic-nat/kustomization.yaml b/config/galactic-nat/kustomization.yaml index e3b06f26..b7aa30f8 100644 --- a/config/galactic-nat/kustomization.yaml +++ b/config/galactic-nat/kustomization.yaml @@ -3,7 +3,7 @@ # deliberately does NOT include base/. A shard's identity -- its SID and # per-family masquerade addresses -- comes from its EgressShard's spec, and # its uplinks are auto-detected, so base/daemonset.yaml no longer needs any -# per-node value to start. But auto-detection attaches XDP to every +# per-node value to start. But auto-detection attaches native XDP to every # interface carrying the IPv6 default route or a BGP-learned route, and on a # node where that includes a management NIC the deployer has to name the # fabric uplinks with GALACTIC_NAT_UPLINK_INTERFACES instead, which only diff --git a/docs/agents/ARCHITECTURE-GATEWAY.md b/docs/agents/ARCHITECTURE-GATEWAY.md index 05b395f5..4f4fe1ee 100644 --- a/docs/agents/ARCHITECTURE-GATEWAY.md +++ b/docs/agents/ARCHITECTURE-GATEWAY.md @@ -149,10 +149,8 @@ galactic/ │ │ # vip_table/vip_stats_table/drop_reasons live │ ├── edgepreflight/ # Startup kernel-capability check for this │ │ # datapath's own requirement list -│ ├── xdpattach/ # Bond-safe XDP attach, shared with -│ │ # natattach (support check + slave wait) -│ └── xdpmode/ # Per-host XDP attach mode (generic on -│ # bnxt_en hosts, native otherwise) +│ └── xdpattach/ # Bond-safe native XDP attach, shared with +│ # natattach (support check + slave wait) ├── config/ │ ├── galactic-gateway/ │ │ ├── serviceaccount.yaml # Applied cluster-wide, idempotent @@ -383,7 +381,7 @@ datapath ever pushed — found via live-kernel investigation, not ### Sharing the XDP hook (`xdpdispatch`) -An interface takes one XDP program, and `galactic-nat`'s egress shard +An interface takes one native XDP program, and `galactic-nat`'s egress shard can run on the same edge nodes, needing the same interfaces: tenant egress arrives on the compute-facing bond (`edge_return`'s) and its replies on the public uplink (`edge_lb`'s). Neither binary owns the hook. The node's shared @@ -411,12 +409,6 @@ datapath runs from its own slot. refuses to start while another datapath's slot is live there. Attached directly, the programs hold their own empty copies of the dispatcher maps, so every unclaimed packet passes to the kernel. -- The dispatcher attaches in the host's XDP mode (`xdpmode`, see - [docs/xdp-attach-mode.md](../xdp-attach-mode.md)): generic on a host with a - `bnxt_en` NIC (#673), native elsewhere. `Linked` and `Coverage` read the - mode back from the kernel, and `EnsureLink` migrates a pinned link left in - the other mode, under the dispatch lock and the bond guards, keeping every - map, slot, lease and role row. - `GALACTIC_GATEWAY_DATAPATH_ENABLED=false` loads nothing, empties the gateway's slots, and makes `NetworkGatewayReconciler` withdraw this node's VIP advertisements and report `DatapathDisabled`. @@ -540,9 +532,8 @@ DaemonSet mounts the host's `/proc/sys/net` at `/host/proc/sys/net` and points `GALACTIC_GATEWAY_PROC_SYS_PATH` at `/host/proc/sys` so the write can succeed without `privileged`. It then loads `edgeprog.EdgedsrObjects` (`edgeattach.Load`), attaches it to every -resolved target (`edgeattach.Attach`, in the host's XDP mode from -`xdpmode.Select`, with no fallback to the other mode, one `link.Link` per -target), and constructs a +resolved target (`edgeattach.Attach` — native XDP driver mode only, no +generic/SKB-mode fallback, one `link.Link` per target), and constructs a `gateway.KernelDatapath` over it, writing this node's SRv6 encap-source address into `encap_config_table` once at construction time. @@ -685,11 +676,10 @@ pod on that node is not a supported configuration. | `internal/maglev` (`table.go`) | galactic-gateway | Pure-Go Maglev consistent-hash lookup table (`New`/`Lookup`/`Backends`) — one `*Table` built per ring; this binary's only importer today (`galactic-nat` has no analogous shard-placement ring — see Known Constraints) | No | | `internal/plumbing/ebpf/edgeprog` | galactic-gateway | Compiled XDP program (`edgedsr.c`, program `edge_lb`) + bpf2go-generated Go bindings (`EdgedsrObjects`) | No | | `internal/plumbing/ebpf/edgemap` | galactic-gateway | `VIPTable`: `vip_table`/`vip_stats_table`/`vip_addr_table` read/write API, `Generation`/`Reconcile` crash-safety mechanism | Yes (via `KernelTable`) | -| `internal/plumbing/ebpf/edgeattach` | galactic-gateway | Load + attach, in the host's XDP mode, of the compiled program to one interface | Yes (pinned maps, held link) | +| `internal/plumbing/ebpf/edgeattach` | galactic-gateway | Load + native-XDP-only attach of the compiled program to one interface | Yes (pinned maps, held link) | | `internal/plumbing/ebpf/edgemetrics` | galactic-gateway | Pull-based `prometheus.Collector` reading `vip_table`/`vip_stats_table`/`drop_reasons` live at every scrape | No | | `internal/plumbing/ebpf/edgepreflight` | galactic-gateway | Startup kernel-capability check (`BPF_PROG_TYPE_XDP`, `BPF_MAP_TYPE_HASH`, kernel BTF, `bpf_xdp_adjust_head`) — no partial pass, no degraded fallback | No | -| `internal/plumbing/ebpf/xdpattach` | gateway, nat | XDP attach across a target list in one mode, read back from the kernel for coverage, shared with `natattach`: per-NIC native-support check before any native attach, each bond slave waited back into its aggregate before the next | No | -| `internal/plumbing/ebpf/xdpmode` | gateway, nat | `Select`: the host's XDP attach mode from its NIC drivers in sysfs, generic when any NIC uses `bnxt_en` (#673), native otherwise; fails on a missing target or an unreadable driver | | +| `internal/plumbing/ebpf/xdpattach` | gateway, nat | Native-XDP attach across a target list, shared with `natattach`: per-NIC support check before any attach, each bond slave waited back into its aggregate before the next | No | --- @@ -823,14 +813,11 @@ pod on that node is not a supported configuration. list is never mistaken for an orphan. `vip_table` state (not a kernel interface) is the only thing this design can leak on a crash — DSR keeps no flow/conntrack state at all to garbage-collect beyond that. -- **One XDP mode per host, never a fallback.** Native (driver) mode is the - default, since it is why this design chose XDP over TC-BPF. A host with - any `bnxt_en` NIC runs every Galactic XDP attachment in generic (SKB) mode - instead, because native redirect on that driver stalls receive queues - (#673). `xdpmode.Select` decides once per process from sysfs, and every - attach requests that mode explicitly and errors rather than retrying in the - other. See [docs/xdp-attach-mode.md](../xdp-attach-mode.md) for the - performance cost and rollback. +- **Native XDP only, no generic-mode fallback.** `edgeattach.Attach` always + requests `link.XDPDriverMode` and errors rather than silently retrying in + `link.XDPGenericMode` (SKB mode) if the interface's driver doesn't + support it — accepting generic mode silently would defeat the reason + this design chose XDP over TC-BPF in the first place. - **`galactic-router` carries no gateway-role code anymore.** Splitting the edge gateway out of `galactic-router` into its own binary means a crash in the XDP-loading, BPF/PERFMON-capable container never takes down the diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 5ee3c7e4..75486759 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -735,11 +735,10 @@ Deleting a rule removes its series. There is no `vpc_attachment` label, since a rule's backends can span many VPCAttachments. Confirm the eBPF program is actually attached to the node's public -interface. `edgeattach.Attach` requests the host's XDP mode, native unless a -NIC on the host uses `bnxt_en` (see [XDP attach mode](../xdp-attach-mode.md)), -so a plain `ip -d link show dev ` on the node itself should -show an attached `xdp` program, or `xdpgeneric` on a `bnxt_en` host, if this -step succeeded; the pod-level checks above (healthy gRPC health status, nonzero +interface — `edgeattach.Attach` requests **native XDP driver mode only** +(never generic/SKB mode), so a plain `ip -d link show dev ` +on the node itself should show an attached `xdp` program if this step +succeeded; the pod-level checks above (healthy gRPC health status, nonzero metrics) are the primary signal and don't require node-level `ip`/`bpftool` access. diff --git a/docs/nat/configuration.md b/docs/nat/configuration.md index 8bdff31a..855db1d5 100644 --- a/docs/nat/configuration.md +++ b/docs/nat/configuration.md @@ -99,13 +99,10 @@ containerlab lab. Detection would attach there too. A bonding master can be named in place of its members. It is expanded to its slaves at startup and the program attached to each of them, never to the master itself, the same way `galactic-gateway` treats a bonded public -interface. As with the gateway, the program is attached in the host's XDP -mode, generic on a host with a `bnxt_en` NIC and native elsewhere (see -[XDP attach mode](../xdp-attach-mode.md)). In native mode every resolved -interface is checked for native XDP support before any is touched. In either -mode the slaves are attached one at a time, each waited back into its LACP -aggregate before the next, so a NIC whose driver drops carrier to attach an -XDP program bounces one +interface. As with the gateway, every resolved interface is checked for +native XDP support before any is touched, and the slaves are attached one +at a time, each waited back into its LACP aggregate before the next, so a +NIC whose driver drops carrier to attach a native XDP program bounces one member of the bond at a time rather than all of them at once. A slave that does not rejoin within 45 seconds fails the shard's startup with the remaining slaves left untouched. Neither guard helps a bond with `miimon` @@ -144,7 +141,7 @@ that still holds the hook directly, and reports those uplinks missing until the gateway joins the dispatcher. - **`direct`** (the binary's default) attaches `nat_ingress` to every - resolved uplink in the host's XDP mode, as described above. The attachment + resolved uplink in native driver mode, as described above. The attachment is not pinned, so it detaches when the process exits, and every restart bounces each uplink, once on the detach and again on the attach. On a node where an earlier `dispatch`-mode shard left the node's XDP @@ -172,18 +169,12 @@ the gateway joins the dispatcher. Nothing in dispatch mode is all-or-nothing. An uplink the dispatcher cannot hold yet, because another XDP program is attached there, the driver has no - native XDP in native mode, or attaching would take a bond down, is reported - missing and + native XDP, or attaching would take a bond down, is reported missing and retried on the next link or route change. The first attach to each bond member, and the wait for it to rejoin its bond, run under a node-wide lock, so two datapaths sharing the dispatcher never bounce two members of one bond at once. - The dispatcher attaches in the host's XDP mode. A pinned link left in the - other mode, such as a native link on a host that now selects generic mode, - counts as uncovered until it is migrated, one bond member at a time under - the same lock, keeping every slot, lease and role. - The dispatcher stays attached after the shard is removed from a node. `hack/xdp-dispatch-release.sh` detaches it. Run it before rolling back to an image that predates dispatch mode, which would otherwise fail to attach diff --git a/docs/xdp-attach-mode.md b/docs/xdp-attach-mode.md deleted file mode 100644 index 4222b8fc..00000000 --- a/docs/xdp-attach-mode.md +++ /dev/null @@ -1,165 +0,0 @@ -# XDP attach mode - -Every Galactic XDP attachment on a host uses one attach mode, chosen once per -process at startup by `internal/plumbing/ebpf/xdpmode`. That covers -`galactic-gateway`'s `edge_lb` and `edge_return`, `galactic-nat`'s -`nat_ingress`, and the shared dispatcher (`xdp_dispatch`) both of them use in -dispatch mode. - -| Host | Mode | Shown by `ip -d link show` as | -| --------------------------- | --------------- | ----------------------------- | -| Any NIC driven by `bnxt_en` | generic (SKB) | `xdpgeneric` | -| Every other host | native (driver) | `xdp` | - -## Why bnxt_en hosts run generic mode - -On `bnxt_en`, native XDP redirect traffic is associated with receive queues -that stop delivering and never recover (#673). The suspected driver defect is -that RX producer updates are suppressed while the paired XDP TX ring is busy. -That mechanism is a hypothesis, not a confirmed root cause. - -Generic mode runs the same programs after the driver has built an skb. Their -`XDP_TX` and `XDP_REDIRECT` verdicts then go through the kernel's ordinary -transmit path, so the driver's XDP TX ring is never used. A staging run with -generic mode saw zero NIC queue discards and zero stalls over 408 seconds of -NAT64 and NAT66 TCP, UDP and ping traffic. That is limited-duration evidence, -not proof that the stall is gone. - -The `GalacticNatUplinkRxQueueStalled` alert (`config/monitoring/`) still -watches for it, and `galactic-nat` still exports the `bnxt_en` ring counters -from #779. - -## What the selection looks at - -- **Drivers come from sysfs.** Each interface under `/sys/class/net` with a - `device` link is a physical NIC, and its driver is the last element of - `device/driver`. Bond masters, VLANs, veths and other virtual interfaces have - no `device` link and are ignored, but they still get the host's mode, so a - host never mixes native and generic redirect paths. -- **A bnxt_en NIC anywhere selects generic for everything.** It doesn't have - to be one of the datapath's own targets. -- **Unknown state fails startup.** A target interface that doesn't exist, a - target NIC whose driver can't be read, or any NIC whose driver link exists - but can't be read, stops the process at startup with the reason in the - error. None of them ever settles the mode by omission. -- **A NIC with no driver at all is skipped.** That happens while a driver - unbinds. If the NIC isn't one of the datapath's targets it carries no - driver traffic, so it is left out of the decision with a warning naming - it. -- **The decision is logged.** Each process logs - `Selected the XDP attach mode for this host` with `mode` and `reason`. The - reason names the matching NICs. -- **The mode is fixed for the life of the process.** A NIC hot-added later - doesn't change it, so restart both pods after adding one. -- **The latest decision is recorded on the host.** Each dispatch-mode process - that attaches records the mode it selected in a pinned record beside the - dispatcher (`/sys/fs/bpf/galactic-xdp/host_mode`). A datapath being turned - off only clears its slots and records nothing. A process whose mode no longer - matches the record doesn't move a link back. It logs the error, reports its - interfaces uncovered, and stays not-ready until it is restarted. Two - processes that disagree therefore never move one link back and forth. - -## Attach, readiness and coverage - -- **No fallback between modes.** Every attach requests its mode explicitly - (`XDPGenericMode` or `XDPDriverMode`) and returns an error rather than - trying the other one. -- **The native-support check runs only in native mode.** The netdev - `xdp-features` check is skipped in generic mode, which runs on any - interface. -- **Readiness reads the real attach mode.** Coverage, and so readiness, reads - the mode back from the kernel (`IFLA_XDP_ATTACHED`). An interface whose hook - holds the program in the other mode counts as uncovered. - - Direct mode: `xdpattach.Set.Attached` and `Reconcile`. - - Dispatch mode: `xdpdispatch.Dispatcher.Linked` and `Coverage`, which - report `ErrModeMismatch`. - - The gateway's `readiness` gRPC service reports NOT_SERVING, and the - shard's `EgressShard` reports `UplinksMissing`, until the mode matches. - -## Moving a running node to the selected mode - -**Direct mode** needs nothing special. Links aren't pinned, so the restart -that brings the new release detaches the old program and attaches in the -selected mode. That bounces each uplink, as every direct-mode restart does. - -**Dispatch mode** keeps its links pinned across restarts, so a node already -running the dispatcher in native mode is migrated in place. - -1. A link in the other mode isn't counted as linked. So the reconcile that - finds it runs the same guards as a first attach, under the node-wide - dispatch lock. -2. A bond member that is the only one carrying traffic is left for a later - pass. Its old link still carries traffic, but its coverage reads as - missing, so the pod stays not-ready until a sibling is up and the member - can be migrated. -3. The pinned link is detached and the root attached again in the selected - mode, then re-pinned. The kernel won't change a link's mode in place or - hold a native and a generic program on one interface at once. -4. The driver resets its rings, as on a first attach. The process waits for - the member to rejoin its bond before touching the next. A member that - doesn't rejoin in time defers every remaining member to the next pass. -5. The interface carries no program between the detach and the new attach. - -Only the link changes. The dispatch maps, the root program, every slot, lease -and role row survive, so each datapath's slot runs again as soon as the new -link is up. - -If the new attach fails, the root is attached again in the mode it was in, the -error is logged, and the interface stays uncovered for readiness. Retries for -that interface back off, from 30 seconds doubling up to 10 minutes, so a -failure that persists doesn't bounce the uplink on every reconcile. If even -the restore fails, the interface has no dispatcher until a later reconcile -attaches one fresh. - -A pinned link whose current mode can't be read is left alone with an error -rather than detached, and so is a foreign XDP program in either mode. - -## Performance - -Generic mode allocates an skb for every frame before the program runs, so -the program no longer sees frames straight from the driver's receive ring. -Expect fewer packets per second per core and -more CPU per packet than native mode on the same NIC. On `bnxt_en` hosts that -cost buys uplinks that keep receiving. - -Compare `galactic_nat_*` and `galactic_edge_*` packet rates and node CPU -before and after the change on the same traffic before relying on the old -capacity figures. - -## Rollback - -- **Rolling back to a release without mode selection doesn't restore native - mode by itself.** Those releases attach in native mode, but in dispatch mode - they accept any live pinned link without checking its mode, so a node - already migrated stays generic. To return such a node to native mode: - 1. Roll back the image. - 2. Run `hack/xdp-dispatch-release.sh` on the node to detach the dispatcher. - 3. Restart `galactic-gateway` and `galactic-nat`. - - Every uplink bounces once, so do this in a maintenance window. -- **In direct mode, rolling back and restarting is enough.** -- **There is no override variable.** The mode follows the host's NICs, so - every datapath on a host always agrees. - -## Staging validation - -On a `bnxt_en` edge node after the release reaches it: - -1. Both pods log `Selected the XDP attach mode for this host` with - `mode=generic` and a reason naming the `bnxt_en` NICs. -2. `ip -d link show` on the node shows `xdpgeneric` with - `prog/xdp ... name xdp_dispatch` on every bond member, and nothing in - native mode. -3. The gateway's readiness probe passes, the `EgressShard` is `Ready`, and - `galactic_nat_uplink_covered` is 1 for every uplink. -4. NAT64 and NAT66 TCP, UDP and ping through the shard, and VIP traffic - through the gateway, all pass. Use `task verify:nat-egress` and - `task verify:gateway-ingress` in the lab, and the same checks against - staging. -5. Over at least a day of normal traffic, `GalacticNatUplinkRxQueueStalled` - doesn't fire, per-queue receive counters keep rising on every queue, and - NIC queue discards stay at zero. - -A node that already runs the dispatcher in generic mode, as the staging lab's -edge node did by hand before this change, needs no migration and sees no -bounce. diff --git a/internal/plumbing/ebpf/edgeattach/attach.go b/internal/plumbing/ebpf/edgeattach/attach.go index 7186067f..555c908b 100644 --- a/internal/plumbing/ebpf/edgeattach/attach.go +++ b/internal/plumbing/ebpf/edgeattach/attach.go @@ -21,7 +21,6 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/edgeprog" "go.datum.net/galactic/internal/plumbing/ebpf/mappin" "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // linkByNameFn and linkListFn are override points, as elsewhere in this @@ -167,24 +166,24 @@ func ResolveTargets(ifaceName string) ([]string, error) { } // Attach attaches program to the XDP hook of every interface in ifaceNames, in -// mode (xdpmode.Select), returning the resulting link for each in the same order +// native driver mode, returning the resulting link for each in the same order // for the caller to hold open and close on shutdown. Callers resolve ifaceNames // through ResolveTargets first, so this is usually a single interface and never // a bond master. // -// The attach itself is xdpattach.Attach, which in native mode checks every -// interface for native XDP support before touching any of them, and waits each bond slave back +// The attach itself is xdpattach.Attach, which checks every interface for +// native XDP support before touching any of them and waits each bond slave back // into its aggregate before attaching the next, so a bonded uplink never loses // every member at once. A failure partway through closes every link already // attached in this call. -func Attach(program *ebpf.Program, ifaceNames []string, mode xdpmode.Mode) ([]link.Link, error) { +func Attach(program *ebpf.Program, ifaceNames []string) ([]link.Link, error) { if program == nil { return nil, errors.New("edgeattach: program is nil") } if len(ifaceNames) == 0 { return nil, errors.New("edgeattach: no interfaces to attach to") } - links, err := xdpattach.Attach(program, ifaceNames, mode) + links, err := xdpattach.Attach(program, ifaceNames) if err != nil { return nil, fmt.Errorf("edgeattach: %w", err) } diff --git a/internal/plumbing/ebpf/edgeattach/attach_test.go b/internal/plumbing/ebpf/edgeattach/attach_test.go index ad44f404..285163ef 100644 --- a/internal/plumbing/ebpf/edgeattach/attach_test.go +++ b/internal/plumbing/ebpf/edgeattach/attach_test.go @@ -19,7 +19,6 @@ import ( "go.datum.net/galactic/internal/plumbing/bond" "go.datum.net/galactic/internal/plumbing/ebpf/edgeprog" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) func requireRoot(t *testing.T) { @@ -59,7 +58,7 @@ func TestLoad_PreflightBlocksLoad(t *testing.T) { // TestAttach_NilProgramIsError covers Attach's defensive nil-program // guard directly, without needing root. func TestAttach_NilProgramIsError(t *testing.T) { - if _, err := Attach(nil, []string{"eth0"}, xdpmode.Native); err == nil { + if _, err := Attach(nil, []string{"eth0"}); err == nil { t.Error("Attach(nil program, ...) error = nil, want an error") } } @@ -67,7 +66,7 @@ func TestAttach_NilProgramIsError(t *testing.T) { // TestAttach_NoInterfacesIsError covers Attach's defensive empty-slice // guard directly, without needing root or a real program. func TestAttach_NoInterfacesIsError(t *testing.T) { - if _, err := Attach(&ebpf.Program{}, nil, xdpmode.Native); err == nil { + if _, err := Attach(&ebpf.Program{}, nil); err == nil { t.Error("Attach(program, nil) error = nil, want an error") } } @@ -247,7 +246,7 @@ func TestLoadAttach_SurvivesRestartWithMapsIntact(t *testing.T) { } defer func() { _ = objs.Close() }() - links, err := Attach(objs.EdgeLb, []string{ifaceName}, xdpmode.Native) + links, err := Attach(objs.EdgeLb, []string{ifaceName}) if err != nil { return fmt.Errorf("attach: %w", err) } @@ -281,7 +280,7 @@ func TestLoadAttach_SurvivesRestartWithMapsIntact(t *testing.T) { } defer func() { _ = objs.Close() }() - links, err := Attach(objs.EdgeLb, []string{ifaceName}, xdpmode.Native) + links, err := Attach(objs.EdgeLb, []string{ifaceName}) if err != nil { return fmt.Errorf("re-attach after restart: %w", err) } @@ -364,7 +363,7 @@ func TestAttach_MultipleInterfacesRollsBackOnPartialFailure(t *testing.T) { defer func() { _ = objs.Close() }() // --- attach to both real interfaces: two links back. --- - links, err := Attach(objs.EdgeLb, []string{iface0Name, iface1Name}, xdpmode.Native) + links, err := Attach(objs.EdgeLb, []string{iface0Name, iface1Name}) if err != nil { return fmt.Errorf("attach to both interfaces: %w", err) } @@ -378,7 +377,7 @@ func TestAttach_MultipleInterfacesRollsBackOnPartialFailure(t *testing.T) { // --- attach again, this time with a second name that can never // attach (it doesn't exist): the first interface's attach must be // rolled back, not left dangling. --- - if _, err := Attach(objs.EdgeLb, []string{iface0Name, "does-not-exist"}, xdpmode.Native); err == nil { + if _, err := Attach(objs.EdgeLb, []string{iface0Name, "does-not-exist"}); err == nil { return errors.New("Attach() with an unresolvable second interface error = nil, want an error") } @@ -386,7 +385,7 @@ func TestAttach_MultipleInterfacesRollsBackOnPartialFailure(t *testing.T) { // fresh solo attach to it succeeds; if it leaked, this fails with // the kernel refusing a second native XDP program on the same // interface. - links, err = Attach(objs.EdgeLb, []string{iface0Name}, xdpmode.Native) + links, err = Attach(objs.EdgeLb, []string{iface0Name}) if err != nil { return fmt.Errorf( "re-attach to %q after a rolled-back partial failure: %w "+ @@ -487,7 +486,7 @@ func TestResolveTargetsAndAttach_RealBondDevice(t *testing.T) { // Attaching to the resolved slaves is exactly what this design // exists to make work. - links, err := Attach(objs.EdgeLb, got, xdpmode.Native) + links, err := Attach(objs.EdgeLb, got) if err != nil { return fmt.Errorf("attach to bond slaves %v: %w", got, err) } diff --git a/internal/plumbing/ebpf/edgeattach/mode_test.go b/internal/plumbing/ebpf/edgeattach/mode_test.go deleted file mode 100644 index 9afc3c31..00000000 --- a/internal/plumbing/ebpf/edgeattach/mode_test.go +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2026 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package edgeattach - -import ( - "fmt" - "os" - "path/filepath" - "slices" - "testing" - - "github.com/containernetworking/plugins/pkg/ns" - "github.com/vishvananda/netlink" - "github.com/vishvananda/netlink/nl" - - "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" -) - -// TestAttach_GenericModeIsWhatTheKernelReports attaches the edge program in -// generic mode, as a bnxt_en host does, and checks the kernel reports SKB -// mode, and that a Set reads coverage back by mode: a Set in generic mode -// counts the interface, and one in native mode over the same link does not. -func TestAttach_GenericModeIsWhatTheKernelReports(t *testing.T) { - requireRoot(t) - - pinDir := filepath.Join("/sys/fs/bpf", fmt.Sprintf("galactic-edge-mode-test-%d", os.Getpid())) - t.Cleanup(func() { _ = os.RemoveAll(pinDir) }) - const ifaceName, peerName = "edgemode0", "edgemode1" - - nsObj, err := ns.TempNetNS() - if err != nil { - t.Fatalf("create test netns: %v", err) - } - defer func() { _ = nsObj.Close() }() - - err = nsObj.Do(func(_ ns.NetNS) error { - veth := &netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: ifaceName}, PeerName: peerName} - if err := netlink.LinkAdd(veth); err != nil { - return fmt.Errorf("add veth pair: %w", err) - } - l, err := netlink.LinkByName(ifaceName) - if err != nil { - return err - } - if err := netlink.LinkSetUp(l); err != nil { - return err - } - - objs, err := Load(pinDir, nil) - if err != nil { - return fmt.Errorf("load: %w", err) - } - defer func() { _ = objs.Close() }() - - links, err := Attach(objs.EdgeLb, []string{ifaceName}, xdpmode.Generic) - if err != nil { - return fmt.Errorf("attach in generic mode: %w", err) - } - defer func() { _ = links[0].Close() }() - - l, err = netlink.LinkByName(ifaceName) - if err != nil { - return err - } - if xdp := l.Attrs().Xdp; xdp == nil || !xdp.Attached || xdp.AttachMode != nl.XDP_ATTACHED_SKB { - return fmt.Errorf("kernel reports XDP %+v, want an SKB-mode attachment", xdp) - } - - generic, err := xdpattach.NewSet(objs.EdgeLb, []string{ifaceName}, links, xdpmode.Generic) - if err != nil { - return err - } - if got := generic.Attached(); !slices.Equal(got, []string{ifaceName}) { - return fmt.Errorf("generic Set Attached() = %v, want [%s]", got, ifaceName) - } - native, err := xdpattach.NewSet(objs.EdgeLb, []string{ifaceName}, links, xdpmode.Native) - if err != nil { - return err - } - if got := native.Attached(); len(got) != 0 { - return fmt.Errorf("native Set over a generic link: Attached() = %v, want none", got) - } - return nil - }) - if err != nil { - t.Fatal(err) - } -} diff --git a/internal/plumbing/ebpf/natattach/attach.go b/internal/plumbing/ebpf/natattach/attach.go index 9a1830b7..ffc7205d 100644 --- a/internal/plumbing/ebpf/natattach/attach.go +++ b/internal/plumbing/ebpf/natattach/attach.go @@ -19,7 +19,6 @@ import ( "go.datum.net/galactic/internal/plumbing/ebpf/mappin" "go.datum.net/galactic/internal/plumbing/ebpf/natprog" "go.datum.net/galactic/internal/plumbing/ebpf/xdpattach" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // linkByNameFn and linkListFn are override points, as elsewhere in this @@ -181,23 +180,22 @@ func ResolveTargets(ifaceNames []string) ([]string, error) { // An empty list is rejected rather than treated as "attach nothing", which // would produce exactly that silence across the whole node. // -// The attach itself is xdpattach.Attach, shared with the edge gateway, in mode -// (xdpmode.Select): in native mode every interface is checked for native XDP -// support before any is touched, and in either mode each +// The attach itself is xdpattach.Attach, shared with the edge gateway: every +// interface is checked for native XDP support before any is touched, and each // bond slave is waited back into its aggregate before the next is attached, so // a bonded uplink never loses every member at once. // // Attachment is all-or-nothing: a failure partway through closes every link // already attached in this call, so a shard that cannot claim every uplink it // was given fails to start rather than running with a hole in its coverage. -func Attach(program *ebpf.Program, ifaceNames []string, mode xdpmode.Mode) ([]link.Link, error) { +func Attach(program *ebpf.Program, ifaceNames []string) ([]link.Link, error) { if program == nil { return nil, errors.New("natattach: program is nil") } if len(ifaceNames) == 0 { return nil, errors.New("natattach: no interfaces to attach to") } - links, err := xdpattach.Attach(program, ifaceNames, mode) + links, err := xdpattach.Attach(program, ifaceNames) if err != nil { return nil, fmt.Errorf("natattach: %w", err) } diff --git a/internal/plumbing/ebpf/natattach/attach_test.go b/internal/plumbing/ebpf/natattach/attach_test.go index 3fafa316..221bdd32 100644 --- a/internal/plumbing/ebpf/natattach/attach_test.go +++ b/internal/plumbing/ebpf/natattach/attach_test.go @@ -18,7 +18,6 @@ import ( "go.datum.net/galactic/internal/plumbing/bond" "go.datum.net/galactic/internal/plumbing/ebpf/natprog" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) func requireRoot(t *testing.T) { @@ -32,7 +31,7 @@ func requireRoot(t *testing.T) { // TestAttach_NilProgramIsError covers Attach's defensive nil-program // guard directly, without needing root. func TestAttach_NilProgramIsError(t *testing.T) { - if _, err := Attach(nil, []string{testUplink}, xdpmode.Native); err == nil { + if _, err := Attach(nil, []string{testUplink}); err == nil { t.Error("Attach(nil program, ...) error = nil, want an error") } } @@ -43,7 +42,7 @@ func TestAttach_NilProgramIsError(t *testing.T) { // untranslated and uncounted -- the silent failure #545 is about, node-wide. func TestAttach_EmptyInterfaceListIsError(t *testing.T) { for _, names := range [][]string{nil, {}} { - if _, err := Attach(nil, names, xdpmode.Native); err == nil { + if _, err := Attach(nil, names); err == nil { t.Errorf("Attach(_, %v) error = nil, want an error", names) } } @@ -212,7 +211,7 @@ func TestLoadAttach_SurvivesRestartWithMapsIntact(t *testing.T) { } defer func() { _ = objs.Close() }() - attached, err := Attach(objs.NatIngress, []string{ifaceName}, xdpmode.Native) + attached, err := Attach(objs.NatIngress, []string{ifaceName}) if err != nil { return fmt.Errorf("attach: %w", err) } @@ -243,7 +242,7 @@ func TestLoadAttach_SurvivesRestartWithMapsIntact(t *testing.T) { } defer func() { _ = objs.Close() }() - attached, err := Attach(objs.NatIngress, []string{ifaceName}, xdpmode.Native) + attached, err := Attach(objs.NatIngress, []string{ifaceName}) if err != nil { return fmt.Errorf("re-attach after restart: %w", err) } @@ -330,7 +329,7 @@ func TestAttach_MultipleUplinksRollsBackOnPartialFailure(t *testing.T) { defer func() { _ = objs.Close() }() // --- both uplinks of a dual-homed shard node: two links back. --- - links, err := Attach(objs.NatIngress, []string{uplink0Name, uplink1Name}, xdpmode.Native) + links, err := Attach(objs.NatIngress, []string{uplink0Name, uplink1Name}) if err != nil { return fmt.Errorf("attach to both uplinks: %w", err) } @@ -343,14 +342,14 @@ func TestAttach_MultipleUplinksRollsBackOnPartialFailure(t *testing.T) { // --- a second uplink that can never attach: the first uplink's // attach must be rolled back, not left dangling. --- - if _, err := Attach(objs.NatIngress, []string{uplink0Name, "does-not-exist"}, xdpmode.Native); err == nil { + if _, err := Attach(objs.NatIngress, []string{uplink0Name, "does-not-exist"}); err == nil { return errors.New("Attach() with an unresolvable second uplink error = nil, want an error") } // If the rollback genuinely closed the first uplink's link, a fresh // solo attach to it succeeds; if it leaked, this fails with the // kernel refusing a second native XDP program on the same interface. - links, err = Attach(objs.NatIngress, []string{uplink0Name}, xdpmode.Native) + links, err = Attach(objs.NatIngress, []string{uplink0Name}) if err != nil { return fmt.Errorf( "re-attach to %q after a rolled-back partial failure: %w "+ @@ -435,7 +434,7 @@ func TestResolveTargetsAndAttach_RealBondDevice(t *testing.T) { } defer func() { _ = objs.Close() }() - links, err := Attach(objs.NatIngress, got, xdpmode.Native) + links, err := Attach(objs.NatIngress, got) if err != nil { return fmt.Errorf("attach to bond slaves %v: %w", got, err) } diff --git a/internal/plumbing/ebpf/xdpattach/attach.go b/internal/plumbing/ebpf/xdpattach/attach.go index ae96b06a..aa774cd5 100644 --- a/internal/plumbing/ebpf/xdpattach/attach.go +++ b/internal/plumbing/ebpf/xdpattach/attach.go @@ -11,8 +11,6 @@ import ( "github.com/cilium/ebpf" "github.com/cilium/ebpf/link" "github.com/vishvananda/netlink" - - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // linkByNameFn is an override point, as elsewhere in this codebase, so the @@ -21,20 +19,18 @@ import ( var linkByNameFn = netlink.LinkByName // Attach attaches program to the XDP hook of every interface in ifaceNames, in -// mode, returning the resulting link for each in the same order for the caller -// to hold open and close on shutdown. Callers resolve bonding masters to their -// slaves first, so a master never reaches here. +// native driver mode, returning the resulting link for each in the same order +// for the caller to hold open and close on shutdown. Callers resolve bonding +// masters to their slaves first, so a master never reaches here. // -// In native mode every interface is checked for native XDP support before any -// of them is touched. In either mode a bond slave is waited back into its -// aggregate before the next interface is attached, so a bonded uplink never -// loses every member at once. An attach that fails in mode is returned as an -// error, never retried in the other mode. +// Every interface is checked for native XDP support before any of them is +// touched, and a bond slave is waited back into its aggregate before the next +// interface is attached, so a bonded uplink never loses every member at once. // // If attaching one interface fails partway through, every link already attached // in this call is closed before returning, so a caller that gets an error holds // no partial attachment to clean up. -func Attach(program *ebpf.Program, ifaceNames []string, mode xdpmode.Mode) ([]link.Link, error) { +func Attach(program *ebpf.Program, ifaceNames []string) ([]link.Link, error) { if program == nil { return nil, errors.New("xdpattach: program is nil") } @@ -42,8 +38,8 @@ func Attach(program *ebpf.Program, ifaceNames []string, mode xdpmode.Mode) ([]li return nil, errors.New("xdpattach: no interfaces to attach to") } - return attachSequentially(ifaceNames, mode, func(ifaceName string) (link.Link, error) { - return attachOne(program, ifaceName, mode) + return attachSequentially(ifaceNames, func(ifaceName string) (link.Link, error) { + return attachOne(program, ifaceName) }) } @@ -52,12 +48,9 @@ func Attach(program *ebpf.Program, ifaceNames []string, mode xdpmode.Mode) ([]li // // Nothing is attached until every interface has been checked, and each bond // slave is waited back into its aggregate before the next interface is -// touched. On a bonded uplink, skipping either ordering can take every member -// down at once. -func attachSequentially(ifaceNames []string, mode xdpmode.Mode, - attach func(string) (link.Link, error), -) ([]link.Link, error) { - if err := checkModeSupport(ifaceNames, mode); err != nil { +// touched. Both orderings are load-bearing on a bonded uplink. +func attachSequentially(ifaceNames []string, attach func(string) (link.Link, error)) ([]link.Link, error) { + if err := checkNativeXDPSupport(ifaceNames); err != nil { return nil, err } @@ -88,9 +81,9 @@ func closeAttached(links []link.Link) { } } -// attachOne attaches program to ifaceName's XDP hook in mode, the +// attachOne attaches program to ifaceName's XDP hook in native driver mode, the // single-interface mechanism Attach applies across its list. -func attachOne(program *ebpf.Program, ifaceName string, mode xdpmode.Mode) (link.Link, error) { +func attachOne(program *ebpf.Program, ifaceName string) (link.Link, error) { iface, err := netlink.LinkByName(ifaceName) if err != nil { return nil, fmt.Errorf("xdpattach: find link %q: %w", ifaceName, err) @@ -99,32 +92,14 @@ func attachOne(program *ebpf.Program, ifaceName string, mode xdpmode.Mode) (link xdpLink, err := link.AttachXDP(link.XDPOptions{ Program: program, Interface: iface.Attrs().Index, - Flags: mode.AttachFlags(), + Flags: link.XDPDriverMode, }) if err != nil { - return nil, fmt.Errorf("xdpattach: attach XDP program to %q in %s mode: %w (the other mode is not attempted)", - ifaceName, mode, err) + return nil, fmt.Errorf( + "xdpattach: attach XDP program to %q in native/driver mode: %w "+ + "(this program requires native XDP support -- generic/SKB mode is not attempted)", + ifaceName, err, + ) } return xdpLink, nil } - -// checkModeSupport is the pre-attach support check for mode. Only native mode -// needs the driver's support; generic mode runs on any interface. -func checkModeSupport(ifaceNames []string, mode xdpmode.Mode) error { - if mode != xdpmode.Native { - return nil - } - return checkNativeXDPSupport(ifaceNames) -} - -// attachedInMode reports whether ifaceName's XDP hook holds a program attached -// in mode, read back from the kernel rather than assumed from how it was -// attached. -func attachedInMode(ifaceName string, mode xdpmode.Mode) (bool, error) { - l, err := linkByNameFn(ifaceName) - if err != nil { - return false, fmt.Errorf("xdpattach: find link %q: %w", ifaceName, err) - } - xdp := l.Attrs().Xdp - return xdp != nil && xdp.Attached && mode.Matches(xdp.AttachMode), nil -} diff --git a/internal/plumbing/ebpf/xdpattach/dispatchset.go b/internal/plumbing/ebpf/xdpattach/dispatchset.go index b4f5bfad..c6d4fd9b 100644 --- a/internal/plumbing/ebpf/xdpattach/dispatchset.go +++ b/internal/plumbing/ebpf/xdpattach/dispatchset.go @@ -15,7 +15,6 @@ import ( "github.com/cilium/ebpf" "go.datum.net/galactic/internal/plumbing/ebpf/xdpdispatch" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // DispatchSet keeps one datapath's slot reachable on a set of interfaces @@ -35,9 +34,6 @@ import ( // set is often partial while routing converges). type DispatchSet struct { backend dispatchBackend - // mode is the dispatcher's attach mode, which decides the pre-attach - // support check. - mode xdpmode.Mode mu sync.Mutex missing []string @@ -46,8 +42,7 @@ type DispatchSet struct { // dispatchBackend is what DispatchSet needs from the dispatcher, so its tests // can run without loading anything into the kernel. type dispatchBackend interface { - // linked reports whether the root already holds ifindex in the - // dispatcher's mode. + // linked reports whether the root already holds ifindex. linked(ifindex int) (bool, error) // ensure, under the dispatch lock, attaches the root to ifindex if it is // not there and adds the datapath's role. It calls guard first, under the @@ -72,21 +67,16 @@ func NewDispatchSet(d *xdpdispatch.Dispatcher, slot xdpdispatch.Slot, if program == nil { return nil, errors.New("xdpattach: program is nil") } - return &DispatchSet{ - backend: &kernelDispatch{d: d, slot: slot, role: role, program: program}, - mode: d.Mode(), - }, nil + return &DispatchSet{backend: &kernelDispatch{d: d, slot: slot, role: role, program: program}}, nil } // Reconcile brings every interface in desired under the dispatcher with this // datapath's role, and returns the ones its slot does not cover afterwards. // // The guards Set applies before an attach apply here too, and only when the -// root is not on the interface in the dispatcher's mode yet, since nothing -// else bounces it: in native mode a definite lack of native XDP support -// leaves the interface missing, and in either mode so does a bond member whose -// bounce would take the bond down. A root linked in the other mode is -// migrated, which bounces the interface like a first attach. The guards, the +// root is not on the interface yet, since nothing else bounces it: a +// definite lack of native XDP support leaves the interface missing, and so +// does a bond member whose bounce would take the bond down. The guards, the // attach, and the wait for a bounced bond member to rejoin its aggregate all // run under the dispatch lock, so no other process bounces a second member of // the same bond in between. @@ -164,7 +154,7 @@ func (s *DispatchSet) Reconcile(ctx context.Context, desired []string) []string // attachable applies the pre-attach guards to name. func (s *DispatchSet) attachable(name string) error { - if err := checkModeSupport([]string{name}, s.mode); err != nil { + if err := checkNativeXDPSupport([]string{name}); err != nil { return err } isolates, err := bounceIsolatesBond(name) diff --git a/internal/plumbing/ebpf/xdpattach/doc.go b/internal/plumbing/ebpf/xdpattach/doc.go index fb41ffb2..2cd5887f 100644 --- a/internal/plumbing/ebpf/xdpattach/doc.go +++ b/internal/plumbing/ebpf/xdpattach/doc.go @@ -2,8 +2,8 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -// Package xdpattach attaches an XDP program to a list of interfaces without -// taking a bonded uplink down in the process. +// Package xdpattach attaches a native XDP program to a list of interfaces +// without taking a bonded uplink down in the process. // // It is the attach step both XDP datapaths share: the edge gateway's // (internal/plumbing/ebpf/edgeattach) and the egress shard's @@ -12,10 +12,7 @@ // for why attaching to a bond slave needs care that attaching to an ordinary // interface does not. // -// Every attach is in the one mode the caller passes, the host's mode from -// internal/plumbing/ebpf/xdpmode: native (driver) mode, or generic (SKB) mode -// on a host with a bnxt_en NIC. An attach that fails returns an error rather -// than retrying in the other mode, which has materially different performance -// characteristics. Coverage is read back from the kernel, so an interface -// holding the program in the other mode counts as uncovered. +// Native mode is required, not preferred. Attach always requests driver mode +// and returns an error rather than silently retrying in generic mode, which has +// materially different performance characteristics. package xdpattach diff --git a/internal/plumbing/ebpf/xdpattach/gate_test.go b/internal/plumbing/ebpf/xdpattach/gate_test.go index bcfc6c33..55cf8efe 100644 --- a/internal/plumbing/ebpf/xdpattach/gate_test.go +++ b/internal/plumbing/ebpf/xdpattach/gate_test.go @@ -15,7 +15,6 @@ import ( "github.com/vishvananda/netlink" "go.datum.net/galactic/internal/plumbing/ebpf/edgepreflight" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // fakeSlaveLink is a netlink.Link carrying bond slave data, the view @@ -227,7 +226,7 @@ func TestAttachSequentially_StopsAtTheFirstSlaveThatDoesNotComeBack(t *testing.T t.Cleanup(func() { BondReadyTimeout = 45 * time.Second }) var attached []string - _, err := attachSequentially([]string{gateSlave, gateSlaveSib}, xdpmode.Native, func(name string) (link.Link, error) { + _, err := attachSequentially([]string{gateSlave, gateSlaveSib}, func(name string) (link.Link, error) { attached = append(attached, name) return nil, nil }) @@ -245,8 +244,7 @@ func TestAttachSequentially_AttachesNothingWhenSupportIsMissing(t *testing.T) { edgepreflight.InterfaceXDPFn = func(name string) (bool, error) { return name != gateSlaveNoXDP, nil } var attached []string - names := []string{gateSlave, gateSlaveNoXDP} - _, err := attachSequentially(names, xdpmode.Native, func(name string) (link.Link, error) { + _, err := attachSequentially([]string{gateSlave, gateSlaveNoXDP}, func(name string) (link.Link, error) { attached = append(attached, name) return nil, nil }) diff --git a/internal/plumbing/ebpf/xdpattach/mode_test.go b/internal/plumbing/ebpf/xdpattach/mode_test.go deleted file mode 100644 index 7dbef667..00000000 --- a/internal/plumbing/ebpf/xdpattach/mode_test.go +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright 2026 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package xdpattach - -import ( - "context" - "slices" - "testing" - "time" - - "github.com/cilium/ebpf" - "github.com/cilium/ebpf/link" - "github.com/vishvananda/netlink" - - "go.datum.net/galactic/internal/plumbing/ebpf/edgepreflight" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" -) - -// TestAttachSequentially_GenericModeSkipsTheNativeCheck covers a bnxt_en host: -// generic mode runs on any interface, so a driver without native support is -// no reason to refuse it. -func TestAttachSequentially_GenericModeSkipsTheNativeCheck(t *testing.T) { - restoreGateVars(t) - edgepreflight.InterfaceXDPFn = func(string) (bool, error) { - t.Fatal("native support checked in generic mode") - return false, nil - } - linkByNameFn = func(name string) (netlink.Link, error) { return plainLink(name, 2), nil } - - var attached []string - if _, err := attachSequentially([]string{gateSlaveNoXDP}, xdpmode.Generic, func(name string) (link.Link, error) { - attached = append(attached, name) - return nil, nil - }); err != nil { - t.Fatalf("attachSequentially in generic mode: %v", err) - } - if !slices.Equal(attached, []string{gateSlaveNoXDP}) { - t.Errorf("attached %v, want [%s]", attached, gateSlaveNoXDP) - } -} - -// TestSet_AttachesInItsMode covers a Set in generic mode: every later attach -// asks for generic mode, and no native-support check runs. -func TestSet_AttachesInItsMode(t *testing.T) { - h := newSetHost(t, plainLink(testUplink0, 2), plainLink(testUplink1, 3)) - edgepreflight.InterfaceXDPFn = func(string) (bool, error) { return false, nil } - s, err := NewSet(new(ebpf.Program), nil, nil, xdpmode.Generic) - if err != nil { - t.Fatal(err) - } - - if missing := s.Reconcile([]string{testUplink0, testUplink1}); len(missing) != 0 { - t.Fatalf("Reconcile missing = %v, want none", missing) - } - for _, name := range []string{testUplink0, testUplink1} { - if got := h.modesAsked[name]; got != xdpmode.Generic { - t.Errorf("%s attached in %s mode, want generic", name, got) - } - } - if s.Mode() != xdpmode.Generic { - t.Errorf("Mode() = %s, want generic", s.Mode()) - } -} - -// TestSet_ReportsAnInterfaceHeldInTheWrongMode covers coverage read back from -// the kernel: an interface whose hook reports the other mode is neither -// attached nor covered, though the Set holds a link for it. -func TestSet_ReportsAnInterfaceHeldInTheWrongMode(t *testing.T) { - h := newSetHost(t, plainLink(testUplink0, 2), plainLink(testUplink1, 3)) - s, _ := newTestSet(t, testUplink0, testUplink1) - h.modes[testUplink0] = xdpmode.Generic - - missing := s.Reconcile([]string{testUplink0, testUplink1}) - if !slices.Equal(missing, []string{testUplink0}) { - t.Errorf("Reconcile missing = %v, want [%s]", missing, testUplink0) - } - if got := s.Attached(); !slices.Equal(got, []string{testUplink1}) { - t.Errorf("Attached() = %v, want [%s]", got, testUplink1) - } - if len(h.attached) != 0 { - t.Errorf("attached %v, want nothing: a held link is never replaced by a Set", h.attached) - } -} - -// TestDispatchSet_GenericModeSkipsTheNativeCheck covers the dispatcher's -// pre-attach guard in generic mode: a driver without native support is -// attached, and the bond guard still runs. -func TestDispatchSet_GenericModeSkipsTheNativeCheck(t *testing.T) { - newSetHost(t, plainLink(testUplink0, 10)) - edgepreflight.InterfaceXDPFn = func(string) (bool, error) { - t.Fatal("native support checked in generic mode") - return false, nil - } - fake := newFakeDispatch() - s := &DispatchSet{backend: fake, mode: xdpmode.Generic} - - if missing := s.Reconcile(context.Background(), []string{testUplink0}); len(missing) != 0 { - t.Errorf("missing = %v, want none", missing) - } - if !slices.Equal(fake.ensured, []int{10}) { - t.Errorf("ensured %v, want [10]", fake.ensured) - } -} - -// TestDispatchSet_MigrationKeepsTheBondGuard covers a root linked in the other -// mode: linked reports false for it, so the bond guard runs before the -// migration bounces the member, and a member whose bounce would isolate its -// bond is left for later. -func TestDispatchSet_MigrationKeepsTheBondGuard(t *testing.T) { - newSetHost(t, lacpMaster(), - slaveLink(gateSlave, netlink.BondLinkUp, lacpCarrying), - slaveLink(gateSlaveSib, netlink.BondLinkDown, lacpSyncOnly), - ) - sleepFn = func(time.Duration) {} - fake := newFakeDispatch() - s := &DispatchSet{backend: fake, mode: xdpmode.Generic} - - missing := s.Reconcile(context.Background(), []string{gateSlave}) - if !slices.Equal(missing, []string{gateSlave}) { - t.Errorf("missing = %v, want [%s]: migrating the only member carrying traffic would take the bond down", - missing, gateSlave) - } - if len(fake.ensured) != 0 { - t.Errorf("ensured %v, want nothing", fake.ensured) - } -} diff --git a/internal/plumbing/ebpf/xdpattach/release.go b/internal/plumbing/ebpf/xdpattach/release.go index 4f7db5c4..ee4baaa9 100644 --- a/internal/plumbing/ebpf/xdpattach/release.go +++ b/internal/plumbing/ebpf/xdpattach/release.go @@ -14,7 +14,6 @@ import ( "slices" "go.datum.net/galactic/internal/plumbing/ebpf/xdpdispatch" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // ErrDispatcherInUse is ReleaseIdleDispatcher refusing to detach a dispatcher @@ -31,15 +30,11 @@ var ErrDispatcherInUse = errors.New("xdpattach: the XDP dispatcher is serving an // on the interface, and detaching it would cut its traffic. The live slots are // read under the dispatch lock, which every Fill takes too, so no datapath can // take a slot between the check and the release. -// -// mode is the host's XDP attach mode. Release detaches a link in either mode. -func ReleaseIdleDispatcher(ctx context.Context, pinDir string, mode xdpmode.Mode, ifaces []string, - own ...xdpdispatch.Slot, -) error { +func ReleaseIdleDispatcher(ctx context.Context, pinDir string, ifaces []string, own ...xdpdispatch.Slot) error { if _, err := os.Stat(filepath.Join(pinDir, "links")); errors.Is(err, os.ErrNotExist) { return nil } - dispatcher, err := xdpdispatch.Open(ctx, pinDir, mode) + dispatcher, err := xdpdispatch.Open(ctx, pinDir) if err != nil { return fmt.Errorf("xdpattach: open the node's XDP dispatcher to release it: %w", err) } @@ -62,11 +57,11 @@ func ReleaseIdleDispatcher(ctx context.Context, pinDir string, mode xdpmode.Mode return fmt.Errorf("xdpattach: find interface %q: %w", name, err) } ifindex := l.Attrs().Index - pinned, err := dispatcher.Pinned(ifindex) + linked, err := dispatcher.Linked(ifindex) if err != nil { return err } - if !pinned { + if !linked { continue } if len(others) > 0 { @@ -91,13 +86,11 @@ func ReleaseIdleDispatcher(ctx context.Context, pinDir string, mode xdpmode.Mode // at once rather than once the slots' leases lapse. Every other slot, and the // dispatcher itself, are left alone. It does nothing on a node that never ran // the dispatcher. -// It opens the dispatcher for slots only, so it never touches the host's -// recorded XDP mode. func ClearDispatcherSlots(ctx context.Context, pinDir string, slots ...xdpdispatch.Slot) error { if _, err := os.Stat(filepath.Join(pinDir, "links")); errors.Is(err, os.ErrNotExist) { return nil } - dispatcher, err := xdpdispatch.OpenForSlots(ctx, pinDir) + dispatcher, err := xdpdispatch.Open(ctx, pinDir) if err != nil { return fmt.Errorf("xdpattach: open the node's XDP dispatcher: %w", err) } diff --git a/internal/plumbing/ebpf/xdpattach/set.go b/internal/plumbing/ebpf/xdpattach/set.go index 45a8bd95..6ff984a7 100644 --- a/internal/plumbing/ebpf/xdpattach/set.go +++ b/internal/plumbing/ebpf/xdpattach/set.go @@ -14,8 +14,6 @@ import ( "github.com/cilium/ebpf" "github.com/cilium/ebpf/link" "github.com/vishvananda/netlink" - - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // linkListFn is an override point, matching linkByNameFn, so the bond sibling @@ -26,10 +24,6 @@ var linkListFn = netlink.LinkList // real program or a real NIC. var attachFn = attachOne -// attachedInModeFn is an override point so Set's tests can report an attach -// mode without a real XDP attachment. -var attachedInModeFn = attachedInMode - // attachedLink is one interface's XDP link, along with the ifindex it was // attached to. The ifindex is what tells a member replaced under the same name // apart from the one this link still points at. @@ -52,7 +46,6 @@ type attachedLink struct { // A Set is safe for concurrent use. type Set struct { program *ebpf.Program - mode xdpmode.Mode mu sync.Mutex links map[string]attachedLink @@ -60,16 +53,15 @@ type Set struct { } // NewSet returns a Set for program that already holds links, the result of an -// Attach of names in mode at startup, in the same order. Every later attach -// uses mode too. -func NewSet(program *ebpf.Program, names []string, links []link.Link, mode xdpmode.Mode) (*Set, error) { +// Attach of names at startup, in the same order. +func NewSet(program *ebpf.Program, names []string, links []link.Link) (*Set, error) { if program == nil { return nil, errors.New("xdpattach: program is nil") } if len(names) != len(links) { return nil, fmt.Errorf("xdpattach: %d interface names for %d links", len(names), len(links)) } - s := &Set{program: program, mode: mode, links: make(map[string]attachedLink, len(names))} + s := &Set{program: program, links: make(map[string]attachedLink, len(names))} for i, name := range names { // An index that cannot be read leaves 0, which no live interface // has, so the next Reconcile re-reads it and releases the link if @@ -108,10 +100,6 @@ func (s *Set) Reconcile(desired []string) []string { blocked := false for _, name := range desired { if _, ok := s.links[name]; ok { - if err := s.checkHeldMode(name); err != nil { - slog.Warn("xdpattach: interface holds the XDP link in the wrong mode", "interface", name, "err", err) - missing = append(missing, name) - } continue } if blocked { @@ -146,38 +134,18 @@ func (s *Set) Missing() []string { return slices.Clone(s.missing) } -// Attached reports every interface currently holding the program in the -// Set's mode, sorted. The mode is read back from the kernel, so an interface -// whose hook holds the program some other way is not reported. +// Attached reports every interface currently holding the program, sorted. func (s *Set) Attached() []string { s.mu.Lock() defer s.mu.Unlock() names := make([]string, 0, len(s.links)) for name := range s.links { - if s.checkHeldMode(name) == nil { - names = append(names, name) - } + names = append(names, name) } slices.Sort(names) return names } -// Mode returns the attach mode the Set attaches in. -func (s *Set) Mode() xdpmode.Mode { return s.mode } - -// checkHeldMode returns an error unless name's hook holds an XDP program in -// the Set's mode. -func (s *Set) checkHeldMode(name string) error { - ok, err := attachedInModeFn(name, s.mode) - if err != nil { - return err - } - if !ok { - return fmt.Errorf("xdpattach: %q does not hold an XDP program in %s mode", name, s.mode) - } - return nil -} - // releaseGone closes the link of every interface that no longer exists, or // exists under the same name with a different ifindex. Either way the link // points at a device the program no longer runs on, and the name is free to be @@ -206,7 +174,7 @@ func (s *Set) releaseGone() { // attachNew attaches the program to one interface not yet in the set, after // the checks that keep doing so from costing a bond its uplink. func (s *Set) attachNew(name string) error { - if err := checkModeSupport([]string{name}, s.mode); err != nil { + if err := checkNativeXDPSupport([]string{name}); err != nil { return err } isolates, err := bounceIsolatesBond(name) @@ -218,7 +186,7 @@ func (s *Set) attachNew(name string) error { "attaching now would take the bond down", name) } - xdpLink, err := attachFn(s.program, name, s.mode) + xdpLink, err := attachFn(s.program, name) if err != nil { return err } @@ -227,8 +195,7 @@ func (s *Set) attachNew(name string) error { index = l.Attrs().Index } s.links[name] = attachedLink{link: xdpLink, index: index} - slog.Info("xdpattach: attached XDP program to interface found after startup", "interface", name, - "mode", s.mode.String()) + slog.Info("xdpattach: attached XDP program to interface found after startup", "interface", name) return nil } diff --git a/internal/plumbing/ebpf/xdpattach/set_test.go b/internal/plumbing/ebpf/xdpattach/set_test.go index 0400b1b4..e30325da 100644 --- a/internal/plumbing/ebpf/xdpattach/set_test.go +++ b/internal/plumbing/ebpf/xdpattach/set_test.go @@ -15,7 +15,6 @@ import ( "github.com/vishvananda/netlink" "go.datum.net/galactic/internal/plumbing/ebpf/edgepreflight" - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // Uplink names Set's and OnNetlinkChange's tests drive. @@ -44,12 +43,6 @@ type setHost struct { attached []string failFor map[string]bool made map[string]*fakeXDPLink - // modes is the attach mode the kernel reports for an interface. One - // with no entry reports whichever mode it is asked about, as a link - // attached at startup does. - modes map[string]xdpmode.Mode - // modesAsked records every mode attachFn was asked for, by interface. - modesAsked map[string]xdpmode.Mode } // newSetHost installs a fake host holding links, every one reporting native @@ -57,13 +50,10 @@ type setHost struct { func newSetHost(t *testing.T, links ...netlink.Link) *setHost { t.Helper() restoreGateVars(t) - origList, origAttach, origMode := linkListFn, attachFn, attachedInModeFn - t.Cleanup(func() { linkListFn, attachFn, attachedInModeFn = origList, origAttach, origMode }) + origList, origAttach := linkListFn, attachFn + t.Cleanup(func() { linkListFn, attachFn = origList, origAttach }) - h := &setHost{ - links: map[string]netlink.Link{}, failFor: map[string]bool{}, made: map[string]*fakeXDPLink{}, - modes: map[string]xdpmode.Mode{}, modesAsked: map[string]xdpmode.Mode{}, - } + h := &setHost{links: map[string]netlink.Link{}, failFor: map[string]bool{}, made: map[string]*fakeXDPLink{}} for _, l := range links { h.links[l.Attrs().Name] = l } @@ -90,18 +80,10 @@ func newSetHost(t *testing.T, links ...netlink.Link) *setHost { } edgepreflight.InterfaceXDPFn = func(string) (bool, error) { return true, nil } sleepFn = func(time.Duration) {} - attachedInModeFn = func(name string, mode xdpmode.Mode) (bool, error) { - if m, ok := h.modes[name]; ok { - return m == mode, nil - } - return true, nil - } - attachFn = func(_ *ebpf.Program, name string, mode xdpmode.Mode) (link.Link, error) { - h.modesAsked[name] = mode + attachFn = func(_ *ebpf.Program, name string) (link.Link, error) { if h.failFor[name] { return nil, errors.New("attach refused") } - h.modes[name] = mode h.attached = append(h.attached, name) l := &fakeXDPLink{name: name} h.made[name] = l @@ -124,7 +106,7 @@ func newTestSet(t *testing.T, names ...string) (*Set, []*fakeXDPLink) { fakes[i] = &fakeXDPLink{name: name} links[i] = fakes[i] } - s, err := NewSet(new(ebpf.Program), names, links, xdpmode.Native) + s, err := NewSet(new(ebpf.Program), names, links) if err != nil { t.Fatalf("NewSet: %v", err) } @@ -265,9 +247,9 @@ func TestSet_AttachesNewlyEnslavedMemberNotYetCarryingTraffic(t *testing.T) { // Comes up once attached: the bond takes it in on the first poll. attach := attachFn - attachFn = func(p *ebpf.Program, name string, _ xdpmode.Mode) (link.Link, error) { + attachFn = func(p *ebpf.Program, name string) (link.Link, error) { h.links[name] = slaveLink(name, netlink.BondLinkUp, lacpCarrying) - return attach(p, name, xdpmode.Native) + return attach(p, name) } if missing := s.Reconcile([]string{gateSlave}); len(missing) != 0 { @@ -284,9 +266,9 @@ func TestSet_WaitsForEachMemberBeforeTheNext(t *testing.T) { // The first member never rejoins after its attach. attach := attachFn - attachFn = func(p *ebpf.Program, name string, _ xdpmode.Mode) (link.Link, error) { + attachFn = func(p *ebpf.Program, name string) (link.Link, error) { h.links[name] = slaveLink(name, netlink.BondLinkUp, lacpSyncOnly) - return attach(p, name, xdpmode.Native) + return attach(p, name) } orig := BondReadyTimeout BondReadyTimeout = 0 @@ -303,10 +285,10 @@ func TestSet_WaitsForEachMemberBeforeTheNext(t *testing.T) { } func TestNewSet_Validates(t *testing.T) { - if _, err := NewSet(nil, nil, nil, xdpmode.Native); err == nil { + if _, err := NewSet(nil, nil, nil); err == nil { t.Error("NewSet(nil program): want an error") } - if _, err := NewSet(new(ebpf.Program), []string{testUplink0}, nil, xdpmode.Native); err == nil { + if _, err := NewSet(new(ebpf.Program), []string{testUplink0}, nil); err == nil { t.Error("NewSet with mismatched names and links: want an error") } } diff --git a/internal/plumbing/ebpf/xdpdispatch/dispatcher.go b/internal/plumbing/ebpf/xdpdispatch/dispatcher.go index 9a198c22..846426f0 100644 --- a/internal/plumbing/ebpf/xdpdispatch/dispatcher.go +++ b/internal/plumbing/ebpf/xdpdispatch/dispatcher.go @@ -11,7 +11,6 @@ import ( "os" "path/filepath" "strconv" - "sync" "time" "github.com/cilium/ebpf" @@ -19,8 +18,6 @@ import ( "github.com/cilium/ebpf/rlimit" "github.com/vishvananda/netlink" "golang.org/x/sys/unix" - - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // PinDir is the bpffs directory every datapath sharing the XDP hook pins its @@ -92,15 +89,8 @@ var ( ErrRoleMissing = errors.New("the interface does not carry the slot's role") ErrSlotNotHeld = errors.New("the slot does not hold this program") ErrLeaseExpired = errors.New("the slot's lease has expired") - ErrModeMismatch = errors.New("the dispatcher link is attached in a different XDP mode than this host selects") ) -// ErrHostModeChanged means another process opened the dispatcher with a -// different XDP mode since this one did, so this process's mode is stale and -// it leaves the links alone rather than move them back. Restart it. -var ErrHostModeChanged = errors.New("xdpdispatch: another process on this host selected a different XDP mode; " + - "restart this one so both agree") - var ( errInvalidSlot = errors.New("xdpdispatch: slot out of range") errNilProgram = errors.New("xdpdispatch: program is nil") @@ -118,17 +108,6 @@ var ( linksDirName = "links" rootPinName = "root" rootStagedPinName = "root_next" // bpffs reserves names containing a dot - // hostModePinName is a one-entry array recording the XDP mode the most - // recently opened dispatcher selected, outside the versioned ABI - // directory since it is no part of the dispatch program's layout. - hostModePinName = "host_mode" -) - -// Migration retries back off per interface, so an attach that keeps failing -// does not bounce an uplink on every reconcile. -const ( - migrationBackoffMin = 30 * time.Second - migrationBackoffMax = 10 * time.Minute ) // Override points for tests. @@ -136,7 +115,6 @@ var ( linkByIndexFn = netlink.LinkByIndex attachXDPFn = link.AttachXDP monotonicNowFn = monotonicNow - wallNowFn = time.Now ) // Dispatcher is an open handle on a node's pinned dispatch state. Closing it @@ -148,26 +126,6 @@ var ( type Dispatcher struct { dir string maps DispatchMaps - // mode is the XDP attach mode this host selected. A link this process - // makes is attached in it, and a pinned link in the other mode is not - // counted as linked or covered until EnsureLink migrates it. - mode xdpmode.Mode - // attaching is false for a dispatcher from OpenForSlots, which attaches - // nothing and has no mode of its own. - attaching bool - // hostMode is the pinned record of the mode the most recently opened - // attaching dispatcher on this host selected. Nil when not attaching. - hostMode *ebpf.Map - - backoffMu sync.Mutex - // backoff holds, per ifindex, when a failed migration may next be tried - // and how long the following wait is. - backoff map[int]migrationRetry -} - -type migrationRetry struct { - next time.Time - delay time.Duration } // Open loads the dispatch state pinned under dir, creating it on first use, @@ -177,23 +135,7 @@ type migrationRetry struct { // // Open takes the dispatch lock itself, waiting until ctx is done, so the // caller must not hold it. -// -// mode is the host's XDP attach mode (xdpmode.Select). Every datapath on a -// host must open the dispatcher with the same mode, which they do by selecting -// it from the same host state. -func Open(ctx context.Context, dir string, mode xdpmode.Mode) (*Dispatcher, error) { - return open(ctx, dir, mode, true) -} - -// OpenForSlots opens the dispatch state like Open, for a caller that only -// fills, clears or renews slots and never attaches, such as a datapath being -// turned off. It records no host mode, so it never overrides the mode the -// attaching datapaths on the host selected, and its EnsureLink refuses to run. -func OpenForSlots(ctx context.Context, dir string) (*Dispatcher, error) { - return open(ctx, dir, xdpmode.Native, false) -} - -func open(ctx context.Context, dir string, mode xdpmode.Mode, attaching bool) (*Dispatcher, error) { +func Open(ctx context.Context, dir string) (*Dispatcher, error) { if err := rlimit.RemoveMemlock(); err != nil { return nil, fmt.Errorf("xdpdispatch: remove memlock rlimit: %w", err) } @@ -232,59 +174,14 @@ func open(ctx context.Context, dir string, mode xdpmode.Mode, attaching bool) (* // The pin, when adoptRoot makes one, keeps the program alive. defer objs.XdpDispatch.Close() //nolint:errcheck // our own descriptor - d := &Dispatcher{dir: dir, maps: objs.DispatchMaps, mode: mode, attaching: attaching, - backoff: map[int]migrationRetry{}} - if attaching { - if d.hostMode, err = recordHostMode(dir, mode); err != nil { - _ = objs.DispatchMaps.Close() - return nil, err - } - } + d := &Dispatcher{dir: dir, maps: objs.DispatchMaps} if err := d.adoptRoot(objs.XdpDispatch); err != nil { - _ = d.Close() + _ = d.maps.Close() return nil, err } return d, nil } -// recordHostMode opens the pinned host mode record under dir, creating it on -// first use, and records mode in it. The caller holds the dispatch lock. The -// record holds the mode plus one, so an entry never written reads as none. -func recordHostMode(dir string, mode xdpmode.Mode) (*ebpf.Map, error) { - path := filepath.Join(dir, hostModePinName) - m, err := ebpf.LoadPinnedMap(path, nil) - if errors.Is(err, os.ErrNotExist) { - m, err = ebpf.NewMap(&ebpf.MapSpec{Name: hostModePinName, Type: ebpf.Array, KeySize: 4, ValueSize: 4, MaxEntries: 1}) - if err == nil { - if pinErr := m.Pin(path); pinErr != nil { - _ = m.Close() - return nil, fmt.Errorf("xdpdispatch: pin host mode record: %w", pinErr) - } - } - } - if err != nil { - return nil, fmt.Errorf("xdpdispatch: open host mode record: %w", err) - } - if err := m.Put(uint32(0), uint32(mode)+1); err != nil { - _ = m.Close() - return nil, fmt.Errorf("xdpdispatch: record host mode: %w", err) - } - return m, nil -} - -// recordedHostMode returns the mode the most recently opened dispatcher on -// this host recorded, and false when none has. -func (d *Dispatcher) recordedHostMode() (xdpmode.Mode, bool, error) { - var v uint32 - if err := d.hostMode.Lookup(uint32(0), &v); err != nil { - return 0, false, fmt.Errorf("xdpdispatch: read host mode record: %w", err) - } - if v == 0 { - return 0, false, nil - } - return xdpmode.Mode(v - 1), true, nil -} - // adoptRoot pins ours as the node's root when the pinned root is older or // missing, and moves every link to it. It records the ABI on first use and // refuses a mismatch. The revision is recorded only once every link has moved, @@ -373,11 +270,7 @@ func (d *Dispatcher) updateLinks(root *ebpf.Program) error { // Close closes this process's descriptors. Pinned state is untouched. func (d *Dispatcher) Close() error { - var err error - if d.hostMode != nil { - err = d.hostMode.Close() - } - return errors.Join(err, d.maps.Close()) + return d.maps.Close() } // Maps returns the pinned dispatch maps, for a datapath whose own programs @@ -414,32 +307,13 @@ func (d *Dispatcher) RootID() (ebpf.ProgramID, error) { return programID(root) } -// Mode returns the XDP attach mode the dispatcher attaches and checks in. -func (d *Dispatcher) Mode() xdpmode.Mode { return d.mode } - -// attachedMode returns the mode ifindex's XDP hook holds a program in, read -// from the kernel, and false when it holds none or holds it some other way. -func attachedMode(ifindex int) (xdpmode.Mode, bool, error) { - nl, err := linkByIndexFn(ifindex) - if err != nil { - return 0, false, fmt.Errorf("xdpdispatch: find interface %d: %w", ifindex, err) - } - xdp := nl.Attrs().Xdp - if xdp == nil || !xdp.Attached { - return 0, false, nil - } - mode, ok := xdpmode.FromAttachMode(xdp.AttachMode) - return mode, ok, nil -} - func (d *Dispatcher) linkPath(ifindex int) string { return filepath.Join(d.dir, linksDirName, strconv.Itoa(ifindex)) } -// Pinned reports whether a live pinned dispatcher link holds ifindex, in -// either mode. Release uses it, since a link in the other mode still holds the -// interface's hook. -func (d *Dispatcher) Pinned(ifindex int) (bool, error) { +// Linked reports whether a live pinned dispatcher link holds ifindex, which +// is when EnsureLink would neither attach nor bounce it. +func (d *Dispatcher) Linked(ifindex int) (bool, error) { pinned, err := link.LoadPinnedLink(d.linkPath(ifindex), nil) if errors.Is(err, os.ErrNotExist) { return false, nil @@ -456,21 +330,6 @@ func (d *Dispatcher) Pinned(ifindex int) (bool, error) { return xdp != nil && int(xdp.Ifindex) == ifindex, nil } -// Linked reports whether a live pinned dispatcher link holds ifindex in the -// dispatcher's mode, which is when EnsureLink would neither attach nor bounce -// it. -func (d *Dispatcher) Linked(ifindex int) (bool, error) { - pinned, err := d.Pinned(ifindex) - if err != nil || !pinned { - return false, err - } - mode, ok, err := attachedMode(ifindex) - if err != nil { - return false, err - } - return ok && mode == d.mode, nil -} - // Locked is the dispatch lock, held. Every call that changes shared state // other than a datapath's own slot program or lease is a method here, so it // cannot run without the lock. Release it with Unlock. @@ -504,13 +363,11 @@ func (l *Locked) Unlock() { } // EnsureLink makes sure the pinned root is attached to ifindex through a -// pinned link, in the dispatcher's mode. bounced reports whether this call -// attached it, which on most drivers resets the interface; the caller then -// waits for a bond member to rejoin. A link running an older root is moved to -// the pinned one. A pinned link in the other mode is migrated (migrateLink), -// which also bounces. A pinned link whose interface is gone is released and -// replaced. An interface held by any other XDP program returns -// ErrForeignProgram and is left alone. +// pinned link. bounced reports whether this call attached it, which on most +// drivers resets the interface; the caller then waits for a bond member to +// rejoin. A link running an older root is moved to the pinned one. A pinned +// link whose interface is gone is released and replaced. An interface held by +// any other XDP program returns ErrForeignProgram and is left alone. // // A link is defunct when the kernel reports its interface index as 0, which it // does once the interface is unregistered. Interface indexes are not reused @@ -518,9 +375,6 @@ func (l *Locked) Unlock() { // different one. func (l *Locked) EnsureLink(ifindex int) (bounced bool, err error) { d := l.d - if !d.attaching { - return false, errors.New("xdpdispatch: EnsureLink on a dispatcher opened for slots only") - } root, err := d.loadRoot() if err != nil { return false, err @@ -544,23 +398,7 @@ func (l *Locked) EnsureLink(ifindex int) (bounced bool, err error) { return false, fmt.Errorf("xdpdispatch: move ifindex %d to the pinned root: %w", ifindex, err) } } - mode, ok, err := attachedMode(ifindex) - if err != nil { - return false, err - } - if ok && mode == d.mode { - return false, nil - } - if !ok { - return false, fmt.Errorf("xdpdispatch: cannot read the XDP mode ifindex %d's pinned link is "+ - "attached in, so it is not moved to %s mode", ifindex, d.mode) - } - if err := d.migrationAllowed(ifindex); err != nil { - return false, err - } - err = l.migrateLink(ifindex, pinned, root, mode) - d.recordMigration(ifindex, err) - return true, err + return false, nil } if err := pinned.Unpin(); err != nil { return false, fmt.Errorf("xdpdispatch: unpin defunct link for ifindex %d: %w", ifindex, err) @@ -578,97 +416,16 @@ func (l *Locked) EnsureLink(ifindex int) (bounced bool, err error) { ErrForeignProgram, nl.Attrs().Name, ifindex, xdp.ProgId) } - if err := attachPinned(root, ifindex, d.mode, path); err != nil { - return true, fmt.Errorf("xdpdispatch: %s: %w", nl.Attrs().Name, err) - } - return true, nil -} - -// attachPinned attaches root to ifindex in mode and pins the link at path. -func attachPinned(root *ebpf.Program, ifindex int, mode xdpmode.Mode, path string) error { - attached, err := attachXDPFn(link.XDPOptions{Program: root, Interface: ifindex, Flags: mode.AttachFlags()}) + attached, err := attachXDPFn(link.XDPOptions{Program: root, Interface: ifindex, Flags: link.XDPDriverMode}) if err != nil { - return fmt.Errorf("attach dispatcher in %s mode: %w", mode, err) + return false, fmt.Errorf("xdpdispatch: attach dispatcher to %s in native/driver mode: %w", + nl.Attrs().Name, err) } defer attached.Close() //nolint:errcheck // our own descriptor; the pin keeps the link if err := attached.Pin(path); err != nil { - return fmt.Errorf("pin link: %w", err) + return true, fmt.Errorf("xdpdispatch: pin link for %s: %w", nl.Attrs().Name, err) } - return nil -} - -// migrationAllowed refuses a migration when another process on the host has -// since recorded a different mode, or while a failed migration of ifindex is -// backing off. The first keeps two processes that disagree from moving one -// link back and forth; the second keeps a failing attach from bouncing an -// uplink on every reconcile. -func (d *Dispatcher) migrationAllowed(ifindex int) error { - recorded, ok, err := d.recordedHostMode() - if err != nil { - return err - } - if ok && recorded != d.mode { - return fmt.Errorf("%w: this process selected %s mode, the host record says %s", - ErrHostModeChanged, d.mode, recorded) - } - d.backoffMu.Lock() - defer d.backoffMu.Unlock() - if r, ok := d.backoff[ifindex]; ok && wallNowFn().Before(r.next) { - return fmt.Errorf("xdpdispatch: moving ifindex %d to %s mode failed recently; retrying after %s", - ifindex, d.mode, r.next.Format(time.RFC3339)) - } - return nil -} - -// recordMigration clears ifindex's backoff after a migration that succeeded, -// and doubles it, from migrationBackoffMin up to migrationBackoffMax, after -// one that failed. -func (d *Dispatcher) recordMigration(ifindex int, err error) { - d.backoffMu.Lock() - defer d.backoffMu.Unlock() - if err == nil { - delete(d.backoff, ifindex) - return - } - delay := migrationBackoffMin - if r, ok := d.backoff[ifindex]; ok { - delay = min(2*r.delay, migrationBackoffMax) - } - d.backoff[ifindex] = migrationRetry{next: wallNowFn().Add(delay), delay: delay} -} - -// migrateLink moves ifindex's pinned link from the mode it is attached in to -// the dispatcher's mode. A bpf_link keeps the mode it was created with, and -// the kernel will not hold a native and a generic program on one interface at -// once, so the link is detached and the root attached again: the interface -// carries no program between the two steps, and the driver resets its rings -// as for a first attach. Only the link changes. The maps, the root, every -// slot, lease and role row stay as they are, so every datapath's slot runs -// again the moment the new link is up. -// -// When the new attach fails, the root is attached again in from, the mode it -// was in, so the interface keeps its traffic, and the failure is returned -// either way. The link stays in the old mode, which Linked and Coverage keep -// reporting, so a later reconcile retries once migrationAllowed lets it. -func (l *Locked) migrateLink(ifindex int, pinned link.Link, root *ebpf.Program, from xdpmode.Mode) error { - d := l.d - path := d.linkPath(ifindex) - if err := pinned.Detach(); err != nil && !errors.Is(err, unix.ENOLINK) { - return fmt.Errorf("xdpdispatch: detach ifindex %d to move it to %s mode: %w", ifindex, d.mode, err) - } - if err := pinned.Unpin(); err != nil { - return fmt.Errorf("xdpdispatch: unpin ifindex %d to move it to %s mode: %w", ifindex, d.mode, err) - } - err := attachPinned(root, ifindex, d.mode, path) - if err == nil { - return nil - } - err = fmt.Errorf("xdpdispatch: move ifindex %d to %s mode: %w", ifindex, d.mode, err) - if restoreErr := attachPinned(root, ifindex, from, path); restoreErr != nil { - return fmt.Errorf("%w; restoring %s mode also failed, so the interface has no dispatcher: %w", - err, from, restoreErr) - } - return fmt.Errorf("%w; restored %s mode", err, from) + return true, nil } // Release detaches the root from ifindex and removes its pin and role row. @@ -946,13 +703,6 @@ func (d *Dispatcher) Coverage(ifindex int, slot Slot, program *ebpf.Program) err if info.Program != rootID { return ErrNotRoot } - mode, ok, err := attachedMode(ifindex) - if err != nil { - return err - } - if !ok || mode != d.mode { - return ErrModeMismatch - } roles, err := d.Roles(ifindex) if err != nil { diff --git a/internal/plumbing/ebpf/xdpdispatch/doc.go b/internal/plumbing/ebpf/xdpdispatch/doc.go index f5922f8a..77e72f53 100644 --- a/internal/plumbing/ebpf/xdpdispatch/doc.go +++ b/internal/plumbing/ebpf/xdpdispatch/doc.go @@ -52,23 +52,6 @@ // Every other call reads the root from its pin, so a process started before // the upgrade follows the newer root instead of moving links back. // -// # Attach mode -// -// The root is attached in the host's XDP mode (internal/plumbing/ebpf/xdpmode), -// which every datapath on the host selects from the same NIC drivers and -// passes to Open. Linked and Coverage read the mode back from the kernel, so a -// pinned link left in the other mode is neither linked nor covered. EnsureLink -// migrates it by detaching the link and attaching the root again in the -// selected mode, since a bpf_link cannot change mode and the kernel will not -// hold a native and a generic program on one interface at once. The maps, the -// root and every slot, lease and role row are untouched. A failed migration -// attaches the root again in the mode it was in and returns the error, and the -// interface's next try backs off. Open records its mode in a pinned host -// record (OpenForSlots, for a caller that only fills or clears slots, does -// not), and a process whose mode no longer matches it returns -// ErrHostModeChanged instead of migrating, so two processes that disagree -// never move a link back and forth. -// // # Locking // // Two processes share the pinned state, so every call that changes the links diff --git a/internal/plumbing/ebpf/xdpdispatch/link_test.go b/internal/plumbing/ebpf/xdpdispatch/link_test.go index f7badb97..03044cb0 100644 --- a/internal/plumbing/ebpf/xdpdispatch/link_test.go +++ b/internal/plumbing/ebpf/xdpdispatch/link_test.go @@ -17,8 +17,6 @@ import ( "github.com/cilium/ebpf" "github.com/cilium/ebpf/link" "github.com/vishvananda/netlink" - - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" ) // testPinDir returns a fresh per-test pin directory under /sys/fs/bpf. @@ -79,7 +77,7 @@ func attachedProgID(t *testing.T, ifindex int) uint32 { func openDispatcher(t *testing.T, dir string) *Dispatcher { t.Helper() - d, err := Open(context.Background(), dir, xdpmode.Native) + d, err := Open(context.Background(), dir) if err != nil { t.Fatalf("Open: %v", err) } @@ -185,7 +183,7 @@ func TestEnsureLink_SurvivesTheOwnerClosing(t *testing.T) { dir := testPinDir(t) ifindex := testVeth(t, "xdpa") - gateway, err := Open(context.Background(), dir, xdpmode.Native) + gateway, err := Open(context.Background(), dir) if err != nil { t.Fatalf("Open: %v", err) } @@ -469,7 +467,7 @@ func TestOpen_RefusesAnotherABI(t *testing.T) { if err := d.maps.DispatchMeta.Put(metaKeyABI, uint64(ABIVersion+1)); err != nil { t.Fatalf("write ABI: %v", err) } - if _, err := Open(context.Background(), dir, xdpmode.Native); !errors.Is(err, ErrIncompatibleLayout) { + if _, err := Open(context.Background(), dir); !errors.Is(err, ErrIncompatibleLayout) { t.Errorf("Open over another ABI = %v, want ErrIncompatibleLayout", err) } } @@ -658,7 +656,7 @@ func TestOpen_UpgradeSkipsADefunctLink(t *testing.T) { t.Fatalf("create stray entry: %v", err) } - upgraded, err := Open(context.Background(), dir, xdpmode.Native) + upgraded, err := Open(context.Background(), dir) if err != nil { t.Fatalf("Open with a defunct link pinned: %v", err) } diff --git a/internal/plumbing/ebpf/xdpdispatch/mode_test.go b/internal/plumbing/ebpf/xdpdispatch/mode_test.go deleted file mode 100644 index 7d6b3e10..00000000 --- a/internal/plumbing/ebpf/xdpdispatch/mode_test.go +++ /dev/null @@ -1,306 +0,0 @@ -// Copyright 2026 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package xdpdispatch - -import ( - "context" - "errors" - "strings" - "testing" - "time" - - "github.com/cilium/ebpf/link" - - "go.datum.net/galactic/internal/plumbing/ebpf/xdpmode" -) - -func openDispatcherInMode(t *testing.T, dir string, mode xdpmode.Mode) *Dispatcher { - t.Helper() - d, err := Open(context.Background(), dir, mode) - if err != nil { - t.Fatalf("Open(%s): %v", mode, err) - } - t.Cleanup(func() { _ = d.Close() }) - return d -} - -// kernelMode returns the mode the kernel reports ifindex's XDP hook in. -func kernelMode(t *testing.T, ifindex int) xdpmode.Mode { - t.Helper() - mode, ok, err := attachedMode(ifindex) - if err != nil { - t.Fatalf("read attach mode of ifindex %d: %v", ifindex, err) - } - if !ok { - t.Fatalf("ifindex %d holds no XDP program in a known mode", ifindex) - } - return mode -} - -// coveredNATSlot fills the NAT slot on ifindex through d and returns a check -// of that slot's coverage through any dispatcher, so a test can watch the slot -// survive a migration. -func coveredNATSlot(t *testing.T, d *Dispatcher, ifindex int) func(*Dispatcher) error { - t.Helper() - nat := stubProgram(t, xdpDrop) - if err := locked(t, d).SetRole(ifindex, RoleEgress); err != nil { - t.Fatalf("SetRole: %v", err) - } - if err := locked(t, d).Fill(SlotNAT, nat); err != nil { - t.Fatalf("Fill: %v", err) - } - return func(through *Dispatcher) error { return through.Coverage(ifindex, SlotNAT, nat) } -} - -func TestEnsureLink_AttachesInTheDispatchersMode(t *testing.T) { - for _, mode := range []xdpmode.Mode{xdpmode.Native, xdpmode.Generic} { - t.Run(mode.String(), func(t *testing.T) { - dir := testPinDir(t) - ifindex := testVeth(t, "xdpm") - d := openDispatcherInMode(t, dir, mode) - - bounced, err := locked(t, d).EnsureLink(ifindex) - if err != nil || !bounced { - t.Fatalf("EnsureLink = %v, %v; want an attach", bounced, err) - } - if got := kernelMode(t, ifindex); got != mode { - t.Errorf("kernel reports %s mode, want %s", got, mode) - } - if linked, err := d.Linked(ifindex); err != nil || !linked { - t.Errorf("Linked = %v, %v; want true", linked, err) - } - if bounced, err := locked(t, d).EnsureLink(ifindex); err != nil || bounced { - t.Errorf("second EnsureLink = %v, %v; want no change", bounced, err) - } - }) - } -} - -// TestEnsureLink_MigratesALinkToTheSelectedMode is the upgrade from native to -// generic mode on a node whose dispatcher already runs: the pinned link is -// moved, and the root, the slot, its lease and the role row come through -// untouched, so the datapath is covered again the moment the new link is up. -func TestEnsureLink_MigratesALinkToTheSelectedMode(t *testing.T) { - dir := testPinDir(t) - ifindex := testVeth(t, "xdpn") - native := openDispatcherInMode(t, dir, xdpmode.Native) - ensureLink(t, native, ifindex) - covered := coveredNATSlot(t, native, ifindex) - if err := covered(native); err != nil { - t.Fatalf("before migration: Coverage = %v", err) - } - rootBefore, err := native.RootID() - if err != nil { - t.Fatal(err) - } - - generic := openDispatcherInMode(t, dir, xdpmode.Generic) - if linked, err := generic.Linked(ifindex); err != nil || linked { - t.Errorf("native link under a generic dispatcher: Linked = %v, %v; want false", linked, err) - } - if pinned, err := generic.Pinned(ifindex); err != nil || !pinned { - t.Errorf("Pinned = %v, %v; want true in either mode", pinned, err) - } - if err := generic.Coverage(ifindex, SlotNAT, nil); !errors.Is(err, ErrModeMismatch) { - t.Errorf("native link under a generic dispatcher: Coverage = %v, want ErrModeMismatch", err) - } - - bounced, err := locked(t, generic).EnsureLink(ifindex) - if err != nil || !bounced { - t.Fatalf("migrating EnsureLink = %v, %v; want a bounce", bounced, err) - } - if got := kernelMode(t, ifindex); got != xdpmode.Generic { - t.Fatalf("after migration the kernel reports %s mode, want generic", got) - } - if linked, err := generic.Linked(ifindex); err != nil || !linked { - t.Errorf("after migration: Linked = %v, %v; want true", linked, err) - } - rootAfter, err := generic.RootID() - if err != nil { - t.Fatal(err) - } - if rootAfter != rootBefore { - t.Errorf("root changed across the migration: %d, then %d", rootBefore, rootAfter) - } - if got := attachedProgID(t, ifindex); got != uint32(rootAfter) { - t.Errorf("ifindex runs program %d after migration, want the root %d", got, rootAfter) - } - roles, err := generic.Roles(ifindex) - if err != nil || roles&RoleEgress == 0 { - t.Errorf("roles after migration = %b, %v; want the egress role kept", roles, err) - } - if err := covered(generic); err != nil { - t.Errorf("after migration: Coverage = %v, want the slot covered again", err) - } - if err := native.Coverage(ifindex, SlotNAT, nil); !errors.Is(err, ErrModeMismatch) { - t.Errorf("a native dispatcher's Coverage after migration = %v, want ErrModeMismatch", err) - } -} - -// TestEnsureLink_FailedMigrationRestoresThePreviousMode covers a migration -// whose new attach fails: the root goes back on in the mode it was in, the -// error is returned, and the link still reports the mismatch so the next -// reconcile retries. -func TestEnsureLink_FailedMigrationRestoresThePreviousMode(t *testing.T) { - dir := testPinDir(t) - ifindex := testVeth(t, "xdpr") - native := openDispatcherInMode(t, dir, xdpmode.Native) - ensureLink(t, native, ifindex) - covered := coveredNATSlot(t, native, ifindex) - - orig := attachXDPFn - t.Cleanup(func() { attachXDPFn = orig }) - attachXDPFn = func(opts link.XDPOptions) (link.Link, error) { - if opts.Flags == link.XDPGenericMode { - return nil, errors.New("injected generic attach failure") - } - return orig(opts) - } - - generic := openDispatcherInMode(t, dir, xdpmode.Generic) - _, err := locked(t, generic).EnsureLink(ifindex) - if err == nil { - t.Fatal("EnsureLink with a failing generic attach: want an error") - } - if msg := err.Error(); !strings.Contains(msg, "injected generic attach failure") || - !strings.Contains(msg, "restored native") { - t.Errorf("error = %v, want the attach failure and the restored mode", err) - } - if got := kernelMode(t, ifindex); got != xdpmode.Native { - t.Errorf("after a failed migration the kernel reports %s mode, want native restored", got) - } - if pinned, err := generic.Pinned(ifindex); err != nil || !pinned { - t.Errorf("after a failed migration: Pinned = %v, %v; want the restored link pinned", pinned, err) - } - if err := generic.Coverage(ifindex, SlotNAT, nil); !errors.Is(err, ErrModeMismatch) { - t.Errorf("after a failed migration: Coverage = %v, want ErrModeMismatch so it is retried", err) - } - if err := covered(native); err != nil { - t.Errorf("after a failed migration the native dispatcher's slot is uncovered: %v", err) - } - - attachXDPFn = orig - if _, err := locked(t, generic).EnsureLink(ifindex); err == nil || !strings.Contains(err.Error(), "retrying after") { - t.Errorf("immediate retry = %v, want it held back by the backoff", err) - } - if got := kernelMode(t, ifindex); got != xdpmode.Native { - t.Errorf("a backed-off retry touched the link: kernel reports %s mode", got) - } - - now := time.Now() - origNow := wallNowFn - t.Cleanup(func() { wallNowFn = origNow }) - wallNowFn = func() time.Time { return now.Add(migrationBackoffMin + time.Second) } - if bounced, err := locked(t, generic).EnsureLink(ifindex); err != nil || !bounced { - t.Fatalf("retried migration = %v, %v; want a bounce", bounced, err) - } - if got := kernelMode(t, ifindex); got != xdpmode.Generic { - t.Errorf("after the retry the kernel reports %s mode, want generic", got) - } - if err := covered(generic); err != nil { - t.Errorf("after the retry: Coverage = %v, want covered", err) - } -} - -// TestEnsureLink_LeavesAForeignGenericProgramAlone: the foreign-program guard -// holds in generic mode too. -func TestEnsureLink_LeavesAForeignGenericProgramAlone(t *testing.T) { - dir := testPinDir(t) - ifindex := testVeth(t, "xdpg") - foreign := stubProgram(t, xdpPass) - fl, err := link.AttachXDP(link.XDPOptions{Program: foreign, Interface: ifindex, Flags: link.XDPGenericMode}) - if err != nil { - t.Fatalf("attach foreign program: %v", err) - } - t.Cleanup(func() { _ = fl.Close() }) - foreignInfo, _ := foreign.Info() - foreignID, _ := foreignInfo.ID() - - for _, mode := range []xdpmode.Mode{xdpmode.Generic, xdpmode.Native} { - d := openDispatcherInMode(t, dir, mode) - if _, err := locked(t, d).EnsureLink(ifindex); !errors.Is(err, ErrForeignProgram) { - t.Errorf("%s dispatcher over a foreign generic program: err = %v, want ErrForeignProgram", mode, err) - } - if got := attachedProgID(t, ifindex); got != uint32(foreignID) { - t.Errorf("%s dispatcher replaced the foreign program: ifindex runs %d, want %d", mode, got, foreignID) - } - } -} - -// TestEnsureLink_LeavesTheLinkToANewerHostMode covers two processes that -// selected different modes, as after a NIC appeared between their starts: the -// one opened first sees the newer record, refuses to move the link back, and -// says it needs a restart. -func TestEnsureLink_LeavesTheLinkToANewerHostMode(t *testing.T) { - dir := testPinDir(t) - ifindex := testVeth(t, "xdph") - generic := openDispatcherInMode(t, dir, xdpmode.Generic) - ensureLink(t, generic, ifindex) - - // A process started later selects native mode and records it. - native := openDispatcherInMode(t, dir, xdpmode.Native) - ensureLink(t, native, ifindex) - if got := kernelMode(t, ifindex); got != xdpmode.Native { - t.Fatalf("the newer process did not move the link: kernel reports %s mode", got) - } - - if _, err := locked(t, generic).EnsureLink(ifindex); !errors.Is(err, ErrHostModeChanged) { - t.Errorf("stale process EnsureLink = %v, want ErrHostModeChanged", err) - } - if got := kernelMode(t, ifindex); got != xdpmode.Native { - t.Errorf("the stale process moved the link back: kernel reports %s mode", got) - } -} - -func TestBackoff_DoublesUpToTheCap(t *testing.T) { - d := &Dispatcher{backoff: map[int]migrationRetry{}} - now := time.Unix(1000, 0) - origNow := wallNowFn - t.Cleanup(func() { wallNowFn = origNow }) - wallNowFn = func() time.Time { return now } - - want := migrationBackoffMin - for range 10 { - d.recordMigration(7, errors.New("attach failed")) - if got := d.backoff[7].delay; got != want { - t.Fatalf("delay = %s, want %s", got, want) - } - want = min(2*want, migrationBackoffMax) - } - d.recordMigration(7, nil) - if _, ok := d.backoff[7]; ok { - t.Error("a successful migration kept its backoff") - } -} - -// TestOpenForSlots_LeavesTheHostModeAlone covers a datapath turned off on a -// generic host: clearing its slots must not record a mode, or every running -// datapath would stop migrating links, and it can never attach. -func TestOpenForSlots_LeavesTheHostModeAlone(t *testing.T) { - dir := testPinDir(t) - ifindex := testVeth(t, "xdps") - generic := openDispatcherInMode(t, dir, xdpmode.Generic) - - slots, err := OpenForSlots(context.Background(), dir) - if err != nil { - t.Fatalf("OpenForSlots: %v", err) - } - t.Cleanup(func() { _ = slots.Close() }) - if err := locked(t, slots).Clear(SlotNAT); err != nil { - t.Fatalf("Clear: %v", err) - } - if _, err := locked(t, slots).EnsureLink(ifindex); err == nil { - t.Error("EnsureLink on a slots-only dispatcher: want an error") - } - - recorded, ok, err := generic.recordedHostMode() - if err != nil || !ok || recorded != xdpmode.Generic { - t.Errorf("host mode after OpenForSlots = %s, %v, %v; want generic", recorded, ok, err) - } - ensureLink(t, generic, ifindex) - if got := kernelMode(t, ifindex); got != xdpmode.Generic { - t.Errorf("kernel reports %s mode, want generic", got) - } -} diff --git a/internal/plumbing/ebpf/xdpmode/mode.go b/internal/plumbing/ebpf/xdpmode/mode.go deleted file mode 100644 index 4fa614d8..00000000 --- a/internal/plumbing/ebpf/xdpmode/mode.go +++ /dev/null @@ -1,190 +0,0 @@ -// Copyright 2026 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -// Package xdpmode decides, once per host, which XDP attach mode every -// Galactic XDP attachment on that host uses. -// -// Native (driver) mode is the default. A host with any NIC driven by bnxt_en -// uses generic (SKB) mode for every attachment instead, virtual interfaces -// included, so the host never mixes native and generic redirect paths. -// Native XDP redirect traffic on bnxt_en is associated with receive queues -// that stall permanently (#673). The suspected driver defect suppresses RX -// producer updates while the paired XDP TX ring is busy; generic mode runs -// the same programs after the driver has built an skb, so the XDP TX ring is -// never used. The cost is per-packet throughput, since generic mode allocates -// an skb for every frame. -package xdpmode - -import ( - "errors" - "fmt" - "os" - "path/filepath" - "slices" - "strings" - - "github.com/cilium/ebpf/link" - "github.com/vishvananda/netlink/nl" -) - -// Mode is an XDP attach mode. -type Mode int - -const ( - // Native attaches in driver mode, the program running in the NIC - // driver's receive path before an skb exists. - Native Mode = iota - // Generic attaches in SKB mode, the program running in the core - // receive path after the driver has built an skb. - Generic -) - -// String returns the mode's name as logs and docs use it. -func (m Mode) String() string { - switch m { - case Native: - return "native" - case Generic: - return "generic" - default: - return fmt.Sprintf("Mode(%d)", int(m)) - } -} - -// AttachFlags returns the flags that request m from link.AttachXDP. Each mode -// names its flag explicitly, so the kernel never picks a mode on its own. -func (m Mode) AttachFlags() link.XDPAttachFlags { - if m == Generic { - return link.XDPGenericMode - } - return link.XDPDriverMode -} - -// Matches reports whether attachMode, an interface's IFLA_XDP_ATTACHED value -// as netlink.LinkXdp.AttachMode reports it, is m. -func (m Mode) Matches(attachMode uint32) bool { - switch m { - case Native: - return attachMode == nl.XDP_ATTACHED_DRV - case Generic: - return attachMode == nl.XDP_ATTACHED_SKB - default: - return false - } -} - -// FromAttachMode returns the Mode for an interface's IFLA_XDP_ATTACHED value, -// and false for none, hardware offload, or several programs at once. -func FromAttachMode(attachMode uint32) (Mode, bool) { - switch attachMode { - case nl.XDP_ATTACHED_DRV: - return Native, true - case nl.XDP_ATTACHED_SKB: - return Generic, true - default: - return 0, false - } -} - -// GenericDrivers are the NIC drivers whose presence on a host selects generic -// mode for every attachment on it. -var GenericDrivers = []string{"bnxt_en"} - -// Decision is Select's result: the mode, why, and what it was based on. -type Decision struct { - Mode Mode - // Drivers maps every physical NIC on the host to its driver. - Drivers map[string]string - // Reason says, in a sentence for a log line, why Mode was chosen. - Reason string - // Skipped lists physical NICs left out because their driver link was - // gone, as while a driver unbinds, each with why. None is a target. - Skipped []string -} - -// sysClassNet is where Select reads interfaces and their drivers from. -// Galactic's XDP processes run in the host network namespace, so it lists the -// host's interfaces. Tests point it elsewhere. -var sysClassNet = "/sys/class/net" - -// Select decides the host's attach mode from the drivers of its physical NICs -// in sysfs. required lists the interfaces the caller is about to attach to; -// one that does not exist fails the decision, as does a required NIC whose -// driver cannot be read, or any NIC whose driver link exists but cannot be -// read, so none of them ever settles a mode by omission. A NIC that is not a -// target and has no driver link at all, as while its driver unbinds, carries -// no driver traffic; it is left out and listed in Skipped. An interface with -// no device link, such as a bond master, a veth or a VLAN, is virtual and has -// no driver of its own to consider. -func Select(required ...string) (Decision, error) { - entries, err := os.ReadDir(sysClassNet) - if err != nil { - return Decision{}, fmt.Errorf("xdpmode: list interfaces in %s: %w", sysClassNet, err) - } - present := make(map[string]bool, len(entries)) - drivers := map[string]string{} - var ( - errs []error - skipped []string - ) - for _, e := range entries { - name := e.Name() - present[name] = true - driver, physical, err := interfaceDriver(name) - switch { - case err != nil && errors.Is(err, os.ErrNotExist) && !slices.Contains(required, name): - skipped = append(skipped, err.Error()) - case err != nil: - errs = append(errs, err) - case physical: - drivers[name] = driver - } - } - for _, name := range required { - if !present[name] { - errs = append(errs, fmt.Errorf("xdpmode: interface %s does not exist", name)) - } - } - if len(errs) > 0 { - return Decision{}, errors.Join(errs...) - } - d := decide(drivers) - d.Skipped = skipped - return d, nil -} - -// interfaceDriver returns name's driver and whether it is a physical NIC, -// which is when sysfs gives it a device link. -func interfaceDriver(name string) (driver string, physical bool, err error) { - dir := filepath.Join(sysClassNet, name) - if _, err := os.Lstat(filepath.Join(dir, "device")); err != nil { - if errors.Is(err, os.ErrNotExist) { - return "", false, nil - } - return "", false, fmt.Errorf("xdpmode: read device of %s: %w", name, err) - } - target, err := os.Readlink(filepath.Join(dir, "device", "driver")) - if err != nil { - return "", true, fmt.Errorf("xdpmode: read driver of %s: %w", name, err) - } - return filepath.Base(target), true, nil -} - -// decide picks the mode for a host whose physical NICs have drivers. -func decide(drivers map[string]string) Decision { - var matched []string - for name, driver := range drivers { - if slices.Contains(GenericDrivers, driver) { - matched = append(matched, name+"="+driver) - } - } - if len(matched) == 0 { - return Decision{Mode: Native, Drivers: drivers, - Reason: "no NIC on this host uses a driver that needs generic mode"} - } - slices.Sort(matched) - return Decision{Mode: Generic, Drivers: drivers, - Reason: "NICs on this host use a driver whose native XDP redirect stalls receive queues (#673): " + - strings.Join(matched, ", ")} -} diff --git a/internal/plumbing/ebpf/xdpmode/mode_test.go b/internal/plumbing/ebpf/xdpmode/mode_test.go deleted file mode 100644 index 247a4861..00000000 --- a/internal/plumbing/ebpf/xdpmode/mode_test.go +++ /dev/null @@ -1,271 +0,0 @@ -// Copyright 2026 Datum Cloud, Inc. -// -// SPDX-License-Identifier: AGPL-3.0-or-later - -package xdpmode - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "github.com/cilium/ebpf/link" - "github.com/vishvananda/netlink/nl" -) - -// Interface names the fixtures reuse. -const ( - testNIC1 = "eno1" - testNIC2 = "eno2" -) - -// fakeSysfs builds a /sys/class/net look-alike under a temp dir and points -// sysClassNet at it. Each physical interface gets a device link whose driver -// link ends in its driver's name, the way sysfs lays out a PCI NIC; each -// virtual one gets a directory and nothing else. -type fakeSysfs struct { - t *testing.T - root string -} - -func newFakeSysfs(t *testing.T) *fakeSysfs { - t.Helper() - root := t.TempDir() - net := filepath.Join(root, "class", "net") - if err := os.MkdirAll(net, 0o755); err != nil { - t.Fatal(err) - } - old := sysClassNet - sysClassNet = net - t.Cleanup(func() { sysClassNet = old }) - return &fakeSysfs{t: t, root: root} -} - -func (f *fakeSysfs) physical(name, driver string) { - f.t.Helper() - dev := filepath.Join(f.root, "devices", name) - drv := filepath.Join(f.root, "bus", "pci", "drivers", driver) - for _, dir := range []string{dev, drv, filepath.Join(sysClassNet, name)} { - if err := os.MkdirAll(dir, 0o755); err != nil { - f.t.Fatal(err) - } - } - if err := os.Symlink(drv, filepath.Join(dev, "driver")); err != nil { - f.t.Fatal(err) - } - if err := os.Symlink(dev, filepath.Join(sysClassNet, name, "device")); err != nil { - f.t.Fatal(err) - } -} - -// unbound adds a physical interface whose device has no driver link, which -// Select must report rather than treat as virtual. -func (f *fakeSysfs) unbound(name string) { - f.t.Helper() - dev := filepath.Join(f.root, "devices", name) - if err := os.MkdirAll(dev, 0o755); err != nil { - f.t.Fatal(err) - } - if err := os.MkdirAll(filepath.Join(sysClassNet, name), 0o755); err != nil { - f.t.Fatal(err) - } - if err := os.Symlink(dev, filepath.Join(sysClassNet, name, "device")); err != nil { - f.t.Fatal(err) - } -} - -func (f *fakeSysfs) virtual(name string) { - f.t.Helper() - if err := os.MkdirAll(filepath.Join(sysClassNet, name), 0o755); err != nil { - f.t.Fatal(err) - } -} - -func TestSelect(t *testing.T) { - tests := []struct { - name string - build func(f *fakeSysfs) - required []string - want Mode - wantReason string - }{ - { - name: "no bnxt_en keeps native", - build: func(f *fakeSysfs) { - f.physical(testNIC1, "igb") - f.physical(testNIC2, "ice") - f.virtual("lo") - f.virtual("bond0") - }, - required: []string{testNIC1, testNIC2}, - want: Native, - }, - { - name: "all bnxt_en selects generic", - build: func(f *fakeSysfs) { - f.physical("ens1f0np0", "bnxt_en") - f.physical("ens3f0np0", "bnxt_en") - f.virtual("bond1") - }, - required: []string{"ens1f0np0", "ens3f0np0"}, - want: Generic, - wantReason: "ens1f0np0=bnxt_en, ens3f0np0=bnxt_en", - }, - { - name: "one bnxt_en NIC among others selects generic for the whole host", - build: func(f *fakeSysfs) { - f.physical(testNIC1, "igb") - f.physical("ens1f0np0", "bnxt_en") - }, - required: []string{testNIC1}, - want: Generic, - wantReason: "ens1f0np0=bnxt_en", - }, - { - name: "virtual targets follow the host's NICs", - build: func(f *fakeSysfs) { - f.physical("ens1f0np0", "bnxt_en") - f.virtual("veth-return") - }, - required: []string{"veth-return"}, - want: Generic, - }, - { - name: "a host of virtual interfaces only stays native", - build: func(f *fakeSysfs) { - f.virtual("lo") - f.virtual("eth0") - }, - required: []string{"eth0"}, - want: Native, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f := newFakeSysfs(t) - tt.build(f) - got, err := Select(tt.required...) - if err != nil { - t.Fatalf("Select: %v", err) - } - if got.Mode != tt.want { - t.Errorf("Mode = %s, want %s (reason %q)", got.Mode, tt.want, got.Reason) - } - if !strings.Contains(got.Reason, tt.wantReason) { - t.Errorf("Reason = %q, want it to name %q", got.Reason, tt.wantReason) - } - }) - } -} - -func TestSelect_ReportsWhatItCannotDecideOn(t *testing.T) { - tests := []struct { - name string - build func(f *fakeSysfs) - required []string - wantErr []string - }{ - { - name: "a missing target interface", - build: func(f *fakeSysfs) { f.physical(testNIC1, "igb") }, - required: []string{testNIC1, "eno9"}, - wantErr: []string{"interface eno9 does not exist"}, - }, - { - name: "a target NIC whose driver cannot be read", - build: func(f *fakeSysfs) { - f.physical(testNIC1, "igb") - f.unbound(testNIC2) - }, - required: []string{testNIC1, testNIC2}, - wantErr: []string{"read driver of eno2"}, - }, - { - name: "both at once", - build: func(f *fakeSysfs) { - f.physical("ens1f0np0", "bnxt_en") - f.unbound(testNIC2) - }, - required: []string{testNIC2, "bond9"}, - wantErr: []string{"read driver of eno2", "interface bond9 does not exist"}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - f := newFakeSysfs(t) - tt.build(f) - _, err := Select(tt.required...) - if err == nil { - t.Fatal("Select: want an error") - } - for _, want := range tt.wantErr { - if !strings.Contains(err.Error(), want) { - t.Errorf("error %q does not mention %q", err, want) - } - } - }) - } -} - -func TestSelect_UnreadableSysfs(t *testing.T) { - old := sysClassNet - sysClassNet = filepath.Join(t.TempDir(), "missing") - t.Cleanup(func() { sysClassNet = old }) - if _, err := Select(); err == nil { - t.Fatal("Select over a missing sysfs: want an error") - } -} - -func TestModeFlagsAndAttachMode(t *testing.T) { - if got := Native.AttachFlags(); got != link.XDPDriverMode { - t.Errorf("Native.AttachFlags() = %v, want XDPDriverMode", got) - } - if got := Generic.AttachFlags(); got != link.XDPGenericMode { - t.Errorf("Generic.AttachFlags() = %v, want XDPGenericMode", got) - } - for _, tt := range []struct { - attachMode uint32 - mode Mode - ok bool - }{ - {nl.XDP_ATTACHED_DRV, Native, true}, - {nl.XDP_ATTACHED_SKB, Generic, true}, - {nl.XDP_ATTACHED_NONE, 0, false}, - {nl.XDP_ATTACHED_HW, 0, false}, - } { - mode, ok := FromAttachMode(tt.attachMode) - if ok != tt.ok || (ok && mode != tt.mode) { - t.Errorf("FromAttachMode(%d) = %s, %v; want %s, %v", tt.attachMode, mode, ok, tt.mode, tt.ok) - } - if ok && !tt.mode.Matches(tt.attachMode) { - t.Errorf("%s.Matches(%d) = false", tt.mode, tt.attachMode) - } - } - if Native.Matches(nl.XDP_ATTACHED_SKB) || Generic.Matches(nl.XDP_ATTACHED_DRV) { - t.Error("a mode matched the other mode's attach value") - } - if Native.String() != "native" || Generic.String() != "generic" { - t.Errorf("String() = %q, %q", Native, Generic) - } -} - -// TestSelect_SkipsANonTargetNICWithNoDriver covers a NIC caught mid-unbind -// that the datapath does not attach to: it carries no driver traffic, so it -// is listed as skipped rather than failing startup, and the rest decide. -func TestSelect_SkipsANonTargetNICWithNoDriver(t *testing.T) { - f := newFakeSysfs(t) - f.physical("ens1f0np0", "bnxt_en") - f.unbound(testNIC2) - - got, err := Select("ens1f0np0") - if err != nil { - t.Fatalf("Select: %v", err) - } - if got.Mode != Generic { - t.Errorf("Mode = %s, want generic", got.Mode) - } - if len(got.Skipped) != 1 || !strings.Contains(got.Skipped[0], "read driver of eno2") { - t.Errorf("Skipped = %v, want eno2 named", got.Skipped) - } -}