[dpdk-dev] pmdinfogen: Fix pmdinfogen to select proper endianess on cross-compile

Message ID 1479494872-9302-1-git-send-email-nhorman@tuxdriver.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
checkpatch/checkpatch warning coding style issues

Commit Message

Neil Horman Nov. 18, 2016, 6:47 p.m. UTC
  pmdinfogen has a bug in which, during build, it pulls in rte_byteorder.h to
obtain the rte macros for byteswapping between the cpu byte order and big or
little endian.  Unfortunately, pmdinfogen is a tool that is only meant to be run
during the build of dpdk components, and so, it runs on the host.  In cross
compile environments however, the rte_byteorder.h is configured using a target
cpu, who's endianess may differ from that of the host, leading to improper
swapping.

The fix is to use host system defined byte swapping routines rather than the
dpdk provided routines.  Note that we are using non posix compliant routines, as
the posix compliant api only addresses 16 and 32 bit swaps, and we also need 64
bit swaps.  Those macros exist (via endian.h), but BSD and Linux put that header
in different locations so some ifdeffery is required.

Tested successfully by myself on Linux and BSD systems.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Hemant Agrawal <hemant.agrawal@nxp.com>
CC: Jerin Jacob <Jerin.Jacob@cavium.com>
CC: Bruce Richardson <bruce.richardson@intel.com>
CC: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 buildtools/pmdinfogen/pmdinfogen.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
  

Comments

Bruce Richardson Nov. 21, 2016, 10:11 a.m. UTC | #1
On Fri, Nov 18, 2016 at 01:47:52PM -0500, Neil Horman wrote:
> pmdinfogen has a bug in which, during build, it pulls in rte_byteorder.h to
> obtain the rte macros for byteswapping between the cpu byte order and big or
> little endian.  Unfortunately, pmdinfogen is a tool that is only meant to be run
> during the build of dpdk components, and so, it runs on the host.  In cross
> compile environments however, the rte_byteorder.h is configured using a target
> cpu, who's endianess may differ from that of the host, leading to improper
> swapping.
> 
> The fix is to use host system defined byte swapping routines rather than the
> dpdk provided routines.  Note that we are using non posix compliant routines, as
> the posix compliant api only addresses 16 and 32 bit swaps, and we also need 64
> bit swaps.  Those macros exist (via endian.h), but BSD and Linux put that header
> in different locations so some ifdeffery is required.
> 
> Tested successfully by myself on Linux and BSD systems.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Hemant Agrawal <hemant.agrawal@nxp.com>
> CC: Jerin Jacob <Jerin.Jacob@cavium.com>
> CC: Bruce Richardson <bruce.richardson@intel.com>
> CC: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  buildtools/pmdinfogen/pmdinfogen.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Compiles fine on FreeBSD with clang.

Tested-by: Bruce Richardson <bruce.richardson@intel.com>
  
Thomas Monjalon Dec. 1, 2016, 3:17 p.m. UTC | #2
2016-11-21 10:11, Bruce Richardson:
> On Fri, Nov 18, 2016 at 01:47:52PM -0500, Neil Horman wrote:
> > pmdinfogen has a bug in which, during build, it pulls in rte_byteorder.h to
> > obtain the rte macros for byteswapping between the cpu byte order and big or
> > little endian.  Unfortunately, pmdinfogen is a tool that is only meant to be run
> > during the build of dpdk components, and so, it runs on the host.  In cross
> > compile environments however, the rte_byteorder.h is configured using a target
> > cpu, who's endianess may differ from that of the host, leading to improper
> > swapping.
> > 
> > The fix is to use host system defined byte swapping routines rather than the
> > dpdk provided routines.  Note that we are using non posix compliant routines, as
> > the posix compliant api only addresses 16 and 32 bit swaps, and we also need 64
> > bit swaps.  Those macros exist (via endian.h), but BSD and Linux put that header
> > in different locations so some ifdeffery is required.
> > 
> > Tested successfully by myself on Linux and BSD systems.
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Hemant Agrawal <hemant.agrawal@nxp.com>
> > CC: Jerin Jacob <Jerin.Jacob@cavium.com>
> > CC: Bruce Richardson <bruce.richardson@intel.com>
> > CC: Thomas Monjalon <thomas.monjalon@6wind.com>
> > ---
> >  buildtools/pmdinfogen/pmdinfogen.h | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Compiles fine on FreeBSD with clang.
> 
> Tested-by: Bruce Richardson <bruce.richardson@intel.com>

Fixed "endianness" typo, headline, added Fixes:, CC: stable@dpdk.org and removed a trailing whitespace, then
applied, thanks
  

Patch

diff --git a/buildtools/pmdinfogen/pmdinfogen.h b/buildtools/pmdinfogen/pmdinfogen.h
index 1da2966..7c787c1 100644
--- a/buildtools/pmdinfogen/pmdinfogen.h
+++ b/buildtools/pmdinfogen/pmdinfogen.h
@@ -16,12 +16,16 @@ 
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
+#ifdef __linux__ 
+#include <endian.h>
+#else
+#include <sys/endian.h>
+#endif
 #include <fcntl.h>
 #include <unistd.h>
 #include <elf.h>
 #include <rte_config.h>
 #include <rte_pci.h>
-#include <rte_byteorder.h>
 
 /* On BSD-alike OSes elf.h defines these according to host's word size */
 #undef ELF_ST_BIND
@@ -75,9 +79,9 @@ 
 #define CONVERT_NATIVE(fend, width, x) ({ \
 typeof(x) ___x; \
 if ((fend) == ELFDATA2LSB) \
-	___x = rte_le_to_cpu_##width(x); \
+	___x = le##width##toh(x); \
 else \
-	___x = rte_be_to_cpu_##width(x); \
+	___x = be##width##toh(x); \
 	___x; \
 })