[dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing

[dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing

[Daniel Mrzyglod]

This is fix for GCC 4.4.7.
flag "-fstrict-aliasing" is default for optimalisation above -O0.

Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")

Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@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 71c519a..5b7554a 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -169,7 +169,8 @@ __rte_raw_cksum(const void *buf, size_t len, uint32_t sum)
 {
 	/* workaround gcc strict-aliasing warning */
 	uintptr_t ptr = (uintptr_t)buf;
-	const uint16_t *u16 = (const uint16_t *)ptr;
+	typedef uint16_t __attribute__((__may_alias__)) u16_p;
+	const u16_p *u16 = (const u16_p *)ptr;
 
 	while (len >= (sizeof(*u16) * 4)) {
 		sum += u16[0];
-- 
2.5.0

Re: [dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing

[Mrzyglod, DanielX T]

This error fix this situation for IPv6 checksum offload error on RHEL65
Any optimalisation above -O0 provide error in IPv6 checksum


Step 1 : start testpmd
./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4  -- -i --portmask=0x3 --disable-hw-vlan --enable-rx-cksum --crc-strip --txqflags=0

Step 2 : settings and start
           set verbose 1
           set fwd csum
           start

Step 3 : calculate correct checksum values of IPv6/TCP and IPv6/UDP by scapy
           Packets info:  IPv6/UDP:Ether(dst="02:00:00:00:00:00", src="90:e2:ba:4a:33:5c")/IPv6(src="::2")/UDP()/("X"*46)
                          IPv6/TCP:  Ether(src="52:00:00:00:00:00", dst="90:e2:ba:4a:33:5d")/IPv6(src="::1")/TCP()/("X"*46)

Step 4 : Send two packets with wrong checksum value,and calculate the right checksum value by port,packets received on another port
           Send packets info: IPv6/UDP：Ether(dst="90:e2:ba:4a:33:5d", src="52:00:00:00:00:00")/IPv6(src="::1")/UDP(chksum=0xf)/("X"*46)
                              IPv6/TCP：Ether(dst="90:e2:ba:4a:33:5d", src="52:00:00:00:00:00")/IPv6(src="::1")/TCP(chksum=0xf)/("X"*46)

RESULTS:
'IPv6/TCP': ['0xd41']}, 'IPv6/UDP': ['0x7d07'],

EXPECTED RESULTS:
'IPv6/TCP': ['0x9f5e']}, 'IPv6/UDP': ['0xf26']

Re: [dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing

[Bruce Richardson]

On Tue, Nov 24, 2015 at 03:47:28PM +0000, Mrzyglod, DanielX T wrote:
> This error fix this situation for IPv6 checksum offload error on RHEL65
> Any optimalisation above -O0 provide error in IPv6 checksum
> 
> 
> Step 1 : start testpmd
> ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4  -- -i --portmask=0x3 --disable-hw-vlan --enable-rx-cksum --crc-strip --txqflags=0
> 
> Step 2 : settings and start
>            set verbose 1
>            set fwd csum
>            start
> 
> Step 3 : calculate correct checksum values of IPv6/TCP and IPv6/UDP by scapy
>            Packets info:  IPv6/UDP:Ether(dst="02:00:00:00:00:00", src="90:e2:ba:4a:33:5c")/IPv6(src="::2")/UDP()/("X"*46)
>                           IPv6/TCP:  Ether(src="52:00:00:00:00:00", dst="90:e2:ba:4a:33:5d")/IPv6(src="::1")/TCP()/("X"*46)
> 
> Step 4 : Send two packets with wrong checksum value,and calculate the right checksum value by port,packets received on another port
>            Send packets info: IPv6/UDP：Ether(dst="90:e2:ba:4a:33:5d", src="52:00:00:00:00:00")/IPv6(src="::1")/UDP(chksum=0xf)/("X"*46)
>                               IPv6/TCP：Ether(dst="90:e2:ba:4a:33:5d", src="52:00:00:00:00:00")/IPv6(src="::1")/TCP(chksum=0xf)/("X"*46)
> 
> RESULTS:
> 'IPv6/TCP': ['0xd41']}, 'IPv6/UDP': ['0x7d07'],
> 
> EXPECTED RESULTS:
> 'IPv6/TCP': ['0x9f5e']}, 'IPv6/UDP': ['0xf26']

Hi Daniel,

this sort of detail - perhaps in abbreviated form - should be in the commit
message for the patch. Can you perhaps do a V2 to include it?

/Bruce

[dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing

[Daniel Mrzyglod]

This fix is for IPv6 checksum offload error on RHEL65.
Any optimalisation above -O0 provide error in IPv6 checksum
flag "-fstrict-aliasing" is default for optimalisation above -O0.
The solution is to add typedef with __attribute__((__may_alias__) for uint16_t.

Step 1 : start testpmd
./testpmd -c 0x6 -n 4  -- -i --portmask=0x3 --disable-hw-vlan --enable-rx-cksum --crc-strip --txqflags=0

Step 2 : settings and start
           set verbose 1
           set fwd csum
           start

Step 3 : send scapy with bad checksum IPv6/TCP packet
        Ether(src="52:00:00:00:00:00", dst="90:e2:ba:4a:33:5d")/IPv6(src="::1")/TCP(chksum=0xf)/("X"*46)

Step 4 : Recieved packets:
RESULTS:
IPv6/TCP': ['0xd41'] or other unexpected.

EXPECTED RESULTS:
IPv6/TCP': ['0x9f5e']

Daniel Mrzyglod (1):
  net: fix build with gcc 4.4.7 and strict aliasing

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

-- 
2.5.0

[dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing

[Daniel Mrzyglod]

This fix is for IPv6 checksum offload error on RHEL65.
Any optimalisation above -O0 provide error in IPv6 checksum
flag "-fstrict-aliasing" is default for optimalisation above -O0.

Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")

Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@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 71c519a..5b7554a 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -169,7 +169,8 @@ __rte_raw_cksum(const void *buf, size_t len, uint32_t sum)
 {
 	/* workaround gcc strict-aliasing warning */
 	uintptr_t ptr = (uintptr_t)buf;
-	const uint16_t *u16 = (const uint16_t *)ptr;
+	typedef uint16_t __attribute__((__may_alias__)) u16_p;
+	const u16_p *u16 = (const u16_p *)ptr;
 
 	while (len >= (sizeof(*u16) * 4)) {
 		sum += u16[0];
-- 
2.5.0

Re: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing

[Ananyev, Konstantin]

Hi Daniel,
So for my own curiosity: what went wrong here?
Did compiler avoid to generate a code for while {} loop?
Or something else?
BTW, it is an internal function, so instead of introducing new typedef,
we can just change the type of buf?
Let say to uint8_t *?
>From gcc manual page: "  A character type may alias any other type."
Would that work?
Konstantin

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> Sent: Tuesday, November 24, 2015 4:31 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing
> 
> This fix is for IPv6 checksum offload error on RHEL65.
> Any optimalisation above -O0 provide error in IPv6 checksum
> flag "-fstrict-aliasing" is default for optimalisation above -O0.
> 
> Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")
> 
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@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 71c519a..5b7554a 100644
> --- a/lib/librte_net/rte_ip.h
> +++ b/lib/librte_net/rte_ip.h
> @@ -169,7 +169,8 @@ __rte_raw_cksum(const void *buf, size_t len, uint32_t sum)
>  {
>  	/* workaround gcc strict-aliasing warning */
>  	uintptr_t ptr = (uintptr_t)buf;
> -	const uint16_t *u16 = (const uint16_t *)ptr;
> +	typedef uint16_t __attribute__((__may_alias__)) u16_p;
> +	const u16_p *u16 = (const u16_p *)ptr;
> 
>  	while (len >= (sizeof(*u16) * 4)) {
>  		sum += u16[0];
> --
> 2.5.0

Re: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing

[Mrzyglod, DanielX T]

If we change input buf:
xapp-gcc/include/rte_ip.h:211: error: dereferencing pointer 'u16' does break strict-aliasing rules
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:204: note: initialized from here
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:212: error: dereferencing pointer '({anonymous})' does break strict-aliasing rules
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:212: note: initialized from here
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:213: error: dereferencing pointer '({anonymous})' does break strict-aliasing rules
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:213: note: initialized from here
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:214: error: dereferencing pointer '({anonymous})' does break strict-aliasing rules
/root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:214: note: initialized from here

The change was pointed  out by Michael Qiu in patch: 2b039d5f20a34016ecaf9b26f8f8b6c4a81bf4b6

We can only cast void to uint8 but it would involve big endian/ little endian macros:

static inline uint32_t __attribute__((__may_alias__))
__rte_raw_cksum(const void *buf, size_t len, uint32_t sum)
{
	/* workaround gcc strict-aliasing warning */
	const uint8_t *u8 = ((const uint8_t *)buf);
	while (len >= (sizeof(*u8) * 8)) {
		sum += *(u8+1) << 8 | *(u8);
		sum += *(u8+3) << 8 | *(u8+2);
		sum += *(u8+5) << 8 | *(u8+4);
		sum += *(u8+7) << 8 | *(u8+6);

		len -= sizeof(*u8) * 8;
		u8 += 8;
	}

	while (len >= 2*sizeof(*u8)) {
		sum += *u8 << 8 | *(u8+1);
		len -= 2*sizeof(*u8);
		u8 += 2;
	}

	/* if length is in odd bytes */
	if (len == 1)
		sum += *((const uint8_t *)u8);

	return sum;
}

I will research about this union usage suggested by Stephen, but for this moment  local typedef & __attribute__((__may_alias__)) is the most clean solution which work under gcc(4.4.7), clang, icc.

>-----Original Message-----
>From: Ananyev, Konstantin
>Sent: Tuesday, November 24, 2015 6:58 PM
>To: Mrzyglod, DanielX T <danielx.t.mrzyglod@intel.com>
>Cc: dev@dpdk.org
>Subject: RE: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing
>
>Hi Daniel,
>So for my own curiosity: what went wrong here?
>Did compiler avoid to generate a code for while {} loop?
>Or something else?
>BTW, it is an internal function, so instead of introducing new typedef,
>we can just change the type of buf?
>Let say to uint8_t *?
>From gcc manual page: "  A character type may alias any other type."
>Would that work?
>Konstantin
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
>> Sent: Tuesday, November 24, 2015 4:31 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing
>>
>> This fix is for IPv6 checksum offload error on RHEL65.
>> Any optimalisation above -O0 provide error in IPv6 checksum
>> flag "-fstrict-aliasing" is default for optimalisation above -O0.
>>
>> Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")
>>
>> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@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 71c519a..5b7554a 100644
>> --- a/lib/librte_net/rte_ip.h
>> +++ b/lib/librte_net/rte_ip.h
>> @@ -169,7 +169,8 @@ __rte_raw_cksum(const void *buf, size_t len, uint32_t
>sum)
>>  {
>>  	/* workaround gcc strict-aliasing warning */
>>  	uintptr_t ptr = (uintptr_t)buf;
>> -	const uint16_t *u16 = (const uint16_t *)ptr;
>> +	typedef uint16_t __attribute__((__may_alias__)) u16_p;
>> +	const u16_p *u16 = (const u16_p *)ptr;
>>
>>  	while (len >= (sizeof(*u16) * 4)) {
>>  		sum += u16[0];
>> --
>> 2.5.0

Re: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing

[Ananyev, Konstantin]

> -----Original Message-----
> From: Mrzyglod, DanielX T
> Sent: Wednesday, November 25, 2015 5:07 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing
> 
> If we change input buf:
> xapp-gcc/include/rte_ip.h:211: error: dereferencing pointer 'u16' does break strict-aliasing rules
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:204: note: initialized from here
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:212: error: dereferencing pointer '({anonymous})' does break strict-aliasing rules
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:212: note: initialized from here
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:213: error: dereferencing pointer '({anonymous})' does break strict-aliasing rules
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:213: note: initialized from here
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:214: error: dereferencing pointer '({anonymous})' does break strict-aliasing rules
> /root/dpdk/x86_64-native-linuxapp-gcc/include/rte_ip.h:214: note: initialized from here
> 
> The change was pointed  out by Michael Qiu in patch: 2b039d5f20a34016ecaf9b26f8f8b6c4a81bf4b6
> 
> We can only cast void to uint8 but it would involve big endian/ little endian macros:
> 
> static inline uint32_t __attribute__((__may_alias__))
> __rte_raw_cksum(const void *buf, size_t len, uint32_t sum)
> {
> 	/* workaround gcc strict-aliasing warning */
> 	const uint8_t *u8 = ((const uint8_t *)buf);
> 	while (len >= (sizeof(*u8) * 8)) {
> 		sum += *(u8+1) << 8 | *(u8);
> 		sum += *(u8+3) << 8 | *(u8+2);
> 		sum += *(u8+5) << 8 | *(u8+4);
> 		sum += *(u8+7) << 8 | *(u8+6);
> 
> 		len -= sizeof(*u8) * 8;
> 		u8 += 8;
> 	}
> 
> 	while (len >= 2*sizeof(*u8)) {
> 		sum += *u8 << 8 | *(u8+1);
> 		len -= 2*sizeof(*u8);
> 		u8 += 2;
> 	}
> 
> 	/* if length is in odd bytes */
> 	if (len == 1)
> 		sum += *((const uint8_t *)u8);
> 
> 	return sum;
> }

So I misinterpreted gcc manual: it only allows conversion from any other type to char, not visa-versa.
Sorry for wrong suggestion.

> 
> I will research about this union usage suggested by Stephen, but for this moment  local typedef & __attribute__((__may_alias__)) is the
> most clean solution which work under gcc(4.4.7), clang, icc.

Ok, if there is no other known way - let's stick with your original patch. 
Konstantin

> 
> >-----Original Message-----
> >From: Ananyev, Konstantin
> >Sent: Tuesday, November 24, 2015 6:58 PM
> >To: Mrzyglod, DanielX T <danielx.t.mrzyglod@intel.com>
> >Cc: dev@dpdk.org
> >Subject: RE: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing
> >
> >Hi Daniel,
> >So for my own curiosity: what went wrong here?
> >Did compiler avoid to generate a code for while {} loop?
> >Or something else?
> >BTW, it is an internal function, so instead of introducing new typedef,
> >we can just change the type of buf?
> >Let say to uint8_t *?
> >From gcc manual page: "  A character type may alias any other type."
> >Would that work?
> >Konstantin
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> >> Sent: Tuesday, November 24, 2015 4:31 PM
> >> To: dev@dpdk.org
> >> Subject: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing
> >>
> >> This fix is for IPv6 checksum offload error on RHEL65.
> >> Any optimalisation above -O0 provide error in IPv6 checksum
> >> flag "-fstrict-aliasing" is default for optimalisation above -O0.
> >>
> >> Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")
> >>
> >> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@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 71c519a..5b7554a 100644
> >> --- a/lib/librte_net/rte_ip.h
> >> +++ b/lib/librte_net/rte_ip.h
> >> @@ -169,7 +169,8 @@ __rte_raw_cksum(const void *buf, size_t len, uint32_t
> >sum)
> >>  {
> >>  	/* workaround gcc strict-aliasing warning */
> >>  	uintptr_t ptr = (uintptr_t)buf;
> >> -	const uint16_t *u16 = (const uint16_t *)ptr;
> >> +	typedef uint16_t __attribute__((__may_alias__)) u16_p;
> >> +	const u16_p *u16 = (const u16_p *)ptr;
> >>
> >>  	while (len >= (sizeof(*u16) * 4)) {
> >>  		sum += u16[0];
> >> --
> >> 2.5.0

Re: [dpdk-dev] [PATCH v2] net: fix build with gcc 4.4.7 and strict aliasing

[Stephen Hemminger]

On Tue, 24 Nov 2015 17:31:17 +0100
Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> wrote:

> This fix is for IPv6 checksum offload error on RHEL65.
> Any optimalisation above -O0 provide error in IPv6 checksum
> flag "-fstrict-aliasing" is default for optimalisation above -O0.
> The solution is to add typedef with __attribute__((__may_alias__) for uint16_t.
> 
> Step 1 : start testpmd
> ./testpmd -c 0x6 -n 4  -- -i --portmask=0x3 --disable-hw-vlan --enable-rx-cksum --crc-strip --txqflags=0
> 
> Step 2 : settings and start
>            set verbose 1
>            set fwd csum
>            start
> 
> Step 3 : send scapy with bad checksum IPv6/TCP packet
>         Ether(src="52:00:00:00:00:00", dst="90:e2:ba:4a:33:5d")/IPv6(src="::1")/TCP(chksum=0xf)/("X"*46)
> 
> Step 4 : Recieved packets:
> RESULTS:
> IPv6/TCP': ['0xd41'] or other unexpected.
> 
> EXPECTED RESULTS:
> IPv6/TCP': ['0x9f5e']
> 
> Daniel Mrzyglod (1):
>   net: fix build with gcc 4.4.7 and strict aliasing
> 
>  lib/librte_net/rte_ip.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

An alternative way of fixing it (without resorting to attributes) would
be to use a union.

Re: [dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing

[Ananyev, Konstantin]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> Sent: Tuesday, November 24, 2015 3:13 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing
> 
> This is fix for GCC 4.4.7.
> flag "-fstrict-aliasing" is default for optimalisation above -O0.
> 
> Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")
> 
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.5.0

Re: [dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing

[Thomas Monjalon]

2015-11-25 18:00, Ananyev, Konstantin:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Daniel Mrzyglod
> > Sent: Tuesday, November 24, 2015 3:13 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] net: fix build with gcc 4.4.7 and strict aliasing
> > 
> > This is fix for GCC 4.4.7.
> > flag "-fstrict-aliasing" is default for optimalisation above -O0.
> > 
> > Fixes: 2b039d5f20a3 ("net: fix build with gcc 4.4.7 and strict aliasing")
> > 
> > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks