Skip to content

Commit 47bf541

Browse files
Peng Chenmeta-codesync[bot]
authored andcommitted
comms/uniflow/benchmarks: say what --tcp-iface does, and does not, do (meta-pytorch#3900)
Summary: Pull Request resolved: meta-pytorch#3900 Three TCP flags overlap enough to be mistaken for one another, and the one easiest to misread had no comment at all: - `--tcp-iface` selects the local address to bind and advertise. It does not choose the egress NIC -- routing does. On a host where several NICs share a route, traffic can leave via one device no matter what this names. - `--tcp-bind-dev` is a bool, not a device name. It promotes whatever `--tcp-iface` already names into an `SO_BINDTODEVICE` pin. - `--tcp-bind-devs` is the multi-device list, and wins over `--tcp-bind-dev` with no warning when both are passed. The help text called `--tcp-iface` the "front-end interface for the TCP transport", which reads like it picks the NIC. This documents the address-vs-device split on the field and in `--help`, and records on `tcpBindDevList` -- where the precedence is implemented -- that `--tcp-iface` keeps selecting the source address when striping, so it wants to name one of the bound devices. Pointing it at an unbound NIC advertises an address on a device carrying no lanes: legal, and confusing. Nothing is added for the parts already covered -- the `--tcp-bind-dev` field comment already explains the routing trap, and the `--tcp-bind-devs` help already covers `i%count`, one listener per device, and the peer-agreement constraints. Restating those would only create places to drift. Comments and help text only; no behaviour change. Differential Revision: D117640557
1 parent 0a21273 commit 47bf541

1 file changed

Lines changed: 10 additions & 1 deletion

File tree

comms/uniflow/benchmarks/main.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ struct CliOptions {
6363
size_t slabSize{0};
6464
int slabNum{0};
6565
std::vector<std::string> rdmaDevices;
66+
// Selects which local IPv6 address the TCP transport binds and advertises,
67+
// and nothing else. Which NIC actually carries the bytes is a separate axis
68+
// -- see --tcp-bind-dev below.
6669
std::string tcpIface{"eth2"};
6770
// SO_SNDBUF/SO_RCVBUF for the TCP data connection. Defaults to the
6871
// TcpSocketConfig default; 0 leaves the kernel's sizing alone.
@@ -96,6 +99,10 @@ std::vector<int> parseIntList(const std::string& s) {
9699

97100
// --tcp-bind-devs wins over --tcp-bind-dev, which is just the single-device
98101
// shorthand for whatever --tcp-iface names. Empty means no device binding.
102+
//
103+
// --tcp-iface keeps selecting the source address either way, so when striping
104+
// it wants to name one of the bound devices: pointing it at an unbound NIC
105+
// advertises an address on a device carrying no lanes.
99106
std::vector<std::string> tcpBindDevList(const CliOptions& opts) {
100107
std::vector<std::string> devices;
101108
if (!opts.tcpBindDevs.empty()) {
@@ -175,7 +182,9 @@ void printUsage(const char* prog) {
175182
<< " --output <path> CSV output file path\n"
176183
<< " --format <fmt> table|csv|both (default: table)\n"
177184
<< " --rdma-devices <list> Comma-separated RDMA device names (default: auto-discover)\n"
178-
<< " --tcp-iface <name> Front-end interface for the TCP transport (default: eth2)\n"
185+
<< " --tcp-iface <name> Interface whose address the TCP transport binds and\n"
186+
<< " advertises (default: eth2). Selects the source address\n"
187+
<< " only -- see --tcp-bind-dev to pin the egress NIC\n"
179188
<< " --tcp-sockbuf <bytes> TCP data-connection SO_SNDBUF/SO_RCVBUF (default: 0,\n"
180189
<< " which leaves it unset so the kernel autotunes the\n"
181190
<< " window; setting it explicitly disables autotuning)\n"

0 commit comments

Comments
 (0)