[PATCH net-next v18 07/14] memory-provider: dmabuf devmem memory provider

[Pavel Begunkov]

On 8/13/24 00:57, Jakub Kicinski wrote:
> On Mon, 12 Aug 2024 20:10:39 +0100 Pavel Begunkov wrote:
>>> 1. Drivers need to be able to say "I support unreadable netmem".
>>> Failure to report unreadable netmem support should cause the netlink
>>> API to fail when the user tries to bind dmabuf/io uring memory.
>>>
>>> 2. Drivers need to be able to say "I want a header pool (with readable
>>> netmem)" or "I want a data pool (potentially with unreadable netmem)".
>>>
>>> Pavel is suggesting implementing both of these in 2 different flags.
>>>
>>> Jakub is suggesting implementing both with 1 flag which says "I can
>>> support unreadable netmem for this pool" , and guarding against #1
>>> with a refcount check to detect if a dmabuf pool should have been
>>> created but wasn't.
>>
>> That would be iffy IIUC, but I think Jakub just explicitly said
>> that the refcount trick was just for debugging purposes and not
>> for gauging errors like "providers are not supported by the driver".
>>
>> "Yup, the refcount (now: check of the page pool list) was meant
>> as a WARN_ONCE() to catch bad drivers."
> 
> Sorry, insufficient caffeine level in the morning.
> We can't WARN_ONCE(), indeed.

I'm getting lost, so repeating myself a bit. What I think
would be a good approach is if we get an error back from
the driver if it doesn't support netiov / providers.

netdev_rx_queue_restart() {
	...
	err = dev->queue_mgmt_ops->ndo_queue_mem_alloc();
	if (err == -EOPNOTSUPP) // the driver doesn't support netiov
		return -EOPNOTSUPP;
	...
}

That can be done if drivers opt in to support providers,
e.g. via a page pool flag.

What I think wouldn't be a great option is getting back a
"success" from the driver even though it ignored

netdev_rx_queue_restart() {
	...
	err = dev->queue_mgmt_ops->ndo_queue_mem_alloc();
	if (err)
		return err;

	// we get err==0 even if the driver doesn't support
	// providers, verify it is _actually_ installed
	if (rxq->mp_params) {
		// or walking pp list, same thing
		if (rxq->mp_params->refcount == 0)
			goto fail;
	}
}

And if we go with the first version, the refcount check can
also be added but as a warning. Maybe it's easier to put it
into code and discuss then.

-- 
Pavel Begunkov