RE: [PATCH v2] eal: fix unaligned loads/stores in rte_memcpy_generic

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

> From: Georg Sauthoff [mailto:mail@gms.tf]
> Sent: Tuesday, 25 January 2022 00.22
> 
> Hello,
> 
> Stephen Hemminger wrote (Sun, 16 Jan 2022 08:32:20 -0800):
> 
> > I would propose that DPDK have same kind of define as the kernel
> > for SAFE_UNALIGNED_ACCESS.  The C standard has to apply to all
> architectures
> > but DPDK will make the choice to be fast rather than standards
> conformant.
> 
> perhaps it's worth mentioning that the Linux Kernel is compiled with
> -fno-strict-aliasing, because it contains code which violates the C
> strict aliasing rules. Such code yields undefined behavior and is thus
> unsafe when compiling with optmizing compilers such as GCC and Clang,
> by
> default. But since the Linux supplies -fno-strict-aliasing its code is
> safe, in the Linux Kernel context.
> 
> In contrast, DPDK isn't compiled with -fno-strict-aliasing, in general.
> Only a few drivers are built with -Wno-strict-aliasing.
> 
> Thus, one has to be careful when importing (or being inspired) by the
> above mentioned kernel defines.
> 
> Especially, it looks like version 5 of this patch yields undefined
> behavior because it violates strict-aliasing rules.
> It's possible that GCC makes some extra guarantees for the used
> constructs, even when compiling without -fno-strict-aliasing. But I'm
> not aware of any.
> 
> Please note that there is a reason GCC enables -fstrict-aliasing when
> compiling with optimizations: Performance
> IOW, -fno-strict-aliasing has performance implications, thus one is
> advised to not use it, if possible.
> IOW, when violating strict-aliasng rules one can easily end up with
> low-performing and/or unsafe (buggy) code.

Generally, I have an extremely strong preference for being able to compile code with all warnings enabled in the compiler (-Wall and then some). This gives the developer a great bug-detector tool (through the compiler warnings).

In my experience, compiling a Linux loadable module with all warnings enabled spews out too many warnings about the kernel itself to be helpful. This stems from bad practice, which should not be adopted by DPDK.

> 
> I haven't inspected the DPDK drivers which supply -Wno-strict-aliasing
> -
> but I wouldn't be surprised if there aren't better alternatives.
> Meaning
> writing the code in a way that doesn't require -Wno-strict-aliasing.
> 
> BTW, are there still systems that DPDK supports but have a memcpy()
> which
> performs worse than rte_memcpy()?

Excellent question! I have also seen memmove() being replaced with a comment about some systems using temporary memory for their implementation of memmove(). And most DPDK core libraries implement their own copy functions - following the general DPDK advice not to use standard C library functions. It's a shame there's no documentation why all this was re-implemented in DPDK, so it can be checked if it is still relevant or could be removed.

> 
> 
> Best regards,
> Georg