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/actions/nix-shell/action.yml b/.github/actions/nix-shell/action.yml index 2336aabfca..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 }}" @@ -32,6 +83,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/.github/workflows/README.md b/.github/workflows/README.md index d41d5b6a72..867c427fce 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 @@ -68,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. @@ -94,6 +99,8 @@ If those queue failures stop being rare, the phasing is worth revisiting. - 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 +- 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 @@ -101,6 +108,76 @@ 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 + ``` + +- `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 = {}, + }, + } + ``` + +- `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 86fe9fb473..468a50e5fd 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -71,6 +71,8 @@ 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 }}" cross: "${{ steps.cross.outputs.value }}" @@ -85,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: @@ -122,6 +145,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 @@ -183,9 +216,18 @@ 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) }}" + 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 @@ -219,6 +261,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: | @@ -319,6 +370,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 @@ -337,6 +398,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 @@ -365,12 +427,9 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: - nix-target: - - frr.dataplane - - dataplane - - 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: @@ -378,6 +437,12 @@ jobs: profile: fuzz - nix-target: dataplane profile: fuzz + - nix-target: dataplane-core-viewer + profile: fuzz + - nix-target: dataplane-dev-debugger + profile: fuzz + - nix-target: dataplane-syscall-tracer + profile: fuzz - nix-target: validator profile: debug - nix-target: validator @@ -391,6 +456,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 @@ -405,7 +481,7 @@ jobs: env: *ci-env strategy: fail-fast: false - max-parallel: 1 + max-parallel: ${{ fromJSON(needs.plan.outputs.parallel) }} matrix: sanitizer: - thread @@ -449,7 +525,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) }}" @@ -639,6 +715,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: @@ -706,6 +786,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. @@ -713,7 +794,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') 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 4696b6cc9f..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 + " docker_sock=/run/docker/docker.sock" + " oci_repo=ghcr.io" + " debug_justfile=" + debug_justfile +_lab := "jobs=" + jobs + " cores=" + cores + _lab-common + +[private] +_lab-test-each := "jobs=" + test_each_jobs + " cores=" + test_each_cores + _lab-common [default] [private] @@ -47,11 +71,14 @@ 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 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 @@ -76,6 +103,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/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 acadbf858e..664ad7a35a 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; @@ -216,7 +220,18 @@ 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"); + # `.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 + # binary cache. + outputsFilter = + p: _type: + (p != "target") && (p != "sysroot") && (p != "devroot") && (p != "results") && (p != ".git"); src = pkgs.lib.cleanSourceWith { filter = full-path: t: @@ -227,16 +242,57 @@ 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"; }; + # 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 "${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; + }; # 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 +311,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 +327,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; @@ -307,6 +363,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 = @@ -316,6 +373,12 @@ 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, + # Skip the binary strip/split step for derivations that produce none. + no-bins ? false, profile, cargo-nextest, hwloc, @@ -350,7 +413,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"; @@ -371,14 +441,16 @@ 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" ] ) else @@ -387,7 +459,16 @@ let } // args )).overrideAttrs - (orig: { + ( + orig: + if for-deps || no-bins 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. @@ -407,6 +488,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 @@ -421,14 +504,94 @@ 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 + '' + ); + + } + ); + + # 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 + ++ timings-args + ); + }; + }; + + 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; @@ -444,6 +607,7 @@ let "--profile=${cargo-profile}" ] ++ cargo-cmd-prefix + ++ timings-args ++ [ "--message-format json-render-diagnostics > $cargoBuildLog" ] @@ -461,7 +625,7 @@ let workspace-check = { pname ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts, }: pkgs.callPackage invoke { builder = craneLib.buildPackage; @@ -477,6 +641,7 @@ let "--profile=${cargo-profile}" ] ++ cargo-cmd-prefix + ++ timings-args ++ [ "--message-format json-render-diagnostics > $cargoBuildLog" ] @@ -494,7 +659,7 @@ let test-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -517,11 +682,12 @@ let ] ++ (if package != null then [ "--package=${pname}" ] else [ ]) ++ cargo-cmd-prefix-tests + ++ timings-args )) # 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 "" ); @@ -542,7 +708,7 @@ let bench-builder = { package ? null, - cargoArtifacts ? null, + cargoArtifacts ? cargo-artifacts-tests, }: let pname = if package != null then package else "all"; @@ -564,6 +730,7 @@ let ] ++ (if package != null then [ "--package=${pname}" ] else [ ]) ++ cargo-cmd-prefix-tests + ++ timings-args ++ [ "--message-format=json-render-diagnostics > $cargoBenchLog;" ] )) + '' @@ -576,24 +743,44 @@ 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, + package ? null, }: + let + pname = if package != null then package else "all"; + in pkgs.callPackage invoke { builder = craneLib.mkCargoDerivation; - profile = profile'; + profile = profile-tests'; args = { inherit pname; - cargoArtifacts = null; + cargoArtifacts = cargo-artifacts-tests; buildPhaseCargoCommand = builtins.concatStringsSep " " ( [ "cargo" "clippy" + "--all-targets" "--profile=${cargo-profile}" - "--package=${pname}" ] - ++ cargo-cmd-prefix + # 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 ++ [ "--" "-D warnings" @@ -602,12 +789,58 @@ 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 + # 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 = { @@ -621,8 +854,11 @@ let profile = profile'; args = { inherit pname; - cargoArtifacts = null; - RUSTDOCFLAGS = "-D warnings"; + cargoArtifacts = cargo-artifacts; + # `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" @@ -632,6 +868,7 @@ let ] ++ (if package != null then [ "--package=${pname}" ] else [ ]) ++ cargo-cmd-prefix + ++ timings-args ); }; }; @@ -646,7 +883,7 @@ let ) package-list; }; - dataplane.tar = pkgs.stdenv'.mkDerivation { + dataplane.tar = (pkgs.stdenv'.mkDerivation { pname = "dataplane.tar"; inherit version; dontUnpack = true; @@ -766,12 +1003,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" @@ -787,15 +1024,36 @@ 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 = [ + pkgs.pkgsBuildHost.coreutils + pkgs.pkgsBuildHost.bashInteractive + pkgs.pkgsHostHost.dockerTools.usrBinEnv - containers.dataplane-debugger = pkgs.dockerTools.buildLayeredImage { - name = "ghcr.io/githedgehog/dataplane/debugger"; + 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"; + contents = (pkgs.buildEnv { + name = "dataplane-core-viewer-env"; pathsToLink = [ "/bin" "/etc" @@ -804,20 +1062,120 @@ let ]; paths = [ pkgs.pkgsBuildHost.gdb - pkgs.pkgsBuildHost.rr - pkgs.pkgsBuildHost.coreutils - pkgs.pkgsBuildHost.bashInteractive - pkgs.pkgsBuildHost.iproute2 - pkgs.pkgsBuildHost.ethtool - pkgs.pkgsHostHost.dockerTools.usrBinEnv + 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; - pkgs.pkgsHostHost.libc.debug - workspace.cli.debug - workspace.dataplane.debug - workspace.init.debug + # 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; + + # 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: @@ -899,7 +1257,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 { @@ -953,7 +1311,7 @@ let "--" ]; config.Cmd = [ "/libexec/frr/docker-start" ]; - }; + }).overrideAttrs source-volatile; containers.frr.host = pkgs.dockerTools.buildLayeredImage { name = "ghcr.io/githedgehog/dataplane/frr-host"; @@ -1020,6 +1378,7 @@ in dataplane devenv devroot + doctests docs package-list pkgs diff --git a/justfile b/justfile index eace8d7db0..7b710f6401 100644 --- a/justfile +++ b/justfile @@ -24,6 +24,27 @@ 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!()` +# 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] @@ -140,7 +161,9 @@ 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_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 @@ -171,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}" \ @@ -192,7 +216,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 }} @@ -245,7 +269,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 @@ -274,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 }}' \ @@ -282,6 +307,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) @@ -305,10 +398,20 @@ 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 }}" + ;; + "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 }}" + ;; + "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.) @@ -355,26 +458,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") + push_image "{{ oci_image_dataplane_core_viewer }}" ;; - "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-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 ;; "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 @@ -382,7 +541,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 }}" ;; @@ -390,12 +550,17 @@ push-container target="dataplane" *args: (build-container target args) && versio >&2 echo "{{target}} is not a valid container" exit 99 esac - -# Push release images with the resolved core budget. [script] push: {{ _just_debuggable_ }} - for container in dataplane 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 @@ -415,6 +580,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_ }} @@ -430,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.all" } else { "clippy.pkg." + package }) args) {{ _just_debuggable_ }} - cargo clippy --all-targets {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} -- -D warnings [script] actionlint: @@ -497,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. @@ -520,7 +742,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 \ @@ -617,15 +839,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}" @@ -759,6 +986,7 @@ shell: --argstr kernel '{{ kernel }}' \ --argstr libc '{{ libc }}' \ --argstr nightly '{{nightly}}' \ + --argstr timings '{{timings}}' \ --argstr platform '{{ platform }}' \ --argstr profile '{{ profile }}' \ --argstr sanitize '{{ sanitize }}' \ 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 diff --git a/nix/overlays/dataplane-dev.nix b/nix/overlays/dataplane-dev.nix index c8829018f2..345466c1d8 100644 --- a/nix/overlays/dataplane-dev.nix +++ b/nix/overlays/dataplane-dev.nix @@ -21,6 +21,42 @@ 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="; + }; + }); + # 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; }; 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" 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/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": { 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; 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())