On Wed, Sep 02, 2026 at 11:38:17PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider:
>
[...]
> > @@ -356,29 +444,68 @@ static void run_server(void)
> > if (ret < 0)
> > error(1, 0, "bind()");
> >
> > - flags |= IORING_SETUP_COOP_TASKRUN;
> > - flags |= IORING_SETUP_SINGLE_ISSUER;
> > - flags |= IORING_SETUP_DEFER_TASKRUN;
> > - flags |= IORING_SETUP_SUBMIT_ALL;
> > - flags |= IORING_SETUP_CQE32;
> > + pthread_barrier_init(&setup_done, NULL, cfg_num_threads + 1);
> > + pthread_barrier_init(&dispatch_done, NULL, cfg_num_threads + 1);
> > +
> > + for (i = 0; i < cfg_num_threads; i++) {
> > + ctxs[i].queue_id = cfg_queue_id + i;
> > + ctxs[i].setup_done = &setup_done;
> > + ctxs[i].dispatch_done = &dispatch_done;
> > + }
> >
> > - io_uring_queue_init(512, &ctx.ring, flags);
> > + for (i = 0; i < cfg_num_threads; i++) {
> > + ret = pthread_create(&threads[i], NULL,
> > + server_worker, &ctxs[i]);
> > + if (ret)
> > + error(1, ret, "pthread_create()");
> > + }
> >
> > - setup_zcrx(&ctx);
> > if (cfg_dry_run)
> > - return;
> > + goto join;
> >
> > if (listen(fd, 1024) < 0)
> > error(1, 0, "listen()");
> >
> > - add_accept(&ctx, fd);
> > + pthread_barrier_wait(&setup_done);
>
> [Severity: High]
> Is the listen() intentionally moved before the workers have registered
> their ifqs? The main thread creates the threads, calls listen(), and only
> then waits on setup_done, while the workers reach that barrier after
> io_uring_queue_init_params() plus setup_zcrx(), i.e. after
> io_uring_register_ifq() has restarted the device RX queue.
>
> Before the patch the sequence was serialized in one thread:
>
> io_uring_queue_init(512, &ctx.ring, flags);
> setup_zcrx(&ctx);
> ...
> listen(fd, 1024);
>
> so the port only became visible once the memory provider was installed.
>
> Two existing tests depend on that ordering. wait_port_listen() in
> net/lib/py/utils.py polls every 5 ms, so it can now return while
> registration is still in flight:
>
> iou-zcrx.py:test_zcrx_large_chunks() {
> wait_port_listen(cfg.port, proto="tcp")
>
> check_iou_rx_buf_len(cfg, rx_buf_len)
> }
>
> check_iou_rx_buf_len() raises "Expected 1 io-uring page pool, found 0",
> and nk_qlease.py:test_destroy() does queue_get() plus
> ksft_in("io-uring", queue_info) immediately after wait_port_listen() with
> no sleep.
>
> Beyond the harness races, data delivered into the queue before the
> provider exists is serviced by the copy fallback rather than zero copy,
> and the queue restart during registration can drop in-flight frames.
>
> Would moving listen() below pthread_barrier_wait(&setup_done) restore the
> previous guarantee?
Just a heads up that I tested this on BRCM and did find this listen()
ordering issue to cause genuine (but flaky) regression for
nk_qlease.test_destroy. Moving the listen() to after wait(setup_done)
indeed fixed it.
Best,
Bobby
