From 5fbd67161583ac0f473b5bc580df99686d1cf31a Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:36:28 -0600 Subject: [PATCH 1/8] 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 | 9 +++++++++ nix/overlays/llvm.nix | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/default.nix b/default.nix index 301dba5408..c1f0f0e509 100644 --- a/default.nix +++ b/default.nix @@ -431,6 +431,8 @@ let # Keep debug paths stable across revisions. Source readers # must resolve this relative prefix from the workspace root. "--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" ] ) else @@ -470,6 +472,13 @@ let mkdir -p $debug/bin for f in $out/bin/*; do mv "$f" "$debug/bin/$(basename "$f")" + # Trade index for size. gdb has consumed `.debug_names` + # as a real DWARF-5 index since 14, and this ships 17.2, + # so dropping it is not free -- gdb rebuilds an index on + # each start instead. The section is large enough on + # these binaries that the image is worth more than the + # startup, and bugstalker does not read it at all. + ${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 2c9340d3a7b9a99aa43017485496cc155bf67fb5 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:37:45 -0600 Subject: [PATCH 2/8] 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 | 2 + default.nix | 82 ++++++++++++++++++++++++++----------- justfile | 17 ++++---- 4 files changed, 84 insertions(+), 32 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index d28097881f..0c1ef549ef 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -103,7 +103,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 @@ -111,6 +112,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 5cb2a33d5c..171e694cfd 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -428,6 +428,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) }}" diff --git a/default.nix b/default.nix index c1f0f0e509..3313c11f20 100644 --- a/default.nix +++ b/default.nix @@ -971,32 +971,68 @@ let }).overrideAttrs source-volatile; - containers.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/debugger"; + name = "ghcr.io/githedgehog/dataplane/core-viewer"; inherit tag; - contents = pkgs.buildEnv { - name = "dataplane-debugger-env"; - pathsToLink = [ - "/bin" - "/etc" - "/var" - "/lib" - ]; - 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 + contents = + (pkgs.buildEnv { + name = "dataplane-core-viewer-env"; + pathsToLink = [ + "/bin" + "/etc" + "/var" + "/lib" + ]; + paths = [ + pkgs.pkgsBuildHost.gdb + rust-gdb-printers + ] + ++ debug-image-paths; + }).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 + ''; + 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" ]; }; }).overrideAttrs source-volatile; diff --git a/justfile b/justfile index 6eed433ff8..408eb0be5f 100644 --- a/justfile +++ b/justfile @@ -114,7 +114,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 @@ -279,10 +279,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.) @@ -386,8 +386,8 @@ push-container target="dataplane" *args: (build-container target args) && versio "dataplane") push_image "{{ oci_image_dataplane }}" ;; - "dataplane-debugger") - push_image "{{ oci_image_dataplane_debugger }}" + "dataplane-core-viewer") + push_image "{{ oci_image_dataplane_core_viewer }}" ;; "debug-tools") >&2 echo "do not push the debug tools!" @@ -421,7 +421,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 9817fc84d8a4d38509191d2d94c7f6d1add4e0e5 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:38:11 -0600 Subject: [PATCH 3/8] 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 | 49 ++++++++++++++++++++++++++++++++-- .github/workflows/dev.yml | 2 ++ default.nix | 48 +++++++++++++++++++++++++++++++++ justfile | 9 +++++++ nix/overlays/dataplane-dev.nix | 9 +++++++ npins/sources.json | 16 +++++++++++ 6 files changed, 131 insertions(+), 2 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 0c1ef549ef..9fcce2c5cf 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -103,8 +103,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 @@ -124,6 +124,51 @@ 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 127.0.0.1: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`. `type` has to match whatever debug + type the BugStalker extension you installed registers -- it is not a name we + choose, and it differs between extensions, so check the one you have rather + than copying this field blind: + + ```json + { + "type": "bs", + "request": "launch", + "name": "dataplane (container)", + "debugServer": 4711, + "program": "/bin/dataplane", + "args": [] + } + ``` + + For `nvim-dap`, where the first line names the adapter itself, so `type = "bs"` + below is our own label rather than an extension's: + + ```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 171e694cfd..5d4f972c06 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -430,6 +430,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) }}" diff --git a/default.nix b/default.nix index 3313c11f20..74ad7468d3 100644 --- a/default.nix +++ b/default.nix @@ -1037,6 +1037,54 @@ let }).overrideAttrs source-volatile; + # 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; + }).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 + ''; + 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" = { }; + }; + }; + }).overrideAttrs + source-volatile; + debug-tools = pkgs: [ diff --git a/justfile b/justfile index 408eb0be5f..07f78247ec 100644 --- a/justfile +++ b/justfile @@ -115,6 +115,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 @@ -284,6 +285,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. @@ -389,6 +395,9 @@ push-container target="dataplane" *args: (build-container target args) && versio "dataplane-core-viewer") push_image "{{ oci_image_dataplane_core_viewer }}" ;; + "dataplane-dev-debugger") + push_image "{{ 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 e303595dffd97069a07684815cafa13abee1f2e7 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:38:36 -0600 Subject: [PATCH 4/8] 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 | 22 +++++++++++++++- .github/workflows/dev.yml | 2 ++ default.nix | 47 ++++++++++++++++++++++++++++++++++ justfile | 9 +++++++ nix/overlays/dataplane-dev.nix | 27 +++++++++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 9fcce2c5cf..a036099f08 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -103,7 +103,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 @@ -169,6 +169,26 @@ 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, so it is smaller than the other two -- though not by as much + as that suggests: like them it ships the source tree, which the entrypoint + makes the working directory. Only the debug symbols and the debuggers + themselves are absent. + + ```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 5d4f972c06..915f57be6a 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -432,6 +432,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) }}" diff --git a/default.nix b/default.nix index 74ad7468d3..6f6345a8f3 100644 --- a/default.nix +++ b/default.nix @@ -1085,6 +1085,53 @@ let }).overrideAttrs source-volatile; + # 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 + ]; + }).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 + ''; + config = { + Entrypoint = [ + "/bin/lurk" + "--json" + # Include the worker threads where the dataplane does its work. + "--follow-forks" + "/bin/dataplane" + ]; + Env = [ "HOME=/tmp" ]; + }; + }).overrideAttrs + source-volatile; + debug-tools = pkgs: [ diff --git a/justfile b/justfile index 07f78247ec..63d9ac2cd9 100644 --- a/justfile +++ b/justfile @@ -116,6 +116,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 @@ -290,6 +291,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. @@ -398,6 +404,9 @@ push-container target="dataplane" *args: (build-container target args) && versio "dataplane-dev-debugger") push_image "{{ oci_image_dataplane_dev_debugger }}" ;; + "dataplane-syscall-tracer") + push_image "{{ 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 417a9eab36d1fd6d6559a4a5f7deff64f5ba47db Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 14 Aug 2026 21:39:43 -0600 Subject: [PATCH 5/8] 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 | 10 ++++++++-- .github/workflows/dev.yml | 22 ++++++++++++---------- justfile | 10 ++++++++-- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index a036099f08..4f8fd6d86d 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -63,6 +63,11 @@ 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, and `ci:+merge-ready` turns them on too, since + `ci-gate` treats that label as enabling every gate; 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 @@ -103,8 +108,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 915f57be6a..547d3e9b33 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -68,6 +68,7 @@ jobs: 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 }}" cross: "${{ steps.cross.outputs.value }}" @@ -132,6 +133,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 @@ -425,16 +436,7 @@ jobs: fail-fast: false max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} 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 63d9ac2cd9..b2f04c564f 100644 --- a/justfile +++ b/justfile @@ -439,8 +439,14 @@ push-container target="dataplane" *args: (build-container target args) && versio [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 dd4e1d45424d9ab675706aa03168ea1c79c9c373 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 21:36:02 -0600 Subject: [PATCH 6/8] 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 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 debugger only for coming up and listening. Driving a real DAP session from CI means carrying a protocol client in-tree, and the contract it would pin is exercised better by pointing an editor at the image. It also would not have caught the defect on that image, which was in the documentation rather than the runtime. 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) Signed-off-by: Daniel Noland --- .github/workflows/dev.yml | 11 ++++ ci.just | 4 ++ justfile | 105 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 547d3e9b33..acd5c20e4f 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -451,6 +451,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 f4c5406f8f..7f659c5ecc 100644 --- a/ci.just +++ b/ci.just @@ -78,6 +78,10 @@ cross platform libc +args: cross-test platform libc: NEXTEST_PROFILE=cross-qemu just {{ _lab }} 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 }} + # Publish both content-derived and discoverable per-commit tags. [script] push-container target profile version: diff --git a/justfile b/justfile index b2f04c564f..fb7ad50d2f 100644 --- a/justfile +++ b/justfile @@ -258,6 +258,111 @@ 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 (if target == "dataplane-core-viewer" { target } else if target == "dataplane-dev-debugger" { target } else if target == "dataplane-syscall-tracer" { target } else { error("smoke-container: no smoke test for '" + target + "'; expected dataplane-core-viewer, dataplane-dev-debugger, or dataplane-syscall-tracer") })) + {{ _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 rather than a + # variable: `declare -x` would put it in the environment, and every + # child process then fails to exec with E2BIG, which reads exactly + # like a failed trace. Here-strings are fine at this size -- bash + # backs them with a pipe or temp file -- and the checks below use + # them on captured output. + declare trace + trace="$(mktemp)" + declare -r trace + # Name the container and remove it by name. `timeout` signals the + # docker CLI, and the daemon -- not the CLI -- owns the container's + # lifetime, so on the timeout path `--rm` alone can leave a `lurk` + # tracing something for as long as the runner lives. + declare cid + cid="smoke-syscall-tracer-$$" + declare -r cid + trap 'rm -f -- "${trace}"; docker rm -f "${cid}" >/dev/null 2>&1 || true' EXIT + # No seccomp relaxation on purpose: this is the documented command. + timeout 60 docker run --rm --name "${cid}" \ + "{{ 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") + # Only that it comes up and listens. Driving a DAP session from + # CI means carrying a protocol client for a contract better + # exercised by pointing a real editor at `just debug bugstalker`. + declare cid + # Let docker pick the host port and read it back. A fixed one + # collides: these run on a shared daemon, and `container_profiles` + # can put two of these jobs on the same node at once, where the + # second bind fails and reads as "the listener never came up". + cid="$(docker run -d --rm -p 127.0.0.1::4711 "{{ oci_image_dataplane_dev_debugger }}")" + declare -r cid + trap 'docker kill "${cid}" >/dev/null 2>&1 || true' EXIT + declare -i waited=0 + declare port + port="$(docker port "${cid}" 4711/tcp | head -1)" + port="${port##*:}" + declare -r port + if [ -z "${port}" ]; then + >&2 echo "::error::docker published no host port for 4711/tcp" + exit 1 + fi + until timeout 1 bash -c "/dev/null; do + if [ "${waited}" -ge 60 ]; then + >&2 echo "::error::dev-debugger never listened on 4711" + >&2 docker logs "${cid}" 2>&1 | tail -20 + exit 1 + fi + sleep 1 + waited=$(( waited + 1 )) + done + echo "dev-debugger: DAP listener up" + ;; + "dataplane-core-viewer") + # The Rust pretty-printers are the reason this image exists, and + # what registers them is the entrypoint's own `source` flag -- so + # drive the real entrypoint rather than invoking gdb directly, + # which would only test a copy of it. + 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" + ;; + *) + # Unreachable: the dependency above rejects anything else first. + >&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) From 9f7481ba89f7abc0cb0205045fd62acf01e9305a Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 15 Aug 2026 21:36:22 -0600 Subject: [PATCH 7/8] feat(debug): run a binary or a single test inside the debug images The published images debug what CI built. Debugging what you are building meant either rebuilding an image by hand or falling back to a system gdb, which is exactly the case where symbols do not line up. `just debug ` builds the image that carries the tool at the current profile, platform, instrumentation, and sanitizer, and runs the target inside it: - `lurk` traces syscalls and streams JSON until the program exits. - `gdb` runs gdbserver and waits, printing the `target remote` line. - `bugstalker` waits for a DAP client, printing the `program` and `args` for the launch request -- in remote-DAP mode it takes those from the client rather than from its own command line. `target` is either one of the binaries the images already carry, in which case nothing needs mounting, or a nextest filter. For a test the archive is built and unpacked, and that binary was built outside the image, so the store comes along read-only and it runs with its package directory as the working directory the way nextest runs it. Naming a target exactly is the tedious part -- test paths are long and nobody remembers them -- so leaving it out offers everything through skim, and a filter matching several offers those. Resolution stays unambiguous: exactly one match runs without asking, no match is an error, and several with no terminal to ask at is an error listing them rather than a guess. That last case is what keeps this safe to call from a script. `just debug-list` prints the same list without running anything, and `just inspect-core` opens a core file in a gdb built from the same settings. Randomization stays enabled under gdbserver. Docker's default seccomp answers personality(ADDR_NO_RANDOMIZE) with EPERM, and while gdbserver treats that as non-fatal -- unlike lurk, which is why lurk is patched -- it otherwise opens with a warning that reads like a real failure. Co-authored-by: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- .github/workflows/README.md | 47 ++++++++ default.nix | 38 +++--- justfile | 228 ++++++++++++++++++++++++++++++++++++ 3 files changed, 298 insertions(+), 15 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 4f8fd6d86d..3c0dc0ba7b 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -205,6 +205,53 @@ If those queue failures stop being rare, the phasing is worth revisiting. Both upload unarchived, so they download as the named file rather than wrapped in a zip. +### Debugging locally + +The published images debug what CI built. To debug what you are building, `just +debug` builds the matching image and runs a workspace binary or a single test +inside it, at whatever `profile`, `platform`, `instrument`, and `sanitize` you +pass. Symbols only line up when those match the build the problem appeared in, +which is the whole reason to go through the image rather than a system gdb. + +```console +just debug # pick from a list +just debug bugstalker # pick, then wait for an editor +just debug lurk dataplane # trace syscalls, streams JSON, runs to exit +just debug gdb dataplane # gdbserver on 2345, waits for a client +just profile=fuzz debug gdb test_parse_interface args +just debug-list # print the same list without running anything +``` + +Name nothing and everything is offered through `skim`, which the dev shell +provides. Name a filter matching one test and it runs without asking; name one +matching several and those are offered. A filter matching nothing is an error +rather than a guess, and so is an ambiguous one when there is no terminal to +ask at, which is what makes this safe to call from a script. + +The third argument narrows which archive is searched, so +`just debug gdb some_test args` builds only `args`' tests. It is worth passing: +the default builds every test in the workspace, which is a long wait if all you +wanted was to pick from a short list. + +`gdb` and `bugstalker` block until you disconnect and interrupt them; that is +the point. `gdb` prints the `target remote` line to use. `bugstalker` prints a +`.zed/debug.json` entry ready to paste, because in remote-DAP mode it takes the +program from the client's launch request rather than from its own command line, +so connecting an editor is only half of it. The `tcp_connection` field in that +entry is what stops the editor spawning a second debugger of its own. + +A test runs with its package directory as the working directory, the way +nextest runs it, so relative paths behave the same as under `just test`. + +To open a core file: + +```console +just inspect-core /path/to/core.1234 +``` + +Pass the same build settings that produced the binary that dumped +(`just profile=release inspect-core ...`), for the same reason. + --- ## Linting and Validation Workflows for Pull Requests diff --git a/default.nix b/default.nix index 6f6345a8f3..1b80c72e94 100644 --- a/default.nix +++ b/default.nix @@ -182,6 +182,7 @@ let qemu-user rust-toolchain shellcheck + skim # the `just debug` picker skopeo wasmtime wget @@ -1014,15 +1015,17 @@ let 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}" + # The remapped prefix is relative, so a debugger resolves source against + # its working directory rather than an absolute path. Ship the tree at + # /src and start there. Referencing ${src} is also what keeps it in the + # image closure: with the remap no longer naming a store path, nothing + # else retains it. + ln -s "${src}" src mkdir -p tmp chmod 1777 tmp ''; config = { + WorkingDir = "/src"; Entrypoint = [ "/bin/gdb" "--directory=/lib/rustlib/etc" @@ -1056,15 +1059,17 @@ let 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}" + # The remapped prefix is relative, so a debugger resolves source against + # its working directory rather than an absolute path. Ship the tree at + # /src and start there. Referencing ${src} is also what keeps it in the + # image closure: with the remap no longer naming a store path, nothing + # else retains it. + ln -s "${src}" src mkdir -p tmp chmod 1777 tmp ''; config = { + WorkingDir = "/src"; Entrypoint = [ "/bin/bs" # Bind the published interface rather than container-local loopback. @@ -1111,15 +1116,17 @@ 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}" + # The remapped prefix is relative, so a debugger resolves source against + # its working directory rather than an absolute path. Ship the tree at + # /src and start there. Referencing ${src} is also what keeps it in the + # image closure: with the remap no longer naming a store path, nothing + # else retains it. + ln -s "${src}" src mkdir -p tmp chmod 1777 tmp ''; config = { + WorkingDir = "/src"; Entrypoint = [ "/bin/lurk" "--json" @@ -1146,6 +1153,7 @@ let # pkgs.wireshark-cli pkgs.bashInteractive + pkgs.bugstalker pkgs.coreutils pkgs.curl pkgs.debianutils diff --git a/justfile b/justfile index fb7ad50d2f..720fdec7ef 100644 --- a/justfile +++ b/justfile @@ -258,6 +258,234 @@ setup-roots *args: {{ args }} done +# Ports the debug helpers listen on. Override for a second session. +gdb_port := "2345" + +bs_port := "4711" + +# Workspace binaries the debug images already carry, spelled as the images +# spell them. The nix attribute is keyed by directory (`workspace.init`) but +# the binary it installs is `dataplane-init`, and it is the binary name that has +# to appear here -- `debug` passes it straight through as `/bin/`. +[private] +_debug_binaries := "dataplane cli dataplane-init" + +# Build settings to forward when a recipe has to re-enter `just` rather than +# depend on it. A debug session is only useful against the same build the +# problem showed up in. +[private] +_forward := "jobs=" + jobs + " cores=" + cores + " debug_justfile=" + debug_justfile \ + + " profile=" + profile + " platform=" + platform + " libc=" + libc \ + + " features=" + features + " default_features=" + default_features \ + + " instrument=" + instrument + " sanitize=" + sanitize + " nightly=" + nightly + +# List what `just debug` can run at the current profile: the workspace binaries +# the images carry, and every test in a nextest archive. +[script] +debug-list package="tests.all": (build (if package == "tests.all" { "tests.all" } else { "tests.pkg." + package })) + {{ _just_debuggable_ }} + declare -r suite="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}" + echo "binaries:" + for b in {{ _debug_binaries }}; do printf ' %s\n' "${b}"; done + echo "tests:" + cargo nextest list --archive-file results/"${suite}"/*.tar.zst --workspace-remap "$(pwd)" + +# Run a workspace binary or a single test under a debug helper, in the image +# that carries it, built at the current profile and instrumentation. +# +# just debug # pick from a list +# just debug bugstalker # pick, then wait for an editor +# just debug lurk dataplane # trace syscalls, streams JSON +# just debug gdb dataplane # gdbserver, waits for gdb +# just profile=fuzz debug gdb test_parse_interface args +# +# `target` is one of the workspace binaries, or a nextest filter. Leave it out +# and everything is offered; give a filter matching one test and it is used +# without asking; give one matching several and those are offered. `package` +# narrows which archive is built to find them, so naming one is much quicker +# than the default of building every test in the workspace. gdb and bugstalker +# wait for a client instead of running to completion. +# +# The remapped source prefix is relative, so a client shows source only if its +# working directory is the root of the tree the binary was built from. This +# passes `-w` for that in every case; a local `gdb` needs you to be standing in +# the right place yourself. +[script] +debug tool="gdb" target="" package="tests.all" *args: (build-container (if tool == "gdb" { "dataplane-core-viewer" } else if tool == "bugstalker" { "dataplane-dev-debugger" } else if tool == "lurk" { "dataplane-syscall-tracer" } else { error("debug: unknown tool '" + tool + "'; expected gdb, bugstalker, or lurk") })) + {{ _just_debuggable_ }} + declare -xr DOCKER_HOST="${DOCKER_HOST:-unix://{{ docker_sock }}}" + + declare image + case "{{ tool }}" in + gdb) image="{{ oci_image_dataplane_core_viewer }}" ;; + bugstalker) image="{{ oci_image_dataplane_dev_debugger }}" ;; + lurk) image="{{ oci_image_dataplane_syscall_tracer }}" ;; + *) + # Unreachable: the dependency above rejects anything else before + # this body runs. Kept so `set -u` cannot meet an unset `image`. + >&2 echo "debug: unknown tool '{{ tool }}'" + exit 1 + ;; + esac + declare -r image + + # Resolve the target to a program, its arguments, and a working directory. + declare program workdir + declare -a program_args=() mounts=() + if [[ -n '{{ target }}' && " {{ _debug_binaries }} " == *" {{ target }} "* ]]; then + # The images carry these already, so nothing needs mounting. `/src` is + # where they ship the sources, and the remapped prefix is relative, so + # the debugger only resolves source if it starts there. + program="/bin/{{ target }}" + workdir="/src" + else + # A test, or nothing yet. Either way the archive has to exist before + # there is anything to name or to choose between. + declare -r suite="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}" + just {{ _forward }} build "${suite}" + declare -r extract="${PWD}/results/debug-extract" + rm -rf -- "${extract}" + mkdir -p -- "${extract}" + declare errors candidates + errors="$(mktemp)" + candidates="$(mktemp)" + declare -r errors candidates + trap 'rm -f -- "${errors}" "${candidates}"' EXIT + # Keep nextest's diagnostics: it writes progress to stderr and JSON to + # stdout, and discarding the former turns "cargo is not on PATH" into + # an empty result that looks like "no such test". + declare listing + if ! listing="$(cargo nextest list --archive-file results/"${suite}"/*.tar.zst \ + --workspace-remap "$(pwd)" --extract-to "${extract}" --extract-overwrite \ + --message-format json \ + {{ if target == "" { "" } else { "-E 'test(/" + target + "/)'" } }} \ + 2>"${errors}")"; then + >&2 echo "::error::could not list the tests in ${suite}" + >&2 cat -- "${errors}" + exit 1 + fi + declare -r listing + + # label \t binary \t workdir \t test, so a picker can show the label + # and the caller can read the rest back off the same line. + if [ -z '{{ target }}' ]; then + for b in {{ _debug_binaries }}; do + printf '%s (binary)\t/bin/%s\t/\t\n' "${b}" "${b}" + done >>"${candidates}" + fi + jq -r ' + ."rust-suites" | to_entries[] | .value as $s + | ($s.testcases // {} | to_entries[] + | select(."value"."filter-match"."status" == "matches") | .key) as $t + | "\($s."binary-id") \($t)\t\($s."binary-path")\t\($s.cwd)\t\($t)" + ' <<<"${listing}" >>"${candidates}" + + declare -i found + found="$(wc -l <"${candidates}")" + declare chosen + if [ "${found}" -eq 0 ]; then + >&2 echo "::error::nothing in ${suite} matches '{{ target }}'" + exit 1 + elif [ "${found}" -eq 1 ]; then + chosen="$(cat -- "${candidates}")" + elif [ -t 0 ]; then + # Several matches and someone to ask. Showing only the label keeps + # the store paths out of the list without losing them. + if ! command -v sk >/dev/null 2>&1; then + >&2 echo "::error::sk (skim) is not on PATH; use the dev shell, or name a target exactly" + exit 1 + fi + chosen="$(sk --delimiter '\t' --with-nth 1 \ + --prompt "{{ tool }} > " --height 40% --reverse <"${candidates}")" + if [ -z "${chosen}" ]; then + >&2 echo "debug: nothing picked" + exit 1 + fi + else + if [ -n '{{ target }}' ]; then + >&2 echo "::error::'{{ target }}' matches ${found} in ${suite}; name one exactly, or pick from a terminal:" + else + >&2 echo "::error::not a terminal, so nothing to ask; name one of:" + fi + >&2 cut -f1 -- "${candidates}" + exit 1 + fi + declare -r chosen + + program="$(cut -f2 <<<"${chosen}")" + # nextest gives a test its package root as the working directory, and + # anything reading a relative path depends on that. + workdir="$(cut -f3 <<<"${chosen}")" + declare test_name + test_name="$(cut -f4 <<<"${chosen}")" + declare -r test_name + if [ -n "${test_name}" ]; then + program_args=(--exact "${test_name}" --nocapture) + # Built outside the image, so it still resolves its loader and its + # libraries through the store; the store has to come along. + mounts=(-v /nix/store:/nix/store:ro -v "${extract}":"${extract}":ro -v "${PWD}":"${PWD}":ro) + fi + fi + declare -r program workdir + + case "{{ tool }}" in + lurk) + docker run --rm -i "${mounts[@]}" -w "${workdir}" --entrypoint /bin/lurk \ + "${image}" --json --follow-forks "${program}" "${program_args[@]}" {{ args }} + ;; + gdb) + >&2 echo "gdbserver on {{ gdb_port }}. Connect with:" + >&2 echo " gdb -ex 'target remote 127.0.0.1:{{ gdb_port }}' '${program}'" + # Randomization stays on: docker's default seccomp answers + # personality(ADDR_NO_RANDOMIZE) with EPERM, and gdbserver would + # otherwise open with an alarming warning about a benign failure. + # Loopback, not `0.0.0.0`: gdbserver runs whatever a client asks + # it to, with no authentication, and a bare `-p` would offer that + # to anything that can reach this machine. + docker run --rm -i -p "127.0.0.1:{{ gdb_port }}:{{ gdb_port }}" "${mounts[@]}" -w "${workdir}" \ + --entrypoint /bin/gdbserver "${image}" --no-disable-randomization \ + ":{{ gdb_port }}" "${program}" "${program_args[@]}" {{ args }} + ;; + bugstalker) + # It takes the program from the client's launch request, not from + # here, so connecting an editor to this is only half of it. Print + # the other half ready to paste into .zed/debug.json: `tcp_connection` + # is what stops the editor spawning a `bs` of its own. + >&2 echo "bugstalker DAP on {{ bs_port }}. Give the editor:" + >&2 jq -n --arg l "container: {{ tool }} {{ target }}" --arg p "${program}" \ + --arg w "${workdir}" --argjson port "{{ bs_port }}" \ + '{label: $l, adapter: "bugstalker-dap", request: "launch", + program: $p, args: $ARGS.positional, cwd: $w, + tcp_connection: {host: "127.0.0.1", port: $port}}' \ + --args "${program_args[@]}" {{ args }} + # The container end is 4711 whatever `bs_port` says: the image's + # entrypoint hardcodes `--dap-remote=0.0.0.0:4711`, so publishing + # `bs_port:bs_port` only worked while `bs_port` was 4711. Loopback + # for the same reason as gdbserver -- a DAP `launch` request runs + # an arbitrary program. + docker run --rm -i -p "127.0.0.1:{{ bs_port }}:4711" "${mounts[@]}" -w "${workdir}" "${image}" + ;; + esac + +# Open a core file in a gdb matched to the build that produced it. +# +# Symbols only line up with the exact version and profile that dumped, so pass +# the same build settings that produced the binary. +[script] +inspect-core core *args: (build-container "dataplane-core-viewer") + {{ _just_debuggable_ }} + declare -xr DOCKER_HOST="${DOCKER_HOST:-unix://{{ docker_sock }}}" + if [ ! -r '{{ core }}' ]; then + >&2 echo "inspect-core: cannot read '{{ core }}'" + exit 1 + fi + declare core_dir core_file + core_dir="$(cd "$(dirname -- '{{ core }}')" && pwd)" + core_file="$(basename -- '{{ core }}')" + declare -r core_dir core_file + docker run --rm -it -v "${core_dir}":/cores:ro \ + "{{ oci_image_dataplane_core_viewer }}" "/cores/${core_file}" {{ args }} + # 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. From f97fb608aab1ccc79328bcf5f03db9c0ca1ccd87 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 7 Aug 2026 00:44:16 -0600 Subject: [PATCH 8/8] fix(net): Store the checksum an incremental update computes `Checksum::increment_update_checksum` works out the new checksum and hands it back. It does not store it, despite taking `&mut self`. `EmbeddedTransport::update_checksum` called it and dropped the answer on the floor, under a comment about ignoring errors on a truncated header -- but there is no error to ignore; the return value is the checksum. So no quoted transport header has ever had its checksum updated. NAT rewrites the ports of the packet quoted inside an ICMP error and calls this to keep the checksum in step, and the call has been doing nothing. The only correct uses of the trait method in the tree are in a test, which is presumably how the shape survived. Setting it can fail, on a header too truncated to hold a checksum -- but that is a header the caller could not have read a checksum out of either, and every caller reads one first. Hence discarding that error rather than the value, which is what the comment was reaching for. update_checksum_for_address is new, and folds a change of one of the quoted packet's addresses in. TCP, UDP and ICMPv6 are checksummed over a pseudo-header built from the source and destination addresses, so rewriting one leaves the quoted checksum describing an address that is no longer there. ICMPv4 has no pseudo-header and is left alone. Incremental for the same reason as the rest: a quote is usually truncated, so there is no payload to compute over from scratch. The tests build the one case where both routes are open -- a full header over a known payload -- and hold the fold against a computation from scratch, over two words for v4 and eight for v6. The checksum starts out correct on purpose: an RFC 1624 update is exact given a correct starting value and says nothing given a wrong one. Both fail against the discarded update; the ICMPv4 one passes either way, which is the point of it. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- net/src/headers/embedded.rs | 171 ++++++++++++++++++++++++++++++++++-- 1 file changed, 163 insertions(+), 8 deletions(-) diff --git a/net/src/headers/embedded.rs b/net/src/headers/embedded.rs index 8017c2231f..c98128d78b 100644 --- a/net/src/headers/embedded.rs +++ b/net/src/headers/embedded.rs @@ -22,6 +22,7 @@ use crate::udp::{TruncatedUdp, UdpChecksum, UdpPort}; use arrayvec::ArrayVec; use core::fmt::Debug; use derive_builder::Builder; +use std::net::IpAddr; use std::num::NonZero; #[cfg(any(test, feature = "bolero"))] @@ -609,42 +610,87 @@ impl EmbeddedTransport { } } + /// Fold the change of a 16-bit field into the header's checksum. + /// + /// `increment_update_checksum` computes the new checksum and hands it back; it does not store + /// it, despite taking `&mut self`. Setting it is the point of this method, and it is what makes + /// a translated quote's checksum agree with the bytes around it. + /// + /// Setting fails only on a header too truncated to hold a checksum, which is a header the + /// caller could not have read one out of either, so there is nothing to do about it here. pub fn update_checksum(&mut self, current_checksum: u16, old_value: u16, new_value: u16) { match self { EmbeddedTransport::Tcp(tcp) => { - // Silently ignore errors if transport header is truncated - let _ = tcp.increment_update_checksum( + let updated = tcp.increment_update_checksum( TcpChecksum::new(current_checksum), old_value, new_value, ); + let _ = tcp.set_checksum(updated); } EmbeddedTransport::Udp(udp) => { - // Silently ignore errors if transport header is truncated - let _ = udp.increment_update_checksum( + let updated = udp.increment_update_checksum( UdpChecksum::new(current_checksum), old_value, new_value, ); + let _ = udp.set_checksum(updated); } EmbeddedTransport::Icmp4(icmp) => { - // Silently ignore errors if transport header is truncated - let _ = icmp.increment_update_checksum( + let updated = icmp.increment_update_checksum( Icmp4Checksum::new(current_checksum), old_value, new_value, ); + let _ = icmp.set_checksum(updated); } EmbeddedTransport::Icmp6(icmp) => { - // Silently ignore errors if transport header is truncated - let _ = icmp.increment_update_checksum( + let updated = icmp.increment_update_checksum( Icmp6Checksum::new(current_checksum), old_value, new_value, ); + let _ = icmp.set_checksum(updated); } } } + + /// Fold a change to one of the quoted packet's IP addresses into this header's checksum. + /// + /// TCP, UDP and `ICMPv6` are checksummed over a pseudo-header built from the source and + /// destination addresses, so rewriting either address of a quoted packet leaves the quoted + /// transport checksum describing an address that is no longer there. `ICMPv4` has no + /// pseudo-header and is left alone. + /// + /// Incremental, because a quote is usually truncated: there is no payload to compute a + /// checksum over from scratch, only a delta to apply to the one already there. + pub fn update_checksum_for_address(&mut self, old: IpAddr, new: IpAddr) { + if matches!(self, EmbeddedTransport::Icmp4(_)) { + return; + } + // Translating between address families would change the shape of the pseudo-header rather + // than a value inside it, so there would be no delta to fold. + if old.is_ipv4() != new.is_ipv4() { + return; + } + for (old_word, new_word) in address_words(old).into_iter().zip(address_words(new)) { + let Some(current) = self.checksum() else { + return; + }; + self.update_checksum(current, old_word, new_word); + } + } +} + +// The 16-bit words of an address, in the order a checksum sums them. +fn address_words(addr: IpAddr) -> ArrayVec { + match addr { + IpAddr::V4(addr) => { + let [a, b, c, d] = addr.octets(); + ArrayVec::from_iter([u16::from_be_bytes([a, b]), u16::from_be_bytes([c, d])]) + } + IpAddr::V6(addr) => ArrayVec::from(addr.segments()), + } } impl DeParse for EmbeddedTransport { @@ -1378,6 +1424,115 @@ mod tests { // Before calling check_full_payload, should be false assert!(!headers.is_full_payload()); } + + // Checksum folding for a rewritten address in a quoted packet. + // + // A quote is usually truncated, so folding a delta into the checksum already there is the only + // option: there is no payload to compute one over. These build the one case where both routes + // are open -- a full header over a known payload -- and hold the fold against a computation + // from scratch. + // + // The checksum starts out correct on purpose. An RFC 1624 update is exact given a correct + // starting value and says nothing at all given a wrong one, so only a correct start tests + // anything. + mod address_folding { + use super::*; + use crate::icmp4::{Icmp4, Icmp4EchoRequest, Icmp4Type}; + use crate::ip::NextHeader; + use crate::ipv4::UnicastIpv4Addr; + use crate::ipv6::UnicastIpv6Addr; + use crate::tcp::{Tcp, TcpChecksumPayload}; + use crate::udp::{UdpChecksumPayload, UdpPort}; + use std::net::{Ipv4Addr, Ipv6Addr}; + + const PAYLOAD: [u8; 4] = [0xde, 0xad, 0xbe, 0xef]; + + fn v4_net(source: Ipv4Addr, next_header: NextHeader) -> Net { + let mut ip = Ipv4::default(); + ip.set_source(UnicastIpv4Addr::new(source).unwrap_or_else(|_| unreachable!())) + .set_destination(Ipv4Addr::new(192, 168, 1, 2)) + .set_next_header(next_header); + Net::Ipv4(ip) + } + + fn v6_net(source: Ipv6Addr, next_header: NextHeader) -> Net { + let mut ip = Ipv6::default(); + ip.set_source(UnicastIpv6Addr::new(source).unwrap_or_else(|_| unreachable!())) + .set_destination(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)) + .set_next_header(next_header); + Net::Ipv6(ip) + } + + /// Two words to fold, over a TCP pseudo-header. + #[test] + fn a_v4_address_change_matches_a_fresh_tcp_checksum() { + let (old, new) = (Ipv4Addr::new(192, 168, 1, 1), Ipv4Addr::new(10, 11, 12, 13)); + + let mut tcp = Tcp::new(123.try_into().unwrap(), 456.try_into().unwrap()); + tcp.update_checksum(&TcpChecksumPayload::new( + &v4_net(old, NextHeader::TCP), + &PAYLOAD, + )) + .unwrap_or_else(|()| unreachable!()); + + let mut quoted = EmbeddedTransport::Tcp(TruncatedTcp::FullHeader(tcp.clone())); + quoted.update_checksum_for_address(IpAddr::V4(old), IpAddr::V4(new)); + + let expected = tcp + .compute_checksum(&TcpChecksumPayload::new( + &v4_net(new, NextHeader::TCP), + &PAYLOAD, + )) + .unwrap_or_else(|()| unreachable!()); + assert_eq!(quoted.checksum(), Some(u16::from(expected))); + } + + /// Eight words to fold, over a UDP pseudo-header. + #[test] + fn a_v6_address_change_matches_a_fresh_udp_checksum() { + let old = Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1); + let new = Ipv6Addr::new(0x2001, 0xdb8, 0xdead, 0xbeef, 1, 2, 3, 4); + + let mut udp = Udp::new( + UdpPort::new_checked(123).unwrap(), + UdpPort::new_checked(456).unwrap(), + ); + udp.update_checksum(&UdpChecksumPayload::new( + &v6_net(old, NextHeader::UDP), + &PAYLOAD, + )) + .unwrap_or_else(|()| unreachable!()); + + let mut quoted = EmbeddedTransport::Udp(TruncatedUdp::FullHeader(udp.clone())); + quoted.update_checksum_for_address(IpAddr::V6(old), IpAddr::V6(new)); + + let expected = udp + .compute_checksum(&UdpChecksumPayload::new( + &v6_net(new, NextHeader::UDP), + &PAYLOAD, + )) + .unwrap_or_else(|()| unreachable!()); + assert_eq!(quoted.checksum(), Some(u16::from(expected))); + } + + /// `ICMPv4` is checksummed over itself alone, so the addresses around it are none of its + /// business and folding one must not disturb it. + #[test] + fn a_v4_address_change_leaves_an_icmpv4_quote_alone() { + let mut icmp = + Icmp4::with_type(Icmp4Type::EchoRequest(Icmp4EchoRequest { id: 18, seq: 2 })); + icmp.update_checksum(&PAYLOAD) + .unwrap_or_else(|()| unreachable!()); + + let mut quoted = EmbeddedTransport::Icmp4(TruncatedIcmp4::FullHeader(icmp)); + let before = quoted.checksum(); + quoted.update_checksum_for_address( + IpAddr::V4(Ipv4Addr::new(192, 168, 1, 1)), + IpAddr::V4(Ipv4Addr::new(10, 11, 12, 13)), + ); + assert_eq!(quoted.checksum(), before); + } + } } #[cfg(any(test, feature = "bolero"))]