From 56987875681395c7a87135e7166e489018ada671 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:36:28 -0600 Subject: [PATCH 01/23] build(nix): keep debug outputs lean Debug binaries retained the complete Rust toolchain through their standard-library source paths, adding roughly 2.4 GB to the closure. They also carried a sizable DWARF index that neither packaged debugger consumes. Point those paths at the much smaller rust-src component, omit unused prebuilt documentation, and remove .debug_names. Source browsing and symbols remain available while the resulting diagnostic images become practical to store and transfer. Co-authored-by: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- default.nix | 4 ++++ nix/overlays/llvm.nix | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index acadbf858e..edf7ef3154 100644 --- a/default.nix +++ b/default.nix @@ -379,6 +379,8 @@ let # debug/release-with-debug-tools containers. Then, connecting from the gdb/lldb container to the # gdb/lldbserver container should allow us to actually debug binaries deployed to test machines. "--remap-path-prefix==${src}" + # Keep debug outputs from retaining the complete Rust toolchain. + "--remap-path-prefix=${pkgs.rust-toolchain}/lib/rustlib/src/rust=${pkgs.rust-toolchain.passthru.availableComponents.rust-src}/lib/rustlib/src/rust" ] ) else @@ -407,6 +409,8 @@ let mkdir -p $debug/bin for f in $out/bin/*; do mv "$f" "$debug/bin/$(basename "$f")" + # Neither packaged debugger reads `.debug_names`. + ${objcopy} --remove-section=.debug_names "$debug/bin/$(basename "$f")" ${strip} --strip-debug "$debug/bin/$(basename "$f")" -o "$f" ${objcopy} --add-gnu-debuglink="$debug/bin/$(basename "$f")" "$f" done diff --git a/nix/overlays/llvm.nix b/nix/overlays/llvm.nix index 334be96d21..9b7c46d39c 100644 --- a/nix/overlays/llvm.nix +++ b/nix/overlays/llvm.nix @@ -36,7 +36,7 @@ let "clippy" "llvm-tools" "rust-analyzer" - "rust-docs" + # Avoid retaining 680M of unused prebuilt documentation. "rust-src" "rust-std" "rustc" From c5a2014459a1a44a2ca32613bb36477e45c6ed0e Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:37:45 -0600 Subject: [PATCH 02/23] feat(debug): add a version-matched core viewer A core collected from the lab is useful only with the exact unstripped binaries and sources that produced it. A general debugging toolbox cannot reconstruct that relationship after the release has moved on. Provide a purpose-built gdb image alongside each build and teach it Rust's standard-library types without retaining rustc. This keeps post-mortem debugging reproducible while avoiding unrelated live-debugging tools. Co-authored-by: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- .github/workflows/README.md | 15 +++++++++- .github/workflows/dev.yml | 4 +++ default.nix | 57 ++++++++++++++++++++++++++++--------- justfile | 19 +++++++------ 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index d41d5b6a72..50273b6e44 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -93,7 +93,8 @@ If those queue failures stop being rare, the phasing is worth revisiting. - Checks: `debug` by default; `release` and `fuzz` on deep runs - Coverage: `debug` by default; `fuzz` on deep runs - Miri: required on deep runs; opt-in on pull requests with `ci:+miri` -- Containers: debug/release for dataplane and FRR; release for validator +- Containers: debug/release for dataplane, its debugger, and FRR; release for + validator - VLAB configurations: spine-leaf fabric mode, L2VNI/L3VNI VPC modes, with gateway enabled @@ -101,6 +102,18 @@ If those queue failures stop being rare, the phasing is worth revisiting. - Container images pushed to GitHub Container Registry (GHCR) - Release containers published on tag pushes via `just push` +- `ghcr.io/githedgehog/dataplane/core-viewer` opens a core file from the lab. + It carries gdb plus the unstripped binaries and sources for the matching + `ghcr.io/githedgehog/dataplane` build. + Pull the tag matching the build the core came from; symbols only line up with + the exact version and profile that produced it. + The entrypoint takes the core as its only argument: + + ```console + docker run --rm -it -v /path/to/cores:/cores \ + ghcr.io/githedgehog/dataplane/core-viewer:TAG /cores/core.1234 + ``` + - Coverage reports from each `coverage/` job, kept for 7 days: - `coverage-html-.tar.gz` - `llvm-cov` HTML report, including the per-branch counts that Codecov does not render. Unpack and open diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 86fe9fb473..1efc34c582 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -370,6 +370,8 @@ jobs: nix-target: - frr.dataplane - dataplane + # Must match the build that produced the core. + - dataplane-core-viewer - validator # TODO: enable cfi and safe-stack on release when possible profile: "${{ fromJSON(needs.plan.outputs.container_profiles) }}" @@ -378,6 +380,8 @@ jobs: profile: fuzz - nix-target: dataplane profile: fuzz + - nix-target: dataplane-core-viewer + profile: fuzz - nix-target: validator profile: debug - nix-target: validator diff --git a/default.nix b/default.nix index edf7ef3154..c3cfde89c3 100644 --- a/default.nix +++ b/default.nix @@ -795,11 +795,32 @@ let config.Entrypoint = [ "/bin/dataplane" ]; }; - containers.dataplane-debugger = pkgs.dockerTools.buildLayeredImage { - name = "ghcr.io/githedgehog/dataplane/debugger"; + # Shared runtime and unstripped binaries for the debugger images. + debug-image-paths = [ + pkgs.pkgsBuildHost.coreutils + pkgs.pkgsBuildHost.bashInteractive + pkgs.pkgsHostHost.dockerTools.usrBinEnv + + pkgs.pkgsHostHost.libc.debug + workspace.cli.debug + workspace.dataplane.debug + workspace.init.debug + ]; + + # Copy Rust's gdb helpers without retaining rustc as a runtime dependency. + rust-gdb-printers = pkgs.runCommand "rust-gdb-printers" { } '' + mkdir -p "$out/lib/rustlib/etc" + for f in gdb_load_rust_pretty_printers.py gdb_lookup.py gdb_providers.py rust_types.py; do + cp -L "${pkgs.rust-toolchain}/lib/rustlib/etc/$f" "$out/lib/rustlib/etc/$f" + done + ''; + + # Opens dataplane core files with matching symbols and sources. + containers.dataplane-core-viewer = pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane/core-viewer"; inherit tag; contents = pkgs.buildEnv { - name = "dataplane-debugger-env"; + name = "dataplane-core-viewer-env"; pathsToLink = [ "/bin" "/etc" @@ -808,18 +829,26 @@ let ]; paths = [ pkgs.pkgsBuildHost.gdb - pkgs.pkgsBuildHost.rr - pkgs.pkgsBuildHost.coreutils - pkgs.pkgsBuildHost.bashInteractive - pkgs.pkgsBuildHost.iproute2 - pkgs.pkgsBuildHost.ethtool - pkgs.pkgsHostHost.dockerTools.usrBinEnv - - pkgs.pkgsHostHost.libc.debug - workspace.cli.debug - workspace.dataplane.debug - workspace.init.debug + rust-gdb-printers + ] + ++ debug-image-paths; + }; + # gdb needs a writable HOME for logs and its index cache. + extraCommands = '' + mkdir -p tmp + chmod 1777 tmp + ''; + config = { + Entrypoint = [ + "/bin/gdb" + "--directory=/lib/rustlib/etc" + "-iex" + "add-auto-load-safe-path /lib/rustlib/etc" + "-iex" + "source /lib/rustlib/etc/gdb_load_rust_pretty_printers.py" + "/bin/dataplane" ]; + Env = [ "HOME=/tmp" ]; }; }; diff --git a/justfile b/justfile index eace8d7db0..3d75866efe 100644 --- a/justfile +++ b/justfile @@ -140,7 +140,7 @@ oci_insecure := "" oci_name := "githedgehog/dataplane" oci_frr_prefix := "githedgehog/dataplane/frr" oci_image_dataplane := oci_repo + "/" + oci_name + ":" + version -oci_image_dataplane_debugger := oci_repo + "/" + oci_name + "/debugger:" + version +oci_image_dataplane_core_viewer := oci_repo + "/" + oci_name + "/core-viewer:" + version oci_image_dataplane_validator := oci_repo + "/" + oci_name + "/validator:" + version oci_image_frr_dataplane := oci_repo + "/" + oci_frr_prefix + ":" + version oci_image_frr_host := oci_repo + "/" + oci_frr_prefix + "-host:" + version @@ -305,10 +305,10 @@ build-container target="dataplane" *args: (build (if target == "dataplane" { "da docker tag "${img}" "{{oci_image_dataplane}}" echo "imported {{ oci_image_dataplane }} (${docker_platform})" ;; - "dataplane-debugger") - docker load < ./results/containers.dataplane-debugger - docker tag "ghcr.io/githedgehog/dataplane/debugger:{{version}}" "{{oci_image_dataplane_debugger}}" - echo "imported {{ oci_image_dataplane_debugger }}" + "dataplane-core-viewer") + docker load < ./results/containers.dataplane-core-viewer + docker tag "ghcr.io/githedgehog/dataplane/core-viewer:{{version}}" "{{oci_image_dataplane_core_viewer}}" + echo "imported {{ oci_image_dataplane_core_viewer }}" ;; "debug-tools") # Uses nix only to produce a base image with the runtime closure (glibc, bash, etc.) @@ -360,9 +360,9 @@ push-container target="dataplane" *args: (build-container target args) && versio skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane }}" "docker://{{ oci_image_dataplane }}" echo "Pushed {{ oci_image_dataplane }}" ;; - "dataplane-debugger") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_debugger }}" "docker://{{ oci_image_dataplane_debugger }}" - echo "Pushed {{ oci_image_dataplane_debugger }}" + "dataplane-core-viewer") + skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_core_viewer }}" "docker://{{ oci_image_dataplane_core_viewer }}" + echo "Pushed {{ oci_image_dataplane_core_viewer }}" ;; "debug-tools") >&2 echo "do not push the debug tools!" @@ -395,7 +395,8 @@ push-container target="dataplane" *args: (build-container target args) && versio [script] push: {{ _just_debuggable_ }} - for container in dataplane frr.dataplane validator; do + # The core viewer must match the release it inspects. + for container in dataplane dataplane-core-viewer frr.dataplane validator; do if [ "${container}" = "validator" ]; then platform="wasm32-wasip1" else From a97e360417d8f03c8f02f13ad22771410c07ed1e Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:38:11 -0600 Subject: [PATCH 03/23] feat(debug): add a live DAP debugger Post-mortem inspection and live debugging need different tools. The core viewer cannot offer an editor-driven session, while bugstalker understands Rust layouts and can expose the running dataplane through the Debug Adapter Protocol. Track bugstalker upstream for its current remote DAP support and package it separately with the matching binaries and sources. Keeping the image single-purpose avoids making every diagnostic artifact carry every debugger. Co-authored-by: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- .github/workflows/README.md | 45 ++++++++++++++++++++++++++++++++-- .github/workflows/dev.yml | 4 +++ default.nix | 39 +++++++++++++++++++++++++++++ justfile | 10 ++++++++ nix/overlays/dataplane-dev.nix | 9 +++++++ npins/sources.json | 16 ++++++++++++ 6 files changed, 121 insertions(+), 2 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 50273b6e44..ce4e18984e 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -93,8 +93,8 @@ If those queue failures stop being rare, the phasing is worth revisiting. - Checks: `debug` by default; `release` and `fuzz` on deep runs - Coverage: `debug` by default; `fuzz` on deep runs - Miri: required on deep runs; opt-in on pull requests with `ci:+miri` -- Containers: debug/release for dataplane, its debugger, and FRR; release for - validator +- Containers: debug/release for dataplane, its two debug images, and FRR; + release for validator - VLAB configurations: spine-leaf fabric mode, L2VNI/L3VNI VPC modes, with gateway enabled @@ -114,6 +114,47 @@ If those queue failures stop being rare, the phasing is worth revisiting. ghcr.io/githedgehog/dataplane/core-viewer:TAG /cores/core.1234 ``` +- `ghcr.io/githedgehog/dataplane/dev-debugger` debugs a live dataplane from an + editor. It carries bugstalker, which understands Rust's std collections and + enum layouts, and listens for a Debug Adapter Protocol client on port 4711. + Publish the port and point the editor's DAP client at it: + + ```console + docker run --rm -p 4711:4711 ghcr.io/githedgehog/dataplane/dev-debugger:TAG + ``` + + Connecting does not by itself start anything. In remote-DAP mode bugstalker + waits for the client's `launch` request to name the program, so the editor + has to send `program`, and any dataplane arguments as `args`. A request + without `program` is rejected with `launch: missing arguments.program`. + For VS Code, in `.vscode/launch.json`: + + ```json + { + "type": "bs", + "request": "launch", + "name": "dataplane (container)", + "debugServer": 4711, + "program": "/bin/dataplane", + "args": [] + } + ``` + + For `nvim-dap`: + + ```lua + dap.adapters.bs = { type = "server", host = "127.0.0.1", port = 4711 } + dap.configurations.rust = { + { + type = "bs", + request = "launch", + name = "dataplane (container)", + program = "/bin/dataplane", + args = {}, + }, + } + ``` + - Coverage reports from each `coverage/` job, kept for 7 days: - `coverage-html-.tar.gz` - `llvm-cov` HTML report, including the per-branch counts that Codecov does not render. Unpack and open diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 1efc34c582..bbd718e04a 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -372,6 +372,8 @@ jobs: - dataplane # Must match the build that produced the core. - dataplane-core-viewer + # Live debugging of the same build, driven from an editor over DAP. + - dataplane-dev-debugger - validator # TODO: enable cfi and safe-stack on release when possible profile: "${{ fromJSON(needs.plan.outputs.container_profiles) }}" @@ -382,6 +384,8 @@ jobs: profile: fuzz - nix-target: dataplane-core-viewer profile: fuzz + - nix-target: dataplane-dev-debugger + profile: fuzz - nix-target: validator profile: debug - nix-target: validator diff --git a/default.nix b/default.nix index c3cfde89c3..0e5451d7e8 100644 --- a/default.nix +++ b/default.nix @@ -852,6 +852,45 @@ let }; }; + # Exposes bugstalker's DAP server for live debugging. + containers.dataplane-dev-debugger = pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane/dev-debugger"; + inherit tag; + contents = pkgs.buildEnv { + name = "dataplane-dev-debugger-env"; + pathsToLink = [ + "/bin" + "/etc" + "/var" + "/lib" + ]; + paths = [ pkgs.pkgsBuildHost.bugstalker ] ++ debug-image-paths; + }; + # bugstalker needs a writable HOME for its keymap and history. + extraCommands = '' + mkdir -p tmp + chmod 1777 tmp + ''; + config = { + Entrypoint = [ + "/bin/bs" + # Bind the published interface rather than container-local loopback. + "--dap-remote=0.0.0.0:4711" + # rustc is absent, so bugstalker cannot infer this path. + "--std-lib-path=${pkgs.rust-toolchain.passthru.availableComponents.rust-src}/lib/rustlib/src/rust" + # No debuggee here on purpose. In `--dap-remote` mode bugstalker + # ignores the CLI debuggee and waits for the client's `launch` request + # to name one, so a path here would be silently dead and would imply + # that connecting alone starts the dataplane. The editor supplies + # `program` instead; see .github/workflows/README.md. + ]; + Env = [ "HOME=/tmp" ]; + ExposedPorts = { + "4711/tcp" = { }; + }; + }; + }; + debug-tools = pkgs: [ diff --git a/justfile b/justfile index 3d75866efe..661da00277 100644 --- a/justfile +++ b/justfile @@ -141,6 +141,7 @@ oci_name := "githedgehog/dataplane" oci_frr_prefix := "githedgehog/dataplane/frr" oci_image_dataplane := oci_repo + "/" + oci_name + ":" + version oci_image_dataplane_core_viewer := oci_repo + "/" + oci_name + "/core-viewer:" + version +oci_image_dataplane_dev_debugger := oci_repo + "/" + oci_name + "/dev-debugger:" + version oci_image_dataplane_validator := oci_repo + "/" + oci_name + "/validator:" + version oci_image_frr_dataplane := oci_repo + "/" + oci_frr_prefix + ":" + version oci_image_frr_host := oci_repo + "/" + oci_frr_prefix + "-host:" + version @@ -310,6 +311,11 @@ build-container target="dataplane" *args: (build (if target == "dataplane" { "da docker tag "ghcr.io/githedgehog/dataplane/core-viewer:{{version}}" "{{oci_image_dataplane_core_viewer}}" echo "imported {{ oci_image_dataplane_core_viewer }}" ;; + "dataplane-dev-debugger") + docker load < ./results/containers.dataplane-dev-debugger + docker tag "ghcr.io/githedgehog/dataplane/dev-debugger:{{version}}" "{{oci_image_dataplane_dev_debugger}}" + echo "imported {{ oci_image_dataplane_dev_debugger }}" + ;; "debug-tools") # Uses nix only to produce a base image with the runtime closure (glibc, bash, etc.) # then layers locally-compiled cargo binaries on top via Dockerfile. @@ -364,6 +370,10 @@ push-container target="dataplane" *args: (build-container target args) && versio skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_core_viewer }}" "docker://{{ oci_image_dataplane_core_viewer }}" echo "Pushed {{ oci_image_dataplane_core_viewer }}" ;; + "dataplane-dev-debugger") + skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_dev_debugger }}" "docker://{{ oci_image_dataplane_dev_debugger }}" + echo "Pushed {{ oci_image_dataplane_dev_debugger }}" + ;; "debug-tools") >&2 echo "do not push the debug tools!" exit 1 diff --git a/nix/overlays/dataplane-dev.nix b/nix/overlays/dataplane-dev.nix index c8829018f2..7b0074591a 100644 --- a/nix/overlays/dataplane-dev.nix +++ b/nix/overlays/dataplane-dev.nix @@ -21,6 +21,15 @@ in opengrep = final.callPackage ../pkgs/opengrep { src = sources.opengrep; }; + # cargoDeps must be fetched from the overridden source too. + bugstalker = prev.bugstalker.overrideAttrs (orig: { + version = final.lib.removePrefix "v" sources.bugstalker.version; + src = sources.bugstalker; + cargoDeps = prev.rustPlatform.fetchCargoVendor { + src = sources.bugstalker; + hash = "sha256-GGi5hnrK5WpvnXHNckpsBch/SJ4lDvH7peSlrCdk218="; + }; + }); cargo-bolero = prev.cargo-bolero.override { inherit (override-packages) rustPlatform; }; cargo-deny = prev.cargo-deny.override { inherit (override-packages) rustPlatform; }; cargo-edit = prev.cargo-edit.override { inherit (override-packages) rustPlatform; }; diff --git a/npins/sources.json b/npins/sources.json index 369e701a56..fde579c3b7 100644 --- a/npins/sources.json +++ b/npins/sources.json @@ -16,6 +16,22 @@ "url": "https://api.github.com/repos/KaTeX/KaTeX/tarball/refs/tags/v0.18.3", "hash": "sha256-FZpiUhKFI2GAZmr667gA5yHFezAoRcAADoS0+NrNsJQ=" }, + "bugstalker": { + "type": "GitRelease", + "repository": { + "type": "GitHub", + "owner": "godzie44", + "repo": "BugStalker" + }, + "pre_releases": false, + "version_upper_bound": null, + "release_prefix": null, + "submodules": false, + "version": "v0.4.7", + "revision": "9c18e546eca6a1d68ef69da340a6fb0f2bc1bab7", + "url": "https://api.github.com/repos/godzie44/BugStalker/tarball/refs/tags/v0.4.7", + "hash": "sha256-AAeSvy/rvyylPH2jTVBGN95QIc6gumREQYuruhRo2ZI=" + }, "crane": { "type": "GitRelease", "repository": { From facbec414d126d64698ec7be7f64bfbb672f76c1 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:38:36 -0600 Subject: [PATCH 04/23] feat(debug): add a syscall tracer image Some failures need a record of the dataplane's kernel interactions rather than an interactive debugger. A small, repeatable tracing environment is easier to deploy and feed into existing log analysis than a general-purpose toolbox. Package lurk around the matching release binaries and follow the worker threads where the dataplane does its work. Because syscall tracing needs no symbols, this image can stay much smaller than the debugger images. Co-authored-by: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- .github/workflows/README.md | 19 ++++++++++++++++- .github/workflows/dev.yml | 4 ++++ default.nix | 38 ++++++++++++++++++++++++++++++++++ justfile | 10 +++++++++ nix/overlays/dataplane-dev.nix | 27 ++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 1 deletion(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index ce4e18984e..b3a2e45cc8 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -93,7 +93,7 @@ If those queue failures stop being rare, the phasing is worth revisiting. - Checks: `debug` by default; `release` and `fuzz` on deep runs - Coverage: `debug` by default; `fuzz` on deep runs - Miri: required on deep runs; opt-in on pull requests with `ci:+miri` -- Containers: debug/release for dataplane, its two debug images, and FRR; +- Containers: debug/release for dataplane, its three debug images, and FRR; release for validator - VLAB configurations: spine-leaf fabric mode, L2VNI/L3VNI VPC modes, with gateway enabled @@ -155,6 +155,23 @@ If those queue failures stop being rare, the phasing is worth revisiting. } ``` +- `ghcr.io/githedgehog/dataplane/syscall-tracer` records what the dataplane + asks the kernel for, as JSON, using lurk. + It carries the same stripped binaries the release image ships, since nothing + here symbolizes, which is why it is a fraction of the size of the other two: + + ```console + docker run --rm ghcr.io/githedgehog/dataplane/syscall-tracer:TAG > trace.jsonl + ``` + + The stream is one JSON object per line, except that tracing child threads + makes lurk announce each one with a bare `Attaching to child ` line. + Filter those out if the consumer needs strict JSONL: + + ```console + jq -R 'fromjson? // empty' < trace.jsonl + ``` + - Coverage reports from each `coverage/` job, kept for 7 days: - `coverage-html-.tar.gz` - `llvm-cov` HTML report, including the per-branch counts that Codecov does not render. Unpack and open diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index bbd718e04a..b645ddba24 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -374,6 +374,8 @@ jobs: - dataplane-core-viewer # Live debugging of the same build, driven from an editor over DAP. - dataplane-dev-debugger + # Syscall trace of the same build, as JSON. + - dataplane-syscall-tracer - validator # TODO: enable cfi and safe-stack on release when possible profile: "${{ fromJSON(needs.plan.outputs.container_profiles) }}" @@ -386,6 +388,8 @@ jobs: profile: fuzz - nix-target: dataplane-dev-debugger profile: fuzz + - nix-target: dataplane-syscall-tracer + profile: fuzz - nix-target: validator profile: debug - nix-target: validator diff --git a/default.nix b/default.nix index 0e5451d7e8..44cf143917 100644 --- a/default.nix +++ b/default.nix @@ -891,6 +891,44 @@ let }; }; + # Traces the release binaries' syscalls as JSON with lurk. + containers.dataplane-syscall-tracer = pkgs.dockerTools.buildLayeredImage { + name = "ghcr.io/githedgehog/dataplane/syscall-tracer"; + inherit tag; + contents = pkgs.buildEnv { + name = "dataplane-syscall-tracer-env"; + pathsToLink = [ + "/bin" + "/etc" + "/var" + "/lib" + ]; + paths = [ + pkgs.pkgsBuildHost.lurk + pkgs.pkgsHostHost.dockerTools.fakeNss + pkgs.pkgsHostHost.busybox + pkgs.pkgsHostHost.dockerTools.usrBinEnv + workspace.cli + workspace.dataplane + workspace.init + ]; + }; + extraCommands = '' + mkdir -p tmp + chmod 1777 tmp + ''; + config = { + Entrypoint = [ + "/bin/lurk" + "--json" + # Include the worker threads where the dataplane does its work. + "--follow-forks" + "/bin/dataplane" + ]; + Env = [ "HOME=/tmp" ]; + }; + }; + debug-tools = pkgs: [ diff --git a/justfile b/justfile index 661da00277..c969d44954 100644 --- a/justfile +++ b/justfile @@ -142,6 +142,7 @@ oci_frr_prefix := "githedgehog/dataplane/frr" oci_image_dataplane := oci_repo + "/" + oci_name + ":" + version oci_image_dataplane_core_viewer := oci_repo + "/" + oci_name + "/core-viewer:" + version oci_image_dataplane_dev_debugger := oci_repo + "/" + oci_name + "/dev-debugger:" + version +oci_image_dataplane_syscall_tracer := oci_repo + "/" + oci_name + "/syscall-tracer:" + version oci_image_dataplane_validator := oci_repo + "/" + oci_name + "/validator:" + version oci_image_frr_dataplane := oci_repo + "/" + oci_frr_prefix + ":" + version oci_image_frr_host := oci_repo + "/" + oci_frr_prefix + "-host:" + version @@ -316,6 +317,11 @@ build-container target="dataplane" *args: (build (if target == "dataplane" { "da docker tag "ghcr.io/githedgehog/dataplane/dev-debugger:{{version}}" "{{oci_image_dataplane_dev_debugger}}" echo "imported {{ oci_image_dataplane_dev_debugger }}" ;; + "dataplane-syscall-tracer") + docker load < ./results/containers.dataplane-syscall-tracer + docker tag "ghcr.io/githedgehog/dataplane/syscall-tracer:{{version}}" "{{oci_image_dataplane_syscall_tracer}}" + echo "imported {{ oci_image_dataplane_syscall_tracer }}" + ;; "debug-tools") # Uses nix only to produce a base image with the runtime closure (glibc, bash, etc.) # then layers locally-compiled cargo binaries on top via Dockerfile. @@ -374,6 +380,10 @@ push-container target="dataplane" *args: (build-container target args) && versio skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_dev_debugger }}" "docker://{{ oci_image_dataplane_dev_debugger }}" echo "Pushed {{ oci_image_dataplane_dev_debugger }}" ;; + "dataplane-syscall-tracer") + skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_syscall_tracer }}" "docker://{{ oci_image_dataplane_syscall_tracer }}" + echo "Pushed {{ oci_image_dataplane_syscall_tracer }}" + ;; "debug-tools") >&2 echo "do not push the debug tools!" exit 1 diff --git a/nix/overlays/dataplane-dev.nix b/nix/overlays/dataplane-dev.nix index 7b0074591a..345466c1d8 100644 --- a/nix/overlays/dataplane-dev.nix +++ b/nix/overlays/dataplane-dev.nix @@ -30,6 +30,33 @@ in hash = "sha256-GGi5hnrK5WpvnXHNckpsBch/SJ4lDvH7peSlrCdk218="; }; }); + # lurk disables ASLR in the tracee before exec, and treats failure as fatal. + # Docker's default seccomp profile answers personality(ADDR_NO_RANDOMIZE) with + # EPERM, so under a plain `docker run` the traced program never starts -- and + # lurk still exits 0 after emitting a well-formed JSON trace of its own child + # failing, which the `jq -R 'fromjson? // empty'` filter we document accepts + # without complaint. + # + # Nothing in the tracer image symbolizes an address, so a fixed layout buys us + # nothing. Make it advisory rather than telling users to pass + # `--security-opt seccomp=unconfined`, which drops confinement on a container + # whose whole job is ptracing a process. + # + # Two single-line substitutions rather than one spanning both: nix strips the + # common indentation from an indented string, so a multi-line search pattern + # would not match the source's own indentation. + lurk = prev.lurk.overrideAttrs (orig: { + postPatch = (orig.postPatch or "") + '' + substituteInPlace src/lib.rs \ + --replace-fail \ + 'personality::set(Persona::ADDR_NO_RANDOMIZE)' \ + 'let _ = personality::set(Persona::ADDR_NO_RANDOMIZE);' \ + --replace-fail \ + '.map_err(|_| anyhow!("Unable to set ADDR_NO_RANDOMIZE"))?;' \ + "" + ''; + }); + cargo-bolero = prev.cargo-bolero.override { inherit (override-packages) rustPlatform; }; cargo-deny = prev.cargo-deny.override { inherit (override-packages) rustPlatform; }; cargo-edit = prev.cargo-edit.override { inherit (override-packages) rustPlatform; }; From 3c872c0488611e3881855008075f593594f722a3 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:39:43 -0600 Subject: [PATCH 05/23] ci: publish debug images on a deliberate cadence The diagnostic images are useful only when they match the build being investigated, but building roughly 850 MB of extra images for every pull request would undermine the runner-load reduction this CI rework is meant to achieve. Build them automatically for pushes, the merge queue, and manual runs, with an explicit label available for debugging a pull request. Publish all three beside tagged releases so the matching tools remain available when a deployed build needs investigation. Co-authored-by: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- .github/workflows/README.md | 8 ++++++-- .github/workflows/dev.yml | 22 ++++++++++++---------- justfile | 11 ++++++++--- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index b3a2e45cc8..b8e4c1fa7d 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -61,6 +61,9 @@ Production artifacts are produced via nix builds in a separate CI workflow. - `ci:+miri` - Run Miri checks - `ci:+wasm` - Run the WASM build check - `ci:+concurrency` - Run Shuttle and Loom tests +- `ci:+debug-images` - Also build and push the core viewer, DAP debugger, and + syscall tracer images. They are built on main, in the merge queue, and on + dispatch regardless; this is for when the build itself needs debugging - `ci:+cross` - Build all cross-platform containers - `ci:+cross/full` - Also run the workspace test suite under qemu-user, on the two aarch64 musl legs. Gated like every other job, so the merge queue and @@ -93,8 +96,9 @@ If those queue failures stop being rare, the phasing is worth revisiting. - Checks: `debug` by default; `release` and `fuzz` on deep runs - Coverage: `debug` by default; `fuzz` on deep runs - Miri: required on deep runs; opt-in on pull requests with `ci:+miri` -- Containers: debug/release for dataplane, its three debug images, and FRR; - release for validator +- Containers: debug/release for dataplane and FRR; release for validator +- Debug images (core viewer, DAP debugger, syscall tracer): deep runs only, + or on a pull request with `ci:+debug-images` - VLAB configurations: spine-leaf fabric mode, L2VNI/L3VNI VPC modes, with gateway enabled diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index b645ddba24..a651f48005 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -71,6 +71,7 @@ jobs: runs-on: "ubuntu-latest" outputs: container_profiles: "${{ steps.container-profiles.outputs.value }}" + container_targets: "${{ steps.container-targets.outputs.value }}" profiles: "${{ steps.profiles.outputs.value }}" concurrency: "${{ steps.concurrency.outputs.value }}" cross: "${{ steps.cross.outputs.value }}" @@ -122,6 +123,16 @@ jobs: on-value: '["debug", "release", "fuzz"]' off-value: '["debug"]' + # Debug images are opt-in on pull requests because of their size. + - id: "container-targets" + uses: *gate + with: + labels: "debug-images" + # Keep this on one line: `ci-gate` writes the value to GITHUB_OUTPUT + # with a plain printf, which a multi-line value would corrupt. + on-value: '["frr.dataplane", "dataplane", "dataplane-core-viewer", "dataplane-dev-debugger", "dataplane-syscall-tracer", "validator"]' + off-value: '["frr.dataplane", "dataplane", "validator"]' + # Lab jobs require release images but not other release/fuzz checks. - id: "container-profiles" uses: *gate @@ -367,16 +378,7 @@ jobs: fail-fast: false max-parallel: 1 matrix: - nix-target: - - frr.dataplane - - dataplane - # Must match the build that produced the core. - - dataplane-core-viewer - # Live debugging of the same build, driven from an editor over DAP. - - dataplane-dev-debugger - # Syscall trace of the same build, as JSON. - - dataplane-syscall-tracer - - validator + nix-target: "${{ fromJSON(needs.plan.outputs.container_targets) }}" # TODO: enable cfi and safe-stack on release when possible profile: "${{ fromJSON(needs.plan.outputs.container_profiles) }}" exclude: diff --git a/justfile b/justfile index c969d44954..91262ee3f6 100644 --- a/justfile +++ b/justfile @@ -411,12 +411,17 @@ push-container target="dataplane" *args: (build-container target args) && versio exit 99 esac -# Push release images with the resolved core budget. [script] push: {{ _just_debuggable_ }} - # The core viewer must match the release it inspects. - for container in dataplane dataplane-core-viewer frr.dataplane validator; do + # Debug images must match the release they inspect. + for container in \ + dataplane \ + dataplane-core-viewer \ + dataplane-dev-debugger \ + dataplane-syscall-tracer \ + frr.dataplane \ + validator; do if [ "${container}" = "validator" ]; then platform="wasm32-wasip1" else From 42665e160cef15841d0ad325b406432e2895a169 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 20:50:30 -0600 Subject: [PATCH 06/23] test(debug): exercise the debug images instead of only building them All three images built green while two of them could not do what the README documented. Building proves an image links; it says nothing about whether its entrypoint runs. `smoke-container` runs each one the way the README tells a user to: - the tracer under a plain `docker run`, with no seccomp relaxation, and requires an `execve` in the trace. Its failure mode is the reason this exists: lurk emitted eight well-formed JSON lines recording its own child failing to start, then exited 0, which the `jq -R 'fromjson? // empty'` filter we document accepts without complaint. Both guards fire against the pre-fix image. - the debugger over DAP, requiring a `process` event rather than a successful connection. Connecting proves nothing on its own: in remote-DAP mode bugstalker waits for the client to name the program. This does not reproduce the entrypoint defect, which was in the documentation rather than the runtime; it pins the launch contract so the next change to it is visible. - the core viewer through its own entrypoint rather than by invoking gdb directly, since the `--directory` and `source` flags that register the printers live in that entrypoint. Checks that the printer set is registered, not merely that gdb started. The trace goes to a file rather than a shell variable: at a few megabytes it overruns the here-string limit, and every grep against it then fails with E2BIG, which reads exactly like a failed trace. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/dev.yml | 11 ++++ ci.just | 4 ++ justfile | 68 +++++++++++++++++++++++ scripts/dap-smoke.py | 111 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 194 insertions(+) create mode 100755 scripts/dap-smoke.py diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index a651f48005..0c71c9bfa9 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -405,6 +405,17 @@ jobs: with: recipe: "ci::push-container" recipe_args: "${{ matrix.nix-target }} ${{ matrix.profile }} ${{ needs.version.outputs.version }}" + + # A debug image that builds is not a debug image that works. Two of + # these shipped with entrypoints that could not do what the README + # documents, and building them said nothing about it. + - name: "smoke" + if: "${{ startsWith(matrix.nix-target, 'dataplane-') }}" + uses: *just + with: + recipe: "ci::smoke-container" + recipe_args: "${{ matrix.nix-target }} ${{ matrix.profile }}" + - *verify-clean-tree - *tmate diff --git a/ci.just b/ci.just index 4696b6cc9f..91b33c4c4c 100644 --- a/ci.just +++ b/ci.just @@ -76,6 +76,10 @@ cross platform libc +args: cross-test platform libc: NEXTEST_PROFILE=cross-qemu just {{ _lab }} share={{ _share }} platform={{ platform }} libc={{ libc }} profile=debug test +# Verify a debug image's entrypoint actually does its job. +smoke-container target profile: + just {{ _lab }} profile={{ profile }} platform=x86-64-v3 smoke-container {{ target }} + # Push both the derived and temporary per-commit tags. [script] push-container target profile version: diff --git a/justfile b/justfile index 91262ee3f6..93b0f14c12 100644 --- a/justfile +++ b/justfile @@ -284,6 +284,74 @@ setup-roots *args: {{ args }} done +# Check that the debug images actually do what the README says they do. +# +# Building an image proves it links; it does not prove the entrypoint runs. +# Both of these shipped broken: the tracer's documented `docker run` produced a +# well-formed JSON trace of its own child failing to start, and still exited 0. +[script] +smoke-container target: (build-container target) + {{ _just_debuggable_ }} + declare -xr DOCKER_HOST="${DOCKER_HOST:-unix://{{ docker_sock }}}" + case "{{ target }}" in + "dataplane-syscall-tracer") + # A trace runs to megabytes, so keep it in a file: a shell variable + # that size overruns the here-string limit and every grep against + # it fails with E2BIG, which reads exactly like a failed trace. + declare trace + trace="$(mktemp)" + declare -r trace + trap 'rm -f -- "${trace}"' EXIT + # No seccomp relaxation on purpose: this is the documented command. + timeout 60 docker run --rm "{{ oci_image_dataplane_syscall_tracer }}" \ + >"${trace}" 2>&1 || true + if grep -q "Unable to set ADDR_NO_RANDOMIZE" "${trace}"; then + >&2 echo "::error::lurk could not disable ASLR, so the tracee never ran" + exit 1 + fi + # The tracee has to actually execute, not merely be attached to. + if ! grep -q '"syscall":"execve"' "${trace}"; then + >&2 echo "::error::no execve in the trace: the traced program never started" + >&2 head -20 "${trace}" + exit 1 + fi + printf 'syscall-tracer: traced %s syscalls\n' "$(grep -c '"type":"SYSCALL"' "${trace}")" + ;; + "dataplane-dev-debugger") + declare cid + cid="$(docker run -d --rm -p 47110:4711 "{{ oci_image_dataplane_dev_debugger }}")" + declare -r cid + trap 'docker kill "${cid}" >/dev/null 2>&1 || true' EXIT + ./scripts/dap-smoke.py 47110 /bin/dataplane + ;; + "dataplane-core-viewer") + # The Rust pretty-printers are the reason this image exists, and + # what registers them is the entrypoint's own `--directory` and + # `source` flags -- so drive the real entrypoint rather than + # invoking gdb directly, which would only test a copy of them. + declare out + out="$(printf 'info pretty-printer\nquit\n' \ + | timeout 120 docker run --rm -i "{{ oci_image_dataplane_core_viewer }}" 2>&1)" + if grep -qiE "traceback|no module named" <<<"${out}"; then + >&2 echo "::error::gdb could not load the rust pretty-printers" + >&2 printf '%s\n' "${out}" + exit 1 + fi + # A registered printer set, not merely a clean start. + for want in StdString StdVec StdHashMap; do + if ! grep -q "${want}" <<<"${out}"; then + >&2 echo "::error::rust pretty-printer ${want} is not registered" + exit 1 + fi + done + echo "core-viewer: rust pretty-printers registered" + ;; + *) + >&2 echo "::error::no smoke test defined for {{ target }}" + exit 1 + ;; + esac + # Build the dataplane container image [script] build-container target="dataplane" *args: (build (if target == "dataplane" { "dataplane.tar" } else if target == "validator" { "workspace.validator" } else { "containers." + target }) args) diff --git a/scripts/dap-smoke.py b/scripts/dap-smoke.py new file mode 100755 index 0000000000..55667c2fda --- /dev/null +++ b/scripts/dap-smoke.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 +# Copyright Open Network Fabric Authors + +"""Check that the dev-debugger image really launches a debuggee over DAP. + +The image only listens; in remote-DAP mode bugstalker ignores the debuggee +named on its command line and waits for the client's `launch` request to +supply `arguments.program`. Connecting therefore proves nothing, which is +how the image shipped with a documented invocation that could not work. + +Exits non-zero unless a launch actually produces a running process. +""" + +import json +import socket +import sys +import time + +TIMEOUT = 60.0 + + +def send(sock: socket.socket, seq: int, command: str, arguments: dict) -> None: + body = json.dumps( + {"seq": seq, "type": "request", "command": command, "arguments": arguments} + ).encode() + sock.sendall(b"Content-Length: %d\r\n\r\n" % len(body) + body) + + +def messages(sock: socket.socket, seconds: float): + """Yield DAP messages until the socket goes quiet for `seconds`.""" + sock.settimeout(0.5) + buf = b"" + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + try: + chunk = sock.recv(65536) + except socket.timeout: + continue + if not chunk: + return + buf += chunk + while b"\r\n\r\n" in buf: + head, rest = buf.split(b"\r\n\r\n", 1) + fields = {} + for line in head.decode(errors="replace").strip().splitlines(): + key, sep, value = line.partition(":") + if sep: + fields[key.strip().lower()] = value.strip() + if "content-length" not in fields: + # Not a header we understand; drop it rather than wedging. + buf = rest + continue + length = int(fields["content-length"]) + if len(rest) < length: + break + yield json.loads(rest[:length]) + buf = rest[length:] + + +def connect(port: int) -> socket.socket: + """Wait for the container's listener rather than assuming it is up.""" + deadline = time.monotonic() + TIMEOUT + while True: + try: + return socket.create_connection(("127.0.0.1", port), timeout=5) + except OSError: + if time.monotonic() >= deadline: + raise + time.sleep(0.5) + + +def main() -> int: + port, program = int(sys.argv[1]), sys.argv[2] + sock = connect(port) + + send(sock, 1, "initialize", {"adapterID": "smoke", "linesStartAt1": True}) + if not any( + m.get("command") == "initialize" and m.get("success") for m in messages(sock, 10) + ): + print("::error::adapter never answered `initialize`", file=sys.stderr) + return 1 + + send(sock, 2, "launch", {"program": program, "args": []}) + launched, pid = False, None + for message in messages(sock, 20): + if message.get("type") == "response" and message.get("command") == "launch": + if not message.get("success"): + print( + f"::error::launch rejected: {message.get('message')}", + file=sys.stderr, + ) + return 1 + launched = True + elif message.get("event") == "process": + pid = message.get("body", {}).get("systemProcessId") + + if not launched: + print("::error::adapter never answered `launch`", file=sys.stderr) + return 1 + if pid is None: + # A successful launch that starts nothing is the failure this guards. + print("::error::launch succeeded but no process event arrived", file=sys.stderr) + return 1 + + print(f"dev-debugger: launched {program} as pid {pid}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From a2130821f67e8db46983c683d854ea1de01474b0 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 22:20:52 -0600 Subject: [PATCH 07/23] ci: reserve Cachix for reusable build inputs Workspace Rust outputs include the repository source and change on nearly every revision. Uploading them consumes transfer and storage while yielding few substitutions, crowding out the slower C and C++ dependencies that benefit from a shared cache. Give source-volatile outputs a common store-path prefix, skip their substitute lookups, and exclude every OCI assembly path that could reintroduce them through its closure. Stable vendored sources, development dependencies, and native libraries remain cacheable. Signed-off-by: Daniel Noland --- .github/actions/nix-shell/action.yml | 2 ++ default.nix | 50 +++++++++++++++------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/.github/actions/nix-shell/action.yml b/.github/actions/nix-shell/action.yml index 2336aabfca..752e72a966 100644 --- a/.github/actions/nix-shell/action.yml +++ b/.github/actions/nix-shell/action.yml @@ -32,6 +32,8 @@ runs: signingKey: '${{ inputs.cachix_signing_key }}' # prettier-ignore authToken: '${{ inputs.cachix_auth_token }}' + # Exclude source-volatile builds and image paths that can reintroduce them through a closure. + pushFilter: '(-dataplane-volatile-|-(dataplane|core-viewer|dev-debugger|syscall-tracer)-(customisation-layer|conf\.json)$|-stream-(dataplane|core-viewer|dev-debugger|syscall-tracer|frr)$|-(frr-conf\.json|layers\.json|excludePaths)$)' - name: "use nix shell" uses: "rrbutani/use-nix-shell-action@59a52b2b9bbfe3cc0e7deb8f9059abe37a439edf" # v1.1.0 diff --git a/default.nix b/default.nix index 44cf143917..53e4126131 100644 --- a/default.nix +++ b/default.nix @@ -237,6 +237,10 @@ let "${pkgs.rust-toolchain.passthru.availableComponents.rust-src}/lib/rustlib/src/rust/library/Cargo.lock" ]; }; + source-volatile = orig: { + name = "dataplane-volatile-${orig.name or "${orig.pname}-${orig.version}"}"; + allowSubstitutes = false; + }; # For wasm32, pkgs is the host nixpkgs (no pkgsCross), so ctarget resolves to the # host platform (e.g. x86_64-unknown-linux-gnu). That means is-cross-compile is # false for wasm, which is intentional: we don't want native cross-compilation @@ -255,7 +259,7 @@ let objcopy = if is-cross-compile then "${ctarget}-objcopy" else "objcopy"; package-list = builtins.fromJSON ( builtins.readFile ( - pkgs.runCommandLocal "package-list" + (pkgs.runCommandLocal "package-list" { TOMLQ = "${pkgs.pkgsBuildHost.yq}/bin/tomlq"; JQ = "${pkgs.pkgsBuildHost.jq}/bin/jq"; @@ -271,7 +275,7 @@ let $TOMLQ --arg p "$p" -r '{ ($p): .package.name }' ${src}/$p/Cargo.toml done | $JQ --sort-keys --slurp 'add' > $out '' - ) + )).overrideAttrs source-volatile ) ); version = (craneLib.crateNameFromCargoToml { inherit src; }).version; @@ -389,7 +393,7 @@ let } // args )).overrideAttrs - (orig: { + (orig: source-volatile orig // { separateDebugInfo = true; # I'm not 100% sure if I would call it a bug in crane or a bug in cargo, but cross compile is tricky here. @@ -650,7 +654,7 @@ let ) package-list; }; - dataplane.tar = pkgs.stdenv'.mkDerivation { + dataplane.tar = (pkgs.stdenv'.mkDerivation { pname = "dataplane.tar"; inherit version; dontUnpack = true; @@ -770,12 +774,12 @@ let ${workspace.cli} \ ${pkgs.pkgsHostHost.busybox} ''; - }; + }).overrideAttrs source-volatile; - containers.dataplane = pkgs.dockerTools.buildLayeredImage { + containers.dataplane = (pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane"; inherit tag; - contents = pkgs.buildEnv { + contents = (pkgs.buildEnv { name = "dataplane-env"; pathsToLink = [ "/bin" @@ -791,9 +795,9 @@ let workspace.dataplane workspace.init ]; - }; + }).overrideAttrs source-volatile; config.Entrypoint = [ "/bin/dataplane" ]; - }; + }).overrideAttrs source-volatile; # Shared runtime and unstripped binaries for the debugger images. debug-image-paths = [ @@ -816,10 +820,10 @@ let ''; # Opens dataplane core files with matching symbols and sources. - containers.dataplane-core-viewer = pkgs.dockerTools.buildLayeredImage { + containers.dataplane-core-viewer = (pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane/core-viewer"; inherit tag; - contents = pkgs.buildEnv { + contents = (pkgs.buildEnv { name = "dataplane-core-viewer-env"; pathsToLink = [ "/bin" @@ -832,7 +836,7 @@ let rust-gdb-printers ] ++ debug-image-paths; - }; + }).overrideAttrs source-volatile; # gdb needs a writable HOME for logs and its index cache. extraCommands = '' mkdir -p tmp @@ -850,13 +854,13 @@ let ]; Env = [ "HOME=/tmp" ]; }; - }; + }).overrideAttrs source-volatile; # Exposes bugstalker's DAP server for live debugging. - containers.dataplane-dev-debugger = pkgs.dockerTools.buildLayeredImage { + containers.dataplane-dev-debugger = (pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane/dev-debugger"; inherit tag; - contents = pkgs.buildEnv { + contents = (pkgs.buildEnv { name = "dataplane-dev-debugger-env"; pathsToLink = [ "/bin" @@ -865,7 +869,7 @@ let "/lib" ]; paths = [ pkgs.pkgsBuildHost.bugstalker ] ++ debug-image-paths; - }; + }).overrideAttrs source-volatile; # bugstalker needs a writable HOME for its keymap and history. extraCommands = '' mkdir -p tmp @@ -889,13 +893,13 @@ let "4711/tcp" = { }; }; }; - }; + }).overrideAttrs source-volatile; # Traces the release binaries' syscalls as JSON with lurk. - containers.dataplane-syscall-tracer = pkgs.dockerTools.buildLayeredImage { + containers.dataplane-syscall-tracer = (pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane/syscall-tracer"; inherit tag; - contents = pkgs.buildEnv { + contents = (pkgs.buildEnv { name = "dataplane-syscall-tracer-env"; pathsToLink = [ "/bin" @@ -912,7 +916,7 @@ let workspace.dataplane workspace.init ]; - }; + }).overrideAttrs source-volatile; extraCommands = '' mkdir -p tmp chmod 1777 tmp @@ -927,7 +931,7 @@ let ]; Env = [ "HOME=/tmp" ]; }; - }; + }).overrideAttrs source-volatile; debug-tools = pkgs: @@ -1009,7 +1013,7 @@ let }; - containers.frr.dataplane = pkgs.dockerTools.buildLayeredImage { + containers.frr.dataplane = (pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane/frr"; inherit tag; contents = pkgs.buildEnv { @@ -1063,7 +1067,7 @@ let "--" ]; config.Cmd = [ "/libexec/frr/docker-start" ]; - }; + }).overrideAttrs source-volatile; containers.frr.host = pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane/frr-host"; From e0397edf52e61886fe32cec00e447849bd759e27 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 22:53:36 -0600 Subject: [PATCH 08/23] ci: accept a prewarmed nix on the lab runners Every lab job realizes the same dev shell against an empty store. The job logs put that at ~45s each: ~22s re-fetching the npins tarballs into an empty `~/.cache/nix` and ~20s substituting ~340 store paths into an empty `/nix/store`. Each job lands on its own ephemeral runner, so both halves are paid every time -- ~28 minutes across a 32-job pull request run, a third of all lab runner time on that run. The lab pool is the binding constraint on this workflow, so that time comes off the queue. The fix belongs in the runner image, which can carry the store and the tarball cache already warm. `install-nix-action` aborts as soon as it finds nix on PATH, which makes such an image a drop-in, but that early exit also skips the nix.conf, NIX_PATH, and TMPDIR setup the action would otherwise do. Supply those here so a job is configured the same way either way. `access-tokens` is the setting that matters: it is per-run, so an image cannot bake it in, and without it a pin that misses the image's tarball cache is fetched anonymously against the rate limit shared by every runner behind the lab's address. This assumes nothing about the image. Where nix is not preinstalled the new step is a no-op and the install proceeds exactly as before, so it is safe ahead of the companion githedgehog/gha-runner change. Co-Authored-By: Claude Opus 5 (1M context) --- .github/actions/nix-shell/action.yml | 51 ++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/.github/actions/nix-shell/action.yml b/.github/actions/nix-shell/action.yml index 752e72a966..d4861fc3a4 100644 --- a/.github/actions/nix-shell/action.yml +++ b/.github/actions/nix-shell/action.yml @@ -19,7 +19,58 @@ inputs: runs: using: "composite" steps: + # Realizing the dev shell costs every job ~45s on a cold store: ~22s + # re-fetching the npins tarballs and ~20s substituting ~340 store paths. + # The lab runner image can carry both already warm (see the `nix.sh` change + # drafted for githedgehog/gha-runner), which reduces that to near zero. + # + # `install-nix-action` aborts as soon as it finds nix on PATH, and that + # early exit also skips the nix.conf and NIX_PATH setup it would otherwise + # do. Supply the parts that matter here so a job is configured the same + # way whether or not the image was prewarmed. Nothing below assumes a + # prewarmed image: on one without nix this step is a no-op and the install + # proceeds exactly as before. + - name: "Configure a prewarmed nix" + id: "prewarmed" + shell: "bash" + env: + # Indirect through the environment rather than interpolating into the + # script body. + github_token: "${{ github.token }}" + # The only values written to the environment file below are two + # literals and `RUNNER_TEMP`, all runner controlled; none of them can + # carry anything from the event payload. + run: | # zizmor: ignore[github-env] + set -euo pipefail + if ! command -v nix >/dev/null 2>&1; then + printf 'prewarmed=false\n' >>"${GITHUB_OUTPUT}" + exit 0 + fi + printf 'prewarmed=true\n' >>"${GITHUB_OUTPUT}" + # `access-tokens` is per-run, so it is the one setting the image cannot + # bake in. Without it a pin that misses the image's tarball cache is + # fetched anonymously and shares github.com's unauthenticated rate + # limit with every other runner behind the lab's address. A + # single-user nix honors this file without a `trusted-users` entry. + install -d -m "0700" "${HOME}/.config/nix" + umask "0077" + cat >"${HOME}/.config/nix/nix.conf" < today -- npins pins + # every source -- but the installed path sets this, so match it. + printf 'NIX_PATH=nixpkgs=channel:nixpkgs-unstable\n' >>"${GITHUB_ENV}" + # Also match where the installed path puts nix's scratch space: the + # job's temp directory on the work volume, not the container's own + # writable layer. The image cannot set this; it is per-run. + if [ -z "${TMPDIR:-}" ]; then + printf 'TMPDIR=%s\n' "${RUNNER_TEMP}" >>"${GITHUB_ENV}" + fi + - name: "Install nix" + if: "${{ steps.prewarmed.outputs.prewarmed != 'true' }}" uses: "cachix/install-nix-action@8aa03977d8d733052d78f4e008a241fd1dbf36b3" # v31.10.6 with: github_access_token: "${{ github.token }}" From 26c6670caf61a0320283f78c890561817832dcc8 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 08:30:14 -0600 Subject: [PATCH 09/23] ci: keep workspace outputs substitutable a5066b67b applied `source-volatile` to the crane workspace builds as well as to image assembly. Its premise -- that workspace outputs change on nearly every revision and so rarely substitute -- holds for a revision that changes Rust code. It does not hold for most of the runs we pay for. `src` admits only cargo sources plus `.md`, `.json`, `.h`, and `.justfile`, so `.github/**` and `*.nix` never reach the derivation. A revision touching only CI or nix hashes identically to its parent, as does any re-run of a tree already built: a label-triggered run, the merge queue, and the push to main after merge are all that. Those runs used to cost almost nothing. On 760a9c2cb `sanitize/address` compiled not one crate; it fetched a single 1.4 GiB `all-0.25.2` nextest archive built by the v0.25.2 tag run on main and spent its 80s running 1235 tests. After a5066b67b the same job compiles the workspace from scratch: 1007s in run 31865573803, and 1051s re-running that same revision on an idle pool, which rules out both runner contention and a one-time cold cache. Across a run that is 87 -> 211 minutes of lab time. Nothing here shares between differently configured jobs. The asan and bluefield3-musl builds carry different RUSTFLAGS and sysroots, so they hash to different store paths, as they must. The reuse is strictly between runs that build identical sources. Keep `source-volatile` on the tar, buildEnv, and container derivations. Those are built once per revision by a single job and reach the lab through ghcr, so a copy in the binary cache buys nothing, and their per-revision image blobs are what motivated the original change. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index 53e4126131..56400c2419 100644 --- a/default.nix +++ b/default.nix @@ -237,6 +237,26 @@ let "${pkgs.rust-toolchain.passthru.availableComponents.rust-src}/lib/rustlib/src/rust/library/Cargo.lock" ]; }; + # Per-revision image assembly: renamed under a common prefix so the CI + # `pushFilter` can keep it out of Cachix, and never substituted. A given + # revision's image is built by exactly one job and reaches the lab through + # ghcr, so a copy in the binary cache buys nothing and costs a lot. + # + # Deliberately not applied to the workspace Rust builds below. Those are + # volatile with respect to `src`, but `src` admits only cargo sources plus + # `.md`, `.json`, `.h`, and `.justfile`, so a revision touching only CI or + # nix hashes identically to its parent -- as does any re-run of a tree we + # have already built, which is what a label-triggered run, the merge queue, + # and the post-merge push to main all are. Excluding those outputs cost + # ~125 minutes of lab time per run (31853726018 -> 31865573803). On + # 760a9c2cb `sanitize/address` compiled nothing: it fetched one 1.4 GiB + # `all-0.25.2` nextest archive built by the v0.25.2 tag run on main. After + # the exclusion the same job compiles the workspace, 1007s in CI and 1051s + # re-running that revision on an idle pool. + # + # None of this shares between differently configured jobs. The asan and + # bluefield3-musl builds carry different RUSTFLAGS and sysroots, so they + # hash to different store paths, as they must. source-volatile = orig: { name = "dataplane-volatile-${orig.name or "${orig.pname}-${orig.version}"}"; allowSubstitutes = false; @@ -393,7 +413,7 @@ let } // args )).overrideAttrs - (orig: source-volatile orig // { + (orig: { separateDebugInfo = true; # I'm not 100% sure if I would call it a bug in crane or a bug in cargo, but cross compile is tricky here. From bde28cd2fa0ff7e5cab22ab85f0ca093e35834c2 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 08:56:40 -0600 Subject: [PATCH 10/23] fix(nix): keep `results` out of the build source `just build` writes its out-links to `results/`, and while that path is gitignored, `lib.cleanSource` does not read gitignore. The directory therefore landed in `src`, so every developer who had run a build carried a `src` hash that differed from CI's -- and that changed again whenever they built a different target, because the symlinks point at store paths. The practical effect was that local builds stopped matching the binary cache after the first `just build`, which is the opposite of what this tree is set up to do. `target`, `sysroot`, and `devroot` were already excluded for the same reason. Verified by instantiation: a working tree containing `results/` now produces the same derivation as a clean checkout of the same commit. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index 56400c2419..8258e72c46 100644 --- a/default.nix +++ b/default.nix @@ -216,7 +216,13 @@ let markdownFilter = p: _type: builtins.match ".*\.md$" p != null; jsonFilter = p: _type: builtins.match ".*\.json$" p != null; cHeaderFilter = p: _type: builtins.match ".*\.h$" p != null; - outputsFilter = p: _type: (p != "target") && (p != "sysroot") && (p != "devroot") && (p != ".git"); + # `results` holds the out-links `just build` creates. It is gitignored, but + # `cleanSource` does not read gitignore, so without it here every developer + # who has run a build carries their own `src` hash and stops matching the + # binary cache. + outputsFilter = + p: _type: + (p != "target") && (p != "sysroot") && (p != "devroot") && (p != "results") && (p != ".git"); src = pkgs.lib.cleanSourceWith { filter = full-path: t: From 976d06252e5ff1b945a8d27c272e4f6f55a8d566 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 09:13:03 -0600 Subject: [PATCH 11/23] perf(nix): share one dependency build per flag-set Every crane derivation passed `cargoArtifacts = null`, which opts out of the deps-only split crane does by default. Each one therefore compiled the whole dependency graph itself: 476 of the 516 crates in a full build are third-party, and `-Zbuild-std` means each also rebuilds the standard library from source. Because those derivations embed `src`, that work was redone for every revision that touches any Rust file. Build the dependencies once per flag-set instead. Crane dummifies the workspace sources for that build, so it hashes on the manifests and substitutes across revisions -- the reuse the per-package derivations can never have. Two variants are needed. `mk-needs-unwind` gives tests `-Zbuild-std=std,panic_unwind` against `panic_abort` for production, and the test builders run under `profile-tests'`; sharing one artifact would miss on fingerprints and rebuild anyway. The test variant builds with `cargo test --no-run` so dev-dependencies land in the artifacts, which the nextest archives need. Scope both with a `--package` per member of `package-list` rather than letting cargo walk the whole dummified workspace. The union the consumers need is strictly smaller, and `package-list` is platform-aware: for wasm32-wasip1 it honours the `wasm = false` opt-out in `workspace.metadata.package`, leaving 13 members. `k8s-intf` is one of the excluded, and it is what pulls `rustls -> aws-lc-rs -> aws-lc-sys`; that crate compiles its C sources with the host gcc and no WASI sysroot (`CC_wasm32_wasip1 = Some(gcc)`, `WASI_SYSROOT = None`) and fails on `pthread_rwlock_t`. Inheriting the platform filtering rather than restating it is what keeps `wasm32-wasip1` and `validator/release` building. Measured locally (debug, 8 jobs). `workspace.dataplane` drops from 476 compiled crates to 52, 63s -> 30s, against 52s for the shared build: break-even at about 1.6 consumers. `tests.all` drops from 556 crates to 41, confirming dev-dependencies are present. The gain is uneven. A small package's baseline build only compiles its own slice of the graph, so unpacking the artifacts can cost as much as it saves: acl 27s -> 28s, args 23s -> 18s, cli 19s -> 15s. These local figures also exclude fetch cost entirely -- both arms read a warm local store -- so they understate the cache on a runner with a fast path to the CDN. CI measurement follows. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 94 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 86 insertions(+), 8 deletions(-) diff --git a/default.nix b/default.nix index 8258e72c46..123aea108d 100644 --- a/default.nix +++ b/default.nix @@ -346,6 +346,10 @@ let pname = null; cargoArtifacts = null; }, + # A deps-only build produces cargo artifacts rather than binaries, so it + # skips the debug-info split and keeps the `target.tar.zst` that the + # package path strips. + for-deps ? false, profile, cargo-nextest, hwloc, @@ -419,7 +423,16 @@ let } // args )).overrideAttrs - (orig: { + ( + orig: + if for-deps then + { + postBuild = (orig.postBuild or "") + '' + unset RUSTFLAGS; + ''; + } + else + { separateDebugInfo = true; # I'm not 100% sure if I would call it a bug in crane or a bug in cargo, but cross compile is tricky here. @@ -458,11 +471,76 @@ let postFixup = (orig.postFixup or "") + '' rm -f $out/target.tar.zst ''; - }); + } + ); + + # One dependency build per flag-set, shared by every package derivation at + # that configuration. Crane dummifies the workspace sources, so this hashes + # on the manifests and survives changes to our own code -- which is the + # substitution the per-package derivations can never get, since they embed + # `src`. + # + # Two variants, because `mk-needs-unwind` gives tests a different + # `-Zbuild-std`: mixing them would fingerprint-miss and rebuild anyway. + mk-cargo-artifacts = + { + for-tests, + cmd-prefix, + deps-profile, + }: + pkgs.callPackage invoke { + builder = craneLib.buildDepsOnly; + profile = deps-profile; + for-deps = true; + args = { + pname = if for-tests then "dataplane-tests" else "dataplane"; + cargoArtifacts = null; + buildPhaseCargoCommand = builtins.concatStringsSep " " ( + # `--no-run` for the test variant so dev-dependencies land in the + # artifacts too; the nextest archives need them. + ( + if for-tests then + [ + "cargo" + "test" + "--no-run" + "--profile=${cargo-profile}" + ] + else + [ + "cargo" + "build" + "--profile=${cargo-profile}" + ] + ) + # Scope to the same packages the consumers build. `package-list` is + # platform-aware -- for wasm it honours the `wasm = false` opt-out in + # `workspace.metadata.package` -- and building the whole workspace + # instead drags excluded members' dependencies in. That is not just + # wasted work: `k8s-intf` pulls `rustls -> aws-lc-rs -> aws-lc-sys`, + # whose C sources cannot compile for wasm32-wasip1. + ++ (map (pname: "--package=${pname}") (builtins.attrValues package-list)) + ++ cmd-prefix + ); + }; + }; + + cargo-artifacts = mk-cargo-artifacts { + for-tests = false; + cmd-prefix = cargo-cmd-prefix; + deps-profile = profile'; + }; + + cargo-artifacts-tests = mk-cargo-artifacts { + for-tests = true; + cmd-prefix = cargo-cmd-prefix-tests; + deps-profile = profile-tests'; + }; + workspace-builder = { pname ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts, }: pkgs.callPackage invoke { builder = craneLib.buildPackage; @@ -495,7 +573,7 @@ let workspace-check = { pname ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts, }: pkgs.callPackage invoke { builder = craneLib.buildPackage; @@ -528,7 +606,7 @@ let test-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -576,7 +654,7 @@ let bench-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -619,7 +697,7 @@ let profile = profile'; args = { inherit pname; - cargoArtifacts = null; + cargoArtifacts = cargo-artifacts; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" @@ -655,7 +733,7 @@ let profile = profile'; args = { inherit pname; - cargoArtifacts = null; + cargoArtifacts = cargo-artifacts; RUSTDOCFLAGS = "-D warnings"; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ From 6ac04981b6d08374859cb31933d524be2a94e6b7 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 12:18:52 -0600 Subject: [PATCH 12/23] perf(nix): remap sources to a fixed prefix `--remap-path-prefix==${src}` put the filtered source store path into RUSTFLAGS, and RUSTFLAGS is part of both the derivation and cargo's per-unit fingerprint. Every revision touching any Rust file therefore gave a new compilation identity to all 516 crates in a build -- the ~475 third-party ones and the standard library included, none of which contain our source. That is what made the dependency split worthless on the runs it was meant to help. Re-running an unchanged tree took 73.6 minutes of lab time against 157.5 for the same jobs before the split; adding a one-line test took 165.2, because the deps artifact was invalidated by the edit it was supposed to survive. The replacement prefix has to be stable *and* present when tests run. Stable is the point of the change; present is load bearing for bolero, which was not obvious. `check!()` records `file!()`, and `TargetLocation::abs_path` resolves it by canonicalising that path, falling back to joining `CARGO_MANIFEST_DIR`'s ancestors. Under nix the manifest dir is `/build/source/...`, gone by the time tests run, so the fallback is dead and resolution rests entirely on `file!()` existing. The old remap satisfied that by accident, the store path being present on the runner. A prefix that does not exist fails all nine `rate::test::derivative_of_arbitrary_*` cases in about ten milliseconds with "could not resolve target work dir", never running an input. So remap to `/tmp/dataplane/src`, which any user can create, and put the tree there for the consumers that need it: - `test`, `test-each`, and `coverage-archive` link it at this checkout before running. - the debug images link it at the sources they ship, which they now carry explicitly through `source-tree`. Until now the source reached those closures by accident: the remapped store path was baked into the binaries, so nix's reference scanner retained it. - `coverage-archive` passes `--path-equivalence` to `llvm-cov show` and `report`, which read sources to render them. `export` does not need it; it only emits paths, and the existing rewrite handles those. Verified: the deps derivation is now identical either side of a source edit (9xs5b0w5...), the workspace builds, and the production binary's closure no longer drags the source along -- which also stops image closures reintroducing source-volatile paths. Verified for bolero through the path that failed rather than a bare `cargo test`, which cannot reproduce it: `just test stats` runs 31 tests green, with `derivative_of_arbitrary_8` taking 1.013s rather than failing in 0.012s. Panic messages and backtraces now name `/tmp/dataplane/src/...` rather than a store path. Inside the debug images that resolves through the symlink. The prefix is spelled in both default.nix and the justfile. Deriving one from the other means either threading an `--argstr` through every build call site or paying a `nix eval` on every `just` invocation; a mismatch fails loudly and immediately. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 39 +++++++++++++++++++++++++++++++++++---- justfile | 26 +++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/default.nix b/default.nix index 123aea108d..4fc24b9ec3 100644 --- a/default.nix +++ b/default.nix @@ -237,6 +237,22 @@ let src = lib.cleanSource ./.; name = "source"; }; + # Where debug info claims our sources live. Deliberately a fixed path rather + # than `${src}`: the remap below lands in RUSTFLAGS, which is part of both the + # derivation and cargo's per-unit fingerprint, so a store path there gives + # every crate -- including the ~475 third-party ones and std -- a new identity + # on any revision that touches Rust. A stable prefix lets the dependency + # build be reused across revisions. + # + # Consumers put the real tree there: the debug images symlink it (see + # `source-tree`), and `coverage-archive` passes `-path-equivalence`. + # Keep in step with `src_prefix` in the justfile, which creates it before + # running tests. It has to be both stable (so the dependency build is not + # revision-specific) and creatable without root (so `file!()` resolves at test + # time -- bolero canonicalises it to find its corpus, and a path that does not + # exist takes down every property test). + src-prefix = "/tmp/dataplane/src"; + cargoVendorDir = craneLib.vendorMultipleCargoDeps { cargoLockList = [ ./Cargo.lock @@ -405,14 +421,14 @@ let # Normally remap-path-prefix takes the form --remap-path-prefix=FROM=TO where FROM and TO are directories. # This is intended to map source code paths to generic, relative, or redacted paths. # We are sorta using that mechanism in reverse here in that the empty FROM in the next expression maps our - # source code in the debug info from the current working directory to ${src} (the nix store path where we - # have copied our source code). + # source code in the debug info from the current working directory to `src-prefix`, a fixed + # path that the debug images point at the matching source tree. # # This is nice in that it should allow us to include ${src} in a container with gdb / lldb + the debug files # we strip out of the final binaries we cook and include a gdbserver binary in some # debug/release-with-debug-tools containers. Then, connecting from the gdb/lldb container to the # gdb/lldbserver container should allow us to actually debug binaries deployed to test machines. - "--remap-path-prefix==${src}" + "--remap-path-prefix==${src-prefix}" # Keep debug outputs from retaining the complete Rust toolchain. "--remap-path-prefix=${pkgs.rust-toolchain}/lib/rustlib/src/rust=${pkgs.rust-toolchain.passthru.availableComponents.rust-src}/lib/rustlib/src/rust" ] @@ -633,7 +649,7 @@ let # Record the remapped source root without changing normal archives. + ( if instrumentation == "coverage" then - "; echo -n '${src}' > $out/source-prefix" + "; echo -n '${src-prefix}' > $out/source-prefix" else "" ); @@ -943,6 +959,11 @@ let }).overrideAttrs source-volatile; # gdb needs a writable HOME for logs and its index cache. extraCommands = '' + # Point `src-prefix` at the sources this image ships. Referencing ${src} + # here is also what keeps it in the image closure: with the remap no + # longer naming a store path, nothing else retains it. + mkdir -p ".$(dirname "${src-prefix}")" + ln -s "${src}" ".${src-prefix}" mkdir -p tmp chmod 1777 tmp ''; @@ -976,6 +997,11 @@ let }).overrideAttrs source-volatile; # bugstalker needs a writable HOME for its keymap and history. extraCommands = '' + # Point `src-prefix` at the sources this image ships. Referencing ${src} + # here is also what keeps it in the image closure: with the remap no + # longer naming a store path, nothing else retains it. + mkdir -p ".$(dirname "${src-prefix}")" + ln -s "${src}" ".${src-prefix}" mkdir -p tmp chmod 1777 tmp ''; @@ -1022,6 +1048,11 @@ let ]; }).overrideAttrs source-volatile; extraCommands = '' + # Point `src-prefix` at the sources this image ships. Referencing ${src} + # here is also what keeps it in the image closure: with the remap no + # longer naming a store path, nothing else retains it. + mkdir -p ".$(dirname "${src-prefix}")" + ln -s "${src}" ".${src-prefix}" mkdir -p tmp chmod 1777 tmp ''; diff --git a/justfile b/justfile index 93b0f14c12..b2deaf0b5e 100644 --- a/justfile +++ b/justfile @@ -24,6 +24,21 @@ cores := "0" # Fraction of `cores` available to this invocation, as a decimal or fraction. share := "1" +# Where the build tells rustc our sources live; keep in step with `src-prefix` +# in default.nix. The build bakes it into debug info, and `file!()` reports it +# at runtime, so it has to exist when tests run: bolero canonicalises `file!()` +# to locate its corpus and takes down every property test if it cannot. +src_prefix := "/tmp/dataplane/src" + +# Point `src_prefix` at this checkout. Idempotent, and deliberately root-free +# so a developer running `just test` needs no special setup. +[private] +[script] +_link-sources: + {{ _just_debuggable_ }} + mkdir -p "$(dirname '{{ src_prefix }}')" + ln -sfn "$(pwd)" '{{ src_prefix }}' + # Resolve zero before scaling and never return less than one. The epsilon # prevents floating-point results just below an integer from rounding down. [private] @@ -194,7 +209,7 @@ pre-flight: (check-dependencies) (fmt "--check") (test) (lint) (doctest) echo "pre flight checks pass" [script] -test package="tests.all" *args: (build (if package == "tests.all" { "tests.all" } else { "tests.pkg." + package }) args) +test package="tests.all" *args: (build (if package == "tests.all" { "tests.all" } else { "tests.pkg." + package }) args) _link-sources {{ _just_debuggable_ }} declare -r target="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}" cargo nextest run --archive-file results/${target}/*.tar.zst --workspace-remap $(pwd) {{ filter }} @@ -247,7 +262,7 @@ check-each *args: (build "check" args) {{ _just_debuggable_ }} [script] -test-each *args: (build "tests.pkg" args) +test-each *args: (build "tests.pkg" args) _link-sources {{ _just_debuggable_ }} declare -a fail=() for test_archive in results/tests.pkg*/*.tar.zst; do @@ -614,7 +629,7 @@ coverage *args: # Report coverage from a Nix-built nextest archive. The optional package # matches `just test`; remaining arguments are forwarded to nextest. [script] -coverage-archive package="tests.all" *args: +coverage-archive package="tests.all" *args: _link-sources {{ _just_debuggable_ }} declare -r target="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}" just \ @@ -711,15 +726,20 @@ coverage-archive package="tests.all" *args: exit 1 fi + # The archive's debug info names a fixed prefix that does not exist here, so + # point llvm-cov at the tree the build actually used. `export` needs no + # equivalence: it only emits paths, which the rewrite above already handles. llvm-cov show \ --format=html \ --output-dir="${out}/html" \ --show-branches=count \ + --path-equivalence="${src_prefix},${root}" \ --instr-profile="${out}/coverage.profdata" \ "${objects[@]}" \ "${src_prefix}" llvm-cov report \ + --path-equivalence="${src_prefix},${root}" \ --instr-profile="${out}/coverage.profdata" \ "${objects[@]}" \ "${src_prefix}" From 1719c82e7335a983560a106cfc0ecac7df5b782c Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 11:47:58 -0600 Subject: [PATCH 13/23] ci: widen matrix parallelism for deep runs A queued merge blocks everything behind it and its result is what gates the merge, so it is worth finishing sooner even though the extra runners come out of the pool pull requests are waiting in. Run the merge queue and pushes to main or a release branch four matrix entries at a time; pull requests stay at one so they cannot crowd the queue out. `strategy` cannot read `env` -- only `github`, `inputs`, `needs`, and `vars` -- so the decision travels as a `plan` output, which also puts it in the job that already decides what a run does. Job outputs are strings, hence `fromJSON`. Jobs read it through `strategy.max-parallel`, which is also what feeds `ci::parallel` into `JUST_VARS`, so the per-job core budget follows without a second switch to keep in step. `cross` is deliberately left at one. `ci.just` turns `ci::parallel` into `share = 1/N` and only `ci::cross` and `ci::cross-test` consume it, so raising it there divides each job's eight cores rather than adding throughput -- unless the runner pods really do get isolated core budgets, which is a question for whoever owns the scale set. `miri` has a single matrix entry, so the setting does nothing for it. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/dev.yml | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 0c71c9bfa9..13604033cc 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -71,6 +71,7 @@ jobs: runs-on: "ubuntu-latest" outputs: container_profiles: "${{ steps.container-profiles.outputs.value }}" + parallel: "${{ steps.parallel.outputs.value }}" container_targets: "${{ steps.container-targets.outputs.value }}" profiles: "${{ steps.profiles.outputs.value }}" concurrency: "${{ steps.concurrency.outputs.value }}" @@ -86,6 +87,27 @@ jobs: with: persist-credentials: "false" + # Deep runs get a wider slice of the lab pool. A queued merge blocks + # everything behind it and its result is what actually gates the merge, + # so it is worth finishing sooner even though the extra runners come out + # of the pool the pull requests are waiting in. Pull requests stay at + # one so they cannot crowd the queue out. + # + # `strategy` cannot read `env`, only `github`/`inputs`/`needs`/`vars`, so + # this travels as a `plan` output. Jobs pick it up through + # `strategy.max-parallel`, which is also what feeds `ci::parallel` into + # `JUST_VARS` -- the per-job core budget follows without a second switch. + - id: "parallel" + env: + EVENT: "${{ github.event_name }}" + run: | + set -euo pipefail + case "${EVENT}" in + merge_group | push) value="4" ;; + *) value="1" ;; + esac + printf 'value=%s\n' "${value}" >>"${GITHUB_OUTPUT}" + - id: "miri" uses: &gate "./.github/actions/ci-gate" with: @@ -194,7 +216,7 @@ jobs: JUST_VARS: "ci::parallel=${{ strategy.max-parallel }}" strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" steps: @@ -376,7 +398,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: nix-target: "${{ fromJSON(needs.plan.outputs.container_targets) }}" # TODO: enable cfi and safe-stack on release when possible @@ -430,7 +452,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: sanitizer: - thread @@ -474,7 +496,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} # Fuzz provides optimized coverage while retaining safety checks. matrix: profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" @@ -664,6 +686,10 @@ jobs: JUST_VARS: "ci::parallel=${{ strategy.max-parallel }}" strategy: fail-fast: false + # Deliberately not widened. `ci.just` turns `ci::parallel` into + # `share = 1/N`, and only `ci::cross`/`ci::cross-test` consume it, so + # raising this divides each job's core budget rather than adding + # throughput unless the runner pods are genuinely core-isolated. max-parallel: 1 matrix: platform: From d76a918a1a204e33502ee2a63734e4e0b88931f7 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 13:50:26 -0600 Subject: [PATCH 14/23] ci: honour ci:-vlab again The label still exists and 23 pull requests have carried it, but nothing has read it since 68dc9e198 made VLAB opt-in. It is also the only way to sit out the lab tests while keeping everything else: ci:+merge-ready turns on every gate, VLAB included, so dropping it to skip an hour of lab time would also drop miri, the sanitizers, cross, concurrency, test_each, and wasm. Skip the whole VLAB matrix, hybrid legs included, when a pull request carries ci:-vlab. This mirrors ci:-upgrade, the subtractive label the workflow already honours. Documentation for the label is deliberately left for a separate commit: `src` admits every `*.md` in the tree, so editing the workflow README rehashes every workspace derivation, and this needs to stay cache-neutral while the timing experiment is running. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/dev.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 13604033cc..19e06010a2 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -757,6 +757,7 @@ jobs: with: # ci:+hlab is required to enable hybrid lab tests on PR # ci:+vlab is required to enable virtual lab tests on PR + # ci:-vlab opts back out, including for ci:+merge-ready # ci:-upgrade disables upgrade tests on PR # hlab is disabled for main and merge_queue till we have gateway tests for it # ci:+merge-ready mirrors the merge queue, which skips HLAB. @@ -764,7 +765,8 @@ jobs: ${{ github.event_name == 'pull_request' && ( - matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+hlab') + contains(github.event.pull_request.labels.*.name, 'ci:-vlab') + || matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+hlab') || !matrix.hybrid && !contains(github.event.pull_request.labels.*.name, 'ci:+vlab') && !contains(github.event.pull_request.labels.*.name, 'ci:+merge-ready') From 10f021dbac0ea7303afb0f9b2bf573a5337dde49 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 13:12:49 -0600 Subject: [PATCH 15/23] ci: retry container pushes ghcr.io drops a push every so often -- twice in the last five runs, both times `writing blob: uploading layer chunked: blob upload unknown`, and 403s have been seen too. A lost push fails a job that has already done all its work, and we have no visibility into the registry's side, so retrying is the best available answer. Two layers, because they cover different failures. `--retry-times` is skopeo's own and retries a blob rather than restarting a copy that may already have moved most of an image; it does not retry `denied`/403 or a blob-upload error, which are the two modes we actually see. An outer loop restarts the whole copy for those. That is safe: skopeo skips blobs the registry already has and a partial upload is discarded server side, so the push is idempotent. `oras push` for the wasm validator goes through the same loop. Retries are announced with `::warning::` so the flake rate stays visible in the run summary; a silent wrapper would turn "ghcr is degrading" into "CI got slower". Errors outside the retryable set fail immediately rather than sitting through a minute of backoff first. Collapsing the six identical `skopeo copy` arms into `push_image` is a side effect of having somewhere to put the retry. Verified against simulated failures: a call that fails twice then succeeds recovers and logs two warnings, a persistent 403 exhausts its attempts and errors, and a non-retryable `manifest invalid` fails in 0s. Co-Authored-By: Claude Opus 5 (1M context) --- justfile | 76 +++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 62 insertions(+), 14 deletions(-) diff --git a/justfile b/justfile index b2deaf0b5e..bf729041de 100644 --- a/justfile +++ b/justfile @@ -450,34 +450,82 @@ build-container-quick: push-container target="dataplane" *args: (build-container target args) && version {{ _just_debuggable_ }} declare -xr DOCKER_HOST="${DOCKER_HOST:-unix://{{docker_sock}}}" + + # ghcr.io fails a push every so often, most often with a 403 or a blob + # transfer error, and a lost push costs the whole job. We have no + # visibility into why, so retrying is the best available answer. + # + # Two layers, because they cover different things. `--retry-times` is + # skopeo's own, and retries a *blob* rather than restarting a copy that may + # already have moved most of an image. It treats `denied`/403 as an auth + # failure and a blob-upload error as a 404, neither of which it will retry + # -- which is exactly what we keep seeing -- so an outer loop restarts the + # whole copy for those. That is safe: skopeo skips blobs the registry + # already has, and a partial upload is discarded server side, so a push is + # idempotent. + # + # Every retry is announced so the flake rate stays visible. A silent + # wrapper would turn "ghcr is degrading" into "CI got slower". + retry() { + declare -r what="$1" + shift + declare -ri attempts=4 + declare -i attempt=1 + declare -i delay + declare out + while true; do + if out="$("$@" 2>&1)"; then + printf '%s\n' "${out}" + return 0 + fi + printf '%s\n' "${out}" >&2 + if [ "${attempt}" -ge "${attempts}" ]; then + >&2 echo "::error::${what} failed after ${attempts} attempts" + return 1 + fi + # Retry only what we have seen recover. Anything else is reported + # now rather than buried under a minute of backoff. + if ! grep -qiE 'blob upload (unknown|invalid)|blob transfer|403|forbidden|denied|too many requests|unexpected EOF|connection reset|i/o timeout|TLS handshake' <<<"${out}"; then + >&2 echo "::error::${what} failed with a non-retryable error" + return 1 + fi + delay=$(( 5 * 2 ** (attempt - 1) + RANDOM % 5 )) + >&2 echo "::warning::${what} failed (attempt ${attempt}/${attempts}), retrying in ${delay}s" + sleep "${delay}" + attempt=$(( attempt + 1 )) + done + } + + push_image() { + declare -r image="$1" + retry "push of ${image}" \ + skopeo copy --retry-times=3 --src-daemon-host="${DOCKER_HOST}" \ + {{ _skopeo_dest_insecure }} "docker-daemon:${image}" "docker://${image}" + echo "Pushed ${image}" + } + case "{{target}}" in "dataplane") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane }}" "docker://{{ oci_image_dataplane }}" - echo "Pushed {{ oci_image_dataplane }}" + push_image "{{ oci_image_dataplane }}" ;; "dataplane-core-viewer") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_core_viewer }}" "docker://{{ oci_image_dataplane_core_viewer }}" - echo "Pushed {{ oci_image_dataplane_core_viewer }}" + push_image "{{ oci_image_dataplane_core_viewer }}" ;; "dataplane-dev-debugger") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_dev_debugger }}" "docker://{{ oci_image_dataplane_dev_debugger }}" - echo "Pushed {{ oci_image_dataplane_dev_debugger }}" + push_image "{{ oci_image_dataplane_dev_debugger }}" ;; "dataplane-syscall-tracer") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{ oci_image_dataplane_syscall_tracer }}" "docker://{{ oci_image_dataplane_syscall_tracer }}" - echo "Pushed {{ oci_image_dataplane_syscall_tracer }}" + push_image "{{ oci_image_dataplane_syscall_tracer }}" ;; "debug-tools") >&2 echo "do not push the debug tools!" exit 1 ;; "frr.dataplane") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{oci_image_frr_dataplane}}" "docker://{{oci_image_frr_dataplane}}" - echo "Pushed {{ oci_image_frr_dataplane }}" + push_image "{{oci_image_frr_dataplane}}" ;; "frr.host") - skopeo copy --src-daemon-host="${DOCKER_HOST}" {{ _skopeo_dest_insecure }} "docker-daemon:{{oci_image_frr_host}}" "docker://{{oci_image_frr_host}}" - echo "Pushed {{ oci_image_frr_host }}" + push_image "{{oci_image_frr_host}}" ;; "validator") if [ "{{platform}}" != "wasm32-wasip1" ]; then @@ -485,7 +533,8 @@ push-container target="dataplane" *args: (build-container target args) && versio exit 1 fi pushd ./results/workspace.validator/bin - oras push --annotation version="{{ version }}" "{{ oci_image_dataplane_validator }}" ./validator.wasm + retry "push of {{ oci_image_dataplane_validator }}" \ + oras push --annotation version="{{ version }}" "{{ oci_image_dataplane_validator }}" ./validator.wasm popd echo "Pushed {{ oci_image_dataplane_validator }}" ;; @@ -493,7 +542,6 @@ push-container target="dataplane" *args: (build-container target args) && versio >&2 echo "{{target}} is not a valid container" exit 99 esac - [script] push: {{ _just_debuggable_ }} From a9eb51eda12e832715a7643941f06f22a50e11c7 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:04:38 -0600 Subject: [PATCH 16/23] perf(nix): keep the git version out of the dependency build The shared dependency artifacts were rehashing on every commit, so the split still bought nothing in CI even after the source prefix was fixed. Runs 5 and 6 built dataplane-tests-deps at 297xbx0p... and kfrigyc3... from trees whose only difference was one test function. `invoke` puts `VERSION = tag` in every crane derivation's environment, and the justfile computes `tag` from `git describe --tags --dirty --always`, which moves with every commit. That reached the dependency build as surely as it reached the workspace builds. A dependency build compiles third-party crates and the standard library. None of them read VERSION, and cargo only fingerprints an env var for crates that actually reference it, so pinning it to a constant there leaves the artifacts valid for consumers while decoupling their hash from the commit. Both invariants now hold: the dependency derivation is unchanged across `--argstr tag dev` versus a git-describe style tag, and unchanged across an edit to a workspace source file. This is the second such channel. The first was the source path in RUSTFLAGS; anything else that varies per commit and lands in the dependency build's environment will defeat the split the same way. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index 4fc24b9ec3..0002362f7b 100644 --- a/default.nix +++ b/default.nix @@ -400,7 +400,14 @@ let ]; env = { - VERSION = tag; + # `tag` comes from `git describe`, so it changes on every commit. A + # dependency build compiles third-party crates and the standard + # library, none of which read VERSION, so threading it in would give + # the shared artifacts a new hash per commit -- precisely what the + # split exists to avoid. Consumers still get the real value, and + # cargo only fingerprints an env var for crates that actually read + # it, so the artifacts stay valid for them. + VERSION = if for-deps then "dependencies" else tag; CARGO_PROFILE = cargo-profile; DATAPLANE_SYSROOT = "${sysroot}"; LIBCLANG_PATH = "${pkgs.pkgsBuildHost.llvmPackages'.libclang.lib}/lib"; From 958cd68c5f99ba9058cbd3e8d3796ad3e04b303f Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:07:42 -0600 Subject: [PATCH 17/23] ci: check that the dependency build stays reusable Two per-commit inputs have leaked into the shared dependency build: the source path, through `--remap-path-prefix` in RUSTFLAGS, and the git version, through `VERSION = tag`. Each defeated the split completely, and each took several CI runs to notice, because the symptom is a cache miss rather than a failure -- the jobs still pass, just slowly, and only a careful read of a build log shows dependencies compiling that should have been fetched. `check-deps-reuse` varies the two things that move per commit -- the tag and a workspace source file -- and requires the dependency derivation to hold still. It runs in `lint`, which already has nix available, and costs a pair of instantiations. Verified to fail on both known regressions rather than merely passing on a healthy tree: reintroducing `VERSION = tag` reports the git version, and restoring `--remap-path-prefix==${src}` reports the workspace source. This does not prove there is no third channel. It does mean a third one is caught by the lint job on the commit that introduces it, rather than by reading a coverage log three runs later. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/dev.yml | 11 ++++++++ justfile | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 19e06010a2..92f582adbe 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -352,6 +352,16 @@ jobs: with: recipe: "markdownlint" + # Guards the property the dependency split rests on. Two per-commit + # inputs have already leaked into it and each cost several CI runs to + # notice, because the symptom is a cache miss rather than a failure. + - name: "check-deps-reuse" + id: "check-deps-reuse" + continue-on-error: true + uses: *just + with: + recipe: "check-deps-reuse" + - name: "license-headers" id: "license-headers" continue-on-error: true @@ -370,6 +380,7 @@ jobs: pinact=${{ steps.pinact.outcome }} actionlint=${{ steps.actionlint.outcome }} markdownlint=${{ steps.markdownlint.outcome }} + check-deps-reuse=${{ steps.check-deps-reuse.outcome }} license-headers=${{ steps.license-headers.outcome }} run: | set -euo pipefail diff --git a/justfile b/justfile index bf729041de..456a8b181f 100644 --- a/justfile +++ b/justfile @@ -572,6 +572,61 @@ check-dependencies *args: {{ _just_debuggable_ }} cargo deny {{ _cargo_feature_flags }} check {{ args }} +# Assert that the shared dependency build is reusable. +# +# It exists so that a revision touching Rust code still substitutes its +# third-party crates and standard library, which only works while the +# derivation depends on the manifests and nothing that moves per commit. Two +# things have already broken that: the source path, via `--remap-path-prefix` +# in RUSTFLAGS, and the git version, via `VERSION = tag`. Both were invisible +# until a CI run rebuilt what it should have fetched, several runs after the +# fact. +# +# So vary each per-commit input and require the derivation to hold still. +[script] +check-deps-reuse: + {{ _just_debuggable_ }} + deps_drv() { + declare drv + drv="$(nix-instantiate default.nix -A tests.all --argstr tag "$1" 2>/dev/null | tail -1)" + grep -ao '/nix/store/[a-z0-9]\{32\}-dataplane-tests-deps[^"]*\.drv' "${drv}" | sort -u + } + + declare -r baseline="$(deps_drv dev)" + if [ -z "${baseline}" ]; then + >&2 echo "::error::could not resolve the dependency derivation" + exit 1 + fi + + # A git-describe style tag, which is what CI actually passes. + declare -r tagged="$(deps_drv v0.25.2-15-gdeadbee-dirty)" + if [ "${tagged}" != "${baseline}" ]; then + >&2 echo "::error::the dependency build depends on the git version" + >&2 echo " tag=dev -> ${baseline}" + >&2 echo " tag=v0.. -> ${tagged}" + exit 1 + fi + + # A workspace source edit, which every real pull request makes. Restore + # from a copy rather than `git checkout`, which would discard whatever the + # caller already had uncommitted in this file. + declare -r probe="args/src/lib.rs" + declare -r saved="$(mktemp)" + cp -- "${probe}" "${saved}" + trap 'cp -- "${saved}" "${probe}"; rm -f -- "${saved}"' EXIT + printf '\n// check-deps-reuse\n' >>"${probe}" + declare edited + edited="$(deps_drv dev)" + cp -- "${saved}" "${probe}" + if [ "${edited}" != "${baseline}" ]; then + >&2 echo "::error::the dependency build depends on the workspace source" + >&2 echo " before -> ${baseline}" + >&2 echo " after -> ${edited}" + exit 1 + fi + + echo "dependency build is reusable: ${baseline}" + [script] opengrep: {{ _just_debuggable_ }} From 0a58ac0759e6bb1d6f2fcc7050be5a4f9bb0c448 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:30:56 -0600 Subject: [PATCH 18/23] build(nix): optional cargo build timing report Answers "where did the time actually go" with per-crate numbers rather than counting `Compiling` lines. That question came up twice while sizing the dependency split: 475 of 516 crates are third-party, but a count of compile units is not a share of wall time, and nothing in the logs distinguished them. Off by default. Enabling it changes every cargo command and so rehashes every derivation, which is not a cost to impose on ordinary builds -- verified: with `timings=false` the derivation is byte-identical to before this commit, and with `timings=true` it differs. just timings=true build tests.all leaves the report in `$out/cargo-timings`. Both `cargo build` and `cargo nextest archive` support the flag, so every builder is covered. Note the report embeds a timestamp, so an output built with it is not reproducible. That is another reason to leave it off outside of investigation. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 32 +++++++++++++++++++++++++++++--- justfile | 9 +++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/default.nix b/default.nix index 0002362f7b..4c5a7a0176 100644 --- a/default.nix +++ b/default.nix @@ -11,6 +11,10 @@ kernel ? "linux", tag ? "dev", nightly ? "false", + # Opt-in: `cargo`'s own per-crate build timing report. Off by default so the + # ordinary derivations are untouched -- turning it on changes every build + # command and so rehashes the world. + timings ? "false", }: let sources = import ./npins; @@ -353,6 +357,7 @@ let else [ ] ); + timings-args = if timings == "true" then [ "--timings" ] else [ ]; cargo-cmd-prefix = mk-cargo-cmd-prefix needs-unwind; cargo-cmd-prefix-tests = mk-cargo-cmd-prefix needs-unwind-tests; invoke = @@ -491,9 +496,23 @@ let done '' ); - postFixup = (orig.postFixup or "") + '' - rm -f $out/target.tar.zst - ''; + postFixup = + (orig.postFixup or "") + + '' + rm -f $out/target.tar.zst + '' + + ( + if timings != "true" then + "" + else + '' + if [ -d target/cargo-timings ]; then + mkdir -p "$out/cargo-timings" + cp -r target/cargo-timings/. "$out/cargo-timings/" + fi + '' + ); + } ); @@ -544,6 +563,7 @@ let # whose C sources cannot compile for wasm32-wasip1. ++ (map (pname: "--package=${pname}") (builtins.attrValues package-list)) ++ cmd-prefix + ++ timings-args ); }; }; @@ -579,6 +599,7 @@ let "--profile=${cargo-profile}" ] ++ cargo-cmd-prefix + ++ timings-args ++ [ "--message-format json-render-diagnostics > $cargoBuildLog" ] @@ -612,6 +633,7 @@ let "--profile=${cargo-profile}" ] ++ cargo-cmd-prefix + ++ timings-args ++ [ "--message-format json-render-diagnostics > $cargoBuildLog" ] @@ -652,6 +674,7 @@ let ] ++ (if package != null then [ "--package=${pname}" ] else [ ]) ++ cargo-cmd-prefix-tests + ++ timings-args )) # Record the remapped source root without changing normal archives. + ( @@ -699,6 +722,7 @@ let ] ++ (if package != null then [ "--package=${pname}" ] else [ ]) ++ cargo-cmd-prefix-tests + ++ timings-args ++ [ "--message-format=json-render-diagnostics > $cargoBenchLog;" ] )) + '' @@ -729,6 +753,7 @@ let "--package=${pname}" ] ++ cargo-cmd-prefix + ++ timings-args ++ [ "--" "-D warnings" @@ -767,6 +792,7 @@ let ] ++ (if package != null then [ "--package=${pname}" ] else [ ]) ++ cargo-cmd-prefix + ++ timings-args ); }; }; diff --git a/justfile b/justfile index 456a8b181f..f4431ac708 100644 --- a/justfile +++ b/justfile @@ -24,6 +24,12 @@ cores := "0" # Fraction of `cores` available to this invocation, as a decimal or fraction. share := "1" +# Ask cargo for a per-crate build timing report. Off by default: enabling it +# changes every cargo command and so rebuilds everything. Turn it on for one +# build when you want to know where the time went, e.g. +# `just timings=true build tests.all`, then read $out/cargo-timings. +timings := "false" + # Where the build tells rustc our sources live; keep in step with `src-prefix` # in default.nix. The build bakes it into debug info, and `file!()` reports it # at runtime, so it has to exist when tests run: bolero canonicalises `file!()` @@ -188,6 +194,7 @@ build target="dataplane.tar" *args: --argstr platform '{{ platform }}' \ --argstr tag '{{version}}' \ --argstr nightly '{{nightly}}' \ + --argstr timings '{{timings}}' \ --print-build-logs \ --show-trace \ --out-link "results/${target}" \ @@ -291,6 +298,7 @@ setup-roots *args: --argstr kernel '{{ kernel }}' \ --argstr libc '{{ libc }}' \ --argstr nightly '{{nightly}}' \ + --argstr timings '{{timings}}' \ --argstr platform '{{ platform }}' \ --argstr profile '{{ profile }}' \ --argstr sanitize '{{ sanitize }}' \ @@ -976,6 +984,7 @@ shell: --argstr kernel '{{ kernel }}' \ --argstr libc '{{ libc }}' \ --argstr nightly '{{nightly}}' \ + --argstr timings '{{timings}}' \ --argstr platform '{{ platform }}' \ --argstr profile '{{ profile }}' \ --argstr sanitize '{{ sanitize }}' \ From 2ee24b794d420127b1f64f3d30e098e4e4e301ed Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 14:56:12 -0600 Subject: [PATCH 19/23] ci: build clippy, doctests, and docs through nix `ci::check-clippy` and `ci::check-doctest` invoked bare `cargo` against a `target/` directory that starts empty on every ephemeral runner, so they recompiled the whole dependency graph every time and no amount of nix or Cachix work could touch them. That is why `check` barely moved across eight runs while the jobs that do build through nix swung widely: run 8's `check/debug` fetched its dependency artifact and still compiled 427 third-party crates, because only one of its four steps goes through nix. The nix derivations for all three already existed and nothing called them, so `docs` rotted: thirty-one broken intra-doc links across six crates, each hidden behind the last because rustdoc stops at the first crate that fails. One of them was `acl`'s link to `crate::reference`, which resolves only where the non-default `reference` feature happens to be on; a code span is correct in every configuration, and the "Always built" claim beside it was simply false. `docs-builder` also set RUSTDOCFLAGS without the `--check-cfg=cfg(emulated)` that RUSTFLAGS carries, so every `cfg_attr(emulated, ...)` site tripped `unexpected_cfgs` -- it would not have built even with every link correct. clippy keeps `--all-targets`. Test code is as load bearing as the rest and deserves the same analysis, and dropping it would have quietly narrowed what is linted. Linting test targets means compiling them, so this takes the unwind flavour of `-Zbuild-std`, the test profile, and `cargo-artifacts-tests`, matching how the tests themselves are built. Doctests run inside the sandbox. They cannot be archived and shipped to the host like the other tests: cargo rejects `--doc --no-run` outright, so there is no build-without-running step to archive. Being the first thing to execute in the sandbox, they turned up two things that never mattered while everything ran on the host -- `.cargo/config.toml` is part of `src` while `scripts/test-runner.sh` was not, and that script's `#!/usr/bin/env bash` has nothing to resolve in the sandbox. Hence `.sh` in the source filter and a `patchShebangs` before the build. Cargo reports both as "No such file or directory" against the test rather than the interpreter, which is why they looked identical. Verified by building each: `clippy.acl`, `doctests.all`, and `docs.all`. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/dev.yml | 9 ++++ acl/src/dpdk/install.rs | 2 +- acl/src/lib.rs | 4 +- ci.just | 3 ++ concurrency/src/slot.rs | 2 +- concurrency/src/stress.rs | 2 +- concurrency/src/thread/mod.rs | 2 +- config/src/external/overlay/acl.rs | 2 +- default.nix | 74 ++++++++++++++++++++++++++++-- justfile | 14 +++--- lifecycle/src/lib.rs | 2 +- net/src/headers/embedded_view.rs | 8 ++-- net/src/headers/mod.rs | 2 +- net/src/headers/pat.rs | 2 +- net/src/headers/view.rs | 24 +++++----- net/src/headers/within.rs | 2 +- net/src/ip_auth/v4.rs | 2 +- net/src/ip_auth/v6.rs | 2 +- net/src/ipv6/hop_by_hop.rs | 2 +- 19 files changed, 119 insertions(+), 41 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 92f582adbe..d4f25d2556 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -252,6 +252,15 @@ jobs: recipe: "ci::check-doctest" recipe_args: "${{ matrix.profile }}" + # Nothing built the API docs, so they rotted: thirty broken intra-doc + # links across five crates, plus a missing `--check-cfg` in RUSTDOCFLAGS + # that no amount of link fixing would have gotten past. + - name: "docs" + uses: *just + with: + recipe: "ci::check-docs" + recipe_args: "${{ matrix.profile }}" + - &verify-clean-tree name: "verify-clean-tree" run: | diff --git a/acl/src/dpdk/install.rs b/acl/src/dpdk/install.rs index 65aa107296..939859c91e 100644 --- a/acl/src/dpdk/install.rs +++ b/acl/src/dpdk/install.rs @@ -14,7 +14,7 @@ use crate::dpdk::rule::RuleSpec; /// /// The `rte_acl` field count is computed from `K`'s layout at runtime and /// dispatched to the const-`N` builder shared with the dynamic install path -/// (see [`dispatch_build_classifier`]). The resulting `DpdkAclLookup` +/// (see `dispatch_build_classifier`, crate-private). The resulting `DpdkAclLookup` /// carries no field-count or stride const generics, so one type covers every /// monomorphization of a generic key. pub fn install_table( diff --git a/acl/src/lib.rs b/acl/src/lib.rs index 789d2ae7e7..a32cc0a40f 100644 --- a/acl/src/lib.rs +++ b/acl/src/lib.rs @@ -18,8 +18,8 @@ //! - `dpdk` module (`dpdk` feature): production `rte_acl` backend -- //! layout planner, rule lowering, install, and the single-shot / //! batched classify path. -//! - [`reference`](mod@reference): linear-scan software classifier; -//! differential oracle for the `dpdk` backend. Always built. +//! - `reference` (behind the `reference` feature): linear-scan software +//! classifier; differential oracle for the `dpdk` backend. //! //! [`lookup::Lookup`]: lookup::Lookup //! [`match_action::MatchKey`]: match_action::MatchKey diff --git a/ci.just b/ci.just index 91b33c4c4c..09045a2df8 100644 --- a/ci.just +++ b/ci.just @@ -47,6 +47,9 @@ check-clippy profile: check-doctest profile: just {{ _lab }} profile={{ profile }} doctest +check-docs profile: + just {{ _lab }} profile={{ profile }} docs + sanitize san profile="fuzz": just {{ _lab }} profile={{ profile }} sanitize={{ san }} test diff --git a/concurrency/src/slot.rs b/concurrency/src/slot.rs index 7d13c02eb2..797ecf87ea 100644 --- a/concurrency/src/slot.rs +++ b/concurrency/src/slot.rs @@ -26,7 +26,7 @@ //! the miri job (which runs against the real `ArcSwap` in //! permissive-provenance mode) is where it lives. //! -//! [`Subscriber::snapshot`]: crate::Subscriber::snapshot +//! [`Subscriber::snapshot`]: crate::quiescent::Subscriber::snapshot // Strict provenance checks fail with arc-swap since it uses hazard pointers and does not (yet) use the new // std features to expose provenance information in their mechanics. diff --git a/concurrency/src/stress.rs b/concurrency/src/stress.rs index 725608e7e5..965061a975 100644 --- a/concurrency/src/stress.rs +++ b/concurrency/src/stress.rs @@ -30,7 +30,7 @@ pub fn shuttle_config() -> shuttle::Config { /// /// * default backend -- one direct call, no scheduling exploration. /// * `loom` -- `loom::model`. -/// * `shuttle` -- the [`shuttle_config`]-configured `PortfolioRunner` +/// * `shuttle` -- the `shuttle_config`-configured `PortfolioRunner` /// (`RandomScheduler` + `PctScheduler`, plus `DfsScheduler` under /// `shuttle_dfs`). /// diff --git a/concurrency/src/thread/mod.rs b/concurrency/src/thread/mod.rs index 751c684a8e..8ef07c5fc8 100644 --- a/concurrency/src/thread/mod.rs +++ b/concurrency/src/thread/mod.rs @@ -7,7 +7,7 @@ //! //! `std::thread::scope` (stable since 1.63) and `shuttle::thread::scope` //! are re-exported directly. `loom` 0.7 does not provide `scope`, so we -//! ship a local shim in [`loom_scope`] that matches the std API on top +//! ship a local shim in `loom_scope` that matches the std API on top //! of loom's `spawn` + `park`/`unpark` + atomic primitives, with a //! narrow `unsafe` lifetime launder (same trick std uses internally). //! diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index 342a06fc33..ffbd037bfa 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -50,7 +50,7 @@ pub struct AclPattern { /// Port ranges for match entries that specified neither `cidr` nor `vpcSubnet`, meaning "any /// address within the peering, restricted to these ports". These can't be resolved into /// concrete prefixes until the peering's manifests are known, so they're materialized into - /// `src`/`dst` during [`AclRule::validate_patterns_coverage`] rather than at conversion time. + /// `src`/`dst` during `AclRule::validate_patterns_coverage` rather than at conversion time. pub src_any_ports: Vec, pub dst_any_ports: Vec, pub proto: AclProtoMatch, diff --git a/default.nix b/default.nix index 4c5a7a0176..741881b4c0 100644 --- a/default.nix +++ b/default.nix @@ -220,6 +220,11 @@ let markdownFilter = p: _type: builtins.match ".*\.md$" p != null; jsonFilter = p: _type: builtins.match ".*\.json$" p != null; cHeaderFilter = p: _type: builtins.match ".*\.h$" p != null; + # `.cargo/config.toml` is in `src` and names `scripts/test-runner.sh`, so the + # script has to be there too. It did not matter while every test ran from an + # archive on the host; doctests run in the sandbox, where cargo could not find + # the runner and reported "No such file or directory". + shellFilter = p: _type: builtins.match ".*\.sh$" p != null; # `results` holds the out-links `just build` creates. It is gitignored, but # `cleanSource` does not read gitignore, so without it here every developer # who has run a build carries their own `src` hash and stops matching the @@ -237,6 +242,7 @@ let || (markdownFilter p t) || (jsonFilter p t) || (cHeaderFilter p t) + || (shellFilter p t) || ((outputsFilter p t) && (craneLib.filterCargoSources full-path t)); src = lib.cleanSource ./.; name = "source"; @@ -371,6 +377,8 @@ let # skips the debug-info split and keeps the `target.tar.zst` that the # package path strips. for-deps ? false, + # Skip the binary strip/split step for derivations that produce none. + no-bins ? false, profile, cargo-nextest, hwloc, @@ -453,7 +461,7 @@ let )).overrideAttrs ( orig: - if for-deps then + if for-deps || no-bins then { postBuild = (orig.postBuild or "") + '' unset RUSTFLAGS; @@ -735,24 +743,32 @@ let benches = bench-builder { }; + # `--all-targets` so tests, benches, and examples are linted too. That code + # is as load bearing as the rest and deserves the same static analysis, and + # the bare `cargo clippy` this replaces already covered it. + # + # Linting test targets means compiling them, so this takes the unwind flavour + # of `-Zbuild-std` and the test profile, matching how the tests themselves + # are built. It shares `cargo-artifacts-tests` for the same reason. clippy-builder = { pname ? null, }: pkgs.callPackage invoke { builder = craneLib.mkCargoDerivation; - profile = profile'; + profile = profile-tests'; args = { inherit pname; - cargoArtifacts = cargo-artifacts; + cargoArtifacts = cargo-artifacts-tests; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" "clippy" + "--all-targets" "--profile=${cargo-profile}" "--package=${pname}" ] - ++ cargo-cmd-prefix + ++ cargo-cmd-prefix-tests ++ timings-args ++ [ "--" @@ -769,6 +785,50 @@ let } ) package-list; + # Doctests cannot be built and run separately: cargo rejects + # `--doc --no-run`, and nextest does not run them at all. So run them in the + # sandbox rather than on the runner. They are ordinary library examples -- + # unlike the integration fixtures, which need netns and caps and therefore + # run from an archive on the host. + doctest-builder = + { + package ? null, + }: + let + pname = if package != null then package else "all"; + in + pkgs.callPackage invoke { + builder = craneLib.mkCargoDerivation; + profile = profile-tests'; + no-bins = true; + args = { + inherit pname; + cargoArtifacts = cargo-artifacts-tests; + # `.cargo/config.toml` runs tests through `scripts/test-runner.sh`, + # whose `#!/usr/bin/env bash` has nothing to resolve in the sandbox -- + # cargo reports that as "No such file or directory" against the test + # rather than the interpreter. Archived tests never hit this because + # they run on the host. + preBuild = "patchShebangs scripts/test-runner.sh"; + buildPhaseCargoCommand = builtins.concatStringsSep " " ( + [ + "cargo" + "test" + "--doc" + "--profile=${cargo-profile}" + ] + ++ (if package != null then [ "--package=${pname}" ] else [ ]) + ++ cargo-cmd-prefix-tests + ++ timings-args + ); + }; + }; + + doctests = { + all = doctest-builder { }; + pkg = builtins.mapAttrs (dir: package: doctest-builder { inherit package; }) package-list; + }; + docs-builder = { package ? null, @@ -782,7 +842,10 @@ let args = { inherit pname; cargoArtifacts = cargo-artifacts; - RUSTDOCFLAGS = "-D warnings"; + # `emulated` is registered for rustc through profiles.nix, but rustdoc + # reads RUSTDOCFLAGS rather than RUSTFLAGS, so it needs its own copy or + # every `cfg_attr(emulated, ...)` site trips `unexpected_cfgs`. + RUSTDOCFLAGS = "-D warnings --check-cfg=cfg(emulated)"; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" @@ -1302,6 +1365,7 @@ in dataplane devenv devroot + doctests docs package-list pkgs diff --git a/justfile b/justfile index f4431ac708..a9e0e26189 100644 --- a/justfile +++ b/justfile @@ -650,10 +650,11 @@ zizmor *args="": {{ _just_debuggable_ }} zizmor --persona=pedantic {{args}} . -[script] -clippy *args: +# Through nix, like `check` and `test`, so a developer and CI run the same +# thing and the result is cached. `cargo clippy --all-targets` direct from the +# dev shell is still there when you want a fast inner loop. +clippy package="" *args: (build (if package == "" { "clippy" } else { "clippy." + package }) args) {{ _just_debuggable_ }} - cargo clippy --all-targets {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} -- -D warnings [script] actionlint: @@ -717,10 +718,11 @@ lint: \ {{ _just_debuggable_ }} # Run doctests -[script] -doctest *args: +# Doctests run inside the sandbox: cargo refuses `--doc --no-run`, so they +# cannot be archived and handed to the host the way the other tests are. +doctest package="" *args: (build (if package == "" { "doctests.all" } else { "doctests.pkg." + package }) args) {{ _just_debuggable_ }} - cargo test --doc {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} + # Run instrumented tests and report coverage. Args are forwarded to nextest; for example, # `just coverage -p dataplane-nat` scopes the run to this crate. diff --git a/lifecycle/src/lib.rs b/lifecycle/src/lib.rs index e7519272da..9d589a136b 100644 --- a/lifecycle/src/lib.rs +++ b/lifecycle/src/lib.rs @@ -162,7 +162,7 @@ impl Subsystem { } /// Default drain deadlines. Per-subsystem deadlines bound only the -/// tokio tasks tracked by each [`Subsystem`]; [`TOTAL`] is the absolute +/// tokio tasks tracked by each [`Subsystem`]; [`default_deadlines::TOTAL`] is the absolute /// process-level ceiling enforced by [`spawn_shutdown_watchdog`]. pub mod default_deadlines { use std::time::Duration; diff --git a/net/src/headers/embedded_view.rs b/net/src/headers/embedded_view.rs index 85a0d8a984..fe6d5eba0e 100644 --- a/net/src/headers/embedded_view.rs +++ b/net/src/headers/embedded_view.rs @@ -19,7 +19,7 @@ //! [`EmbeddedTransport`] enum, because the inner packet may have been //! truncated by the ICMP source. //! * IPv6 extension-header gap-check semantics carry over unchanged via -//! the embedded variants on [`ExtGapCheck`](super::pat::ExtGapCheck) +//! the embedded variants on [`ExtGapCheck`] //! (`ext_gap_ok_embedded`). //! //! [`EmbeddedHeadersView`] is the type-level qualifier that closes @@ -63,9 +63,9 @@ use super::{EmbeddedHeaders, EmbeddedStart, EmbeddedTransport, Headers, Net, Net /// Declared, checkable shapes for embedded ICMP-error payloads. /// /// Any tuple whose layers chain through the [`Within`] adjacency -/// graph and the [`EmbeddedStep`] trait is a valid embedded shape. +/// graph and the `EmbeddedStep` trait is a valid embedded shape. /// External crates cannot add new shapes (they cannot implement -/// [`EmbeddedStep`]), but they can write any existing shape at the type +/// `EmbeddedStep`), but they can write any existing shape at the type /// level and let the trait bounds do the filtering. pub trait EmbeddedShape: embedded_sealed::Sealed {} @@ -525,7 +525,7 @@ pub trait EmbeddedLook { /// Extract typed references to the matched inner layers. /// /// Compiles to the same sequence of variant reads as - /// [`embedded_sealed::Sealed::matches`], plus `unwrap_unchecked` + /// `embedded_sealed::Sealed::matches`, plus `unwrap_unchecked` /// at each step; the `EmbeddedHeadersView` type invariant /// guarantees success so the `None` branches are pruned by the /// optimizer. diff --git a/net/src/headers/mod.rs b/net/src/headers/mod.rs index bedb460ddf..22fb5237ca 100644 --- a/net/src/headers/mod.rs +++ b/net/src/headers/mod.rs @@ -844,7 +844,7 @@ impl Headers { /// /// # Errors /// - /// Returns [`PushVlanError::TooManyVlans`] if there are already [`MAX_VLANS`] VLANs on the + /// Returns [`PushVlanError::TooManyVlans`] if there are already `MAX_VLANS` VLANs on the /// stack. /// Returns [`PushVlanError::NoEthernetHeader`] if no Ethernet header is present. pub fn push_vlan(&mut self, vid: Vid) -> Result<(), PushVlanError> { diff --git a/net/src/headers/pat.rs b/net/src/headers/pat.rs index 3f24d9c9db..a5f640119d 100644 --- a/net/src/headers/pat.rs +++ b/net/src/headers/pat.rs @@ -215,7 +215,7 @@ impl_strict_ext!(HopByHop, DestOpts, Routing, Fragment, Ipv4Auth, Ipv6Auth); /// /// `Acc` is the tuple of references accumulated so far. /// -/// Runtime cursors track progress into the `vlan` and `net_ext` [`ArrayVec`] +/// Runtime cursors track progress into the `vlan` and `net_ext` `ArrayVec` /// fields so that skipped intermediate layers are detected. #[must_use = "a Matcher does nothing until .done() is called"] pub struct Matcher<'a, Pos, Acc> { diff --git a/net/src/headers/view.rs b/net/src/headers/view.rs index cbde9522e1..516f60ff28 100644 --- a/net/src/headers/view.rs +++ b/net/src/headers/view.rs @@ -23,7 +23,7 @@ //! references to the matched layers via the [`Look`] trait without //! re-validating at each access site. For mutable access, a //! `&mut HeadersView` yields `&mut` references via [`LookMut::look_mut`], -//! which delegates to [`MatcherMut`](super::pat::MatcherMut) so the +//! which delegates to [`MatcherMut`] so the //! multi-`&mut` tuple is built from the same pre-split //! [`Fields`](super::pat::Fields) helper used by the rest of the //! matcher. @@ -32,26 +32,26 @@ //! //! Zero-cost extraction is achieved by informing the optimizer, via //! `Option::unwrap_unchecked`, that the `HeadersView` type invariant rules -//! out the `None` arms of each [`ViewStep::step`] call. The +//! out the `None` arms of each `ViewStep::step` call. The //! `unsafe` required for this is fully contained: //! //! * [`Headers::as_view`] / [`Headers::as_view_mut`] are the only //! ways to obtain a `&HeadersView` / `&mut HeadersView`, and they run -//! the [`sealed::Sealed::matches`] check -- which threads the same -//! cursors and gap checks as [`ViewStep::step`] would -- before +//! the `sealed::Sealed::matches` check -- which threads the same +//! cursors and gap checks as `ViewStep::step` would -- before //! the `#[repr(transparent)]` reference cast. -//! * [`ViewStep`] is crate-private. Its `step` method is a safe +//! * `ViewStep` is crate-private. Its `step` method is a safe //! `Option`-returning function; [`Look::look`] simply invokes it //! and unwraps unchecked, relying on the `HeadersView` newtype //! invariant. -//! * [`ViewStepMut`] is crate-private and mirrors `ViewStep` for +//! * `ViewStepMut` is crate-private and mirrors `ViewStep` for //! the mutable path, dispatching to -//! [`MatcherMut`](super::pat::MatcherMut) so aliasing of the +//! [`MatcherMut`] so aliasing of the //! returned `&mut` tuple is handled by the existing `Fields` //! pre-split. [`LookMut::look_mut`] unwraps the chain's //! `Option` unchecked under the same `HeadersView` invariant. //! * External callers see only [`HeadersView`], [`Look`], and [`LookMut`]. -//! They cannot implement [`ViewStep`] or [`ViewStepMut`] or call +//! They cannot implement `ViewStep` or `ViewStepMut` or call //! them directly, so they cannot forge a `HeadersView` that sidesteps //! `matches`. //! * `HeadersView` has private fields and no owning constructor; @@ -210,7 +210,7 @@ pub struct HeadersView(Headers, PhantomData); /// Declared, checkable shapes for [`HeadersView`]. /// /// Any tuple whose layers chain through the [`Within`] adjacency -/// graph and the [`ViewStep`] trait is a [`Shape`]. External +/// graph and the `ViewStep` trait is a [`Shape`]. External /// crates cannot add new shapes (they cannot implement `ViewStep`), /// but they can write any existing shape at the type level and let the /// trait bounds do the filtering. @@ -355,7 +355,7 @@ pub trait Look { /// Extract typed references to the matched layers. /// /// Compiles to the same sequence of field/variant reads as - /// [`sealed::Sealed::matches`], plus `unwrap_unchecked` at each + /// `sealed::Sealed::matches`, plus `unwrap_unchecked` at each /// step; the `HeadersView` type invariant guarantees success so the /// `None` branches are pruned by the optimizer. fn look<'a>(&'a self) -> Self::Refs<'a> @@ -367,7 +367,7 @@ pub trait Look { /// /// Yields a tuple of `&mut` references to the matched layers. Aliasing /// between the returned references is handled by -/// [`MatcherMut`](super::pat::MatcherMut)'s pre-split +/// [`MatcherMut`]'s pre-split /// [`Fields`](super::pat::Fields) -- `look_mut` delegates to a /// `MatcherMut` chain and unwraps the result unchecked, relying on the /// `HeadersView` shape invariant. @@ -379,7 +379,7 @@ pub trait LookMut { /// Extract typed `&mut` references to the matched layers. /// - /// Compiles to the same [`MatcherMut`](super::pat::MatcherMut) chain + /// Compiles to the same [`MatcherMut`] chain /// as the corresponding `Matcher` chain used by [`Look::look`], but /// mutable. The `HeadersView` type invariant guarantees the chain /// matches, so the final `.done()` is unwrapped unchecked and the diff --git a/net/src/headers/within.rs b/net/src/headers/within.rs index 68df2bddc2..2fc8646360 100644 --- a/net/src/headers/within.rs +++ b/net/src/headers/within.rs @@ -418,7 +418,7 @@ use crate::icmp6::TruncatedIcmp6; use crate::tcp::TruncatedTcp; use crate::udp::TruncatedUdp; -/// Marker type for the starting position of an [`EmbeddedMatcher`]. +/// Marker type for the starting position of an [`crate::headers::pat::EmbeddedMatcher`]. /// /// Embedded headers begin at the network layer (no Eth, no VLAN), so /// `Within` is implemented for Ipv4, Ipv6, and Net. diff --git a/net/src/ip_auth/v4.rs b/net/src/ip_auth/v4.rs index 5aad029619..db168e140a 100644 --- a/net/src/ip_auth/v4.rs +++ b/net/src/ip_auth/v4.rs @@ -3,7 +3,7 @@ //! IPv4-context IP Authentication Header. //! -//! This is a [`repr(transparent)`] newtype over [`IpAuth`] that marks the +//! This is a `repr(transparent)` newtype over [`IpAuth`] that marks the //! header as appearing in an IPv4 extension chain. The builder uses this //! type to restrict `Within` impls so that `Ipv4Auth` can only follow //! IPv4-legal parents (e.g. `Ipv4`), preventing it from being stacked diff --git a/net/src/ip_auth/v6.rs b/net/src/ip_auth/v6.rs index 08aa2ba9a9..a77e029192 100644 --- a/net/src/ip_auth/v6.rs +++ b/net/src/ip_auth/v6.rs @@ -3,7 +3,7 @@ //! IPv6-context IP Authentication Header. //! -//! This is a [`repr(transparent)`] newtype over [`IpAuth`] that marks the +//! This is a `repr(transparent)` newtype over [`IpAuth`] that marks the //! header as appearing in an IPv6 extension chain. The builder uses this //! type to restrict `Within` impls so that `Ipv6Auth` can only follow //! IPv6-legal parents (e.g. `Ipv6`, `Fragment`, `Routing`). diff --git a/net/src/ipv6/hop_by_hop.rs b/net/src/ipv6/hop_by_hop.rs index f52ab90fe4..ea091788db 100644 --- a/net/src/ipv6/hop_by_hop.rs +++ b/net/src/ipv6/hop_by_hop.rs @@ -6,7 +6,7 @@ //! The Hop-by-Hop Options header carries optional information that **must** be //! examined by every node along a packet's delivery path. Per [RFC 8200 section 4.1], //! it **must** immediately follow the IPv6 header when present -- the builder -//! enforces this via [`Within`] bounds. +//! enforces this via `Within` bounds. //! //! [RFC 8200 section 4.1]: https://datatracker.ietf.org/doc/html/rfc8200#section-4.1 //! [RFC 8200 section 4.3]: https://datatracker.ietf.org/doc/html/rfc8200#section-4.3 From 0afb5480e46b148a5a1754624eefbaad6682c0e9 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 15:16:24 -0600 Subject: [PATCH 20/23] perf(nix): lint the workspace in one derivation `clippy` ran as forty-one per-package derivations, each unpacking the shared dependency artifacts before linting one crate. `clippy.args` -- one of the smallest packages -- took 17.8s with those artifacts already warm, so almost all of it was fixed setup. Forty-one of those is about twelve minutes to lint a workspace that takes 32.4s in a single derivation, measured either side of this change. The per-package split bought no cache granularity to pay for it: `src` covers the whole workspace, so editing any file invalidates all forty-one at once. `clippy.pkg.` is still there for linting one crate by hand. Both `--package` lists come from `package-list` rather than `--workspace`, because it is platform-aware: on wasm32-wasip1 the excluded members pull in dependencies that cannot build for that target. Co-Authored-By: Claude Opus 5 (1M context) --- default.nix | 29 +++++++++++++++++++++-------- justfile | 2 +- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/default.nix b/default.nix index 741881b4c0..664ad7a35a 100644 --- a/default.nix +++ b/default.nix @@ -752,8 +752,11 @@ let # are built. It shares `cargo-artifacts-tests` for the same reason. clippy-builder = { - pname ? null, + package ? null, }: + let + pname = if package != null then package else "all"; + in pkgs.callPackage invoke { builder = craneLib.mkCargoDerivation; profile = profile-tests'; @@ -766,8 +769,16 @@ let "clippy" "--all-targets" "--profile=${cargo-profile}" - "--package=${pname}" ] + # Name the packages rather than passing `--workspace`: `package-list` + # is platform-aware, and on wasm32-wasip1 the excluded members pull in + # dependencies that cannot build for it. + ++ ( + if package != null then + [ "--package=${pname}" ] + else + map (p: "--package=${p}") (builtins.attrValues package-list) + ) ++ cargo-cmd-prefix-tests ++ timings-args ++ [ @@ -778,12 +789,14 @@ let }; }; - clippy = builtins.mapAttrs ( - dir: pname: - clippy-builder { - inherit pname; - } - ) package-list; + # One derivation over the whole workspace by default. Per-package linting + # bought no cache granularity -- `src` is workspace wide, so an edit anywhere + # invalidates every one of them together -- while each paid its own unpack of + # the shared artifacts, about 17s of fixed cost times forty-one packages. + clippy = { + all = clippy-builder { }; + pkg = builtins.mapAttrs (dir: package: clippy-builder { inherit package; }) package-list; + }; # Doctests cannot be built and run separately: cargo rejects # `--doc --no-run`, and nextest does not run them at all. So run them in the diff --git a/justfile b/justfile index a9e0e26189..7b710f6401 100644 --- a/justfile +++ b/justfile @@ -653,7 +653,7 @@ zizmor *args="": # Through nix, like `check` and `test`, so a developer and CI run the same # thing and the result is cached. `cargo clippy --all-targets` direct from the # dev shell is still there when you want a fast inner loop. -clippy package="" *args: (build (if package == "" { "clippy" } else { "clippy." + package }) args) +clippy package="" *args: (build (if package == "" { "clippy.all" } else { "clippy.pkg." + package }) args) {{ _just_debuggable_ }} [script] From 9aa9b77f253a25efb389a6227fd7785fe77bfa69 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 17:31:48 -0600 Subject: [PATCH 21/23] ci: give nix the whole core budget, and split it for test_each The lab cgroup allows ten cores. `cores=8` left two of them idle on every job, and nothing inside the container can see the limit to catch that: `nproc` reports the node's 32 and the cgroup files are not readable from the pod. Measuring throughput from inside cannot recover it either, since concurrent slots that fit within ten cores show no contention. Treat the ten as given rather than inferred. One derivation at a time suits the heavy jobs, which each build a single large derivation: on an otherwise identical run `dataplane/release` went 559s -> 454s and `check/fuzz` 944s -> 709s. `test_each` is the exception, building one derivation per package, and it went the other way at 613s -> 887s. Give it two jobs of five cores so that a package too small to saturate the budget no longer leaves most of it idle, while the total request stays inside the cgroup. Co-Authored-By: Claude Opus 5 (1M context) --- ci.just | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/ci.just b/ci.just index 09045a2df8..34b2ce9b1b 100644 --- a/ci.just +++ b/ci.just @@ -21,13 +21,37 @@ _parallel := if parallel == "" { "1" } else { parallel } [private] _share := if _parallel == "1" { "1" } else { "1/" + _parallel } -# Nix build budget for a 10-core lab runner. +# Nix build budget for a lab runner. +# +# The cgroup allows ten cores. Nothing inside the container can see that: +# `nproc` reports the node's 32 and the cgroup files are not readable from the +# pod. Measuring throughput from inside cannot recover it either, since a load +# that fits within ten cores shows no contention. Treat the ten as given. +# +# `jobs * cores` is the request, so it has to come to ten. One derivation at a +# time with the whole budget suits the heavy jobs, which each build a single +# large derivation -- `check`, the sanitizers, and the container builds all do. jobs := "1" -cores := "8" +cores := "10" + +# `test_each` is the exception. It builds one derivation per package, so a +# single job leaves most of the budget idle whenever a package is too small to +# saturate ten cores: it cost 887s at `jobs=1 cores=10` against 613s at +# `jobs=4 cores=8`, and the latter is faster only because it overcommits the +# cgroup. Split the ten two ways instead. +test_each_jobs := "2" + +test_each_cores := "5" + +[private] +_lab-common := " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile + +[private] +_lab := "jobs=" + jobs + " cores=" + cores + _lab-common [private] -_lab := "jobs=" + jobs + " cores=" + cores + " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile +_lab-test-each := "jobs=" + test_each_jobs + " cores=" + test_each_cores + _lab-common [default] [private] @@ -54,7 +78,7 @@ sanitize san profile="fuzz": just {{ _lab }} profile={{ profile }} sanitize={{ san }} test test-each profile="debug": - just {{ _lab }} profile={{ profile }} test-each + just {{ _lab-test-each }} profile={{ profile }} test-each coverage profile="debug": just {{ _lab }} profile={{ profile }} instrument=coverage coverage-archive From 6367dd8441fea2197b4ca50b6d4959ae29c2bcce Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 17:38:59 -0600 Subject: [PATCH 22/23] ci: cut the coverage test floor and stop checking the fuzz profile twice Two costs that no amount of caching reaches. `test_concurrency_fibtable` drives 100k packets through six threads. Under `-Cinstrument-coverage` every counter is instrumented and it runs 47.6s of an 84s suite -- nextest cannot finish faster than its slowest test, so it is a floor under `check`, `coverage`, `sanitize`, and `test_each` alike. Coverage measures which lines execute, not how often, so give it 2k packets there via an `instrumented` cfg, registered and set the same way `emulated` already is. Deliberately not set for the sanitizers: those runs want the iterations, because that is how they find races. Three smaller concurrency tests in the same file cost 5-9s each and are left alone for now. `check/fuzz` compiles the workspace optimised to re-verify what `check/release` has already covered, for 709s in the differential run. The fuzz build still gets exercised by `coverage/fuzz`, both sanitizers, and the fuzzing campaign these worker-minutes are being freed up for. Co-Authored-By: Claude Opus 5 (1M context) --- .cargo/config.toml | 2 +- .github/workflows/dev.yml | 9 +++++++++ nix/profiles.nix | 7 +++++++ routing/src/fib/test.rs | 3 +++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 01ff41a155..bba0a7acb2 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -13,7 +13,7 @@ CARGO_LLVM_COV_BUILD_DIR = { value = "target/llvm-cov/target", relative = true, [build] # Register `emulated` so cfg_attr sites don't trip unexpected_cfgs natively. -rustflags = ["--cfg=tokio_unstable", "--check-cfg=cfg(emulated)"] +rustflags = ["--cfg=tokio_unstable", "--check-cfg=cfg(emulated)", "--check-cfg=cfg(instrumented)"] [target.wasm32-wasip1] # Trailing `--` separates wasmtime's CLI from the module + module args diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index d4f25d2556..468a50e5fd 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -219,6 +219,15 @@ jobs: max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: profile: "${{ fromJSON(needs.plan.outputs.profiles) }}" + exclude: + # The fuzz profile is release plus the safety checks, so checking it + # here mostly re-verifies what `check/release` already covered, at + # the price of compiling the workspace optimised a second time -- + # 944s in the differential run, the most expensive job in it. The + # fuzz build is exercised where it matters: `coverage/fuzz`, both + # sanitizers, and the fuzzing campaign these worker-minutes are being + # freed up for. + - profile: "fuzz" steps: - *checkout diff --git a/nix/profiles.nix b/nix/profiles.nix index db0e0b6502..794a0fa229 100644 --- a/nix/profiles.nix +++ b/nix/profiles.nix @@ -45,6 +45,12 @@ let # Register `emulated` so `#[cfg_attr(emulated, ...)]` never trips # `unexpected_cfgs`; only *set* for is-emulated-test and miri. "--check-cfg=cfg(emulated)" + # Likewise `instrumented`, set only under coverage. Counter-heavy loops + # cost far more with `-Cinstrument-coverage` -- one fib test runs 47.6s of + # an 84s suite -- and coverage cares which lines execute, not how many + # times. Deliberately not set for the sanitizers: those runs want the + # iterations, since that is how they find races. + "--check-cfg=cfg(instrumented)" "-Cdebuginfo=full" "-Cdwarf-version=5" "-Csymbol-mangling-version=v0" @@ -61,6 +67,7 @@ let ] ) ++ (if is-emulated-test then [ "--cfg=emulated" ] else [ ]) + ++ (if instrumentation == "coverage" then [ "--cfg=instrumented" ] else [ ]) ++ (map (flag: "-Clink-arg=${flag}") common.NIX_CFLAGS_LINK); optimize-for.debug.NIX_CFLAGS_COMPILE = [ "-fno-inline" diff --git a/routing/src/fib/test.rs b/routing/src/fib/test.rs index 7070eee4c4..bac97ec8ca 100644 --- a/routing/src/fib/test.rs +++ b/routing/src/fib/test.rs @@ -253,6 +253,9 @@ mod tests { const NUM_WORKERS: u16 = 6; const NUM_PACKETS: u64 = cfg_select! { emulated => 30, + // Coverage instruments every counter; the full count costs ~47s + // and reaches no additional lines. + instrumented => 2_000, _ => 100_000, }; const TENTH: u64 = NUM_PACKETS / 10; From c495efc4554cd3de421b946216830c7daa441c01 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 18:18:40 -0600 Subject: [PATCH 23/23] docs(ci): document the ci:-vlab label `ci:-vlab` has been honoured since the label handling landed, but the workflow README listed `ci:-upgrade` as the sole subtractive label. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index b8e4c1fa7d..867c427fce 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -71,10 +71,12 @@ Production artifacts are produced via nix builds in a separate CI workflow. - `ci:+vlab` - Run VLAB tests on this PR - `ci:+hlab` - Run HLAB tests on this PR - `ci:+release` - Enable release tests for VLAB/HLAB on this PR +- `ci:-vlab` - Skip VLAB and HLAB tests on this PR, even with `ci:+merge-ready` - `ci:-upgrade` - Disable upgrade tests on this PR Labels are additive, and optional: a pull request needs none of them. -`ci:-upgrade` is the sole exception, subtracting a job that would otherwise run. +`ci:-vlab` and `ci:-upgrade` are the exceptions, subtracting jobs that would +otherwise run. Adding a label starts a **new** workflow run, and that run repeats the default jobs as well as the ones the label enabled.