On Tue, Jul 29, 2025 at 9:41 AM Stanislav Fomichev [off-list ref] wrote:

On 07/28, Mina Almasry wrote:

quoted

On Mon, Jul 28, 2025 at 3:06 PM Stanislav Fomichev [off-list ref] wrote:

quoted

On 07/28, Pavel Begunkov wrote:

quoted

On 7/28/25 21:21, Stanislav Fomichev wrote:

quoted

On 07/28, Pavel Begunkov wrote:

quoted

On 7/28/25 18:13, Stanislav Fomichev wrote:

...>>> Supporting big buffers is the right direction, but I have the same

quoted

quoted

quoted

feedback:

Let me actually check the feedback for the queue config RFC...

it would be nice to fit a cohesive story for the devmem as well.

Only the last patch is zcrx specific, the rest is agnostic,
devmem can absolutely reuse that. I don't think there are any
issues wiring up devmem?

Right, but the patch number 2 exposes per-queue rx-buf-len which
I'm not sure is the right fit for devmem, see below. If all you

I guess you're talking about uapi setting it, because as an
internal per queue parameter IMHO it does make sense for devmem.

quoted

care is exposing it via io_uring, maybe don't expose it from netlink for

Sure, I can remove the set operation.

quoted

now? Although I'm not sure I understand why you're also passing
this per-queue value via io_uring. Can you not inherit it from the
queue config?

It's not a great option. It complicates user space with netlink.
And there are convenience configuration features in the future
that requires io_uring to parse memory first. E.g. instead of
user specifying a particular size, it can say "choose the largest
length under 32K that the backing memory allows".

Don't you already need a bunch of netlink to setup rss and flow
steering? And if we end up adding queue api, you'll have to call that
one over netlink also.

I'm thinking one thing that could work is extending bind-rx with an
optional rx-buf-len arg, which in the code translates into devmem
using the new net_mp_open_rxq variant which not only restarts the
queue but also sets the size. From there the implementation should be
fairly straightforward in devmem. devmem currently rejects any pp for
which pp.order != 0. It would need to start accepting that and
forwarding the order to the gen_pool doing the allocations, etc.

Right, that's the logical alternative, to put that rx-buf-len on the
binding to control the size of the niovs. But then what do we do with
the queue's rx-buf-len? bnxt patch in the series does
page_pool_dev_alloc_frag(..., bp->rx_page_size). bp->rx_page_size comes
from netlink. Does it need to be inherited from the pp in the devmem
case somehow?

I need to review the series closely, but the only thing that makes
sense to me off the bat is that the rx-buf-len option sets the
rx-buf-len of the queue as if you called the queue-set API in a
separate call (and the unbind would reset the value to default).

-- 
Thanks,
Mina