Issue 242 thor benchmark followups - #248
Conversation
scripts/build-container.sh assigned DAQIRI_OS_BASE_IMAGE unconditionally inside the BASE_IMAGE case, so an exported value was silently discarded and the CUDA base was effectively pinned to 13.1.0. A container whose CUDA runtime is newer than the host driver cannot initialize CUDA at all, which makes the script unusable on hosts with an older driver: on IGX Thor (driver 580.00 / CUDA 13.0) a 13.1 image fails with cuInit=803, "system has unsupported display driver / cuda driver combination". cuda-compat forward compatibility does not apply to Tegra, so there is no runtime workaround. Honor a pre-set DAQIRI_OS_BASE_IMAGE, and add CUDA_VERSION (default 13.1.0, so the default build is unchanged) and UBUNTU_VERSION shorthands for the cuda base. BASE_IMAGE keeps its cuda|torch selector semantics and torch is unaffected. Testing (IGX Thor, driver 580.00 / CUDA 13.0): - Resolution paths: default still yields cuda:13.1.0-devel-ubuntu24.04 unchanged; CUDA_VERSION=13.0.0 yields 13.0.0; an exported DAQIRI_OS_BASE_IMAGE overrides both; BASE_IMAGE=torch unaffected; an invalid BASE_IMAGE still exits 1. - Built --target base on cuda:13.0.0-devel-ubuntu24.04 and compared CUDA driver init against the host driver: 13.0 gives cuInit=0 and enumerates the discrete RTX PRO 6000; the 13.1 default gives cuInit=803 (unsupported display driver / cuda driver combination), count=-1. Assisted-by: Claude Signed-off-by: Ramya Gurunathan <rgurunathan@nvidia.com>
adjust_memory_regions() sized queue-backed MRs purely relative to the NIC descriptor ring: it bumped anything below 1.5x ring up to 3x ring. That models worker refill headroom but ignores the TX path, where is_tx_burst_available() refuses a burst unless 2x batch_size buffers are free *on top of* whatever the ring is holding. With the shipped batch_size of 10240 and the default 8192-deep ring, the 3x ring bump target (24576) is below the 28672 that TX requires, so the bump produced a configuration that wedges TX permanently at bursts=0 while logging that it had just prevented a deadlock. Nothing reported an error: 0 missed, 0 errored, exit 0, and only one burst ever reaching the wire. Take the max of both constraints instead: floor at max(1.5x ring, ring + 2x batch_size) and bump to max(3x ring, ring + 4x batch_size). Small-batch configs keep their previous sizing, so this only raises the bar where batch_size demands it. Also report the one remaining route to a silent TX stall that config sizing cannot cover: an application asking for a burst larger than its configured batch_size. is_tx_burst_available() now distinguishes ordinary backpressure from a request the pool can never satisfy however long it drains, and logs that once per queue with the num_bufs needed. A short pool that will drain stays silent. Testing (IGX Thor, ConnectX-7 dual-port cable loopback, 8 KB frames unless noted): - Bracketed the deadlock boundary at ring 8192 / batch 10240. 24576 (the old bump target) and 26624 wedge at bursts=1; 28672 (= ring + 2x batch) runs. - Measured where starvation clears: 28672 still logs 1.4e9 rx_mbuf_allocation_errors, 38912 logs 1.7e7, 49152 (= ring + 4x batch) is the first value with zero alloc errors and zero drops. 51200 (what the example configs ship) is also clean, which is consistent with the new target. - The two configs that previously wedged (explicit num_bufs 24576, and num_bufs 8000 auto-bumped into 24576) now run at line rate with 0 missed and 0 alloc errors. Wire counters via ethtool -S corroborate: before the fix only 10,242 frames left the NIC in 10 s. - Oversized-burst guard: a bench batch of 40000 against a 51200 pool logs the new error exactly once and names the required num_bufs. - No regressions, A/B against main with matched build configuration (CMAKE_CUDA_ARCHITECTURES and DAQIRI_BUILD_PYTHON aligned; an earlier mismatch made reorder_seq look artificially better on this branch): daqiri_bench_raw_gpudirect, daqiri_bench_raw_hds (2 MRs per queue, which exercises the new per-MR max-batch map), the 4-queue config, daqiri_bench_raw_sw_loopback, and daqiri_bench_raw_reorder_seq all match baseline within run-to-run variance. - Shipped example configs are unaffected: all raw configs in examples/ and applications/ already clear both thresholds, so none are bumped and no new warnings appear. Assisted-by: Claude Signed-off-by: Ramya Gurunathan <rgurunathan@nvidia.com>
When CU_DEVICE_ATTRIBUTE_DMA_BUF_SUPPORTED is 0, register_memory_regions() fell back to rte_extmem_register() and logged "Successfully registered external memory" even with nvidia_peermem absent, i.e. with no usable GPUDirect path at all. The failure then surfaced two functions later in map_memory_regions() as a bare rte_dev_dma_map "Invalid argument", by which point nothing identified the cause. The common trigger is selecting the wrong GPU: on Tegra/IGX the container runtime exposes only the integrated GPU unless the discrete one is passed through explicitly, and an integrated GPU cannot export a dma-buf. Treat dma-buf unsupported plus peermem absent as the unsatisfiable combination it is and fail there, naming the CUDA device by ordinal, name, UUID and integrated status. Report what else CUDA can see, so a user who picked the wrong ordinal is pointed at the discrete device to use instead, and one whose container exposes no discrete GPU is told that rather than being sent after peermem, which does not apply to Tegra. Decode cuGetErrorString on the cuMemGetHandleForAddressRange failure path too, and note the kernel and driver prerequisites there, matching the ibverbs engine's message from #239. Before, on the Thor iGPU: WARN dma-buf not supported for device 0. Attempting to use nvidia-peermem INFO Successfully registered external memory for Data_TX_GPU CRITICAL Could not DMA map EXT memory: -1 err=Invalid argument After: CRITICAL Memory region 'Data_TX_GPU' requests kind: device on CUDA device 0 ('NVIDIA Thor', integrated) GPU-a7c66ad2-... , which cannot export a dma-buf, and the nvidia_peermem module is not loaded -- there is no usable GPUDirect path for this device. CUDA sees 1 device(s), none of them discrete. On Tegra/IGX the container runtime exposes only the integrated GPU unless the discrete GPU is passed through explicitly (CDI: --device nvidia.com/gpu=N); alternatively use kind: host_pinned. Uses only libcuda, which is already linked; no new dependency. The discrete-GPU path is unchanged and still registers via dma-buf. Testing (IGX Thor): - Reproduced the original failure by exposing only the Tegra iGPU (--runtime=nvidia --gpus all, which forces csv mode; the iGPU reports INTEGRATED=1 and DMA_BUF_SUPPORTED=0) and confirmed the new message replaces it, with the misleading "Successfully registered" and "Invalid argument" lines gone and init aborting at registration. - Discrete-GPU path unchanged: with the RTX PRO 6000 exposed via CDI, DMA_BUF_SUPPORTED=1 and every raw bench still registers through dma-buf ("dma-buf supported for device 0") and runs at line rate. Assisted-by: Claude Signed-off-by: Ramya Gurunathan <rgurunathan@nvidia.com>
|
| Filename | Overview |
|---|---|
| src/engine_dpdk.cpp | The previously unguarded DMA-BUF export-failure fallback now checks for nvidia_peermem; initialization abort and resource cleanup remain correctly wired. |
| src/engines/dpdk/daqiri_dpdk_engine.cpp | Memory-region sizing now accounts for batch headroom, and unsatisfiable TX burst requests receive a rate-limited diagnostic. |
| src/engines/dpdk/daqiri_dpdk_engine.h | Adds per-queue atomic state used to emit the oversized-burst diagnostic only once. |
| scripts/build-container.sh | Honors explicit base-image overrides and derives the default CUDA image from configurable CUDA and Ubuntu versions. |
| docs/api-reference/configuration.md | Documents the revised raw-DPDK memory-region floor and automatic bump target. |
| docs/getting-started.md | Documents host-driver compatibility and container base-image version overrides. |
| docs/tutorials/configuration-walkthrough.md | Updates the annotated num_bufs guidance to match the new ring-and-batch sizing formula. |
| AGENTS.md | Records the container-build overrides and DPDK memory-region sizing behavior for maintainers. |
Reviews (2): Last reviewed commit: "#242 - Guard every peermem fallback, not..." | Re-trigger Greptile
… case The previous commit only fail-fasted when CU_DEVICE_ATTRIBUTE_DMA_BUF_SUPPORTED was 0. Two other paths still fell back to rte_extmem_register() without checking that nvidia_peermem was loaded: when the device advertises dma-buf but cuMemGetHandleForAddressRange fails, and when the DPDK build predates rte_extmem_register_dmabuf. In both cases registration can still succeed and the failure resurfaces later in map_memory_regions() as the same non-actionable rte_dev_dma_map "Invalid argument" this change set out to remove. Factor the check into peermem_fallback_ok() and apply it at all three fallback sites, so the actionable message is emitted wherever GPUDirect has no path left. The pre-24.11 site closes the exported fd before bailing out. Reported by Greptile on #248. Testing: re-verified both ends on IGX Thor. The Tegra iGPU still fails fast at registration with the device named and no "Successfully registered" or "Invalid argument" lines; the discrete RTX PRO 6000 still takes the dma-buf path ("dma-buf supported for device 0") and runs at line rate. Assisted-by: Claude Signed-off-by: Ramya Gurunathan <rgurunathan@nvidia.com>
| "MR '{}' had num_bufs={}, below the {} required by a {}-deep NIC ring and " | ||
| "batch_size={} (ring + 2x batch); TX would wedge with no error and the RX refill " | ||
| "path would starve. Bumping to {} (ring + 4x batch), the smallest size measured " | ||
| "free of mbuf allocation errors.", |
There was a problem hiding this comment.
Should these warnings tell the user what to do? ie increase num_bufs?
| DAQIRI_LOG_CRITICAL( | ||
| "Memory region '{}' requests kind: device on CUDA device {}, but {}, and the " | ||
| "nvidia_peermem module is not loaded -- there is no usable GPUDirect path for this " | ||
| "device. {}.", |
There was a problem hiding this comment.
Suggest trying host_pinned?
cliffburdick
left a comment
There was a problem hiding this comment.
Does anything here apply to ibverbs path as well?
Yes, good call. I think at least item 2 on the list would also need to be ported over to ibverbs. I'll make sure I've gone through all the ibverbs testing and adjust the warning/helper messages based on your comments. |
Addresses #242. All issues reproduced on current
main(676b260) before being fixed.1. Size queue memory regions against
batch_size, not just ring depth.adjust_memory_regions()sized MRs relative to the NIC ring only, ignoring thatis_tx_burst_available()needs2x batch_sizefree on top of the ring — so at the shippedbatch_size: 10240its own 3x-ring bump target (24576) sat below the 28672 TX requires andsilently wedged TX at
bursts=0. Floor is nowmax(1.5x ring, ring + 2x batch_size)and thebump target
max(3x ring, ring + 4x batch_size), plus a one-shot diagnostic for bursts thepool can never satisfy.
2. Fail fast when a device memory region has no GPUDirect path.
With dma-buf unsupported and
nvidia_peermemabsent, registration logged "Successfullyregistered" and then died two functions later as a bare
Invalid argument; it now fails atregistration, naming the CUDA device and what else CUDA can see so "wrong GPU selected" is
readable off one line.
3. Allow overriding the container CUDA base image.
scripts/build-container.shoverwrote an exportedDAQIRI_OS_BASE_IMAGE, pinning CUDA to13.1.0 and making the script unusable on hosts with an older driver; it now honors a pre-set
value and adds
CUDA_VERSION/UBUNTU_VERSION, with default output unchanged.Testing. On IGX Thor with a ConnectX-7 cable loopback: bracketed the deadlock boundary
exactly, confirmed both previously-wedging configs now run at line rate with zero drops
(corroborated by
ethtool -Swire counters), reproduced the GPUDirect failure on the TegraiGPU, and verified CUDA 13.0 inits where 13.1 returns
cuInit=803. A/B againstmainwithmatched build configuration shows no regressions across
gpudirect,hds, the 4-queueconfig,
sw_loopback, andreorder_seq.Note: #242's BAR1 premise is incorrect — the card has 8 GiB, not 256 MiB — so nothing
forces a smaller ring and the per-queue ring-depth YAML field the issue proposes is not
included; the sizing formula was the actual bug.