* [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 19:59 ` Shannon Nelson
0 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 19:59 UTC (permalink / raw)
To: netdev, davem; +Cc: sparclinux, linux-kernel, Shannon Nelson
Fix up a data alignment issue on sparc by swapping the order
of the cookie byte array field with the length field in
struct tcp_fastopen_cookie
This addresses log complaints like these:
log_unaligned: 113 callbacks suppressed
Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360
Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360
Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360
Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
include/linux/tcp.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index fc5848d..95cda75 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -62,8 +62,8 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
/* TCP Fast Open Cookie as stored in memory */
struct tcp_fastopen_cookie {
- s8 len;
u8 val[TCP_FASTOPEN_COOKIE_MAX];
+ s8 len;
bool exp; /* In RFC6994 experimental option format */
};
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 19:59 ` Shannon Nelson
0 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 19:59 UTC (permalink / raw)
To: netdev, davem; +Cc: sparclinux, linux-kernel, Shannon Nelson
Fix up a data alignment issue on sparc by swapping the order
of the cookie byte array field with the length field in
struct tcp_fastopen_cookie
This addresses log complaints like these:
log_unaligned: 113 callbacks suppressed
Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360
Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360
Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360
Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
include/linux/tcp.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index fc5848d..95cda75 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -62,8 +62,8 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
/* TCP Fast Open Cookie as stored in memory */
struct tcp_fastopen_cookie {
- s8 len;
u8 val[TCP_FASTOPEN_COOKIE_MAX];
+ s8 len;
bool exp; /* In RFC6994 experimental option format */
};
--
1.7.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 19:59 ` Shannon Nelson
@ 2017-01-12 20:13 ` Eric Dumazet
-1 siblings, 0 replies; 26+ messages in thread
From: Eric Dumazet @ 2017-01-12 20:13 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, sparclinux, linux-kernel
On Thu, 2017-01-12 at 11:59 -0800, Shannon Nelson wrote:
> Fix up a data alignment issue on sparc by swapping the order
> of the cookie byte array field with the length field in
> struct tcp_fastopen_cookie
>
> This addresses log complaints like these:
> log_unaligned: 113 callbacks suppressed
> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
> Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360
> Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360
> Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360
> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
> include/linux/tcp.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index fc5848d..95cda75 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -62,8 +62,8 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
>
> /* TCP Fast Open Cookie as stored in memory */
> struct tcp_fastopen_cookie {
> - s8 len;
> u8 val[TCP_FASTOPEN_COOKIE_MAX];
> + s8 len;
> bool exp; /* In RFC6994 experimental option format */
> };
>
Strange... Do you have an explanation of why this patch would be
needed ? A compiler issue ?
s8 and u8 are bytes after all.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:13 ` Eric Dumazet
0 siblings, 0 replies; 26+ messages in thread
From: Eric Dumazet @ 2017-01-12 20:13 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, sparclinux, linux-kernel
On Thu, 2017-01-12 at 11:59 -0800, Shannon Nelson wrote:
> Fix up a data alignment issue on sparc by swapping the order
> of the cookie byte array field with the length field in
> struct tcp_fastopen_cookie
>
> This addresses log complaints like these:
> log_unaligned: 113 callbacks suppressed
> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
> Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360
> Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360
> Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360
> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
> include/linux/tcp.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index fc5848d..95cda75 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -62,8 +62,8 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
>
> /* TCP Fast Open Cookie as stored in memory */
> struct tcp_fastopen_cookie {
> - s8 len;
> u8 val[TCP_FASTOPEN_COOKIE_MAX];
> + s8 len;
> bool exp; /* In RFC6994 experimental option format */
> };
>
Strange... Do you have an explanation of why this patch would be
needed ? A compiler issue ?
s8 and u8 are bytes after all.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:13 ` Eric Dumazet
@ 2017-01-12 20:15 ` Rob Gardner
-1 siblings, 0 replies; 26+ messages in thread
From: Rob Gardner @ 2017-01-12 20:15 UTC (permalink / raw)
To: Eric Dumazet, Shannon Nelson; +Cc: netdev, davem, sparclinux, linux-kernel
On 01/12/2017 01:13 PM, Eric Dumazet wrote:
> On Thu, 2017-01-12 at 11:59 -0800, Shannon Nelson wrote:
>> Fix up a data alignment issue on sparc by swapping the order
>> of the cookie byte array field with the length field in
>> struct tcp_fastopen_cookie
>>
>> This addresses log complaints like these:
>> log_unaligned: 113 callbacks suppressed
>> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
>> Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360
>> Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360
>> Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360
>> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>> include/linux/tcp.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
>> index fc5848d..95cda75 100644
>> --- a/include/linux/tcp.h
>> +++ b/include/linux/tcp.h
>> @@ -62,8 +62,8 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
>>
>> /* TCP Fast Open Cookie as stored in memory */
>> struct tcp_fastopen_cookie {
>> - s8 len;
>> u8 val[TCP_FASTOPEN_COOKIE_MAX];
>> + s8 len;
>> bool exp; /* In RFC6994 experimental option format */
>> };
>>
> Strange... Do you have an explanation of why this patch would be
> needed ? A compiler issue ?
>
>
> s8 and u8 are bytes after all.
>
>
I suspect that someplace, somebody is casting val to an int * or
something like that.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:15 ` Rob Gardner
0 siblings, 0 replies; 26+ messages in thread
From: Rob Gardner @ 2017-01-12 20:15 UTC (permalink / raw)
To: Eric Dumazet, Shannon Nelson; +Cc: netdev, davem, sparclinux, linux-kernel
On 01/12/2017 01:13 PM, Eric Dumazet wrote:
> On Thu, 2017-01-12 at 11:59 -0800, Shannon Nelson wrote:
>> Fix up a data alignment issue on sparc by swapping the order
>> of the cookie byte array field with the length field in
>> struct tcp_fastopen_cookie
>>
>> This addresses log complaints like these:
>> log_unaligned: 113 callbacks suppressed
>> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
>> Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360
>> Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360
>> Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360
>> Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>> ---
>> include/linux/tcp.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
>> index fc5848d..95cda75 100644
>> --- a/include/linux/tcp.h
>> +++ b/include/linux/tcp.h
>> @@ -62,8 +62,8 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
>>
>> /* TCP Fast Open Cookie as stored in memory */
>> struct tcp_fastopen_cookie {
>> - s8 len;
>> u8 val[TCP_FASTOPEN_COOKIE_MAX];
>> + s8 len;
>> bool exp; /* In RFC6994 experimental option format */
>> };
>>
> Strange... Do you have an explanation of why this patch would be
> needed ? A compiler issue ?
>
>
> s8 and u8 are bytes after all.
>
>
I suspect that someplace, somebody is casting val to an int * or
something like that.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:15 ` Rob Gardner
@ 2017-01-12 20:25 ` Eric Dumazet
-1 siblings, 0 replies; 26+ messages in thread
From: Eric Dumazet @ 2017-01-12 20:25 UTC (permalink / raw)
To: Rob Gardner; +Cc: Shannon Nelson, netdev, davem, sparclinux, linux-kernel
On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>
> I suspect that someplace, somebody is casting val to an int * or
> something like that.
Then that would be the bug. Can we root cause this please ?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:25 ` Eric Dumazet
0 siblings, 0 replies; 26+ messages in thread
From: Eric Dumazet @ 2017-01-12 20:25 UTC (permalink / raw)
To: Rob Gardner; +Cc: Shannon Nelson, netdev, davem, sparclinux, linux-kernel
On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>
> I suspect that someplace, somebody is casting val to an int * or
> something like that.
Then that would be the bug. Can we root cause this please ?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:25 ` Eric Dumazet
@ 2017-01-12 20:30 ` Shannon Nelson
-1 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 20:30 UTC (permalink / raw)
To: Eric Dumazet, Rob Gardner; +Cc: netdev, davem, sparclinux, linux-kernel
On 1/12/2017 12:25 PM, Eric Dumazet wrote:
> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>
>>
>> I suspect that someplace, somebody is casting val to an int * or
>> something like that.
>
> Then that would be the bug. Can we root cause this please ?
>
>
Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
struct in6_addr *buf = (struct in6_addr *) tmp.val;
sln
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:30 ` Shannon Nelson
0 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 20:30 UTC (permalink / raw)
To: Eric Dumazet, Rob Gardner; +Cc: netdev, davem, sparclinux, linux-kernel
On 1/12/2017 12:25 PM, Eric Dumazet wrote:
> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>
>>
>> I suspect that someplace, somebody is casting val to an int * or
>> something like that.
>
> Then that would be the bug. Can we root cause this please ?
>
>
Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
struct in6_addr *buf = (struct in6_addr *) tmp.val;
sln
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:25 ` Eric Dumazet
@ 2017-01-12 20:39 ` David Miller
-1 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 20:39 UTC (permalink / raw)
To: eric.dumazet
Cc: rob.gardner, shannon.nelson, netdev, sparclinux, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 Jan 2017 12:25:33 -0800
> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>
>>
>> I suspect that someplace, somebody is casting val to an int * or
>> something like that.
>
> Then that would be the bug. Can we root cause this please ?
The three accesses to foc->val are via function calls, at least when I
try to build it, one via memcmp(), one via memcpy() (for the structure
assignment at the end of the function) and one via a call into the
crypto layer when we do tcp_fastopen_cookie_gen).
So if the PC is inside of tcp_try_fastopen() it has to be something
else, or something specific to your gcc and build.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:39 ` David Miller
0 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 20:39 UTC (permalink / raw)
To: eric.dumazet
Cc: rob.gardner, shannon.nelson, netdev, sparclinux, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 Jan 2017 12:25:33 -0800
> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>
>>
>> I suspect that someplace, somebody is casting val to an int * or
>> something like that.
>
> Then that would be the bug. Can we root cause this please ?
The three accesses to foc->val are via function calls, at least when I
try to build it, one via memcmp(), one via memcpy() (for the structure
assignment at the end of the function) and one via a call into the
crypto layer when we do tcp_fastopen_cookie_gen).
So if the PC is inside of tcp_try_fastopen() it has to be something
else, or something specific to your gcc and build.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:30 ` Shannon Nelson
@ 2017-01-12 20:41 ` David Miller
-1 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 20:41 UTC (permalink / raw)
To: shannon.nelson
Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Thu, 12 Jan 2017 12:30:38 -0800
> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>>
>>>
>>> I suspect that someplace, somebody is casting val to an int * or
>>> something like that.
>>
>> Then that would be the bug. Can we root cause this please ?
>>
>>
>
> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
>
> struct in6_addr *buf = (struct in6_addr *) tmp.val;
Oh yeah, that's it. I didn't notice that at all.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:41 ` David Miller
0 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 20:41 UTC (permalink / raw)
To: shannon.nelson
Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Thu, 12 Jan 2017 12:30:38 -0800
> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>>
>>>
>>> I suspect that someplace, somebody is casting val to an int * or
>>> something like that.
>>
>> Then that would be the bug. Can we root cause this please ?
>>
>>
>
> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
>
> struct in6_addr *buf = (struct in6_addr *) tmp.val;
Oh yeah, that's it. I didn't notice that at all.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:41 ` David Miller
@ 2017-01-12 20:56 ` Shannon Nelson
-1 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 20:56 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
On 1/12/2017 12:41 PM, David Miller wrote:
> From: Shannon Nelson <shannon.nelson@oracle.com>
> Date: Thu, 12 Jan 2017 12:30:38 -0800
>
>> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>>>
>>>>
>>>> I suspect that someplace, somebody is casting val to an int * or
>>>> something like that.
>>>
>>> Then that would be the bug. Can we root cause this please ?
>>>
>>>
>>
>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
>>
>> struct in6_addr *buf = (struct in6_addr *) tmp.val;
>
> Oh yeah, that's it. I didn't notice that at all.
>
It looked to me like swapping the data fields would be the easiest and
least impactive way to fix this. I didn't want to mess with the logic.
I'm certainly open to other suggestions.
sln
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 20:56 ` Shannon Nelson
0 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 20:56 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
On 1/12/2017 12:41 PM, David Miller wrote:
> From: Shannon Nelson <shannon.nelson@oracle.com>
> Date: Thu, 12 Jan 2017 12:30:38 -0800
>
>> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>>>
>>>>
>>>> I suspect that someplace, somebody is casting val to an int * or
>>>> something like that.
>>>
>>> Then that would be the bug. Can we root cause this please ?
>>>
>>>
>>
>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
>>
>> struct in6_addr *buf = (struct in6_addr *) tmp.val;
>
> Oh yeah, that's it. I didn't notice that at all.
>
It looked to me like swapping the data fields would be the easiest and
least impactive way to fix this. I didn't want to mess with the logic.
I'm certainly open to other suggestions.
sln
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 20:56 ` Shannon Nelson
@ 2017-01-12 21:18 ` David Miller
-1 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 21:18 UTC (permalink / raw)
To: shannon.nelson
Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Thu, 12 Jan 2017 12:56:08 -0800
>
>
> On 1/12/2017 12:41 PM, David Miller wrote:
>> From: Shannon Nelson <shannon.nelson@oracle.com>
>> Date: Thu, 12 Jan 2017 12:30:38 -0800
>>
>>> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
>>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>>>>
>>>>>
>>>>> I suspect that someplace, somebody is casting val to an int * or
>>>>> something like that.
>>>>
>>>> Then that would be the bug. Can we root cause this please ?
>>>>
>>>>
>>>
>>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
>>>
>>> struct in6_addr *buf = (struct in6_addr *) tmp.val;
>>
>> Oh yeah, that's it. I didn't notice that at all.
>>
>
> It looked to me like swapping the data fields would be the easiest and
> least impactive way to fix this. I didn't want to mess with the
> logic. I'm certainly open to other suggestions.
Given the nature of the problem, your fix is probably fine.
Eric, any objections?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 21:18 ` David Miller
0 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 21:18 UTC (permalink / raw)
To: shannon.nelson
Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Thu, 12 Jan 2017 12:56:08 -0800
>
>
> On 1/12/2017 12:41 PM, David Miller wrote:
>> From: Shannon Nelson <shannon.nelson@oracle.com>
>> Date: Thu, 12 Jan 2017 12:30:38 -0800
>>
>>> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
>>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
>>>>
>>>>>
>>>>> I suspect that someplace, somebody is casting val to an int * or
>>>>> something like that.
>>>>
>>>> Then that would be the bug. Can we root cause this please ?
>>>>
>>>>
>>>
>>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
>>>
>>> struct in6_addr *buf = (struct in6_addr *) tmp.val;
>>
>> Oh yeah, that's it. I didn't notice that at all.
>>
>
> It looked to me like swapping the data fields would be the easiest and
> least impactive way to fix this. I didn't want to mess with the
> logic. I'm certainly open to other suggestions.
Given the nature of the problem, your fix is probably fine.
Eric, any objections?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 21:18 ` David Miller
@ 2017-01-12 21:36 ` Eric Dumazet
-1 siblings, 0 replies; 26+ messages in thread
From: Eric Dumazet @ 2017-01-12 21:36 UTC (permalink / raw)
To: David Miller
Cc: shannon.nelson, rob.gardner, netdev, sparclinux, linux-kernel
On Thu, 2017-01-12 at 16:18 -0500, David Miller wrote:
> From: Shannon Nelson <shannon.nelson@oracle.com>
> Date: Thu, 12 Jan 2017 12:56:08 -0800
>
> >
> >
> > On 1/12/2017 12:41 PM, David Miller wrote:
> >> From: Shannon Nelson <shannon.nelson@oracle.com>
> >> Date: Thu, 12 Jan 2017 12:30:38 -0800
> >>
> >>> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
> >>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
> >>>>
> >>>>>
> >>>>> I suspect that someplace, somebody is casting val to an int * or
> >>>>> something like that.
> >>>>
> >>>> Then that would be the bug. Can we root cause this please ?
> >>>>
> >>>>
> >>>
> >>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
> >>>
> >>> struct in6_addr *buf = (struct in6_addr *) tmp.val;
> >>
> >> Oh yeah, that's it. I didn't notice that at all.
> >>
> >
> > It looked to me like swapping the data fields would be the easiest and
> > least impactive way to fix this. I didn't want to mess with the
> > logic. I'm certainly open to other suggestions.
>
> Given the nature of the problem, your fix is probably fine.
>
> Eric, any objections?
I am still objecting to this fix.
gcc makes no provision for aligning an variable that has alignof() = 1
We had such issues in the past.
We need the proper annotation on ->val field itself, to get proper
alignment.
Then moving around the other field is a matter of avoiding a hole.
val should be an union, so that proper alignment is enforced by one
member.
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index fc5848dad7a43216b3f124c4afdaa6b64b23910c..5b790abf4c16313c9110996683be3a7fb368b66f 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -62,8 +62,13 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
/* TCP Fast Open Cookie as stored in memory */
struct tcp_fastopen_cookie {
+ union {
+ u8 val[TCP_FASTOPEN_COOKIE_MAX];
+#if IS_ENABLED(CONFIG_IPV6)
+ struct in6_addr addr;
+#endif
+ };
s8 len;
- u8 val[TCP_FASTOPEN_COOKIE_MAX];
bool exp; /* In RFC6994 experimental option format */
};
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 21:36 ` Eric Dumazet
0 siblings, 0 replies; 26+ messages in thread
From: Eric Dumazet @ 2017-01-12 21:36 UTC (permalink / raw)
To: David Miller
Cc: shannon.nelson, rob.gardner, netdev, sparclinux, linux-kernel
On Thu, 2017-01-12 at 16:18 -0500, David Miller wrote:
> From: Shannon Nelson <shannon.nelson@oracle.com>
> Date: Thu, 12 Jan 2017 12:56:08 -0800
>
> >
> >
> > On 1/12/2017 12:41 PM, David Miller wrote:
> >> From: Shannon Nelson <shannon.nelson@oracle.com>
> >> Date: Thu, 12 Jan 2017 12:30:38 -0800
> >>
> >>> On 1/12/2017 12:25 PM, Eric Dumazet wrote:
> >>>> On Thu, 2017-01-12 at 13:15 -0700, Rob Gardner wrote:
> >>>>
> >>>>>
> >>>>> I suspect that someplace, somebody is casting val to an int * or
> >>>>> something like that.
> >>>>
> >>>> Then that would be the bug. Can we root cause this please ?
> >>>>
> >>>>
> >>>
> >>> Look in net/ipv4/tcp_fastopen.c:tcp_fastopen_cookie_gen() for the line
> >>>
> >>> struct in6_addr *buf = (struct in6_addr *) tmp.val;
> >>
> >> Oh yeah, that's it. I didn't notice that at all.
> >>
> >
> > It looked to me like swapping the data fields would be the easiest and
> > least impactive way to fix this. I didn't want to mess with the
> > logic. I'm certainly open to other suggestions.
>
> Given the nature of the problem, your fix is probably fine.
>
> Eric, any objections?
I am still objecting to this fix.
gcc makes no provision for aligning an variable that has alignof() = 1
We had such issues in the past.
We need the proper annotation on ->val field itself, to get proper
alignment.
Then moving around the other field is a matter of avoiding a hole.
val should be an union, so that proper alignment is enforced by one
member.
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index fc5848dad7a43216b3f124c4afdaa6b64b23910c..5b790abf4c16313c9110996683be3a7fb368b66f 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -62,8 +62,13 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
/* TCP Fast Open Cookie as stored in memory */
struct tcp_fastopen_cookie {
+ union {
+ u8 val[TCP_FASTOPEN_COOKIE_MAX];
+#if IS_ENABLED(CONFIG_IPV6)
+ struct in6_addr addr;
+#endif
+ };
s8 len;
- u8 val[TCP_FASTOPEN_COOKIE_MAX];
bool exp; /* In RFC6994 experimental option format */
};
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 21:36 ` Eric Dumazet
@ 2017-01-12 21:47 ` David Miller
-1 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 21:47 UTC (permalink / raw)
To: eric.dumazet
Cc: shannon.nelson, rob.gardner, netdev, sparclinux, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 Jan 2017 13:36:30 -0800
> val should be an union, so that proper alignment is enforced by one
> member.
Sure, annotating the type so that it is aligned correctly makes
sense.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 21:47 ` David Miller
0 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 21:47 UTC (permalink / raw)
To: eric.dumazet
Cc: shannon.nelson, rob.gardner, netdev, sparclinux, linux-kernel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 12 Jan 2017 13:36:30 -0800
> val should be an union, so that proper alignment is enforced by one
> member.
Sure, annotating the type so that it is aligned correctly makes
sense.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 21:47 ` David Miller
@ 2017-01-12 21:58 ` Shannon Nelson
-1 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 21:58 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: rob.gardner, netdev, sparclinux, linux-kernel
On 1/12/2017 1:47 PM, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 12 Jan 2017 13:36:30 -0800
>
>> val should be an union, so that proper alignment is enforced by one
>> member.
>
> Sure, annotating the type so that it is aligned correctly makes
> sense.
>
... and we should change the offending pointer assignment as well. I
can use this and respin a v2 if we're happy with this solution.
sln
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 21:58 ` Shannon Nelson
0 siblings, 0 replies; 26+ messages in thread
From: Shannon Nelson @ 2017-01-12 21:58 UTC (permalink / raw)
To: David Miller, eric.dumazet; +Cc: rob.gardner, netdev, sparclinux, linux-kernel
On 1/12/2017 1:47 PM, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 12 Jan 2017 13:36:30 -0800
>
>> val should be an union, so that proper alignment is enforced by one
>> member.
>
> Sure, annotating the type so that it is aligned correctly makes
> sense.
>
... and we should change the offending pointer assignment as well. I
can use this and respin a v2 if we're happy with this solution.
sln
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
2017-01-12 21:58 ` Shannon Nelson
@ 2017-01-12 21:58 ` David Miller
-1 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 21:58 UTC (permalink / raw)
To: shannon.nelson
Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Thu, 12 Jan 2017 13:58:10 -0800
>
>
> On 1/12/2017 1:47 PM, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu, 12 Jan 2017 13:36:30 -0800
>>
>>> val should be an union, so that proper alignment is enforced by one
>>> member.
>>
>> Sure, annotating the type so that it is aligned correctly makes
>> sense.
>>
>
> ... and we should change the offending pointer assignment as well. I
> can use this and respin a v2 if we're happy with this solution.
I am.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc
@ 2017-01-12 21:58 ` David Miller
0 siblings, 0 replies; 26+ messages in thread
From: David Miller @ 2017-01-12 21:58 UTC (permalink / raw)
To: shannon.nelson
Cc: eric.dumazet, rob.gardner, netdev, sparclinux, linux-kernel
From: Shannon Nelson <shannon.nelson@oracle.com>
Date: Thu, 12 Jan 2017 13:58:10 -0800
>
>
> On 1/12/2017 1:47 PM, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu, 12 Jan 2017 13:36:30 -0800
>>
>>> val should be an union, so that proper alignment is enforced by one
>>> member.
>>
>> Sure, annotating the type so that it is aligned correctly makes
>> sense.
>>
>
> ... and we should change the offending pointer assignment as well. I
> can use this and respin a v2 if we're happy with this solution.
I am.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-01-12 21:59 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 19:59 [PATCH] tcp: fix tcp_fastopen unaligned access complaints on sparc Shannon Nelson
2017-01-12 19:59 ` Shannon Nelson
2017-01-12 20:13 ` Eric Dumazet
2017-01-12 20:13 ` Eric Dumazet
2017-01-12 20:15 ` Rob Gardner
2017-01-12 20:15 ` Rob Gardner
2017-01-12 20:25 ` Eric Dumazet
2017-01-12 20:25 ` Eric Dumazet
2017-01-12 20:30 ` Shannon Nelson
2017-01-12 20:30 ` Shannon Nelson
2017-01-12 20:41 ` David Miller
2017-01-12 20:41 ` David Miller
2017-01-12 20:56 ` Shannon Nelson
2017-01-12 20:56 ` Shannon Nelson
2017-01-12 21:18 ` David Miller
2017-01-12 21:18 ` David Miller
2017-01-12 21:36 ` Eric Dumazet
2017-01-12 21:36 ` Eric Dumazet
2017-01-12 21:47 ` David Miller
2017-01-12 21:47 ` David Miller
2017-01-12 21:58 ` Shannon Nelson
2017-01-12 21:58 ` Shannon Nelson
2017-01-12 21:58 ` David Miller
2017-01-12 21:58 ` David Miller
2017-01-12 20:39 ` David Miller
2017-01-12 20:39 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.