Re: [PATCH v1 06/15] net: page_pool: add ->scrub mem provider callback

[Pavel Begunkov <asml.silence@gmail.com>]

On 11/1/24 17:18, Mina Almasry wrote:
> On Wed, Oct 16, 2024 at 10:42 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
...
>>> The critical point is as I said above, if you free the memory only
>>> when the pp is destroyed, then the memory lives from 1 io_uring ZC
>>> instance to the next. The next instance will see a reduced address
>>> space because the previously destroyed io_uring ZC connection did not
>>> free the memory. You could have users in production opening thousands
>>> of io_uring ZC connections between rxq resets, and not cleaning up
>>> those connections. In that case I think eventually they'll run out of
>>> memory as the memory leaks until it's cleaned up with a pp destroy
>>> (driver reset?).
>>
>> Not sure what giving memory from one io_uring zc instance to
>> another means. And it's perfectly valid to receive a buffer, close
>> the socket and only after use the data, it logically belongs to
>> the user, not the socket. It's only bound to io_uring zcrx/queue
>> object for clean up purposes if io_uring goes down, it's different
>> from devmem TCP.
>>
> 
> (responding here because I'm looking at the latest iteration after
> vacation, but the discussion is here)
> 
> Huh, interesting. For devmem TCP we bind a region of memory to the
> queue once, and after that we can create N connections all reusing the
> same memory region. Is that not the case for io_uring? There are no

Hmm, I think we already discussed the same question before. Yes, it
does indeed support arbitrary number of connections. For what I was
saying above, the devmem TCP analogy would be attaching buffers to the
netlink socket instead of a tcp socket (that new xarray you added) when
you give it to user space. Then, you can close the connection after a
receive and the buffer you've got would still be alive.

That's pretty intuitive as well, with normal receives the kernel
doesn't nuke the buffer you got data into from a normal recv(2) just
because the connection got closed.

> docs or selftest with the series to show sample code using this, but

There should be a good bunch of tests in liburing if you follow
links in the cover letter, as well as added support to some
benchmark tools, kperf and netbench. Also, as mentioned, need to
add a simpler example to liburing, not sure why it was removed.
There will also be man pages, that's better to be done after
merging it since things could change.


> the cover letter mentions that RSS + flow steering needs to be
> configured for io ZC to work. The configuration of flow steering
> implies that the user is responsible for initiating the connection. If
> the user is initiating 1 connection then they can initiate many
> without reconfiguring the memory binding, right?

Right

> When the user initiates the second connection, any pages not cleaned
> up from the previous connection (because we're waiting for the scrub
> callback to be hit), will be occupied when they should not be, right?

I'm not sure what you mean, but seems like the question comes from
the assumptions that it supports only one connection at a time,
which is not the case.

-- 
Pavel Begunkov