Merge pull request #573 from noonghunna/feat/p2p-engagement-verdict

Interconnect verdict: warn when P2P hardware sits idle (#488-class triage)
2026-07-05 05:12:08 +05:00
parent e8a8e7886e b0c8933800
commit 0f41e739ed
5 changed files with 225 additions and 2 deletions
--- a/docs/PCIE_P2P.md
+++ b/docs/PCIE_P2P.md
@@ -66,7 +66,7 @@ A 3-slot (triple-width) card like most 3090s covers its own slot **plus the two

 Two hard truths set expectations before you start:

-1. **The stock NVIDIA driver refuses P2P on GeForce cards over `PHB`.** Even with perfect topology and BIOS, the consumer driver disables peer access. Enabling it requires a **patched kernel module** — the community [`aikitoria/open-gpu-kernel-modules`](https://github.com/aikitoria/open-gpu-kernel-modules) fork ([Sam McLeod's walkthrough](https://smcleod.net/2026/02/patching-nvidias-driver-and-vllm-to-enable-p2p-on-consumer-gpus/)). This is a custom DKMS module — weigh the maintenance cost.
+1. **The stock NVIDIA driver refuses P2P on GeForce cards over `PHB`.** Even with perfect topology and BIOS, the consumer driver disables peer access. Enabling it requires a **patched kernel module** — the community [`aikitoria/open-gpu-kernel-modules`](https://github.com/aikitoria/open-gpu-kernel-modules) fork ([Sam McLeod's walkthrough](https://smcleod.net/2026/02/patching-nvidias-driver-and-vllm-to-enable-p2p-on-consumer-gpus/)). This is a custom DKMS module — weigh the maintenance cost. (Should the walkthrough link ever rot, the shape of it: clone the fork matching your driver branch → build + install via DKMS in place of the stock `nvidia` kernel module → reboot → `nvidia-smi topo -p2p r` should now report `OK` between your GPUs.)
 2. **`PHB` P2P is PCIe-bounded** (~25 GB/s on PCIe 4.0 x16), well under NVLink. So the win is real but modest and workload-shaped (§6).

 **On this stack**, once the patched module is installed you don't edit composes — set one env var:
@@ -104,7 +104,7 @@ Capability (`topo -m` / `topo -p2p`) tells you it *can* — it doesn't tell you
 bash scripts/report.sh
 ```

-Read the **"Interconnect / P2P engagement"** field under *Boot log highlights* — it surfaces the `[nvlink]` boot line and the resolved `NCCL_P2P_LEVEL` + custom-all-reduce state for the running container, so you see e.g. `P2P ENABLED — NCCL_P2P_LEVEL=PHB, custom all-reduce ON` rather than guessing. (This is exactly the round-trip the field was added to avoid — [#446](https://github.com/noonghunna/club-3090/issues/446), [#488](https://github.com/noonghunna/club-3090/issues/488).)
+Read the **"Interconnect verdict"** line under *Boot log highlights* — the report cross-references host capability against the running container's engagement automatically: `✓ engaged`, `⚠ WARN` (NVLink bridge present but idle), or `ℹ` (P2P-capable driver, container not using it), each naming the fix. The raw evidence sits directly above it: the `[nvlink]` boot line plus the resolved `NCCL_P2P_LEVEL` + custom-all-reduce env. On rigs with no P2P capability the verdict line is deliberately absent — silence means "nothing to gain here", not "check failed". (This is exactly the round-trip the field was added to avoid — [#446](https://github.com/noonghunna/club-3090/issues/446), [#488](https://github.com/noonghunna/club-3090/issues/488).)

 ---

--- a/scripts/lib/p2p-state.sh
+++ b/scripts/lib/p2p-state.sh
@@ -0,0 +1,101 @@
+#!/usr/bin/env bash
+# p2p-state.sh — interconnect capability × engagement VERDICT (the #488/#158
+# triage matrix). Read-only AUDITOR; the boot-time DECIDER is
+# scripts/detect_nvlink.sh. The capability probes here mirror the decider's
+# semantics on purpose — test-p2p-state.sh runs BOTH against shared fixtures
+# so they cannot drift apart silently.
+#
+# Verdict matrix (report.sh renders it; preflight prints the capability line):
+#   <2 GPUs, or no capability          -> silent (nothing useful to say)
+#   capability + engagement ON         -> one OK line
+#   NVLink bridge + engagement OFF     -> WARN (hardware idle; ~15% decode
+#                                        left on the table per the #77 A/B)
+#   PCIe-P2P-capable + engagement OFF  -> INFO (launcher boots auto-enable;
+#                                        direct compose users can opt in)
+
+# GPU count (host).
+p2p_gpu_count() {
+  nvidia-smi -L 2>/dev/null | grep -c '^GPU ' || echo 0
+}
+
+# Host capability: "nvlink" | "pcie_p2p" | "none".
+# NVLink probe matches detect_nvlink.sh's auto path (topo -m, \bNV<n>\b).
+p2p_host_capability() {
+  local count="${1:-$(p2p_gpu_count)}"
+  if [[ "${count:-0}" -lt 2 ]]; then
+    echo none
+    return 0
+  fi
+  if nvidia-smi topo -m 2>/dev/null | grep -qP '\bNV[0-9]+\b'; then
+    echo nvlink
+    return 0
+  fi
+  if _p2p_pairs_ok; then
+    echo pcie_p2p
+    return 0
+  fi
+  echo none
+}
+
+# True when nvidia-smi reports working P2P between ALL GPU pairs. Parser
+# mirrors detect_nvlink.sh `_pcie_p2p_available` (stock GeForce drivers report
+# CNS; only a patched driver on a P2P-capable layout reports OK).
+_p2p_pairs_ok() {
+  nvidia-smi topo -p2p r 2>/dev/null | awk '
+    $1 ~ /^GPU[0-9]+$/ {
+      hasX = 0
+      for (i = 2; i <= NF; i++) if ($i == "X") hasX = 1
+      if (!hasX) next
+      rows++
+      for (i = 2; i <= NF; i++) if ($i != "X" && $i != "OK") bad = 1
+    }
+    END { exit (rows > 0 && !bad) ? 0 : 1 }
+  '
+}
+
+# Engagement classifier — PURE (takes the boot-trail/env text on stdin so the
+# caller decides where it comes from and tests can feed fixtures).
+# report.sh already gathers exactly this text: the container's `[nvlink]`
+# boot lines + resolved NCCL_P2P*/NVLINK_MODE env.
+# Prints: "on" | "off" | "unknown".
+p2p_classify_engagement() {
+  local text
+  text="$(cat)"
+  # The [nvlink] decision trail is authoritative (it states what the boot
+  # resolved, post-override); env is the fallback for pre-trail entrypoints.
+  case "$text" in
+    *"custom all-reduce ON"*|*"enabling NVLink mode"*) echo on; return 0 ;;
+  esac
+  case "$text" in
+    *"P2P off"*|*"using PCIe mode"*|*"forcing PCIe mode"*) echo off; return 0 ;;
+  esac
+  case "$text" in
+    *NCCL_P2P_DISABLE=1*) echo off; return 0 ;;
+    *NCCL_P2P_LEVEL=*)    echo on;  return 0 ;;
+  esac
+  echo unknown
+}
+
+# Pure verdict matrix: p2p_verdict <gpu_count> <capability> <engagement>.
+# Prints zero or one line; silent cases print nothing (exit 0 always).
+p2p_verdict() {
+  local count="$1" cap="$2" eng="$3" state
+  [[ "${count:-0}" -ge 2 ]] || return 0
+  [[ "$cap" != "none" ]] || return 0
+  case "$eng" in
+    on)      state="" ;;
+    off)     state="is running with P2P OFF" ;;
+    unknown) state="shows no P2P engagement signal (no [nvlink] boot line / NCCL env)" ;;
+    *)       return 0 ;;
+  esac
+  case "$cap:$eng" in
+    nvlink:on)
+      echo "✓ interconnect: NVLink engaged (custom all-reduce ON)" ;;
+    pcie_p2p:on)
+      echo "✓ interconnect: PCIe P2P engaged (patched driver, custom all-reduce ON)" ;;
+    nvlink:*)
+      echo "⚠ interconnect WARN: an NVLink bridge is present on this host but the serving container ${state} — the bridge is idle, leaving ~15% decode on the table (controlled A/B, BENCHMARKS #77). Boot via launch.sh/switch.sh (auto-detects) or set NVLINK_MODE=force_on; if auto-detect misses on your rig, please file it. Full guide: docs/PCIE_P2P.md" ;;
+    pcie_p2p:*)
+      echo "ℹ interconnect: this driver reports PCIe P2P available (patched driver / P2P-capable layout) but the serving container ${state}. Launcher boots auto-enable it; for direct docker compose set NVLINK_MODE=pcie_p2p (+10–22% code TPS measured, #91/#295). Full guide: docs/PCIE_P2P.md" ;;
+  esac
+}
--- a/scripts/preflight.sh
+++ b/scripts/preflight.sh
@@ -88,6 +88,25 @@ preflight_gpu() {
      echo "[preflight] arch:    ${_arch_class} (sm_${_cap}) — arch-aware KV defaults active for pilot slugs (#246)"
    fi
  fi
+  # Interconnect capability (display-only; the engagement VERDICT lives in
+  # report.sh — pre-boot there is no container to audit). Silent on
+  # single-GPU / stock-PCIe rigs so the line is always signal.
+  if [[ "$gpu_count" -ge 2 ]]; then
+    local _p2p_cap=""
+    # shellcheck source=lib/p2p-state.sh
+    source "$(dirname "${BASH_SOURCE[0]}")/lib/p2p-state.sh" 2>/dev/null && \
+      _p2p_cap="$(p2p_host_capability "$gpu_count")"
+    case "$_p2p_cap" in
+      nvlink)
+        if [[ "${NVLINK_MODE:-auto}" == "force_off" ]]; then
+          echo "[preflight] p2p:     NVLink bridge detected but NVLINK_MODE=force_off — the bridge will sit idle this boot (~15% decode, BENCHMARKS #77)"
+        else
+          echo "[preflight] p2p:     NVLink bridge detected — launcher auto-engages it (NVLINK_MODE=${NVLINK_MODE:-auto})"
+        fi ;;
+      pcie_p2p)
+        echo "[preflight] p2p:     driver reports PCIe P2P available — launcher auto-engages it (patched-driver path, NVLINK_MODE=${NVLINK_MODE:-auto}; docs/PCIE_P2P.md)" ;;
+    esac
+  fi
  # Cross-rig friendliness: surface a hint when 4090 / 5090 cards are
  # detected. Composes run cross-rig but per-class gotchas (ctx derate,
  # VRAM envelope, SM-gated kernels) live in the FAQ — easier to catch
--- a/scripts/report.sh
+++ b/scripts/report.sh
@@ -73,6 +73,8 @@ cd "$REPO_ROOT"

 # KV-calc calibration helpers (engine/model detection + per-model filter, #168).
 source "$REPO_ROOT/scripts/lib/report_calib.sh"
+# shellcheck source=lib/p2p-state.sh
+source "$REPO_ROOT/scripts/lib/p2p-state.sh"

 # Pick up a saved MODEL_DIR (and other config) from the repo .env — same as
 # launch.sh / switch.sh, and what setup.sh writes there. An explicit exported
@@ -716,6 +718,12 @@ else
    else
      echo "_No \`[nvlink]\` boot line or NCCL_P2P/NVLINK_MODE env found — P2P engagement undetermined (single-GPU, a non-NCCL engine like llama.cpp, or an entrypoint predating detect_nvlink.sh)._"
    fi
+    # Cross-referenced VERDICT (capability x engagement — the #488/#158 matrix).
+    # Silent on single-GPU / no-capability rigs so the OK/WARN/INFO line is
+    # always signal, never boilerplate.
+    _p2p_verdict_line="$(p2p_verdict "$(p2p_gpu_count)" "$(p2p_host_capability)" \
+      "$(printf '%s\n%s' "$nvlink_boot" "$p2p_env" | p2p_classify_engagement)")"
+    [[ -n "$_p2p_verdict_line" ]] && { echo; echo "**Interconnect verdict:** ${_p2p_verdict_line}"; }
    echo

    genesis_results=$(docker logs "$CONTAINER" 2>&1 | grep -E '\[INFO:genesis\.apply_all\] (Genesis|✅) Results' | tail -1)
--- a/scripts/tests/test-p2p-state.sh
+++ b/scripts/tests/test-p2p-state.sh
@@ -0,0 +1,95 @@
+#!/usr/bin/env bash
+# test-p2p-state — the interconnect verdict matrix (scripts/lib/p2p-state.sh)
+# + a consistency guard that runs the DECIDER (detect_nvlink.sh) and the
+# AUDITOR (p2p_host_capability) against the same faked nvidia-smi and asserts
+# they agree — the two parse the same probes and must not drift.
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+cd "$ROOT_DIR"
+
+TMP="$(mktemp -d)"
+trap 'rm -rf "$TMP"' EXIT
+
+fail() { echo "FAIL: $1" >&2; exit 1; }
+assert_contains() { [[ "$1" == *"$2"* ]] || { echo "FAIL: missing '$2' in: $1" >&2; exit 1; }; }
+assert_empty() { [[ -z "$1" ]] || { echo "FAIL: expected silence, got: $1" >&2; exit 1; }; }
+
+source scripts/lib/p2p-state.sh
+
+# ── 1. pure verdict matrix ────────────────────────────────────────────────────
+assert_empty "$(p2p_verdict 1 nvlink off)"                          # single GPU -> silent
+assert_empty "$(p2p_verdict 2 none off)"                            # stock PCIe -> silent
+assert_empty "$(p2p_verdict 2 none unknown)"
+out="$(p2p_verdict 2 nvlink on)";     assert_contains "$out" "✓ interconnect: NVLink engaged"
+out="$(p2p_verdict 2 pcie_p2p on)";   assert_contains "$out" "PCIe P2P engaged"
+out="$(p2p_verdict 2 nvlink off)";    assert_contains "$out" "⚠ interconnect WARN"
+assert_contains "$out" "NVLINK_MODE=force_on"
+out="$(p2p_verdict 2 nvlink unknown)"; assert_contains "$out" "⚠ interconnect WARN"
+out="$(p2p_verdict 2 pcie_p2p off)";  assert_contains "$out" "ℹ interconnect"
+assert_contains "$out" "NVLINK_MODE=pcie_p2p"
+out="$(p2p_verdict 4 nvlink off)";    assert_contains "$out" "WARN"   # multi-GPU too
+
+# ── 2. engagement classifier (pure, stdin fixtures) ───────────────────────────
+r="$(echo '[nvlink] detected NVLink (NV4) between GPU0-GPU1 — enabling NVLink mode' | p2p_classify_engagement)"
+[[ "$r" == "on" ]] || fail "nvlink boot line -> on (got $r)"
+r="$(echo '[nvlink] NVLINK_MODE=pcie_p2p — forcing PCIe P2P (NCCL_P2P_LEVEL=PHB, custom all-reduce ON)' | p2p_classify_engagement)"
+[[ "$r" == "on" ]] || fail "pcie_p2p boot line -> on (got $r)"
+r="$(echo '[nvlink] PCIe topology (PHB), P2P not available (topo -p2p: no OK) — using PCIe mode' | p2p_classify_engagement)"
+[[ "$r" == "off" ]] || fail "pcie-mode boot line -> off (got $r)"
+r="$(echo '[nvlink] NVLINK_MODE=force_off — forcing PCIe mode (P2P off)' | p2p_classify_engagement)"
+[[ "$r" == "off" ]] || fail "force_off boot line -> off (got $r)"
+r="$(echo 'NCCL_P2P_DISABLE=1' | p2p_classify_engagement)"
+[[ "$r" == "off" ]] || fail "env disable -> off (got $r)"
+r="$(echo 'NCCL_P2P_LEVEL=NVL' | p2p_classify_engagement)"
+[[ "$r" == "on" ]] || fail "env level -> on (got $r)"
+r="$(echo '' | p2p_classify_engagement)"
+[[ "$r" == "unknown" ]] || fail "empty -> unknown (got $r)"
+# boot trail beats env: a trail that resolved OFF wins over a leftover LEVEL var
+r="$(printf '%s\n%s' '[nvlink] NVLINK_MODE=force_off — forcing PCIe mode (P2P off)' 'NCCL_P2P_LEVEL=PHB' | p2p_classify_engagement)"
+[[ "$r" == "off" ]] || fail "trail-over-env precedence (got $r)"
+
+# ── 3. capability probes via faked nvidia-smi ────────────────────────────────
+mk_smi() { cat > "$TMP/nvidia-smi" <<EOF
+#!/usr/bin/env bash
+case "\$*" in
+  -L) printf '%b' "$1" ;;
+  "topo -m") printf '%b' "$2" ;;
+  "topo -p2p r") printf '%b' "$3" ;;
+esac
+EOF
+chmod +x "$TMP/nvidia-smi"; }
+
+L2='GPU 0: RTX 3090\nGPU 1: RTX 3090\n'
+TOPO_NV='\tGPU0\tGPU1\nGPU0\t X \tNV4\nGPU1\tNV4\t X \n'
+TOPO_PHB='\tGPU0\tGPU1\nGPU0\t X \tPHB\nGPU1\tPHB\t X \n'
+P2P_OK=' \tGPU0\tGPU1\nGPU0\tX\tOK\nGPU1\tOK\tX\n'
+P2P_CNS=' \tGPU0\tGPU1\nGPU0\tX\tCNS\nGPU1\tCNS\tX\n'
+
+mk_smi "$L2" "$TOPO_NV" "$P2P_CNS"
+r="$(PATH="$TMP:$PATH" bash -c 'source scripts/lib/p2p-state.sh; p2p_host_capability')"
+[[ "$r" == "nvlink" ]] || fail "NV topo -> nvlink (got $r)"
+
+mk_smi "$L2" "$TOPO_PHB" "$P2P_OK"
+r="$(PATH="$TMP:$PATH" bash -c 'source scripts/lib/p2p-state.sh; p2p_host_capability')"
+[[ "$r" == "pcie_p2p" ]] || fail "PHB + p2p OK -> pcie_p2p (got $r)"
+
+mk_smi "$L2" "$TOPO_PHB" "$P2P_CNS"
+r="$(PATH="$TMP:$PATH" bash -c 'source scripts/lib/p2p-state.sh; p2p_host_capability')"
+[[ "$r" == "none" ]] || fail "stock PCIe -> none (got $r)"
+
+mk_smi 'GPU 0: RTX 3090\n' "$TOPO_PHB" "$P2P_OK"
+r="$(PATH="$TMP:$PATH" bash -c 'source scripts/lib/p2p-state.sh; p2p_host_capability')"
+[[ "$r" == "none" ]] || fail "single GPU -> none even with p2p OK (got $r)"
+
+# ── 4. decider↔auditor consistency: detect_nvlink.sh on the same fixtures ────
+run_decider() { PATH="$TMP:$PATH" NVLINK_MODE=auto bash scripts/detect_nvlink.sh 2>/dev/null || true; }
+mk_smi "$L2" "$TOPO_NV" "$P2P_CNS"
+d="$(run_decider)"; assert_contains "$d" "enabling NVLink mode"       # decider: nvlink
+mk_smi "$L2" "$TOPO_PHB" "$P2P_OK"
+d="$(run_decider)"; assert_contains "$d" "P2P=OK"                     # decider: pcie_p2p
+mk_smi "$L2" "$TOPO_PHB" "$P2P_CNS"
+d="$(run_decider)"; assert_contains "$d" "using PCIe mode"            # decider: none
+echo "  ✓ decider (detect_nvlink.sh) and auditor (p2p-state.sh) agree on all 3 fixtures"
+
+echo "test-p2p-state: ok"