On Wed, Sep 09, 2026 at 09:16:58AM -0600, Bobby Eshleman wrote: > 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
thanks for testing on BRCM and I will do the same as a venue to validate any fix I do for the listen() ordering issue in v7!
