[michal-shalev]

What?

Implement uct_ep_get_sgl_zcopy for RC mlx5, report UCT_IFACE_FLAG_V2_GET_SGL_ZCOPY and max_get_sgl_zcopy_count, and add the external plugin hooks.

Why?

Enables SGL support in ucp_get_nbx over RC, as put_sgl_zcopy does for the write direction (#11677, #11735).

How?

Posts one RDMA READ WQE per element in a single doorbell, following put_sgl_zcopy.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

src/uct/ib/mlx5/rc/rc_mlx5_ep.c

Residual note (not a finding): op->length is unsigned (32-bit) while total is size_t, so a single SGL GET whose aggregate size exceeds 4 GB would truncate the read-credit release via uct_rc_ep_get_zcopy_completion_handler. Pre-existing limitation of the send_op length field, only reachable with non-default TX_NUM_GET_BYTES plus very large aggregate transfers; does not block this PR.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

Residual (non-blocking): the previously-noted op->length (unsigned) vs total (size_t) truncation for >4 GB aggregate SGL GET remains, same as put.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests roce on worker 1) · commit 92955a61

TL;DR: The roce on worker 1 job failed at compile time, not in tests: rc_mlx5_ep.c:215 declares size_t total; which is only consumed by UCT_TL_EP_STAT_OP() — a no-op when stats are disabled — so GCC's -Werror=unused-but-set-variable kills the build. Mark it UCS_V_UNUSED (as is already done for max_count two lines above).

Full analysis

Summary: make failed building src/uct/ib/mlx5/rc/rc_mlx5_ep.c with error: variable 'total' set but not used [-Werror=unused-but-set-variable], aborting the whole roce on worker 1 job before any test ran.

Root cause: In uct_rc_mlx5_base_ep_put_sgl_zcopy(), total is assigned from uct_rc_mlx5_base_ep_sgl_zcopy_post() (line 232) and its only reader is UCT_TL_EP_STAT_OP(&ep->super.super, PUT, ZCOPY, total) (line 237). In this build configuration statistics are disabled, so UCT_TL_EP_STAT_OP expands to nothing and total becomes set-but-unused. The build uses -Wall -Werror, so the warning is fatal. Note the neighbouring max_count, which has the same stats-only/assert-only usage pattern, is already declared size_t UCS_V_UNUSED max_count; (line 212) — the new variable just missed that annotation. The sibling uct_rc_mlx5_base_ep_get_sgl_zcopy() (line 312) is not affected because its total is also consumed by UCT_RC_RDMA_READ_POSTED(), which is real code in all configurations.

Evidence from the log:

.../src/uct/ib/mlx5/rc/rc_mlx5_ep.c: In function 'uct_rc_mlx5_base_ep_put_sgl_zcopy':
.../rc_mlx5_ep.c:215:12: error: variable 'total' set but not used [-Werror=unused-but-set-variable]
  215 |     size_t total;
cc1: all warnings being treated as errors
make[5]: *** [Makefile:920: rc/libuct_ib_mlx5_la-rc_mlx5_ep.lo] Error 1

Implicated commit: 92955a61 — "UCT/IB/MLX5/RC: Deduplicate put/get SGL zcopy WQE post path", Michal Shalev (the PR head commit; it introduced the total return value from the shared post helper)

File: src/uct/ib/mlx5/rc/rc_mlx5_ep.c:215 (assignment at :232, sole use at :237)

Suggested fix: Annotate the variable so it survives stats-disabled builds, matching the existing style in the same function:

-    size_t total;
+    size_t UCS_V_UNUSED total;

Verify locally with a stats-disabled configure (./configure --disable-stats ... && make) — or simply build the same way CI does — since a stats-enabled build will not reproduce this. Also worth double-checking any other new variable in this PR whose only consumer is a UCT_TL_EP_STAT_OP / ucs_assert style macro.

Related: PR #11763 (branch uct-mlx5-rc-sgl-get); builds on #11677 (put_sgl_zcopy) and commit fcb6b31a (get_sgl_zcopy).

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

🤖 Review complete — no issues found.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

Verified as correct (no action): Fence handling for GET: resolve_atomic_rkey=0 + fence_flag = uct_rc_ep_fm(..., atomic_fence_flag) matches the existing uct_rc_mlx5_ep_fence_get semantics (always direct rkey, atomic fence flag).

Verified as correct (no action): UCT_RC_RDMA_READ_POSTED/reads_available accounting and the min_zcopy/max_get_zcopy length bounds mirror the single-op get_zcopy path.

Verified as correct (no action): The refactor of PUT into uct_rc_mlx5_base_ep_sgl_zcopy_post preserves the original PUT behavior (fence passed as resolve_atomic_rkey, put_fence_flag).

Verified as correct (no action): iface_query_v2, the ops table wiring, and the plugin dispatch for ep_get_sgl_zcopy/max_get_sgl_zcopy_count are consistent with the existing PUT structure.

Residual (already noted in prior reviews, not re-raised): The op->length (unsigned) vs total (size_t) truncation for >4 GB aggregate SGL transfers remains for GET as it does for PUT.

[brminich]

seems we support sgl op without completion. Where do we return resources then when comp is NULL?

[michal-shalev]

seems we support sgl op without completion.

Yes, just like regular IOV.

Where do we return resources then when comp is NULL?

TX credits return via CQ polling, no op needed. Read credits need the op, so with comp == NULL they leak, same as regular get_zcopy.

I suggest to fix both in a follow-up rather than in this PR.

[michal-shalev]

Following @jeynman's comment, SGL GET now always creates a send op with a dedicated handler, so read credits are returned even when comp == NULL. The same leak in regular get_zcopy is still there.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[michal-shalev]

Done

[michal-shalev]

Done

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.

[michal-shalev]

Done

[svc-nvidia-pr-review]

🤖 Starting review — findings will be posted here when done.