fix (client): pre-resolve master address in LaunchTransferBench

[nileshnegi]

Motivation

The host list was forwarded verbatim as TB_MASTER_ADDR, so an ssh_config alias that the local SSH client understands would fail getaddrinfo() on the workers, leaving rank 0 waiting on connections that never arrive.

Technical Details

Expand the entry via ssh -G and prefer a literal IPv4, since workers resolve the master address remotely and only over AF_INET.

Test Plan

Test Result

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

[Copilot]

🟡 Not ready to approve

The new TB_MASTER_ADDR interpolation into remote SSH commands should be shell-quoted to avoid remote command parsing issues if the resolved value contains characters that need escaping.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR improves multi-node launches by ensuring TB_MASTER_ADDR is a worker-resolvable IPv4 address rather than an SSH-only alias, preventing workers from failing getaddrinfo() and leaving rank 0 waiting indefinitely.

Changes:

• Add resolve_master_addr() to expand SSH aliases via ssh -G and prefer a literal IPv4 via getent ahostsv4.
• Use the resolved master_addr for TB_MASTER_ADDR when launching worker ranks.
• Pass TB_MASTER_ADDR to the master rank invocation as well, aligning the launch environment across ranks.

File summaries

File	Description
LaunchTransferBench.sh	Pre-resolves the master address (SSH alias → hostname → IPv4) and propagates it via TB_MASTER_ADDR to all ranks.

Review details

Suppressed comments (1)

LaunchTransferBench.sh:242

• Same quoting issue for rank 0 launch: TB_MASTER_ADDR is embedded into the remote command unquoted, which can break parsing if it contains whitespace/metacharacters. Quote it consistently with the worker command.

master_cmd="TB_NUM_RANKS=$num_ranks TB_RANK=0 TB_SINGLE_LOG=1 TB_MASTER_ADDR=$master_addr $env_string '$transferbench_path'$tb_args_escaped"

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

TB_MASTER_ADDR is unquoted in the master ssh command string, which can lead to remote-shell misparsing/expansion when the resolver falls back to a non-IPv4 value.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

LaunchTransferBench.sh:242

• In master_cmd, TB_MASTER_ADDR is injected unquoted, while the worker command safely quotes it. If resolve_master_addr falls back to a non-IPv4 hostname (or any value containing glob chars/spaces), the remote shell can misparse/expand it. Quote it consistently to make the remote invocation robust.

master_cmd="TB_NUM_RANKS=$num_ranks TB_RANK=0 TB_SINGLE_LOG=1 TB_MASTER_ADDR=$master_addr $env_string '$transferbench_path'$tb_args_escaped"

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The new address-resolution and SSH invocation paths need small correctness and hardening fixes (unusable IPv4 handling and ssh -- option-termination) to avoid hangs and option injection risks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

LaunchTransferBench.sh:236

• As with ssh -G, ssh treats a destination starting with - as an option unless you pass --. Add -- before $worker_host to avoid option injection from the hosts list.

    worker_cmd="TB_NUM_RANKS=$num_ranks TB_RANK=$rank TB_SINGLE_LOG=1 TB_MASTER_ADDR='$master_addr' $env_string '$transferbench_path'$tb_args_escaped"
    ssh -q -o LogLevel=ERROR "$worker_host" "$worker_cmd" >/dev/null 2>&1 &

LaunchTransferBench.sh:243

• Add -- before $master_host for the ssh invocation so a host entry starting with - can’t be parsed as additional SSH options.

master_cmd="TB_NUM_RANKS=$num_ranks TB_RANK=0 TB_SINGLE_LOG=1 TB_MASTER_ADDR='$master_addr' $env_string '$transferbench_path'$tb_args_escaped"
if ! ssh -q -o LogLevel=ERROR "$master_host" "$master_cmd"; then

LaunchTransferBench.sh:213

• If getent ahostsv4 resolves the master host to a loopback/unspecified IPv4 (e.g. 127.x or 0.0.0.0), workers will never be able to connect. Treat those IPv4s as unusable and fall back to the post-ssh -G hostname with a warning.

    if [[ ! "$addr" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
        ipv4=$(getent ahostsv4 "$addr" 2>/dev/null | awk 'NR==1{print $1}')
        if [[ -n "$ipv4" ]]; then
            addr="$ipv4"
        else

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Not ready to approve

The current resolution/propagation logic can select unusable IPv4s (e.g., loopback) and also forces TB_MASTER_ADDR on rank 0, overriding TransferBench’s safer on-host IPv4 detection in a way that can break connectivity on multi-homed systems.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

LaunchTransferBench.sh:242

• TB_MASTER_ADDR is now being set for the rank-0 (master) process as well. In TransferBench, rank 0 binds to INADDR_ANY and, when TB_MASTER_ADDR is unset, it auto-detects a primary IPv4 on the master host (see src/header/TransferBench.hpp around DetectPrimaryIpv4). For multi-homed hosts or environments where the local launcher’s DNS differs from the master’s routing, pre-resolving on the launch machine and forcing TB_MASTER_ADDR on rank 0 can cause the master to advertise an address that workers can’t reach. Only worker ranks require TB_MASTER_ADDR; consider leaving it unset for rank 0 so it can detect the correct address locally.

master_cmd="TB_NUM_RANKS=$num_ranks TB_RANK=0 TB_SINGLE_LOG=1 TB_MASTER_ADDR='$master_addr' $env_string '$transferbench_path'$tb_args_escaped"

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.