On 01/16/2015 01:56 PM, Chuck Lever wrote: > > On Jan 16, 2015, at 1:33 PM, Anna Schumaker <[email protected]> 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 <[email protected]> >>> --- >>> 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 [email protected] >>> 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-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
