Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Bruce Richardson]

On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
> 	does break strict-aliasing rules
> build/include/rte_ip.h:157: note: initialized from here
> 	...
> 
> The root cause is that, compile enable strict aliasing by default,
> while in function rte_raw_cksum() try to convert 'const char *'
> to 'const uint16_t *'.
>

What compiler version is this with? Is there any other way to fix this
other than disabling the compiler warnings. Turning off strict aliasing may
affect performance as it reduces the number of optimizations that the compiler
can perform.

/Bruce

> Need to add CFLAG '-Wno-strict-aliasing' to avoid this issue.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  app/test-pmd/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
> index 97dc2e6..995c874 100644
> --- a/app/test-pmd/Makefile
> +++ b/app/test-pmd/Makefile
> @@ -38,7 +38,7 @@ ifeq ($(CONFIG_RTE_TEST_PMD),y)
>  #
>  APP = testpmd
>  
> -CFLAGS += -O3
> +CFLAGS += -O3 -Wno-strict-aliasing
>  CFLAGS += $(WERROR_FLAGS)
>  
>  ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> -- 
> 1.9.3
> 

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

On 2014/12/3 19:43, Richardson, Bruce wrote:
> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>> 	does break strict-aliasing rules
>> build/include/rte_ip.h:157: note: initialized from here
>> 	...
>>
>> The root cause is that, compile enable strict aliasing by default,
>> while in function rte_raw_cksum() try to convert 'const char *'
>> to 'const uint16_t *'.
>>
> What compiler version is this with? Is there any other way to fix this
> other than disabling the compiler warnings. Turning off strict aliasing may
> affect performance as it reduces the number of optimizations that the compiler
> can perform.

The compile version is:

$ gcc -v
Using built-in specs.
Target: x86_64-redhat-linux
...
gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)


The OS is centos6.5 x86_64


Actually, another possible solution is, as gcc manual shows, use union.
In function rte_raw_cksum() of lib/librte_net/rte_ip.h:

static inline uint16_t
rte_raw_cksum(const char *buf, size_t len){
    union {
        const char *ubuf;
        const uint16_t *uu16;
    } convert;

    convert.ubuf = buf;
    const uint16_t *u16 = convert.uu16;
    ...
}

This may be work, but not test yet.

Any comments about this solution?

Thanks,
Michael
>
> /Bruce
>
>> Need to add CFLAG '-Wno-strict-aliasing' to avoid this issue.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>> ---
>>  app/test-pmd/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
>> index 97dc2e6..995c874 100644
>> --- a/app/test-pmd/Makefile
>> +++ b/app/test-pmd/Makefile
>> @@ -38,7 +38,7 @@ ifeq ($(CONFIG_RTE_TEST_PMD),y)
>>  #
>>  APP = testpmd
>>  
>> -CFLAGS += -O3
>> +CFLAGS += -O3 -Wno-strict-aliasing
>>  CFLAGS += $(WERROR_FLAGS)
>>  
>>  ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>> -- 
>> 1.9.3
>>

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Bruce Richardson]

On Wed, Dec 03, 2014 at 01:59:58PM +0000, Qiu, Michael wrote:
> On 2014/12/3 19:43, Richardson, Bruce wrote:
> > On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
> >> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> >> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
> >> 	does break strict-aliasing rules
> >> build/include/rte_ip.h:157: note: initialized from here
> >> 	...
> >>
> >> The root cause is that, compile enable strict aliasing by default,
> >> while in function rte_raw_cksum() try to convert 'const char *'
> >> to 'const uint16_t *'.
> >>
> > What compiler version is this with? Is there any other way to fix this
> > other than disabling the compiler warnings. Turning off strict aliasing may
> > affect performance as it reduces the number of optimizations that the compiler
> > can perform.
> 
> The compile version is:
> 
> $ gcc -v
> Using built-in specs.
> Target: x86_64-redhat-linux
> ...
> gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)
> 
> 
> The OS is centos6.5 x86_64
> 
> 
> Actually, another possible solution is, as gcc manual shows, use union.
> In function rte_raw_cksum() of lib/librte_net/rte_ip.h:
> 
> static inline uint16_t
> rte_raw_cksum(const char *buf, size_t len){
>     union {
>         const char *ubuf;
>         const uint16_t *uu16;
>     } convert;
> 
>     convert.ubuf = buf;
>     const uint16_t *u16 = convert.uu16;
>     ...
> }
> 
> This may be work, but not test yet.
> 
> Any comments about this solution?

what happens if you make rte_raw_cksum take a void * (or const void *) parameter
instead of "const char *"?

/Bruce

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

On 2014/12/3 22:51, Richardson, Bruce wrote:
> On Wed, Dec 03, 2014 at 01:59:58PM +0000, Qiu, Michael wrote:
>> On 2014/12/3 19:43, Richardson, Bruce wrote:
>>> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
>>>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
>>>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>>>> 	does break strict-aliasing rules
>>>> build/include/rte_ip.h:157: note: initialized from here
>>>> 	...
>>>>
>>>> The root cause is that, compile enable strict aliasing by default,
>>>> while in function rte_raw_cksum() try to convert 'const char *'
>>>> to 'const uint16_t *'.
>>>>
>>> What compiler version is this with? Is there any other way to fix this
>>> other than disabling the compiler warnings. Turning off strict aliasing may
>>> affect performance as it reduces the number of optimizations that the compiler
>>> can perform.
>> The compile version is:
>>
>> $ gcc -v
>> Using built-in specs.
>> Target: x86_64-redhat-linux
>> ...
>> gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)
>>
>>
>> The OS is centos6.5 x86_64
>>
>>
>> Actually, another possible solution is, as gcc manual shows, use union.
>> In function rte_raw_cksum() of lib/librte_net/rte_ip.h:
>>
>> static inline uint16_t
>> rte_raw_cksum(const char *buf, size_t len){
>>     union {
>>         const char *ubuf;
>>         const uint16_t *uu16;
>>     } convert;
>>
>>     convert.ubuf = buf;
>>     const uint16_t *u16 = convert.uu16;
>>     ...
>> }
>>
>> This may be work, but not test yet.
>>
>> Any comments about this solution?
> what happens if you make rte_raw_cksum take a void * (or const void *) parameter
> instead of "const char *"?

"size_t len" is for type char, it is the the array size(for char array
is byte numbers), if we use void *, the meaning maybe confuse I think.
But it should work with other code change.

Thanks,
Michael
>
> /Bruce
>

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Olivier MATZ]

Hi Bruce,

On 12/03/2014 12:42 PM, Bruce Richardson wrote:
> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>> 	does break strict-aliasing rules
>> build/include/rte_ip.h:157: note: initialized from here
>> 	...
>>
>> The root cause is that, compile enable strict aliasing by default,
>> while in function rte_raw_cksum() try to convert 'const char *'
>> to 'const uint16_t *'.
>>
>
> What compiler version is this with? Is there any other way to fix this
> other than disabling the compiler warnings. Turning off strict aliasing may
> affect performance as it reduces the number of optimizations that the compiler
> can perform.

I can reproduce the issue with a gcc-4.4.6 toolchain. But I think it's
a toolchain bug as the warning does not occur with other versions I've
tested.

If it's the case, we could either:

- do nothing: in this case the user need to upgrade its toolchain, or
   pass the -Wno-strict-aliasing manually in EXTRA_CFLAGS

- add the -Wno-strict-aliasing only for gcc 4.4 in the Makefile

What do you think?

Regards,
Olivier

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Bruce Richardson]

On Wed, Dec 03, 2014 at 03:19:34PM +0000, Qiu, Michael wrote:
> On 2014/12/3 22:51, Richardson, Bruce wrote:
> > On Wed, Dec 03, 2014 at 01:59:58PM +0000, Qiu, Michael wrote:
> >> On 2014/12/3 19:43, Richardson, Bruce wrote:
> >>> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
> >>>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> >>>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
> >>>> 	does break strict-aliasing rules
> >>>> build/include/rte_ip.h:157: note: initialized from here
> >>>> 	...
> >>>>
> >>>> The root cause is that, compile enable strict aliasing by default,
> >>>> while in function rte_raw_cksum() try to convert 'const char *'
> >>>> to 'const uint16_t *'.
> >>>>
> >>> What compiler version is this with? Is there any other way to fix this
> >>> other than disabling the compiler warnings. Turning off strict aliasing may
> >>> affect performance as it reduces the number of optimizations that the compiler
> >>> can perform.
> >> The compile version is:
> >>
> >> $ gcc -v
> >> Using built-in specs.
> >> Target: x86_64-redhat-linux
> >> ...
> >> gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)
> >>
> >>
> >> The OS is centos6.5 x86_64
> >>
> >>
> >> Actually, another possible solution is, as gcc manual shows, use union.
> >> In function rte_raw_cksum() of lib/librte_net/rte_ip.h:
> >>
> >> static inline uint16_t
> >> rte_raw_cksum(const char *buf, size_t len){
> >>     union {
> >>         const char *ubuf;
> >>         const uint16_t *uu16;
> >>     } convert;
> >>
> >>     convert.ubuf = buf;
> >>     const uint16_t *u16 = convert.uu16;
> >>     ...
> >> }
> >>
> >> This may be work, but not test yet.
> >>
> >> Any comments about this solution?
> > what happens if you make rte_raw_cksum take a void * (or const void *) parameter
> > instead of "const char *"?
> 
> "size_t len" is for type char, it is the the array size(for char array
> is byte numbers), if we use void *, the meaning maybe confuse I think.
That shouldn't be a big issue. We can rename it to "size" instead of "len" if
needed.

> But it should work with other code change.
> 
> Thanks,
> Michael
> >
> > /Bruce
> >
> 

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Dayu Qiu]

Just re-post this mail as Thomas said it missed in mail list.

On Wed, Dec 3, 2014 at 7:42 PM, Bruce Richardson <bruce.richardson@intel.com
> wrote:

> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
> > app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
> >       does break strict-aliasing rules
> > build/include/rte_ip.h:157: note: initialized from here
> >       ...
> >
> > The root cause is that, compile enable strict aliasing by default,
> > while in function rte_raw_cksum() try to convert 'const char *'
> > to 'const uint16_t *'.
> >
>
> What compiler version is this with? Is there any other way to fix this
> other than disabling the compiler warnings. Turning off strict aliasing may
> affect performance as it reduces the number of optimizations that the
> compiler
> can perform.
>
>

The compile version is:

$ gcc -v
Using built-in specs.
Target: x86_64-redhat-linux
...
gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)


The OS is centos6.5 x86_64


Actually, another possible solution is, as gcc manual shows, use union.
In function rte_raw_cksum() of lib/librte_net/rte_ip.h:

static inline uint16_t
rte_raw_cksum(const char *buf, size_t len){
    union {
        const char *ubuf;
        const uint16_t *uu16;
    } convert;

    convert.ubuf = buf;
    const uint16_t *u16 = convert.uu16;
    ...
}

This may be work, but not test yet.


Thanks,
Michael

> /Bruce
>
> > Need to add CFLAG '-Wno-strict-aliasing' to avoid this issue.
> >
> > Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > ---
> >  app/test-pmd/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
> > index 97dc2e6..995c874 100644
> > --- a/app/test-pmd/Makefile
> > +++ b/app/test-pmd/Makefile
> > @@ -38,7 +38,7 @@ ifeq ($(CONFIG_RTE_TEST_PMD),y)
> >  #
> >  APP = testpmd
> >
> > -CFLAGS += -O3
> > +CFLAGS += -O3 -Wno-strict-aliasing
> >  CFLAGS += $(WERROR_FLAGS)
> >
> >  ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > --
> > 1.9.3
> >
>



-- 
Thanks & Best Regards
Mike

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Dayu Qiu]

Hi Olivier,

You can check gcc manual.

-fstrict-aliasing
           Allow the compiler to assume the strictest aliasing rules
applicable to the language being
           compiled.  For C (and C++), this activates optimizations based
on the type of expressions.  In
           particular, an object of one type is assumed never to reside at
the same address as an object of a
           different type, unless the types are almost the same.  For
example, an "unsigned int" can alias an
           "int", but not a "void*" or a "double".  A character type may
alias any other type.

So it should not be a bug, but I have no idea about why other version does
not reproduce.

Thanks,
Michael

On Wed, Dec 3, 2014 at 11:24 PM, Olivier MATZ <olivier.matz@6wind.com>
wrote:

> Hi Bruce,
>
> On 12/03/2014 12:42 PM, Bruce Richardson wrote:
>
>> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
>>
>>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
>>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>>>         does break strict-aliasing rules
>>> build/include/rte_ip.h:157: note: initialized from here
>>>         ...
>>>
>>> The root cause is that, compile enable strict aliasing by default,
>>> while in function rte_raw_cksum() try to convert 'const char *'
>>> to 'const uint16_t *'.
>>>
>>>
>> What compiler version is this with? Is there any other way to fix this
>> other than disabling the compiler warnings. Turning off strict aliasing
>> may
>> affect performance as it reduces the number of optimizations that the
>> compiler
>> can perform.
>>
>
> I can reproduce the issue with a gcc-4.4.6 toolchain. But I think it's
> a toolchain bug as the warning does not occur with other versions I've
> tested.
>
> If it's the case, we could either:
>
> - do nothing: in this case the user need to upgrade its toolchain, or
>   pass the -Wno-strict-aliasing manually in EXTRA_CFLAGS
>
> - add the -Wno-strict-aliasing only for gcc 4.4 in the Makefile
>
> What do you think?
>
> Regards,
> Olivier
>
>


-- 
Thanks & Best Regards
Mike

Re: [dpdk-dev] [PATCH] test-pmd: Fix "__BYTE_ORDER__" not defined error

[Qiu, Michael]

Hi all,

What about this patch?

It may be some network or mail client issue of me, so not sure this
patch posted to mail list successful.

If failed, I will re-post it later.

Thanks,
Michael

On 2014/12/3 19:28, Michael Qiu wrote:
> app/test-pmd/csumonly.c:84:5: error: "__BYTE_ORDER__" is not defined
> app/test-pmd/csumonly.c:84:23: error: "__ORDER_LITTLE_ENDIAN__" is not defined
>
> This because old gcc version, like 4.4.7, does not has these buildin macros.
>
> $ gcc -v
> Using built-in specs.
> Target: x86_64-redhat-linux
> ...
> gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)
>
> $ echo | gcc -dM -E -| grep "LITTLE"
> (none)
>
> In this situation, we should back to use the macros defined in
> <endian.h>
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  app/test-pmd/csumonly.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 6f43761..3fa81a2 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -81,7 +81,13 @@
>  
>  /* we cannot use htons() from arpa/inet.h due to name conflicts, and we
>   * cannot use rte_cpu_to_be_16() on a constant in a switch/case */
> -#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#ifdef __BYTE_ORDER__
> +#define LITTLE_ENDIAN_CHECK (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> +#else
> +#define LITTLE_ENDIAN_CHECK (__BYTE_ORDER == __LITTLE_ENDIAN)
> +#endif
> +
> +#if LITTLE_ENDIAN_CHECK
>  #define _htons(x) ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8)))
>  #else
>  #define _htons(x) (x)

Re: [dpdk-dev] [PATCH] test-pmd: Fix "__BYTE_ORDER__" not defined error

[Thomas Monjalon]

2014-12-03 16:26, Qiu, Michael:
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -81,7 +81,13 @@
> >  
> >  /* we cannot use htons() from arpa/inet.h due to name conflicts, and we
> >   * cannot use rte_cpu_to_be_16() on a constant in a switch/case */
> > -#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> > +#ifdef __BYTE_ORDER__
> > +#define LITTLE_ENDIAN_CHECK (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> > +#else
> > +#define LITTLE_ENDIAN_CHECK (__BYTE_ORDER == __LITTLE_ENDIAN)
> > +#endif
> > +
> > +#if LITTLE_ENDIAN_CHECK
> >  #define _htons(x) ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8)))
> >  #else
> >  #define _htons(x) (x)

I don't agree we should fix it in testpmd.
It's a more general problem which must be solved in EAL.
I'm preparing a patchset to fix it.

-- 
Thomas

[dpdk-dev] [PATCH 0/2] fix endianness in EAL

[Thomas Monjalon]

It can be hard to implement a reliable detection of endianness.
Previous trials in testpmd failed.
The IBM Power patchset introduced a config option.

This patchset try to improve the situation by having a detection
in EAL headers.

Please test it (especially on IBM Power) with different toolchains
or distributions.

Thomas Monjalon (2):
  eal: detect endianness
  app/testpmd: fix endianness detection

-- 
2.1.3

[dpdk-dev] [PATCH 1/2] eal: detect endianness

[Thomas Monjalon]

There is no standard to check endianness.
So we need to try different checks.
Previous trials were done in testpmd (see commits
51f694dd40f56 and 64741f237cf29) without full success.
This one is not guaranteed to work everywhere so it could
evolve when exceptions are found.

If endianness is not detected, there is a fallback on x86
to little endian. It could be forced before doing detection
but it would add some arch-dependent code in the generic header.

The option CONFIG_RTE_ARCH_BIG_ENDIAN introduced for IBM Power only
(commit a982ec81d84d53) can be removed. A compile-time check is better.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 config/defconfig_ppc_64-power8-linuxapp-gcc        |  1 -
 .../common/include/arch/ppc_64/rte_byteorder.h     |  4 ++--
 .../common/include/arch/x86/rte_byteorder.h        |  4 ++++
 .../common/include/generic/rte_byteorder.h         | 28 ++++++++++++++++++++++
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
index 48018c3..d97a885 100644
--- a/config/defconfig_ppc_64-power8-linuxapp-gcc
+++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
@@ -34,7 +34,6 @@ CONFIG_RTE_MACHINE="power8"
 
 CONFIG_RTE_ARCH="ppc_64"
 CONFIG_RTE_ARCH_PPC_64=y
-CONFIG_RTE_ARCH_BIG_ENDIAN=y
 CONFIG_RTE_ARCH_64=y
 
 CONFIG_RTE_TOOLCHAIN="gcc"
diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h b/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h
index 1a89051..80436f2 100644
--- a/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h
+++ b/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h
@@ -105,7 +105,7 @@ static inline uint64_t rte_arch_bswap64(uint64_t _x)
 /* Power 8 have both little endian and big endian mode
  * Power 7 only support big endian
  */
-#ifndef RTE_ARCH_BIG_ENDIAN
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 
 #define rte_cpu_to_le_16(x) (x)
 #define rte_cpu_to_le_32(x) (x)
@@ -123,7 +123,7 @@ static inline uint64_t rte_arch_bswap64(uint64_t _x)
 #define rte_be_to_cpu_32(x) rte_bswap32(x)
 #define rte_be_to_cpu_64(x) rte_bswap64(x)
 
-#else
+#else /* RTE_BIG_ENDIAN */
 
 #define rte_cpu_to_le_16(x) rte_bswap16(x)
 #define rte_cpu_to_le_32(x) rte_bswap32(x)
diff --git a/lib/librte_eal/common/include/arch/x86/rte_byteorder.h b/lib/librte_eal/common/include/arch/x86/rte_byteorder.h
index 1aa6985..ffdb6ef 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_byteorder.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_byteorder.h
@@ -40,6 +40,10 @@ extern "C" {
 
 #include "generic/rte_byteorder.h"
 
+#ifndef RTE_BYTE_ORDER
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif
+
 /*
  * An architecture-optimized byte swap for a 16-bit value.
  *
diff --git a/lib/librte_eal/common/include/generic/rte_byteorder.h b/lib/librte_eal/common/include/generic/rte_byteorder.h
index 9358136..ea23fdf 100644
--- a/lib/librte_eal/common/include/generic/rte_byteorder.h
+++ b/lib/librte_eal/common/include/generic/rte_byteorder.h
@@ -44,6 +44,34 @@
  */
 
 #include <stdint.h>
+#ifdef RTE_EXEC_ENV_BSDAPP
+#include <sys/endian.h>
+#else
+#include <endian.h>
+#endif
+
+/*
+ * Compile-time endianness detection
+ */
+#define RTE_BIG_ENDIAN    1
+#define RTE_LITTLE_ENDIAN 2
+#if defined __BYTE_ORDER
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
+#elif __BYTE_ORDER == __LITTLE_ENDIAN
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif /* __BYTE_ORDER */
+#elif defined __BYTE_ORDER__
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
+#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif /* __BYTE_ORDER__ */
+#elif defined __BIG_ENDIAN__
+#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
+#elif defined __LITTLE_ENDIAN__
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif
 
 /*
  * An internal function to swap bytes in a 16-bit value.
-- 
2.1.3

[dpdk-dev] [PATCH 2/2] app/testpmd: fix endianness detection

[Thomas Monjalon]

Use endianness detection factorized in EAL.

The comment about arpa/inet.h is not valid anymore since
commit d07180f211c08 ("net: fix conflict with libc").

The macro _htons could also be moved in rte_byteorder.h
by providing some constant byte swapping.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test-pmd/config.c   | 2 +-
 app/test-pmd/csumonly.c | 5 ++---
 app/test-pmd/testpmd.h  | 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f0b770e..69a83c2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -628,7 +628,7 @@ ring_dma_zone_lookup(const char *ring_name, uint8_t port_id, uint16_t q_id)
 union igb_ring_dword {
 	uint64_t dword;
 	struct {
-#ifdef RTE_ARCH_BIG_ENDIAN
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
 		uint32_t lo;
 		uint32_t hi;
 #else
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 6f43761..da4bca4 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -79,9 +79,8 @@
 #define IP_HDRLEN  0x05 /* default IP header length == five 32-bits words. */
 #define IP_VHL_DEF (IP_VERSION | IP_HDRLEN)
 
-/* we cannot use htons() from arpa/inet.h due to name conflicts, and we
- * cannot use rte_cpu_to_be_16() on a constant in a switch/case */
-#if  __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+/* We cannot use rte_cpu_to_be_16() on a constant in a switch/case */
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 #define _htons(x) ((uint16_t)((((x) & 0x00ffU) << 8) | (((x) & 0xff00U) >> 8)))
 #else
 #define _htons(x) (x)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index c925910..f8b0740 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -567,7 +567,7 @@ int tx_queue_id_is_invalid(queueid_t txq_id);
 #define RTE_BE_TO_CPU_16(be_16_v)  rte_be_to_cpu_16((be_16_v))
 #define RTE_CPU_TO_BE_16(cpu_16_v) rte_cpu_to_be_16((cpu_16_v))
 #else
-#ifdef __big_endian__
+#if RTE_BYTE_ORDER == RTE_BIG_ENDIAN
 #define RTE_BE_TO_CPU_16(be_16_v)  (be_16_v)
 #define RTE_CPU_TO_BE_16(cpu_16_v) (cpu_16_v)
 #else
-- 
2.1.3

Re: [dpdk-dev] [PATCH 1/2] eal: detect endianness

[Qiu, Michael]

On 12/4/2014 5:26 AM, Thomas Monjalon wrote:
> There is no standard to check endianness.
> So we need to try different checks.
> Previous trials were done in testpmd (see commits
> 51f694dd40f56 and 64741f237cf29) without full success.
> This one is not guaranteed to work everywhere so it could
> evolve when exceptions are found.
>
> If endianness is not detected, there is a fallback on x86
> to little endian. It could be forced before doing detection
> but it would add some arch-dependent code in the generic header.
>
> The option CONFIG_RTE_ARCH_BIG_ENDIAN introduced for IBM Power only
> (commit a982ec81d84d53) can be removed. A compile-time check is better.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  config/defconfig_ppc_64-power8-linuxapp-gcc        |  1 -
>  .../common/include/arch/ppc_64/rte_byteorder.h     |  4 ++--
>  .../common/include/arch/x86/rte_byteorder.h        |  4 ++++
>  .../common/include/generic/rte_byteorder.h         | 28 ++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
> index 48018c3..d97a885 100644
> --- a/config/defconfig_ppc_64-power8-linuxapp-gcc
> +++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
> @@ -34,7 +34,6 @@ CONFIG_RTE_MACHINE="power8"
>  
>  CONFIG_RTE_ARCH="ppc_64"
>  CONFIG_RTE_ARCH_PPC_64=y
> -CONFIG_RTE_ARCH_BIG_ENDIAN=y
>  CONFIG_RTE_ARCH_64=y
>  
>  CONFIG_RTE_TOOLCHAIN="gcc"
> diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h b/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h
> index 1a89051..80436f2 100644
> --- a/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h
> +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_byteorder.h
> @@ -105,7 +105,7 @@ static inline uint64_t rte_arch_bswap64(uint64_t _x)
>  /* Power 8 have both little endian and big endian mode
>   * Power 7 only support big endian
>   */
> -#ifndef RTE_ARCH_BIG_ENDIAN
> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
>  
>  #define rte_cpu_to_le_16(x) (x)
>  #define rte_cpu_to_le_32(x) (x)
> @@ -123,7 +123,7 @@ static inline uint64_t rte_arch_bswap64(uint64_t _x)
>  #define rte_be_to_cpu_32(x) rte_bswap32(x)
>  #define rte_be_to_cpu_64(x) rte_bswap64(x)
>  
> -#else
> +#else /* RTE_BIG_ENDIAN */
>  
>  #define rte_cpu_to_le_16(x) rte_bswap16(x)
>  #define rte_cpu_to_le_32(x) rte_bswap32(x)
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_byteorder.h b/lib/librte_eal/common/include/arch/x86/rte_byteorder.h
> index 1aa6985..ffdb6ef 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_byteorder.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_byteorder.h
> @@ -40,6 +40,10 @@ extern "C" {
>  
>  #include "generic/rte_byteorder.h"
>  
> +#ifndef RTE_BYTE_ORDER
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif
> +
>  /*
>   * An architecture-optimized byte swap for a 16-bit value.
>   *
> diff --git a/lib/librte_eal/common/include/generic/rte_byteorder.h b/lib/librte_eal/common/include/generic/rte_byteorder.h
> index 9358136..ea23fdf 100644
> --- a/lib/librte_eal/common/include/generic/rte_byteorder.h
> +++ b/lib/librte_eal/common/include/generic/rte_byteorder.h
> @@ -44,6 +44,34 @@
>   */
>  
>  #include <stdint.h>
> +#ifdef RTE_EXEC_ENV_BSDAPP
> +#include <sys/endian.h>
> +#else
> +#include <endian.h>
> +#endif
> +
> +/*
> + * Compile-time endianness detection
> + */
> +#define RTE_BIG_ENDIAN    1
> +#define RTE_LITTLE_ENDIAN 2
> +#if defined __BYTE_ORDER
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif /* __BYTE_ORDER */
> +#elif defined __BYTE_ORDER__
> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif /* __BYTE_ORDER__ */
> +#elif defined __BIG_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> +#elif defined __LITTLE_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif

What do you think about :

+/*
+  * Compile-time endianness detection
+ */
+#define RTE_BIG_ENDIAN 1
+#define RTE_LITTLE_ENDIAN 2
+if defined __BYTE_ORDER__    /* Prefer gcc build-in macros */
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
+#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif /* __BYTE_ORDER__ */
+#else
+#if defined RTE_EXEC_ENV_BSDAPP
+#include <sys/endian.h>
+#else
+#include <endian.h>
+#endif
+#if defined __BYTE_ORDER
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
+#elif __BYTE_ORDER == __LITTLE_ENDIAN
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif /* __BYTE_ORDER */
+#elif defined __BIG_ENDIAN__
+#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
+#elif defined __LITTLE_ENDIAN__
+#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
+#endif
+#endif
>  
>  /*
>   * An internal function to swap bytes in a 16-bit value.

Re: [dpdk-dev] [PATCH] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

On 12/3/2014 11:36 PM, Richardson, Bruce wrote:
> On Wed, Dec 03, 2014 at 03:19:34PM +0000, Qiu, Michael wrote:
>> On 2014/12/3 22:51, Richardson, Bruce wrote:
>>> On Wed, Dec 03, 2014 at 01:59:58PM +0000, Qiu, Michael wrote:
>>>> On 2014/12/3 19:43, Richardson, Bruce wrote:
>>>>> On Wed, Dec 03, 2014 at 07:28:19PM +0800, Michael Qiu wrote:
>>>>>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
>>>>>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>>>>>> 	does break strict-aliasing rules
>>>>>> build/include/rte_ip.h:157: note: initialized from here
>>>>>> 	...
>>>>>>
>>>>>> The root cause is that, compile enable strict aliasing by default,
>>>>>> while in function rte_raw_cksum() try to convert 'const char *'
>>>>>> to 'const uint16_t *'.
>>>>>>
>>>>> What compiler version is this with? Is there any other way to fix this
>>>>> other than disabling the compiler warnings. Turning off strict aliasing may
>>>>> affect performance as it reduces the number of optimizations that the compiler
>>>>> can perform.
>>>> The compile version is:
>>>>
>>>> $ gcc -v
>>>> Using built-in specs.
>>>> Target: x86_64-redhat-linux
>>>> ...
>>>> gcc version 4.4.7 20120313 (Red Hat 4.4.7-4) (GCC)
>>>>
>>>>
>>>> The OS is centos6.5 x86_64
>>>>
>>>>
>>>> Actually, another possible solution is, as gcc manual shows, use union.
>>>> In function rte_raw_cksum() of lib/librte_net/rte_ip.h:
>>>>
>>>> static inline uint16_t
>>>> rte_raw_cksum(const char *buf, size_t len){
>>>>     union {
>>>>         const char *ubuf;
>>>>         const uint16_t *uu16;
>>>>     } convert;
>>>>
>>>>     convert.ubuf = buf;
>>>>     const uint16_t *u16 = convert.uu16;
>>>>     ...
>>>> }
>>>>
>>>> This may be work, but not test yet.
>>>>
>>>> Any comments about this solution?
>>> what happens if you make rte_raw_cksum take a void * (or const void *) parameter
>>> instead of "const char *"?
>> "size_t len" is for type char, it is the the array size(for char array
>> is byte numbers), if we use void *, the meaning maybe confuse I think.
> That shouldn't be a big issue. We can rename it to "size" instead of "len" if
> needed.

As I tested in my env just, void * is still not work, the same errors
post when compile.

Then the solution for DPDK should be:
-  As Olivier suggest, add the -Wno-strict-aliasing only for gcc 4.4 in
the Makefile
- As gcc manual suggest, use "union" to work around this issue.

Thanks,
Michael
>> But it should work with other code change.
>>
>> Thanks,
>> Michael
>>> /Bruce
>>>

[dpdk-dev] [PATCH v2] test-pmd: Fix pointer aliasing error

[Michael Qiu]

app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
        does break strict-aliasing rules
build/include/rte_ip.h:157: note: initialized from here
        ...

The root cause is that, compile enable strict aliasing by default,
while in function rte_raw_cksum() try to convert 'const char *'
to 'const uint16_t *'.

This patch is one workaround fix.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 lib/librte_net/rte_ip.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 61e4457..f1c7087 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -154,7 +154,8 @@ struct ipv4_hdr {
 static inline uint16_t
 rte_raw_cksum(const char *buf, size_t len)
 {
-	const uint16_t *u16 = (const uint16_t *)buf;
+	unsigned long ptr = (unsigned long)buf;
+	const uint16_t *u16 = (const uint16_t *)ptr;
 	uint32_t sum = 0;
 
 	while (len >= (sizeof(*u16) * 4)) {
-- 
1.9.3

[dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Michael Qiu]

app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
        does break strict-aliasing rules
build/include/rte_ip.h:157: note: initialized from here
        ...

The root cause is that, compile enable strict aliasing by default,
while in function rte_raw_cksum() try to convert 'const char *'
to 'const uint16_t *'.

This patch is one workaround fix.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
v3 --> v2:
	use uintptr_t instead of unsigned long to
	save pointer.

v2 --> v1:
	Workaround solution instead of shut off the
	gcc params.

 lib/librte_net/rte_ip.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index 61e4457..cda3436 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -154,7 +154,8 @@ struct ipv4_hdr {
 static inline uint16_t
 rte_raw_cksum(const char *buf, size_t len)
 {
-	const uint16_t *u16 = (const uint16_t *)buf;
+	uintptr_t ptr = (uintptr_t)buf;
+	const uint16_t *u16 = (const uint16_t *)ptr;
 	uint32_t sum = 0;
 
 	while (len >= (sizeof(*u16) * 4)) {
-- 
1.9.3

Re: [dpdk-dev] [PATCH 1/2] eal: detect endianness

[Thomas Monjalon]

2014-12-04 02:28, Qiu, Michael:
> On 12/4/2014 5:26 AM, Thomas Monjalon wrote:
> > There is no standard to check endianness.
> > So we need to try different checks.
> > Previous trials were done in testpmd (see commits
> > 51f694dd40f56 and 64741f237cf29) without full success.
> > This one is not guaranteed to work everywhere so it could
> > evolve when exceptions are found.
[...]
> >  #include <stdint.h>
> > +#ifdef RTE_EXEC_ENV_BSDAPP
> > +#include <sys/endian.h>
> > +#else
> > +#include <endian.h>
> > +#endif
> > +
> > +/*
> > + * Compile-time endianness detection
> > + */
> > +#define RTE_BIG_ENDIAN    1
> > +#define RTE_LITTLE_ENDIAN 2
> > +#if defined __BYTE_ORDER
> > +#if __BYTE_ORDER == __BIG_ENDIAN
> > +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> > +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> > +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> > +#endif /* __BYTE_ORDER */
> > +#elif defined __BYTE_ORDER__
> > +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> > +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> > +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> > +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> > +#endif /* __BYTE_ORDER__ */
> > +#elif defined __BIG_ENDIAN__
> > +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> > +#elif defined __LITTLE_ENDIAN__
> > +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> > +#endif
> 
> What do you think about :
> 
> +/*
> +  * Compile-time endianness detection
> + */
> +#define RTE_BIG_ENDIAN 1
> +#define RTE_LITTLE_ENDIAN 2
> +if defined __BYTE_ORDER__    /* Prefer gcc build-in macros */
> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif /* __BYTE_ORDER__ */
> +#else
> +#if defined RTE_EXEC_ENV_BSDAPP
> +#include <sys/endian.h>
> +#else
> +#include <endian.h>
> +#endif
> +#if defined __BYTE_ORDER
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif /* __BYTE_ORDER */
> +#elif defined __BIG_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> +#elif defined __LITTLE_ENDIAN__
> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> +#endif
> +#endif

Please, could you give more explanations about your proposal?
Why not always try to include endian.h?
Why giving high priority to __BYTE_ORDER__?

-- 
Thomas

Re: [dpdk-dev] [PATCH 0/2] fix endianness in EAL

[Chao Zhu]

On 2014/12/4 4:47, Thomas Monjalon wrote:
> It can be hard to implement a reliable detection of endianness.
> Previous trials in testpmd failed.
> The IBM Power patchset introduced a config option.
>
> This patchset try to improve the situation by having a detection
> in EAL headers.
>
> Please test it (especially on IBM Power) with different toolchains
> or distributions.
>
> Thomas Monjalon (2):
>    eal: detect endianness
>    app/testpmd: fix endianness detection
>
I tested it on IBM Power with GCC 4.8. It works fine.
If there is no other better way to do the detection, I think this patch 
is good enough.

Acked-by: Chao Zhu<chaozhu@linux.vnet.ibm.com>

Re: [dpdk-dev] [PATCH 1/2] eal: detect endianness

[Qiu, Michael]

On 12/4/2014 5:01 PM, Thomas Monjalon wrote:
> 2014-12-04 02:28, Qiu, Michael:
>> On 12/4/2014 5:26 AM, Thomas Monjalon wrote:
>>> There is no standard to check endianness.
>>> So we need to try different checks.
>>> Previous trials were done in testpmd (see commits
>>> 51f694dd40f56 and 64741f237cf29) without full success.
>>> This one is not guaranteed to work everywhere so it could
>>> evolve when exceptions are found.
> [...]
>>>  #include <stdint.h>
>>> +#ifdef RTE_EXEC_ENV_BSDAPP
>>> +#include <sys/endian.h>
>>> +#else
>>> +#include <endian.h>
>>> +#endif
>>> +
>>> +/*
>>> + * Compile-time endianness detection
>>> + */
>>> +#define RTE_BIG_ENDIAN    1
>>> +#define RTE_LITTLE_ENDIAN 2
>>> +#if defined __BYTE_ORDER
>>> +#if __BYTE_ORDER == __BIG_ENDIAN
>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>> +#endif /* __BYTE_ORDER */
>>> +#elif defined __BYTE_ORDER__
>>> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>> +#endif /* __BYTE_ORDER__ */
>>> +#elif defined __BIG_ENDIAN__
>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>> +#elif defined __LITTLE_ENDIAN__
>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>> +#endif
>> What do you think about :
>>
>> +/*
>> +  * Compile-time endianness detection
>> + */
>> +#define RTE_BIG_ENDIAN 1
>> +#define RTE_LITTLE_ENDIAN 2
>> +if defined __BYTE_ORDER__    /* Prefer gcc build-in macros */
>> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>> +#endif /* __BYTE_ORDER__ */
>> +#else
>> +#if defined RTE_EXEC_ENV_BSDAPP
>> +#include <sys/endian.h>
>> +#else
>> +#include <endian.h>
>> +#endif
>> +#if defined __BYTE_ORDER
>> +#if __BYTE_ORDER == __BIG_ENDIAN
>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>> +#endif /* __BYTE_ORDER */
>> +#elif defined __BIG_ENDIAN__
>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>> +#elif defined __LITTLE_ENDIAN__
>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>> +#endif
>> +#endif
> Please, could you give more explanations about your proposal?
> Why not always try to include endian.h?

I assume that if gcc can handler why we need include that file?

Also it seems that only old version could have this issue, newer
versions has build in this marcos.

So that's why I prefer  "__BYTE_ORDER__" for high priority.

Thanks,
Michael
> Why giving high priority to __BYTE_ORDER__?
>

Re: [dpdk-dev] [PATCH 1/2] eal: detect endianness

[Thomas Monjalon]

2014-12-04 10:28, Qiu, Michael:
> On 12/4/2014 5:01 PM, Thomas Monjalon wrote:
> > 2014-12-04 02:28, Qiu, Michael:
> >> On 12/4/2014 5:26 AM, Thomas Monjalon wrote:
> >>> There is no standard to check endianness.
> >>> So we need to try different checks.
> >>> Previous trials were done in testpmd (see commits
> >>> 51f694dd40f56 and 64741f237cf29) without full success.
> >>> This one is not guaranteed to work everywhere so it could
> >>> evolve when exceptions are found.
> > [...]
> >>>  #include <stdint.h>
> >>> +#ifdef RTE_EXEC_ENV_BSDAPP
> >>> +#include <sys/endian.h>
> >>> +#else
> >>> +#include <endian.h>
> >>> +#endif
> >>> +
> >>> +/*
> >>> + * Compile-time endianness detection
> >>> + */
> >>> +#define RTE_BIG_ENDIAN    1
> >>> +#define RTE_LITTLE_ENDIAN 2
> >>> +#if defined __BYTE_ORDER
> >>> +#if __BYTE_ORDER == __BIG_ENDIAN
> >>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> >>> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> >>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> >>> +#endif /* __BYTE_ORDER */
> >>> +#elif defined __BYTE_ORDER__
> >>> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> >>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> >>> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> >>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> >>> +#endif /* __BYTE_ORDER__ */
> >>> +#elif defined __BIG_ENDIAN__
> >>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> >>> +#elif defined __LITTLE_ENDIAN__
> >>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> >>> +#endif
> >> What do you think about :
> >>
> >> +/*
> >> +  * Compile-time endianness detection
> >> + */
> >> +#define RTE_BIG_ENDIAN 1
> >> +#define RTE_LITTLE_ENDIAN 2
> >> +if defined __BYTE_ORDER__    /* Prefer gcc build-in macros */
> >> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> >> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> >> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> >> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> >> +#endif /* __BYTE_ORDER__ */
> >> +#else
> >> +#if defined RTE_EXEC_ENV_BSDAPP
> >> +#include <sys/endian.h>
> >> +#else
> >> +#include <endian.h>
> >> +#endif
> >> +#if defined __BYTE_ORDER
> >> +#if __BYTE_ORDER == __BIG_ENDIAN
> >> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> >> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> >> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> >> +#endif /* __BYTE_ORDER */
> >> +#elif defined __BIG_ENDIAN__
> >> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
> >> +#elif defined __LITTLE_ENDIAN__
> >> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
> >> +#endif
> >> +#endif
> > 
> > Please, could you give more explanations about your proposal?
> > Why not always try to include endian.h?
> 
> I assume that if gcc can handler why we need include that file?

Separating include on top is easier to read, and I'm not sure it won't
be needed for __BYTE_ORDER__ with some toolchains.

> Also it seems that only old version could have this issue, newer
> versions has build in this marcos.
> 
> So that's why I prefer  "__BYTE_ORDER__" for high priority.

I have no problem with reversing this priority.

> > Why giving high priority to __BYTE_ORDER__?

Any other comment? May I apply with above change?

-- 
Thomas

Re: [dpdk-dev] [PATCH 1/2] eal: detect endianness

[Qiu, Michael]

On 12/4/2014 8:20 PM, Thomas Monjalon wrote:
> 2014-12-04 10:28, Qiu, Michael:
>> On 12/4/2014 5:01 PM, Thomas Monjalon wrote:
>>> 2014-12-04 02:28, Qiu, Michael:
>>>> On 12/4/2014 5:26 AM, Thomas Monjalon wrote:
>>>>> There is no standard to check endianness.
>>>>> So we need to try different checks.
>>>>> Previous trials were done in testpmd (see commits
>>>>> 51f694dd40f56 and 64741f237cf29) without full success.
>>>>> This one is not guaranteed to work everywhere so it could
>>>>> evolve when exceptions are found.
>>> [...]
>>>>>  #include <stdint.h>
>>>>> +#ifdef RTE_EXEC_ENV_BSDAPP
>>>>> +#include <sys/endian.h>
>>>>> +#else
>>>>> +#include <endian.h>
>>>>> +#endif
>>>>> +
>>>>> +/*
>>>>> + * Compile-time endianness detection
>>>>> + */
>>>>> +#define RTE_BIG_ENDIAN    1
>>>>> +#define RTE_LITTLE_ENDIAN 2
>>>>> +#if defined __BYTE_ORDER
>>>>> +#if __BYTE_ORDER == __BIG_ENDIAN
>>>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>>>> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
>>>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>>>> +#endif /* __BYTE_ORDER */
>>>>> +#elif defined __BYTE_ORDER__
>>>>> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>>>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>>>> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>>>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>>>> +#endif /* __BYTE_ORDER__ */
>>>>> +#elif defined __BIG_ENDIAN__
>>>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>>>> +#elif defined __LITTLE_ENDIAN__
>>>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>>>> +#endif
>>>> What do you think about :
>>>>
>>>> +/*
>>>> +  * Compile-time endianness detection
>>>> + */
>>>> +#define RTE_BIG_ENDIAN 1
>>>> +#define RTE_LITTLE_ENDIAN 2
>>>> +if defined __BYTE_ORDER__    /* Prefer gcc build-in macros */
>>>> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
>>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>>> +#elif __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
>>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>>> +#endif /* __BYTE_ORDER__ */
>>>> +#else
>>>> +#if defined RTE_EXEC_ENV_BSDAPP
>>>> +#include <sys/endian.h>
>>>> +#else
>>>> +#include <endian.h>
>>>> +#endif
>>>> +#if defined __BYTE_ORDER
>>>> +#if __BYTE_ORDER == __BIG_ENDIAN
>>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>>> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
>>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>>> +#endif /* __BYTE_ORDER */
>>>> +#elif defined __BIG_ENDIAN__
>>>> +#define RTE_BYTE_ORDER RTE_BIG_ENDIAN
>>>> +#elif defined __LITTLE_ENDIAN__
>>>> +#define RTE_BYTE_ORDER RTE_LITTLE_ENDIAN
>>>> +#endif
>>>> +#endif
>>> Please, could you give more explanations about your proposal?
>>> Why not always try to include endian.h?
>> I assume that if gcc can handler why we need include that file?
> Separating include on top is easier to read, and I'm not sure it won't
> be needed for __BYTE_ORDER__ with some toolchains.
>
>> Also it seems that only old version could have this issue, newer
>> versions has build in this marcos.
>>
>> So that's why I prefer  "__BYTE_ORDER__" for high priority.
> I have no problem with reversing this priority.
>
>>> Why giving high priority to __BYTE_ORDER__?
> Any other comment? May I apply with above change?

Acked-by: Michael Qiu <michael.qiu@intel.com>

Re: [dpdk-dev] [PATCH v2] test-pmd: Fix pointer aliasing error

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michael Qiu
> Sent: Thursday, December 04, 2014 3:29 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] test-pmd: Fix pointer aliasing error
> 
> 
> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>         does break strict-aliasing rules
> build/include/rte_ip.h:157: note: initialized from here
>         ...
> 
> The root cause is that, compile enable strict aliasing by default,
> while in function rte_raw_cksum() try to convert 'const char *'
> to 'const uint16_t *'.
> 
> This patch is one workaround fix.
> 
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  lib/librte_net/rte_ip.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index 61e4457..f1c7087 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -154,7 +154,8 @@ struct ipv4_hdr {
>  static inline uint16_t
>  rte_raw_cksum(const char *buf, size_t len)
>  {
> -	const uint16_t *u16 = (const uint16_t *)buf;
> +	unsigned long ptr = (unsigned long)buf;

I think better use uintptr_t here.
It is specially designed to be long enough to hold pointer value.

> +	const uint16_t *u16 = (const uint16_t *)ptr;
>  	uint32_t sum = 0;
> 
>  	while (len >= (sizeof(*u16) * 4)) {
> --
> 1.9.3

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

Any comments about this version? a new workaround solution :)

Thanks,
Michael
On 12/4/2014 9:35 PM, Michael Qiu wrote:
> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>         does break strict-aliasing rules
> build/include/rte_ip.h:157: note: initialized from here
>         ...
>
> The root cause is that, compile enable strict aliasing by default,
> while in function rte_raw_cksum() try to convert 'const char *'
> to 'const uint16_t *'.
>
> This patch is one workaround fix.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
> v3 --> v2:
> 	use uintptr_t instead of unsigned long to
> 	save pointer.
>
> v2 --> v1:
> 	Workaround solution instead of shut off the
> 	gcc params.
>
>  lib/librte_net/rte_ip.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index 61e4457..cda3436 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -154,7 +154,8 @@ struct ipv4_hdr {
>  static inline uint16_t
>  rte_raw_cksum(const char *buf, size_t len)
>  {
> -	const uint16_t *u16 = (const uint16_t *)buf;
> +	uintptr_t ptr = (uintptr_t)buf;
> +	const uint16_t *u16 = (const uint16_t *)ptr;
>  	uint32_t sum = 0;
>  
>  	while (len >= (sizeof(*u16) * 4)) {

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Thomas Monjalon]

2014-12-05 05:34, Qiu, Michael:
> Any comments about this version? a new workaround solution :)

Yes, one comment: I think it's ugly :)
These aliasing errors are not reliable so I think we can disable it (like
Linux does).
But in case you don't want to disable the warning, please add a comment to your
workaround to explain it is caused by GCC strict-aliasing check.

> > -	const uint16_t *u16 = (const uint16_t *)buf;
> > +	uintptr_t ptr = (uintptr_t)buf;
> > +	const uint16_t *u16 = (const uint16_t *)ptr;

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH 0/2] fix endianness in EAL

[Thomas Monjalon]

> > It can be hard to implement a reliable detection of endianness.
> > Previous trials in testpmd failed.
> > The IBM Power patchset introduced a config option.
> >
> > This patchset try to improve the situation by having a detection
> > in EAL headers.
> >
> > Please test it (especially on IBM Power) with different toolchains
> > or distributions.
> >
> > Thomas Monjalon (2):
> >    eal: detect endianness
> >    app/testpmd: fix endianness detection
> >
> I tested it on IBM Power with GCC 4.8. It works fine.
> If there is no other better way to do the detection, I think this patch 
> is good enough.
> 
> Acked-by: Chao Zhu<chaozhu@linux.vnet.ibm.com>

Applied
-- 
Thomas

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

On 12/5/2014 5:26 PM, Thomas Monjalon wrote:
> 2014-12-05 05:34, Qiu, Michael:
>> Any comments about this version? a new workaround solution :)
> Yes, one comment: I think it's ugly :)
> These aliasing errors are not reliable so I think we can disable it (like
> Linux does).
> But in case you don't want to disable the warning, please add a comment to your
> workaround to explain it is caused by GCC strict-aliasing check.

Yes, really ugly ....

But lots of reviewer voted against with disable it as my first version
is disable.

I could not think out a better solution for all reviewers can accept.

I will add the comment with the thread of v3 patch.

Thanks,
Michael

>>> -	const uint16_t *u16 = (const uint16_t *)buf;
>>> +	uintptr_t ptr = (uintptr_t)buf;
>>> +	const uint16_t *u16 = (const uint16_t *)ptr;

> Thanks

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

On 12/4/2014 9:35 PM, Michael Qiu wrote:
> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>         does break strict-aliasing rules
> build/include/rte_ip.h:157: note: initialized from here
>         ...
>
> The root cause is that, compile enable strict aliasing by default,
> while in function rte_raw_cksum() try to convert 'const char *'
> to 'const uint16_t *'.
>
> This patch is one workaround fix.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
> v3 --> v2:
> 	use uintptr_t instead of unsigned long to
> 	save pointer.
>
> v2 --> v1:
> 	Workaround solution instead of shut off the
> 	gcc params.
>
>  lib/librte_net/rte_ip.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
> index 61e4457..cda3436 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -154,7 +154,8 @@ struct ipv4_hdr {
>  static inline uint16_t
>  rte_raw_cksum(const char *buf, size_t len)
>  {
> -	const uint16_t *u16 = (const uint16_t *)buf;
> +	uintptr_t ptr = (uintptr_t)buf;
> +	const uint16_t *u16 = (const uint16_t *)ptr;
>  	uint32_t sum = 0;
>  
>  	while (len >= (sizeof(*u16) * 4)) {
This workaround is to solve the compile issue of GCC strict-aliasing(Two
different type pointers should not be point to the same memory address).

For GCC 4.4.7 it will definitely occurs if  flags "-fstrict-aliasing"
and "-Wall" used.

Thanks,
Michael

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

Hi Thomas,

What's going on with this patch?

I really do not have other better solution.

Thanks,
Michael
On 12/8/2014 9:30 AM, Qiu, Michael wrote:
> On 12/4/2014 9:35 PM, Michael Qiu wrote:
>> app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
>> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
>>         does break strict-aliasing rules
>> build/include/rte_ip.h:157: note: initialized from here
>>         ...
>>
>> The root cause is that, compile enable strict aliasing by default,
>> while in function rte_raw_cksum() try to convert 'const char *'
>> to 'const uint16_t *'.
>>
>> This patch is one workaround fix.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>> ---
>> v3 --> v2:
>> 	use uintptr_t instead of unsigned long to
>> 	save pointer.
>>
>> v2 --> v1:
>> 	Workaround solution instead of shut off the
>> 	gcc params.
>>
>>  lib/librte_net/rte_ip.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
>> index 61e4457..cda3436 100644
>> --- a/lib/librte_net/rte_ip.h
>> +++ b/lib/librte_net/rte_ip.h
>> @@ -154,7 +154,8 @@ struct ipv4_hdr {
>>  static inline uint16_t
>>  rte_raw_cksum(const char *buf, size_t len)
>>  {
>> -	const uint16_t *u16 = (const uint16_t *)buf;
>> +	uintptr_t ptr = (uintptr_t)buf;
>> +	const uint16_t *u16 = (const uint16_t *)ptr;
>>  	uint32_t sum = 0;
>>  
>>  	while (len >= (sizeof(*u16) * 4)) {
> This workaround is to solve the compile issue of GCC strict-aliasing(Two
> different type pointers should not be point to the same memory address).
>
> For GCC 4.4.7 it will definitely occurs if  flags "-fstrict-aliasing"
> and "-Wall" used.
>
> Thanks,
> Michael
>

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Thomas Monjalon]

> > app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
> >         does break strict-aliasing rules
> > build/include/rte_ip.h:157: note: initialized from here
> >         ...
> >
> > The root cause is that, compile enable strict aliasing by default,
> > while in function rte_raw_cksum() try to convert 'const char *'
> > to 'const uint16_t *'.
> >
> > This patch is one workaround fix.
> >
> > Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > ---
> > v3 --> v2:
> > 	use uintptr_t instead of unsigned long to
> > 	save pointer.
> >
> > v2 --> v1:
> > 	Workaround solution instead of shut off the
> > 	gcc params.
> 
> This workaround is to solve the compile issue of GCC strict-aliasing(Two
> different type pointers should not be point to the same memory address).
> 
> For GCC 4.4.7 it will definitely occurs if  flags "-fstrict-aliasing"
> and "-Wall" used.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>> 

Applied with a comment in the code.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[r k]

Thomas, Michael,

Wouldn't it cause unaligned memory access (new changes as well as the
previous code)? Wondering if get_unaligned/put_unaligned macros similar to
the ones used in kernel be ported to user-space?

Thanks,
Ravi

On Wed, Dec 10, 2014 at 4:54 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:
>
> > > app/test-pmd/csumonly.c: In function 'get_psd_sum':
> > > build/include/rte_ip.h:161: error: dereferencing pointer 'u16'
> > >         does break strict-aliasing rules
> > > build/include/rte_ip.h:157: note: initialized from here
> > >         ...
> > >
> > > The root cause is that, compile enable strict aliasing by default,
> > > while in function rte_raw_cksum() try to convert 'const char *'
> > > to 'const uint16_t *'.
> > >
> > > This patch is one workaround fix.
> > >
> > > Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> > > ---
> > > v3 --> v2:
> > >     use uintptr_t instead of unsigned long to
> > >     save pointer.
> > >
> > > v2 --> v1:
> > >     Workaround solution instead of shut off the
> > >     gcc params.
> >
> > This workaround is to solve the compile issue of GCC strict-aliasing(Two
> > different type pointers should not be point to the same memory address).
> >
> > For GCC 4.4.7 it will definitely occurs if  flags "-fstrict-aliasing"
> > and "-Wall" used.
>
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>>
>
> Applied with a comment in the code.
>
> Thanks
> --
> Thomas
>

Re: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error

[Qiu, Michael]

On 2014/12/12 1:51, r k wrote:
Thomas, Michael,

Wouldn't it cause unaligned memory access (new changes as well as the previous code)? Wondering if get_unaligned/put_unaligned macros similar to the ones used in kernel be ported to user-space?


I think it will not, as all buf point to are struct udp_hdr/struct tcp_hdr/struct ipv6_psd_header/struct ipv4_psd_header, they are all aligned with uint16_t.

Thanks
Michael
Thanks,
Ravi

On Wed, Dec 10, 2014 at 4:54 PM, Thomas Monjalon <thomas.monjalon@6wind.com<mailto:thomas.monjalon@6wind.com>> wrote:
> > app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
> > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
> >         does break strict-aliasing rules
> > build/include/rte_ip.h:157: note: initialized from here
> >         ...
> >
> > The root cause is that, compile enable strict aliasing by default,
> > while in function rte_raw_cksum() try to convert 'const char *'
> > to 'const uint16_t *'.
> >
> > This patch is one workaround fix.
> >
> > Signed-off-by: Michael Qiu <michael.qiu@intel.com<mailto:michael.qiu@intel.com>>
> > ---
> > v3 --> v2:
> >     use uintptr_t instead of unsigned long to
> >     save pointer.
> >
> > v2 --> v1:
> >     Workaround solution instead of shut off the
> >     gcc params.
>
> This workaround is to solve the compile issue of GCC strict-aliasing(Two
> different type pointers should not be point to the same memory address).
>
> For GCC 4.4.7 it will definitely occurs if  flags "-fstrict-aliasing"
> and "-Wall" used.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com<mailto:thomas.monjalon@6wind.com>>>

Applied with a comment in the code.

Thanks
--
Thomas