[PATCH v2 0/5] Remove NFS client header dependencies from LOCALIO

[PATCH v2 0/5] Remove NFS client header dependencies from LOCALIO

[Chuck Lever]

Clean up: fs/nfsd/localio.c is a server-side component, so it should
avoid dependency on client-specific headers and XDR utilities.

No change in behavior is expected.

---
Changes in v2:
- Including <linux/nfs4.h> is also no longer needed (Neil Brown)

Chuck Lever (5):
  NFSD: Move the RPC program definition for LOCALIO
  nfs_common: Remove "#include <linux/nfs.h>" from linux/nfslocalio.h
  NFSD: Tighten header includes in localio.c
  NFSD: Name the fh_maxsize value that carries no NFS version
  NFSD: Don't apply NFS version-specific behavior to LOCALIO requests

 fs/nfsd/localio.c          | 10 ++++------
 fs/nfsd/lockd.c            |  3 +--
 fs/nfsd/nfsfh.c            |  2 ++
 fs/nfsd/nfsfh.h            | 14 ++++++++++++++
 include/linux/nfs.h        |  7 -------
 include/linux/nfslocalio.h | 11 ++++++++++-
 6 files changed, 31 insertions(+), 16 deletions(-)

-- 
2.54.0

[PATCH v2 1/5] NFSD: Move the RPC program definition for LOCALIO

[Chuck Lever]

Clean up: The definitions for the LOCALIO program are not needed by
most files that include linux/nfs.h. Following the convention used
by most other in-kernel RPC program implementations, relocate the
LOCALIO program definitions to a localio-specific header.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 include/linux/nfs.h        | 7 -------
 include/linux/nfslocalio.h | 8 ++++++++
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/linux/nfs.h b/include/linux/nfs.h
index 0e2b210c103b..8c2818db43c5 100644
--- a/include/linux/nfs.h
+++ b/include/linux/nfs.h
@@ -15,11 +15,4 @@
 
 #include <uapi/linux/nfs.h>
 
-/* The LOCALIO program is entirely private to Linux and is
- * NOT part of the uapi.
- */
-#define NFS_LOCALIO_PROGRAM		400122
-#define LOCALIOPROC_NULL		0
-#define LOCALIOPROC_UUID_IS_LOCAL	1
-
 #endif /* _LINUX_NFS_H */
diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
index 3d91043254e6..d2b39e6e6c6a 100644
--- a/include/linux/nfslocalio.h
+++ b/include/linux/nfslocalio.h
@@ -16,6 +16,14 @@
 #include <linux/nfs.h>
 #include <net/net_namespace.h>
 
+/*
+ * The LOCALIO program is entirely private to Linux and is NOT part of
+ * the uapi.
+ */
+#define NFS_LOCALIO_PROGRAM		400122
+#define LOCALIOPROC_NULL		0
+#define LOCALIOPROC_UUID_IS_LOCAL	1
+
 struct nfs_client;
 struct nfs_file_localio;
 
-- 
2.54.0

[PATCH v2 2/5] nfs_common: Remove "#include <linux/nfs.h>" from linux/nfslocalio.h

[Chuck Lever]

Clean up: linux/nfslocalio.h pulls in linux/nfs.h only for the
definition of struct nfs_fh, which now lives in linux/nfs_fh.h.

Replace linux/nfs.h with linux/nfs_fh.h so that nfslocalio.h no longer
carries uapi/linux/nfs.h into its consumers.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 include/linux/nfslocalio.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/nfslocalio.h b/include/linux/nfslocalio.h
index d2b39e6e6c6a..8ce4d978a636 100644
--- a/include/linux/nfslocalio.h
+++ b/include/linux/nfslocalio.h
@@ -13,7 +13,8 @@
 #include <linux/uuid.h>
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/svcauth.h>
-#include <linux/nfs.h>
+#include <linux/nfs_fh.h>
+
 #include <net/net_namespace.h>
 
 /*
-- 
2.54.0

[PATCH v2 3/5] NFSD: Tighten header includes in localio.c

[Chuck Lever]

As a prerequisite to converting NFSD to use xdrgen more broadly,
NFSD source files should not depend on NFS client headers.
fs/nfsd/localio.c is server-side LOCALIO code, yet it pulled in
three of them: <linux/nfs_fs.h>, the client inode header (struct
nfs_inode, NFS_I(), writeback helpers), which server code never
uses; <linux/nfs_xdr.h>, whose only referenced symbol is
decode_opaque_fixed(), a static inline that exists to remap the
error return to -EIO for client call sites; and the catch-all
<linux/nfs.h>.

Convert the UUID decoder to call the canonical SUNRPC primitive
xdr_stream_decode_opaque_fixed() directly. It is shared by client
and server, performs the identical bounds check, and is already
reachable through <linux/sunrpc/clnt.h>. With the wrapper gone,
localio.c references no symbol from <linux/nfs_xdr.h>, and with
that header gone, none of the NFSv3 definitions its structs embed
are needed here.

Drop all three client includes and add what the file actually
uses: struct nfs_fh comes from <linux/nfs_fh.h>, included directly
rather than through nfslocalio.h's conditional re-export, and
NFS4_FHSIZE from <linux/nfs4.h>. enum nfs_stat and
nfs_stat_to_errno continue to come from the already-included
<linux/nfs_common.h>.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/localio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index c458c01e9478..4110be02b750 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -11,11 +11,10 @@
 #include <linux/exportfs.h>
 #include <linux/sunrpc/svcauth.h>
 #include <linux/sunrpc/clnt.h>
-#include <linux/nfs.h>
+#include <linux/nfs4.h>
 #include <linux/nfs_common.h>
+#include <linux/nfs_fh.h>
 #include <linux/nfslocalio.h>
-#include <linux/nfs_fs.h>
-#include <linux/nfs_xdr.h>
 #include <linux/string.h>
 
 #include "nfsd.h"
@@ -179,7 +178,7 @@ static bool localio_decode_uuidarg(struct svc_rqst *rqstp,
 	struct localio_uuidarg *argp = rqstp->rq_argp;
 	u8 uuid[UUID_SIZE];
 
-	if (decode_opaque_fixed(xdr, uuid, UUID_SIZE))
+	if (xdr_stream_decode_opaque_fixed(xdr, uuid, UUID_SIZE) < 0)
 		return false;
 	import_uuid(&argp->uuid, uuid);
 
-- 
2.54.0

[PATCH v2 4/5] NFSD: Name the fh_maxsize value that carries no NFS version

[Chuck Lever]

nfsd_set_fh_dentry() selects behavior specific to an NFS protocol
version by matching fh_maxsize against NFS_FHSIZE, NFS3_FHSIZE, or
NFS4_FHSIZE. A filehandle that reaches NFSD outside an NFS request has
no such version. nlm_fopen() opts out of the switch by passing a bare
0, which matches no arm, and the literal says nothing about why, so an
adjacent comment has to carry it.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/lockd.c |  3 +--
 fs/nfsd/nfsfh.c |  2 ++
 fs/nfsd/nfsfh.h | 14 ++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/lockd.c b/fs/nfsd/lockd.c
index f5a4f352f8ab..5ec0f5456063 100644
--- a/fs/nfsd/lockd.c
+++ b/fs/nfsd/lockd.c
@@ -34,8 +34,7 @@ static int nlm_fopen(struct svc_rqst *rqstp, struct nfs_fh *f,
 	int		access;
 	struct svc_fh	fh;
 
-	/* must initialize before using! but maxsize doesn't matter */
-	fh_init(&fh,0);
+	fh_init(&fh, NFSD_FHSIZE_UNSPEC);
 	fh.fh_handle.fh_size = f->size;
 	memcpy(&fh.fh_handle.fh_raw, f->data, f->size);
 	fh.fh_export = NULL;
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index fd721a5a6b37..b1f3c22af525 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -335,6 +335,8 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct net *net,
 	}
 
 	switch (fhp->fh_maxsize) {
+	case NFSD_FHSIZE_UNSPEC:
+		break;
 	case NFS4_FHSIZE:
 		if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOATOMIC_ATTR)
 			fhp->fh_no_atomic_attr = true;
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index ab15b59ac7b3..7d8e3f015307 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -246,6 +246,20 @@ fh_copy_shallow(struct knfsd_fh *dst, const struct knfsd_fh *src)
 	memcpy(&dst->fh_raw, &src->fh_raw, src->fh_size);
 }
 
+#define NFSD_FHSIZE_UNSPEC	0
+
+/**
+ * fh_init - Prepare a file handle for fh_compose() or fh_verify()
+ * @fhp: File handle to initialize
+ * @maxsize: Largest file handle, in bytes, to build in @fhp
+ *
+ * @maxsize bounds the handle fh_compose() may build: NFS_FHSIZE,
+ * NFS3_FHSIZE, and NFS4_FHSIZE additionally select version-specific
+ * handling in fh_verify(). Callers that only verify an incoming
+ * handle pass NFSD_FHSIZE_UNSPEC, which cannot be composed.
+ *
+ * Return: @fhp
+ */
 static __inline__ struct svc_fh *
 fh_init(struct svc_fh *fhp, int maxsize)
 {
-- 
2.54.0

[PATCH v2 5/5] NFSD: Don't apply NFS version-specific behavior to LOCALIO requests

[Chuck Lever]

LOCALIO serves NFS clients of every version through one entry point,
so no protocol version is associated with such a request.
nfsd_set_fh_dentry() selects version-specific behavior anyway: its
switch keys off fh_maxsize, and nfsd_open_local_fh() passes
NFS4_FHSIZE because that is the size of the buffer it copies into, so
LOCALIO lands in the NFSv4 arm. fh_getattr() keys off fh_maxsize too
and does run on a LOCALIO open, adding STATX_BTIME and
STATX_CHANGE_COOKIE to the mask it requests: work on filesystems that
compute them for a caller that never reads them.

nfsd_open_local_fh() only verifies a handle it received, so it has no
maximum size to state. Pass NFSD_FHSIZE_UNSPEC as nlm_fopen() already
does, which selects the switch arm that applies no version-specific
behavior, and state the bound on the copy out of struct nfs_fh as
NFS_MAXFHSIZE.

Suggested-by: NeilBrown <neil@brown.name>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/localio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index 4110be02b750..33b56d1b3f44 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -11,7 +11,6 @@
 #include <linux/exportfs.h>
 #include <linux/sunrpc/svcauth.h>
 #include <linux/sunrpc/clnt.h>
-#include <linux/nfs4.h>
 #include <linux/nfs_common.h>
 #include <linux/nfs_fh.h>
 #include <linux/nfslocalio.h>
@@ -54,7 +53,7 @@ nfsd_open_local_fh(struct net *net, struct auth_domain *dom,
 	struct nfsd_file *localio;
 	__be32 beres;
 
-	if (nfs_fh->size > NFS4_FHSIZE)
+	if (nfs_fh->size > NFS_MAXFHSIZE)
 		return ERR_PTR(-EINVAL);
 
 	if (!nfsd_net_try_get(net))
@@ -67,7 +66,7 @@ nfsd_open_local_fh(struct net *net, struct auth_domain *dom,
 		return localio;
 
 	/* nfs_fh -> svc_fh */
-	fh_init(&fh, NFS4_FHSIZE);
+	fh_init(&fh, NFSD_FHSIZE_UNSPEC);
 	fh.fh_handle.fh_size = nfs_fh->size;
 	memcpy(fh.fh_handle.fh_raw, nfs_fh->data, nfs_fh->size);
 
-- 
2.54.0