Skip to content

[RDMA] Replace stale EQDS pull-target bits on retransmission - #1107

Merged
zhongjiechen merged 2 commits into
uccl-project:mainfrom
LiRunGuo:fix/eqds-retransmit-pull-target
Nov 3, 2026
Merged

zhongjiechen merged 2 commits into
uccl-project:mainfrom
LiRunGuo:fix/eqds-retransmit-pull-target

Conversation

@LiRunGuo

Copy link
Copy Markdown
Contributor

Description

Fixes #1097.

EQDS retransmission decodes an existing work request's immediate data and calls SetTarget() with a newly computed pull target. The setter previously ORed new bits into the old target: changing 127 to 128 encoded 255, and setting zero did not clear the old value.

Clear the eight target bits before inserting the new value. Preserve FID, RID, CSN, HINT, and the existing truncation of input to eight bits.

Add imm_data_test, picked up by the existing native Makefiles. It checks all 65,536 old/new target combinations through network-byte-order encode/decode, varies the remaining fields, and checks clearing and oversized inputs.

Type of Change

  • Bug fix
  • New feature
  • Documentation update

How Has This Been Tested?

Linux x86-64 / GCC 13.3 / CUDA 12.8 headers, comparing upstream 54e216ee and this fix with the same test:

  • Baseline: 58,977 failures out of 65,539 checks.
  • Fixed: 65,539 checks pass, with all non-target fields preserved.
  • ASan + UBSan build of the test and param.cc: all checks pass, including leak checking.
  • Native builds of imm_data_test, all RDMA transport objects, and libnccl-net-uccl.so pass for baseline and fix, using the repository's pinned NCCL headers (2ea4ee94). Because the host lacks the unused gflags/gtest libraries, used LIBS="-lz -lelf -libverbs -L/usr/local/cuda-12.8/lib64 -lcudart -lpthread" and LIBS_SHARED="-lz -lelf -libverbs".
  • Changed files pass clang-format 14.0.6 and git diff --check.
make -C collective/rdma imm_data_test
./collective/rdma/imm_data_test

The test executes the production immediate-data types without a device or remote host. Live packet-loss/retransmission traffic and throughput were not measured. This affects EQDS when enabled; the default receiver-CCA configuration is unchanged.

  • Unit tests
  • Integration tests
  • Manual testing

Checklist

  • I have run format.sh to follow the style guidelines. (Ran clang-format 14 directly on the changed files.)
  • I have run build.sh to verify compilation. (Used native Makefile builds above.)
  • I have removed redundant variables and comments.
  • I have updated the documentation. (No public API or configuration change.)
  • I have added tests.

@YangZhou1997

Copy link
Copy Markdown
Member

cc @zhongjiechen

@zhongjiechen

Copy link
Copy Markdown
Member

/run-benchmark l4

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Benchmark l4 passed

PR: #1107
Commit: c900edc021a3875f6f31dca5eea81a1650cc7ae6
Result: success
Workflow run: https://github.com/uccl-project/uccl/actions/runs/41646729401

@zhongjiechen
zhongjiechen merged commit d323d6a into uccl-project:main Nov 3, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[collective/rdma][EQDS] SetTarget preserves stale pull-target bits on retransmission

3 participants