DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Mattias Rönnblom" <hofors@lysator.liu.se>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org
Subject: Re: [PATCH v5 00/32] replace use of rte_memcpy() with fixed size
Date: Mon, 27 May 2024 08:06:36 +0200	[thread overview]
Message-ID: <b1c52db0-35d1-4383-96ec-9b5abc5adeaa@lysator.liu.se> (raw)
In-Reply-To: <20240526163232.171ca90d@hermes.local>

On 2024-05-27 01:32, Stephen Hemminger wrote:
> On Sun, 26 May 2024 16:51:52 +0200
> Mattias Rönnblom <hofors@lysator.liu.se> wrote:
> 
>>> This does not change the resulting binary on almost all architectures
>>> because x86 version of intrisics and glibc are the same, and
>>> other architectures were using __builtin_constant_p().
>>>
>>> The main benefit is that analysis tools like fortify, Coverity, and ASAN
>>> analyzers can check these memcpy's. A recent example is that
>>> on Ubuntu 22.04 detected undefined use of memcpy such as:
>>> 	memcpy(dst, NULL, 0)
>>>    
>>
>> Would it be possible to instead have a build mode where rte_memcpy() is
>> *always* delegating to memcpy(), and run these tools on that configuration?
>>
>> It seems easier to just always using rte_memcpy() in DPDK code, then let
>> the platform (not the programmer) choose whatever is most appropriate.
> 
> I would prefer that rte_memcpy be laid to rest and go away.

Sounds good, but is this piecemeal approach the way to do it. Why ask 
DPDK programmers to memorize rules about when to use memcpy() and when 
not to, when that can be coded into the rte_memcpy() implementation.

Worst case, when people have learned those rules, we'll change them. 
"Use rte_memcpy() only for large copies," or whatever it might be.

Use rte_memcpy() across the board until memcpy() is a complete 
replacement, for every use case, for every CPU/compiler/OS.

Applications using DPDK will benefit from any heuristics encoded into 
rte_memcpy.h. The won't run coccinelle script.

> It never had a reason to be there.

Sure, it did. Historically it has had better performance, up to at least 
fairly recently that seemed still be the case. I've patched DPDK to 
replace rte_memcpy() with memcpy() several times, and run with 
real-world apps. The result is always the same.

The compiler is in a better position than DPDK to implement memcpy(), so 
eventually the compiler will win.


  reply	other threads:[~2024-05-27  6:06 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03 16:32 [PATCH 0/2] uuid: enhancements and tests Stephen Hemminger
2024-04-03 16:32 ` [PATCH 1/2] eal: add functions to generate uuid values Stephen Hemminger
2024-04-04 16:11   ` Tyler Retzlaff
2024-04-03 16:32 ` [PATCH 2/2] test: add functional test for uuid Stephen Hemminger
2024-04-03 22:11 ` [PATCH v2 0/2] uuid: add generate functions and tests Stephen Hemminger
2024-04-03 22:11   ` [PATCH v2 1/2] eal: add functions to generate uuid values Stephen Hemminger
2024-04-04 16:16     ` Tyler Retzlaff
2024-04-03 22:11   ` [PATCH v2 2/2] test: add functional test for uuid Stephen Hemminger
2024-04-04 16:18     ` Tyler Retzlaff
2024-04-04 16:22 ` [PATCH v3 0/2] uuid: add generate functions and tests Stephen Hemminger
2024-04-04 16:22   ` [PATCH v3 1/2] eal: add functions to generate uuid values Stephen Hemminger
2024-04-04 16:22   ` [PATCH v3 2/2] test: add functional test for uuid Stephen Hemminger
2024-04-05 16:53 ` [PATCH v4 00/30] replace use of rte_memcpy with fixed sizes Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 01/30] cocci/rte_memcpy: add script to eliminate fixed size rte_memcpy Stephen Hemminger
2024-04-06  9:01     ` Morten Brørup
2024-04-05 16:53   ` [PATCH v4 02/30] eal: replace use of " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 03/30] ethdev: replace uses of rte_memcpy Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 04/30] eventdev: replace use of fixed size rte_memcpy Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 05/30] cryptodev: " Stephen Hemminger
2024-04-10 15:40     ` [EXTERNAL] " Akhil Goyal
2024-04-05 16:53   ` [PATCH v4 06/30] ip_frag: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 07/30] net: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 08/30] lpm: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 09/30] node: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 10/30] pdcp: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 11/30] pipeline: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 12/30] rib: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 13/30] security: " Stephen Hemminger
2024-04-10 15:40     ` [EXTERNAL] " Akhil Goyal
2024-04-05 16:53   ` [PATCH v4 14/30] bus: remove unneeded rte_memcpy.h include Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 15/30] net: replace use of fixed size rte_memcpy Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 16/30] raw: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 17/30] baseband: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 18/30] common: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 19/30] crypto: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 20/30] " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 21/30] event: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 22/30] mempool: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 23/30] ml/cnxk: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 24/30] app/test-pmd: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 25/30] app/graph: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 26/30] app/test-eventdev: " Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 27/30] app/test: " Stephen Hemminger
2024-04-10 18:28     ` [EXTERNAL] " Akhil Goyal
2024-04-05 16:53   ` [PATCH v4 28/30] app/test-pipeline: remove unused rte_memcpy.h include Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 29/30] app/test-bbdev: remove unnecessary include of rte_memcpy.h Stephen Hemminger
2024-04-05 16:53   ` [PATCH v4 30/30] examples: replace use of fixed size rte_memcpy Stephen Hemminger
2024-04-09 17:05 ` [PATCH v4 0/2] uuid: generator functions and unit test Stephen Hemminger
2024-04-09 17:05   ` [PATCH v4 1/2] eal: add functions to generate uuid values Stephen Hemminger
2024-04-09 17:05   ` [PATCH v4 2/2] test: add functional test for uuid Stephen Hemminger
2024-05-22  3:27 ` [PATCH v5 00/32] replace use of rte_memcpy() with fixed size Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 01/32] cocci/rte_memcpy: add script to eliminate fixed size rte_memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 02/32] eal: replace use of " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 03/32] ethdev: replace uses of rte_memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 04/32] eventdev: replace use of fixed size rte_memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 05/32] cryptodev: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 06/32] ip_frag: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 07/32] net: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 08/32] lpm: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 09/32] node: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 10/32] pdcp: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 11/32] pipeline: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 12/32] rib: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 13/32] security: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 14/32] bus: remove unneeded rte_memcpy.h include Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 15/32] raw: replace use of fixed size rte_memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 16/32] baseband: " Stephen Hemminger
2024-05-23 18:28     ` Chautru, Nicolas
2024-05-22  3:27   ` [PATCH v5 17/32] common: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 18/32] crypto: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 19/32] event: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 20/32] mempool: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 21/32] ml/cnxk: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 22/32] app/test-pmd: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 23/32] app/graph: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 24/32] app/test-eventdev: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 25/32] app/test: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 26/32] app/test-pipeline: remove unused rte_memcpy.h include Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 27/32] app/test-bbdev: remove unnecessary include of rte_memcpy.h Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 28/32] examples: replace use of fixed size rte_memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 29/32] net/null: replace use of fixed size memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 30/32] net/tap: replace use of fixed size rte_memcpy Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 31/32] net/pcap: " Stephen Hemminger
2024-05-22  3:27   ` [PATCH v5 32/32] net/af_xdp:: " Stephen Hemminger
2024-05-26 14:51   ` [PATCH v5 00/32] replace use of rte_memcpy() with fixed size Mattias Rönnblom
2024-05-26 23:32     ` Stephen Hemminger
2024-05-27  6:06       ` Mattias Rönnblom [this message]
2024-05-27  6:38       ` Morten Brørup

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=b1c52db0-35d1-4383-96ec-9b5abc5adeaa@lysator.liu.se \
    --to=hofors@lysator.liu.se \
    --cc=dev@dpdk.org \
    --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).