Re: [PATCH net-next v9 5/5] selftests: drv-net: devmem: add autorelease test

[Bobby Eshleman <bobbyeshleman@gmail.com>]

On Sun, Jan 11, 2026 at 11:16:37AM -0800, Mina Almasry wrote:
> On Fri, Jan 9, 2026 at 6:19 PM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
> >
> > From: Bobby Eshleman <bobbyeshleman@meta.com>
> >
> > Add test case for autorelease.
> >
> > The test case is the same as the RX test, but enables autorelease.  The
> > original RX test is changed to use the -a 0 flag to disable autorelease.
> >
> > TAP version 13
> > 1..4
> > ok 1 devmem.check_rx
> > ok 2 devmem.check_rx_autorelease
> > ok 3 devmem.check_tx
> > ok 4 devmem.check_tx_chunks
> >
> > Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
> 
> Can you add a test for the problematic/weird scenario I comment on patch 3?
> 
> 1. User does bind (autorelease on or off)
> 2. Data is received.
> 3. User does unbind.
> 4. User calls recevmsg()
> 5. User calls dontneed on the frags obtained in 4.
> 
> This should work with autorelease=on or off, or at least emit a clean
> error message (kernel must not splat).

IIUC, this looks something like (psuedo-code):

ncdevmem.c:

do_server(...) {

	client_fd = accept(...);

	if (check_premature_unbind) {
		/* wait for data but don't recvmsg yet */
		epoll(client_fd, ...);

		/* unbind */
		ynl_sock_destroy(ys);
		
		while (1) {
			ret = recvmsg(client_fd, &msg, MSG_SOCK_DEVMEM);
			/* check ret */

			ret = setsockopt(client_fd, SOL_SOCKET, SO_DEVMEM_DONTNEED, ...)
			/* check ret */
		}
	} else { ... }
}

... then devmem.py checks dmesg?

> 
> I realize a made a suggestion in patch 3 that may make this hard to
> test (i.e. put the kernel in autorelease on/off mode for the boot
> session on the first unbind). If we can add a test while making that
> simplification great, if not, lets not make the simplification I
> guess.

I think we can do both the simplification and this test, but in general
we would have to skip any test when rx bind fails due to the test's new
mode not matching. Not sure if that is desired.

I tend to like the simplification because I really dislike having to
track the RX binding count, but I'm not sure if there is a good way to
do that with making our tests locked into a single mode.

Maybe a debugfs reset option that rejects when rx_bindings_count is
non-zero? That way we can remove all the program logic around
rx_bindings_count and make it's inc/dec wrapper functions no-ops in
production (CONFIG_DEBUG_NET_DEVMEM=n), but still test both modes?


The handler would look something like (approx.):

#ifdef CONFIG_DEBUG_NET_DEVMEM
static ssize_t devmem_reset_write(struct file *file, const char __user *buf,
				  size_t count, loff_t *ppos)
{
	int ret = count;

	mutex_lock(&devmem_ar_lock);

	if (net_devmem_rx_bindings_count_read() != 0) {
		ret = -EBUSY;
		goto unlock;
	}

	/* enable setting the key again via bind_rx) */
	tcp_devmem_ar_locked = false;

	static_branch_disable(&tcp_devmem_ar_key);

unlock:
	mutex_unlock(&devmem_ar_lock);
	return ret;
}
[...]
#endif


... but I couldn't find a good precedent for this in the current
selftests. 

Best,
Bobby