DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: "Morten Brørup" <mb@smartsharesystems.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>,
	Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
	dev@dpdk.org
Subject: Re: [RFC 8/8] ip_frag: fix gcc-12 warnings
Date: Tue, 14 Jun 2022 23:20:46 +0200	[thread overview]
Message-ID: <2651319.mvXUDI8C0e@thomas> (raw)
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D87101@smartserver.smartshare.dk>

09/06/2022 09:09, Morten Brørup:
> > 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. ;-)

No it doesn't apply, the only limitation is the number of contributions.
Feel free to propose cleanups.



      reply	other threads:[~2022-06-14 21:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-07 17:17 [RFC 0/8] Gcc-12 warning fixes Stephen Hemminger
2022-06-07 17:17 ` [RFC 1/8] net/ena: fix warnings related to rte_memcpy and gcc-12 Stephen Hemminger
2022-06-08 12:29   ` Michał Krawczyk
2022-06-08 15:32     ` Stephen Hemminger
2022-06-08 19:18       ` Michał Krawczyk
2022-06-08 20:52         ` Stephen Hemminger
2022-06-07 17:17 ` [RFC 2/8] net/qede: fix gcc-12 rte_memcpy warnings Stephen Hemminger
2022-06-23 14:16   ` David Marchand
2022-06-07 17:17 ` [RFC 3/8] net/ice: fix rte_memcpy warnings with gcc-12 Stephen Hemminger
2022-06-07 17:17 ` [RFC 4/8] test/ipfrag: fix gcc-12 warnings Stephen Hemminger
2022-06-07 17:17 ` [RFC 5/8] test/ipsec: fix gcc-12 rte_memcpy warnings Stephen Hemminger
2022-06-07 17:17 ` [RFC 6/8] net/enetfc: fix array out of bounds warning Stephen Hemminger
2022-06-07 17:17 ` [RFC 7/8] vhost: replace rte_memcpy to fix warning Stephen Hemminger
2022-06-07 17:17 ` [RFC 8/8] ip_frag: fix gcc-12 warnings Stephen Hemminger
2022-06-08  8:19   ` Konstantin Ananyev
2022-06-08 15:26     ` Stephen Hemminger
2022-06-09  7:09       ` Morten Brørup
2022-06-14 21:20         ` Thomas Monjalon [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2651319.mvXUDI8C0e@thomas \
    --to=thomas@monjalon.net \
    --cc=dev@dpdk.org \
    --cc=konstantin.v.ananyev@yandex.ru \
    --cc=mb@smartsharesystems.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).