bug fix: probe RDMA link rate via sysfs, survive probe failure - #734
bug fix: probe RDMA link rate via sysfs, survive probe failure#734Xuan-1998 wants to merge 1 commit into
Conversation
get_rdma_gbs() only knew how to ask ibstat for a CA named EP_NIC_NAME (default mlx5_0). EFA devices expose no umad CA, so on EFA hosts the probe returned 0 and get_theoretical_num_sms() divided by it -- any multi-node run without an explicit --num-sms crashed with ZeroDivisionError. Read /sys/class/infiniband/<nic>/ports/*/rate first, which works for every verbs provider, and fall back to ibstat for setups whose rate only shows there. When EP_NIC_NAME is unset and the default device is absent, pick the fastest device under /sys/class/infiniband instead of failing; an explicitly named NIC still fails loudly rather than guessing.
|
|
||
| # The un-overridden default may simply not exist on this fabric; an explicitly named NIC | ||
| # must not fall back silently | ||
| if 'EP_NIC_NAME' not in os.environ: |
There was a problem hiding this comment.
🟡 warning: 不要把显式 nic_name 回退到其他设备: 当调用方显式传入 get_rdma_gbs("missing-or-down-nic") 且未设置 EP_NIC_NAME 时，此条件仍会扫描全部设备并返回另一块 NIC 的速率。这违背了显式指定 NIC 不应猜测回退的约定，并可能使用错误带宽进行调优；应依据参数是否被省略来决定是否自动发现设备。
🤖 v6
| ports_dir = os.path.join('/sys/class/infiniband', nic_name, 'ports') | ||
| try: | ||
| for port in os.listdir(ports_dir): | ||
| with open(os.path.join(ports_dir, port, 'rate')) as f: |
There was a problem hiding this comment.
🔵 suggestion: 正则 r'\s*(\d+)\sGb/sec' 无法匹配带小数的速率（如老式 SDR 1X 的 "2.5 Gb/sec"），此类设备会被解析为 0。可考虑改用 r'\s(\d+(?:.\d+)?)\s*Gb/sec' 并用 float 解析。对现代 HPC 网卡影响可忽略，仅作健壮性建议。
🤖 v5
| gbs: the device's link rate in GB/s (0 if the device or its rate is unavailable). | ||
| """ | ||
| rate = 0 | ||
| ports_dir = os.path.join('/sys/class/infiniband', nic_name, 'ports') |
There was a problem hiding this comment.
🔵 suggestion: sysfs 的 ports//rate 在端口 state 为 DOWN 时也可能报告一个速率。在 EP_NIC_NAME 未设置的『选最快设备』分支里，可能选中一个链路未激活的设备。可选改进：同时读取 ports//state，仅统计 ACTIVE（"4: ACTIVE"）端口。
🤖 v5
| if gbs > 0: | ||
| return gbs | ||
|
|
||
| # The un-overridden default may simply not exist on this fabric; an explicitly named NIC |
There was a problem hiding this comment.
🔵 suggestion: 当所有探测（sysfs、最快设备扫描、ibstat）都失败时函数仍返回 0，调用方 get_theoretical_num_sms()（deep_ep/buffers/elastic.py:761 附近）在 num_rdma_ranks > 1 时仍可能触发 ZeroDivisionError。本 MR 已消除 EFA 场景的主要触发路径，但如果想彻底『survive probe failure』，建议在调用方对 rdma_gbs == 0 给出明确报错（提示设置 EP_NIC_NAME 或显式传入 --num-sms）。
🤖 v5
| devices = sorted(os.listdir('/sys/class/infiniband')) | ||
| except OSError: | ||
| devices = [] | ||
| gbs = max((_get_sysfs_rdma_gbs(device) for device in devices), default=0) |
There was a problem hiding this comment.
🟡 warning: 当 /sys/class/infiniband 下没有任何设备（或所有设备的 rate 都读不到）且后续 ibstat 也失败时，get_rdma_gbs() 仍会返回 0；而 deep_ep/buffers/elastic.py 的 get_theoretical_num_sms() 在 num_rdma_ranks > 1 时会执行 rdma_traffic / rdma_gbs，仍会触发 ZeroDivisionError。既然目标是 survive probe failure，建议在调用处对探测结果为 0 的情况做兜底（例如回退到保守默认带宽或给出明确报错），而不是继续除以 0。
🤖 v4p
|
|
||
| # The un-overridden default may simply not exist on this fabric; an explicitly named NIC | ||
| # must not fall back silently | ||
| if 'EP_NIC_NAME' not in os.environ: |
There was a problem hiding this comment.
🔵 suggestion: 自动猜测逻辑用 'EP_NIC_NAME' not in os.environ 判断“是否显式指定了网卡”，但函数参数 nic_name 也可以由调用方显式传入。例如未设置 EP_NIC_NAME 时调用 get_rdma_gbs('mlx5_0')，若 mlx5_0 不存在，代码仍会静默选择其他最快设备，与“显式指定网卡应 loudly fail”的说明不一致。建议用 sentinel 默认值（如 nic_name=None 时再取环境变量默认值）来区分默认值与显式传参。
🤖 v4p
🤖 ds-review-bot Code Reviewv6自动发现条件无法区分默认调用与显式函数参数，导致显式指定失败时静默使用无关设备的速率。 v5该 MR 修复了 EFA 主机上 get_rdma_gbs() 探测失败导致 get_theoretical_num_sms() 除零崩溃的问题。改动仅涉及 deep_ep/utils/envs.py（与描述一致，变更文件数为 1）：新增 _get_sysfs_rdma_gbs() 优先从 /sys/class/infiniband/<nic>/ports//rate 读取链路速率（对所有 verbs provider 通用，包括 EFA 的 rdmap 设备）；保留 ibstat 作为最终回退；当 EP_NIC_NAME 未显式设置且默认设备（mlx5_0）不存在时，扫描 /sys/class/infiniband 选取最快设备，而显式指定的 NIC 不会静默回退、仍会明确失败。已核对的行为要点：多端口 CA 取各端口速率的 max，与旧 ibstat 路径（只取第一个 Port 的 Rate）语义一致；『不静默回退』通过 'EP_NIC_NAME' in os.environ 判断实现，显式传参 nic_name 时同样不回退，符合描述；lru_cache 以 nic_name 为键、行为依赖进程内稳定的环境变量，无问题；OSError 均被捕获，防御性良好；文件头新增的 MIT/Amazon 版权声明为合规性改动，与功能无关。整体实现正确、与提交说明完全吻合，建议合并。以下评论均为非阻塞的改进建议。 v4p该 MR 旨在修复 RDMA 带宽探测在 EFA 等无 umad CA 设备上返回 0、进而导致 get_theoretical_num_sms 自动计算 SM 数时除零崩溃的问题：新增从 /sys/class/infiniband/<nic>/ports/*/rate 读取速率，保留 ibstat 作为回退，并在未设置 EP_NIC_NAME 且默认网卡缺失时自动选择 sysfs 下最快的 RDMA 设备。整体方向正确、实现简洁，但探测完全失败时仍会返回 0，且显式传入 nic_name 参数时仍可能被自动猜测，建议补齐这两个边界。 Files reviewed: 1 |
2 participants
get_rdma_gbs() only knew how to ask ibstat for a CA named EP_NIC_NAME (default mlx5_0). EFA devices expose no umad CA, so on EFA hosts the probe returned 0 and get_theoretical_num_sms() divided by it -- any multi-node run without an explicit --num-sms crashed with ZeroDivisionError.
Read /sys/class/infiniband//ports/*/rate first, which works for every verbs provider, and fall back to ibstat for setups whose rate only shows there. When EP_NIC_NAME is unset and the default device is absent, pick the fastest device under /sys/class/infiniband instead of failing; an explicitly named NIC still fails loudly rather than guessing.