Re: [net-next,v6,5/6] selftests: net: add multithread server support to iou-zcrx

[Juanlu Herrero <juanlu@fastmail.com>]

On Wed, Sep 09, 2026 at 09:16:58AM -0600, Bobby Eshleman wrote:
> On Wed, Sep 02, 2026 at 11:38:17PM +0000, netdev-bot+sashiko@kernel.org 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!