[PATCH 00/11] Overhaul NFSv2-related XDR functions

[PATCH 00/11] Overhaul NFSv2-related XDR functions

[Chuck Lever]

Trond-

This patch series overhauls NFSv2 and NLMv3 XDR functions to
use the same strict buffer checking tools that the NFSv4 XDR
functions use.

I did some testing with OProfile and perf to see what kind of
CPU impact the additional checking has... didn't notice much
if any change on the workloads I tried (fsx and cthon).  I
was careful to structure the code (especially conditionals)
so that the common path was as short as possible.

---

Chuck Lever (11):
      lockd: Introduce new-style XDR functions for NLMv3
      NFS: Move and update xdr_decode_foo() functions that we're keeping
      NFS: Replace old NFSv2 decoder functions with xdr_stream-based ones
      NFS: Introduce new-style XDR decoding functions for NFSv2
      NFS: Use the "nfs_stat" enum for nfs_stat_to_errno()'s argument
      NFS: Move and update xdr_encode_foo() functions that we're keeping
      NFS: Remove old NFSv2 encoder functions
      NFS: Introduce new-style XDR encoding functions for NFSv2
      SUNRPC: Refactor logic to NUL-terminate strings in pages
      SUNRPC: Correct an rpcbind debugging message
      NFS: Fix NFSv3 debugging messages in fs/nfs/nfs3proc.c


 fs/lockd/Makefile          |    4 
 fs/lockd/clntxdr.c         |  657 +++++++++++++++++++++
 fs/lockd/xdr.c             |  258 --------
 fs/nfs/internal.h          |    4 
 fs/nfs/nfs2xdr.c           | 1349 ++++++++++++++++++++++++++++++--------------
 fs/nfs/nfs3proc.c          |    5 
 fs/nfs/nfs3xdr.c           |    6 
 fs/nfs/nfs4xdr.c           |    5 
 fs/nfs/proc.c              |    2 
 include/linux/sunrpc/xdr.h |    1 
 net/sunrpc/rpcb_clnt.c     |    5 
 net/sunrpc/xdr.c           |   17 +
 12 files changed, 1598 insertions(+), 715 deletions(-)
 create mode 100644 fs/lockd/clntxdr.c

-- 
Chuck Lever

[PATCH 01/11] NFS: Fix NFSv3 debugging messages in fs/nfs/nfs3proc.c

[Chuck Lever]

Clean up.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs3proc.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index fabb4f2..2792895 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -652,7 +652,8 @@ nfs3_proc_readdir(struct dentry *dentry, struct rpc_cred *cred,
 
 	nfs_free_fattr(res.dir_attr);
 out:
-	dprintk("NFS reply readdir: %d\n", status);
+	dprintk("NFS reply readdir%s: %d\n",
+			plus? "plus" : "", status);
 	return status;
 }
 
@@ -722,7 +723,7 @@ nfs3_proc_statfs(struct nfs_server *server, struct nfs_fh *fhandle,
 	dprintk("NFS call  fsstat\n");
 	nfs_fattr_init(stat->fattr);
 	status = rpc_call_sync(server->client, &msg, 0);
-	dprintk("NFS reply statfs: %d\n", status);
+	dprintk("NFS reply fsstat: %d\n", status);
 	return status;
 }
 

[PATCH 02/11] SUNRPC: Correct an rpcbind debugging message

[Chuck Lever]

Clean up.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 net/sunrpc/rpcb_clnt.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index c961094..63ec116 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -211,8 +211,9 @@ static int rpcb_create_local(void)
 	 */
 	clnt4 = rpc_bind_new_program(clnt, &rpcb_program, RPCBVERS_4);
 	if (IS_ERR(clnt4)) {
-		dprintk("RPC:       failed to create local rpcbind v4 "
-				"cleint (errno %ld).\n", PTR_ERR(clnt4));
+		dprintk("RPC:       failed to bind second program to "
+				"rpcbind v4 client (errno %ld).\n",
+				PTR_ERR(clnt4));
 		clnt4 = NULL;
 	}
 

[PATCH 03/11] SUNRPC: Refactor logic to NUL-terminate strings in pages

[Chuck Lever]

Clean up: Introduce a helper to '\0'-terminate XDR strings
that are placed in a page in the page cache.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs2xdr.c           |    6 +-----
 fs/nfs/nfs3xdr.c           |    6 +-----
 fs/nfs/nfs4xdr.c           |    5 +----
 include/linux/sunrpc/xdr.h |    1 +
 net/sunrpc/xdr.c           |   17 +++++++++++++++++
 5 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index db8846a..43413f2 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -596,7 +596,6 @@ nfs_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, void *dummy)
 	struct kvec *iov = rcvbuf->head;
 	size_t hdrlen;
 	u32 len, recvd;
-	char	*kaddr;
 	int	status;
 
 	if ((status = ntohl(*p++)))
@@ -623,10 +622,7 @@ nfs_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, void *dummy)
 		return -EIO;
 	}
 
-	/* NULL terminate the string we got */
-	kaddr = (char *)kmap_atomic(rcvbuf->pages[0], KM_USER0);
-	kaddr[len+rcvbuf->page_base] = '\0';
-	kunmap_atomic(kaddr, KM_USER0);
+	xdr_terminate_string(rcvbuf, len);
 	return 0;
 }
 
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index 9769704..a8170a0 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -824,7 +824,6 @@ nfs3_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	struct kvec *iov = rcvbuf->head;
 	size_t hdrlen;
 	u32 len, recvd;
-	char	*kaddr;
 	int	status;
 
 	status = ntohl(*p++);
@@ -857,10 +856,7 @@ nfs3_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 		return -EIO;
 	}
 
-	/* NULL terminate the string we got */
-	kaddr = (char*)kmap_atomic(rcvbuf->pages[0], KM_USER0);
-	kaddr[len+rcvbuf->page_base] = '\0';
-	kunmap_atomic(kaddr, KM_USER0);
+	xdr_terminate_string(rcvbuf, len);
 	return 0;
 }
 
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 08ef912..e1ed89e 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4299,7 +4299,6 @@ static int decode_readlink(struct xdr_stream *xdr, struct rpc_rqst *req)
 	size_t hdrlen;
 	u32 len, recvd;
 	__be32 *p;
-	char *kaddr;
 	int status;
 
 	status = decode_op_hdr(xdr, OP_READLINK);
@@ -4330,9 +4329,7 @@ static int decode_readlink(struct xdr_stream *xdr, struct rpc_rqst *req)
 	 * and and null-terminate the text (the VFS expects
 	 * null-termination).
 	 */
-	kaddr = (char *)kmap_atomic(rcvbuf->pages[0], KM_USER0);
-	kaddr[len+rcvbuf->page_base] = '\0';
-	kunmap_atomic(kaddr, KM_USER0);
+	xdr_terminate_string(rcvbuf, len);
 	return 0;
 out_overflow:
 	print_overflow_msg(__func__, xdr);
diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
index 35cf2e8..8c1dcbb 100644
--- a/include/linux/sunrpc/xdr.h
+++ b/include/linux/sunrpc/xdr.h
@@ -108,6 +108,7 @@ void	xdr_encode_pages(struct xdr_buf *, struct page **, unsigned int,
 			 unsigned int);
 void	xdr_inline_pages(struct xdr_buf *, unsigned int,
 			 struct page **, unsigned int, unsigned int);
+void	xdr_terminate_string(struct xdr_buf *, const u32);
 
 static inline __be32 *xdr_encode_array(__be32 *p, const void *s, unsigned int len)
 {
diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index a1f82a8..d463d20 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -111,6 +111,23 @@ xdr_decode_string_inplace(__be32 *p, char **sp,
 }
 EXPORT_SYMBOL_GPL(xdr_decode_string_inplace);
 
+/**
+ * xdr_terminate_string - '\0'-terminate a string residing in an xdr_buf
+ * @buf: XDR buffer where string resides
+ * @len: length of string, in bytes
+ *
+ */
+void
+xdr_terminate_string(struct xdr_buf *buf, const u32 len)
+{
+	char *kaddr;
+
+	kaddr = kmap_atomic(buf->pages[0], KM_USER0);
+	kaddr[buf->page_base + len] = '\0';
+	kunmap_atomic(kaddr, KM_USER0);
+}
+EXPORT_SYMBOL(xdr_terminate_string);
+
 void
 xdr_encode_pages(struct xdr_buf *xdr, struct page **pages, unsigned int base,
 		 unsigned int len)

[PATCH 04/11] NFS: Introduce new-style XDR encoding functions for NFSv2

[Chuck Lever]

Note that none of the helper functions are explicitly inlined.  The
compiler's optimizer is generally smart enough to figure out
automatically which functions may be inlined to conserve executable
size or streamline performance.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs2xdr.c |  411 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 408 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 43413f2..81c4d87 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -61,6 +61,23 @@
 #define NFS_readdirres_sz	(1)
 #define NFS_statfsres_sz	(1+NFS_info_sz)
 
+
+/*
+ * While encoding arguments, set up the reply buffer in advance to
+ * receive reply data directly into the page cache.
+ */
+static void prepare_reply_buffer(struct rpc_rqst *req, struct page **pages,
+				 unsigned int base, unsigned int len,
+				 unsigned int bufsize)
+{
+	struct rpc_auth	*auth = req->rq_cred->cr_auth;
+	unsigned int replen;
+
+	replen = RPC_REPHDRSIZE + auth->au_rslack + bufsize;
+	xdr_inline_pages(&req->rq_rcv_buf, replen << 2, pages, base, len);
+}
+
+
 /*
  * Common NFS XDR functions as inlines
  */
@@ -81,7 +98,7 @@ xdr_decode_fhandle(__be32 *p, struct nfs_fh *fhandle)
 }
 
 static inline __be32*
-xdr_encode_time(__be32 *p, struct timespec *timep)
+xdr_encode_time(__be32 *p, const struct timespec *timep)
 {
 	*p++ = htonl(timep->tv_sec);
 	/* Convert nanoseconds into microseconds */
@@ -90,7 +107,7 @@ xdr_encode_time(__be32 *p, struct timespec *timep)
 }
 
 static inline __be32*
-xdr_encode_current_server_time(__be32 *p, struct timespec *timep)
+xdr_encode_current_server_time(__be32 *p, const struct timespec *timep)
 {
 	/*
 	 * Passing the invalid value useconds=1000000 is a
@@ -174,6 +191,140 @@ xdr_encode_sattr(__be32 *p, struct iattr *attr)
 }
 
 /*
+ * Encode/decode NFSv2 basic data types
+ *
+ * Basic NFSv2 data types are defined in section 2.3 of RFC 1094:
+ * "NFS: Network File System Protocol Specification".
+ *
+ * Not all basic data types have their own encoding and decoding
+ * functions.  For run-time efficiency, some data types are encoded
+ * or decoded inline.
+ */
+
+/*
+ * 2.3.3.  fhandle
+ *
+ *	typedef opaque fhandle[FHSIZE];
+ */
+static void encode_fhandle(struct xdr_stream *xdr, const struct nfs_fh *fh)
+{
+	__be32 *p;
+
+	BUG_ON(unlikely(fh->size != NFS2_FHSIZE));
+	p = xdr_reserve_space(xdr, NFS2_FHSIZE);
+	BUG_ON(unlikely(p == NULL));
+	memcpy(p, fh->data, NFS2_FHSIZE);
+}
+
+/*
+ * 2.3.6.  sattr
+ *
+ *	struct sattr {
+ *		unsigned int	mode;
+ *		unsigned int	uid;
+ *		unsigned int	gid;
+ *		unsigned int	size;
+ *		timeval		atime;
+ *		timeval		mtime;
+ *	};
+ */
+
+#define NFS2_SATTR_NOT_SET	(0xffffffff)
+
+static __be32 *xdr_time_not_set(__be32 *p)
+{
+	*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
+	*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
+	return p;
+}
+
+static void encode_sattr(struct xdr_stream *xdr, const struct iattr *attr)
+{
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, NFS_sattr_sz << 2);
+	BUG_ON(unlikely(p == NULL));
+
+	if (attr->ia_valid & ATTR_MODE)
+		*p++ = cpu_to_be32(attr->ia_mode);
+	else
+		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
+	if (attr->ia_valid & ATTR_UID)
+		*p++ = cpu_to_be32(attr->ia_uid);
+	else
+		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
+	if (attr->ia_valid & ATTR_GID)
+		*p++ = cpu_to_be32(attr->ia_gid);
+	else
+		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
+	if (attr->ia_valid & ATTR_SIZE)
+		*p++ = cpu_to_be32((u32)attr->ia_size);
+	else
+		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
+
+	if (attr->ia_valid & ATTR_ATIME_SET)
+		p = xdr_encode_time(p, &attr->ia_atime);
+	else if (attr->ia_valid & ATTR_ATIME)
+		p = xdr_encode_current_server_time(p, &attr->ia_atime);
+	else
+		p = xdr_time_not_set(p);
+	if (attr->ia_valid & ATTR_MTIME_SET)
+		xdr_encode_time(p, &attr->ia_mtime);
+	else if (attr->ia_valid & ATTR_MTIME)
+		xdr_encode_current_server_time(p, &attr->ia_mtime);
+	else
+		xdr_time_not_set(p);
+}
+
+/*
+ * 2.3.7.  filename
+ *
+ *	typedef string filename<MAXNAMLEN>;
+ */
+static void encode_filename(struct xdr_stream *xdr,
+			    const char *name, u32 length)
+{
+	__be32 *p;
+
+	BUG_ON(unlikely(length > NFS2_MAXNAMLEN));
+	p = xdr_reserve_space(xdr, 4 + length);
+	BUG_ON(unlikely(p == NULL));
+	xdr_encode_opaque(p, name, length);
+}
+
+/*
+ * 2.3.8.  path
+ *
+ *	typedef string path<MAXPATHLEN>;
+ */
+static void encode_path(struct xdr_stream *xdr, struct page **pages, u32 length)
+{
+	__be32 *p;
+
+	BUG_ON(unlikely(length > NFS2_MAXPATHLEN));
+	p = xdr_reserve_space(xdr, 4);
+	BUG_ON(unlikely(p == NULL));
+	*p = cpu_to_be32(length);
+	xdr_write_pages(xdr, pages, 0, length);
+}
+
+/*
+ * 2.3.10.  diropargs
+ *
+ *	struct diropargs {
+ *		fhandle  dir;
+ *		filename name;
+ *	};
+ */
+static void encode_diropargs(struct xdr_stream *xdr, const struct nfs_fh *fh,
+			     const char *name, u32 length)
+{
+	encode_fhandle(xdr, fh);
+	encode_filename(xdr, name, length);
+}
+
+
+/*
  * NFS encode functions
  */
 /*
@@ -188,6 +339,16 @@ nfs_xdr_fhandle(struct rpc_rqst *req, __be32 *p, struct nfs_fh *fh)
 	return 0;
 }
 
+static int nfs2_xdr_enc_fhandle(struct rpc_rqst *req, __be32 *p,
+				const struct nfs_fh *fh)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_fhandle(&xdr, fh);
+	return 0;
+}
+
 /*
  * Encode SETATTR arguments
  */
@@ -201,6 +362,25 @@ nfs_xdr_sattrargs(struct rpc_rqst *req, __be32 *p, struct nfs_sattrargs *args)
 }
 
 /*
+ * 2.2.3.  sattrargs
+ *
+ *	struct sattrargs {
+ *		fhandle file;
+ *		sattr attributes;
+ *	};
+ */
+static int nfs2_xdr_enc_sattrargs(struct rpc_rqst *req, __be32 *p,
+				  const struct nfs_sattrargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_fhandle(&xdr, args->fh);
+	encode_sattr(&xdr, args->sattr);
+	return 0;
+}
+
+/*
  * Encode directory ops argument
  * LOOKUP, RMDIR
  */
@@ -213,6 +393,16 @@ nfs_xdr_diropargs(struct rpc_rqst *req, __be32 *p, struct nfs_diropargs *args)
 	return 0;
 }
 
+static int nfs2_xdr_enc_diropargs(struct rpc_rqst *req, __be32 *p,
+				  const struct nfs_diropargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_diropargs(&xdr, args->fh, args->name, args->len);
+	return 0;
+}
+
 /*
  * Encode REMOVE argument
  */
@@ -225,6 +415,18 @@ nfs_xdr_removeargs(struct rpc_rqst *req, __be32 *p, const struct nfs_removeargs
 	return 0;
 }
 
+static int nfs2_xdr_enc_readlinkargs(struct rpc_rqst *req, __be32 *p,
+				     const struct nfs_readlinkargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_fhandle(&xdr, args->fh);
+	prepare_reply_buffer(req, args->pages, args->pgbase,
+					args->pglen, NFS_readlinkres_sz);
+	return 0;
+}
+
 /*
  * Arguments to a READ call. Since we read data directly into the page
  * cache, we also set up the reply iovec here so that iov[1] points
@@ -253,6 +455,45 @@ nfs_xdr_readargs(struct rpc_rqst *req, __be32 *p, struct nfs_readargs *args)
 }
 
 /*
+ * 2.2.7.  readargs
+ *
+ *	struct readargs {
+ *		fhandle file;
+ *		unsigned offset;
+ *		unsigned count;
+ *		unsigned totalcount;
+ *	};
+ */
+static void encode_readargs(struct xdr_stream *xdr,
+			    const struct nfs_readargs *args)
+{
+	u32 offset = args->offset;
+	u32 count = args->count;
+	__be32 *p;
+
+	encode_fhandle(xdr, args->fh);
+
+	p = xdr_reserve_space(xdr, 4 + 4 + 4);
+	BUG_ON(unlikely(p == NULL));
+	*p++ = cpu_to_be32(offset);
+	*p++ = cpu_to_be32(count);
+	*p = cpu_to_be32(count);
+}
+
+static int nfs2_xdr_enc_readargs(struct rpc_rqst *req, __be32 *p,
+				 const struct nfs_readargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_readargs(&xdr, args);
+	prepare_reply_buffer(req, args->pages, args->pgbase,
+					args->count, NFS_readres_sz);
+	req->rq_rcv_buf.flags |= XDRBUF_READ;
+	return 0;
+}
+
+/*
  * Decode READ reply
  */
 static int
@@ -318,6 +559,48 @@ nfs_xdr_writeargs(struct rpc_rqst *req, __be32 *p, struct nfs_writeargs *args)
 }
 
 /*
+ * 2.2.9.  writeargs
+ *
+ *	struct writeargs {
+ *		fhandle file;
+ *		unsigned beginoffset;
+ *		unsigned offset;
+ *		unsigned totalcount;
+ *		nfsdata data;
+ *	};
+ */
+static void encode_writeargs(struct xdr_stream *xdr,
+			     const struct nfs_writeargs *args)
+{
+	u32 offset = args->offset;
+	u32 count = args->count;
+	__be32 *p;
+
+	encode_fhandle(xdr, args->fh);
+
+	p = xdr_reserve_space(xdr, 4 + 4 + 4 + 4);
+	BUG_ON(unlikely(p == NULL));
+	*p++ = cpu_to_be32(offset);
+	*p++ = cpu_to_be32(offset);
+	*p++ = cpu_to_be32(count);
+
+	/* nfsdata */
+	*p = cpu_to_be32(count);
+	xdr_write_pages(xdr, args->pages, args->pgbase, count);
+}
+
+static int nfs2_xdr_enc_writeargs(struct rpc_rqst *req, __be32 *p,
+				  const struct nfs_writeargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_writeargs(&xdr, args);
+	xdr.buf->flags |= XDRBUF_WRITE;
+	return 0;
+}
+
+/*
  * Encode create arguments
  * CREATE, MKDIR
  */
@@ -332,6 +615,35 @@ nfs_xdr_createargs(struct rpc_rqst *req, __be32 *p, struct nfs_createargs *args)
 }
 
 /*
+ * 2.2.10.  createargs
+ *
+ *	struct createargs {
+ *		diropargs where;
+ *		sattr attributes;
+ *	};
+ */
+static int nfs2_xdr_enc_createargs(struct rpc_rqst *req, __be32 *p,
+				   const struct nfs_createargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_diropargs(&xdr, args->fh, args->name, args->len);
+	encode_sattr(&xdr, args->sattr);
+	return 0;
+}
+
+static int nfs2_xdr_enc_removeargs(struct rpc_rqst *req, __be32 *p,
+				   const struct nfs_removeargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_diropargs(&xdr, args->fh, args->name.name, args->name.len);
+	return 0;
+}
+
+/*
  * Encode RENAME arguments
  */
 static int
@@ -346,6 +658,25 @@ nfs_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
 }
 
 /*
+ * 2.2.12.  renameargs
+ *
+ *	struct renameargs {
+ *		diropargs from;
+ *		diropargs to;
+ *	};
+ */
+static int nfs2_xdr_enc_renameargs(struct rpc_rqst *req, __be32 *p,
+				   const struct nfs_renameargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_diropargs(&xdr, args->fromfh, args->fromname, args->fromlen);
+	encode_diropargs(&xdr, args->tofh, args->toname, args->tolen);
+	return 0;
+}
+
+/*
  * Encode LINK arguments
  */
 static int
@@ -359,6 +690,25 @@ nfs_xdr_linkargs(struct rpc_rqst *req, __be32 *p, struct nfs_linkargs *args)
 }
 
 /*
+ * 2.2.13.  linkargs
+ *
+ *	struct linkargs {
+ *		fhandle from;
+ *		diropargs to;
+ *	};
+ */
+static int nfs2_xdr_enc_linkargs(struct rpc_rqst *req, __be32 *p,
+				 const struct nfs_linkargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_fhandle(&xdr, args->fromfh);
+	encode_diropargs(&xdr, args->tofh, args->toname, args->tolen);
+	return 0;
+}
+
+/*
  * Encode SYMLINK arguments
  */
 static int
@@ -388,6 +738,27 @@ nfs_xdr_symlinkargs(struct rpc_rqst *req, __be32 *p, struct nfs_symlinkargs *arg
 }
 
 /*
+ * 2.2.14.  symlinkargs
+ *
+ *	struct symlinkargs {
+ *		diropargs from;
+ *		path to;
+ *		sattr attributes;
+ *	};
+ */
+static int nfs2_xdr_enc_symlinkargs(struct rpc_rqst *req, __be32 *p,
+				    const struct nfs_symlinkargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_diropargs(&xdr, args->fromfh, args->fromname, args->fromlen);
+	encode_path(&xdr, args->pages, args->pathlen);
+	encode_sattr(&xdr, args->sattr);
+	return 0;
+}
+
+/*
  * Encode arguments to readdir call
  */
 static int
@@ -409,6 +780,40 @@ nfs_xdr_readdirargs(struct rpc_rqst *req, __be32 *p, struct nfs_readdirargs *arg
 }
 
 /*
+ * 2.2.17.  readdirargs
+ *
+ *	struct readdirargs {
+ *		fhandle dir;
+ *		nfscookie cookie;
+ *		unsigned count;
+ *	};
+ */
+static void encode_readdirargs(struct xdr_stream *xdr,
+			       const struct nfs_readdirargs *args)
+{
+	__be32 *p;
+
+	encode_fhandle(xdr, args->fh);
+
+	p = xdr_reserve_space(xdr, 4 + 4);
+	BUG_ON(unlikely(p == NULL));
+	*p++ = cpu_to_be32(args->cookie);
+	*p = cpu_to_be32(args->count);
+}
+
+static int nfs2_xdr_enc_readdirargs(struct rpc_rqst *req, __be32 *p,
+				    const struct nfs_readdirargs *args)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_readdirargs(&xdr, args);
+	prepare_reply_buffer(req, args->pages, 0,
+					args->count, NFS_readdirres_sz);
+	return 0;
+}
+
+/*
  * Decode the result of a readdir call.
  * We're not really decoding anymore, we just leave the buffer untouched
  * and only check that it is syntactically correct.
@@ -719,7 +1124,7 @@ nfs_stat_to_errno(int stat)
 #define PROC(proc, argtype, restype, timer)				\
 [NFSPROC_##proc] = {							\
 	.p_proc	    =  NFSPROC_##proc,					\
-	.p_encode   =  (kxdrproc_t) nfs_xdr_##argtype,			\
+	.p_encode   =  (kxdrproc_t) nfs2_xdr_enc_##argtype,		\
 	.p_decode   =  (kxdrproc_t) nfs_xdr_##restype,			\
 	.p_arglen   =  NFS_##argtype##_sz,				\
 	.p_replen   =  NFS_##restype##_sz,				\

Re: [PATCH 04/11] NFS: Introduce new-style XDR encoding functions for NFSv2

[Trond Myklebust]

On Fri, 2010-09-10 at 16:09 -0400, Chuck Lever wrote:
> Note that none of the helper functions are explicitly inlined.  The
> compiler's optimizer is generally smart enough to figure out
> automatically which functions may be inlined to conserve executable
> size or streamline performance.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfs/nfs2xdr.c |  411 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 408 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
> index 43413f2..81c4d87 100644
> --- a/fs/nfs/nfs2xdr.c
> +++ b/fs/nfs/nfs2xdr.c
> @@ -61,6 +61,23 @@
>  #define NFS_readdirres_sz	(1)
>  #define NFS_statfsres_sz	(1+NFS_info_sz)
>  
> +
> +/*
> + * While encoding arguments, set up the reply buffer in advance to
> + * receive reply data directly into the page cache.
> + */
> +static void prepare_reply_buffer(struct rpc_rqst *req, struct page **pages,
> +				 unsigned int base, unsigned int len,
> +				 unsigned int bufsize)
> +{
> +	struct rpc_auth	*auth = req->rq_cred->cr_auth;
> +	unsigned int replen;
> +
> +	replen = RPC_REPHDRSIZE + auth->au_rslack + bufsize;
> +	xdr_inline_pages(&req->rq_rcv_buf, replen << 2, pages, base, len);
> +}
> +
> +
>  /*
>   * Common NFS XDR functions as inlines
>   */
> @@ -81,7 +98,7 @@ xdr_decode_fhandle(__be32 *p, struct nfs_fh *fhandle)
>  }
>  
>  static inline __be32*
> -xdr_encode_time(__be32 *p, struct timespec *timep)
> +xdr_encode_time(__be32 *p, const struct timespec *timep)
>  {
>  	*p++ = htonl(timep->tv_sec);
>  	/* Convert nanoseconds into microseconds */
> @@ -90,7 +107,7 @@ xdr_encode_time(__be32 *p, struct timespec *timep)
>  }
>  
>  static inline __be32*
> -xdr_encode_current_server_time(__be32 *p, struct timespec *timep)
> +xdr_encode_current_server_time(__be32 *p, const struct timespec *timep)
>  {
>  	/*
>  	 * Passing the invalid value useconds=1000000 is a
> @@ -174,6 +191,140 @@ xdr_encode_sattr(__be32 *p, struct iattr *attr)
>  }
>  
>  /*
> + * Encode/decode NFSv2 basic data types
> + *
> + * Basic NFSv2 data types are defined in section 2.3 of RFC 1094:
> + * "NFS: Network File System Protocol Specification".
> + *
> + * Not all basic data types have their own encoding and decoding
> + * functions.  For run-time efficiency, some data types are encoded
> + * or decoded inline.
> + */
> +
> +/*
> + * 2.3.3.  fhandle
> + *
> + *	typedef opaque fhandle[FHSIZE];
> + */
> +static void encode_fhandle(struct xdr_stream *xdr, const struct nfs_fh *fh)
> +{
> +	__be32 *p;
> +
> +	BUG_ON(unlikely(fh->size != NFS2_FHSIZE));

Can we please stop all these 'unlikely()' declarations. Firstly, it is
100% redundant here, since the definition of BUG_ON() is as follows:

#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); }
while(0)

Secondly, we're putting way too many of these things in the code. At
this point they're starting to just obfuscate...

> +	p = xdr_reserve_space(xdr, NFS2_FHSIZE);
> +	BUG_ON(unlikely(p == NULL));

This test is unnecessary. If p==NULL, we'll Oops anyway right here:

> +	memcpy(p, fh->data, NFS2_FHSIZE);
> +}
> +
> +/*
> + * 2.3.6.  sattr
> + *
> + *	struct sattr {
> + *		unsigned int	mode;
> + *		unsigned int	uid;
> + *		unsigned int	gid;
> + *		unsigned int	size;
> + *		timeval		atime;
> + *		timeval		mtime;
> + *	};
> + */
> +
> +#define NFS2_SATTR_NOT_SET	(0xffffffff)
> +
> +static __be32 *xdr_time_not_set(__be32 *p)
> +{
> +	*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
> +	*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
> +	return p;
> +}
> +
> +static void encode_sattr(struct xdr_stream *xdr, const struct iattr *attr)
> +{
> +	__be32 *p;
> +
> +	p = xdr_reserve_space(xdr, NFS_sattr_sz << 2);
> +	BUG_ON(unlikely(p == NULL));

The above is unnecessary. We'll Oops below anyway:

> +
> +	if (attr->ia_valid & ATTR_MODE)
> +		*p++ = cpu_to_be32(attr->ia_mode);
> +	else
> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
> +	if (attr->ia_valid & ATTR_UID)
> +		*p++ = cpu_to_be32(attr->ia_uid);
> +	else
> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
> +	if (attr->ia_valid & ATTR_GID)
> +		*p++ = cpu_to_be32(attr->ia_gid);
> +	else
> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
> +	if (attr->ia_valid & ATTR_SIZE)
> +		*p++ = cpu_to_be32((u32)attr->ia_size);
> +	else
> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
> +
> +	if (attr->ia_valid & ATTR_ATIME_SET)
> +		p = xdr_encode_time(p, &attr->ia_atime);
> +	else if (attr->ia_valid & ATTR_ATIME)
> +		p = xdr_encode_current_server_time(p, &attr->ia_atime);
> +	else
> +		p = xdr_time_not_set(p);
> +	if (attr->ia_valid & ATTR_MTIME_SET)
> +		xdr_encode_time(p, &attr->ia_mtime);
> +	else if (attr->ia_valid & ATTR_MTIME)
> +		xdr_encode_current_server_time(p, &attr->ia_mtime);
> +	else
> +		xdr_time_not_set(p);
> +}
> +
> +/*
> + * 2.3.7.  filename
> + *
> + *	typedef string filename<MAXNAMLEN>;
> + */
> +static void encode_filename(struct xdr_stream *xdr,
> +			    const char *name, u32 length)
> +{
> +	__be32 *p;
> +
> +	BUG_ON(unlikely(length > NFS2_MAXNAMLEN));

See comment on BUG_ON(unlikely()) at the top...

> +	p = xdr_reserve_space(xdr, 4 + length);
> +	BUG_ON(unlikely(p == NULL));

The above is unnecessary... If p==NULL, we'll find out when we do:

> +	xdr_encode_opaque(p, name, length);
> +}
> +
> +/*
> + * 2.3.8.  path
> + *
> + *	typedef string path<MAXPATHLEN>;
> + */
> +static void encode_path(struct xdr_stream *xdr, struct page **pages, u32 length)
> +{
> +	__be32 *p;
> +
> +	BUG_ON(unlikely(length > NFS2_MAXPATHLEN));

Ditto...

> +	p = xdr_reserve_space(xdr, 4);
> +	BUG_ON(unlikely(p == NULL));

Ditto...

> +	*p = cpu_to_be32(length);
> +	xdr_write_pages(xdr, pages, 0, length);
> +}
> +
> +/*
> + * 2.3.10.  diropargs
> + *
> + *	struct diropargs {
> + *		fhandle  dir;
> + *		filename name;
> + *	};
> + */
> +static void encode_diropargs(struct xdr_stream *xdr, const struct nfs_fh *fh,
> +			     const char *name, u32 length)
> +{
> +	encode_fhandle(xdr, fh);
> +	encode_filename(xdr, name, length);
> +}
> +
> +
> +/*
>   * NFS encode functions
>   */
>  /*
> @@ -188,6 +339,16 @@ nfs_xdr_fhandle(struct rpc_rqst *req, __be32 *p, struct nfs_fh *fh)
>  	return 0;
>  }
>  
> +static int nfs2_xdr_enc_fhandle(struct rpc_rqst *req, __be32 *p,
> +				const struct nfs_fh *fh)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_fhandle(&xdr, fh);
> +	return 0;
> +}
> +
>  /*
>   * Encode SETATTR arguments
>   */
> @@ -201,6 +362,25 @@ nfs_xdr_sattrargs(struct rpc_rqst *req, __be32 *p, struct nfs_sattrargs *args)
>  }
>  
>  /*
> + * 2.2.3.  sattrargs
> + *
> + *	struct sattrargs {
> + *		fhandle file;
> + *		sattr attributes;
> + *	};
> + */
> +static int nfs2_xdr_enc_sattrargs(struct rpc_rqst *req, __be32 *p,
> +				  const struct nfs_sattrargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_fhandle(&xdr, args->fh);
> +	encode_sattr(&xdr, args->sattr);
> +	return 0;
> +}
> +
> +/*
>   * Encode directory ops argument
>   * LOOKUP, RMDIR
>   */
> @@ -213,6 +393,16 @@ nfs_xdr_diropargs(struct rpc_rqst *req, __be32 *p, struct nfs_diropargs *args)
>  	return 0;
>  }
>  
> +static int nfs2_xdr_enc_diropargs(struct rpc_rqst *req, __be32 *p,
> +				  const struct nfs_diropargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_diropargs(&xdr, args->fh, args->name, args->len);
> +	return 0;
> +}
> +
>  /*
>   * Encode REMOVE argument
>   */
> @@ -225,6 +415,18 @@ nfs_xdr_removeargs(struct rpc_rqst *req, __be32 *p, const struct nfs_removeargs
>  	return 0;
>  }
>  
> +static int nfs2_xdr_enc_readlinkargs(struct rpc_rqst *req, __be32 *p,
> +				     const struct nfs_readlinkargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_fhandle(&xdr, args->fh);
> +	prepare_reply_buffer(req, args->pages, args->pgbase,
> +					args->pglen, NFS_readlinkres_sz);
> +	return 0;
> +}
> +
>  /*
>   * Arguments to a READ call. Since we read data directly into the page
>   * cache, we also set up the reply iovec here so that iov[1] points
> @@ -253,6 +455,45 @@ nfs_xdr_readargs(struct rpc_rqst *req, __be32 *p, struct nfs_readargs *args)
>  }
>  
>  /*
> + * 2.2.7.  readargs
> + *
> + *	struct readargs {
> + *		fhandle file;
> + *		unsigned offset;
> + *		unsigned count;
> + *		unsigned totalcount;
> + *	};
> + */
> +static void encode_readargs(struct xdr_stream *xdr,
> +			    const struct nfs_readargs *args)
> +{
> +	u32 offset = args->offset;
> +	u32 count = args->count;
> +	__be32 *p;
> +
> +	encode_fhandle(xdr, args->fh);
> +
> +	p = xdr_reserve_space(xdr, 4 + 4 + 4);
> +	BUG_ON(unlikely(p == NULL));

Ditto...

> +	*p++ = cpu_to_be32(offset);
> +	*p++ = cpu_to_be32(count);
> +	*p = cpu_to_be32(count);
> +}
> +
> +static int nfs2_xdr_enc_readargs(struct rpc_rqst *req, __be32 *p,
> +				 const struct nfs_readargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_readargs(&xdr, args);
> +	prepare_reply_buffer(req, args->pages, args->pgbase,
> +					args->count, NFS_readres_sz);
> +	req->rq_rcv_buf.flags |= XDRBUF_READ;
> +	return 0;
> +}
> +
> +/*
>   * Decode READ reply
>   */
>  static int
> @@ -318,6 +559,48 @@ nfs_xdr_writeargs(struct rpc_rqst *req, __be32 *p, struct nfs_writeargs *args)
>  }
>  
>  /*
> + * 2.2.9.  writeargs
> + *
> + *	struct writeargs {
> + *		fhandle file;
> + *		unsigned beginoffset;
> + *		unsigned offset;
> + *		unsigned totalcount;
> + *		nfsdata data;
> + *	};
> + */
> +static void encode_writeargs(struct xdr_stream *xdr,
> +			     const struct nfs_writeargs *args)
> +{
> +	u32 offset = args->offset;
> +	u32 count = args->count;
> +	__be32 *p;
> +
> +	encode_fhandle(xdr, args->fh);
> +
> +	p = xdr_reserve_space(xdr, 4 + 4 + 4 + 4);
> +	BUG_ON(unlikely(p == NULL));

Ditto...

> +	*p++ = cpu_to_be32(offset);
> +	*p++ = cpu_to_be32(offset);
> +	*p++ = cpu_to_be32(count);
> +
> +	/* nfsdata */
> +	*p = cpu_to_be32(count);
> +	xdr_write_pages(xdr, args->pages, args->pgbase, count);
> +}
> +
> +static int nfs2_xdr_enc_writeargs(struct rpc_rqst *req, __be32 *p,
> +				  const struct nfs_writeargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_writeargs(&xdr, args);
> +	xdr.buf->flags |= XDRBUF_WRITE;
> +	return 0;
> +}
> +
> +/*
>   * Encode create arguments
>   * CREATE, MKDIR
>   */
> @@ -332,6 +615,35 @@ nfs_xdr_createargs(struct rpc_rqst *req, __be32 *p, struct nfs_createargs *args)
>  }
>  
>  /*
> + * 2.2.10.  createargs
> + *
> + *	struct createargs {
> + *		diropargs where;
> + *		sattr attributes;
> + *	};
> + */
> +static int nfs2_xdr_enc_createargs(struct rpc_rqst *req, __be32 *p,
> +				   const struct nfs_createargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_diropargs(&xdr, args->fh, args->name, args->len);
> +	encode_sattr(&xdr, args->sattr);
> +	return 0;
> +}
> +
> +static int nfs2_xdr_enc_removeargs(struct rpc_rqst *req, __be32 *p,
> +				   const struct nfs_removeargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_diropargs(&xdr, args->fh, args->name.name, args->name.len);
> +	return 0;
> +}
> +
> +/*
>   * Encode RENAME arguments
>   */
>  static int
> @@ -346,6 +658,25 @@ nfs_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
>  }
>  
>  /*
> + * 2.2.12.  renameargs
> + *
> + *	struct renameargs {
> + *		diropargs from;
> + *		diropargs to;
> + *	};
> + */
> +static int nfs2_xdr_enc_renameargs(struct rpc_rqst *req, __be32 *p,
> +				   const struct nfs_renameargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_diropargs(&xdr, args->fromfh, args->fromname, args->fromlen);
> +	encode_diropargs(&xdr, args->tofh, args->toname, args->tolen);
> +	return 0;
> +}
> +
> +/*
>   * Encode LINK arguments
>   */
>  static int
> @@ -359,6 +690,25 @@ nfs_xdr_linkargs(struct rpc_rqst *req, __be32 *p, struct nfs_linkargs *args)
>  }
>  
>  /*
> + * 2.2.13.  linkargs
> + *
> + *	struct linkargs {
> + *		fhandle from;
> + *		diropargs to;
> + *	};
> + */
> +static int nfs2_xdr_enc_linkargs(struct rpc_rqst *req, __be32 *p,
> +				 const struct nfs_linkargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_fhandle(&xdr, args->fromfh);
> +	encode_diropargs(&xdr, args->tofh, args->toname, args->tolen);
> +	return 0;
> +}
> +
> +/*
>   * Encode SYMLINK arguments
>   */
>  static int
> @@ -388,6 +738,27 @@ nfs_xdr_symlinkargs(struct rpc_rqst *req, __be32 *p, struct nfs_symlinkargs *arg
>  }
>  
>  /*
> + * 2.2.14.  symlinkargs
> + *
> + *	struct symlinkargs {
> + *		diropargs from;
> + *		path to;
> + *		sattr attributes;
> + *	};
> + */
> +static int nfs2_xdr_enc_symlinkargs(struct rpc_rqst *req, __be32 *p,
> +				    const struct nfs_symlinkargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_diropargs(&xdr, args->fromfh, args->fromname, args->fromlen);
> +	encode_path(&xdr, args->pages, args->pathlen);
> +	encode_sattr(&xdr, args->sattr);
> +	return 0;
> +}
> +
> +/*
>   * Encode arguments to readdir call
>   */
>  static int
> @@ -409,6 +780,40 @@ nfs_xdr_readdirargs(struct rpc_rqst *req, __be32 *p, struct nfs_readdirargs *arg
>  }
>  
>  /*
> + * 2.2.17.  readdirargs
> + *
> + *	struct readdirargs {
> + *		fhandle dir;
> + *		nfscookie cookie;
> + *		unsigned count;
> + *	};
> + */
> +static void encode_readdirargs(struct xdr_stream *xdr,
> +			       const struct nfs_readdirargs *args)
> +{
> +	__be32 *p;
> +
> +	encode_fhandle(xdr, args->fh);
> +
> +	p = xdr_reserve_space(xdr, 4 + 4);
> +	BUG_ON(unlikely(p == NULL));

Ditto...

> +	*p++ = cpu_to_be32(args->cookie);
> +	*p = cpu_to_be32(args->count);
> +}
> +
> +static int nfs2_xdr_enc_readdirargs(struct rpc_rqst *req, __be32 *p,
> +				    const struct nfs_readdirargs *args)
> +{
> +	struct xdr_stream xdr;
> +
> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
> +	encode_readdirargs(&xdr, args);
> +	prepare_reply_buffer(req, args->pages, 0,
> +					args->count, NFS_readdirres_sz);
> +	return 0;
> +}
> +
> +/*
>   * Decode the result of a readdir call.
>   * We're not really decoding anymore, we just leave the buffer untouched
>   * and only check that it is syntactically correct.
> @@ -719,7 +1124,7 @@ nfs_stat_to_errno(int stat)
>  #define PROC(proc, argtype, restype, timer)				\
>  [NFSPROC_##proc] = {							\
>  	.p_proc	    =  NFSPROC_##proc,					\
> -	.p_encode   =  (kxdrproc_t) nfs_xdr_##argtype,			\
> +	.p_encode   =  (kxdrproc_t) nfs2_xdr_enc_##argtype,		\
>  	.p_decode   =  (kxdrproc_t) nfs_xdr_##restype,			\
>  	.p_arglen   =  NFS_##argtype##_sz,				\
>  	.p_replen   =  NFS_##restype##_sz,				\
> 

Otherwise, the code looks fine...

Re: [PATCH 04/11] NFS: Introduce new-style XDR encoding functions for NFSv2

[Chuck Lever]

On Sep 13, 2010, at 3:52 PM, Trond Myklebust wrote:

> On Fri, 2010-09-10 at 16:09 -0400, Chuck Lever wrote:
>> Note that none of the helper functions are explicitly inlined.  The
>> compiler's optimizer is generally smart enough to figure out
>> automatically which functions may be inlined to conserve executable
>> size or streamline performance.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 
>> fs/nfs/nfs2xdr.c |  411 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 408 insertions(+), 3 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
>> index 43413f2..81c4d87 100644
>> --- a/fs/nfs/nfs2xdr.c
>> +++ b/fs/nfs/nfs2xdr.c
>> @@ -61,6 +61,23 @@
>> #define NFS_readdirres_sz	(1)
>> #define NFS_statfsres_sz	(1+NFS_info_sz)
>> 
>> +
>> +/*
>> + * While encoding arguments, set up the reply buffer in advance to
>> + * receive reply data directly into the page cache.
>> + */
>> +static void prepare_reply_buffer(struct rpc_rqst *req, struct page **pages,
>> +				 unsigned int base, unsigned int len,
>> +				 unsigned int bufsize)
>> +{
>> +	struct rpc_auth	*auth = req->rq_cred->cr_auth;
>> +	unsigned int replen;
>> +
>> +	replen = RPC_REPHDRSIZE + auth->au_rslack + bufsize;
>> +	xdr_inline_pages(&req->rq_rcv_buf, replen << 2, pages, base, len);
>> +}
>> +
>> +
>> /*
>>  * Common NFS XDR functions as inlines
>>  */
>> @@ -81,7 +98,7 @@ xdr_decode_fhandle(__be32 *p, struct nfs_fh *fhandle)
>> }
>> 
>> static inline __be32*
>> -xdr_encode_time(__be32 *p, struct timespec *timep)
>> +xdr_encode_time(__be32 *p, const struct timespec *timep)
>> {
>> 	*p++ = htonl(timep->tv_sec);
>> 	/* Convert nanoseconds into microseconds */
>> @@ -90,7 +107,7 @@ xdr_encode_time(__be32 *p, struct timespec *timep)
>> }
>> 
>> static inline __be32*
>> -xdr_encode_current_server_time(__be32 *p, struct timespec *timep)
>> +xdr_encode_current_server_time(__be32 *p, const struct timespec *timep)
>> {
>> 	/*
>> 	 * Passing the invalid value useconds=1000000 is a
>> @@ -174,6 +191,140 @@ xdr_encode_sattr(__be32 *p, struct iattr *attr)
>> }
>> 
>> /*
>> + * Encode/decode NFSv2 basic data types
>> + *
>> + * Basic NFSv2 data types are defined in section 2.3 of RFC 1094:
>> + * "NFS: Network File System Protocol Specification".
>> + *
>> + * Not all basic data types have their own encoding and decoding
>> + * functions.  For run-time efficiency, some data types are encoded
>> + * or decoded inline.
>> + */
>> +
>> +/*
>> + * 2.3.3.  fhandle
>> + *
>> + *	typedef opaque fhandle[FHSIZE];
>> + */
>> +static void encode_fhandle(struct xdr_stream *xdr, const struct nfs_fh *fh)
>> +{
>> +	__be32 *p;
>> +
>> +	BUG_ON(unlikely(fh->size != NFS2_FHSIZE));
> 
> Can we please stop all these 'unlikely()' declarations. Firstly, it is
> 100% redundant here, since the definition of BUG_ON() is as follows:
> 
> #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); }
> while(0)
> 
> Secondly, we're putting way too many of these things in the code. At
> this point they're starting to just obfuscate...

Sure, I'll see what can be done.

> 
>> +	p = xdr_reserve_space(xdr, NFS2_FHSIZE);
>> +	BUG_ON(unlikely(p == NULL));
> 
> This test is unnecessary. If p==NULL, we'll Oops anyway right here:
> 
>> +	memcpy(p, fh->data, NFS2_FHSIZE);
>> +}
>> +
>> +/*
>> + * 2.3.6.  sattr
>> + *
>> + *	struct sattr {
>> + *		unsigned int	mode;
>> + *		unsigned int	uid;
>> + *		unsigned int	gid;
>> + *		unsigned int	size;
>> + *		timeval		atime;
>> + *		timeval		mtime;
>> + *	};
>> + */
>> +
>> +#define NFS2_SATTR_NOT_SET	(0xffffffff)
>> +
>> +static __be32 *xdr_time_not_set(__be32 *p)
>> +{
>> +	*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
>> +	*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
>> +	return p;
>> +}
>> +
>> +static void encode_sattr(struct xdr_stream *xdr, const struct iattr *attr)
>> +{
>> +	__be32 *p;
>> +
>> +	p = xdr_reserve_space(xdr, NFS_sattr_sz << 2);
>> +	BUG_ON(unlikely(p == NULL));
> 
> The above is unnecessary. We'll Oops below anyway:
> 
>> +
>> +	if (attr->ia_valid & ATTR_MODE)
>> +		*p++ = cpu_to_be32(attr->ia_mode);
>> +	else
>> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
>> +	if (attr->ia_valid & ATTR_UID)
>> +		*p++ = cpu_to_be32(attr->ia_uid);
>> +	else
>> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
>> +	if (attr->ia_valid & ATTR_GID)
>> +		*p++ = cpu_to_be32(attr->ia_gid);
>> +	else
>> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
>> +	if (attr->ia_valid & ATTR_SIZE)
>> +		*p++ = cpu_to_be32((u32)attr->ia_size);
>> +	else
>> +		*p++ = cpu_to_be32(NFS2_SATTR_NOT_SET);
>> +
>> +	if (attr->ia_valid & ATTR_ATIME_SET)
>> +		p = xdr_encode_time(p, &attr->ia_atime);
>> +	else if (attr->ia_valid & ATTR_ATIME)
>> +		p = xdr_encode_current_server_time(p, &attr->ia_atime);
>> +	else
>> +		p = xdr_time_not_set(p);
>> +	if (attr->ia_valid & ATTR_MTIME_SET)
>> +		xdr_encode_time(p, &attr->ia_mtime);
>> +	else if (attr->ia_valid & ATTR_MTIME)
>> +		xdr_encode_current_server_time(p, &attr->ia_mtime);
>> +	else
>> +		xdr_time_not_set(p);
>> +}
>> +
>> +/*
>> + * 2.3.7.  filename
>> + *
>> + *	typedef string filename<MAXNAMLEN>;
>> + */
>> +static void encode_filename(struct xdr_stream *xdr,
>> +			    const char *name, u32 length)
>> +{
>> +	__be32 *p;
>> +
>> +	BUG_ON(unlikely(length > NFS2_MAXNAMLEN));
> 
> See comment on BUG_ON(unlikely()) at the top...
> 
>> +	p = xdr_reserve_space(xdr, 4 + length);
>> +	BUG_ON(unlikely(p == NULL));
> 
> The above is unnecessary... If p==NULL, we'll find out when we do:
> 
>> +	xdr_encode_opaque(p, name, length);
>> +}
>> +
>> +/*
>> + * 2.3.8.  path
>> + *
>> + *	typedef string path<MAXPATHLEN>;
>> + */
>> +static void encode_path(struct xdr_stream *xdr, struct page **pages, u32 length)
>> +{
>> +	__be32 *p;
>> +
>> +	BUG_ON(unlikely(length > NFS2_MAXPATHLEN));
> 
> Ditto...
> 
>> +	p = xdr_reserve_space(xdr, 4);
>> +	BUG_ON(unlikely(p == NULL));
> 
> Ditto...
> 
>> +	*p = cpu_to_be32(length);
>> +	xdr_write_pages(xdr, pages, 0, length);
>> +}
>> +
>> +/*
>> + * 2.3.10.  diropargs
>> + *
>> + *	struct diropargs {
>> + *		fhandle  dir;
>> + *		filename name;
>> + *	};
>> + */
>> +static void encode_diropargs(struct xdr_stream *xdr, const struct nfs_fh *fh,
>> +			     const char *name, u32 length)
>> +{
>> +	encode_fhandle(xdr, fh);
>> +	encode_filename(xdr, name, length);
>> +}
>> +
>> +
>> +/*
>>  * NFS encode functions
>>  */
>> /*
>> @@ -188,6 +339,16 @@ nfs_xdr_fhandle(struct rpc_rqst *req, __be32 *p, struct nfs_fh *fh)
>> 	return 0;
>> }
>> 
>> +static int nfs2_xdr_enc_fhandle(struct rpc_rqst *req, __be32 *p,
>> +				const struct nfs_fh *fh)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_fhandle(&xdr, fh);
>> +	return 0;
>> +}
>> +
>> /*
>>  * Encode SETATTR arguments
>>  */
>> @@ -201,6 +362,25 @@ nfs_xdr_sattrargs(struct rpc_rqst *req, __be32 *p, struct nfs_sattrargs *args)
>> }
>> 
>> /*
>> + * 2.2.3.  sattrargs
>> + *
>> + *	struct sattrargs {
>> + *		fhandle file;
>> + *		sattr attributes;
>> + *	};
>> + */
>> +static int nfs2_xdr_enc_sattrargs(struct rpc_rqst *req, __be32 *p,
>> +				  const struct nfs_sattrargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_fhandle(&xdr, args->fh);
>> +	encode_sattr(&xdr, args->sattr);
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Encode directory ops argument
>>  * LOOKUP, RMDIR
>>  */
>> @@ -213,6 +393,16 @@ nfs_xdr_diropargs(struct rpc_rqst *req, __be32 *p, struct nfs_diropargs *args)
>> 	return 0;
>> }
>> 
>> +static int nfs2_xdr_enc_diropargs(struct rpc_rqst *req, __be32 *p,
>> +				  const struct nfs_diropargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_diropargs(&xdr, args->fh, args->name, args->len);
>> +	return 0;
>> +}
>> +
>> /*
>>  * Encode REMOVE argument
>>  */
>> @@ -225,6 +415,18 @@ nfs_xdr_removeargs(struct rpc_rqst *req, __be32 *p, const struct nfs_removeargs
>> 	return 0;
>> }
>> 
>> +static int nfs2_xdr_enc_readlinkargs(struct rpc_rqst *req, __be32 *p,
>> +				     const struct nfs_readlinkargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_fhandle(&xdr, args->fh);
>> +	prepare_reply_buffer(req, args->pages, args->pgbase,
>> +					args->pglen, NFS_readlinkres_sz);
>> +	return 0;
>> +}
>> +
>> /*
>>  * Arguments to a READ call. Since we read data directly into the page
>>  * cache, we also set up the reply iovec here so that iov[1] points
>> @@ -253,6 +455,45 @@ nfs_xdr_readargs(struct rpc_rqst *req, __be32 *p, struct nfs_readargs *args)
>> }
>> 
>> /*
>> + * 2.2.7.  readargs
>> + *
>> + *	struct readargs {
>> + *		fhandle file;
>> + *		unsigned offset;
>> + *		unsigned count;
>> + *		unsigned totalcount;
>> + *	};
>> + */
>> +static void encode_readargs(struct xdr_stream *xdr,
>> +			    const struct nfs_readargs *args)
>> +{
>> +	u32 offset = args->offset;
>> +	u32 count = args->count;
>> +	__be32 *p;
>> +
>> +	encode_fhandle(xdr, args->fh);
>> +
>> +	p = xdr_reserve_space(xdr, 4 + 4 + 4);
>> +	BUG_ON(unlikely(p == NULL));
> 
> Ditto...
> 
>> +	*p++ = cpu_to_be32(offset);
>> +	*p++ = cpu_to_be32(count);
>> +	*p = cpu_to_be32(count);
>> +}
>> +
>> +static int nfs2_xdr_enc_readargs(struct rpc_rqst *req, __be32 *p,
>> +				 const struct nfs_readargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_readargs(&xdr, args);
>> +	prepare_reply_buffer(req, args->pages, args->pgbase,
>> +					args->count, NFS_readres_sz);
>> +	req->rq_rcv_buf.flags |= XDRBUF_READ;
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Decode READ reply
>>  */
>> static int
>> @@ -318,6 +559,48 @@ nfs_xdr_writeargs(struct rpc_rqst *req, __be32 *p, struct nfs_writeargs *args)
>> }
>> 
>> /*
>> + * 2.2.9.  writeargs
>> + *
>> + *	struct writeargs {
>> + *		fhandle file;
>> + *		unsigned beginoffset;
>> + *		unsigned offset;
>> + *		unsigned totalcount;
>> + *		nfsdata data;
>> + *	};
>> + */
>> +static void encode_writeargs(struct xdr_stream *xdr,
>> +			     const struct nfs_writeargs *args)
>> +{
>> +	u32 offset = args->offset;
>> +	u32 count = args->count;
>> +	__be32 *p;
>> +
>> +	encode_fhandle(xdr, args->fh);
>> +
>> +	p = xdr_reserve_space(xdr, 4 + 4 + 4 + 4);
>> +	BUG_ON(unlikely(p == NULL));
> 
> Ditto...
> 
>> +	*p++ = cpu_to_be32(offset);
>> +	*p++ = cpu_to_be32(offset);
>> +	*p++ = cpu_to_be32(count);
>> +
>> +	/* nfsdata */
>> +	*p = cpu_to_be32(count);
>> +	xdr_write_pages(xdr, args->pages, args->pgbase, count);
>> +}
>> +
>> +static int nfs2_xdr_enc_writeargs(struct rpc_rqst *req, __be32 *p,
>> +				  const struct nfs_writeargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_writeargs(&xdr, args);
>> +	xdr.buf->flags |= XDRBUF_WRITE;
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Encode create arguments
>>  * CREATE, MKDIR
>>  */
>> @@ -332,6 +615,35 @@ nfs_xdr_createargs(struct rpc_rqst *req, __be32 *p, struct nfs_createargs *args)
>> }
>> 
>> /*
>> + * 2.2.10.  createargs
>> + *
>> + *	struct createargs {
>> + *		diropargs where;
>> + *		sattr attributes;
>> + *	};
>> + */
>> +static int nfs2_xdr_enc_createargs(struct rpc_rqst *req, __be32 *p,
>> +				   const struct nfs_createargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_diropargs(&xdr, args->fh, args->name, args->len);
>> +	encode_sattr(&xdr, args->sattr);
>> +	return 0;
>> +}
>> +
>> +static int nfs2_xdr_enc_removeargs(struct rpc_rqst *req, __be32 *p,
>> +				   const struct nfs_removeargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_diropargs(&xdr, args->fh, args->name.name, args->name.len);
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Encode RENAME arguments
>>  */
>> static int
>> @@ -346,6 +658,25 @@ nfs_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
>> }
>> 
>> /*
>> + * 2.2.12.  renameargs
>> + *
>> + *	struct renameargs {
>> + *		diropargs from;
>> + *		diropargs to;
>> + *	};
>> + */
>> +static int nfs2_xdr_enc_renameargs(struct rpc_rqst *req, __be32 *p,
>> +				   const struct nfs_renameargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_diropargs(&xdr, args->fromfh, args->fromname, args->fromlen);
>> +	encode_diropargs(&xdr, args->tofh, args->toname, args->tolen);
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Encode LINK arguments
>>  */
>> static int
>> @@ -359,6 +690,25 @@ nfs_xdr_linkargs(struct rpc_rqst *req, __be32 *p, struct nfs_linkargs *args)
>> }
>> 
>> /*
>> + * 2.2.13.  linkargs
>> + *
>> + *	struct linkargs {
>> + *		fhandle from;
>> + *		diropargs to;
>> + *	};
>> + */
>> +static int nfs2_xdr_enc_linkargs(struct rpc_rqst *req, __be32 *p,
>> +				 const struct nfs_linkargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_fhandle(&xdr, args->fromfh);
>> +	encode_diropargs(&xdr, args->tofh, args->toname, args->tolen);
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Encode SYMLINK arguments
>>  */
>> static int
>> @@ -388,6 +738,27 @@ nfs_xdr_symlinkargs(struct rpc_rqst *req, __be32 *p, struct nfs_symlinkargs *arg
>> }
>> 
>> /*
>> + * 2.2.14.  symlinkargs
>> + *
>> + *	struct symlinkargs {
>> + *		diropargs from;
>> + *		path to;
>> + *		sattr attributes;
>> + *	};
>> + */
>> +static int nfs2_xdr_enc_symlinkargs(struct rpc_rqst *req, __be32 *p,
>> +				    const struct nfs_symlinkargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_diropargs(&xdr, args->fromfh, args->fromname, args->fromlen);
>> +	encode_path(&xdr, args->pages, args->pathlen);
>> +	encode_sattr(&xdr, args->sattr);
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Encode arguments to readdir call
>>  */
>> static int
>> @@ -409,6 +780,40 @@ nfs_xdr_readdirargs(struct rpc_rqst *req, __be32 *p, struct nfs_readdirargs *arg
>> }
>> 
>> /*
>> + * 2.2.17.  readdirargs
>> + *
>> + *	struct readdirargs {
>> + *		fhandle dir;
>> + *		nfscookie cookie;
>> + *		unsigned count;
>> + *	};
>> + */
>> +static void encode_readdirargs(struct xdr_stream *xdr,
>> +			       const struct nfs_readdirargs *args)
>> +{
>> +	__be32 *p;
>> +
>> +	encode_fhandle(xdr, args->fh);
>> +
>> +	p = xdr_reserve_space(xdr, 4 + 4);
>> +	BUG_ON(unlikely(p == NULL));
> 
> Ditto...
> 
>> +	*p++ = cpu_to_be32(args->cookie);
>> +	*p = cpu_to_be32(args->count);
>> +}
>> +
>> +static int nfs2_xdr_enc_readdirargs(struct rpc_rqst *req, __be32 *p,
>> +				    const struct nfs_readdirargs *args)
>> +{
>> +	struct xdr_stream xdr;
>> +
>> +	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
>> +	encode_readdirargs(&xdr, args);
>> +	prepare_reply_buffer(req, args->pages, 0,
>> +					args->count, NFS_readdirres_sz);
>> +	return 0;
>> +}
>> +
>> +/*
>>  * Decode the result of a readdir call.
>>  * We're not really decoding anymore, we just leave the buffer untouched
>>  * and only check that it is syntactically correct.
>> @@ -719,7 +1124,7 @@ nfs_stat_to_errno(int stat)
>> #define PROC(proc, argtype, restype, timer)				\
>> [NFSPROC_##proc] = {							\
>> 	.p_proc	    =  NFSPROC_##proc,					\
>> -	.p_encode   =  (kxdrproc_t) nfs_xdr_##argtype,			\
>> +	.p_encode   =  (kxdrproc_t) nfs2_xdr_enc_##argtype,		\
>> 	.p_decode   =  (kxdrproc_t) nfs_xdr_##restype,			\
>> 	.p_arglen   =  NFS_##argtype##_sz,				\
>> 	.p_replen   =  NFS_##restype##_sz,				\
>> 
> 
> Otherwise, the code looks fine...

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

[PATCH 05/11] NFS: Remove old NFSv2 encoder functions

[Chuck Lever]

Clean up:  Remove unused encoder helper functions, and move and update
remaining ones.  Removed unused argument encoder functions.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs2xdr.c |  249 +-----------------------------------------------------
 1 files changed, 4 insertions(+), 245 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 81c4d87..cd9e2d8 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -82,13 +82,6 @@ static void prepare_reply_buffer(struct rpc_rqst *req, struct page **pages,
  * Common NFS XDR functions as inlines
  */
 static inline __be32 *
-xdr_encode_fhandle(__be32 *p, const struct nfs_fh *fhandle)
-{
-	memcpy(p, fhandle->data, NFS2_FHSIZE);
-	return p + XDR_QUADLEN(NFS2_FHSIZE);
-}
-
-static inline __be32 *
 xdr_decode_fhandle(__be32 *p, struct nfs_fh *fhandle)
 {
 	/* NFSv2 handles have a fixed length */
@@ -160,36 +153,6 @@ xdr_decode_fattr(__be32 *p, struct nfs_fattr *fattr)
 	return p;
 }
 
-static inline __be32 *
-xdr_encode_sattr(__be32 *p, struct iattr *attr)
-{
-	const __be32 not_set = __constant_htonl(0xFFFFFFFF);
-
-	*p++ = (attr->ia_valid & ATTR_MODE) ? htonl(attr->ia_mode) : not_set;
-	*p++ = (attr->ia_valid & ATTR_UID) ? htonl(attr->ia_uid) : not_set;
-	*p++ = (attr->ia_valid & ATTR_GID) ? htonl(attr->ia_gid) : not_set;
-	*p++ = (attr->ia_valid & ATTR_SIZE) ? htonl(attr->ia_size) : not_set;
-
-	if (attr->ia_valid & ATTR_ATIME_SET) {
-		p = xdr_encode_time(p, &attr->ia_atime);
-	} else if (attr->ia_valid & ATTR_ATIME) {
-		p = xdr_encode_current_server_time(p, &attr->ia_atime);
-	} else {
-		*p++ = not_set;
-		*p++ = not_set;
-	}
-
-	if (attr->ia_valid & ATTR_MTIME_SET) {
-		p = xdr_encode_time(p, &attr->ia_mtime);
-	} else if (attr->ia_valid & ATTR_MTIME) {
-		p = xdr_encode_current_server_time(p, &attr->ia_mtime);
-	} else {
-		*p++ = not_set;	
-		*p++ = not_set;
-	}
-  	return p;
-}
-
 /*
  * Encode/decode NFSv2 basic data types
  *
@@ -325,19 +288,11 @@ static void encode_diropargs(struct xdr_stream *xdr, const struct nfs_fh *fh,
 
 
 /*
- * NFS encode functions
- */
-/*
- * Encode file handle argument
- * GETATTR, READLINK, STATFS
+ * NFSv2 XDR encode functions
+ *
+ * NFSv2 argument types are defined in section 2.2 of RFC 1094:
+ * "NFS: Network File System Protocol Specification".
  */
-static int
-nfs_xdr_fhandle(struct rpc_rqst *req, __be32 *p, struct nfs_fh *fh)
-{
-	p = xdr_encode_fhandle(p, fh);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
 
 static int nfs2_xdr_enc_fhandle(struct rpc_rqst *req, __be32 *p,
 				const struct nfs_fh *fh)
@@ -350,18 +305,6 @@ static int nfs2_xdr_enc_fhandle(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Encode SETATTR arguments
- */
-static int
-nfs_xdr_sattrargs(struct rpc_rqst *req, __be32 *p, struct nfs_sattrargs *args)
-{
-	p = xdr_encode_fhandle(p, args->fh);
-	p = xdr_encode_sattr(p, args->sattr);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-/*
  * 2.2.3.  sattrargs
  *
  *	struct sattrargs {
@@ -380,19 +323,6 @@ static int nfs2_xdr_enc_sattrargs(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
-/*
- * Encode directory ops argument
- * LOOKUP, RMDIR
- */
-static int
-nfs_xdr_diropargs(struct rpc_rqst *req, __be32 *p, struct nfs_diropargs *args)
-{
-	p = xdr_encode_fhandle(p, args->fh);
-	p = xdr_encode_array(p, args->name, args->len);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
 static int nfs2_xdr_enc_diropargs(struct rpc_rqst *req, __be32 *p,
 				  const struct nfs_diropargs *args)
 {
@@ -403,18 +333,6 @@ static int nfs2_xdr_enc_diropargs(struct rpc_rqst *req, __be32 *p,
 	return 0;
 }
 
-/*
- * Encode REMOVE argument
- */
-static int
-nfs_xdr_removeargs(struct rpc_rqst *req, __be32 *p, const struct nfs_removeargs *args)
-{
-	p = xdr_encode_fhandle(p, args->fh);
-	p = xdr_encode_array(p, args->name.name, args->name.len);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
 static int nfs2_xdr_enc_readlinkargs(struct rpc_rqst *req, __be32 *p,
 				     const struct nfs_readlinkargs *args)
 {
@@ -428,33 +346,6 @@ static int nfs2_xdr_enc_readlinkargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Arguments to a READ call. Since we read data directly into the page
- * cache, we also set up the reply iovec here so that iov[1] points
- * exactly to the page we want to fetch.
- */
-static int
-nfs_xdr_readargs(struct rpc_rqst *req, __be32 *p, struct nfs_readargs *args)
-{
-	struct rpc_auth	*auth = req->rq_cred->cr_auth;
-	unsigned int replen;
-	u32 offset = (u32)args->offset;
-	u32 count = args->count;
-
-	p = xdr_encode_fhandle(p, args->fh);
-	*p++ = htonl(offset);
-	*p++ = htonl(count);
-	*p++ = htonl(count);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-
-	/* Inline the page array */
-	replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS_readres_sz) << 2;
-	xdr_inline_pages(&req->rq_rcv_buf, replen,
-			 args->pages, args->pgbase, count);
-	req->rq_rcv_buf.flags |= XDRBUF_READ;
-	return 0;
-}
-
-/*
  * 2.2.7.  readargs
  *
  *	struct readargs {
@@ -536,29 +427,6 @@ nfs_xdr_readres(struct rpc_rqst *req, __be32 *p, struct nfs_readres *res)
 
 
 /*
- * Write arguments. Splice the buffer to be written into the iovec.
- */
-static int
-nfs_xdr_writeargs(struct rpc_rqst *req, __be32 *p, struct nfs_writeargs *args)
-{
-	struct xdr_buf *sndbuf = &req->rq_snd_buf;
-	u32 offset = (u32)args->offset;
-	u32 count = args->count;
-
-	p = xdr_encode_fhandle(p, args->fh);
-	*p++ = htonl(offset);
-	*p++ = htonl(offset);
-	*p++ = htonl(count);
-	*p++ = htonl(count);
-	sndbuf->len = xdr_adjust_iovec(sndbuf->head, p);
-
-	/* Copy the page array */
-	xdr_encode_pages(sndbuf, args->pages, args->pgbase, count);
-	sndbuf->flags |= XDRBUF_WRITE;
-	return 0;
-}
-
-/*
  * 2.2.9.  writeargs
  *
  *	struct writeargs {
@@ -601,20 +469,6 @@ static int nfs2_xdr_enc_writeargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Encode create arguments
- * CREATE, MKDIR
- */
-static int
-nfs_xdr_createargs(struct rpc_rqst *req, __be32 *p, struct nfs_createargs *args)
-{
-	p = xdr_encode_fhandle(p, args->fh);
-	p = xdr_encode_array(p, args->name, args->len);
-	p = xdr_encode_sattr(p, args->sattr);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-/*
  * 2.2.10.  createargs
  *
  *	struct createargs {
@@ -644,20 +498,6 @@ static int nfs2_xdr_enc_removeargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Encode RENAME arguments
- */
-static int
-nfs_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
-{
-	p = xdr_encode_fhandle(p, args->fromfh);
-	p = xdr_encode_array(p, args->fromname, args->fromlen);
-	p = xdr_encode_fhandle(p, args->tofh);
-	p = xdr_encode_array(p, args->toname, args->tolen);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-/*
  * 2.2.12.  renameargs
  *
  *	struct renameargs {
@@ -677,19 +517,6 @@ static int nfs2_xdr_enc_renameargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Encode LINK arguments
- */
-static int
-nfs_xdr_linkargs(struct rpc_rqst *req, __be32 *p, struct nfs_linkargs *args)
-{
-	p = xdr_encode_fhandle(p, args->fromfh);
-	p = xdr_encode_fhandle(p, args->tofh);
-	p = xdr_encode_array(p, args->toname, args->tolen);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-/*
  * 2.2.13.  linkargs
  *
  *	struct linkargs {
@@ -709,35 +536,6 @@ static int nfs2_xdr_enc_linkargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Encode SYMLINK arguments
- */
-static int
-nfs_xdr_symlinkargs(struct rpc_rqst *req, __be32 *p, struct nfs_symlinkargs *args)
-{
-	struct xdr_buf *sndbuf = &req->rq_snd_buf;
-	size_t pad;
-
-	p = xdr_encode_fhandle(p, args->fromfh);
-	p = xdr_encode_array(p, args->fromname, args->fromlen);
-	*p++ = htonl(args->pathlen);
-	sndbuf->len = xdr_adjust_iovec(sndbuf->head, p);
-
-	xdr_encode_pages(sndbuf, args->pages, 0, args->pathlen);
-
-	/*
-	 * xdr_encode_pages may have added a few bytes to ensure the
-	 * pathname ends on a 4-byte boundary.  Start encoding the
-	 * attributes after the pad bytes.
-	 */
-	pad = sndbuf->tail->iov_len;
-	if (pad > 0)
-		p++;
-	p = xdr_encode_sattr(p, args->sattr);
-	sndbuf->len += xdr_adjust_iovec(sndbuf->tail, p) - pad;
-	return 0;
-}
-
-/*
  * 2.2.14.  symlinkargs
  *
  *	struct symlinkargs {
@@ -759,27 +557,6 @@ static int nfs2_xdr_enc_symlinkargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Encode arguments to readdir call
- */
-static int
-nfs_xdr_readdirargs(struct rpc_rqst *req, __be32 *p, struct nfs_readdirargs *args)
-{
-	struct rpc_auth	*auth = req->rq_cred->cr_auth;
-	unsigned int replen;
-	u32 count = args->count;
-
-	p = xdr_encode_fhandle(p, args->fh);
-	*p++ = htonl(args->cookie);
-	*p++ = htonl(count); /* see above */
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-
-	/* Inline the page array */
-	replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS_readdirres_sz) << 2;
-	xdr_inline_pages(&req->rq_rcv_buf, replen, args->pages, 0, count);
-	return 0;
-}
-
-/*
  * 2.2.17.  readdirargs
  *
  *	struct readdirargs {
@@ -974,24 +751,6 @@ nfs_xdr_diropres(struct rpc_rqst *req, __be32 *p, struct nfs_diropok *res)
 }
 
 /*
- * Encode READLINK args
- */
-static int
-nfs_xdr_readlinkargs(struct rpc_rqst *req, __be32 *p, struct nfs_readlinkargs *args)
-{
-	struct rpc_auth	*auth = req->rq_cred->cr_auth;
-	unsigned int replen;
-
-	p = xdr_encode_fhandle(p, args->fh);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-
-	/* Inline the page array */
-	replen = (RPC_REPHDRSIZE + auth->au_rslack + NFS_readlinkres_sz) << 2;
-	xdr_inline_pages(&req->rq_rcv_buf, replen, args->pages, args->pgbase, args->pglen);
-	return 0;
-}
-
-/*
  * Decode READLINK reply
  */
 static int

[PATCH 06/11] NFS: Move and update xdr_encode_foo() functions that we're keeping

[Chuck Lever]

Clean up.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs2xdr.c |   59 ++++++++++++++++++++++++++++++------------------------
 1 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index cd9e2d8..0a3f830 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -91,32 +91,6 @@ xdr_decode_fhandle(__be32 *p, struct nfs_fh *fhandle)
 }
 
 static inline __be32*
-xdr_encode_time(__be32 *p, const struct timespec *timep)
-{
-	*p++ = htonl(timep->tv_sec);
-	/* Convert nanoseconds into microseconds */
-	*p++ = htonl(timep->tv_nsec ? timep->tv_nsec / 1000 : 0);
-	return p;
-}
-
-static inline __be32*
-xdr_encode_current_server_time(__be32 *p, const struct timespec *timep)
-{
-	/*
-	 * Passing the invalid value useconds=1000000 is a
-	 * Sun convention for "set to current server time".
-	 * It's needed to make permissions checks for the
-	 * "touch" program across v2 mounts to Solaris and
-	 * Irix boxes work correctly. See description of
-	 * sattr in section 6.1 of "NFS Illustrated" by
-	 * Brent Callaghan, Addison-Wesley, ISBN 0-201-32750-5
-	 */
-	*p++ = htonl(timep->tv_sec);
-	*p++ = htonl(1000000);
-	return p;
-}
-
-static inline __be32*
 xdr_decode_time(__be32 *p, struct timespec *timep)
 {
 	timep->tv_sec = ntohl(*p++);
@@ -180,6 +154,39 @@ static void encode_fhandle(struct xdr_stream *xdr, const struct nfs_fh *fh)
 }
 
 /*
+ * 2.3.4.  timeval
+ *
+ *	struct timeval {
+ *		unsigned int seconds;
+ *		unsigned int useconds;
+ *	};
+ */
+static __be32 *xdr_encode_time(__be32 *p, const struct timespec *timep)
+{
+	*p++ = cpu_to_be32(timep->tv_sec);
+	if (timep->tv_nsec != 0)
+		*p++ = cpu_to_be32(timep->tv_nsec / NSEC_PER_USEC);
+	else
+		*p++ = cpu_to_be32(0);
+	return p;
+}
+
+/*
+ * Passing the invalid value useconds=1000000 is a Sun convention for
+ * "set to current server time".  It's needed to make permissions checks
+ * for the "touch" program across v2 mounts to Solaris and Irix servers
+ * work correctly.  See description of sattr in section 6.1 of "NFS
+ * Illustrated" by Brent Callaghan, Addison-Wesley, ISBN 0-201-32750-5.
+ */
+static __be32 *xdr_encode_current_server_time(__be32 *p,
+					      const struct timespec *timep)
+{
+	*p++ = cpu_to_be32(timep->tv_sec);
+	*p++ = cpu_to_be32(1000000);
+	return p;
+}
+
+/*
  * 2.3.6.  sattr
  *
  *	struct sattr {

[PATCH 07/11] NFS: Use the "nfs_stat" enum for nfs_stat_to_errno()'s argument

[Chuck Lever]

Clean up:  To distinguish more clearly between the on-the-wire NFSERR_
value and our local errno values, use the proper type for the argument
of nfs_stat_to_errno().

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/internal.h |    2 +-
 fs/nfs/nfs2xdr.c  |   16 +++++++++-------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index c65ee43..01da8d4 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -179,7 +179,7 @@ extern int __init nfs_init_directcache(void);
 extern void nfs_destroy_directcache(void);
 
 /* nfs2xdr.c */
-extern int nfs_stat_to_errno(int);
+extern int nfs_stat_to_errno(enum nfs_stat);
 extern struct rpc_procinfo nfs_procedures[];
 extern __be32 * nfs_decode_dirent(__be32 *, struct nfs_entry *, int);
 
diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 0a3f830..769e74e 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -870,20 +870,22 @@ static struct {
 	{ -1,			-EIO		}
 };
 
-/*
- * Convert an NFS error code to a local one.
- * This one is used jointly by NFSv2 and NFSv3.
+/**
+ * nfs_stat_to_errno - convert an NFS status code to a local errno
+ * @status: NFS status code to convert
+ *
+ * Returns a local errno value, or -EIO if the NFS status code is
+ * not recognized.  This function is used jointly by NFSv2 and NFSv3.
  */
-int
-nfs_stat_to_errno(int stat)
+int nfs_stat_to_errno(enum nfs_stat status)
 {
 	int i;
 
 	for (i = 0; nfs_errtbl[i].stat != -1; i++) {
-		if (nfs_errtbl[i].stat == stat)
+		if (nfs_errtbl[i].stat == (int)status)
 			return nfs_errtbl[i].errno;
 	}
-	dprintk("nfs_stat_to_errno: bad nfs status return value: %d\n", stat);
+	dprintk("NFS: Unrecognized nfs status value: %u\n", status);
 	return nfs_errtbl[i].errno;
 }
 

[PATCH 08/11] NFS: Introduce new-style XDR decoding functions for NFSv2

[Chuck Lever]

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/internal.h |    2 
 fs/nfs/nfs2xdr.c  |  562 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/proc.c     |    2 
 3 files changed, 563 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 01da8d4..6246d40 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -181,7 +181,7 @@ extern void nfs_destroy_directcache(void);
 /* nfs2xdr.c */
 extern int nfs_stat_to_errno(enum nfs_stat);
 extern struct rpc_procinfo nfs_procedures[];
-extern __be32 * nfs_decode_dirent(__be32 *, struct nfs_entry *, int);
+extern __be32 *nfs2_decode_dirent(__be32 *, struct nfs_entry *, int);
 
 /* nfs3xdr.c */
 extern struct rpc_procinfo nfs3_procedures[];
diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 769e74e..8ba3f34 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -77,6 +77,16 @@ static void prepare_reply_buffer(struct rpc_rqst *req, struct page **pages,
 	xdr_inline_pages(&req->rq_rcv_buf, replen << 2, pages, base, len);
 }
 
+/*
+ * Handle decode buffer overflows out-of-line.
+ */
+static void print_overflow_msg(const char *func, const struct xdr_stream *xdr)
+{
+	dprintk("NFS: %s prematurely hit the end of our receive buffer. "
+		"Remaining buffer length is %tu words.\n",
+		func, xdr->end - xdr->p);
+}
+
 
 /*
  * Common NFS XDR functions as inlines
@@ -139,6 +149,74 @@ xdr_decode_fattr(__be32 *p, struct nfs_fattr *fattr)
  */
 
 /*
+ *	typedef opaque	nfsdata<>;
+ */
+static int decode_nfsdata(struct xdr_stream *xdr, struct nfs_readres *result)
+{
+	u32 recvd, count;
+	size_t hdrlen;
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	count = be32_to_cpup(p);
+	hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base;
+	recvd = xdr->buf->len - hdrlen;
+	if (unlikely(count > recvd))
+		goto out_cheating;
+out:
+	xdr_read_pages(xdr, count);
+	result->eof = 0;	/* NFSv2 does not pass EOF flag on the wire. */
+	result->count = count;
+	return count;
+out_cheating:
+	dprintk("NFS: server cheating in read result: "
+		"count %u > recvd %u\n", count, recvd);
+	count = recvd;
+	goto out;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
+ *	enum stat {
+ *		NFS_OK = 0,
+ *		NFSERR_PERM = 1,
+ *		NFSERR_NOENT = 2,
+ *		NFSERR_IO = 5,
+ *		NFSERR_NXIO = 6,
+ *		NFSERR_ACCES = 13,
+ *		NFSERR_EXIST = 17,
+ *		NFSERR_NODEV = 19,
+ *		NFSERR_NOTDIR = 20,
+ *		NFSERR_ISDIR = 21,
+ *		NFSERR_FBIG = 27,
+ *		NFSERR_NOSPC = 28,
+ *		NFSERR_ROFS = 30,
+ *		NFSERR_NAMETOOLONG = 63,
+ *		NFSERR_NOTEMPTY = 66,
+ *		NFSERR_DQUOT = 69,
+ *		NFSERR_STALE = 70,
+ *		NFSERR_WFLUSH = 99
+ *	};
+ */
+static int decode_stat(struct xdr_stream *xdr, enum nfs_stat *status)
+{
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	*status = be32_to_cpup(p);
+	return 0;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
  * 2.3.3.  fhandle
  *
  *	typedef opaque fhandle[FHSIZE];
@@ -153,6 +231,21 @@ static void encode_fhandle(struct xdr_stream *xdr, const struct nfs_fh *fh)
 	memcpy(p, fh->data, NFS2_FHSIZE);
 }
 
+static int decode_fhandle(struct xdr_stream *xdr, struct nfs_fh *fh)
+{
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, NFS2_FHSIZE);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	fh->size = NFS2_FHSIZE;
+	memcpy(fh->data, p, NFS2_FHSIZE);
+	return 0;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
 /*
  * 2.3.4.  timeval
  *
@@ -187,6 +280,41 @@ static __be32 *xdr_encode_current_server_time(__be32 *p,
 }
 
 /*
+ * 2.3.5.  fattr
+ *
+ *	struct fattr {
+ *		ftype		type;
+ *		unsigned int	mode;
+ *		unsigned int	nlink;
+ *		unsigned int	uid;
+ *		unsigned int	gid;
+ *		unsigned int	size;
+ *		unsigned int	blocksize;
+ *		unsigned int	rdev;
+ *		unsigned int	blocks;
+ *		unsigned int	fsid;
+ *		unsigned int	fileid;
+ *		timeval		atime;
+ *		timeval		mtime;
+ *		timeval		ctime;
+ *	};
+ *
+ */
+static int decode_fattr(struct xdr_stream *xdr, struct nfs_fattr *fattr)
+{
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, NFS_fattr_sz << 2);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	xdr_decode_fattr(p, fattr);
+	return 0;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
  * 2.3.6.  sattr
  *
  *	struct sattr {
@@ -278,6 +406,65 @@ static void encode_path(struct xdr_stream *xdr, struct page **pages, u32 length)
 	xdr_write_pages(xdr, pages, 0, length);
 }
 
+static int decode_path(struct xdr_stream *xdr)
+{
+	u32 length, recvd;
+	size_t hdrlen;
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	length = be32_to_cpup(p);
+	if (unlikely(length >= xdr->buf->page_len || length > NFS_MAXPATHLEN))
+		goto out_size;
+	hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base;
+	recvd = xdr->buf->len - hdrlen;
+	if (unlikely(length > recvd))
+		goto out_cheating;
+
+	xdr_read_pages(xdr, length);
+	xdr_terminate_string(xdr->buf, length);
+	return 0;
+out_size:
+	dprintk("NFS: returned pathname too long: %u\n", length);
+	return -ENAMETOOLONG;
+out_cheating:
+	dprintk("NFS: server cheating in pathname result: "
+		"length %u > received %u\n", length, recvd);
+	return -EIO;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
+ * 2.3.9.  attrstat
+ *
+ *	union attrstat switch (stat status) {
+ *	case NFS_OK:
+ *		fattr attributes;
+ *	default:
+ *		void;
+ *	};
+ */
+static int decode_attrstat(struct xdr_stream *xdr, struct nfs_fattr *result)
+{
+	enum nfs_stat status;
+	int error;
+
+	error = decode_stat(xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+	error = decode_fattr(xdr, result);
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
 /*
  * 2.3.10.  diropargs
  *
@@ -293,6 +480,48 @@ static void encode_diropargs(struct xdr_stream *xdr, const struct nfs_fh *fh,
 	encode_filename(xdr, name, length);
 }
 
+/*
+ * 2.3.11.  diropres
+ *
+ *	union diropres switch (stat status) {
+ *	case NFS_OK:
+ *		struct {
+ *			fhandle file;
+ *			fattr   attributes;
+ *		} diropok;
+ *	default:
+ *		void;
+ *	};
+ */
+static int decode_diropok(struct xdr_stream *xdr, struct nfs_diropok *result)
+{
+	int error;
+
+	error = decode_fhandle(xdr, result->fh);
+	if (unlikely(error))
+		goto out;
+	error = decode_fattr(xdr, result->fattr);
+out:
+	return error;
+}
+
+static int decode_diropres(struct xdr_stream *xdr, struct nfs_diropok *result)
+{
+	enum nfs_stat status;
+	int error;
+
+	error = decode_stat(xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+	error = decode_diropok(xdr, result);
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
 
 /*
  * NFSv2 XDR encode functions
@@ -726,6 +955,25 @@ nfs_xdr_stat(struct rpc_rqst *req, __be32 *p, void *dummy)
 	return status;
 }
 
+static int nfs2_xdr_dec_stat(struct rpc_rqst *req, __be32 *p,
+			     void *__unused)
+{
+	struct xdr_stream xdr;
+	enum nfs_stat status;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_stat(&xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
 /*
  * Decode attrstat reply
  * GETATTR, SETATTR, WRITE
@@ -741,6 +989,15 @@ nfs_xdr_attrstat(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
 	return 0;
 }
 
+static int nfs2_xdr_dec_attrstat(struct rpc_rqst *req, __be32 *p,
+				 struct nfs_fattr *result)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	return decode_attrstat(&xdr, result);
+}
+
 /*
  * Decode diropres reply
  * LOOKUP, CREATE, MKDIR
@@ -757,6 +1014,15 @@ nfs_xdr_diropres(struct rpc_rqst *req, __be32 *p, struct nfs_diropok *res)
 	return 0;
 }
 
+static int nfs2_xdr_dec_diropres(struct rpc_rqst *req, __be32 *p,
+				 struct nfs_diropok *result)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	return decode_diropres(&xdr, result);
+}
+
 /*
  * Decode READLINK reply
  */
@@ -798,6 +1064,70 @@ nfs_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, void *dummy)
 }
 
 /*
+ * 2.2.6.  readlinkres
+ *
+ *	union readlinkres switch (stat status) {
+ *	case NFS_OK:
+ *		path data;
+ *	default:
+ *		void;
+ *	};
+ */
+static int nfs2_xdr_dec_readlinkres(struct rpc_rqst *req, __be32 *p,
+				    void *__unused)
+{
+	struct xdr_stream xdr;
+	enum nfs_stat status;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_stat(&xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+	error = decode_path(&xdr);
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
+/*
+ * 2.2.7.  readres
+ *
+ *	union readres switch (stat status) {
+ *	case NFS_OK:
+ *		fattr attributes;
+ *		nfsdata data;
+ *	default:
+ *		void;
+ *	};
+ */
+static int nfs2_xdr_dec_readres(struct rpc_rqst *req, __be32 *p,
+				struct nfs_readres *result)
+{
+	struct xdr_stream xdr;
+	enum nfs_stat status;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_stat(&xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+	error = decode_fattr(&xdr, result->fattr);
+	if (unlikely(error))
+		goto out;
+	error = decode_nfsdata(&xdr, result);
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
+/*
  * Decode WRITE reply
  */
 static int
@@ -807,6 +1137,181 @@ nfs_xdr_writeres(struct rpc_rqst *req, __be32 *p, struct nfs_writeres *res)
 	return nfs_xdr_attrstat(req, p, res->fattr);
 }
 
+static int nfs2_xdr_dec_writeres(struct rpc_rqst *req, __be32 *p,
+				 struct nfs_writeres *result)
+{
+	struct xdr_stream xdr;
+
+	/* All NFSv2 writes are "file sync" writes */
+	result->verf->committed = NFS_FILE_SYNC;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	return decode_attrstat(&xdr, result->fattr);
+}
+
+/*
+ * 2.2.17.  readdirres
+ *
+ *	struct entry {
+ *		unsigned	fileid;
+ *		filename	name;
+ *		nfscookie	cookie;
+ *		entry		*nextentry;
+ *	};
+ *
+ *	union readdirres switch (stat status) {
+ *	case NFS_OK:
+ *		struct {
+ *			entry *entries;
+ *			bool eof;
+ *		} readdirok;
+ *	default:
+ *		void;
+ *	};
+ *
+ * The type (size) of nfscookie isn't defined in RFC 1094.
+ *
+ * The Linux implementation of readdir is limited to receiving not
+ * more than a single page of entries at a time.
+ *
+ * Here, the XDR buffer is checked for correct syntax.  The actual
+ * decoding is done by nfs_decode_entry() during subsequent
+ * nfs_readdir() calls on readdir data cached in the page cache.
+ */
+static int decode_readdirok(struct xdr_stream *xdr)
+{
+	__be32 *p, *end, *entry, *kaddr;
+	unsigned int nr, pglen, recvd;
+	struct page **page;
+	size_t hdrlen;
+	u32 count;
+
+	pglen = xdr->buf->page_len;
+	hdrlen = (u8 *)xdr->p - (u8 *)xdr->iov->iov_base;
+	recvd = xdr->buf->len - hdrlen;
+	if (pglen > recvd)
+		pglen = recvd;
+	xdr_read_pages(xdr, pglen);
+
+	page = xdr->buf->pages;
+	kaddr = p = kmap_atomic(*page, KM_USER0);
+	end = (__be32 *)((char *)p + pglen);
+	entry = p;
+
+	/* Make sure the packet actually has a value_follows and EOF entry */
+	if (unlikely((entry + 1) > end))
+		goto out_short_reply;
+
+	nr = 0;
+	for (; *p++ != xdr_zero; nr++) {
+		if (unlikely(p + 2 > end))
+			goto out_short_reply;
+		p++;				/* fileid */
+		count = be32_to_cpup(p++);
+		if (unlikely(count > NFS2_MAXNAMLEN))
+			goto out_nametoolong;
+		p += XDR_QUADLEN(count) + 1;	/* name plus cookie */
+		if (unlikely(p + 2 > end))
+			goto out_short_reply;
+		entry = p;
+	}
+
+	/*
+	 * Apparently some server sends responses that are a valid size,
+	 * but contain no entries, and have value_follows==0 and EOF==0.
+	 * For those, just set the EOF marker.
+	 */
+	if (unlikely(!nr && entry[1] == xdr_zero))
+		goto out_truncated;
+out:
+	kunmap_atomic(kaddr, KM_USER0);
+	return nr;
+out_short_reply:
+	/*
+	 * When we get a short reply there are 2 possibilities.  We can
+	 * return an error, or fix up the response to look like a valid
+	 * response and return what we have so far.  If there are no
+	 * entries and the reply was short, then return -EIO.  If there
+	 * are valid entries in the response, return them and pretend
+	 * that the call was successful, but incomplete.  The caller can
+	 * retry the readdir starting at the last cookie.
+	 */
+	dprintk("NFS: short readdir reply at entry %d\n", nr);
+	entry[0] = entry[1] = xdr_zero;
+	if (!nr)
+		nr = -EIO;
+	goto out;
+out_nametoolong:
+	dprintk("NFS: filename too long during readdir (len 0x%x)\n", count);
+	nr = -EIO;
+	goto out;
+out_truncated:
+	dprintk("NFS: readdir reply truncated\n");
+	entry[1] = xdr_one;
+	goto out;
+}
+
+static int nfs2_xdr_dec_readdirres(struct rpc_rqst *req, __be32 *p,
+				   void *__unused)
+{
+	struct xdr_stream xdr;
+	enum nfs_stat status;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_stat(&xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+	error = decode_readdirok(&xdr);
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
+/**
+ * nfs2_decode_dirent - Decode a single NFSv2 directory entry stored in
+ *                     the local page cache.
+ * @p: pointer to buffer where entry resides
+ * @entry: entry struct to fill out with data
+ * @plus: boolean indicating whether this should be a readdirplus entry
+ *
+ * Returns the position of the next item in the buffer, or an ERR_PTR.
+ *
+ * This function is not invoked during READDIR reply decoding, but
+ * rather whenever an application invokes the getdents(2) system call.
+ *
+ * 2.2.17.  entry
+ *
+ *	struct entry {
+ *		unsigned	fileid;
+ *		filename	name;
+ *		nfscookie	cookie;
+ *		entry		*nextentry;
+ *	};
+ */
+__be32 *nfs2_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)
+{
+	if (*p++ == xdr_zero) {
+		if (*p == xdr_zero)
+			return ERR_PTR(-EAGAIN);
+		entry->eof = 1;
+		return ERR_PTR(-EBADCOOKIE);
+	}
+
+	entry->ino = be32_to_cpup(p++);
+	p = xdr_decode_string_inplace(p, (char **)&entry->name,
+					&entry->len, NFS2_MAXNAMLEN);
+	entry->prev_cookie = entry->cookie;
+	entry->cookie = be32_to_cpup(p++);
+
+	/* Peek at the next entry */
+	entry->eof = (p[0] == xdr_zero) && (p[1] != xdr_zero);
+	return p;
+}
+
 /*
  * Decode STATFS reply
  */
@@ -827,6 +1332,61 @@ nfs_xdr_statfsres(struct rpc_rqst *req, __be32 *p, struct nfs2_fsstat *res)
 }
 
 /*
+ * 2.2.18.  statfsres
+ *
+ *	union statfsres (stat status) {
+ *	case NFS_OK:
+ *		struct {
+ *			unsigned tsize;
+ *			unsigned bsize;
+ *			unsigned blocks;
+ *			unsigned bfree;
+ *			unsigned bavail;
+ *		} info;
+ *	default:
+ *		void;
+ *	};
+ */
+static int decode_info(struct xdr_stream *xdr, struct nfs2_fsstat *result)
+{
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, NFS_info_sz << 2);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	result->tsize  = be32_to_cpup(p++);
+	result->bsize  = be32_to_cpup(p++);
+	result->blocks = be32_to_cpup(p++);
+	result->bfree  = be32_to_cpup(p++);
+	result->bavail = be32_to_cpup(p);
+	return 0;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+static int nfs2_xdr_dec_statfsres(struct rpc_rqst *req, __be32 *p,
+				  struct nfs2_fsstat *result)
+{
+	struct xdr_stream xdr;
+	enum nfs_stat status;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_stat(&xdr, &status);
+	if (unlikely(error))
+		goto out;
+	if (status != NFS_OK)
+		goto out_default;
+	error = decode_info(&xdr, result);
+out:
+	return error;
+out_default:
+	return nfs_stat_to_errno(status);
+}
+
+
+/*
  * We need to translate between nfs status return values and
  * the local errno values which may not be the same.
  */
@@ -893,7 +1453,7 @@ int nfs_stat_to_errno(enum nfs_stat status)
 [NFSPROC_##proc] = {							\
 	.p_proc	    =  NFSPROC_##proc,					\
 	.p_encode   =  (kxdrproc_t) nfs2_xdr_enc_##argtype,		\
-	.p_decode   =  (kxdrproc_t) nfs_xdr_##restype,			\
+	.p_decode   =  (kxdrproc_t) nfs2_xdr_dec_##restype,		\
 	.p_arglen   =  NFS_##argtype##_sz,				\
 	.p_replen   =  NFS_##restype##_sz,				\
 	.p_timer    =  timer,						\
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index 611bec2..12a0200 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -714,7 +714,7 @@ const struct nfs_rpc_ops nfs_v2_clientops = {
 	.statfs		= nfs_proc_statfs,
 	.fsinfo		= nfs_proc_fsinfo,
 	.pathconf	= nfs_proc_pathconf,
-	.decode_dirent	= nfs_decode_dirent,
+	.decode_dirent	= nfs2_decode_dirent,
 	.read_setup	= nfs_proc_read_setup,
 	.read_done	= nfs_read_done,
 	.write_setup	= nfs_proc_write_setup,

[PATCH 09/11] NFS: Replace old NFSv2 decoder functions with xdr_stream-based ones

[Chuck Lever]

Clean up.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs2xdr.c |  281 +-----------------------------------------------------
 1 files changed, 4 insertions(+), 277 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 8ba3f34..78c9d27 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -91,15 +91,6 @@ static void print_overflow_msg(const char *func, const struct xdr_stream *xdr)
 /*
  * Common NFS XDR functions as inlines
  */
-static inline __be32 *
-xdr_decode_fhandle(__be32 *p, struct nfs_fh *fhandle)
-{
-	/* NFSv2 handles have a fixed length */
-	fhandle->size = NFS2_FHSIZE;
-	memcpy(fhandle->data, p, NFS2_FHSIZE);
-	return p + XDR_QUADLEN(NFS2_FHSIZE);
-}
-
 static inline __be32*
 xdr_decode_time(__be32 *p, struct timespec *timep)
 {
@@ -621,48 +612,6 @@ static int nfs2_xdr_enc_readargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Decode READ reply
- */
-static int
-nfs_xdr_readres(struct rpc_rqst *req, __be32 *p, struct nfs_readres *res)
-{
-	struct kvec *iov = req->rq_rcv_buf.head;
-	size_t hdrlen;
-	u32 count, recvd;
-	int status;
-
-	if ((status = ntohl(*p++)))
-		return nfs_stat_to_errno(status);
-	p = xdr_decode_fattr(p, res->fattr);
-
-	count = ntohl(*p++);
-	res->eof = 0;
-	hdrlen = (u8 *) p - (u8 *) iov->iov_base;
-	if (iov->iov_len < hdrlen) {
-		dprintk("NFS: READ reply header overflowed:"
-				"length %Zu > %Zu\n", hdrlen, iov->iov_len);
-		return -errno_NFSERR_IO;
-	} else if (iov->iov_len != hdrlen) {
-		dprintk("NFS: READ header is short. iovec will be shifted.\n");
-		xdr_shift_buf(&req->rq_rcv_buf, iov->iov_len - hdrlen);
-	}
-
-	recvd = req->rq_rcv_buf.len - hdrlen;
-	if (count > recvd) {
-		dprintk("NFS: server cheating in read reply: "
-			"count %u > recvd %u\n", count, recvd);
-		count = recvd;
-	}
-
-	dprintk("RPC:      readres OK count %u\n", count);
-	if (count < res->count)
-		res->count = count;
-
-	return count;
-}
-
-
-/*
  * 2.2.9.  writeargs
  *
  *	struct writeargs {
@@ -827,133 +776,11 @@ static int nfs2_xdr_enc_readdirargs(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Decode the result of a readdir call.
- * We're not really decoding anymore, we just leave the buffer untouched
- * and only check that it is syntactically correct.
- * The real decoding happens in nfs_decode_entry below, called directly
- * from nfs_readdir for each entry.
- */
-static int
-nfs_xdr_readdirres(struct rpc_rqst *req, __be32 *p, void *dummy)
-{
-	struct xdr_buf *rcvbuf = &req->rq_rcv_buf;
-	struct kvec *iov = rcvbuf->head;
-	struct page **page;
-	size_t hdrlen;
-	unsigned int pglen, recvd;
-	u32 len;
-	int status, nr = 0;
-	__be32 *end, *entry, *kaddr;
-
-	if ((status = ntohl(*p++)))
-		return nfs_stat_to_errno(status);
-
-	hdrlen = (u8 *) p - (u8 *) iov->iov_base;
-	if (iov->iov_len < hdrlen) {
-		dprintk("NFS: READDIR reply header overflowed:"
-				"length %Zu > %Zu\n", hdrlen, iov->iov_len);
-		return -errno_NFSERR_IO;
-	} else if (iov->iov_len != hdrlen) {
-		dprintk("NFS: READDIR header is short. iovec will be shifted.\n");
-		xdr_shift_buf(rcvbuf, iov->iov_len - hdrlen);
-	}
-
-	pglen = rcvbuf->page_len;
-	recvd = rcvbuf->len - hdrlen;
-	if (pglen > recvd)
-		pglen = recvd;
-	page = rcvbuf->pages;
-	kaddr = p = kmap_atomic(*page, KM_USER0);
-	end = (__be32 *)((char *)p + pglen);
-	entry = p;
-
-	/* Make sure the packet actually has a value_follows and EOF entry */
-	if ((entry + 1) > end)
-		goto short_pkt;
-
-	for (; *p++; nr++) {
-		if (p + 2 > end)
-			goto short_pkt;
-		p++; /* fileid */
-		len = ntohl(*p++);
-		p += XDR_QUADLEN(len) + 1;	/* name plus cookie */
-		if (len > NFS2_MAXNAMLEN) {
-			dprintk("NFS: giant filename in readdir (len 0x%x)!\n",
-						len);
-			goto err_unmap;
-		}
-		if (p + 2 > end)
-			goto short_pkt;
-		entry = p;
-	}
-
-	/*
-	 * Apparently some server sends responses that are a valid size, but
-	 * contain no entries, and have value_follows==0 and EOF==0. For
-	 * those, just set the EOF marker.
-	 */
-	if (!nr && entry[1] == 0) {
-		dprintk("NFS: readdir reply truncated!\n");
-		entry[1] = 1;
-	}
- out:
-	kunmap_atomic(kaddr, KM_USER0);
-	return nr;
- short_pkt:
-	/*
-	 * When we get a short packet there are 2 possibilities. We can
-	 * return an error, or fix up the response to look like a valid
-	 * response and return what we have so far. If there are no
-	 * entries and the packet was short, then return -EIO. If there
-	 * are valid entries in the response, return them and pretend that
-	 * the call was successful, but incomplete. The caller can retry the
-	 * readdir starting at the last cookie.
-	 */
-	entry[0] = entry[1] = 0;
-	if (!nr)
-		nr = -errno_NFSERR_IO;
-	goto out;
-err_unmap:
-	nr = -errno_NFSERR_IO;
-	goto out;
-}
-
-__be32 *
-nfs_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)
-{
-	if (!*p++) {
-		if (!*p)
-			return ERR_PTR(-EAGAIN);
-		entry->eof = 1;
-		return ERR_PTR(-EBADCOOKIE);
-	}
-
-	entry->ino	  = ntohl(*p++);
-	entry->len	  = ntohl(*p++);
-	entry->name	  = (const char *) p;
-	p		 += XDR_QUADLEN(entry->len);
-	entry->prev_cookie	  = entry->cookie;
-	entry->cookie	  = ntohl(*p++);
-	entry->eof	  = !p[0] && p[1];
-
-	return p;
-}
-
-/*
- * NFS XDR decode functions
- */
-/*
- * Decode simple status reply
+ * NFSv2 XDR decode functions
+ *
+ * NFSv2 result types are defined in section 2.2 of RFC 1094:
+ * "NFS: Network File System Protocol Specification".
  */
-static int
-nfs_xdr_stat(struct rpc_rqst *req, __be32 *p, void *dummy)
-{
-	int	status;
-
-	if ((status = ntohl(*p++)) != 0)
-		status = nfs_stat_to_errno(status);
-	return status;
-}
 
 static int nfs2_xdr_dec_stat(struct rpc_rqst *req, __be32 *p,
 			     void *__unused)
@@ -974,21 +801,6 @@ out_default:
 	return nfs_stat_to_errno(status);
 }
 
-/*
- * Decode attrstat reply
- * GETATTR, SETATTR, WRITE
- */
-static int
-nfs_xdr_attrstat(struct rpc_rqst *req, __be32 *p, struct nfs_fattr *fattr)
-{
-	int	status;
-
-	if ((status = ntohl(*p++)))
-		return nfs_stat_to_errno(status);
-	xdr_decode_fattr(p, fattr);
-	return 0;
-}
-
 static int nfs2_xdr_dec_attrstat(struct rpc_rqst *req, __be32 *p,
 				 struct nfs_fattr *result)
 {
@@ -998,22 +810,6 @@ static int nfs2_xdr_dec_attrstat(struct rpc_rqst *req, __be32 *p,
 	return decode_attrstat(&xdr, result);
 }
 
-/*
- * Decode diropres reply
- * LOOKUP, CREATE, MKDIR
- */
-static int
-nfs_xdr_diropres(struct rpc_rqst *req, __be32 *p, struct nfs_diropok *res)
-{
-	int	status;
-
-	if ((status = ntohl(*p++)))
-		return nfs_stat_to_errno(status);
-	p = xdr_decode_fhandle(p, res->fh);
-	xdr_decode_fattr(p, res->fattr);
-	return 0;
-}
-
 static int nfs2_xdr_dec_diropres(struct rpc_rqst *req, __be32 *p,
 				 struct nfs_diropok *result)
 {
@@ -1024,46 +820,6 @@ static int nfs2_xdr_dec_diropres(struct rpc_rqst *req, __be32 *p,
 }
 
 /*
- * Decode READLINK reply
- */
-static int
-nfs_xdr_readlinkres(struct rpc_rqst *req, __be32 *p, void *dummy)
-{
-	struct xdr_buf *rcvbuf = &req->rq_rcv_buf;
-	struct kvec *iov = rcvbuf->head;
-	size_t hdrlen;
-	u32 len, recvd;
-	int	status;
-
-	if ((status = ntohl(*p++)))
-		return nfs_stat_to_errno(status);
-	/* Convert length of symlink */
-	len = ntohl(*p++);
-	if (len >= rcvbuf->page_len) {
-		dprintk("nfs: server returned giant symlink!\n");
-		return -ENAMETOOLONG;
-	}
-	hdrlen = (u8 *) p - (u8 *) iov->iov_base;
-	if (iov->iov_len < hdrlen) {
-		dprintk("NFS: READLINK reply header overflowed:"
-				"length %Zu > %Zu\n", hdrlen, iov->iov_len);
-		return -errno_NFSERR_IO;
-	} else if (iov->iov_len != hdrlen) {
-		dprintk("NFS: READLINK header is short. iovec will be shifted.\n");
-		xdr_shift_buf(rcvbuf, iov->iov_len - hdrlen);
-	}
-	recvd = req->rq_rcv_buf.len - hdrlen;
-	if (recvd < len) {
-		dprintk("NFS: server cheating in readlink reply: "
-				"count %u > recvd %u\n", len, recvd);
-		return -EIO;
-	}
-
-	xdr_terminate_string(rcvbuf, len);
-	return 0;
-}
-
-/*
  * 2.2.6.  readlinkres
  *
  *	union readlinkres switch (stat status) {
@@ -1127,16 +883,6 @@ out_default:
 	return nfs_stat_to_errno(status);
 }
 
-/*
- * Decode WRITE reply
- */
-static int
-nfs_xdr_writeres(struct rpc_rqst *req, __be32 *p, struct nfs_writeres *res)
-{
-	res->verf->committed = NFS_FILE_SYNC;
-	return nfs_xdr_attrstat(req, p, res->fattr);
-}
-
 static int nfs2_xdr_dec_writeres(struct rpc_rqst *req, __be32 *p,
 				 struct nfs_writeres *result)
 {
@@ -1313,25 +1059,6 @@ __be32 *nfs2_decode_dirent(__be32 *p, struct nfs_entry *entry, int plus)
 }
 
 /*
- * Decode STATFS reply
- */
-static int
-nfs_xdr_statfsres(struct rpc_rqst *req, __be32 *p, struct nfs2_fsstat *res)
-{
-	int	status;
-
-	if ((status = ntohl(*p++)))
-		return nfs_stat_to_errno(status);
-
-	res->tsize  = ntohl(*p++);
-	res->bsize  = ntohl(*p++);
-	res->blocks = ntohl(*p++);
-	res->bfree  = ntohl(*p++);
-	res->bavail = ntohl(*p++);
-	return 0;
-}
-
-/*
  * 2.2.18.  statfsres
  *
  *	union statfsres (stat status) {

[PATCH 10/11] NFS: Move and update xdr_decode_foo() functions that we're keeping

[Chuck Lever]

Clean up.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/nfs2xdr.c |   97 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index 78c9d27..dc3841d 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -89,46 +89,6 @@ static void print_overflow_msg(const char *func, const struct xdr_stream *xdr)
 
 
 /*
- * Common NFS XDR functions as inlines
- */
-static inline __be32*
-xdr_decode_time(__be32 *p, struct timespec *timep)
-{
-	timep->tv_sec = ntohl(*p++);
-	/* Convert microseconds into nanoseconds */
-	timep->tv_nsec = ntohl(*p++) * 1000;
-	return p;
-}
-
-static __be32 *
-xdr_decode_fattr(__be32 *p, struct nfs_fattr *fattr)
-{
-	u32 rdev, type;
-	type = ntohl(*p++);
-	fattr->mode = ntohl(*p++);
-	fattr->nlink = ntohl(*p++);
-	fattr->uid = ntohl(*p++);
-	fattr->gid = ntohl(*p++);
-	fattr->size = ntohl(*p++);
-	fattr->du.nfs2.blocksize = ntohl(*p++);
-	rdev = ntohl(*p++);
-	fattr->du.nfs2.blocks = ntohl(*p++);
-	fattr->fsid.major = ntohl(*p++);
-	fattr->fsid.minor = 0;
-	fattr->fileid = ntohl(*p++);
-	p = xdr_decode_time(p, &fattr->atime);
-	p = xdr_decode_time(p, &fattr->mtime);
-	p = xdr_decode_time(p, &fattr->ctime);
-	fattr->valid |= NFS_ATTR_FATTR_V2;
-	fattr->rdev = new_decode_dev(rdev);
-	if (type == NFCHR && rdev == NFS2_FIFO_DEV) {
-		fattr->mode = (fattr->mode & ~S_IFMT) | S_IFIFO;
-		fattr->rdev = 0;
-	}
-	return p;
-}
-
-/*
  * Encode/decode NFSv2 basic data types
  *
  * Basic NFSv2 data types are defined in section 2.3 of RFC 1094:
@@ -208,6 +168,27 @@ out_overflow:
 }
 
 /*
+ * 2.3.2.  ftype
+ *
+ *	enum ftype {
+ *		NFNON = 0,
+ *		NFREG = 1,
+ *		NFDIR = 2,
+ *		NFBLK = 3,
+ *		NFCHR = 4,
+ *		NFLNK = 5
+ *	};
+ *
+ */
+static __be32 *xdr_decode_ftype(__be32 *p, u32 *type)
+{
+	*type = be32_to_cpup(p++);
+	if (unlikely(*type > NF2FIFO))
+		*type = NFBAD;
+	return p;
+}
+
+/*
  * 2.3.3.  fhandle
  *
  *	typedef opaque fhandle[FHSIZE];
@@ -270,6 +251,13 @@ static __be32 *xdr_encode_current_server_time(__be32 *p,
 	return p;
 }
 
+static __be32 *xdr_decode_time(__be32 *p, struct timespec *timep)
+{
+	timep->tv_sec = be32_to_cpup(p++);
+	timep->tv_nsec = be32_to_cpup(p++) * NSEC_PER_USEC;
+	return p;
+}
+
 /*
  * 2.3.5.  fattr
  *
@@ -293,12 +281,39 @@ static __be32 *xdr_encode_current_server_time(__be32 *p,
  */
 static int decode_fattr(struct xdr_stream *xdr, struct nfs_fattr *fattr)
 {
+	u32 rdev, type;
 	__be32 *p;
 
 	p = xdr_inline_decode(xdr, NFS_fattr_sz << 2);
 	if (unlikely(p == NULL))
 		goto out_overflow;
-	xdr_decode_fattr(p, fattr);
+
+	fattr->valid |= NFS_ATTR_FATTR_V2;
+
+	p = xdr_decode_ftype(p, &type);
+
+	fattr->mode		= be32_to_cpup(p++);
+	fattr->nlink		= be32_to_cpup(p++);
+	fattr->uid		= be32_to_cpup(p++);
+	fattr->gid		= be32_to_cpup(p++);
+	fattr->size		= be32_to_cpup(p++);
+	fattr->du.nfs2.blocksize = be32_to_cpup(p++);
+
+	rdev			= be32_to_cpup(p++);
+	fattr->rdev		= new_decode_dev(rdev);
+	if (type == (u32)NFCHR && rdev == (u32)NFS2_FIFO_DEV) {
+		fattr->mode = (fattr->mode & ~S_IFMT) | S_IFIFO;
+		fattr->rdev = 0;
+	}
+
+	fattr->du.nfs2.blocks	= be32_to_cpup(p++);
+	fattr->fsid.major	= be32_to_cpup(p++);
+	fattr->fsid.minor	= 0;
+	fattr->fileid		= be32_to_cpup(p++);
+
+	p = xdr_decode_time(p, &fattr->atime);
+	p = xdr_decode_time(p, &fattr->mtime);
+	xdr_decode_time(p, &fattr->ctime);
 	return 0;
 out_overflow:
 	print_overflow_msg(__func__, xdr);

[PATCH 11/11] lockd: Introduce new-style XDR functions for NLMv3

[Chuck Lever]

We'd like to prevent local buffer overflows caused by malicious or
broken servers.  New xdr_stream style decoders can do that.

For efficiency, we also want to be able to pass xdr_streams from
call_encode() to all XDR encoding functions, rather than building
an xdr_stream in every XDR encoding function in the kernel.

To do all of this, rewrite the XDR encoding and decoding functions in
fs/lockd/xdr.c to use xdr_streams.  This makes them more or less
incompatible with server-side XDR helper functions, so break them out
into a separate source file.

SHARE-related functionality doesn't seem to be used, as those
functions are hiding behind a #define that isn't set anywhere that I
can find.  And, they've been in there forever (at least as far back as
the kernel's git history goes), yet remain unused.  Let's take the
opportunity to bin them.  It should be easy enough for someone to
introduce proper XDR functions if at some point SHARE-related NLM
functionality is desired.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/lockd/Makefile  |    4 
 fs/lockd/clntxdr.c |  657 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/lockd/xdr.c     |  258 --------------------
 3 files changed, 659 insertions(+), 260 deletions(-)
 create mode 100644 fs/lockd/clntxdr.c

diff --git a/fs/lockd/Makefile b/fs/lockd/Makefile
index cd123e0..e2e973f 100644
--- a/fs/lockd/Makefile
+++ b/fs/lockd/Makefile
@@ -7,7 +7,7 @@ EXTRA_CFLAGS = -W -Wall -Wextra -Wsign-compare \
 
 obj-$(CONFIG_LOCKD) += lockd.o
 
-lockd-objs-y := clntlock.o clntproc.o host.o svc.o svclock.o svcshare.o \
-	        svcproc.o svcsubs.o mon.o xdr.o grace.o
+lockd-objs-y := clntlock.o clntproc.o clntxdr.o host.o svc.o svclock.o \
+	        svcshare.o svcproc.o svcsubs.o mon.o xdr.o grace.o
 lockd-objs-$(CONFIG_LOCKD_V4) += xdr4.o svc4proc.o
 lockd-objs		      := $(lockd-objs-y)
diff --git a/fs/lockd/clntxdr.c b/fs/lockd/clntxdr.c
new file mode 100644
index 0000000..23b30d1
--- /dev/null
+++ b/fs/lockd/clntxdr.c
@@ -0,0 +1,657 @@
+/*
+ * linux/fs/lockd/clntxdr.c
+ *
+ * XDR functions to encode/decode NLM version 3 RPC arguments and results.
+ * NLM version 3 is backwards compatible with NLM versions 1 and 2.
+ *
+ * NLM client-side only.
+ *
+ * Copyright (C) 2010, Oracle.  All rights reserved.
+ */
+
+#include <linux/types.h>
+#include <linux/sunrpc/xdr.h>
+#include <linux/sunrpc/clnt.h>
+#include <linux/sunrpc/stats.h>
+#include <linux/lockd/lockd.h>
+
+#define NLMDBG_FACILITY		NLMDBG_XDR
+
+#if (NLMCLNT_OHSIZE > XDR_MAX_NETOBJ)
+#  error "NLM host name cannot be larger than XDR_MAX_NETOBJ!"
+#endif
+
+/*
+ * Declare the space requirements for NLM arguments and replies as
+ * number of 32bit-words
+ */
+#define NLM_cookie_sz		(1+(NLM_MAXCOOKIELEN>>2))
+#define NLM_caller_sz		(1+(NLMCLNT_OHSIZE>>2))
+#define NLM_owner_sz		(1+(NLMCLNT_OHSIZE>>2))
+#define NLM_fhandle_sz		(1+(NFS2_FHSIZE>>2))
+#define NLM_lock_sz		(3+NLM_caller_sz+NLM_owner_sz+NLM_fhandle_sz)
+#define NLM_holder_sz		(4+NLM_owner_sz)
+
+#define NLM_testargs_sz		(NLM_cookie_sz+1+NLM_lock_sz)
+#define NLM_lockargs_sz		(NLM_cookie_sz+4+NLM_lock_sz)
+#define NLM_cancargs_sz		(NLM_cookie_sz+2+NLM_lock_sz)
+#define NLM_unlockargs_sz	(NLM_cookie_sz+NLM_lock_sz)
+
+#define NLM_testres_sz		(NLM_cookie_sz+1+NLM_holder_sz)
+#define NLM_res_sz		(NLM_cookie_sz+1)
+#define NLM_norep_sz		(0)
+
+
+static s32 loff_t_to_s32(loff_t offset)
+{
+	s32 res;
+
+	if (offset >= NLM_OFFSET_MAX)
+		res = NLM_OFFSET_MAX;
+	else if (offset <= -NLM_OFFSET_MAX)
+		res = -NLM_OFFSET_MAX;
+	else
+		res = offset;
+	return res;
+}
+
+static void nlm_compute_offsets(const struct nlm_lock *lock,
+				u32 *l_offset, u32 *l_len)
+{
+	const struct file_lock *fl = &lock->fl;
+
+	BUG_ON(unlikely(fl->fl_start > NLM_OFFSET_MAX));
+	BUG_ON(unlikely(fl->fl_end > NLM_OFFSET_MAX &&
+				fl->fl_end != OFFSET_MAX));
+
+	*l_offset = loff_t_to_s32(fl->fl_start);
+	if (fl->fl_end == OFFSET_MAX)
+		*l_len = 0;
+	else
+		*l_len = loff_t_to_s32(fl->fl_end - fl->fl_start + 1);
+}
+
+/*
+ * Handle decode buffer overflows out-of-line.
+ */
+static void print_overflow_msg(const char *func, const struct xdr_stream *xdr)
+{
+	dprintk("lockd: %s prematurely hit the end of our receive buffer. "
+		"Remaining buffer length is %tu words.\n",
+		func, xdr->end - xdr->p);
+}
+
+
+/*
+ * Encode/decode NLMv3 basic data types
+ *
+ * Basic NLMv3 data types are not defined in an IETF standards
+ * document.  X/Open has a description of these data types that
+ * is useful.  See Chapter 10 of "Protocols for Interworking:
+ * XNFS, Version 3W".
+ *
+ * Not all basic data types have their own encoding and decoding
+ * functions.  For run-time efficiency, some data types are encoded
+ * or decoded inline.
+ */
+
+static void encode_bool(struct xdr_stream *xdr, const int value)
+{
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 4);
+	BUG_ON(unlikely(p == NULL));
+	*p = value ? xdr_one : xdr_zero;
+}
+
+static void encode_int32(struct xdr_stream *xdr, const s32 value)
+{
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 4);
+	BUG_ON(unlikely(p == NULL));
+	*p = cpu_to_be32(value);
+}
+
+/*
+ *	typedef opaque netobj<MAXNETOBJ_SZ>
+ */
+static void encode_netobj(struct xdr_stream *xdr,
+			  const u8 *data, const unsigned int length)
+{
+	__be32 *p;
+
+	BUG_ON(unlikely(length > XDR_MAX_NETOBJ));
+	p = xdr_reserve_space(xdr, 4 + length);
+	BUG_ON(unlikely(p == NULL));
+	xdr_encode_opaque(p, data, length);
+}
+
+static int decode_netobj(struct xdr_stream *xdr,
+			 struct xdr_netobj *obj)
+{
+	u32 length;
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	length = be32_to_cpup(p++);
+	if (unlikely(length > XDR_MAX_NETOBJ))
+		goto out_size;
+	obj->len = length;
+	obj->data = (u8 *)p;
+	return 0;
+out_size:
+	dprintk("NFS: returned netobj was too long: %u\n", length);
+	return -EIO;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
+ *	netobj cookie;
+ */
+static void encode_cookie(struct xdr_stream *xdr,
+			  const struct nlm_cookie *cookie)
+{
+	BUG_ON(unlikely(cookie->len > NLM_MAXCOOKIELEN));
+	encode_netobj(xdr, (u8 *)&cookie->data, cookie->len);
+}
+
+static int decode_cookie(struct xdr_stream *xdr,
+			 struct nlm_cookie *cookie)
+{
+	u32 length;
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	length = be32_to_cpup(p++);
+	/* apparently HPUX can return empty cookies */
+	if (length == 0)
+		goto out_hpux;
+	if (length > NLM_MAXCOOKIELEN)
+		goto out_size;
+	p = xdr_inline_decode(xdr, length);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	cookie->len = length;
+	memcpy(cookie->data, p, length);
+	return 0;
+out_hpux:
+	cookie->len = 4;
+	memset(cookie->data, 0, 4);
+	return 0;
+out_size:
+	dprintk("NFS: returned cookie was too long: %u\n", length);
+	return -EIO;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
+ *	netobj fh;
+ */
+static void encode_fh(struct xdr_stream *xdr, const struct nfs_fh *fh)
+{
+	BUG_ON(unlikely(fh->size != NFS2_FHSIZE));
+	encode_netobj(xdr, (u8 *)&fh->data, NFS2_FHSIZE);
+}
+
+/*
+ *	enum nlm_stats {
+ *		LCK_GRANTED = 0,
+ *		LCK_DENIED = 1,
+ *		LCK_DENIED_NOLOCKS = 2,
+ *		LCK_BLOCKED = 3,
+ *		LCK_DENIED_GRACE_PERIOD = 4
+ *	};
+ *
+ *
+ *	struct nlm_stat {
+ *		nlm_stats stat;
+ *	};
+ *
+ */
+
+enum nlm_stats {
+	LCK_GRANTED = 0,
+	LCK_DENIED = 1,
+	LCK_DENIED_NOLOCKS = 2,
+	LCK_BLOCKED = 3,
+	LCK_DENIED_GRACE_PERIOD = 4
+};
+
+static void encode_nlm_stat(struct xdr_stream *xdr,
+			    const enum nlm_stats stat)
+{
+	__be32 *p;
+
+	BUG_ON(unlikely(stat > LCK_DENIED_GRACE_PERIOD));
+	p = xdr_reserve_space(xdr, 4);
+	BUG_ON(unlikely(p == NULL));
+	*p = cpu_to_be32(stat);
+}
+
+static int decode_nlm_stat(struct xdr_stream *xdr,
+			   enum nlm_stats *stat)
+{
+	u32 status;
+	__be32 *p;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	status = be32_to_cpup(p);
+	if (unlikely(status > LCK_DENIED_GRACE_PERIOD))
+		goto out_enum;
+	*stat = (enum nlm_stats)status;
+	return 0;
+out_enum:
+	dprintk("%s: server returned invalid nlm_stats value: %u\n",
+		__func__, status);
+	return -EIO;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
+ *	struct nlm_holder {
+ *		bool exclusive;
+ *		int uppid;
+ *		netobj oh;
+ *		unsigned l_offset;
+ *		unsigned l_len;
+ *	};
+ */
+static void encode_nlm_holder(struct xdr_stream *xdr,
+			      const struct nlm_res *result)
+{
+	const struct nlm_lock *lock = &result->lock;
+	u32 l_offset, l_len;
+	__be32 *p;
+
+	encode_bool(xdr, lock->fl.fl_type == F_RDLCK);
+	encode_int32(xdr, lock->svid);
+	encode_netobj(xdr, lock->oh.data, lock->oh.len);
+
+	p = xdr_reserve_space(xdr, 4 + 4);
+	BUG_ON(unlikely(p == NULL));
+	nlm_compute_offsets(lock, &l_offset, &l_len);
+	*p++ = cpu_to_be32(l_offset);
+	*p   = cpu_to_be32(l_len);
+}
+
+static int decode_nlm_holder(struct xdr_stream *xdr, struct nlm_res *result)
+{
+	struct nlm_lock *lock = &result->lock;
+	struct file_lock *fl = &lock->fl;
+	u32 exclusive, l_offset, l_len;
+	int error;
+	__be32 *p;
+	s32 end;
+
+	memset(lock, 0, sizeof(*lock));
+	locks_init_lock(fl);
+
+	p = xdr_inline_decode(xdr, 4 + 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+	exclusive = be32_to_cpup(p++);
+	lock->svid = be32_to_cpup(p);
+	fl->fl_pid = (pid_t)lock->svid;
+
+	error = decode_netobj(xdr, &lock->oh);
+	if (unlikely(error))
+		goto out;
+
+	p = xdr_inline_decode(xdr, 4 + 4);
+	if (unlikely(p == NULL))
+		goto out_overflow;
+
+	fl->fl_flags = FL_POSIX;
+	fl->fl_type  = exclusive != 0 ? F_WRLCK : F_RDLCK;
+	l_offset = be32_to_cpup(p++);
+	l_len = be32_to_cpup(p);
+	end = l_offset + l_len - 1;
+
+	fl->fl_start = (loff_t)l_offset;
+	if (l_len == 0 || end < 0)
+		fl->fl_end = OFFSET_MAX;
+	else
+		fl->fl_end = (loff_t)end;
+	error = 0;
+out:
+	return error;
+out_overflow:
+	print_overflow_msg(__func__, xdr);
+	return -EIO;
+}
+
+/*
+ *	string caller_name<LM_MAXSTRLEN>;
+ */
+static void encode_caller_name(struct xdr_stream *xdr, const char *name)
+{
+	/* NB: client-side does not set lock->len */
+	u32 length = strlen(name);
+	__be32 *p;
+
+	BUG_ON(unlikely(length > NLM_MAXSTRLEN));
+	p = xdr_reserve_space(xdr, 4 + length);
+	BUG_ON(unlikely(p == NULL));
+	xdr_encode_opaque(p, name, length);
+}
+
+/*
+ *	struct nlm_lock {
+ *		string caller_name<LM_MAXSTRLEN>;
+ *		netobj fh;
+ *		netobj oh;
+ *		int uppid;
+ *		unsigned l_offset;
+ *		unsigned l_len;
+ *	};
+ */
+static void encode_nlm_lock(struct xdr_stream *xdr,
+			    const struct nlm_lock *lock)
+{
+	u32 l_offset, l_len;
+	__be32 *p;
+
+	encode_caller_name(xdr, lock->caller);
+	encode_fh(xdr, &lock->fh);
+	encode_netobj(xdr, lock->oh.data, lock->oh.len);
+
+	p = xdr_reserve_space(xdr, 4 + 4 + 4);
+	BUG_ON(unlikely(p == NULL));
+	*p++ = cpu_to_be32(lock->svid);
+
+	nlm_compute_offsets(lock, &l_offset, &l_len);
+	*p++ = cpu_to_be32(l_offset);
+	*p   = cpu_to_be32(l_len);
+}
+
+
+/*
+ * NLMv3 XDR encode functions
+ *
+ * NLMv3 argument types are defined in Chapter 10 of The Open Group's
+ * "Protocols for Interworking: XNFS, Version 3W".
+ */
+
+/*
+ *	struct nlm_testargs {
+ *		netobj cookie;
+ *		bool exclusive;
+ *		struct nlm_lock alock;
+ *	};
+ */
+static int nlm_xdr_enc_testargs(struct rpc_rqst *req, __be32 *p,
+				const struct nlm_args *args)
+{
+	const struct nlm_lock *lock = &args->lock;
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_cookie(&xdr, &args->cookie);
+	encode_bool(&xdr, lock->fl.fl_type == F_WRLCK);
+	encode_nlm_lock(&xdr, lock);
+	return 0;
+}
+
+/*
+ *	struct nlm_lockargs {
+ *		netobj cookie;
+ *		bool block;
+ *		bool exclusive;
+ *		struct nlm_lock alock;
+ *		bool reclaim;
+ *		int state;
+ *	};
+ */
+static int nlm_xdr_enc_lockargs(struct rpc_rqst *req, __be32 *p,
+				const struct nlm_args *args)
+{
+	const struct nlm_lock *lock = &args->lock;
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_cookie(&xdr, &args->cookie);
+	encode_bool(&xdr, args->block);
+	encode_bool(&xdr, lock->fl.fl_type == F_WRLCK);
+	encode_nlm_lock(&xdr, lock);
+	encode_bool(&xdr, args->reclaim);
+	encode_int32(&xdr, args->state);
+	return 0;
+}
+
+/*
+ *	struct nlm_cancargs {
+ *		netobj cookie;
+ *		bool block;
+ *		bool exclusive;
+ *		struct nlm_lock alock;
+ *	};
+ */
+static int nlm_xdr_enc_cancargs(struct rpc_rqst *req, __be32 *p,
+				const struct nlm_args *args)
+{
+	const struct nlm_lock *lock = &args->lock;
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_cookie(&xdr, &args->cookie);
+	encode_bool(&xdr, args->block);
+	encode_bool(&xdr, lock->fl.fl_type == F_WRLCK);
+	encode_nlm_lock(&xdr, lock);
+	return 0;
+}
+
+/*
+ *	struct nlm_unlockargs {
+ *		netobj cookie;
+ *		struct nlm_lock alock;
+ *	};
+ */
+static int nlm_xdr_enc_unlockargs(struct rpc_rqst *req, __be32 *p,
+				  const struct nlm_args *args)
+{
+	const struct nlm_lock *lock = &args->lock;
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_cookie(&xdr, &args->cookie);
+	encode_nlm_lock(&xdr, lock);
+	return 0;
+}
+
+/*
+ *	struct nlm_res {
+ *		netobj cookie;
+ *		nlm_stat stat;
+ *	};
+ */
+static int nlm_xdr_enc_res(struct rpc_rqst *req, __be32 *p,
+			   const struct nlm_res *result)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_cookie(&xdr, &result->cookie);
+	encode_nlm_stat(&xdr, result->status);
+	return 0;
+}
+
+/*
+ *	union nlm_testrply switch (nlm_stats stat) {
+ *	case LCK_DENIED:
+ *		struct nlm_holder holder;
+ *	default:
+ *		void;
+ *	};
+ *
+ *	struct nlm_testres {
+ *		netobj cookie;
+ *		nlm_testrply test_stat;
+ *	};
+ */
+static void encode_nlm_testrply(struct xdr_stream *xdr,
+				const struct nlm_res *result)
+{
+	if (result->status == LCK_DENIED)
+		encode_nlm_holder(xdr, result);
+}
+
+static int nlm_xdr_enc_testres(struct rpc_rqst *req, __be32 *p,
+			       const struct nlm_res *result)
+{
+	struct xdr_stream xdr;
+
+	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
+	encode_cookie(&xdr, &result->cookie);
+	encode_nlm_stat(&xdr, result->status);
+	encode_nlm_testrply(&xdr, result);
+	return 0;
+}
+
+
+/*
+ * NLMv3 XDR decode functions
+ *
+ * NLMv3 result types are defined in Chapter 10 of The Open Group's
+ * "Protocols for Interworking: XNFS, Version 3W".
+ */
+
+/*
+ *	union nlm_testrply switch (nlm_stats stat) {
+ *	case LCK_DENIED:
+ *		struct nlm_holder holder;
+ *	default:
+ *		void;
+ *	};
+ *
+ *	struct nlm_testres {
+ *		netobj cookie;
+ *		nlm_testrply test_stat;
+ *	};
+ */
+static int decode_nlm_testrply(struct xdr_stream *xdr,
+			       struct nlm_res *result)
+{
+	int error;
+
+	error = decode_nlm_stat(xdr, &result->status);
+	if (unlikely(error))
+		goto out;
+	if (result->status == LCK_DENIED)
+		error = decode_nlm_holder(xdr, result);
+out:
+	return error;
+}
+
+static int nlm_xdr_dec_testres(struct rpc_rqst *req, __be32 *p,
+			       struct nlm_res *result)
+{
+	struct xdr_stream xdr;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_cookie(&xdr, &result->cookie);
+	if (unlikely(error))
+		goto out;
+	error = decode_nlm_testrply(&xdr, result);
+out:
+	return error;
+}
+
+/*
+ *	struct nlm_res {
+ *		netobj cookie;
+ *		nlm_stat stat;
+ *	};
+ */
+static int nlm_xdr_dec_res(struct rpc_rqst *req, __be32 *p,
+			   struct nlm_res *result)
+{
+	struct xdr_stream xdr;
+	int error;
+
+	xdr_init_decode(&xdr, &req->rq_rcv_buf, p);
+	error = decode_cookie(&xdr, &result->cookie);
+	if (unlikely(error))
+		goto out;
+	error = decode_nlm_stat(&xdr, &result->status);
+out:
+	return error;
+}
+
+
+/*
+ * For NLM, a void procedure really returns nothing
+ */
+#define nlm_xdr_dec_norep	NULL
+
+#define PROC(proc, argtype, restype)	\
+[NLMPROC_##proc] = {							\
+	.p_proc      = NLMPROC_##proc,					\
+	.p_encode    = (kxdrproc_t)nlm_xdr_enc_##argtype,		\
+	.p_decode    = (kxdrproc_t)nlm_xdr_dec_##restype,		\
+	.p_arglen    = NLM_##argtype##_sz,				\
+	.p_replen    = NLM_##restype##_sz,				\
+	.p_statidx   = NLMPROC_##proc,					\
+	.p_name      = #proc,						\
+	}
+
+static struct rpc_procinfo	nlm_procedures[] = {
+    PROC(TEST,		testargs,	testres),
+    PROC(LOCK,		lockargs,	res),
+    PROC(CANCEL,	cancargs,	res),
+    PROC(UNLOCK,	unlockargs,	res),
+    PROC(GRANTED,	testargs,	res),
+    PROC(TEST_MSG,	testargs,	norep),
+    PROC(LOCK_MSG,	lockargs,	norep),
+    PROC(CANCEL_MSG,	cancargs,	norep),
+    PROC(UNLOCK_MSG,	unlockargs,	norep),
+    PROC(GRANTED_MSG,	testargs,	norep),
+    PROC(TEST_RES,	testres,	norep),
+    PROC(LOCK_RES,	res,		norep),
+    PROC(CANCEL_RES,	res,		norep),
+    PROC(UNLOCK_RES,	res,		norep),
+    PROC(GRANTED_RES,	res,		norep),
+};
+
+static struct rpc_version	nlm_version1 = {
+		.number		= 1,
+		.nrprocs	= 16,
+		.procs		= nlm_procedures,
+};
+
+static struct rpc_version	nlm_version3 = {
+		.number		= 3,
+		.nrprocs	= 24,
+		.procs		= nlm_procedures,
+};
+
+static struct rpc_version	*nlm_versions[] = {
+	[1] = &nlm_version1,
+	[3] = &nlm_version3,
+#ifdef CONFIG_LOCKD_V4
+	[4] = &nlm_version4,
+#endif
+};
+
+static struct rpc_stat		nlm_rpc_stats;
+
+struct rpc_program		nlm_program = {
+		.name		= "lockd",
+		.number		= NLM_PROGRAM,
+		.nrvers		= ARRAY_SIZE(nlm_versions),
+		.version	= nlm_versions,
+		.stats		= &nlm_rpc_stats,
+};
diff --git a/fs/lockd/xdr.c b/fs/lockd/xdr.c
index b583ab0..0eb694d 100644
--- a/fs/lockd/xdr.c
+++ b/fs/lockd/xdr.c
@@ -149,37 +149,6 @@ nlm_decode_lock(__be32 *p, struct nlm_lock *lock)
 }
 
 /*
- * Encode a lock as part of an NLM call
- */
-static __be32 *
-nlm_encode_lock(__be32 *p, struct nlm_lock *lock)
-{
-	struct file_lock	*fl = &lock->fl;
-	__s32			start, len;
-
-	if (!(p = xdr_encode_string(p, lock->caller))
-	 || !(p = nlm_encode_fh(p, &lock->fh))
-	 || !(p = nlm_encode_oh(p, &lock->oh)))
-		return NULL;
-
-	if (fl->fl_start > NLM_OFFSET_MAX
-	 || (fl->fl_end > NLM_OFFSET_MAX && fl->fl_end != OFFSET_MAX))
-		return NULL;
-
-	start = loff_t_to_s32(fl->fl_start);
-	if (fl->fl_end == OFFSET_MAX)
-		len = 0;
-	else
-		len = loff_t_to_s32(fl->fl_end - fl->fl_start + 1);
-
-	*p++ = htonl(lock->svid);
-	*p++ = htonl(start);
-	*p++ = htonl(len);
-
-	return p;
-}
-
-/*
  * Encode result of a TEST/TEST_MSG call
  */
 static __be32 *
@@ -373,233 +342,6 @@ nlmsvc_encode_void(struct svc_rqst *rqstp, __be32 *p, void *dummy)
 	return xdr_ressize_check(rqstp, p);
 }
 
-/*
- * Now, the client side XDR functions
- */
-#ifdef NLMCLNT_SUPPORT_SHARES
-static int
-nlmclt_decode_void(struct rpc_rqst *req, u32 *p, void *ptr)
-{
-	return 0;
-}
-#endif
-
-static int
-nlmclt_encode_testargs(struct rpc_rqst *req, __be32 *p, nlm_args *argp)
-{
-	struct nlm_lock	*lock = &argp->lock;
-
-	if (!(p = nlm_encode_cookie(p, &argp->cookie)))
-		return -EIO;
-	*p++ = (lock->fl.fl_type == F_WRLCK)? xdr_one : xdr_zero;
-	if (!(p = nlm_encode_lock(p, lock)))
-		return -EIO;
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-static int
-nlmclt_decode_testres(struct rpc_rqst *req, __be32 *p, struct nlm_res *resp)
-{
-	if (!(p = nlm_decode_cookie(p, &resp->cookie)))
-		return -EIO;
-	resp->status = *p++;
-	if (resp->status == nlm_lck_denied) {
-		struct file_lock	*fl = &resp->lock.fl;
-		u32			excl;
-		s32			start, len, end;
-
-		memset(&resp->lock, 0, sizeof(resp->lock));
-		locks_init_lock(fl);
-		excl = ntohl(*p++);
-		resp->lock.svid = ntohl(*p++);
-		fl->fl_pid = (pid_t)resp->lock.svid;
-		if (!(p = nlm_decode_oh(p, &resp->lock.oh)))
-			return -EIO;
-
-		fl->fl_flags = FL_POSIX;
-		fl->fl_type  = excl? F_WRLCK : F_RDLCK;
-		start = ntohl(*p++);
-		len = ntohl(*p++);
-		end = start + len - 1;
-
-		fl->fl_start = s32_to_loff_t(start);
-		if (len == 0 || end < 0)
-			fl->fl_end = OFFSET_MAX;
-		else
-			fl->fl_end = s32_to_loff_t(end);
-	}
-	return 0;
-}
-
-
-static int
-nlmclt_encode_lockargs(struct rpc_rqst *req, __be32 *p, nlm_args *argp)
-{
-	struct nlm_lock	*lock = &argp->lock;
-
-	if (!(p = nlm_encode_cookie(p, &argp->cookie)))
-		return -EIO;
-	*p++ = argp->block? xdr_one : xdr_zero;
-	*p++ = (lock->fl.fl_type == F_WRLCK)? xdr_one : xdr_zero;
-	if (!(p = nlm_encode_lock(p, lock)))
-		return -EIO;
-	*p++ = argp->reclaim? xdr_one : xdr_zero;
-	*p++ = htonl(argp->state);
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-static int
-nlmclt_encode_cancargs(struct rpc_rqst *req, __be32 *p, nlm_args *argp)
-{
-	struct nlm_lock	*lock = &argp->lock;
-
-	if (!(p = nlm_encode_cookie(p, &argp->cookie)))
-		return -EIO;
-	*p++ = argp->block? xdr_one : xdr_zero;
-	*p++ = (lock->fl.fl_type == F_WRLCK)? xdr_one : xdr_zero;
-	if (!(p = nlm_encode_lock(p, lock)))
-		return -EIO;
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-static int
-nlmclt_encode_unlockargs(struct rpc_rqst *req, __be32 *p, nlm_args *argp)
-{
-	struct nlm_lock	*lock = &argp->lock;
-
-	if (!(p = nlm_encode_cookie(p, &argp->cookie)))
-		return -EIO;
-	if (!(p = nlm_encode_lock(p, lock)))
-		return -EIO;
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-static int
-nlmclt_encode_res(struct rpc_rqst *req, __be32 *p, struct nlm_res *resp)
-{
-	if (!(p = nlm_encode_cookie(p, &resp->cookie)))
-		return -EIO;
-	*p++ = resp->status;
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-static int
-nlmclt_encode_testres(struct rpc_rqst *req, __be32 *p, struct nlm_res *resp)
-{
-	if (!(p = nlm_encode_testres(p, resp)))
-		return -EIO;
-	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
-	return 0;
-}
-
-static int
-nlmclt_decode_res(struct rpc_rqst *req, __be32 *p, struct nlm_res *resp)
-{
-	if (!(p = nlm_decode_cookie(p, &resp->cookie)))
-		return -EIO;
-	resp->status = *p++;
-	return 0;
-}
-
-#if (NLMCLNT_OHSIZE > XDR_MAX_NETOBJ)
-#  error "NLM host name cannot be larger than XDR_MAX_NETOBJ!"
-#endif
-
-/*
- * Buffer requirements for NLM
- */
-#define NLM_void_sz		0
-#define NLM_cookie_sz		1+XDR_QUADLEN(NLM_MAXCOOKIELEN)
-#define NLM_caller_sz		1+XDR_QUADLEN(NLMCLNT_OHSIZE)
-#define NLM_owner_sz		1+XDR_QUADLEN(NLMCLNT_OHSIZE)
-#define NLM_fhandle_sz		1+XDR_QUADLEN(NFS2_FHSIZE)
-#define NLM_lock_sz		3+NLM_caller_sz+NLM_owner_sz+NLM_fhandle_sz
-#define NLM_holder_sz		4+NLM_owner_sz
-
-#define NLM_testargs_sz		NLM_cookie_sz+1+NLM_lock_sz
-#define NLM_lockargs_sz		NLM_cookie_sz+4+NLM_lock_sz
-#define NLM_cancargs_sz		NLM_cookie_sz+2+NLM_lock_sz
-#define NLM_unlockargs_sz	NLM_cookie_sz+NLM_lock_sz
-
-#define NLM_testres_sz		NLM_cookie_sz+1+NLM_holder_sz
-#define NLM_res_sz		NLM_cookie_sz+1
-#define NLM_norep_sz		0
-
-/*
- * For NLM, a void procedure really returns nothing
- */
-#define nlmclt_decode_norep	NULL
-
-#define PROC(proc, argtype, restype)	\
-[NLMPROC_##proc] = {							\
-	.p_proc      = NLMPROC_##proc,					\
-	.p_encode    = (kxdrproc_t) nlmclt_encode_##argtype,		\
-	.p_decode    = (kxdrproc_t) nlmclt_decode_##restype,		\
-	.p_arglen    = NLM_##argtype##_sz,				\
-	.p_replen    = NLM_##restype##_sz,				\
-	.p_statidx   = NLMPROC_##proc,					\
-	.p_name      = #proc,						\
-	}
-
-static struct rpc_procinfo	nlm_procedures[] = {
-    PROC(TEST,		testargs,	testres),
-    PROC(LOCK,		lockargs,	res),
-    PROC(CANCEL,	cancargs,	res),
-    PROC(UNLOCK,	unlockargs,	res),
-    PROC(GRANTED,	testargs,	res),
-    PROC(TEST_MSG,	testargs,	norep),
-    PROC(LOCK_MSG,	lockargs,	norep),
-    PROC(CANCEL_MSG,	cancargs,	norep),
-    PROC(UNLOCK_MSG,	unlockargs,	norep),
-    PROC(GRANTED_MSG,	testargs,	norep),
-    PROC(TEST_RES,	testres,	norep),
-    PROC(LOCK_RES,	res,		norep),
-    PROC(CANCEL_RES,	res,		norep),
-    PROC(UNLOCK_RES,	res,		norep),
-    PROC(GRANTED_RES,	res,		norep),
-#ifdef NLMCLNT_SUPPORT_SHARES
-    PROC(SHARE,		shareargs,	shareres),
-    PROC(UNSHARE,	shareargs,	shareres),
-    PROC(NM_LOCK,	lockargs,	res),
-    PROC(FREE_ALL,	notify,		void),
-#endif
-};
-
-static struct rpc_version	nlm_version1 = {
-		.number		= 1,
-		.nrprocs	= 16,
-		.procs		= nlm_procedures,
-};
-
-static struct rpc_version	nlm_version3 = {
-		.number		= 3,
-		.nrprocs	= 24,
-		.procs		= nlm_procedures,
-};
-
-static struct rpc_version *	nlm_versions[] = {
-	[1] = &nlm_version1,
-	[3] = &nlm_version3,
-#ifdef 	CONFIG_LOCKD_V4
-	[4] = &nlm_version4,
-#endif
-};
-
-static struct rpc_stat		nlm_stats;
-
-struct rpc_program		nlm_program = {
-		.name		= "lockd",
-		.number		= NLM_PROGRAM,
-		.nrvers		= ARRAY_SIZE(nlm_versions),
-		.version	= nlm_versions,
-		.stats		= &nlm_stats,
-};
-
 #ifdef RPC_DEBUG
 const char *nlmdbg_cookie2a(const struct nlm_cookie *cookie)
 {