diff mbox

[v2,02/20] xprtrdma: Modernize htonl and ntohl

Message ID 20150113162459.14086.38318.stgit@manet.1015granger.net (mailing list archive)
State New, archived
Headers show

Commit Message

Chuck Lever Jan. 13, 2015, 4:25 p.m. UTC
Clean up: Replace htonl and ntohl with the be32 equivalents.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 include/linux/sunrpc/rpc_rdma.h |    9 +++++++
 include/linux/sunrpc/svc_rdma.h |    2 --
 net/sunrpc/xprtrdma/rpc_rdma.c  |   48 +++++++++++++++++++++------------------
 3 files changed, 35 insertions(+), 24 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Schumaker, Anna Jan. 16, 2015, 6:33 p.m. UTC | #1
Hi Chuck,

On 01/13/2015 11:25 AM, Chuck Lever wrote:
> Clean up: Replace htonl and ntohl with the be32 equivalents.

After applying this patch I still see an ntohl() in both xprtrdma/transport.c and xprtrdma/verbs.c.  Should these be changed as well?

Thanks,
Anna
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  include/linux/sunrpc/rpc_rdma.h |    9 +++++++
>  include/linux/sunrpc/svc_rdma.h |    2 --
>  net/sunrpc/xprtrdma/rpc_rdma.c  |   48 +++++++++++++++++++++------------------
>  3 files changed, 35 insertions(+), 24 deletions(-)
> 
> diff --git a/include/linux/sunrpc/rpc_rdma.h b/include/linux/sunrpc/rpc_rdma.h
> index b78f16b..1578ed2 100644
> --- a/include/linux/sunrpc/rpc_rdma.h
> +++ b/include/linux/sunrpc/rpc_rdma.h
> @@ -42,6 +42,9 @@
>  
>  #include <linux/types.h>
>  
> +#define RPCRDMA_VERSION		1
> +#define rpcrdma_version		cpu_to_be32(RPCRDMA_VERSION)
> +
>  struct rpcrdma_segment {
>  	__be32 rs_handle;	/* Registered memory handle */
>  	__be32 rs_length;	/* Length of the chunk in bytes */
> @@ -115,4 +118,10 @@ enum rpcrdma_proc {
>  	RDMA_ERROR = 4		/* An RPC RDMA encoding error */
>  };
>  
> +#define rdma_msg	cpu_to_be32(RDMA_MSG)
> +#define rdma_nomsg	cpu_to_be32(RDMA_NOMSG)
> +#define rdma_msgp	cpu_to_be32(RDMA_MSGP)
> +#define rdma_done	cpu_to_be32(RDMA_DONE)
> +#define rdma_error	cpu_to_be32(RDMA_ERROR)
> +
>  #endif				/* _LINUX_SUNRPC_RPC_RDMA_H */
> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
> index 975da75..ddfe88f 100644
> --- a/include/linux/sunrpc/svc_rdma.h
> +++ b/include/linux/sunrpc/svc_rdma.h
> @@ -63,8 +63,6 @@ extern atomic_t rdma_stat_rq_prod;
>  extern atomic_t rdma_stat_sq_poll;
>  extern atomic_t rdma_stat_sq_prod;
>  
> -#define RPCRDMA_VERSION 1
> -
>  /*
>   * Contexts are built when an RDMA request is created and are a
>   * record of the resources that can be recovered when the request
> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
> index df01d12..a6fb30b 100644
> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
> @@ -209,9 +209,11 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>  		if (cur_rchunk) {	/* read */
>  			cur_rchunk->rc_discrim = xdr_one;
>  			/* all read chunks have the same "position" */
> -			cur_rchunk->rc_position = htonl(pos);
> -			cur_rchunk->rc_target.rs_handle = htonl(seg->mr_rkey);
> -			cur_rchunk->rc_target.rs_length = htonl(seg->mr_len);
> +			cur_rchunk->rc_position = cpu_to_be32(pos);
> +			cur_rchunk->rc_target.rs_handle =
> +						cpu_to_be32(seg->mr_rkey);
> +			cur_rchunk->rc_target.rs_length =
> +						cpu_to_be32(seg->mr_len);
>  			xdr_encode_hyper(
>  					(__be32 *)&cur_rchunk->rc_target.rs_offset,
>  					seg->mr_base);
> @@ -222,8 +224,10 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>  			cur_rchunk++;
>  			r_xprt->rx_stats.read_chunk_count++;
>  		} else {		/* write/reply */
> -			cur_wchunk->wc_target.rs_handle = htonl(seg->mr_rkey);
> -			cur_wchunk->wc_target.rs_length = htonl(seg->mr_len);
> +			cur_wchunk->wc_target.rs_handle =
> +						cpu_to_be32(seg->mr_rkey);
> +			cur_wchunk->wc_target.rs_length =
> +						cpu_to_be32(seg->mr_len);
>  			xdr_encode_hyper(
>  					(__be32 *)&cur_wchunk->wc_target.rs_offset,
>  					seg->mr_base);
> @@ -257,7 +261,7 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>  		*iptr++ = xdr_zero;	/* encode a NULL reply chunk */
>  	} else {
>  		warray->wc_discrim = xdr_one;
> -		warray->wc_nchunks = htonl(nchunks);
> +		warray->wc_nchunks = cpu_to_be32(nchunks);
>  		iptr = (__be32 *) cur_wchunk;
>  		if (type == rpcrdma_writech) {
>  			*iptr++ = xdr_zero; /* finish the write chunk list */
> @@ -404,11 +408,11 @@ rpcrdma_marshal_req(struct rpc_rqst *rqst)
>  
>  	/* build RDMA header in private area at front */
>  	headerp = (struct rpcrdma_msg *) req->rl_base;
> -	/* don't htonl XID, it's already done in request */
> +	/* don't byte-swap XID, it's already done in request */
>  	headerp->rm_xid = rqst->rq_xid;
> -	headerp->rm_vers = xdr_one;
> -	headerp->rm_credit = htonl(r_xprt->rx_buf.rb_max_requests);
> -	headerp->rm_type = htonl(RDMA_MSG);
> +	headerp->rm_vers = rpcrdma_version;
> +	headerp->rm_credit = cpu_to_be32(r_xprt->rx_buf.rb_max_requests);
> +	headerp->rm_type = rdma_msg;
>  
>  	/*
>  	 * Chunks needed for results?
> @@ -482,11 +486,11 @@ rpcrdma_marshal_req(struct rpc_rqst *rqst)
>  						RPCRDMA_INLINE_PAD_VALUE(rqst));
>  
>  		if (padlen) {
> -			headerp->rm_type = htonl(RDMA_MSGP);
> +			headerp->rm_type = rdma_msgp;
>  			headerp->rm_body.rm_padded.rm_align =
> -				htonl(RPCRDMA_INLINE_PAD_VALUE(rqst));
> +				cpu_to_be32(RPCRDMA_INLINE_PAD_VALUE(rqst));
>  			headerp->rm_body.rm_padded.rm_thresh =
> -				htonl(RPCRDMA_INLINE_PAD_THRESH);
> +				cpu_to_be32(RPCRDMA_INLINE_PAD_THRESH);
>  			headerp->rm_body.rm_padded.rm_pempty[0] = xdr_zero;
>  			headerp->rm_body.rm_padded.rm_pempty[1] = xdr_zero;
>  			headerp->rm_body.rm_padded.rm_pempty[2] = xdr_zero;
> @@ -570,7 +574,7 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
>  	unsigned int i, total_len;
>  	struct rpcrdma_write_chunk *cur_wchunk;
>  
> -	i = ntohl(**iptrp);	/* get array count */
> +	i = be32_to_cpu(**iptrp);
>  	if (i > max)
>  		return -1;
>  	cur_wchunk = (struct rpcrdma_write_chunk *) (*iptrp + 1);
> @@ -582,11 +586,11 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
>  			xdr_decode_hyper((__be32 *)&seg->rs_offset, &off);
>  			dprintk("RPC:       %s: chunk %d@0x%llx:0x%x\n",
>  				__func__,
> -				ntohl(seg->rs_length),
> +				be32_to_cpu(seg->rs_length),
>  				(unsigned long long)off,
> -				ntohl(seg->rs_handle));
> +				be32_to_cpu(seg->rs_handle));
>  		}
> -		total_len += ntohl(seg->rs_length);
> +		total_len += be32_to_cpu(seg->rs_length);
>  		++cur_wchunk;
>  	}
>  	/* check and adjust for properly terminated write chunk */
> @@ -749,9 +753,9 @@ rpcrdma_reply_handler(struct rpcrdma_rep *rep)
>  		goto repost;
>  	}
>  	headerp = (struct rpcrdma_msg *) rep->rr_base;
> -	if (headerp->rm_vers != xdr_one) {
> +	if (headerp->rm_vers != rpcrdma_version) {
>  		dprintk("RPC:       %s: invalid version %d\n",
> -			__func__, ntohl(headerp->rm_vers));
> +			__func__, be32_to_cpu(headerp->rm_vers));
>  		goto repost;
>  	}
>  
> @@ -793,7 +797,7 @@ repost:
>  	/* check for expected message types */
>  	/* The order of some of these tests is important. */
>  	switch (headerp->rm_type) {
> -	case htonl(RDMA_MSG):
> +	case rdma_msg:
>  		/* never expect read chunks */
>  		/* never expect reply chunks (two ways to check) */
>  		/* never expect write chunks without having offered RDMA */
> @@ -832,7 +836,7 @@ repost:
>  		rpcrdma_inline_fixup(rqst, (char *)iptr, rep->rr_len, rdmalen);
>  		break;
>  
> -	case htonl(RDMA_NOMSG):
> +	case rdma_nomsg:
>  		/* never expect read or write chunks, always reply chunks */
>  		if (headerp->rm_body.rm_chunks[0] != xdr_zero ||
>  		    headerp->rm_body.rm_chunks[1] != xdr_zero ||
> @@ -853,7 +857,7 @@ badheader:
>  		dprintk("%s: invalid rpcrdma reply header (type %d):"
>  				" chunks[012] == %d %d %d"
>  				" expected chunks <= %d\n",
> -				__func__, ntohl(headerp->rm_type),
> +				__func__, be32_to_cpu(headerp->rm_type),
>  				headerp->rm_body.rm_chunks[0],
>  				headerp->rm_body.rm_chunks[1],
>  				headerp->rm_body.rm_chunks[2],
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Jan. 16, 2015, 6:56 p.m. UTC | #2
On Jan 16, 2015, at 1:33 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:

> Hi Chuck,
> 
> On 01/13/2015 11:25 AM, Chuck Lever wrote:
>> Clean up: Replace htonl and ntohl with the be32 equivalents.
> 
> After applying this patch I still see an ntohl() in both xprtrdma/transport.c and xprtrdma/verbs.c.  Should these be changed as well?

Thanks for the review!

The one in verbs.c is removed in 08/20.

I was mostly interested in updating the XDR usage of the byte
swapping macros. For sin_addr, transport.c uses ntohl() the same way
as xprtsock.c. It’s typical for IP address manipulation to use ntohl().
git grep shows only two or three places in the kernel where the new
style byte-swap macros are used with sin_addr.

The code in xprt_rdma_format_addresses() should be fixed up to handle
IPv6 too.

> Thanks,
> Anna
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> include/linux/sunrpc/rpc_rdma.h |    9 +++++++
>> include/linux/sunrpc/svc_rdma.h |    2 --
>> net/sunrpc/xprtrdma/rpc_rdma.c  |   48 +++++++++++++++++++++------------------
>> 3 files changed, 35 insertions(+), 24 deletions(-)
>> 
>> diff --git a/include/linux/sunrpc/rpc_rdma.h b/include/linux/sunrpc/rpc_rdma.h
>> index b78f16b..1578ed2 100644
>> --- a/include/linux/sunrpc/rpc_rdma.h
>> +++ b/include/linux/sunrpc/rpc_rdma.h
>> @@ -42,6 +42,9 @@
>> 
>> #include <linux/types.h>
>> 
>> +#define RPCRDMA_VERSION		1
>> +#define rpcrdma_version		cpu_to_be32(RPCRDMA_VERSION)
>> +
>> struct rpcrdma_segment {
>> 	__be32 rs_handle;	/* Registered memory handle */
>> 	__be32 rs_length;	/* Length of the chunk in bytes */
>> @@ -115,4 +118,10 @@ enum rpcrdma_proc {
>> 	RDMA_ERROR = 4		/* An RPC RDMA encoding error */
>> };
>> 
>> +#define rdma_msg	cpu_to_be32(RDMA_MSG)
>> +#define rdma_nomsg	cpu_to_be32(RDMA_NOMSG)
>> +#define rdma_msgp	cpu_to_be32(RDMA_MSGP)
>> +#define rdma_done	cpu_to_be32(RDMA_DONE)
>> +#define rdma_error	cpu_to_be32(RDMA_ERROR)
>> +
>> #endif				/* _LINUX_SUNRPC_RPC_RDMA_H */
>> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
>> index 975da75..ddfe88f 100644
>> --- a/include/linux/sunrpc/svc_rdma.h
>> +++ b/include/linux/sunrpc/svc_rdma.h
>> @@ -63,8 +63,6 @@ extern atomic_t rdma_stat_rq_prod;
>> extern atomic_t rdma_stat_sq_poll;
>> extern atomic_t rdma_stat_sq_prod;
>> 
>> -#define RPCRDMA_VERSION 1
>> -
>> /*
>>  * Contexts are built when an RDMA request is created and are a
>>  * record of the resources that can be recovered when the request
>> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
>> index df01d12..a6fb30b 100644
>> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
>> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
>> @@ -209,9 +209,11 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>> 		if (cur_rchunk) {	/* read */
>> 			cur_rchunk->rc_discrim = xdr_one;
>> 			/* all read chunks have the same "position" */
>> -			cur_rchunk->rc_position = htonl(pos);
>> -			cur_rchunk->rc_target.rs_handle = htonl(seg->mr_rkey);
>> -			cur_rchunk->rc_target.rs_length = htonl(seg->mr_len);
>> +			cur_rchunk->rc_position = cpu_to_be32(pos);
>> +			cur_rchunk->rc_target.rs_handle =
>> +						cpu_to_be32(seg->mr_rkey);
>> +			cur_rchunk->rc_target.rs_length =
>> +						cpu_to_be32(seg->mr_len);
>> 			xdr_encode_hyper(
>> 					(__be32 *)&cur_rchunk->rc_target.rs_offset,
>> 					seg->mr_base);
>> @@ -222,8 +224,10 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>> 			cur_rchunk++;
>> 			r_xprt->rx_stats.read_chunk_count++;
>> 		} else {		/* write/reply */
>> -			cur_wchunk->wc_target.rs_handle = htonl(seg->mr_rkey);
>> -			cur_wchunk->wc_target.rs_length = htonl(seg->mr_len);
>> +			cur_wchunk->wc_target.rs_handle =
>> +						cpu_to_be32(seg->mr_rkey);
>> +			cur_wchunk->wc_target.rs_length =
>> +						cpu_to_be32(seg->mr_len);
>> 			xdr_encode_hyper(
>> 					(__be32 *)&cur_wchunk->wc_target.rs_offset,
>> 					seg->mr_base);
>> @@ -257,7 +261,7 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>> 		*iptr++ = xdr_zero;	/* encode a NULL reply chunk */
>> 	} else {
>> 		warray->wc_discrim = xdr_one;
>> -		warray->wc_nchunks = htonl(nchunks);
>> +		warray->wc_nchunks = cpu_to_be32(nchunks);
>> 		iptr = (__be32 *) cur_wchunk;
>> 		if (type == rpcrdma_writech) {
>> 			*iptr++ = xdr_zero; /* finish the write chunk list */
>> @@ -404,11 +408,11 @@ rpcrdma_marshal_req(struct rpc_rqst *rqst)
>> 
>> 	/* build RDMA header in private area at front */
>> 	headerp = (struct rpcrdma_msg *) req->rl_base;
>> -	/* don't htonl XID, it's already done in request */
>> +	/* don't byte-swap XID, it's already done in request */
>> 	headerp->rm_xid = rqst->rq_xid;
>> -	headerp->rm_vers = xdr_one;
>> -	headerp->rm_credit = htonl(r_xprt->rx_buf.rb_max_requests);
>> -	headerp->rm_type = htonl(RDMA_MSG);
>> +	headerp->rm_vers = rpcrdma_version;
>> +	headerp->rm_credit = cpu_to_be32(r_xprt->rx_buf.rb_max_requests);
>> +	headerp->rm_type = rdma_msg;
>> 
>> 	/*
>> 	 * Chunks needed for results?
>> @@ -482,11 +486,11 @@ rpcrdma_marshal_req(struct rpc_rqst *rqst)
>> 						RPCRDMA_INLINE_PAD_VALUE(rqst));
>> 
>> 		if (padlen) {
>> -			headerp->rm_type = htonl(RDMA_MSGP);
>> +			headerp->rm_type = rdma_msgp;
>> 			headerp->rm_body.rm_padded.rm_align =
>> -				htonl(RPCRDMA_INLINE_PAD_VALUE(rqst));
>> +				cpu_to_be32(RPCRDMA_INLINE_PAD_VALUE(rqst));
>> 			headerp->rm_body.rm_padded.rm_thresh =
>> -				htonl(RPCRDMA_INLINE_PAD_THRESH);
>> +				cpu_to_be32(RPCRDMA_INLINE_PAD_THRESH);
>> 			headerp->rm_body.rm_padded.rm_pempty[0] = xdr_zero;
>> 			headerp->rm_body.rm_padded.rm_pempty[1] = xdr_zero;
>> 			headerp->rm_body.rm_padded.rm_pempty[2] = xdr_zero;
>> @@ -570,7 +574,7 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
>> 	unsigned int i, total_len;
>> 	struct rpcrdma_write_chunk *cur_wchunk;
>> 
>> -	i = ntohl(**iptrp);	/* get array count */
>> +	i = be32_to_cpu(**iptrp);
>> 	if (i > max)
>> 		return -1;
>> 	cur_wchunk = (struct rpcrdma_write_chunk *) (*iptrp + 1);
>> @@ -582,11 +586,11 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
>> 			xdr_decode_hyper((__be32 *)&seg->rs_offset, &off);
>> 			dprintk("RPC:       %s: chunk %d@0x%llx:0x%x\n",
>> 				__func__,
>> -				ntohl(seg->rs_length),
>> +				be32_to_cpu(seg->rs_length),
>> 				(unsigned long long)off,
>> -				ntohl(seg->rs_handle));
>> +				be32_to_cpu(seg->rs_handle));
>> 		}
>> -		total_len += ntohl(seg->rs_length);
>> +		total_len += be32_to_cpu(seg->rs_length);
>> 		++cur_wchunk;
>> 	}
>> 	/* check and adjust for properly terminated write chunk */
>> @@ -749,9 +753,9 @@ rpcrdma_reply_handler(struct rpcrdma_rep *rep)
>> 		goto repost;
>> 	}
>> 	headerp = (struct rpcrdma_msg *) rep->rr_base;
>> -	if (headerp->rm_vers != xdr_one) {
>> +	if (headerp->rm_vers != rpcrdma_version) {
>> 		dprintk("RPC:       %s: invalid version %d\n",
>> -			__func__, ntohl(headerp->rm_vers));
>> +			__func__, be32_to_cpu(headerp->rm_vers));
>> 		goto repost;
>> 	}
>> 
>> @@ -793,7 +797,7 @@ repost:
>> 	/* check for expected message types */
>> 	/* The order of some of these tests is important. */
>> 	switch (headerp->rm_type) {
>> -	case htonl(RDMA_MSG):
>> +	case rdma_msg:
>> 		/* never expect read chunks */
>> 		/* never expect reply chunks (two ways to check) */
>> 		/* never expect write chunks without having offered RDMA */
>> @@ -832,7 +836,7 @@ repost:
>> 		rpcrdma_inline_fixup(rqst, (char *)iptr, rep->rr_len, rdmalen);
>> 		break;
>> 
>> -	case htonl(RDMA_NOMSG):
>> +	case rdma_nomsg:
>> 		/* never expect read or write chunks, always reply chunks */
>> 		if (headerp->rm_body.rm_chunks[0] != xdr_zero ||
>> 		    headerp->rm_body.rm_chunks[1] != xdr_zero ||
>> @@ -853,7 +857,7 @@ badheader:
>> 		dprintk("%s: invalid rpcrdma reply header (type %d):"
>> 				" chunks[012] == %d %d %d"
>> 				" expected chunks <= %d\n",
>> -				__func__, ntohl(headerp->rm_type),
>> +				__func__, be32_to_cpu(headerp->rm_type),
>> 				headerp->rm_body.rm_chunks[0],
>> 				headerp->rm_body.rm_chunks[1],
>> 				headerp->rm_body.rm_chunks[2],
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
> 

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Schumaker, Anna Jan. 16, 2015, 7:01 p.m. UTC | #3
On 01/16/2015 01:56 PM, Chuck Lever wrote:
> 
> On Jan 16, 2015, at 1:33 PM, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
> 
>> Hi Chuck,
>>
>> On 01/13/2015 11:25 AM, Chuck Lever wrote:
>>> Clean up: Replace htonl and ntohl with the be32 equivalents.
>>
>> After applying this patch I still see an ntohl() in both xprtrdma/transport.c and xprtrdma/verbs.c.  Should these be changed as well?
> 
> Thanks for the review!
> 
> The one in verbs.c is removed in 08/20.

I'll take a look in a sec, I'm up to 07/20 :)

> 
> I was mostly interested in updating the XDR usage of the byte
> swapping macros. For sin_addr, transport.c uses ntohl() the same way
> as xprtsock.c. It’s typical for IP address manipulation to use ntohl().
> git grep shows only two or three places in the kernel where the new
> style byte-swap macros are used with sin_addr.

Fair enough.  Thanks for answering!

Anna

> 
> The code in xprt_rdma_format_addresses() should be fixed up to handle
> IPv6 too.
> 
>> Thanks,
>> Anna
>>>
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> include/linux/sunrpc/rpc_rdma.h |    9 +++++++
>>> include/linux/sunrpc/svc_rdma.h |    2 --
>>> net/sunrpc/xprtrdma/rpc_rdma.c  |   48 +++++++++++++++++++++------------------
>>> 3 files changed, 35 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/include/linux/sunrpc/rpc_rdma.h b/include/linux/sunrpc/rpc_rdma.h
>>> index b78f16b..1578ed2 100644
>>> --- a/include/linux/sunrpc/rpc_rdma.h
>>> +++ b/include/linux/sunrpc/rpc_rdma.h
>>> @@ -42,6 +42,9 @@
>>>
>>> #include <linux/types.h>
>>>
>>> +#define RPCRDMA_VERSION		1
>>> +#define rpcrdma_version		cpu_to_be32(RPCRDMA_VERSION)
>>> +
>>> struct rpcrdma_segment {
>>> 	__be32 rs_handle;	/* Registered memory handle */
>>> 	__be32 rs_length;	/* Length of the chunk in bytes */
>>> @@ -115,4 +118,10 @@ enum rpcrdma_proc {
>>> 	RDMA_ERROR = 4		/* An RPC RDMA encoding error */
>>> };
>>>
>>> +#define rdma_msg	cpu_to_be32(RDMA_MSG)
>>> +#define rdma_nomsg	cpu_to_be32(RDMA_NOMSG)
>>> +#define rdma_msgp	cpu_to_be32(RDMA_MSGP)
>>> +#define rdma_done	cpu_to_be32(RDMA_DONE)
>>> +#define rdma_error	cpu_to_be32(RDMA_ERROR)
>>> +
>>> #endif				/* _LINUX_SUNRPC_RPC_RDMA_H */
>>> diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
>>> index 975da75..ddfe88f 100644
>>> --- a/include/linux/sunrpc/svc_rdma.h
>>> +++ b/include/linux/sunrpc/svc_rdma.h
>>> @@ -63,8 +63,6 @@ extern atomic_t rdma_stat_rq_prod;
>>> extern atomic_t rdma_stat_sq_poll;
>>> extern atomic_t rdma_stat_sq_prod;
>>>
>>> -#define RPCRDMA_VERSION 1
>>> -
>>> /*
>>>  * Contexts are built when an RDMA request is created and are a
>>>  * record of the resources that can be recovered when the request
>>> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
>>> index df01d12..a6fb30b 100644
>>> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
>>> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
>>> @@ -209,9 +209,11 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>>> 		if (cur_rchunk) {	/* read */
>>> 			cur_rchunk->rc_discrim = xdr_one;
>>> 			/* all read chunks have the same "position" */
>>> -			cur_rchunk->rc_position = htonl(pos);
>>> -			cur_rchunk->rc_target.rs_handle = htonl(seg->mr_rkey);
>>> -			cur_rchunk->rc_target.rs_length = htonl(seg->mr_len);
>>> +			cur_rchunk->rc_position = cpu_to_be32(pos);
>>> +			cur_rchunk->rc_target.rs_handle =
>>> +						cpu_to_be32(seg->mr_rkey);
>>> +			cur_rchunk->rc_target.rs_length =
>>> +						cpu_to_be32(seg->mr_len);
>>> 			xdr_encode_hyper(
>>> 					(__be32 *)&cur_rchunk->rc_target.rs_offset,
>>> 					seg->mr_base);
>>> @@ -222,8 +224,10 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>>> 			cur_rchunk++;
>>> 			r_xprt->rx_stats.read_chunk_count++;
>>> 		} else {		/* write/reply */
>>> -			cur_wchunk->wc_target.rs_handle = htonl(seg->mr_rkey);
>>> -			cur_wchunk->wc_target.rs_length = htonl(seg->mr_len);
>>> +			cur_wchunk->wc_target.rs_handle =
>>> +						cpu_to_be32(seg->mr_rkey);
>>> +			cur_wchunk->wc_target.rs_length =
>>> +						cpu_to_be32(seg->mr_len);
>>> 			xdr_encode_hyper(
>>> 					(__be32 *)&cur_wchunk->wc_target.rs_offset,
>>> 					seg->mr_base);
>>> @@ -257,7 +261,7 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>>> 		*iptr++ = xdr_zero;	/* encode a NULL reply chunk */
>>> 	} else {
>>> 		warray->wc_discrim = xdr_one;
>>> -		warray->wc_nchunks = htonl(nchunks);
>>> +		warray->wc_nchunks = cpu_to_be32(nchunks);
>>> 		iptr = (__be32 *) cur_wchunk;
>>> 		if (type == rpcrdma_writech) {
>>> 			*iptr++ = xdr_zero; /* finish the write chunk list */
>>> @@ -404,11 +408,11 @@ rpcrdma_marshal_req(struct rpc_rqst *rqst)
>>>
>>> 	/* build RDMA header in private area at front */
>>> 	headerp = (struct rpcrdma_msg *) req->rl_base;
>>> -	/* don't htonl XID, it's already done in request */
>>> +	/* don't byte-swap XID, it's already done in request */
>>> 	headerp->rm_xid = rqst->rq_xid;
>>> -	headerp->rm_vers = xdr_one;
>>> -	headerp->rm_credit = htonl(r_xprt->rx_buf.rb_max_requests);
>>> -	headerp->rm_type = htonl(RDMA_MSG);
>>> +	headerp->rm_vers = rpcrdma_version;
>>> +	headerp->rm_credit = cpu_to_be32(r_xprt->rx_buf.rb_max_requests);
>>> +	headerp->rm_type = rdma_msg;
>>>
>>> 	/*
>>> 	 * Chunks needed for results?
>>> @@ -482,11 +486,11 @@ rpcrdma_marshal_req(struct rpc_rqst *rqst)
>>> 						RPCRDMA_INLINE_PAD_VALUE(rqst));
>>>
>>> 		if (padlen) {
>>> -			headerp->rm_type = htonl(RDMA_MSGP);
>>> +			headerp->rm_type = rdma_msgp;
>>> 			headerp->rm_body.rm_padded.rm_align =
>>> -				htonl(RPCRDMA_INLINE_PAD_VALUE(rqst));
>>> +				cpu_to_be32(RPCRDMA_INLINE_PAD_VALUE(rqst));
>>> 			headerp->rm_body.rm_padded.rm_thresh =
>>> -				htonl(RPCRDMA_INLINE_PAD_THRESH);
>>> +				cpu_to_be32(RPCRDMA_INLINE_PAD_THRESH);
>>> 			headerp->rm_body.rm_padded.rm_pempty[0] = xdr_zero;
>>> 			headerp->rm_body.rm_padded.rm_pempty[1] = xdr_zero;
>>> 			headerp->rm_body.rm_padded.rm_pempty[2] = xdr_zero;
>>> @@ -570,7 +574,7 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
>>> 	unsigned int i, total_len;
>>> 	struct rpcrdma_write_chunk *cur_wchunk;
>>>
>>> -	i = ntohl(**iptrp);	/* get array count */
>>> +	i = be32_to_cpu(**iptrp);
>>> 	if (i > max)
>>> 		return -1;
>>> 	cur_wchunk = (struct rpcrdma_write_chunk *) (*iptrp + 1);
>>> @@ -582,11 +586,11 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
>>> 			xdr_decode_hyper((__be32 *)&seg->rs_offset, &off);
>>> 			dprintk("RPC:       %s: chunk %d@0x%llx:0x%x\n",
>>> 				__func__,
>>> -				ntohl(seg->rs_length),
>>> +				be32_to_cpu(seg->rs_length),
>>> 				(unsigned long long)off,
>>> -				ntohl(seg->rs_handle));
>>> +				be32_to_cpu(seg->rs_handle));
>>> 		}
>>> -		total_len += ntohl(seg->rs_length);
>>> +		total_len += be32_to_cpu(seg->rs_length);
>>> 		++cur_wchunk;
>>> 	}
>>> 	/* check and adjust for properly terminated write chunk */
>>> @@ -749,9 +753,9 @@ rpcrdma_reply_handler(struct rpcrdma_rep *rep)
>>> 		goto repost;
>>> 	}
>>> 	headerp = (struct rpcrdma_msg *) rep->rr_base;
>>> -	if (headerp->rm_vers != xdr_one) {
>>> +	if (headerp->rm_vers != rpcrdma_version) {
>>> 		dprintk("RPC:       %s: invalid version %d\n",
>>> -			__func__, ntohl(headerp->rm_vers));
>>> +			__func__, be32_to_cpu(headerp->rm_vers));
>>> 		goto repost;
>>> 	}
>>>
>>> @@ -793,7 +797,7 @@ repost:
>>> 	/* check for expected message types */
>>> 	/* The order of some of these tests is important. */
>>> 	switch (headerp->rm_type) {
>>> -	case htonl(RDMA_MSG):
>>> +	case rdma_msg:
>>> 		/* never expect read chunks */
>>> 		/* never expect reply chunks (two ways to check) */
>>> 		/* never expect write chunks without having offered RDMA */
>>> @@ -832,7 +836,7 @@ repost:
>>> 		rpcrdma_inline_fixup(rqst, (char *)iptr, rep->rr_len, rdmalen);
>>> 		break;
>>>
>>> -	case htonl(RDMA_NOMSG):
>>> +	case rdma_nomsg:
>>> 		/* never expect read or write chunks, always reply chunks */
>>> 		if (headerp->rm_body.rm_chunks[0] != xdr_zero ||
>>> 		    headerp->rm_body.rm_chunks[1] != xdr_zero ||
>>> @@ -853,7 +857,7 @@ badheader:
>>> 		dprintk("%s: invalid rpcrdma reply header (type %d):"
>>> 				" chunks[012] == %d %d %d"
>>> 				" expected chunks <= %d\n",
>>> -				__func__, ntohl(headerp->rm_type),
>>> +				__func__, be32_to_cpu(headerp->rm_type),
>>> 				headerp->rm_body.rm_chunks[0],
>>> 				headerp->rm_body.rm_chunks[1],
>>> 				headerp->rm_body.rm_chunks[2],
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/sunrpc/rpc_rdma.h b/include/linux/sunrpc/rpc_rdma.h
index b78f16b..1578ed2 100644
--- a/include/linux/sunrpc/rpc_rdma.h
+++ b/include/linux/sunrpc/rpc_rdma.h
@@ -42,6 +42,9 @@ 
 
 #include <linux/types.h>
 
+#define RPCRDMA_VERSION		1
+#define rpcrdma_version		cpu_to_be32(RPCRDMA_VERSION)
+
 struct rpcrdma_segment {
 	__be32 rs_handle;	/* Registered memory handle */
 	__be32 rs_length;	/* Length of the chunk in bytes */
@@ -115,4 +118,10 @@  enum rpcrdma_proc {
 	RDMA_ERROR = 4		/* An RPC RDMA encoding error */
 };
 
+#define rdma_msg	cpu_to_be32(RDMA_MSG)
+#define rdma_nomsg	cpu_to_be32(RDMA_NOMSG)
+#define rdma_msgp	cpu_to_be32(RDMA_MSGP)
+#define rdma_done	cpu_to_be32(RDMA_DONE)
+#define rdma_error	cpu_to_be32(RDMA_ERROR)
+
 #endif				/* _LINUX_SUNRPC_RPC_RDMA_H */
diff --git a/include/linux/sunrpc/svc_rdma.h b/include/linux/sunrpc/svc_rdma.h
index 975da75..ddfe88f 100644
--- a/include/linux/sunrpc/svc_rdma.h
+++ b/include/linux/sunrpc/svc_rdma.h
@@ -63,8 +63,6 @@  extern atomic_t rdma_stat_rq_prod;
 extern atomic_t rdma_stat_sq_poll;
 extern atomic_t rdma_stat_sq_prod;
 
-#define RPCRDMA_VERSION 1
-
 /*
  * Contexts are built when an RDMA request is created and are a
  * record of the resources that can be recovered when the request
diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index df01d12..a6fb30b 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -209,9 +209,11 @@  rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
 		if (cur_rchunk) {	/* read */
 			cur_rchunk->rc_discrim = xdr_one;
 			/* all read chunks have the same "position" */
-			cur_rchunk->rc_position = htonl(pos);
-			cur_rchunk->rc_target.rs_handle = htonl(seg->mr_rkey);
-			cur_rchunk->rc_target.rs_length = htonl(seg->mr_len);
+			cur_rchunk->rc_position = cpu_to_be32(pos);
+			cur_rchunk->rc_target.rs_handle =
+						cpu_to_be32(seg->mr_rkey);
+			cur_rchunk->rc_target.rs_length =
+						cpu_to_be32(seg->mr_len);
 			xdr_encode_hyper(
 					(__be32 *)&cur_rchunk->rc_target.rs_offset,
 					seg->mr_base);
@@ -222,8 +224,10 @@  rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
 			cur_rchunk++;
 			r_xprt->rx_stats.read_chunk_count++;
 		} else {		/* write/reply */
-			cur_wchunk->wc_target.rs_handle = htonl(seg->mr_rkey);
-			cur_wchunk->wc_target.rs_length = htonl(seg->mr_len);
+			cur_wchunk->wc_target.rs_handle =
+						cpu_to_be32(seg->mr_rkey);
+			cur_wchunk->wc_target.rs_length =
+						cpu_to_be32(seg->mr_len);
 			xdr_encode_hyper(
 					(__be32 *)&cur_wchunk->wc_target.rs_offset,
 					seg->mr_base);
@@ -257,7 +261,7 @@  rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
 		*iptr++ = xdr_zero;	/* encode a NULL reply chunk */
 	} else {
 		warray->wc_discrim = xdr_one;
-		warray->wc_nchunks = htonl(nchunks);
+		warray->wc_nchunks = cpu_to_be32(nchunks);
 		iptr = (__be32 *) cur_wchunk;
 		if (type == rpcrdma_writech) {
 			*iptr++ = xdr_zero; /* finish the write chunk list */
@@ -404,11 +408,11 @@  rpcrdma_marshal_req(struct rpc_rqst *rqst)
 
 	/* build RDMA header in private area at front */
 	headerp = (struct rpcrdma_msg *) req->rl_base;
-	/* don't htonl XID, it's already done in request */
+	/* don't byte-swap XID, it's already done in request */
 	headerp->rm_xid = rqst->rq_xid;
-	headerp->rm_vers = xdr_one;
-	headerp->rm_credit = htonl(r_xprt->rx_buf.rb_max_requests);
-	headerp->rm_type = htonl(RDMA_MSG);
+	headerp->rm_vers = rpcrdma_version;
+	headerp->rm_credit = cpu_to_be32(r_xprt->rx_buf.rb_max_requests);
+	headerp->rm_type = rdma_msg;
 
 	/*
 	 * Chunks needed for results?
@@ -482,11 +486,11 @@  rpcrdma_marshal_req(struct rpc_rqst *rqst)
 						RPCRDMA_INLINE_PAD_VALUE(rqst));
 
 		if (padlen) {
-			headerp->rm_type = htonl(RDMA_MSGP);
+			headerp->rm_type = rdma_msgp;
 			headerp->rm_body.rm_padded.rm_align =
-				htonl(RPCRDMA_INLINE_PAD_VALUE(rqst));
+				cpu_to_be32(RPCRDMA_INLINE_PAD_VALUE(rqst));
 			headerp->rm_body.rm_padded.rm_thresh =
-				htonl(RPCRDMA_INLINE_PAD_THRESH);
+				cpu_to_be32(RPCRDMA_INLINE_PAD_THRESH);
 			headerp->rm_body.rm_padded.rm_pempty[0] = xdr_zero;
 			headerp->rm_body.rm_padded.rm_pempty[1] = xdr_zero;
 			headerp->rm_body.rm_padded.rm_pempty[2] = xdr_zero;
@@ -570,7 +574,7 @@  rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
 	unsigned int i, total_len;
 	struct rpcrdma_write_chunk *cur_wchunk;
 
-	i = ntohl(**iptrp);	/* get array count */
+	i = be32_to_cpu(**iptrp);
 	if (i > max)
 		return -1;
 	cur_wchunk = (struct rpcrdma_write_chunk *) (*iptrp + 1);
@@ -582,11 +586,11 @@  rpcrdma_count_chunks(struct rpcrdma_rep *rep, unsigned int max, int wrchunk, __b
 			xdr_decode_hyper((__be32 *)&seg->rs_offset, &off);
 			dprintk("RPC:       %s: chunk %d@0x%llx:0x%x\n",
 				__func__,
-				ntohl(seg->rs_length),
+				be32_to_cpu(seg->rs_length),
 				(unsigned long long)off,
-				ntohl(seg->rs_handle));
+				be32_to_cpu(seg->rs_handle));
 		}
-		total_len += ntohl(seg->rs_length);
+		total_len += be32_to_cpu(seg->rs_length);
 		++cur_wchunk;
 	}
 	/* check and adjust for properly terminated write chunk */
@@ -749,9 +753,9 @@  rpcrdma_reply_handler(struct rpcrdma_rep *rep)
 		goto repost;
 	}
 	headerp = (struct rpcrdma_msg *) rep->rr_base;
-	if (headerp->rm_vers != xdr_one) {
+	if (headerp->rm_vers != rpcrdma_version) {
 		dprintk("RPC:       %s: invalid version %d\n",
-			__func__, ntohl(headerp->rm_vers));
+			__func__, be32_to_cpu(headerp->rm_vers));
 		goto repost;
 	}
 
@@ -793,7 +797,7 @@  repost:
 	/* check for expected message types */
 	/* The order of some of these tests is important. */
 	switch (headerp->rm_type) {
-	case htonl(RDMA_MSG):
+	case rdma_msg:
 		/* never expect read chunks */
 		/* never expect reply chunks (two ways to check) */
 		/* never expect write chunks without having offered RDMA */
@@ -832,7 +836,7 @@  repost:
 		rpcrdma_inline_fixup(rqst, (char *)iptr, rep->rr_len, rdmalen);
 		break;
 
-	case htonl(RDMA_NOMSG):
+	case rdma_nomsg:
 		/* never expect read or write chunks, always reply chunks */
 		if (headerp->rm_body.rm_chunks[0] != xdr_zero ||
 		    headerp->rm_body.rm_chunks[1] != xdr_zero ||
@@ -853,7 +857,7 @@  badheader:
 		dprintk("%s: invalid rpcrdma reply header (type %d):"
 				" chunks[012] == %d %d %d"
 				" expected chunks <= %d\n",
-				__func__, ntohl(headerp->rm_type),
+				__func__, be32_to_cpu(headerp->rm_type),
 				headerp->rm_body.rm_chunks[0],
 				headerp->rm_body.rm_chunks[1],
 				headerp->rm_body.rm_chunks[2],