On Jan 16, 2015, at 1:33 PM, Anna Schumaker <Anna.Schumaker@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
