On Mon, Jul 7, 2025 at 2:35 PM Dragos Tatulea [off-list ref] wrote:

On Mon, Jul 07, 2025 at 11:44:19AM -0700, Mina Almasry wrote:

quoted

On Fri, Jul 4, 2025 at 6:11 AM Dragos Tatulea [off-list ref] wrote:

quoted

On Thu, Jul 03, 2025 at 01:58:50PM +0200, Parav Pandit wrote:

quoted

quoted

From: Jakub Kicinski <kuba@kernel.org>
Sent: 03 July 2025 02:23 AM

[...]

quoted

quoted

Maybe someone with closer understanding can chime in. If the kind of
subfunctions you describe are expected, and there's a generic way of
recognizing them -- automatically going to parent of parent would indeed be
cleaner and less error prone, as you suggest.

I am not sure when the parent of parent assumption would fail, but can be
a good start.

If netdev 8 bytes extension to store dma_dev is concern,
probably a netdev IFF_DMA_DEV_PARENT can be elegant to refer parent->parent?
So that there is no guess work in devmem layer.

That said, my understanding of devmem is limited, so I could be mistaken here.

In the long term, the devmem infrastructure likely needs to be
modernized to support queue-level DMA mapping.
This is useful because drivers like mlx5 already support
socket-direct netdev that span across two PCI devices.

Currently, devmem is limited to a single PCI device per netdev.
While the buffer pool could be per device, the actual DMA
mapping might need to be deferred until buffer posting
time to support such multi-device scenarios.

In an offline discussion, Dragos mentioned that io_uring already
operates at the queue level, may be some ideas can be picked up
from io_uring?

The problem for devmem is that the device based API is already set in
stone so not sure how we can change this. Maybe Mina can chime in.

I think what's being discussed here is pretty straight forward and
doesn't need UAPI changes, right? Or were you referring to another
API?

I was referring to the fact that devmem takes one big buffer, maps it
for a single device (in net_devmem_bind_dmabuf()) and then assigns it to
queues in net_devmem_bind_dmabuf_to_queue(). As the single buffer is
part of the API, I don't see how the mapping could be done in a per
queue way.

Oh, I see. devmem does support mapping a single buffer to multiple
queues in a single netlink API call, but there is nothing stopping the
user from mapping N buffers to N queues in N netlink API calls.

quoted

quoted

To sum the conversation up, there are 2 imperfect and overlapping
solutions:

1) For the common case of having a single PCI device per netdev, going one
   parent up if the parent device is not DMA capable would be a good
   starting point.

2) For multi-PF netdev [0], a per-queue get_dma_dev() op would be ideal
   as it provides the right PF device for the given queue.

Agreed these are the 2 options.

quoted

io_uring
   could use this but devmem can't. Devmem could use 1. but the
   driver has to detect and block the multi PF case.

Why? AFAICT both io_uring and devmem are in the exact same boat right
now, and your patchset seems to show that? Both use dev->dev.parent as
the mapping device, and AFAIU you want to use dev->dev.parent.parent
or something like that?

Right. My patches show that. But the issue raised by Parav is different:
different queues can belong to different DMA devices from different
PFs in the case of Multi PF netdev.

io_uring can do it because it maps individual buffers to individual
queues. So it would be trivial to get the DMA device of each queue through
a new queue op.

Right, devmem doesn't stop you from mapping individual buffers to
individual queues. It just also supports mapping the same buffer to
multiple queues. AFAIR, io_uring also supports mapping a single buffer
to multiple queues, but I could easily be very wrong about that. It's
just a vague recollection from reviewing the iozcrx.c implementation a
while back.

In your case, I think, if the user is trying to map a single buffer to
multiple queues, and those queues have different dma-devices, then you
have to error out. I don't see how to sanely handle that without
adding a lot of code. The user would have to fall back onto mapping a
single buffer to a single queue (or multiple queues that share the
same dma-device).

quoted

Also AFAIU the driver won't need to block the multi PF case, it's
actually core that would need to handle that. For example, if devmem
wants to bind a dmabuf to 4 queues, but queues 0 & 1 use 1 dma device,
but queues 2 & 3 use another dma-device, then core doesn't know what
to do, because it can't map the dmabuf to both devices at once. The
restriction would be at bind time that all the queues being bound to
have the same dma device. Core would need to check that and return an
error if the devices diverge. I imagine all of this is the same for
io_uring, unless I'm missing something.

Agreed. Currently I didn't see an API for Multi PF netdev to expose
this information so my thinking defaulted to "let's block it from the
driver side".

Agreed.

quoted

quoted

I think we need both. Either that or a netdev op with an optional queue
parameter. Any thoughts?

At the moment, from your description of the problem, I would lean to
going with Jakub's approach and handling the common case via #1. If
more use cases that require a very custom dma device to be passed we
can always move to #2 later, but FWIW I don't see a reason to come up
with a super future proof complicated solution right now, but I'm
happy to hear disagreements.

But we also don't want to start off on the left foot when we know of
both issues right now. And I think we can wrap it up nicely in a single
function similary to how the current patch does it.

FWIW I don't have a strong preference. I'm fine with the simple
solution for now and I'm fine with the slightly more complicated
future proof solution.

-- 
Thanks,
Mina