[TE] Enabling UB Transport on the Kunpeng SuperNode Phase 1 - #1805
Conversation
…ort_dev # Conflicts: # mooncake-transfer-engine/src/transport/kunpeng_transport/ub_context.cpp # mooncake-transfer-engine/src/transport/kunpeng_transport/urma_endpoint.cpp
There was a problem hiding this comment.
Code Review
This pull request introduces the 'ub' (User-space Bridge) transport protocol, specifically integrating the URMA (User-space RDMA) implementation. The changes include new configuration parameters, metadata structures, and a complete transport implementation within the kunpeng_transport directory. The topology discovery mechanism has been updated to support UB devices, and example benchmarks now include 'ub' protocol support. Review feedback highlights several critical memory management issues, such as incorrect use of delete on std::shared_ptr and malloc-allocated memory, as well as the need to return Slice objects to their cache instead of deleting them. Logic errors were also identified where getters returning vectors by value were incorrectly used for modifications, and an off-by-one error was found in a bounds check.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds an initial UB (Unified Bus) transport implementation to Mooncake Transfer Engine, targeting Kunpeng platforms via the openEuler UMDK URMA APIs. It introduces new transport/context/endpoint codepaths, integrates UB into build + transport registration, and extends metadata/topology to support UB device discovery and connection handshakes.
Changes:
- Introduce
UbTransportwith URMA-backedUrmaContext/UrmaEndpoint, plus worker pool + endpoint store. - Add
USE_UBbuild option and wire UB transport into TE init, multi-transport registry, metadata encoding/handshake, and topology discovery. - Add a UB “test” executable and update example benchmarks to accept
--protocol=ub.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| mooncake-transfer-engine/tests/ub_transport_test.cpp | Adds a UB integration-style executable (currently behaves like a manual 2-process runner). |
| mooncake-transfer-engine/tests/CMakeLists.txt | Registers ub_transport_test in build + CTest under USE_UB. |
| mooncake-transfer-engine/src/transport/kunpeng_transport/urma_endpoint.cpp | Implements URMA context + endpoint (registration/import, jetty setup, posting/polling). |
| mooncake-transfer-engine/src/transport/kunpeng_transport/ub_transport.cpp | Implements the Transport interface for UB and wires it to contexts + handshake. |
| mooncake-transfer-engine/src/transport/kunpeng_transport/ub_context.cpp | Implements endpoint store + worker pool (post-send scheduling, polling, redispatch). |
| mooncake-transfer-engine/src/transport/kunpeng_transport/CMakeLists.txt | Adds CMake target for UB transport sources + URMA linkage. |
| mooncake-transfer-engine/src/transport/CMakeLists.txt | Adds kunpeng_transport subdir and objects into transport under USE_UB. |
| mooncake-transfer-engine/src/transfer_metadata.cpp | Extends handshake + segment descriptor (encode/decode) for UB (EID, jetty_num, tseg). |
| mooncake-transfer-engine/src/transfer_engine_impl.cpp | Auto-installs UB transport when USE_UB is enabled. |
| mooncake-transfer-engine/src/topology.cpp | Adds UB device discovery via URMA + sysfs and switches CPU topology discovery under USE_UB. |
| mooncake-transfer-engine/src/multi_transport.cpp | Registers "ub" protocol in MultiTransport. |
| mooncake-transfer-engine/src/config.cpp | Extends env parsing to also populate UB JFC/JFCE config from MC_NUM_CQ_PER_CTX. |
| mooncake-transfer-engine/src/CMakeLists.txt | Links UB transport target when USE_UB is enabled. |
| mooncake-transfer-engine/include/transport/transport.h | Extends Transport::Slice union with UB-specific fields. |
| mooncake-transfer-engine/include/transport/kunpeng_transport/urma_endpoint.h | Adds URMA context/endpoint declarations and URMA-specific helpers. |
| mooncake-transfer-engine/include/transport/kunpeng_transport/ub_transport.h | Adds UB transport interface declaration. |
| mooncake-transfer-engine/include/transport/kunpeng_transport/ub_endpoint.h | Adds abstract UB endpoint interface. |
| mooncake-transfer-engine/include/transport/kunpeng_transport/ub_context.h | Adds UB context abstraction and worker pool / endpoint store declarations. |
| mooncake-transfer-engine/include/transfer_metadata.h | Extends metadata structures for UB (EID, tseg/l_seg_index, jetty_num). |
| mooncake-transfer-engine/include/config.h | Adds UB-related config fields (jfc/jfce counts, eid_index, max_seg_size, etc.). |
| mooncake-transfer-engine/example/transfer_engine_bench.cpp | Adds UB transport install path and changes host free path for --protocol=ub. |
| mooncake-transfer-engine/example/transfer_engine_bench_with_retry.cpp | Adds UB transport install path. |
| mooncake-transfer-engine/example/transfer_engine_bench_with_notify.cpp | Adds UB transport install path. |
| mooncake-common/common.cmake | Adds USE_UB option and compile definition. |
Comments suppressed due to low confidence (1)
mooncake-transfer-engine/src/topology.cpp:490
- When
USE_UBis enabled,all_hcais astd::vector<UBDevice>, butdiscoverCudaTopologyis declared to takeconst std::vector<InfinibandDevice>&. This call will not compile in CUDA/MUSA/HIP builds. Consider templating/overloadingdiscoverCudaTopologyfor UBDevice, or using a shared device struct type for both RDMA and UB discovery paths.
#if defined(USE_CUDA) || defined(USE_MUSA) || defined(USE_HIP)
for (auto &ent : discoverCudaTopology(all_hca)) {
matrix_[ent.name] = ent;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The usage guide should be included in the docs. In addition, what's the main advantage of using this approach, rather than other compatible xports? |
Sure, I will add some more details about the usage guide later. By the way, which directory should I write it to? @alogfans Here are some of my thoughts on the advantages of the current proposal:
@alogfans @stmatengss welcome any suggestions for this issue. I am very happy to participate in the construction of the community ecosystem together. |
9a61ac0 to
76c8e46
Compare
|
Please fix this CI error. https://github.com/kvcache-ai/Mooncake/actions/runs/23950310040/job/69858773912?pr=1805 |
@stmatengss could you please rerun it again? I haven't been able to pinpoint the problem yet. I tried running CI/Testing on my personal repository pull request zchuango#2 and it can run normally, so I suspect it might be an intermittent issue. |
|
Please update the documentation at https://github.com/kvcache-ai/Mooncake/tree/main/docs/source/design/transfer-engine or consider it as the next step. |
@alogfans Please double-check
|
Yes, I plan to update the documentation in the next step. |
…ai#1805) * add the support for ub transport * refine the code format * change some log for ub transport * change ub_transport to kunpeng_transport * add the support for ub transport * refine the code format * change some log for ub transport * change ub_transport to kunpeng_transport * clean some comments and unused code * fix the config.h merge error * fix the code with review suggestion * add the usage docs and refine some code for code review * fix some comments with chinese and modify the CMakeLists.txt file

Description
Overview
This PR introduces UbTransport, a new transport implementation for Mooncake's Transfer Engine that enables high-performance data transfer on Kunpeng 950 platforms using the UB (Unified Bus) protocol and URMA (Unified Remote Memory Access) APIs from the openEuler UMDK.
Related Issue: #1773
Motivation
Build & Compile
Prerequisites
yum install umdk-urma-develor build from sourceCMake Configuration
Verification
Run & Test
Single-node benchmark
Multi-device benchmark
# Discover multiple URMA devices ./transfer_engine_bench \ --protocol=ub \ --device_name=urma0,urma1,urma2,urma3Phase Contribution Plan
Following suggestion, we propose four phased PRs for incremental review and integration:
Review focus: Transport interface compliance, WorkerPool design, error handling
Acknowledgments
Thanks to @stmatengss and the Mooncake team for the phased contribution guidance.
Ready for review: Phase 1 implementation is complete and tested. Please advise on any architectural adjustments before we proceed with detailed code review.
@stmatengss @alogfans @ascend-direct-dev
Module
mooncake-transfer-engine)mooncake-store)mooncake-ep)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-pg)mooncake-rl)Type of Change
How Has This Been Tested?
Checklist
./scripts/code_format.shbefore submitting.