UCP/WIREUP: Add a token trailer and an ACK message to the lane address exchange

[evgeny-leksikov]

What?

Append an optional token trailer to the LANES_ADDR wireup messages and add the
LANES_ADDR_ACK message which closes the exchange. The trailer is framing only:
its token sections are present but empty, and no token is queried or consumed yet.

Why?

Groundwork for hardware PSN fault tolerance. Purging a failed UCT lane requires the
PSN tokens of both directions, which have to travel with the lane addresses that
recovery already re-exchanges. Splitting the wire format out of the feature keeps the
protocol change reviewable on its own and lets the token query, per-lane storage and
their consumer land in a follow-up.

How?

Recovery today is a two-legged exchange over the AM lane. The endpoint which detects
a lane failure sends LANES_ADDR_REQUEST, packing the addresses of the lanes it
rebuilt in provided_lane_map; the peer replies with LANES_ADDR_REPLY, packing the
addresses of the lanes it managed to rebuild in turn.

The trailer goes between the fixed ucp_wireup_msg_lanes_info_t, which UCX 1.22
already ships, and the packed addresses:

[ucp_wireup_msg_t][lanes_info][request_id][TX lengths][RX lengths][addresses]

Each token section holds one length byte per lane followed by the tokens themselves,
so a lane which has no token costs one zero byte. The sections take their lanes from
the maps already in lanes_info rather than from maps of their own: the TX section
belongs to provided_lane_map, the lanes the message carries addresses for, and the
RX section to requested_lane_map, redefined as the lanes the message answers. The
addresses stay last, so they need no length of their own.

Both peers decide on the trailer from the peer release version in
ucp_ep_config_key_t::dst_version, gated by UCP_WIREUP_ADDR_TOKEN_MIN_DST_VERSION,
so nothing is added to the fixed header and older peers keep parsing the addresses
where they are today. request_id carries the recovery generation, echoed by the
peer, for the follow-up to tell answers of different rounds apart.

Why a third leg: an RX token is derived from the TX token of the remote side, so
each peer can only produce the RX tokens of the other one. The request carries the
initiator's TX tokens, and the reply carries both the responder's TX tokens and the
RX tokens it derived for the initiator. Nothing has yet delivered the RX tokens the
initiator derives from the reply, which is what LANES_ADDR_ACK is for. It exists
only to carry the trailer, so it is withheld from peers below the minimum version.

Testing: gtest coverage of the section parser, including truncated sections which the
empty sections of the wire itself never produce, plus the fault-tolerance suite on
ConnectX-7 (test_ucp_fault_tolerance and test_ucp_wireup, no failures).

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

The comment states request_id exists 'so that answers of an older round are recognized', but nothing compares the received request_id against the stored one; stale-reply recognition is not actually implemented.

[svc-nvidia-pr-review]

Minor: address_length narrowing. tokens_info->address_length is uint32_t but assigned from size_t address_length. Not a practical issue, but a latent narrowing worth a comment or ucs_assert.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

UCP_WIREUP_ADDR_TOKEN_MIN_DST_VERSION is 23 but ucx_ver_minor is 24. If a released v23 peer already understands LANES_ADDR without the trailer, gating at >= 23 makes us append a trailer it can't parse and it will unpack the addresses from the wrong offset. Should this be 24 (the first version that adds the trailer)?

[svc-nvidia-pr-review]

Coverage note: The gtest (test_ucp_wireup_token_section) only exercises ucp_wireup_skip_token_section in isolation. There is no end-to-end test that a v24<->v24 recovery exchange round-trips the trailer, nor a mixed-version test confirming an older peer still parses a trailer-less LANES_ADDR. The mixed-version path is the one the version gate above protects, so if practical it would be worth confirming via an existing recovery/fault-tolerance job.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (WireCompat Build on althca) · commit a4d85e49

TL;DR: The WireCompat Build on althca job failed to compile, not to test: src/ucp/wireup/wireup.c triggers -Werror=maybe-uninitialized on the local lanes_info pointer in ucp_wireup_msg_handler. Fix by initializing lanes_info = NULL at its declaration (line 1314).

Full analysis

Summary: make aborted with wireup/wireup.c:1373:44: error: 'lanes_info' may be used uninitialized in this function [-Werror=maybe-uninitialized] while building wireup/libucp_la-wireup.lo; cc1: all warnings being treated as errors → make[2]: *** [wireup/libucp_la-wireup.lo] Error 1.

Root cause: In ucp_wireup_msg_handler, const ucp_wireup_msg_lanes_info_t *lanes_info; (wireup.c:1314) is declared without an initializer and is only ever assigned indirectly, via the out-parameter of ucp_wireup_parse_lanes_addr(), inside the guarded block if (ucp_wireup_msg_is_lanes_addr(msg->type)) (lines 1336–1343). It is then read at lines 1370 and 1373 under the separate conditions msg->type == UCP_WIREUP_MSG_LANES_ADDR_REQUEST / ..._REPLY.

The code is functionally correct — ucp_wireup_msg_is_lanes_addr() is true for exactly the LANES_ADDR_REQUEST/REPLY/ACK types, so the pointer is always set before use — but GCC's flow analysis at -O3 cannot correlate the predicate ucp_wireup_msg_is_lanes_addr(msg->type) with the later equality tests on msg->type, especially since the value is written through a pointer in an inlined static helper. The compiler on the althca agent is stricter/older than on the other build nodes, so this warning only fires there. The build uses -O3 -g -Wall -Werror (see the configure summary in the log), which turns the diagnostic into a hard failure.

Note the helper also has a real (though currently unreachable) hazard that reinforces the warning: the truncation check at wireup.c:1251–1255 returns UCS_ERR_MESSAGE_TRUNCATED before *lanes_info_p is assigned at line 1261, so on that path the caller's variable genuinely stays uninitialized. The caller does goto out on error, so it isn't dereferenced today, but the pattern is fragile.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/WIREUP: Add a token trailer and an ACK message to the lane address exchange", Evgeny Leksikov (2026-08-26). This commit introduced ucp_wireup_parse_lanes_addr() and moved the lanes_info assignment out of the handler into an out-parameter, which is what defeats GCC's initialization analysis. The PR head commit is [REDACTED:Hex High Entropy String] on pr-wireup-tokens-framing (PR #11843).

File: src/ucp/wireup/wireup.c:1314 (declaration) — error reported at src/ucp/wireup/wireup.c:1373; secondary issue at src/ucp/wireup/wireup.c:1251-1261.

Suggested fix: Initialize the pointer at its declaration so no code path can leave it indeterminate:

const ucp_wireup_msg_lanes_info_t *lanes_info = NULL;

This is a one-line, zero-cost change that silences the diagnostic on every compiler without restructuring the control flow. Two recommended hardening follow-ups in the same patch:

1. Move *lanes_info_p = lanes_info; (and *request_id_p = 0;) in ucp_wireup_parse_lanes_addr to before the truncation check at line 1251, or explicitly set *lanes_info_p = NULL on that early-return path, so the helper never returns leaving the caller's pointer untouched.
2. Optionally add ucs_assert(lanes_info != NULL); at the top of the LANES_ADDR_REQUEST/REPLY branches (lines 1369–1374) to document and check the invariant that ucp_wireup_msg_is_lanes_addr() guarantees.

Avoid "fixing" this by dropping -Werror or adding a #pragma GCC diagnostic ignored — the warning is pointing at a real initialization gap in the helper's error path.

Related: PR #11843 (this build). Prior work in the same series that introduced the LANES_ADDR exchange and its handlers: 6c7c8ff9 (#11545), 9db2e488 (#11562), db208ee4 (#11563). No existing issue matches the lanes_info may be used uninitialized signature.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 312b23c3-63e1-4a22-80fe-6c58d90b9571 in the triage console for the audit trail.

[svc-nvidia-pr-review]

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

[svc-nvidia-pr-review]

(minor) The ucs_assert(ep != NULL) moved earlier, dropping the msg->dst_ep_id != UCS_PTR_MAP_KEY_INVALID check. The per-type branches previously asserted this; now the only guard before ucp_wireup_parse_lanes_addr(ep, ...) dereferences ep is ucs_assert(ep != NULL). Release-build behavior is unchanged (ep was already dereferenced downstream), so not a regression, just slightly weaker validation. Fine to leave as is.

[svc-nvidia-pr-review]

src/ucp/wireup/wireup.c

(question) Trailer presence relies on symmetric dst_version: both peers independently decide whether the token trailer is present using their own ucp_ep_config(ep)->key.dst_version (ucp_wireup_ep_supports_tokens). If the two ends hold asymmetric config views of the peer release version, the sender may add a trailer the receiver doesn't expect (or vice versa), causing the receiver to drop the message or misparse addresses. For recovery both directions are established, so dst_version should be consistent — but please confirm key.dst_version is guaranteed symmetric on both peers when LANES_ADDR messages flow. If yes, a note in the parse function would help; if not, this could silently stall recovery.

No test/coverage gap for the parser itself; the runtime path (real recovery with tokens-capable peers exchanging ACKs) is exercised only by the existing recovery/fault-tolerance suites — worth confirming one of those runs with dst_version >= 24 peers so the ACK path is actually hit, since the trailer is version-gated.

[svc-ucx]

🤖 CI Triage Agent — UCX PR (jucx JUCX Test aarch64 java11) · commit 06585cb5

TL;DR: The aarch64 java11 job failed in the "Build UCX" step, not in any Java test: src/ucp/wireup/wireup.c fails to compile with -Werror=maybe-uninitialized because lanes_info is only assigned inside a conditional block. Initialize it to NULL at declaration (and/or assert non-NULL at use).

Full analysis

Summary: make aborted while compiling wireup/libucp_la-wireup.lo — GCC (aarch64) reported ‘lanes_info’ may be used uninitialized in this function [-Werror=maybe-uninitialized], so the JUCX aarch64/java11 job never got to run tests.

Root cause: In ucp_wireup_msg_handler(), const ucp_wireup_msg_lanes_info_t *lanes_info; (line 1315) is declared without an initializer and is only written by ucp_wireup_parse_lanes_addr() inside if (ucp_wireup_msg_is_lanes_addr(msg->type)) { ... } (lines 1337–1344). It is later read unconditionally in the UCP_WIREUP_MSG_LANES_ADDR_REQUEST / ..._REPLY branches (lines 1371 and 1374). The invariant "message type is LANES_ADDR_REQUEST/REPLY ⇒ ucp_wireup_msg_is_lanes_addr() was true" is real, but GCC cannot prove it across the helper predicate, so it emits the warning — and this build uses -Werror, turning it into a hard error. (The same pattern applies to request_id, which is initialized to 0 at line 1314 and therefore does not warn — evidence the omission for lanes_info was an oversight.)

This is a new-code regression from the PR itself: the refactor that moved lanes-info parsing into ucp_wireup_parse_lanes_addr() with an out-parameter (**lanes_info_p) is what broke GCC's ability to see the definite assignment. It is compiler/arch-sensitive, which is why only the aarch64 GCC leg tripped on it.

Implicated commit: [REDACTED:Hex High Entropy String] — "UCP/WIREUP: Add a token trailer and an ACK message to the lane address exchange", Evgeny Leksikov (introduced ucp_wireup_parse_lanes_addr() and the out-param indirection); still present at PR head 06585cb5 — "UCP/WIREUP: Gate LANES_ADDR token trailers on release 1.24", same author.

File: src/ucp/wireup/wireup.c:1315 (declaration); error reported at src/ucp/wireup/wireup.c:1371, same issue at :1374

Suggested fix: Initialize the pointer at its declaration so the definite-assignment analysis is trivially satisfied:

    const ucp_wireup_msg_lanes_info_t *lanes_info = NULL;

Optionally add ucs_assert(lanes_info != NULL); at the top of the two LANES_ADDR branches (lines 1371/1374) to keep the real invariant documented and checked in debug builds, rather than silently relying on NULL. Avoid "fixing" this by relaxing -Werror or the warning flag — the diagnostic is a legitimate false positive but the one-line initializer is the cheap, standard remedy used elsewhere in this same function (request_id = 0).

Related: PR #11843 (#11843) — the failure is self-contained in this PR; no pre-existing issue found.

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 234cdd97-b301-4aa8-9e52-0c15496ee943 in the triage console for the audit trail.