RE: [RFC v2] eal/linux: add support for fast virt/iova translation

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

> From: Don Wallwork [mailto:donw@xsightlabs.com]
> Sent: Thursday, 15 September 2022 14.14
> 
> On 9/15/2022 3:40 AM, Morten Brørup wrote:
> >> From: Don Wallwork [mailto:donw@xsightlabs.com]
> >> Sent: Wednesday, 14 September 2022 23.12
> >>
> >> This patch maps hugepage memory such that address translation from
> >> virtual to iova or vice versa can be done by simple addition/
> >> subtraction of a constant value without any page table walks.
> >>
> >> A new '--const-translate' EAL option is added to enable this mode.

[...]

> > rte_mem_fast_virt2iova() and rte_mem_fast_iova2virt() serve the same
> purpose as rte_mem_virt2iova() and rte_mem_iova2virt(). Can't this
> alternative algorithm be implemented in the existing functions, or is
> there a reason for adding separate functions?
> It is necessary to keep a version of the translation functions that
> does
> the page table walks since that method must be used at init time.  My
> intention was to add new functions that are safe for use in fast path
> code.
> >
> > Minor detail about names: The DPDK community has a general aversion
> against using "fast" in function names - because everything in DPDK is
> supposed to be fast. If they need to be separate, perhaps use "_fp"
> (for fast path) at the end of their names, like the Trace library's
> RTE_TRACE_POINT_FP() variant of the RTE_TRACE_POINT() macro.
> Thought that might be the case.. I can change to _fp.

That should fly - at least there's a reference to point at. :-)

> >
> > If these two functions need to be really fast, they could be inline
> instead of proper functions. In my opinion, they are short enough to
> allow inlining. And, if merged with the original functions, the fast
> code path could be inline and fall back to calling the original, slower
> functions (which would then need to be renamed). Others might disagree
> about inlining. Reusing the existing names might also probably break
> the ABI.
> Agree that this could be inlined, but probably best not to merge with
> the page walk version since it would not be desirable for fast path
> code
> to fall back to that mode.

I have no strong opinion regarding merging.

And since I don't know how much these functions are going to be used, I have no strong opinion about inlining.

> Also since this patch implements support for
> Linux EAL only, the inline functions may need to be added to a Linux
> specific header.

These fast path functions (whether merged with their slow cousins or not) will be part of the public API, so fallback implementations for other operating systems need to be implemented. Making them inline or proper functions shouldn't have a big impact on the development effort.

I must admit that the way the DPDK EAL implements inline functions for various architectures seems a bit weird to me; being based largely on copy-paste, rather than a clear separation of declaration and implementation. I guess that the DPDK EAL suffers from the same weirdness for various operating systems.

> >
> >  From a high level perspective - and referring to my previous
> argument about optimizations not being features [1] - I wonder if this
> patch should be a build time option instead of a runtime feature?

Making it a build time option would probably also implicitly answer some of the questions about how to implement the fast path functions.

> > @Don: My preference for build time options is somewhat controversial,
> so don't waste time converting to a build time option before the
> community has provided feedback on the subject.
> >
> > [1]:
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Finbox.
> dpdk.org%2Fdev%2F98CBD80474FA8B44BF855DF32C47DC35D870EB%40smartserver.s
> martshare.dk%2F&data=05%7C01%7Cdonw%40xsightlabs.com%7C27c0102845d0
> 42a17bb408da96ed96f7%7C646a3e3483ea42739177ab01923abaa9%7C0%7C0%7C63798
> 8244443228274%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=bW9ZmyZ9Ap9
> rAG5juYGEUgvCNTCJ96Q1IwPf8ExaEGI%3D&reserved=0