Skip to content

[TENT] Cover the RDMA context enable failure paths and link-speed events without an RNIC - #4263

Merged
alogfans merged 1 commit into
kvcache-ai:mainfrom
SongOf:test/tent-rdma-context-lifecycle
Sep 22, 2026
Merged

alogfans merged 1 commit into
kvcache-ai:mainfrom
SongOf:test/tent-rdma-context-lifecycle

Conversation

@SongOf

@SongOf SongOf commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Description

RdmaContext::enable() acquires a device, a PD, an epoll set, completion channels, data CQs and a notification CQ in order, and #3325 made a failure between any two steps release what had been acquired so far. None of that has been testable without an RNIC. The per-context verbs table from #3777 lets a test stand in for individual verbs, but openDevice() still called libibverbs directly for its GID scan, and ibv_query_gid_ex() is a header inline that dereferences context->device, so a stand-in ibv_context crashed inside the library before enable() got anywhere.

Two small production changes open the seam:

  • _ibv_query_gid_ex, the entry point behind that inline, joins IbvSymbols with a QueryGidEx() helper hiding entry_size, and the three GID-scan call sites go through the table. The binary already links the symbol, so requiring it loses no host.
  • kIbvEventDeviceSpeedChange names IBV_EVENT_DEVICE_SPEED_CHANGE where the header lacks the enumerator (the async-event ABI is append-only, so its value is 20, after WQ_FATAL), and applyContextEvent() switches on int. The branch that handles a speed change without a link flap is now compiled and tested everywhere instead of sitting behind #ifdef. A static_assert pins the fallback to the real enumerator on any build whose header has it, so a header that ever broke the append-only rule fails to compile rather than misroute an event.

On that seam, a FakeRnic stands in for a whole device with live counters of what is currently held, and 19 cases cover:

  • every fakeable exit of enable() and openDevice() — unknown name, open failure, ibv_query_device, port out of range, no usable GID, ibv_alloc_pd, a later completion channel, a later data CQ, the notification CQ, the final port query — each asserting nothing is left held on either the device or the context;
  • a port that is not ACTIVE at open lands in DEVICE_PAUSED with everything built;
  • openDevice() seeds the effective speed, and falls back to the encoded rate when the verb reports 0;
  • five async-event cases through Workers::applyContextEvent: PORT_ACTIVE re-seeds the selector only when the speed changed and still recovers the device when its re-read fails; DEVICE_SPEED_CHANGE re-seeds without touching availability, keeps the EWMA at an unchanged speed, and changes nothing when the re-read fails.

Not covered: epoll_create1 and the fcntl/epoll_ctl exits, which are syscalls the table cannot stand in for, and the active_speed_ex decode, which the build image's rdma-core header lacks.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake Conductor (mooncake-conductor)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other (test coverage, plus two small seam changes that make the paths testable)

How Has This Been Tested?

Every case was red-checked by disabling the production line it is about: removing the disable() call after any fakeable acquisition in enable() (PD, completion channel, data CQ, notification CQ, final port query) fails exactly the matching case; leaking the scoped device handle on the ibv_query_device or no-GID exit, or skipping ibv_free_device_list after a failed open, fails exactly the three openDevice() cases and no other; putting the speed-change branch back behind #ifdef fails all three of its cases, since each asserts the re-read happened.

Test commands:

cmake --build build-tent --target tent_rdma_transport_test
./tent_rdma_transport_test
pre-commit run --files mooncake-transfer-engine/tent/include/tent/transport/rdma/ibv_loader.h mooncake-transfer-engine/tent/include/tent/transport/rdma/workers.h mooncake-transfer-engine/tent/src/transport/rdma/context.cpp mooncake-transfer-engine/tent/src/transport/rdma/ibv_loader.cpp mooncake-transfer-engine/tent/src/transport/rdma/workers.cpp mooncake-transfer-engine/tent/tests/rdma_transport_test.cpp
./scripts/code_format.sh --check --changed-lines --base origin/main

Test results:

  • Unit tests pass (tent_rdma_transport_test: 104 passed, 2 skipped for want of an RDMA device, same as main)
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

Claude Code helped design the stand-in device, write the cases and the two seam changes. Every changed line has been reviewed by the submitter.

@SongOf
SongOf force-pushed the test/tent-rdma-context-lifecycle branch from c77ec5e to 7e05f33 Compare September 21, 2026 17:23

@alogfans alogfans left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alogfans
alogfans merged commit a6ea18c into kvcache-ai:main Sep 22, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants