Re: [PATCH net-next v2 2/4] bpf: test_run: Use system page pool for XDP live frame mode

[Jesper Dangaard Brouer <hawk@kernel.org>]

On 03/04/2024 22.39, John Fastabend wrote:
> Alexander Lobakin wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> Date: Tue, 20 Feb 2024 22:03:39 +0100
>>
>>> The BPF_TEST_RUN code in XDP live frame mode creates a new page pool
>>> each time it is called and uses that to allocate the frames used for the
>>> XDP run. This works well if the syscall is used with a high repetitions
>>> number, as it allows for efficient page recycling. However, if used with
>>> a small number of repetitions, the overhead of creating and tearing down
>>> the page pool is significant, and can even lead to system stalls if the
>>> syscall is called in a tight loop.
>>>
>>> Now that we have a persistent system page pool instance, it becomes
>>> pretty straight forward to change the test_run code to use it. The only
>>> wrinkle is that we can no longer rely on a custom page init callback
>>> from page_pool itself; instead, we change the test_run code to write a
>>> random cookie value to the beginning of the page as an indicator that
>>> the page has been initialised and can be re-used without copying the
>>> initial data again.
>>>
>>> The cookie is a random 128-bit value, which means the probability that
>>> we will get accidental collisions (which would lead to recycling the
>>> wrong page values and reading garbage) is on the order of 2^-128. This
>>> is in the "won't happen before the heat death of the universe" range, so
>>> this marking is safe for the intended usage.
>>>
>>> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>>> Tested-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> Hey,
>>
>> What's the status of this series, now that the window is open?
>>
>> Thanks,
>> Olek
> 
> Hi Toke,
> 
> I read the thread from top to bottom so seems someone else notices the
> 2^128 is unique numbers not the collision probability. Anywaays I'm still
> a bit confused, whats the use case here? Maybe I need to understand
> what this XDP live frame mode is better?
> 
> Could another solution be to avoid calling BPF_TEST_RUN multiple times
> in a row? Or perhaps have a BPF_SETUP_RUN that does the config and lets
> BPF_TEST_RUN skip the page allocation? Another idea just have the first
> run of BPF_TEST_RUN init a page pool and not destroy it.
> 

I like John's idea of "the first run of BPF_TEST_RUN init a page pool
and not destroy it".  On exit we could start a work-queue that tried to
"destroy" the PP (in the future) if it's not in use.

Page pool (PP) performance comes from having an association with a
SINGLE RX-queue, which means no synchronization is needed then
"allocating" new pages (from the alloc cache array).

Thus, IMHO each BPF_TEST_RUN should gets it's own PP instance, as then
lockless PP scheme works (and we don't have to depend on NAPI /
BH-disable).  This still works with John's idea, as we could simply have
a list of PP instances that can be reused.

--Jesper