[PAL] Add barex transport adaptor - #526
Open
ymmyys wants to merge 10 commits into
Open
Conversation
ymmyys
requested review from
MC952-arch,
aoyulong and
mikethegoblin
as code owners
July 30, 2026 10:34
ymmyys
force-pushed
the
accl-transport
branch
2 times, most recently
from
July 31, 2026 05:25
c81d106 to
8e15c58
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces an ACCL (accl::barex) transport path to FlagCX, adding both a new net adaptor (“BAREX”) and a P2P-engine transport option selected via FLAGCX_P2P_TRANSPORT=accl, targeting PPU + vsolar environments where peer-mem/DMA-BUF isn’t available.
Changes:
- Add
USE_ACCL_BAREXbuild option and link/include integration forlibaccl_barex. - Add ACCL P2P transport implementation (
flagcx_p2p_accl.*) and dispatch routing inflagcx_p2p.ccvia kind tags. - Add
barexnet adaptor implementation and wire it into unified net adaptor selection when enabled.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Adds USE_ACCL_BAREX build toggle and injects ACCL include/link flags into compile/link steps. |
| flagcx/core/include/flagcx_p2p_accl.h | Declares ACCL P2P transport interface + kind-tag routing helper/stubs. |
| flagcx/core/flagcx_p2p.cc | Routes public P2P API entry points to ACCL transport based on runtime selection and kind tags. |
| flagcx/core/flagcx_p2p_accl.cc | Implements ACCL-based P2P engine/connection, registration, RDMA ops, metadata, and notif plumbing. |
| flagcx/adaptor/net/barex_adaptor.cc | Implements BAREX net adaptor over ACCL barex (listen/connect/accept, MR reg, isend/irecv/test). |
| flagcx/adaptor/adaptor.cc | Selects flagcxNetBarex as the unified IBRC adaptor when USE_ACCL_BAREX is enabled. |
Suppressed comments (3)
flagcx/adaptor/net/barex_adaptor.cc:641
- If HELLO buffer allocation fails after creating/inserting a BarexComm, the code deletes connectState but leaks the BarexComm and leaves the XChannel registered in channelComm. It also leaves handle->state in BAREX_CONN_CONNECTING, preventing retries.
if (e->mempool->AllocBuffer(msg, sizeof(BarexHelloMsg), accl::barex::CPU,
ch->GetLocalNicId(),
0) != accl::barex::BAREX_SUCCESS) {
WARN("NET/BAREX : HELLO buffer alloc failed");
delete st;
flagcx/adaptor/net/barex_adaptor.cc:666
- If HELLO send fails synchronously, barexConnect deletes connectState but leaks the BarexComm/channel mapping and keeps handle->state at BAREX_CONN_CONNECTING. This can leak resources and makes retries impossible.
if (r != accl::barex::BAREX_SUCCESS) {
WARN("NET/BAREX : HELLO send sync error: %s", bxstr(r));
/* per xchannel.h: on send failure the buffer is NOT auto-released */
e->mempool->ReleaseBuffer(msg.buf, accl::barex::CPU);
delete st;
flagcx/adaptor/net/barex_adaptor.cc:678
- If HELLO delivery fails asynchronously (helloFailed), barexConnect deletes connectState but leaves the created BarexComm/XChannel in channelComm and does not reset handle->state. This leaks resources and prevents recovery.
/* BAREX_CONN_HELLO */
if (st->helloFailed.load(std::memory_order_acquire)) {
WARN("NET/BAREX : HELLO delivery failed");
delete st;
handle->connectState = nullptr;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @mkdir -p `dirname $@` | ||
| @echo "Linking $@" | ||
| @$(LINKER) $^ -o $@ -L$(CCL_LIB) -L$(DEVICE_LIB) -L$(HOST_CCL_LIB) -L$(UCX_LIB) -shared -fvisibility=default -Wl,--no-as-needed -Wl,-rpath,$(LIBDIR) -Wl,-rpath,$(CCL_LIB) -Wl,-rpath,$(HOST_CCL_LIB) -Wl,-rpath,$(UCX_LIB) -lpthread -lrt -ldl $(CCL_LINK) $(DEVICE_LINK) $(HOST_CCL_LINK) $(UCX_LINK) -g | ||
| @$(LINKER) $^ -o $@ -L$(CCL_LIB) -L$(DEVICE_LIB) -L$(HOST_CCL_LIB) -L$(UCX_LIB) -L$(ACCL_BAREX_LIB) -shared -fvisibility=default -Wl,--no-as-needed -Wl,-rpath,$(LIBDIR) -Wl,-rpath,$(CCL_LIB) -Wl,-rpath,$(HOST_CCL_LIB) -Wl,-rpath,$(UCX_LIB) -Wl,-rpath,$(ACCL_BAREX_LIB) -lpthread -lrt -ldl $(CCL_LINK) $(DEVICE_LINK) $(HOST_CCL_LINK) $(UCX_LINK) $(ACCL_BAREX_LINK) -g |
| ACCL_BAREX_INCLUDE = $(ACCL_BAREX_HOME)/include | ||
| ACCL_BAREX_LIB = $(ACCL_BAREX_HOME)/lib | ||
| ACCL_BAREX_LINK = -laccl_barex | ||
| NET_ADAPTOR_FLAG += -DUSE_ACCL_BAREX -DCMAKE_INCLUDE=1 |
| @mkdir -p `dirname $@` | ||
| @echo "Compiling $@" | ||
| @$(HOST_COMPILER) $< -o $@ $(foreach dir,$(INCLUDEDIR),-I$(dir)) -I$(CCL_INCLUDE) $(addprefix -I,$(DEVICE_INCLUDE)) -I$(HOST_CCL_INCLUDE) -I$(UCX_INCLUDE) $(ADAPTOR_FLAG) $(HOST_CCL_ADAPTOR_FLAG) $(NET_ADAPTOR_FLAG) $(COMPILE_KERNEL_HOST_FLAG) -c -fPIC -fvisibility=default -Wvla -Wno-unused-function -Wno-sign-compare -Wall -MMD -MP -g | ||
| @$(HOST_COMPILER) $< -o $@ $(foreach dir,$(INCLUDEDIR),-I$(dir)) -I$(CCL_INCLUDE) $(addprefix -I,$(DEVICE_INCLUDE)) -I$(HOST_CCL_INCLUDE) -I$(UCX_INCLUDE) -I$(ACCL_BAREX_INCLUDE) $(ADAPTOR_FLAG) $(HOST_CCL_ADAPTOR_FLAG) $(NET_ADAPTOR_FLAG) $(COMPILE_KERNEL_HOST_FLAG) -c -fPIC -fvisibility=default -Wvla -Wno-unused-function -Wno-sign-compare -Wall -MMD -MP -g |
Comment on lines
+613
to
+619
| if (st->connectFailed.load(std::memory_order_acquire)) { | ||
| WARN("NET/BAREX : channel connect failed (commId 0x%llx)", | ||
| (unsigned long long)handle->commId); | ||
| delete st; | ||
| handle->connectState = nullptr; | ||
| return flagcxInternalError; | ||
| } |
On PPU (810e) + vsolar hosts, device memory can only be registered and moved through the vendor ACCL library: peer-mem and DMA-BUF registration are unsupported by the driver stack (probe: both ibv_reg_mr paths fail; even a valid dmabuf fd is rejected), and CUDA-VMM memory cannot be pinned at all (RegUserMr -> BAREX_ERR_MR, vsolar_pin_user_mem err=-22). Add a second P2P engine transport selected at engine creation with FLAGCX_P2P_TRANSPORT=accl (default remains ibrc; its code paths are untouched). The public flagcx_p2p.h API is unchanged - each entry point routes on a kind tag placed at the head of the engine/conn structs. The transport mirrors Mooncake's barex_transport: one XSimpleMempool across the selected NICs (RegUserMr yields per-NIC lkey/rkey vectors), one server+client XContext per NIC, XListener/XConnector for setup, WriteBatch/ReadBatch with completion callbacks for data movement. Rendezvous reuses the FlagCX bootstrap handshake; the desc table carries rkey vectors, folded into the existing 64-byte desc (rkeys[0] at the ibrc offset, count in nmsgs, rest in padding). Build with USE_ACCL_BAREX=1 (links -laccl_barex). v1 scope: initiator- side transfers (NIXL PD-disaggregation pattern), no IPC fast path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements the flagcxNetAdaptor vtable over the vendor ACCL library
(accl::barex) so the hetero proxy path can register and move PPU device
memory without peer-mem/DMA-BUF (neither exists in the PPU driver
stack) and without relying on the NCCL accl-lite plugin.
Protocol: listen() publishes {OOB IP, shared barex data port, random
commId} in the 64-byte handle; connect() opens an XChannel and sends
HELLO{commId} which the acceptor demuxes to its pending accept. irecv
sends CTS{slot, addr, size, per-NIC rkeys} to the sender; isend answers
with one WriteSingle(signal_peer, imm=slot) straight into the posted
receive buffer; the receiver completes on OnImmRecvCall. Registration
forwards to XSimpleMempool::RegUserMr (engine-wide refcounted MR map,
regIsGlobal=1), which also serves the flagcxCommRegister user-buffer
path that previously failed on PPU. connect/accept follow the proxy's
non-blocking resumable contract; isend returns a NULL request while no
CTS is queued (proxy retries); iflush returns the 0x1 instant-done
sentinel (write-with-imm already orders payload before completion).
Selected via USE_ACCL_BAREX=1 (takes the IBRC registry slot, like
USE_UCX); FLAGCX_BAREX_DISABLE=1 opts out at runtime. Run with
FLAGCX_VMM_ENABLE=0 on PPU: CUDA-VMM memory cannot be pinned.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings fixed (all latent — the happy path passed on hardware): - connect rendezvous: move the latch/mutex/channel list into a heap control block shared with ACCL callbacks; a callback arriving after the 20s timeout previously touched destroyed stack state (UB) and leaked its channel. Late callbacks now self-clean via an abandoned flag. - notif accept: accepted fds don't inherit O_NONBLOCK from the listen socket; a stalled peer could wedge the single poll loop indefinitely. Bound the magic read with SO_RCVTIMEO during the handshake. - StopAccept: close the bootstrap listen socket so the rpc accept thread actually unblocks before Destroy joins it. - regMr dedup: re-check the map after dropping and retaking the lock; two threads registering the same base concurrently could double- register and orphan one MR. - ctrl-message sends: per xchannel.h, a failed Send does NOT auto- release the pooled buffer — release it explicitly on the sync-error paths (HELLO + CTS). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CTS control messages are delivered by ACCL's multi-threaded callback pool, so they can be enqueued out of the receiver's post order. The sender paired its k-th isend with whichever CTS happened to be at the queue head, writing chunk k's payload into the buffer posted for a different chunk. The proxy pipelines up to 16 chunks per message, so transfers of ~32MB and above (8+ in-flight chunks) corrupted data while small transfers appeared fine; perf runs passed because they never verify bytes. Stamp each CTS with the receiver's post-order sequence number (stored in the CTS message's former tail padding; wire size stays 64 bytes) and make the sender consume CTS strictly in sequence order, retrying via the proxy when the next-expected CTS has not arrived yet. Verified on 810e (PPU dev5,6 + vsolar_1): byte-verify at 1MB, 16MB+8K, 32MB, 64MB now ALL_MATCH twice in a row (previously MISMATCH at 32/64MB), and -R 0 staging-path verify at 64/128/256MB ALL_MATCH (256MB = 64 chunks, 4x ring-buffer wrap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Shorten the comments introduced by the ACCL transport work across the barex net adaptor, the p2p ACCL engine/header, and the Makefile build block so their density matches FlagCX's existing style, keeping the load-bearing notes (CTS seq ordering, listen-handle size, write-with-imm + iflush sentinel, u2mm RTLD_LOCAL plugin requirement). Also collapse a pre-existing 3-line getSharedCq() accessor onto one line so the full-file clang-format check stays clean now that this PR touches flagcx_p2p.cc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…c and unified environment variables
Upstream flagos-ai#522/flagos-ai#529 route flagcx_device.cc through the devApiBackend pointer, but only nvidia.mk compiles a backend implementation, so USE_PPU=1 builds produce libflagcx.so with an undefined devApiBackend symbol that fails at dlopen. Mirror nvidia.mk's default path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Match upstream comment density: keep only constraint/contract comments (wire-format limits, locking, vendor API caveats), drop decorative section banners and restate-the-code narration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test/make.inc had no USE_PPU case, so test builds on PPU died with "No platform selected". Route it to makefiles/ppu.mk and set DEVICE_PLATFORM there (CUDA-compatible stack) since make.inc requires it non-empty for kernel/IR dir resolution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vLLM PD registers the whole KV-cache region in one flagcxP2pRegister call (2.5GB+); vsolar's verbs reject GPU MRs above ~64MB (ENOMEM), so engine creation-time registration failed and PD could not start on 810e. Mirror Mooncake's barex transport (eic_max_block_size): split registrations into 64MB chunks (FLAGCX_ACCL_MAX_MR_MB overrides, 0 disables), publish one desc-table entry per chunk in the handshake, and split each submitted iov at local and remote chunk boundaries with per-chunk lkeys/rkeys. MakeDesc validates ranges against merged spans so cross-chunk writes stay accepted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+603
to
+606
| WARN("NET/BAREX : HELLO buffer alloc failed"); | ||
| delete st; | ||
| handle->connectState = nullptr; | ||
| return flagcxInternalError; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Category
PAL
PR Types
New Features
PR Description
This PR adds a Barex transport adapter to FlagCX and integrates it with PPU ACCL.