All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/rds: fix unaligned memory access
@ 2015-04-30 14:27 David Ahern
  2015-04-30 15:23 ` David Laight
  2015-04-30 15:44 ` David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: David Ahern @ 2015-04-30 14:27 UTC (permalink / raw)
  To: rds-devel, netdev; +Cc: shamir rabinovitch, David Ahern

From: shamir rabinovitch <shamir.rabinovitch@oracle.com>

rdma_conn_param private data is copied using memcpy after headers such
as cma_hdr (see cma_resolve_ib_udp as example). so the start of the
private data is aligned to the end of the structure that come before. if
this structure end with u32 the meaning is that the start of the private
data will be 4 bytes aligned. structures that use u8/u16/u32/u64 are
naturally aligned but in case the structure start is not 8 bytes aligned,
all u64 members of this structure will not be aligned. to solve this issue
we must use special macros that allow unaligned access to those
unaligned members.

Addresses the following kernel log seen when attempting to use RDMA:

Kernel unaligned access at TPC[10507a88] rds_ib_cm_connect_complete+0x1bc/0x1e0 [rds_rdma]

Acked-by: Chien Yen <chien.yen@oracle.com>
Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
[Minor tweaks for top of tree by:]
Signed-off-by: David Ahern <david.ahern@oracle.com>
---
 net/rds/ib_cm.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 31b74f5..e0bbbac 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -183,8 +183,17 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
 
 	/* If the peer gave us the last packet it saw, process this as if
 	 * we had received a regular ACK. */
-	if (dp && dp->dp_ack_seq)
-		rds_send_drop_acked(conn, be64_to_cpu(dp->dp_ack_seq), NULL);
+	if (dp) {
+		/* dp structure start is not guaranteed to be 8 bytes aligned
+		 * so on SPARC we get trap for nu-aligned access. we solve
+		 * this by using the macros for unaligned memory access
+		 */
+		u64 dp_ack_seq = get_unaligned(&dp->dp_ack_seq);
+
+		if (dp_ack_seq)
+			rds_send_drop_acked(conn, be64_to_cpu(dp_ack_seq),
+					    NULL);
+	}
 
 	rds_connect_complete(conn);
 }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH] net/rds: fix unaligned memory access
  2015-04-30 14:27 [PATCH] net/rds: fix unaligned memory access David Ahern
@ 2015-04-30 15:23 ` David Laight
  2015-04-30 15:58   ` David Miller
  2015-04-30 15:44 ` David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: David Laight @ 2015-04-30 15:23 UTC (permalink / raw)
  To: 'David Ahern', rds-devel, netdev; +Cc: shamir rabinovitch

From: David Ahern
> Sent: 30 April 2015 15:28
> rdma_conn_param private data is copied using memcpy after headers such
> as cma_hdr (see cma_resolve_ib_udp as example). so the start of the
> private data is aligned to the end of the structure that come before. if
> this structure end with u32 the meaning is that the start of the private
> data will be 4 bytes aligned. structures that use u8/u16/u32/u64 are
> naturally aligned but in case the structure start is not 8 bytes aligned,
> all u64 members of this structure will not be aligned. to solve this issue
> we must use special macros that allow unaligned access to those
> unaligned members.
> 
> Addresses the following kernel log seen when attempting to use RDMA:
> 
> Kernel unaligned access at TPC[10507a88] rds_ib_cm_connect_complete+0x1bc/0x1e0 [rds_rdma]
...
> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> index 31b74f5..e0bbbac 100644
> --- a/net/rds/ib_cm.c
> +++ b/net/rds/ib_cm.c
> @@ -183,8 +183,17 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
> 
>  	/* If the peer gave us the last packet it saw, process this as if
>  	 * we had received a regular ACK. */
> -	if (dp && dp->dp_ack_seq)
> -		rds_send_drop_acked(conn, be64_to_cpu(dp->dp_ack_seq), NULL);

Or just mark 'dp_ack_seq' with __attribute((aligned(4))).
Then the compiler will generate two 32bit loads.
(provided be64_to_cpu() doesn't do anything stupid.)

	David

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/rds: fix unaligned memory access
  2015-04-30 14:27 [PATCH] net/rds: fix unaligned memory access David Ahern
  2015-04-30 15:23 ` David Laight
@ 2015-04-30 15:44 ` David Miller
  2015-04-30 16:19   ` David Ahern
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2015-04-30 15:44 UTC (permalink / raw)
  To: david.ahern; +Cc: rds-devel, netdev, shamir.rabinovitch

From: David Ahern <david.ahern@oracle.com>
Date: Thu, 30 Apr 2015 10:27:53 -0400

> @@ -183,8 +183,17 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
>  
>  	/* If the peer gave us the last packet it saw, process this as if
>  	 * we had received a regular ACK. */
> -	if (dp && dp->dp_ack_seq)
> -		rds_send_drop_acked(conn, be64_to_cpu(dp->dp_ack_seq), NULL);
> +	if (dp) {
> +		/* dp structure start is not guaranteed to be 8 bytes aligned
> +		 * so on SPARC we get trap for nu-aligned access. we solve
> +		 * this by using the macros for unaligned memory access
> +		 */

"nu-aligned" is misspelled, and Mentioning sparc specifically is completely
inappropriate because this is not a sparc specific problem.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/rds: fix unaligned memory access
  2015-04-30 15:23 ` David Laight
@ 2015-04-30 15:58   ` David Miller
  0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2015-04-30 15:58 UTC (permalink / raw)
  To: David.Laight; +Cc: david.ahern, rds-devel, netdev, shamir.rabinovitch

From: David Laight <David.Laight@ACULAB.COM>
Date: Thu, 30 Apr 2015 15:23:39 +0000

> From: David Ahern
>> Sent: 30 April 2015 15:28
>> rdma_conn_param private data is copied using memcpy after headers such
>> as cma_hdr (see cma_resolve_ib_udp as example). so the start of the
>> private data is aligned to the end of the structure that come before. if
>> this structure end with u32 the meaning is that the start of the private
>> data will be 4 bytes aligned. structures that use u8/u16/u32/u64 are
>> naturally aligned but in case the structure start is not 8 bytes aligned,
>> all u64 members of this structure will not be aligned. to solve this issue
>> we must use special macros that allow unaligned access to those
>> unaligned members.
>> 
>> Addresses the following kernel log seen when attempting to use RDMA:
>> 
>> Kernel unaligned access at TPC[10507a88] rds_ib_cm_connect_complete+0x1bc/0x1e0 [rds_rdma]
> ...
>> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
>> index 31b74f5..e0bbbac 100644
>> --- a/net/rds/ib_cm.c
>> +++ b/net/rds/ib_cm.c
>> @@ -183,8 +183,17 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
>> 
>>  	/* If the peer gave us the last packet it saw, process this as if
>>  	 * we had received a regular ACK. */
>> -	if (dp && dp->dp_ack_seq)
>> -		rds_send_drop_acked(conn, be64_to_cpu(dp->dp_ack_seq), NULL);
> 
> Or just mark 'dp_ack_seq' with __attribute((aligned(4))).
> Then the compiler will generate two 32bit loads.
> (provided be64_to_cpu() doesn't do anything stupid.)

It's a __be64 which already requires 8 byte alignment.

That's not what is causing the problem here.  It's the opaque area that
this structure can end up being placed afterwards.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/rds: fix unaligned memory access
  2015-04-30 15:44 ` David Miller
@ 2015-04-30 16:19   ` David Ahern
  0 siblings, 0 replies; 5+ messages in thread
From: David Ahern @ 2015-04-30 16:19 UTC (permalink / raw)
  To: David Miller; +Cc: rds-devel, netdev, shamir.rabinovitch

On 4/30/15 9:44 AM, David Miller wrote:
> From: David Ahern <david.ahern@oracle.com>
> Date: Thu, 30 Apr 2015 10:27:53 -0400
>
>> @@ -183,8 +183,17 @@ void rds_ib_cm_connect_complete(struct rds_connection *conn, struct rdma_cm_even
>>
>>   	/* If the peer gave us the last packet it saw, process this as if
>>   	 * we had received a regular ACK. */
>> -	if (dp && dp->dp_ack_seq)
>> -		rds_send_drop_acked(conn, be64_to_cpu(dp->dp_ack_seq), NULL);
>> +	if (dp) {
>> +		/* dp structure start is not guaranteed to be 8 bytes aligned
>> +		 * so on SPARC we get trap for nu-aligned access. we solve
>> +		 * this by using the macros for unaligned memory access
>> +		 */
>
> "nu-aligned" is misspelled, and Mentioning sparc specifically is completely
> inappropriate because this is not a sparc specific problem.
>

oops meant to take that out. Will fix and resubmit.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-30 16:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-30 14:27 [PATCH] net/rds: fix unaligned memory access David Ahern
2015-04-30 15:23 ` David Laight
2015-04-30 15:58   ` David Miller
2015-04-30 15:44 ` David Miller
2015-04-30 16:19   ` David Ahern

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.