UCT/IB/MLX5: support in progress error handler

[jeynmann]

What?

Allow an RC mlx5 error handler to return UCS_INPROGRESS so the transport does not immediately purge outstanding TX WQEs.

Why?

On endpoint failure, RC mlx5 currently always purges outstanding ops and returns both CQ and QP resources in one step. Failover needs those WQEs to stay posted so the UCP can inspect or recover them.

How?

Allows error handler return UCS_INPROGRESS to indicate that completions should not be purged imediately.
Split ep invalidate from #11684

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Tests BlueField on worker 3) · commit 2123627c

TL;DR: Two RC/mlx5 error-path gtests hung (3 min and 15 min of zero output, ending in the gtest watchdog Connection timed out - abort testing → SIGABRT/core), because the new "no completions" error-handling path in uct_rc_mlx5_iface_handle_failure() stops purging/completing outstanding TX ops, so flush(CANCEL) completions and UCP requests never complete. Fix: complete (or purge) the outstanding send/flush ops when entering the UCS_INPROGRESS/NO_COMPLETIONS state instead of leaving them pending forever.

Full analysis

Summary: shm_ib/test_ucp_peer_failure.force_close/0 failed after 540 s and rc_mlx5/uct_cancel_test.am_zcopy/0 hung 15 min in uct_cancel_test::flush_and_reconnect() until the test watchdog aborted the gtest binary (make: *** [Makefile:4713: test] Aborted (core dumped)).

Root cause: Evidence of a hang, not slowness: the log jumps 07:27:15 → 07:42:15 with no output while in test_flush.cc:767 (while (done.count != 1) progress();), and 07:19:53 → 07:22:53 → 07:25:54 (3-min "request did not complete on time" ticks) in force_close, followed by rc_ep.c:468 destroying txqp ... with uncompleted operation ... uct_rc_ep_send_op_completion_handler, an ucp_requests mpool leak and a leftover callbackq entry. All of these mean outstanding RC TX ops are never completed after a QP error. In the PR's new code, uct_rc_mlx5_iface_handle_failure() skips uct_rc_txqp_purge_outstanding() whenever the EP error handler returns UCS_INPROGRESS (sets UCT_RC_MLX5_EP_FLAG_NO_COMPLETIONS, src/uct/ib/mlx5/rc/rc_mlx5_iface.c:227-234) and also skips returning TX QP resources (:236-239), while the pre-existing early exit for ERR_HANDLER_INVOKED | FLUSH_CANCEL (:210-213) jumps past the purge too. Since every error CQE is funneled into handle_failure (uct_ib_mlx5_check_completion_with_err, src/uct/ib/mlx5/ib_mlx5.c:515-523), nothing ever invokes the flush-cancel completion added by uct_rc_txqp_add_flush_comp() (src/uct/ib/mlx5/rc/rc_mlx5_ep.c:773), so done.count never reaches 1 and UCP requests never complete.

Implicated commit: 2123627 "UCT/IB/MLX5: add support for no completions" (Zihao Zhao), building on 106bf44 "UCT/IB/MLX5: support err handle return inprogess"

File: src/uct/ib/mlx5/rc/rc_mlx5_iface.c:210-239 (with src/uct/ib/mlx5/rc/rc_mlx5_ep.c:726-775)

Suggested fix: In the UCS_INPROGRESS / UCT_RC_MLX5_EP_FLAG_NO_COMPLETIONS branch, do not leave the txqp outstanding queue untouched: at minimum complete the ops that can never receive a CQE — call uct_rc_txqp_purge_outstanding() (or a variant that only completes uct_rc_ep_flush_op_completion_handler / keepalive comps) for WQEs up to pi, and return the corresponding TX QP credits. Additionally, do not enter NO_COMPLETIONS mode when UCT_RC_EP_FLAG_FLUSH_CANCEL is set (the canceling flow must still complete), and make uct_rc_mlx5_base_ep_flush(UCT_FLUSH_FLAG_CANCEL) on an EP already in NO_COMPLETIONS state purge outstanding ops and complete the flush comp immediately rather than queueing a completion that no CQE will ever trigger. Also verify uct_rc_mlx5_ep_outstanding_purge() invokes the user completions of the purged ops so UCP requests are released.

Related: PR #11829 ("[DNM] UCT/IB/MLX5: support in progress error handler"), prior work PR #11668 ("UCT: outstanding purge implementation")

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

uct_rc_mlx5_ep_update_tx_qp_res runs on the error path of the purge, using a possibly-stale ft_ci.

ucs_status_t status = uct_ib_mlx5_ext_ep_outstanding_purge(tl_ep, params);
uct_rc_mlx5_ep_update_tx_qp_res(ep, ep->tx.wq.ft_ci);
if (status != UCS_OK) {
    ...
    return status;
}

uct_ep_outstanding_purge() is a public API and can be invoked on an ep that never went through the UCS_INPROGRESS failure path, so ft_ci may still be its reset value UINT16_MAX. update_tx_qp_res then computes available = bb_max - (prev_sw_pi - UINT16_MAX), which is bogus and will trip ucs_assert(available >= prev_available) (assert builds) or corrupt txqp->available (release builds). It is also called unconditionally even when the ext purge returned UCS_ERR_UNSUPPORTED/no plugin handled it. Suggested draft comment:

can we only update tx qp res when the ext purge actually handled the ep (status == UCS_OK) and the ep is in the NO_COMPLETIONS state? otherwise ft_ci is still UINT16_MAX and update_tx_qp_res computes a bad available.

[jeynmann]

It is intentional. uct_ep_outstanding_purge may partial successfully purge, when e.g. memory is insufficient. Updating resources only when status == UCS_OK would discard that committed progress and could cause double purge.

[svc-nvidia-pr-review]

Switch from the update_bb formula to incremental hw_ci - prev_hw_ci assumes strict monotonicity.

uint16_t bb_num = hw_ci - prev_hw_ci;
ucs_assertv(bb_num > 0, ...);
txwq->hw_ci = hw_ci;

The previous update_tx_res derived bb_num from prev_sw_pi/available, which was robust to repeated completions reporting the same counter. The new code requires every reported hw_ci/pi to strictly advance past the stored hw_ci. In handle_failure the flushed error CQEs feed pi into this function via out_update_tx_res, and this now also runs on the repeated-failure path (ERR_HANDLER_INVOKED/FLUSH_CANCEL -> goto out_update_tx_res). If a subsequent error CQE reports the same wqe_counter, bb_num == 0 trips the assert. Worth confirming that flushed error CQEs always carry a strictly increasing counter, otherwise this is a regression from the old formula.

[jeynmann]

It's strict monotonicity. HW should not generates 2 cqes for 1 wqe.

[svc-nvidia-pr-review]

UCT_RC_MLX5_EP_FLAG_NO_COMPLETIONS is set but never cleared. Once set on the first UCS_INPROGRESS failure, subsequent handle_failure calls on the same ep permanently skip uct_rc_txqp_purge_outstanding and update_tx_qp_res, while update_tx_cq_res keeps releasing CQ credits. Please confirm this is intended (the outstanding WQEs and their QP resources are handed off to the plugin for the life of the ep) and that no later CQE on this QP needs the purge/qp-res path.

[jeynmann]

It is intentional. Once the error handler returns UCS_INPROGRESS, UCP then decides when to purge, clean up, and destroy the endpoint. The endpoint does not return to normal completion processing.

[svc-nvidia-pr-review]

Minor: inconsistent range in the two log messages. rc_mlx5_iface.c L230 uses (ft_ci, sw_pi) while rc_mlx5_ep.c L796 uses (ft_ci, prev_sw_pi) for what reads like the same outstanding range. Pick one end consistently.

[svc-nvidia-pr-review]

Notes / no action: Making hw_ci and the new ft_ci always-compiled (dropping the #if UCS_ENABLE_ASSERT) is consistent since hw_ci is now functional, not debug-only; uct_ib_mlx5_txwq_update_bb unconditionally writing hw_ci is harmless for the UD/DC paths that still use it. VFS now exposing ft_ci/hw_ci unconditionally is fine. ep->flags = 0 is correctly initialized in UCS_CLASS_INIT_FUNC(uct_rc_mlx5_base_ep_t) before any consumer.

Test coverage: This is a behavioral change to the RC error/purge path. I did not find an added regression test in the diff. Since the new ep_outstanding_purge path depends on an external plugin, please state which CI job or existing error-injection test exercises the UCS_INPROGRESS failure -> plugin purge -> update_tx_qp_res flow, or add a focused test for the non-plugin ep_outstanding_purge call (finding 1) which is reachable without any plugin.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

blocker: uct_rc_mlx5_ep_outstanding_purge calls uct_rc_mlx5_ep_update_tx_qp_res(ep, ep->tx.wq.ft_ci) unconditionally, before the status != UCS_OK check.

• ep_outstanding_purge is reachable from the public uct_ep_outstanding_purge v2 API on any RC mlx5 EP, including EPs that never took the handle_failure UCS_INPROGRESS path. In that case ft_ci is still the reset sentinel UINT16_MAX, so update_tx_qp_res computes available = bb_max - (prev_sw_pi - 0xFFFF) (garbage), tripping ucs_assert(available >= prev_available) or corrupting txqp->available.
• When the plugin returns an error (e.g. UCS_ERR_UNSUPPORTED when no plugin is registered), nothing was drained, so releasing QP resources is also incorrect on that path.

Suggested fix — only release QP resources after a successful purge (and it belongs after the failure early-return):

ucs_status_t uct_rc_mlx5_ep_outstanding_purge(
        uct_ep_h tl_ep, const uct_ep_outstanding_purge_params_t *params)
{
    uct_rc_mlx5_base_ep_t *ep = ucs_derived_of(tl_ep, uct_rc_mlx5_base_ep_t);
    ucs_status_t status = uct_ib_mlx5_ext_ep_outstanding_purge(tl_ep, params);

    if (status != UCS_OK) {
        ucs_trace("ep %p purge outstanding range (%u, %u) status %s", ep,
                  ep->tx.wq.ft_ci, ep->tx.wq.sw_pi, ucs_status_string(status));
        return status;
    }

    uct_rc_mlx5_ep_update_tx_qp_res(ep, ep->tx.wq.ft_ci);
    ucs_debug("ep %p purge outstanding OK", ep);
    return UCS_OK;
}

Consider also gating this on ep->flags & UCT_RC_MLX5_EP_FLAG_NO_COMPLETIONS (and clearing it) so a purge that did not follow a deferred failure is a no-op for QP resources.

[jeynmann]

It is intentional. uct_ep_outstanding_purge may partial successfully purge, when e.g. memory is insufficient. Updating resources only when status == UCS_OK would discard that committed progress and could cause double purge.

[svc-nvidia-pr-review]

Test coverage: no regression test in the diff for the deferred UCS_INPROGRESS → plugin purge → update_tx_qp_res flow, and the non-plugin path (public API with unset ft_ci) is reachable without any plugin. Please add a focused test for the plugin-less ep_outstanding_purge call, or point to the CI/error-injection job that exercises it.

[svc-ucx]

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

TL;DR: The roce on worker 1 job failed in run_ucx_perftest_fault_tolerance because the helper ucx_drop_tool could not create an RDMA_TX steering flow table on mlx5_0 (FW syndrome 0x31ed04, "Remote I/O error") and exited immediately — so no traffic was ever blocked, no failover/reconfiguration happened, and the test's verify_active_devices for cfg#1 failed. This is an environment/test-harness problem (RoCE host swx-rain04 doesn't allow/support the tool's flow table), not a defect in the PR's failover code; the script must check the drop tool started successfully and skip/report instead of failing the PR.

Full analysis

Summary: contrib/test_jenkins.sh → run_ucx_perftest_fault_tolerance returned 1 with "Error: Failed to find protocol configuration count 1 in standard out" after /hpc/local/oss/ucx/fault_tolerance/ucx_drop_tool failed to install its flow table.

Root cause: At 11:49:40 the log shows:

• ucx_drop_tool.c:409 UCX ERROR failed to create TX RDMA flow table, syndrome 0x31ed04: Remote I/O error
• ucx_drop_tool.c:661 UCX ERROR failed to setup flow table for device 0: Input/output error

The drop tool therefore died right after being backgrounded (confirmed later by kill: (2675014) - No such process). Because traffic on mlx5_0 was never dropped, ucx_perftest kept running happily on both rails (the trailing [thread 0] lines show steady ~82 µs / 12 GB/s progress with no reconfiguration), so no second protocol-config block (| perftest self cfg#1 |) was ever printed. parse_active_devices then returned found=0 and the test failed.

The script has no check on the drop tool's success: line 549 launches ${drop_tool} ... & and immediately proceeds to sleep 10 + verify_active_devices, so any failure of the tool (unsupported FW/steering domain, missing privileges, RoCE-specific SL/TC matching not available on this host) is reported as a UCX failover regression. Note also that on a RoCE worker, matching on IB service level (-s 11) is not meaningful — only the traffic class/DSCP path is, which makes this job the most likely place for the tool's RDMA_TX flow-table setup to be rejected.

Implicated commit: [REDACTED:Hex High Entropy String] — "CI/FT: Adds fault tolerance CI test (#11570)", william-gallagher-nv (added run_ucx_perftest_fault_tolerance; PR head commit [REDACTED:Hex High Entropy String] on failover_uct_rcx_no_comp_impl is not implicated by the log)

File: contrib/test_jenkins.sh:549 (drop tool launch, unchecked) — failure surfaces at contrib/test_jenkins.sh:557 via parse_active_devices at contrib/test_jenkins.sh:430

Suggested fix:

1. Make the drop tool failure explicit and non-fatal in CI. Redirect its output to a file, verify the process is still alive and reported success before verifying failover, e.g.:

${drop_tool} -d ${dev} -s ${sl} -t ${tc} > drop_tool.${dev}.log 2>&1 &
drop_pid=$!
sleep 2
if ! kill -0 ${drop_pid} 2>/dev/null; then
    cat drop_tool.${dev}.log
    echo "Warning: drop tool failed on ${dev} (flow table not supported/permitted) - skipping FT test"
    kill_background_processes "${background_pids[@]}"
    return 0   # or 'continue' if other devices should still be tried
fi
background_pids+=(${drop_pid})

2. Gate the test on the transport/link layer: skip run_ucx_perftest_fault_tolerance on RoCE hosts (or when the device's link_layer is Ethernet) unless the drop tool is known to support DSCP/TC-only matching there, so the IB workers keep the coverage.
3. Separately, ask the CI/infra owner to check why mlx5_0 on swx-rain04 rejects the RDMA_TX flow table (FW syndrome 0x31ed04 — likely missing FW steering capability for the RDMA_TX domain or insufficient privileges in the agent container).
4. Re-run this PR job once the harness is fixed; the perftest itself showed no errors, so there is no evidence of a failover regression from commit [REDACTED:Hex High Entropy String].

Related: PR #11570 (added the fault tolerance CI test); PR #11829 (this build) — no existing issue found for syndrome 0x31ed04, worth opening one against the CI/FT test.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 3c9f03e0-83cc-4d07-8d6c-d0be882e8235 in the triage console for the audit trail.

[roiedanino]

Too many spaces (about 4 too many)

[roiedanino]

Should we gate here with NO_COMPLETIONS flag in the EP? otherwise ft_ci value would be UINT16_MAX
we should also clean NO_COMPLETIONS flag after that

[roiedanino]

So that API stays in EXT?

[jeynmann]

It can be removed in the PR for the purge implementation. In this PR it keeps the behavior the same as master. IMO, cleanup plugin call is not this PR’s responsibility. WDYT?

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

uct_rc_mlx5_ep_outstanding_purge asserts ep->no_comp on a public API path. This function is installed as the transport's public ep_outstanding_purge op (uct_ep_outstanding_purge). The API contract does not require the ep to have first entered the deferred-completion (no_comp) state. If a caller invokes uct_ep_outstanding_purge on an ep that never failed, ep->no_comp == 0: in debug build the ucs_assert(ep->no_comp) fires; in release build ep->tx.wq.ft_ci is still UINT16_MAX (its reset value) so uct_rc_mlx5_ep_update_tx_qp_res(ep, ft_ci) computes wrong tx-qp resources. Please guard this path (return an error, or early-out) when no_comp is not set, instead of asserting it.

[svc-nvidia-pr-review]

ft_ci passed to update_tx_qp_res on purge leaves the outstanding range's tx-qp credits unreclaimed. ft_ci is set in handle_failure to the hw_ci of the first outstanding WQE: ft_ci = prev_sw_pi - (bb_max - available). Feeding that same value back into available = bb_max - (prev_sw_pi - ft_ci) recomputes exactly the pre-failure available, so the purge returns zero tx-qp resources for the just-purged range. If the intent is to reclaim the purged WQEs, this looks like it should pass sw_pi (end of the range), not ft_ci. Can you confirm the intended value here, and whether the ep is expected to be destroyed right after so the unreclaimed credits don't matter?

[svc-nvidia-pr-review]

src/uct/ib/mlx5/rc/rc_mlx5.h

Minor: bool no_comp; is added to uct_rc_mlx5_base_ep_t outside the tx sub-struct. UCX generally uses uint8_t for such flags (see uct_rc_ep_t::flags); consider uint8_t no_comp; for consistency. Not blocking.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (Basic_compile Basic compile) · commit e428402f

TL;DR: The Basic compile job failed with -Werror=implicit-function-declaration in src/uct/ib/mlx5/rc/rc_mlx5_ep.c:825 — the new uct_rc_mlx5_ep_outstanding_purge() calls uct_ib_mlx5_ext_ep_outstanding_purge() without the prototype from uct/ib/mlx5/ib_mlx5_ext.h being visible; add that #include to rc_mlx5_ep.c (or rebase, since the current branch tip already has it at line 17).

Full analysis

Summary: Build of src/uct/ib/mlx5/rc/libuct_ib_mlx5_la-rc_mlx5_ep.lo aborted; make returned Error 1/2 and the job exited with code 2.

Root cause: Compiler output (log 02:12:31.83):

rc_mlx5_ep.c: In function 'uct_rc_mlx5_ep_outstanding_purge':
rc_mlx5_ep.c:825:14: error: implicit declaration of function
  'uct_ib_mlx5_ext_ep_outstanding_purge'; did you mean 'uct_rc_mlx5_ep_outstanding_purge'?
  [-Werror=implicit-function-declaration]
rc_mlx5_ep.c:825:14: error: nested extern declaration of
  'uct_ib_mlx5_ext_ep_outstanding_purge' [-Werror=nested-externs]
cc1: all warnings being treated as errors

The build uses -Wall -Werror -Werror-implicit-function-declaration -Wnested-externs, so a missing prototype is fatal. uct_ib_mlx5_ext_ep_outstanding_purge() is defined in src/uct/ib/mlx5/ib_mlx5_ext.c:265 and declared only in src/uct/ib/mlx5/ib_mlx5_ext.h:203-204 (both added by [REDACTED:Hex High Entropy String], "UCT: outstanding purge implementation (#11668)"). In the compiled snapshot of the PR/merge, rc_mlx5_ep.c invokes it (in the new uct_rc_mlx5_ep_outstanding_purge(), repo line 794) while ib_mlx5_ext.h was not included in that translation unit — the "nested extern declaration" diagnostic confirms no prototype was in scope. Note the compiled file is ~31 lines longer before the call site than the file I read, i.e. the built tree is the refs/pull/11829/merge result, so this is very likely a semantic merge/rebase artifact where the #include line was lost while the new call survived. Everything else in the log (configure, ucm/ucs/uct) is clean — this is a pure compile break, not an environment or timing issue.

Implicated commit: e428402 "UCT/IB/MLX5: address comments" (Zihao Zhao), on top of b27def8 "UCT/IB/MLX5: add support for no completions" (Zihao Zhao) — PR #11829

File: src/uct/ib/mlx5/rc/rc_mlx5_ep.c:825 (call site; repo copy line 794) — prototype at src/uct/ib/mlx5/ib_mlx5_ext.h:203

Suggested fix: Ensure rc_mlx5_ep.c includes the ext header before using the helper:

#include <uct/ib/mlx5/ib_mlx5_log.h>
#include <uct/ib/mlx5/ib_mlx5_ext.h>   /* <- required for
                                          uct_ib_mlx5_ext_ep_outstanding_purge() */

The branch tip already carries this include at rc_mlx5_ep.c:17, so rebase the PR onto current master (refs/pull/11829/merge regenerated) and re-run CI; verify the merge result still contains the include. To catch this class of failure earlier, build locally with the same flags (--enable-debug-style -Werror -Werror-implicit-function-declaration) after a merge with master.

Related: PR #11829 (this PR), PR #11668 (added uct_ib_mlx5_ext_ep_outstanding_purge), PR #11684 (related "no completion invalidate" work)

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id c02df64c-c033-4c12-ad37-6adfab070336 in the triage console for the audit trail.