net/ena: revert redefining memcpy
Checks
Commit Message
Redefining memcpy as rte_memcpy has no performance gain on
current compilers, and introduced bugs like this one where
rte_memcpy() will be detected as referencing past the destination.
Bugzilla ID: 1510
Fixes: 142778b3702a ("net/ena: switch memcpy to optimized version")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/ena/base/ena_plat_dpdk.h | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
Comments
> Subject: [PATCH] net/ena: revert redefining memcpy
>
> Redefining memcpy as rte_memcpy has no performance gain on current
> compilers, and introduced bugs like this one where
> rte_memcpy() will be detected as referencing past the destination.
>
> Bugzilla ID: 1510
> Fixes: 142778b3702a ("net/ena: switch memcpy to optimized version")
Similar observations in Arm when RTE_ARCH_ARM64_MEMCPY flag is on.
Acked-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> drivers/net/ena/base/ena_plat_dpdk.h | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/net/ena/base/ena_plat_dpdk.h
> b/drivers/net/ena/base/ena_plat_dpdk.h
> index 21b96113c7..291db6cabe 100644
> --- a/drivers/net/ena/base/ena_plat_dpdk.h
> +++ b/drivers/net/ena/base/ena_plat_dpdk.h
> @@ -26,7 +26,6 @@
> #include <rte_spinlock.h>
>
> #include <sys/time.h>
> -#include <rte_memcpy.h>
>
> typedef uint64_t u64;
> typedef uint32_t u32;
> @@ -70,13 +69,7 @@ typedef uint64_t dma_addr_t; #define
> ENA_UDELAY(x) rte_delay_us_block(x)
>
> #define ENA_TOUCH(x) ((void)(x))
> -/* Redefine memcpy with caution: rte_memcpy can be simply aliased to
> memcpy, so
> - * make the redefinition only if it's safe (and beneficial) to do so.
> - */
> -#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) ||
> defined(RTE_ARCH_ARM_NEON_MEMCPY) -#undef memcpy -#define
> memcpy rte_memcpy -#endif
> +
> #define wmb rte_wmb
> #define rmb rte_rmb
> #define mb rte_mb
> --
> 2.43.0
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
>
> Redefining memcpy as rte_memcpy has no performance gain on
> current compilers, and introduced bugs like this one where
> rte_memcpy() will be detected as referencing past the destination.
>
> Bugzilla ID: 1510
> Fixes: 142778b3702a ("net/ena: switch memcpy to optimized version")
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Morten Brørup <mb@smartsharesystems.com>
On Mon, Aug 12, 2024 at 08:34:17AM -0700, Stephen Hemminger wrote:
> Redefining memcpy as rte_memcpy has no performance gain on
> current compilers, and introduced bugs like this one where
> rte_memcpy() will be detected as referencing past the destination.
>
> Bugzilla ID: 1510
> Fixes: 142778b3702a ("net/ena: switch memcpy to optimized version")
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
On 9/12/2024 6:12 AM, Tyler Retzlaff wrote:
> On Mon, Aug 12, 2024 at 08:34:17AM -0700, Stephen Hemminger wrote:
>> Redefining memcpy as rte_memcpy has no performance gain on
>> current compilers, and introduced bugs like this one where
>> rte_memcpy() will be detected as referencing past the destination.
>>
>> Bugzilla ID: 1510
>> Fixes: 142778b3702a ("net/ena: switch memcpy to optimized version")
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> ---
>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>
Hi Shai,
Did you have any chance to check/test this patch?
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, September 12, 2024 11:20 AM
> To: Tyler Retzlaff <roretzla@linux.microsoft.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Brandes, Shai <shaibran@amazon.com>
> Cc: dev@dpdk.org; Schmeilin, Evgeny <evgenys@amazon.com>; Beider, Ron
> <rbeider@amazon.com>; Bernstein, Amit <amitbern@amazon.com>;
> Atrash, Wajeeh <atrwajee@amazon.com>; Chauskin, Igor
> <igorch@amazon.com>; Michal Krawczyk <mk@semihalf.com>; Artur Rojek
> <ar@semihalf.com>
> Subject: RE: [EXTERNAL] [PATCH] net/ena: revert redefining memcpy
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you can confirm the sender and know the
> content is safe.
>
>
>
> On 9/12/2024 6:12 AM, Tyler Retzlaff wrote:
> > On Mon, Aug 12, 2024 at 08:34:17AM -0700, Stephen Hemminger wrote:
> >> Redefining memcpy as rte_memcpy has no performance gain on current
> >> compilers, and introduced bugs like this one where
> >> rte_memcpy() will be detected as referencing past the destination.
> >>
> >> Bugzilla ID: 1510
> >> Fixes: 142778b3702a ("net/ena: switch memcpy to optimized version")
> >>
> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >> ---
> >
> > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >
>
> Hi Shai,
>
> Did you have any chance to check/test this patch?
[Brandes, Shai] sorry, I missed this. The change will be tested by the upcoming Monday is this is fine.
@@ -26,7 +26,6 @@
#include <rte_spinlock.h>
#include <sys/time.h>
-#include <rte_memcpy.h>
typedef uint64_t u64;
typedef uint32_t u32;
@@ -70,13 +69,7 @@ typedef uint64_t dma_addr_t;
#define ENA_UDELAY(x) rte_delay_us_block(x)
#define ENA_TOUCH(x) ((void)(x))
-/* Redefine memcpy with caution: rte_memcpy can be simply aliased to memcpy, so
- * make the redefinition only if it's safe (and beneficial) to do so.
- */
-#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64_MEMCPY) || defined(RTE_ARCH_ARM_NEON_MEMCPY)
-#undef memcpy
-#define memcpy rte_memcpy
-#endif
+
#define wmb rte_wmb
#define rmb rte_rmb
#define mb rte_mb