RE: [RFC 8/8] ip_frag: fix gcc-12 warnings

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

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 8 June 2022 17.27
> 
> On Wed, 8 Jun 2022 09:19:20 +0100
> Konstantin Ananyev <konstantin.v.ananyev@yandex.ru> wrote:
> 
> > 07/06/2022 18:17, Stephen Hemminger пишет:
> > > The function rte_memcpy can derference past source buffer which
> > > will cause array out of bounds warnings. But there is no good
> reason
> > > to use rte_memcpy instead of memcpy in this code. Memcpy is just
> > > as fast for these small inputs, and compiler will optimize.
> >
> >
> > AFAIK, rte_memcpy() will outperform memcpy() when _size_ parameter
> > is a variable. Unfortunately that's exactly the case here.
> > So not sure it is a good change, at least without extensive perf
> testing.
> > BTW, if rte_memcpy() really access src buffer beyond it's boundaries,
> > I think that's definitely a bug that needs to be fixed.
> 
> Yes and no.
> IMHO DPDK should not in the C library business, and glibc etc should be
> more optimized if necessary.

A very big +1 to that!

DPDK contains a lot of optimizations that really belong in the compiler and/or C library, but weren't back then, so the clever DPDK developers put them inside DPDK instead.

Over time, the compilers and C libraries have improved, and many of these manually implemented optimizations have become obsolete. They should be cleaned up and replaced by simpler code, and the documentation about optimizing code should be updated accordingly.

Until that happens, we have to expect contributors to use rte_memcpy() and other obsolete optimizations - they are only doing what the DPDK documentation and reference code tells them. Just like application developers are using KNI, because it is so heavily promoted in DPDK documentation.

The DPDK community has a very high focus on the risk of performance regressions when touching DPDK Core libraries, so a general cleaning is probably not going to happen. Luckily, there are exceptions to every rule, such as Georg Sauthoff's patch removing the manual loop unroll in __rte_raw_cksum() [1], which allowed the compiler to generate something better.

I guess that "if it isn't broken, don't fix it" applies to DPDK Core libraries too. ;-)


PS: A funny example of an exotic optimization is the use of Duff's Device in rte_pktmbuf_alloc_bulk() [2]; a C implementation of an optimization for assembler code.

[1] http://inbox.dpdk.org/dev/20211017203718.801998-2-mail@gms.tf/
[2] https://elixir.bootlin.com/dpdk/latest/source/lib/mbuf/rte_mbuf.h#L893