From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anna Schumaker Subject: Re: [PATCH v2 02/20] xprtrdma: Modernize htonl and ntohl Date: Fri, 16 Jan 2015 14:01:51 -0500 Message-ID: <54B9601F.2030302@Netapp.com> References: <20150113161440.14086.24801.stgit@manet.1015granger.net> <20150113162459.14086.38318.stgit@manet.1015granger.net> <54B95965.3080806@Netapp.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Chuck Lever Cc: linux-rdma , Linux NFS Mailing List List-Id: linux-rdma@vger.kernel.org On 01/16/2015 01:56 PM, Chuck Lever wrote: >=20 > On Jan 16, 2015, at 1:33 PM, Anna Schumaker wrote: >=20 >> 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/tr= ansport.c and xprtrdma/verbs.c. Should these be changed as well? >=20 > Thanks for the review! >=20 > The one in verbs.c is removed in 08/20. I'll take a look in a sec, I'm up to 07/20 :) >=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=E2=80=99s typical for IP address manipulation to us= e ntohl(). > git grep shows only two or three places in the kernel where the new > style byte-swap macros are used with sin_addr. =46air enough. Thanks for answering! Anna >=20 > The code in xprt_rdma_format_addresses() should be fixed up to handle > IPv6 too. >=20 >> Thanks, >> Anna >>> >>> Signed-off-by: Chuck Lever >>> --- >>> 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 >>> >>> +#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 =3D 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/r= pc_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, s= truct xdr_buf *target, >>> if (cur_rchunk) { /* read */ >>> cur_rchunk->rc_discrim =3D xdr_one; >>> /* all read chunks have the same "position" */ >>> - cur_rchunk->rc_position =3D htonl(pos); >>> - cur_rchunk->rc_target.rs_handle =3D htonl(seg->mr_rkey); >>> - cur_rchunk->rc_target.rs_length =3D htonl(seg->mr_len); >>> + cur_rchunk->rc_position =3D cpu_to_be32(pos); >>> + cur_rchunk->rc_target.rs_handle =3D >>> + cpu_to_be32(seg->mr_rkey); >>> + cur_rchunk->rc_target.rs_length =3D >>> + 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, s= truct xdr_buf *target, >>> cur_rchunk++; >>> r_xprt->rx_stats.read_chunk_count++; >>> } else { /* write/reply */ >>> - cur_wchunk->wc_target.rs_handle =3D htonl(seg->mr_rkey); >>> - cur_wchunk->wc_target.rs_length =3D htonl(seg->mr_len); >>> + cur_wchunk->wc_target.rs_handle =3D >>> + cpu_to_be32(seg->mr_rkey); >>> + cur_wchunk->wc_target.rs_length =3D >>> + 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, st= ruct xdr_buf *target, >>> *iptr++ =3D xdr_zero; /* encode a NULL reply chunk */ >>> } else { >>> warray->wc_discrim =3D xdr_one; >>> - warray->wc_nchunks =3D htonl(nchunks); >>> + warray->wc_nchunks =3D cpu_to_be32(nchunks); >>> iptr =3D (__be32 *) cur_wchunk; >>> if (type =3D=3D rpcrdma_writech) { >>> *iptr++ =3D 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 =3D (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 =3D rqst->rq_xid; >>> - headerp->rm_vers =3D xdr_one; >>> - headerp->rm_credit =3D htonl(r_xprt->rx_buf.rb_max_requests); >>> - headerp->rm_type =3D htonl(RDMA_MSG); >>> + headerp->rm_vers =3D rpcrdma_version; >>> + headerp->rm_credit =3D cpu_to_be32(r_xprt->rx_buf.rb_max_requests= ); >>> + headerp->rm_type =3D 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 =3D htonl(RDMA_MSGP); >>> + headerp->rm_type =3D rdma_msgp; >>> headerp->rm_body.rm_padded.rm_align =3D >>> - htonl(RPCRDMA_INLINE_PAD_VALUE(rqst)); >>> + cpu_to_be32(RPCRDMA_INLINE_PAD_VALUE(rqst)); >>> headerp->rm_body.rm_padded.rm_thresh =3D >>> - htonl(RPCRDMA_INLINE_PAD_THRESH); >>> + cpu_to_be32(RPCRDMA_INLINE_PAD_THRESH); >>> headerp->rm_body.rm_padded.rm_pempty[0] =3D xdr_zero; >>> headerp->rm_body.rm_padded.rm_pempty[1] =3D xdr_zero; >>> headerp->rm_body.rm_padded.rm_pempty[2] =3D xdr_zero; >>> @@ -570,7 +574,7 @@ rpcrdma_count_chunks(struct rpcrdma_rep *rep, u= nsigned int max, int wrchunk, __b >>> unsigned int i, total_len; >>> struct rpcrdma_write_chunk *cur_wchunk; >>> >>> - i =3D ntohl(**iptrp); /* get array count */ >>> + i =3D be32_to_cpu(**iptrp); >>> if (i > max) >>> return -1; >>> cur_wchunk =3D (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 +=3D ntohl(seg->rs_length); >>> + total_len +=3D 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 =3D (struct rpcrdma_msg *) rep->rr_base; >>> - if (headerp->rm_vers !=3D xdr_one) { >>> + if (headerp->rm_vers !=3D 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] !=3D xdr_zero || >>> headerp->rm_body.rm_chunks[1] !=3D xdr_zero || >>> @@ -853,7 +857,7 @@ badheader: >>> dprintk("%s: invalid rpcrdma reply header (type %d):" >>> " chunks[012] =3D=3D %d %d %d" >>> " expected chunks <=3D %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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >=20 > -- > Chuck Lever > chuck[dot]lever[at]oracle[dot]com >=20 >=20 >=20 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" i= n the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html