Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 2 · Low: 0
- [Medium] `rss_multiqueue()` in
tools/testing/selftests/drivers/net/hw/iou-zcrx.py claims to "exercise…
- [Medium] The readiness handshake used by the Python driver no longer
implies that the receiver is armed.
--
selftests: net: add rss_multiqueue test variant to iou-zcrx
Adds a new rss_multiqueue variant to the iou-zcrx Python test which builds
a two-queue RSS context, steers the tcp6 flow into it, and runs the
iou-zcrx helper with two receive threads. The existing test_zcrx variants
now also pass an explicit thread count of one.
> diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> index b7a225fe4beae..c833535d8a03c 100755
> --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py
> @@ -30,6 +30,13 @@ def create_rss_ctx(cfg):
> return int(values)
>
>
> +def create_rss_ctx_multi(cfg, start, count):
> + """Create an RSS context spanning count queues from start, return its
> ID."""
> + output = ethtool(f"-X {cfg.ifname} context new start {start} equal
> {count}").stdout
> + values = re.search(r'New RSS context is (\d+)', output).group(1)
> + return int(values)
> +
> +
[ ... ]
> @@ -127,17 +134,56 @@ def _require_ntuple(cfg):
> defer(ethtool, f"-K {cfg.ifname} ntuple-filters off")
>
>
> +def rss_multiqueue(cfg):
> + """Steer the test flow to a multi-queue RSS context for multi-thread
> zcrx."""
[ ... ]
> + cfg.num_threads = 2
> + cfg.target = channels - cfg.num_threads
> + ethtool(f"-X {cfg.ifname} equal {cfg.target}")
> + defer(ethtool, f"-X {cfg.ifname} default")
> +
> + rss_ctx_id = create_rss_ctx_multi(cfg, cfg.target, cfg.num_threads)
> + defer(ethtool, f"-X {cfg.ifname} delete context {rss_ctx_id}")
> +
> + flow_rule_id = set_flow_rule_rss(cfg, rss_ctx_id)
> + defer(ethtool, f"-N {cfg.ifname} delete {flow_rule_id}")
[Severity: Medium]
Can this variant report a pass when only one of the two queues carried
zcrx traffic?
The RSS context spans queues [cfg.target, cfg.target + 1] and the flow rule
matches only flow-type tcp6 dst-port, so all connections share the same
addresses and destination port. The variant never configures or checks
ethtool -n rx-flow-hash tcp6, so on a device whose tcp6 hash covers only
addresses, every connection lands on the same queue. Even with a full
4-tuple hash, the four flows over a two-entry indirection table all collide
on a single queue roughly one run in eight.
On the helper side, run_server() distributes accepted connections purely by
the hardware-selected napi id:
tools/testing/selftests/drivers/net/hw/iou-zcrx.c:run_server() {
total_conns = cfg_num_threads * cfg_num_threads;
while (accepted < total_conns) {
int idx = 0;
connfd = accept(fd, NULL, NULL);
...
if (cfg_num_threads > 1)
idx = find_thread_by_conn(ctxs, connfd);
...
}
and a worker that ends up with no connections exits successfully:
tools/testing/selftests/drivers/net/hw/iou-zcrx.c:server_worker() {
...
while (ctx->nr_conns > 0 && gettimeofday_ms() < tstop)
server_loop(ctx);
if (ctx->nr_conns != 0)
error(1, 0, "test failed: %d connections incomplete",
ctx->nr_conns);
...
}
With nr_conns == 0 the loop and the check are both skipped, the process
exits 0, and rss_multiqueue passes while behaving exactly like the existing
single-queue rss variant.
Would it make sense to require L4 tcp6 hashing, fail when a worker in
multi-thread mode gets zero connections or zero bytes, and/or verify
io-uring page pool activity for both cfg.target and cfg.target + 1 from
Python? check_iou_rx_buf_len() and cfg.netnl.page_pool_get() already give
the per-queue view needed for the last part.
> @ksft_variants([
> KsftNamedVariant("single", single),
> KsftNamedVariant("rss", rss),
> + KsftNamedVariant("rss_multiqueue", rss_multiqueue),
> ])
> def test_zcrx(cfg, setup) -> None:
> cfg.require_ipver('6')
> _require_ntuple(cfg)
>
> + cfg.num_threads = 1
> +
> setup(cfg)
> - rx_cmd = f"{cfg.bin_local} -s -p {cfg.port} -i {cfg.ifname} -q
> {cfg.target}"
> - tx_cmd = f"{cfg.bin_remote} -c -h {cfg.addr_v['6']} -p {cfg.port} -l
> 12840"
> +
> + rx_cmd = (f"{cfg.bin_local} -s -p {cfg.port} -i {cfg.ifname} "
> + f"-q {cfg.target} -t {cfg.num_threads}")
> + tx_cmd = (f"{cfg.bin_remote} -c -h {cfg.addr_v['6']} -p {cfg.port} "
> + f"-l 12840 -t {cfg.num_threads}")
> with bkg(rx_cmd, exit_wait=True):
> wait_port_listen(cfg.port, proto="tcp")
> cmd(tx_cmd, host=cfg.remote)
[Severity: Medium]
Does wait_port_listen() still mean the receiver is armed once every
test_zcrx variant goes through the threaded server path?
wait_port_listen() in tools/testing/selftests/net/lib/py/utils.py only
polls /proc/net/tcp for TCP state 0A (LISTEN). In the threaded server the
listen() now happens before the setup barrier:
tools/testing/selftests/drivers/net/hw/iou-zcrx.c:run_server() {
...
/* pthread_create() loop for the workers */
...
if (listen(fd, 1024) < 0)
error(1, 0, "listen()");
pthread_barrier_wait(&setup_done);
...
}
while the worker only reaches that barrier after the 32 MB mmap and
io_uring_register_ifq() done by setup_zcrx():
tools/testing/selftests/drivers/net/hw/iou-zcrx.c:server_worker() {
io_uring_queue_init_params(512, &ctx->ring, ¶ms);
setup_zcrx(ctx);
if (cfg_dry_run)
return NULL;
pthread_barrier_wait(ctx->setup_done);
...
}
So the port is observable as LISTEN while the zcrx ifq is not yet
registered, and cmd(tx_cmd, host=cfg.remote) can start streaming into that
window.
Two consequences seem possible. Bytes arriving before
io_uring_register_ifq() land in ordinary driver pages and
io_zcrx_recv_frag() falls back to io_zcrx_copy_frag(), so the payload check
still succeeds while the zero-copy path is not exercised.
Also, test_zcrx_large_chunks() calls check_iou_rx_buf_len() right after
wait_port_listen(), which raises when the pool is not there yet:
tools/testing/selftests/drivers/net/hw/iou-zcrx.py:check_iou_rx_buf_len() {
...
if len(pools) != 1:
raise Exception(f"Expected 1 io-uring page pool, found {len(pools)}")
...
}
Would moving listen(fd, 1024) to after pthread_barrier_wait(&setup_done)
restore the previous ordering, where the port only became observable once
zcrx registration had completed?
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831-iou-zcrx-v6-0-02d1d012f0fd%40fastmail.com
