RE: [PATCH v2] net/af_xdp: fix umem map size for zero copy

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Loftus, Ciara [mailto:ciara.loftus@intel.com]
> Sent: Wednesday, 29 May 2024 14.58
> 
> > From: Du, Frank <frank.du@intel.com>
> > Sent: Thursday, May 23, 2024 8:56 AM
> >
> > > From: Morten Brørup <mb@smartsharesystems.com>
> > > Sent: Thursday, May 23, 2024 3:41 PM
> > >
> > > > From: Du, Frank [mailto:frank.du@intel.com]
> > > > Sent: Thursday, 23 May 2024 08.56
> > > >
> > > > > From: Morten Brørup <mb@smartsharesystems.com>
> > > > > Sent: Wednesday, May 22, 2024 3:27 PM
> > > > >
> > > > > > From: Du, Frank [mailto:frank.du@intel.com]
> > > > > > Sent: Wednesday, 22 May 2024 03.25
> > > > > >
> > > > > > > From: Ferruh Yigit <ferruh.yigit@amd.com>
> > > > > > > Sent: Wednesday, May 22, 2024 1:58 AM

[...]

> > > > > > >
> > > > > > > Isn't there a mempool flag that can help us figure out mempool
> > > > > > > is not IOVA contiguous? Isn't it sufficient on its own?
> > > > > >
> > > > > > Indeed, what we need to ascertain is whether it's contiguous in
> > > > > > CPU virtual space, not IOVA. I haven't come across a flag
> > > > > > specifically for CPU virtual contiguity. The major limitation in
> > > > > > XDP is XSK UMEM only supports registering a single contiguous
> > > > > > virtual memory area.
> > > > >
> > > > > I would assume that the EAL memory manager merges free memory into
> > > > > contiguous chunks whenever possible.
> > > > > @Anatoly, please confirm?
> > > > >
> > > > > If my assumption is correct, it means that if mp->nb_mem_chunks !=
> > > > > 1, then the
> > > > > mempool is not virtual contiguous. And if mp->nb_mem_chunks == 1,
> > > > > then it is;
> > > > > there is no need to iterate through the memhdr list.
> > > >
> > > > If this's true now, however, this assumption may not hold true in the
> > > > future code change, iterating through the list may is a safer way as
> > > > it carefully checks the virtual address without relying on any
> > > > condition.
> > >
> > > If there is exactly one memory chunk, it is virtual contiguous. It has one
> > > address and one length, so it must be.
> > >
> > > If there are more than one memory chunk, I consider it unlikely that they
> > > are contiguous.
> > > Have you ever observed the opposite, i.e. a mempool with multiple memory
> > > chunks being virtual contiguous?
> > >
> > > Iterating through the list does not seem safer to me, quite the opposite.
> > > Which future change are you trying to prepare for?
> > >
> > > Keeping it simple is more likely to not break with future changes.
> >
> > No, I haven't encountered a mempool with multiple memory chunks actually,
> > not know how to construct such mempool. The initial approach was to return
> > an error if multiple chunks were detected, and the iteration method was
> > introduced later. I can revert to the original, simpler way.
> 
> The mempool created in my (virtualized) test environment always has multiple
> memory chunks and the iterative check for virtual contiguity in v2 of this
> patch succeeds for me.
> However in v4, since mp->nb_mem_chunks != 1, it will fail for me.
> So it appears that virtual contiguity is possible even if mp->nb_mem_chunks !=
> 1 so I don't think we can rely on that value for determining virtual
> contiguity.

Excellent feedback, Ciara!
Once again, reality beats assumptions. Memory management is not easy. :-)

In another thread [1], I have asked Paul Szczepanek to pick up on this, and coordinate directly with Frank Du.
Paul is working on a closely related function in the mempool library, and it makes sense to merge this feature into the function he is providing.
Alternatively provide another function in the mempool library.

[1]: https://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35E9F4C4@smartserver.smartshare.dk/T/#m8bf62c45d12f34659becf965cf101f8456723c94