Re: [RFC v2] eal: provide option to use compiler memcpy instead of RTE

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 31 May 2024 07:19:41 +0200
Mattias Rönnblom <hofors@lysator.liu.se> wrote:

> On 2024-05-28 17:09, Bruce Richardson wrote:
> > On Tue, May 28, 2024 at 07:59:36AM -0700, Stephen Hemminger wrote:  
> >> On Tue, 28 May 2024 10:19:15 +0200
> >> Mattias Rönnblom <hofors@lysator.liu.se> wrote:
> >>  
> >>>>      
> >>>
> >>> I've tested this patch some with DSW micro benchmarks, and the result is
> >>> a 2.5% reduction of the DSW+testapp overhead with cc/libc memcpy. GCC 11.4.
> >>>
> >>> We've also run characteristic test suite of a large, real world app.
> >>> Here, we saw no effect. GCC 10.5.
> >>>
> >>> x86_64 in both cases (Skylake and Raptor Lake).
> >>>
> >>> Last time we did the same, there were a noticeable performance
> >>> degradation in both the above cases.
> >>>
> >>> This is not a lot of data points, but I think it we should consider
> >>> making the custom RTE memcpy() implementations optional in the next
> >>> release, and if no-one complains, remove the implementations in the next
> >>> release.  
> >>
> >> Lets go farther.
> >>
> >> 1. Announce that rte_memcpy will be marked deprecated in 24.11 release
> >>
> >> 2. In 24.11 do a global replace of rte_memcpy on the tree.
> >>     And mark rte_memcpy as deprecated.
> >>
> >> 3. In 25.11 it can go away.  
> > 
> > While I'd like us to be able to do so, I believe that to be premature. We
> > need to see where/if there are regressions first, and see about fixing
> > them.
> > 
> > /Bruce  
> 
> Should I turn this RFC into a PATCH?
> 
> Is use_cc_memcpy a good name for the configuration parameter?
> 

I did a slightly more direct test and found a couple of things:
   1. Ena driver is redefining memcpy as rte_memcpy, this should be removed and should have
      been blocked during code review.
   2. A couple of drivers are implicitly expecting simd vector routines to be available.
      This works because rte_memcpy.h includes rte_vect.h.  The fix is to have these
      places include rte_vect.h