Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
* [RFC PATCH 02/35] nfs/nfsd: basic NFS4 extended attribute definitions
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
@ 2019-07-01 17:56 ` Frank van der Linden
  2019-08-26 21:38 ` [RFC PATCH 03/35] NFSv4.2: query the server for extended attribute support Frank van der Linden
                   ` (34 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-07-01 17:56 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Add definitions for the new operations, errors and flag as defined
in RFC 8276 (File System Extended Attributes in NFSv4).

Since this increments LAST_NFS4_OP/LAST_NFS42_OP, the nfsd code
now "knows" about this op, so add enotsup decode entry points.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c         |  6 ++++++
 include/linux/nfs4.h      | 27 ++++++++++++++++++++++++++-
 include/linux/nfs_fs.h    |  3 +++
 include/uapi/linux/nfs4.h |  3 +++
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 442811809f3d..f2090f7fed42 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1875,6 +1875,12 @@ static const nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_SEEK]		= (nfsd4_dec)nfsd4_decode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_CLONE]		= (nfsd4_dec)nfsd4_decode_clone,
+
+        /* Placeholders for RFC 8276 extended atributes operations */
+        [OP_GETXATTR]           = (nfsd4_dec)nfsd4_decode_notsupp,
+        [OP_SETXATTR]           = (nfsd4_dec)nfsd4_decode_notsupp,
+        [OP_LISTXATTRS]         = (nfsd4_dec)nfsd4_decode_notsupp,
+        [OP_REMOVEXATTR]        = (nfsd4_dec)nfsd4_decode_notsupp,
 };
 
 static inline bool
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index fd59904a282c..4aaa67e1dbad 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -149,6 +149,12 @@ enum nfs_opnum4 {
 	OP_WRITE_SAME = 70,
 	OP_CLONE = 71,
 
+	/* xattr support (RFC8726) */
+	OP_GETXATTR                = 72,
+	OP_SETXATTR                = 73,
+	OP_LISTXATTRS              = 74,
+	OP_REMOVEXATTR             = 75,
+
 	OP_ILLEGAL = 10044,
 };
 
@@ -158,7 +164,7 @@ Needs to be updated if more operations are defined in future.*/
 #define FIRST_NFS4_OP	OP_ACCESS
 #define LAST_NFS40_OP	OP_RELEASE_LOCKOWNER
 #define LAST_NFS41_OP	OP_RECLAIM_COMPLETE
-#define LAST_NFS42_OP	OP_CLONE
+#define LAST_NFS42_OP	OP_REMOVEXATTR
 #define LAST_NFS4_OP	LAST_NFS42_OP
 
 enum nfsstat4 {
@@ -279,6 +285,10 @@ enum nfsstat4 {
 	NFS4ERR_WRONG_LFS = 10092,
 	NFS4ERR_BADLABEL = 10093,
 	NFS4ERR_OFFLOAD_NO_REQS = 10094,
+
+	/* xattr (RFC8276) */
+	NFS4ERR_NOXATTR        = 10095,
+	NFS4ERR_XATTR2BIG      = 10096,
 };
 
 static inline bool seqid_mutating_err(u32 err)
@@ -451,6 +461,7 @@ enum change_attr_type4 {
 #define FATTR4_WORD2_CHANGE_ATTR_TYPE	(1UL << 15)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
 #define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
+#define FATTR4_WORD2_XATTR_SUPPORT	(1UL << 18)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)
@@ -539,6 +550,11 @@ enum {
 
 	NFSPROC4_CLNT_LOOKUPP,
 	NFSPROC4_CLNT_LAYOUTERROR,
+
+	NFSPROC4_CLNT_GETXATTR,
+	NFSPROC4_CLNT_SETXATTR,
+	NFSPROC4_CLNT_LISTXATTRS,
+	NFSPROC4_CLNT_REMOVEXATTR,
 };
 
 /* nfs41 types */
@@ -674,4 +690,13 @@ struct nfs4_op_map {
 	} u;
 };
 
+/*
+ * Options for setxattr. These match the flags for setxattr(2).
+ */
+enum nfs4_setxattr_options {
+	SETXATTR4_EITHER      = 0,
+	SETXATTR4_CREATE      = 1,
+	SETXATTR4_REPLACE     = 2,
+};
+
 #endif
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 0a11712a80e3..e87c894d1960 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -207,6 +207,9 @@ struct nfs4_copy_state {
 #define NFS_ACCESS_EXTEND      0x0008
 #define NFS_ACCESS_DELETE      0x0010
 #define NFS_ACCESS_EXECUTE     0x0020
+#define NFS_ACCESS_XAREAD      0x0040
+#define NFS_ACCESS_XAWRITE     0x0080
+#define NFS_ACCESS_XALIST      0x0100
 
 /*
  * Cache validity bit flags
diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h
index 8572930cf5b0..bf197e99b98f 100644
--- a/include/uapi/linux/nfs4.h
+++ b/include/uapi/linux/nfs4.h
@@ -33,6 +33,9 @@
 #define NFS4_ACCESS_EXTEND      0x0008
 #define NFS4_ACCESS_DELETE      0x0010
 #define NFS4_ACCESS_EXECUTE     0x0020
+#define NFS4_ACCESS_XAREAD      0x0040
+#define NFS4_ACCESS_XAWRITE     0x0080
+#define NFS4_ACCESS_XALIST      0x0100
 
 #define NFS4_FH_PERSISTENT		0x0000
 #define NFS4_FH_NOEXPIRE_WITH_OPEN	0x0001
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 03/35] NFSv4.2: query the server for extended attribute support
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
  2019-07-01 17:56 ` [RFC PATCH 02/35] nfs/nfsd: basic NFS4 extended attribute definitions Frank van der Linden
@ 2019-08-26 21:38 ` Frank van der Linden
  2019-08-26 21:53 ` [RFC PATCH 04/35] nfs: parse the {no}user_xattr option Frank van der Linden
                   ` (33 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 21:38 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Query the server for extended attribute support, and record it
as the NFS_CAP_XATTR flag in the server capabilities.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs4proc.c         | 18 ++++++++++++++++--
 fs/nfs/nfs4xdr.c          | 27 +++++++++++++++++++++++++++
 include/linux/nfs_fs_sb.h |  2 ++
 include/linux/nfs_xdr.h   |  1 +
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1406858bae6c..dad22450546d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3650,6 +3650,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
 static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
 {
 	u32 bitmask[3] = {}, minorversion = server->nfs_client->cl_minorversion;
+	u32 fattr4_word2_nfs42_mask;
 	struct nfs4_server_caps_arg args = {
 		.fhandle = fhandle,
 		.bitmask = bitmask,
@@ -3671,6 +3672,15 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 	if (minorversion)
 		bitmask[2] = FATTR4_WORD2_SUPPATTR_EXCLCREAT;
 
+	fattr4_word2_nfs42_mask = FATTR4_WORD2_NFS42_MASK;
+
+#ifdef CONFIG_NFS_V4_XATTR
+	if (minorversion >= 2 && (server->flags & NFS_MOUNT_USER_XATTR)) {
+		bitmask[2] |= FATTR4_WORD2_XATTR_SUPPORT;
+		fattr4_word2_nfs42_mask |= FATTR4_WORD2_XATTR_SUPPORT;
+	}
+#endif
+
 	status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, &res.seq_res, 0);
 	if (status == 0) {
 		/* Sanity check the server answers */
@@ -3683,7 +3693,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 			res.attr_bitmask[2] &= FATTR4_WORD2_NFS41_MASK;
 			break;
 		case 2:
-			res.attr_bitmask[2] &= FATTR4_WORD2_NFS42_MASK;
+			res.attr_bitmask[2] &= fattr4_word2_nfs42_mask;
 		}
 		memcpy(server->attr_bitmask, res.attr_bitmask, sizeof(server->attr_bitmask));
 		server->caps &= ~(NFS_CAP_ACLS|NFS_CAP_HARDLINKS|
@@ -3691,7 +3701,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 				NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
 				NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
 				NFS_CAP_CTIME|NFS_CAP_MTIME|
-				NFS_CAP_SECURITY_LABEL);
+				NFS_CAP_SECURITY_LABEL|NFS_CAP_XATTR);
 		if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
 				res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
 			server->caps |= NFS_CAP_ACLS;
@@ -3718,6 +3728,10 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
 			server->caps |= NFS_CAP_SECURITY_LABEL;
+#endif
+#ifdef CONFIG_NFS_V4_XATTR
+		if (res.has_xattr)
+			server->caps |= NFS_CAP_XATTR;
 #endif
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 46a8d636d151..00fd5732c59d 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4204,6 +4204,28 @@ static int decode_attr_time_modify(struct xdr_stream *xdr, uint32_t *bitmap, str
 	return status;
 }
 
+#ifdef CONFIG_NFS_V4_XATTR
+static int decode_attr_xattrsupport(struct xdr_stream *xdr, uint32_t *bitmap,
+				    uint32_t *res)
+{
+	__be32 *p;
+
+	*res = 0;
+	if (unlikely(bitmap[2] & (FATTR4_WORD2_XATTR_SUPPORT - 1U)))
+		return -EIO;
+	if (likely(bitmap[2] & FATTR4_WORD2_XATTR_SUPPORT)) {
+		p = xdr_inline_decode(xdr, 4);
+		if (unlikely(!p))
+			return -EIO;
+		*res = be32_to_cpup(p);
+		bitmap[2] &= ~FATTR4_WORD2_XATTR_SUPPORT;
+	}
+	dprintk("%s: XATTR support=%s\n", __func__,
+		*res == 0 ? "false" : "true");
+	return 0;
+}
+#endif
+
 static int verify_attr_len(struct xdr_stream *xdr, unsigned int savep, uint32_t attrlen)
 {
 	unsigned int attrwords = XDR_QUADLEN(attrlen);
@@ -4371,6 +4393,11 @@ static int decode_server_caps(struct xdr_stream *xdr, struct nfs4_server_caps_re
 	if ((status = decode_attr_exclcreat_supported(xdr, bitmap,
 				res->exclcreat_bitmask)) != 0)
 		goto xdr_error;
+#ifdef CONFIG_NFS_V4_XATTR
+	if ((status = decode_attr_xattrsupport(xdr, bitmap,
+					       &res->has_xattr)) != 0)
+		goto xdr_error;
+#endif
 	status = verify_attr_len(xdr, savep, attrlen);
 xdr_error:
 	dprintk("%s: xdr returned %d!\n", __func__, -status);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index a87fe854f008..290d1c521226 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -149,6 +149,7 @@ struct nfs_server {
 #define NFS_MOUNT_LOCAL_FLOCK		0x100000
 #define NFS_MOUNT_LOCAL_FCNTL		0x200000
 #define NFS_MOUNT_SOFTERR		0x400000
+#define NFS_MOUNT_USER_XATTR		0x800000
 
 	unsigned int		caps;		/* server capabilities */
 	unsigned int		rsize;		/* read size */
@@ -276,5 +277,6 @@ struct nfs_server {
 #define NFS_CAP_COPY		(1U << 24)
 #define NFS_CAP_OFFLOAD_CANCEL	(1U << 25)
 #define NFS_CAP_LAYOUTERROR	(1U << 26)
+#define NFS_CAP_XATTR		(1U << 27)
 
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 9b8324ec08f3..f289c024943d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1178,6 +1178,7 @@ struct nfs4_server_caps_res {
 	u32				has_links;
 	u32				has_symlinks;
 	u32				fh_expire_type;
+	u32				has_xattr;
 };
 
 #define NFS4_PATHNAME_MAXCOMPONENTS 512
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 04/35] nfs: parse the {no}user_xattr option
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
  2019-07-01 17:56 ` [RFC PATCH 02/35] nfs/nfsd: basic NFS4 extended attribute definitions Frank van der Linden
  2019-08-26 21:38 ` [RFC PATCH 03/35] NFSv4.2: query the server for extended attribute support Frank van der Linden
@ 2019-08-26 21:53 ` Frank van der Linden
  2019-08-26 22:06 ` [RFC PATCH 05/35] NFSv4.2: define a function to compute the maximum XDR size for listxattr Frank van der Linden
                   ` (32 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 21:53 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Parse the {no}user_xattr option to enable the optional NFS4.2 user
extended attribute support.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/super.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 703f595dce90..bbb93bc87887 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -93,6 +93,7 @@ enum {
 	Opt_resvport, Opt_noresvport,
 	Opt_fscache, Opt_nofscache,
 	Opt_migration, Opt_nomigration,
+	Opt_user_xattr, Opt_nouser_xattr,
 
 	/* Mount options that take integer arguments */
 	Opt_port,
@@ -190,6 +191,9 @@ static const match_table_t nfs_mount_option_tokens = {
 	{ Opt_fscache_uniq, "fsc=%s" },
 	{ Opt_local_lock, "local_lock=%s" },
 
+	{ Opt_user_xattr, "user_xattr" },
+	{ Opt_nouser_xattr, "nouser_xattr" },
+
 	/* The following needs to be listed after all other options */
 	{ Opt_nfsvers, "v%s" },
 
@@ -643,6 +647,7 @@ static void nfs_show_mount_options(struct seq_file *m, struct nfs_server *nfss,
 		{ NFS_MOUNT_NORDIRPLUS, ",nordirplus", "" },
 		{ NFS_MOUNT_UNSHARED, ",nosharecache", "" },
 		{ NFS_MOUNT_NORESVPORT, ",noresvport", "" },
+		{ NFS_MOUNT_USER_XATTR, ",user_xattr", "" },
 		{ 0, NULL, NULL }
 	};
 	const struct proc_nfs_info *nfs_infop;
@@ -1316,6 +1321,12 @@ static int nfs_parse_mount_options(char *raw,
 		case Opt_noacl:
 			mnt->flags |= NFS_MOUNT_NOACL;
 			break;
+		case Opt_user_xattr:
+			mnt->flags |= NFS_MOUNT_USER_XATTR;
+			break;
+		case Opt_nouser_xattr:
+			mnt->flags &= ~NFS_MOUNT_USER_XATTR;
+			break;
 		case Opt_rdirplus:
 			mnt->flags &= ~NFS_MOUNT_NORDIRPLUS;
 			break;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 05/35] NFSv4.2: define a function to compute the maximum XDR size for listxattr
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (2 preceding siblings ...)
  2019-08-26 21:53 ` [RFC PATCH 04/35] nfs: parse the {no}user_xattr option Frank van der Linden
@ 2019-08-26 22:06 ` Frank van der Linden
  2019-08-26 22:23 ` [RFC PATCH 06/35] NFSv4.2: define and set initial limits for extended attributes Frank van der Linden
                   ` (31 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 22:06 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

RFC 8276 specifies the maximum return size of the LISTXATTRS operation
as the XDR-encoded size of the entire reply.

Define a function that computes the maximum needed XDR size (minus the
cookie and the attribute count, which are in the XDR buffer header), to
have an upper bound to check against.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs42.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
index 901cca7542f9..935651345be7 100644
--- a/fs/nfs/nfs42.h
+++ b/fs/nfs/nfs42.h
@@ -6,6 +6,10 @@
 #ifndef __LINUX_FS_NFS_NFS4_2_H
 #define __LINUX_FS_NFS_NFS4_2_H
 
+#ifdef CONFIG_NFS_V4_XATTR
+#include <linux/xattr.h>
+#endif
+
 /*
  * FIXME:  four LAYOUTSTATS calls per compound at most! Do we need to support
  * more? Need to consider not to pre-alloc too much for a compound.
@@ -24,4 +28,20 @@ int nfs42_proc_layouterror(struct pnfs_layout_segment *lseg,
 			   const struct nfs42_layout_error *errors,
 			   size_t n);
 
+#ifdef CONFIG_NFS_V4_XATTR
+/*
+ * Maximum XDR buffer size needed for a listxattr buffer of buflen size.
+ *
+ * The upper boundary is a buffer with all 1-byte sized attribute names.
+ * They would be 7 bytes long in the eventual buffer ("user.x\0"), and
+ * 8 bytes long XDR-encoded.
+ *
+ * Include the trailing eof word as well.
+ */
+static inline u32 nfs42_listxattr_xdrsize(u32 buflen)
+{
+	return ((buflen / (XATTR_USER_PREFIX_LEN + 2)) * 8) + 4;
+}
+#endif
+
 #endif /* __LINUX_FS_NFS_NFS4_2_H */
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 06/35] NFSv4.2: define and set initial limits for extended attributes
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (3 preceding siblings ...)
  2019-08-26 22:06 ` [RFC PATCH 05/35] NFSv4.2: define a function to compute the maximum XDR size for listxattr Frank van der Linden
@ 2019-08-26 22:23 ` Frank van der Linden
  2019-08-26 22:32 ` [RFC PATCH 07/35] NFSv4.2: define argument and response structures for xattr operations Frank van der Linden
                   ` (30 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 22:23 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Set limits for extended attributes (attribute value size and listxattr
buffer size), based on the fs-independent limits (XATTR_*_MAX).

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/client.c           | 17 +++++++++++++++--
 include/linux/nfs_fs_sb.h |  5 +++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 30838304a0bf..4a988787049c 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -50,6 +50,7 @@
 #include "nfs.h"
 #include "netns.h"
 #include "sysfs.h"
+#include "nfs42.h"
 
 #define NFSDBG_FACILITY		NFSDBG_CLIENT
 
@@ -733,7 +734,7 @@ static int nfs_init_server(struct nfs_server *server,
 static void nfs_server_set_fsinfo(struct nfs_server *server,
 				  struct nfs_fsinfo *fsinfo)
 {
-	unsigned long max_rpc_payload;
+	unsigned long max_rpc_payload, raw_max_rpc_payload;
 
 	/* Work out a lot of parameters */
 	if (server->rsize == 0)
@@ -746,7 +747,9 @@ static void nfs_server_set_fsinfo(struct nfs_server *server,
 	if (fsinfo->wtmax >= 512 && server->wsize > fsinfo->wtmax)
 		server->wsize = nfs_block_size(fsinfo->wtmax, NULL);
 
-	max_rpc_payload = nfs_block_size(rpc_max_payload(server->client), NULL);
+	raw_max_rpc_payload = rpc_max_payload(server->client);
+	max_rpc_payload = nfs_block_size(raw_max_rpc_payload, NULL);
+
 	if (server->rsize > max_rpc_payload)
 		server->rsize = max_rpc_payload;
 	if (server->rsize > NFS_MAX_FILE_IO_SIZE)
@@ -779,6 +782,16 @@ static void nfs_server_set_fsinfo(struct nfs_server *server,
 	server->clone_blksize = fsinfo->clone_blksize;
 	/* We're airborne Set socket buffersize */
 	rpc_setbufsize(server->client, server->wsize + 100, server->rsize + 100);
+
+#ifdef CONFIG_NFS_V4_XATTR
+	/*
+	 * Defaults until limited by the session parameters.
+	 */
+	server->gxasize = min_t(unsigned, raw_max_rpc_payload, XATTR_SIZE_MAX);
+	server->sxasize = min_t(unsigned, raw_max_rpc_payload, XATTR_SIZE_MAX);
+	server->lxasize = min_t(unsigned, raw_max_rpc_payload,
+				nfs42_listxattr_xdrsize(XATTR_LIST_MAX));
+#endif
 }
 
 /*
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 290d1c521226..db190a139d26 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -160,6 +160,11 @@ struct nfs_server {
 	unsigned int		dtsize;		/* readdir size */
 	unsigned short		port;		/* "port=" setting */
 	unsigned int		bsize;		/* server block size */
+#ifdef CONFIG_NFS_V4_XATTR
+	unsigned int		gxasize;	/* getxattr size */
+	unsigned int		sxasize;	/* setxattr size */
+	unsigned int		lxasize;	/* listxattr size */
+#endif
 	unsigned int		acregmin;	/* attr cache timeouts */
 	unsigned int		acregmax;
 	unsigned int		acdirmin;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 07/35] NFSv4.2: define argument and response structures for xattr operations
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (4 preceding siblings ...)
  2019-08-26 22:23 ` [RFC PATCH 06/35] NFSv4.2: define and set initial limits for extended attributes Frank van der Linden
@ 2019-08-26 22:32 ` Frank van der Linden
  2019-08-26 22:44 ` [RFC PATCH 08/35] NFSv4.2: define the encode/decode sizes for the XATTR operations Frank van der Linden
                   ` (29 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 22:32 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Define the argument and response structures that will be used for
RFC 8276 extended attribute RPC calls.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 include/linux/nfs_xdr.h | 61 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index f289c024943d..9299a2465c02 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1480,7 +1480,66 @@ struct nfs42_seek_res {
 	u32	sr_eof;
 	u64	sr_offset;
 };
-#endif
+
+#ifdef CONFIG_NFS_V4_XATTR
+struct nfs42_setxattrargs {
+	struct nfs4_sequence_args	seq_args;
+	struct nfs_fh *			fh;
+	const char *			xattr_name;
+	u32				xattr_flags;
+	size_t				xattr_len;
+	struct page **			xattr_pages;
+};
+
+struct nfs42_setxattrres {
+	struct nfs4_sequence_res	seq_res;
+	struct nfs4_change_info		cinfo;
+};
+
+struct nfs42_getxattrargs {
+	struct nfs4_sequence_args 	seq_args;
+	struct nfs_fh *			fh;
+	const char *			xattr_name;
+	size_t				xattr_len;
+	struct page **			xattr_pages;
+};
+
+struct nfs42_getxattrres {
+	struct nfs4_sequence_res	seq_res;
+	size_t				xattr_len;
+};
+
+struct nfs42_listxattrsargs {
+	struct nfs4_sequence_args	seq_args;
+	struct nfs_fh *			fh;
+	u32				count;
+	u64				cookie;
+	struct page **			xattr_pages;
+};
+
+struct nfs42_listxattrsres {
+	struct nfs4_sequence_res	seq_res;
+	struct page *			scratch;
+	void *				xattr_buf;
+	size_t				xattr_len;
+	u64				cookie;
+	bool				eof;
+	size_t				copied;
+};
+
+struct nfs42_removexattrargs {
+	struct nfs4_sequence_args	seq_args;
+	struct nfs_fh *			fh;
+	const char *			xattr_name;
+};
+
+struct nfs42_removexattrres {
+	struct nfs4_sequence_res	seq_res;
+	struct nfs4_change_info		cinfo;
+};
+
+#endif /* CONFIG_NFS_V4_XATTR */
+#endif /* CONFIG_NFS_V4_2 */
 
 struct nfs_page;
 
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 08/35] NFSv4.2: define the encode/decode sizes for the XATTR operations
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (5 preceding siblings ...)
  2019-08-26 22:32 ` [RFC PATCH 07/35] NFSv4.2: define argument and response structures for xattr operations Frank van der Linden
@ 2019-08-26 22:44 ` Frank van der Linden
  2019-08-26 23:09 ` [RFC PATCH 09/35] NFSv4.2: define and use extended attribute overhead sizes Frank van der Linden
                   ` (28 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 22:44 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Define the maximum XDR sizes for the RFC 8276 XATTR operations.
In the case of operations that carry a larger payload (SETXATTR,
GETXATTR, LISTXATTR), these exclude that payload, which is added
as seperate pages, like other operations do.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs42xdr.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index aed865a84629..d7657be74557 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -150,6 +150,57 @@
 					 decode_clone_maxsz + \
 					 decode_getattr_maxsz)
 
+#ifdef CONFIG_NFS_V4_XATTR
+/* Not limited by NFS itself, limited by the generic xattr code */
+#define nfs4_xattr_name_maxsz   XDR_QUADLEN(XATTR_NAME_MAX)
+
+#define encode_getxattr_maxsz   (op_encode_hdr_maxsz + 1 + \
+				 nfs4_xattr_name_maxsz)
+#define decode_getxattr_maxsz   (op_decode_hdr_maxsz + 1 + 1)
+#define encode_setxattr_maxsz   (op_encode_hdr_maxsz + \
+				 1 + nfs4_xattr_name_maxsz + 1)
+#define decode_setxattr_maxsz   (op_decode_hdr_maxsz + decode_change_info_maxsz)
+#define encode_listxattrs_maxsz  (op_encode_hdr_maxsz + 2 + 1)
+#define decode_listxattrs_maxsz  (op_decode_hdr_maxsz + 2 + 1 + 1)
+#define encode_removexattr_maxsz (op_encode_hdr_maxsz + 1 + \
+				  nfs4_xattr_name_maxsz)
+#define decode_removexattr_maxsz (op_decode_hdr_maxsz + \
+				  decode_change_info_maxsz)
+
+#define NFS4_enc_getxattr_sz	(compound_encode_hdr_maxsz + \
+				encode_sequence_maxsz + \
+				encode_putfh_maxsz + \
+				encode_getxattr_maxsz)
+#define NFS4_dec_getxattr_sz	(compound_decode_hdr_maxsz + \
+				decode_sequence_maxsz + \
+				decode_putfh_maxsz + \
+				decode_getxattr_maxsz)
+#define NFS4_enc_setxattr_sz	(compound_encode_hdr_maxsz + \
+				encode_sequence_maxsz + \
+				encode_putfh_maxsz + \
+				encode_setxattr_maxsz)
+#define NFS4_dec_setxattr_sz	(compound_decode_hdr_maxsz + \
+				decode_sequence_maxsz + \
+				decode_putfh_maxsz + \
+				decode_setxattr_maxsz)
+#define NFS4_enc_listxattrs_sz	(compound_encode_hdr_maxsz + \
+				encode_sequence_maxsz + \
+				encode_putfh_maxsz + \
+				encode_listxattrs_maxsz)
+#define NFS4_dec_listxattrs_sz	(compound_decode_hdr_maxsz + \
+				decode_sequence_maxsz + \
+				decode_putfh_maxsz + \
+				decode_listxattrs_maxsz)
+#define NFS4_enc_removexattr_sz	(compound_encode_hdr_maxsz + \
+				encode_sequence_maxsz + \
+				encode_putfh_maxsz + \
+				encode_removexattr_maxsz)
+#define NFS4_dec_removexattr_sz	(compound_decode_hdr_maxsz + \
+				decode_sequence_maxsz + \
+				decode_putfh_maxsz + \
+				decode_removexattr_maxsz)
+#endif
+
 static void encode_fallocate(struct xdr_stream *xdr,
 			     const struct nfs42_falloc_args *args)
 {
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 09/35] NFSv4.2: define and use extended attribute overhead sizes
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (6 preceding siblings ...)
  2019-08-26 22:44 ` [RFC PATCH 08/35] NFSv4.2: define the encode/decode sizes for the XATTR operations Frank van der Linden
@ 2019-08-26 23:09 ` Frank van der Linden
  2019-08-27 15:34 ` [RFC PATCH 10/35] NFSv4.2: add client side XDR handling for extended attributes Frank van der Linden
                   ` (27 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-26 23:09 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Define, much like for read and write operations, the maximum overhead
sizes for get/set/listxattr, and use them to limit the maximum payload
size for those operations, in combination with the channel attributes.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/internal.h   |  5 +++++
 fs/nfs/nfs42xdr.c   | 23 +++++++++++++++++++++++
 fs/nfs/nfs4client.c | 31 +++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index e64f810223be..a1464bf8d178 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -295,6 +295,11 @@ extern const u32 nfs41_maxread_overhead;
 extern const u32 nfs41_maxwrite_overhead;
 extern const u32 nfs41_maxgetdevinfo_overhead;
 #endif
+#ifdef CONFIG_NFS_V4_XATTR
+extern const u32 nfs42_maxsetxattr_overhead;
+extern const u32 nfs42_maxgetxattr_overhead;
+extern const u32 nfs42_maxlistxattrs_overhead;
+#endif
 
 /* nfs4proc.c */
 #if IS_ENABLED(CONFIG_NFS_V4)
diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index d7657be74557..ca29d2702017 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -199,6 +199,29 @@
 				decode_sequence_maxsz + \
 				decode_putfh_maxsz + \
 				decode_removexattr_maxsz)
+
+/*
+ * These values specify the maximum amount of data that is not
+ * associated with the extended attribute name or extended
+ * attribute list in the SETXATTR, GETXATTR and LISTXATTR
+ * respectively.
+ */
+const u32 nfs42_maxsetxattr_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
+					compound_encode_hdr_maxsz +
+					encode_sequence_maxsz +
+					encode_putfh_maxsz + 1 +
+					nfs4_xattr_name_maxsz)
+					* XDR_UNIT);
+
+const u32 nfs42_maxgetxattr_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
+					compound_decode_hdr_maxsz +
+					decode_sequence_maxsz +
+					decode_putfh_maxsz + 1) * XDR_UNIT);
+
+const u32 nfs42_maxlistxattrs_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
+					compound_decode_hdr_maxsz +
+					decode_sequence_maxsz +
+					decode_putfh_maxsz + 3) * XDR_UNIT);
 #endif
 
 static void encode_fallocate(struct xdr_stream *xdr,
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index da6204025a2d..d6f28bcd0929 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -990,6 +990,36 @@ static void nfs4_session_limit_rwsize(struct nfs_server *server)
 #endif /* CONFIG_NFS_V4_1 */
 }
 
+/*
+ * Limit xattr sizes using the channel attributes.
+ */
+static void nfs4_session_limit_xasize(struct nfs_server *server)
+{
+#ifdef CONFIG_NFS_V4_XATTR
+	struct nfs4_session *sess;
+	u32 server_gxa_sz;
+	u32 server_sxa_sz;
+	u32 server_lxa_sz;
+
+	if (!nfs4_has_session(server->nfs_client))
+		return;
+
+	sess = server->nfs_client->cl_session;
+
+	server_gxa_sz = sess->fc_attrs.max_resp_sz - nfs42_maxgetxattr_overhead;
+	server_sxa_sz = sess->fc_attrs.max_rqst_sz - nfs42_maxsetxattr_overhead;
+	server_lxa_sz = sess->fc_attrs.max_resp_sz -
+	    nfs42_maxlistxattrs_overhead;
+
+	if (server->gxasize > server_gxa_sz)
+		server->gxasize = server_gxa_sz;
+	if (server->sxasize > server_sxa_sz)
+		server->sxasize = server_sxa_sz;
+	if (server->lxasize > server_lxa_sz)
+		server->lxasize = server_lxa_sz;
+#endif
+}
+
 static int nfs4_server_common_setup(struct nfs_server *server,
 		struct nfs_fh *mntfh, bool auth_probe)
 {
@@ -1037,6 +1067,7 @@ static int nfs4_server_common_setup(struct nfs_server *server,
 		goto out;
 
 	nfs4_session_limit_rwsize(server);
+	nfs4_session_limit_xasize(server);
 
 	if (server->namelen == 0 || server->namelen > NFS4_MAXNAMLEN)
 		server->namelen = NFS4_MAXNAMLEN;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 10/35] NFSv4.2: add client side XDR handling for extended attributes
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (7 preceding siblings ...)
  2019-08-26 23:09 ` [RFC PATCH 09/35] NFSv4.2: define and use extended attribute overhead sizes Frank van der Linden
@ 2019-08-27 15:34 ` Frank van der Linden
  2019-08-27 15:46 ` [RFC PATCH 11/35] nfs: define nfs_access_get_cached function Frank van der Linden
                   ` (26 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-27 15:34 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Implement the necessary functions to encode/decode the extended
attribute operations.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs42xdr.c | 372 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/nfs4xdr.c  |   8 +
 2 files changed, 380 insertions(+)

diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
index ca29d2702017..dbd27d52f119 100644
--- a/fs/nfs/nfs42xdr.c
+++ b/fs/nfs/nfs42xdr.c
@@ -354,6 +354,212 @@ static void encode_layouterror(struct xdr_stream *xdr,
 	encode_device_error(xdr, &args->errors[0]);
 }
 
+#ifdef CONFIG_NFS_V4_XATTR
+static void encode_setxattr(struct xdr_stream *xdr,
+			    const struct nfs42_setxattrargs *arg,
+			    struct compound_hdr *hdr)
+{
+	__be32 *p;
+
+	BUILD_BUG_ON(XATTR_CREATE != SETXATTR4_CREATE);
+	BUILD_BUG_ON(XATTR_REPLACE != SETXATTR4_REPLACE);
+
+	encode_op_hdr(xdr, OP_SETXATTR, decode_setxattr_maxsz, hdr);
+	p = reserve_space(xdr, 4);
+	*p = cpu_to_be32(arg->xattr_flags);
+	encode_string(xdr, strlen(arg->xattr_name), arg->xattr_name);
+	p = reserve_space(xdr, 4);
+	*p = cpu_to_be32(arg->xattr_len);
+	if (arg->xattr_len)
+		xdr_write_pages(xdr, arg->xattr_pages, 0, arg->xattr_len);
+}
+
+static int decode_setxattr(struct xdr_stream *xdr,
+			   struct nfs4_change_info *cinfo)
+{
+	int status;
+
+	status = decode_op_hdr(xdr, OP_SETXATTR);
+	if (status)
+		goto out;
+	status = decode_change_info(xdr, cinfo);
+out:
+	return status;
+}
+
+
+static void encode_getxattr(struct xdr_stream *xdr, const char *name,
+			    struct compound_hdr *hdr)
+{
+	encode_op_hdr(xdr, OP_GETXATTR, decode_getxattr_maxsz, hdr);
+	encode_string(xdr, strlen(name), name);
+}
+
+static int decode_getxattr(struct xdr_stream *xdr,
+			   struct nfs42_getxattrres *res,
+			   struct rpc_rqst *req)
+{
+	int status;
+	__be32 *p;
+	u32 len, rdlen;
+
+	status = decode_op_hdr(xdr, OP_GETXATTR);
+	if (status)
+		return status;
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(!p))
+		return -EIO;
+
+	len = be32_to_cpup(p);
+	if (len > req->rq_rcv_buf.page_len)
+		return -ERANGE;
+
+	res->xattr_len = len;
+
+	if (len > 0) {
+		rdlen = xdr_read_pages(xdr, len);
+		if (rdlen < len)
+			return -EIO;
+	}
+
+	return 0;
+}
+
+static void encode_removexattr(struct xdr_stream *xdr, const char *name,
+			       struct compound_hdr *hdr)
+{
+	encode_op_hdr(xdr, OP_REMOVEXATTR, decode_removexattr_maxsz, hdr);
+	encode_string(xdr, strlen(name), name);
+}
+
+
+static int decode_removexattr(struct xdr_stream *xdr,
+			   struct nfs4_change_info *cinfo)
+{
+	int status;
+
+	status = decode_op_hdr(xdr, OP_REMOVEXATTR);
+	if (status)
+		goto out;
+
+	status = decode_change_info(xdr, cinfo);
+out:
+	return status;
+}
+
+static void encode_listxattrs(struct xdr_stream *xdr,
+			     const struct nfs42_listxattrsargs *arg,
+			     struct compound_hdr *hdr)
+{
+	 __be32 *p;
+
+	encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz + 1, hdr);
+
+	p = reserve_space(xdr, 12);
+	if (unlikely(!p))
+		return;
+
+	p = xdr_encode_hyper(p, arg->cookie);
+	/*
+	 * RFC 8276 says to specify the full max length of the LISTXATTRS
+	 * XDR reply. Count is set to the XDR length of the names array
+	 * plus the EOF marker. So, add the cookie and the names count.
+	 */
+	*p = cpu_to_be32(arg->count + 8 + 4);
+}
+
+static int decode_listxattrs(struct xdr_stream *xdr,
+			    struct nfs42_listxattrsres *res)
+{
+	int status;
+	__be32 *p;
+	u32 count, len, ulen;
+	size_t left, copied;
+	char *buf;
+
+	status = decode_op_hdr(xdr, OP_LISTXATTRS);
+	if (status) {
+		/*
+		 * Special case: for LISTXATTRS, NFS4ERR_TOOSMALL
+		 * should be translated to ERANGE.
+		 */
+		if (status == -ETOOSMALL)
+			status = -ERANGE;
+		goto out;
+	}
+
+	p = xdr_inline_decode(xdr, 8);
+	if (unlikely(!p))
+		return -EIO;
+
+	xdr_decode_hyper(p, &res->cookie);
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(!p))
+		return -EIO;
+
+	left = res->xattr_len;
+	buf = res->xattr_buf;
+
+	count = be32_to_cpup(p);
+	copied = 0;
+
+	/*
+	 * We have asked for enough room to encode the maximum number
+	 * of possible attribute names, so everything should fit.
+	 *
+	 * But, don't rely on that assumption. Just decode entries
+	 * until they don't fit anymore, just in case the server did
+	 * something odd.
+	 */
+	while (count--) {
+		p = xdr_inline_decode(xdr, 4);
+		if (unlikely(!p))
+			return -EIO;
+
+		len = be32_to_cpup(p);
+		if (len > (XATTR_NAME_MAX - XATTR_USER_PREFIX_LEN)) {
+			status = -ERANGE;
+			goto out;
+		}
+
+		p = xdr_inline_decode(xdr, len);
+		if (unlikely(!p))
+			return -EIO;
+
+		ulen = len + XATTR_USER_PREFIX_LEN + 1;
+		if (buf) {
+			if (ulen > left) {
+				status = -ERANGE;
+				goto out;
+			}
+
+			memcpy(buf, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
+			memcpy(buf + XATTR_USER_PREFIX_LEN, p, len);
+
+			buf[ulen - 1] = 0;
+			buf += ulen;
+			left -= ulen;
+		}
+		copied += ulen;
+	}
+
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(!p))
+		return -EIO;
+
+	res->eof = be32_to_cpup(p);
+	res->copied = copied;
+
+out:
+	if (status == -ERANGE && res->xattr_len == XATTR_LIST_MAX)
+		status = -E2BIG;
+
+	return status;
+}
+#endif
+
 /*
  * Encode ALLOCATE request
  */
@@ -876,4 +1082,170 @@ static int nfs4_xdr_dec_layouterror(struct rpc_rqst *rqstp,
 	return status;
 }
 
+#ifdef CONFIG_NFS_V4_XATTR
+static void nfs4_xdr_enc_setxattr(struct rpc_rqst *req, struct xdr_stream *xdr,
+                                const void *data)
+{
+	const struct nfs42_setxattrargs *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_setxattr(xdr, args, &hdr);
+	encode_nops(&hdr);
+}
+
+static int nfs4_xdr_dec_setxattr(struct rpc_rqst *req, struct xdr_stream *xdr,
+                                 void *data)
+{
+	struct nfs42_setxattrres *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, req);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+
+	status = decode_setxattr(xdr, &res->cinfo);
+out:
+	return status;
+}
+
+static void nfs4_xdr_enc_getxattr(struct rpc_rqst *req, struct xdr_stream *xdr,
+                                const void *data)
+{
+	const struct nfs42_getxattrargs *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+	size_t plen;
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_getxattr(xdr, args->xattr_name, &hdr);
+
+	plen = args->xattr_len ? args->xattr_len : XATTR_SIZE_MAX;
+
+	rpc_prepare_reply_pages(req, args->xattr_pages, 0, plen,
+	    hdr.replen);
+	req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
+
+	encode_nops(&hdr);
+}
+
+static int nfs4_xdr_dec_getxattr(struct rpc_rqst *rqstp,
+                                 struct xdr_stream *xdr,
+                                 void *data)
+{
+	struct nfs42_getxattrres *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_getxattr(xdr, res, rqstp);
+out:
+        return status;
+}
+
+static void nfs4_xdr_enc_listxattrs(struct rpc_rqst *req,
+				    struct xdr_stream *xdr,
+				    const void *data)
+{
+	const struct nfs42_listxattrsargs *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_listxattrs(xdr, args, &hdr);
+
+	rpc_prepare_reply_pages(req, args->xattr_pages, 0, args->count,
+	    hdr.replen);
+	req->rq_rcv_buf.flags |= XDRBUF_SPARSE_PAGES;
+
+	encode_nops(&hdr);
+}
+
+static int nfs4_xdr_dec_listxattrs(struct rpc_rqst *rqstp,
+                                 struct xdr_stream *xdr,
+                                 void *data)
+{
+	struct nfs42_listxattrsres *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	xdr_set_scratch_buffer(xdr, page_address(res->scratch), PAGE_SIZE);
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, rqstp);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+	status = decode_listxattrs(xdr, res);
+out:
+        return status;
+}
+
+static void nfs4_xdr_enc_removexattr(struct rpc_rqst *req,
+				     struct xdr_stream *xdr, const void *data)
+{
+	const struct nfs42_removexattrargs *args = data;
+	struct compound_hdr hdr = {
+		.minorversion = nfs4_xdr_minorversion(&args->seq_args),
+	};
+
+	encode_compound_hdr(xdr, req, &hdr);
+	encode_sequence(xdr, &args->seq_args, &hdr);
+	encode_putfh(xdr, args->fh, &hdr);
+	encode_removexattr(xdr, args->xattr_name, &hdr);
+	encode_nops(&hdr);
+}
+
+static int nfs4_xdr_dec_removexattr(struct rpc_rqst *req,
+				      struct xdr_stream *xdr,
+				      void *data)
+{
+	struct nfs42_removexattrres *res = data;
+	struct compound_hdr hdr;
+	int status;
+
+	status = decode_compound_hdr(xdr, &hdr);
+	if (status)
+		goto out;
+	status = decode_sequence(xdr, &res->seq_res, req);
+	if (status)
+		goto out;
+	status = decode_putfh(xdr);
+	if (status)
+		goto out;
+
+	status = decode_removexattr(xdr, &res->cinfo);
+out:
+	return status;
+}
+#endif
 #endif /* __LINUX_FS_NFS_NFS4_2XDR_H */
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 00fd5732c59d..7b9c39e66d15 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -7483,6 +7483,8 @@ static struct {
 	{ NFS4ERR_SYMLINK,	-ELOOP		},
 	{ NFS4ERR_OP_ILLEGAL,	-EOPNOTSUPP	},
 	{ NFS4ERR_DEADLOCK,	-EDEADLK	},
+	{ NFS4ERR_NOXATTR,	-ENODATA	},
+	{ NFS4ERR_XATTR2BIG,	-E2BIG		},
 	{ -1,			-EIO		}
 };
 
@@ -7610,6 +7612,12 @@ const struct rpc_procinfo nfs4_procedures[] = {
 	PROC42(OFFLOAD_CANCEL,	enc_offload_cancel,	dec_offload_cancel),
 	PROC(LOOKUPP,		enc_lookupp,		dec_lookupp),
 	PROC42(LAYOUTERROR,	enc_layouterror,	dec_layouterror),
+#ifdef CONFIG_NFS_V4_XATTR
+	PROC42(GETXATTR,	enc_getxattr,		dec_getxattr),
+	PROC42(SETXATTR,	enc_setxattr,		dec_setxattr),
+	PROC42(LISTXATTRS,	enc_listxattrs,		dec_listxattrs),
+	PROC42(REMOVEXATTR,	enc_removexattr,	dec_removexattr),
+#endif
 };
 
 static unsigned int nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 11/35] nfs: define nfs_access_get_cached function
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (8 preceding siblings ...)
  2019-08-27 15:34 ` [RFC PATCH 10/35] NFSv4.2: add client side XDR handling for extended attributes Frank van der Linden
@ 2019-08-27 15:46 ` Frank van der Linden
  2019-08-27 16:01 ` [RFC PATCH 12/35] NFSv4.2: query the extended attribute access bits Frank van der Linden
                   ` (25 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-27 15:46 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

The only consumer of nfs_access_get_cached_rcu and nfs_access_cached
calls these static funtions in order to first try RCU access, and
then locked access.

Combine them in to a single function, and call that. Make this function
available to the rest of the NFS code.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/dir.c           | 20 ++++++++++++++++----
 include/linux/nfs_fs.h |  2 ++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 0adfd8840110..e7f4c25de362 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2285,7 +2285,7 @@ static struct nfs_access_entry *nfs_access_search_rbtree(struct inode *inode, co
 	return NULL;
 }
 
-static int nfs_access_get_cached(struct inode *inode, const struct cred *cred, struct nfs_access_entry *res, bool may_block)
+static int nfs_access_get_cached_locked(struct inode *inode, const struct cred *cred, struct nfs_access_entry *res, bool may_block)
 {
 	struct nfs_inode *nfsi = NFS_I(inode);
 	struct nfs_access_entry *cache;
@@ -2358,6 +2358,20 @@ static int nfs_access_get_cached_rcu(struct inode *inode, const struct cred *cre
 	return err;
 }
 
+int nfs_access_get_cached(struct inode *inode, const struct cred *cred, struct
+nfs_access_entry *res, bool may_block)
+{
+	int status;
+
+	status = nfs_access_get_cached_rcu(inode, cred, res);
+	if (status != 0)
+		status = nfs_access_get_cached_locked(inode, cred, res,
+		    may_block);
+
+	return status;
+}
+EXPORT_SYMBOL_GPL(nfs_access_get_cached);
+
 static void nfs_access_add_rbtree(struct inode *inode, struct nfs_access_entry *set)
 {
 	struct nfs_inode *nfsi = NFS_I(inode);
@@ -2472,9 +2486,7 @@ static int nfs_do_access(struct inode *inode, const struct cred *cred, int mask)
 
 	trace_nfs_access_enter(inode);
 
-	status = nfs_access_get_cached_rcu(inode, cred, &cache);
-	if (status != 0)
-		status = nfs_access_get_cached(inode, cred, &cache, may_block);
+	status = nfs_access_get_cached(inode, cred, &cache, may_block);
 	if (status == 0)
 		goto out_cached;
 
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index e87c894d1960..dec76ec9808c 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -497,6 +497,8 @@ extern int nfs_instantiate(struct dentry *dentry, struct nfs_fh *fh,
 			struct nfs_fattr *fattr, struct nfs4_label *label);
 extern int nfs_may_open(struct inode *inode, const struct cred *cred, int openflags);
 extern void nfs_access_zap_cache(struct inode *inode);
+extern int nfs_access_get_cached(struct inode *inode, const struct cred *cred, struct nfs_access_entry *res,
+				 bool may_block);
 
 /*
  * linux/fs/nfs/symlink.c
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 12/35] NFSv4.2: query the extended attribute access bits
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (9 preceding siblings ...)
  2019-08-27 15:46 ` [RFC PATCH 11/35] nfs: define nfs_access_get_cached function Frank van der Linden
@ 2019-08-27 16:01 ` Frank van der Linden
  2019-08-27 22:51 ` [RFC PATCH 13/35] nfs: modify update_changeattr to deal with regular files Frank van der Linden
                   ` (24 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-27 16:01 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

RFC 8276 defines seperate ACCESS bits for extended attribute checking.
Query them in nfs_do_access and opendata.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/dir.c      | 4 ++++
 fs/nfs/nfs4proc.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e7f4c25de362..78bc808addc9 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2498,6 +2498,10 @@ static int nfs_do_access(struct inode *inode, const struct cred *cred, int mask)
 	 * Determine which access bits we want to ask for...
 	 */
 	cache.mask = NFS_ACCESS_READ | NFS_ACCESS_MODIFY | NFS_ACCESS_EXTEND;
+	if (nfs_server_capable(inode, NFS_CAP_XATTR)) {
+		cache.mask |= NFS_ACCESS_XAREAD | NFS_ACCESS_XAWRITE |
+		    NFS_ACCESS_XALIST;
+	}
 	if (S_ISDIR(inode->i_mode))
 		cache.mask |= NFS_ACCESS_DELETE | NFS_ACCESS_LOOKUP;
 	else
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index dad22450546d..029b07fef37f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1313,6 +1313,12 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 				NFS4_ACCESS_MODIFY |
 				NFS4_ACCESS_EXTEND |
 				NFS4_ACCESS_EXECUTE;
+#ifdef CONFIG_NFS_V4_XATTR
+			if (server->caps & NFS_CAP_XATTR)
+				p->o_arg.access |= NFS4_ACCESS_XAREAD |
+				    NFS4_ACCESS_XAWRITE |
+				    NFS4_ACCESS_XALIST;
+#endif
 		}
 	}
 	p->o_arg.clientid = server->nfs_client->cl_clientid;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 13/35] nfs: modify update_changeattr to deal with regular files
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (10 preceding siblings ...)
  2019-08-27 16:01 ` [RFC PATCH 12/35] NFSv4.2: query the extended attribute access bits Frank van der Linden
@ 2019-08-27 22:51 ` Frank van der Linden
  2019-08-30 22:48 ` [RFC PATCH 14/35] nfs: define and use the NFS_INO_INVALID_XATTR flag Frank van der Linden
                   ` (23 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-27 22:51 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Until now, change attributes in change_info form were only returned by
directory operations. However, they are also used for the RFC 8276
extended attribute operations, which work on both directories
and regular files.  Modify update_changeattr to deal:

* Rename it to nfs4_update_changeattr and make it non-static.
* Don't always use INO_INVALID_DATA, this isn't needed for a
  directory that only had its extended attributes changed by us.
* Existing callers now always pass in INO_INVALID_DATA.

For the current callers of this function, behavior is unchanged.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs4_fs.h  |  5 ++++
 fs/nfs/nfs4proc.c | 70 +++++++++++++++++++++++++++++------------------
 2 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 3564da1ba8a1..f684dcdc38bd 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -314,6 +314,11 @@ extern int nfs4_set_rw_stateid(nfs4_stateid *stateid,
 
 extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
 		struct nfs_fsinfo *fsinfo);
+extern void nfs4_update_changeattr(struct inode *dir,
+				   struct nfs4_change_info *cinfo,
+				   unsigned long timestamp,
+				   unsigned long cache_validity);
+
 #if defined(CONFIG_NFS_V4_1)
 extern int nfs41_sequence_done(struct rpc_task *, struct nfs4_sequence_res *);
 extern int nfs4_proc_create_session(struct nfs_client *, const struct cred *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 029b07fef37f..a8a6ddb34ad7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1130,37 +1130,48 @@ nfs4_dec_nlink_locked(struct inode *inode)
 }
 
 static void
-update_changeattr_locked(struct inode *dir, struct nfs4_change_info *cinfo,
+nfs4_update_changeattr_locked(struct inode *inode,
+		struct nfs4_change_info *cinfo,
 		unsigned long timestamp, unsigned long cache_validity)
 {
-	struct nfs_inode *nfsi = NFS_I(dir);
+	struct nfs_inode *nfsi = NFS_I(inode);
 
 	nfsi->cache_validity |= NFS_INO_INVALID_CTIME
 		| NFS_INO_INVALID_MTIME
-		| NFS_INO_INVALID_DATA
 		| cache_validity;
-	if (cinfo->atomic && cinfo->before == inode_peek_iversion_raw(dir)) {
+
+	if (cinfo->atomic && cinfo->before == inode_peek_iversion_raw(inode)) {
 		nfsi->cache_validity &= ~NFS_INO_REVAL_PAGECACHE;
 		nfsi->attrtimeo_timestamp = jiffies;
 	} else {
-		nfs_force_lookup_revalidate(dir);
-		if (cinfo->before != inode_peek_iversion_raw(dir))
+		if (S_ISDIR(inode->i_mode)) {
+			nfsi->cache_validity |= NFS_INO_INVALID_DATA;
+			nfs_force_lookup_revalidate(inode);
+		} else {
+			if (!NFS_PROTO(inode)->have_delegation(inode,
+							       FMODE_READ))
+				nfsi->cache_validity |= NFS_INO_REVAL_PAGECACHE;
+		}
+
+		if (cinfo->before != inode_peek_iversion_raw(inode))
 			nfsi->cache_validity |= NFS_INO_INVALID_ACCESS |
-				NFS_INO_INVALID_ACL;
+						NFS_INO_INVALID_ACL;
 	}
-	inode_set_iversion_raw(dir, cinfo->after);
+	inode_set_iversion_raw(inode, cinfo->after);
 	nfsi->read_cache_jiffies = timestamp;
 	nfsi->attr_gencount = nfs_inc_attr_generation_counter();
 	nfsi->cache_validity &= ~NFS_INO_INVALID_CHANGE;
-	nfs_fscache_invalidate(dir);
+
+	if (nfsi->cache_validity & NFS_INO_INVALID_DATA)
+		nfs_fscache_invalidate(inode);
 }
 
-static void
-update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo,
+void
+nfs4_update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo,
 		unsigned long timestamp, unsigned long cache_validity)
 {
 	spin_lock(&dir->i_lock);
-	update_changeattr_locked(dir, cinfo, timestamp, cache_validity);
+	nfs4_update_changeattr_locked(dir, cinfo, timestamp, cache_validity);
 	spin_unlock(&dir->i_lock);
 }
 
@@ -2620,8 +2631,9 @@ static int _nfs4_proc_open(struct nfs4_opendata *data,
 			data->file_created = true;
 		if (data->file_created ||
 		    inode_peek_iversion_raw(dir) != o_res->cinfo.after)
-			update_changeattr(dir, &o_res->cinfo,
-					o_res->f_attr->time_start, 0);
+			nfs4_update_changeattr(dir, &o_res->cinfo,
+					o_res->f_attr->time_start,
+					NFS_INO_INVALID_DATA);
 	}
 	if ((o_res->rflags & NFS4_OPEN_RESULT_LOCKTYPE_POSIX) == 0)
 		server->caps &= ~NFS_CAP_POSIX_LOCK;
@@ -4440,7 +4452,8 @@ _nfs4_proc_remove(struct inode *dir, const struct qstr *name, u32 ftype)
 	status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, &res.seq_res, 1);
 	if (status == 0) {
 		spin_lock(&dir->i_lock);
-		update_changeattr_locked(dir, &res.cinfo, timestamp, 0);
+		nfs4_update_changeattr_locked(dir, &res.cinfo, timestamp,
+					      NFS_INO_INVALID_DATA);
 		/* Removing a directory decrements nlink in the parent */
 		if (ftype == NF4DIR && dir->i_nlink > 2)
 			nfs4_dec_nlink_locked(dir);
@@ -4524,8 +4537,9 @@ static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir)
 				    &data->timeout) == -EAGAIN)
 		return 0;
 	if (task->tk_status == 0)
-		update_changeattr(dir, &res->cinfo,
-				res->dir_attr->time_start, 0);
+		nfs4_update_changeattr(dir, &res->cinfo,
+				res->dir_attr->time_start,
+				NFS_INO_INVALID_DATA);
 	return 1;
 }
 
@@ -4569,16 +4583,18 @@ static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
 	if (task->tk_status == 0) {
 		if (new_dir != old_dir) {
 			/* Note: If we moved a directory, nlink will change */
-			update_changeattr(old_dir, &res->old_cinfo,
+			nfs4_update_changeattr(old_dir, &res->old_cinfo,
 					res->old_fattr->time_start,
-					NFS_INO_INVALID_OTHER);
-			update_changeattr(new_dir, &res->new_cinfo,
+					NFS_INO_INVALID_OTHER |
+					    NFS_INO_INVALID_DATA);
+			nfs4_update_changeattr(new_dir, &res->new_cinfo,
 					res->new_fattr->time_start,
-					NFS_INO_INVALID_OTHER);
+					NFS_INO_INVALID_OTHER |
+					    NFS_INO_INVALID_DATA);
 		} else
-			update_changeattr(old_dir, &res->old_cinfo,
+			nfs4_update_changeattr(old_dir, &res->old_cinfo,
 					res->old_fattr->time_start,
-					0);
+					NFS_INO_INVALID_DATA);
 	}
 	return 1;
 }
@@ -4619,7 +4635,8 @@ static int _nfs4_proc_link(struct inode *inode, struct inode *dir, const struct
 
 	status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1);
 	if (!status) {
-		update_changeattr(dir, &res.cinfo, res.fattr->time_start, 0);
+		nfs4_update_changeattr(dir, &res.cinfo, res.fattr->time_start,
+				       NFS_INO_INVALID_DATA);
 		status = nfs_post_op_update_inode(inode, res.fattr);
 		if (!status)
 			nfs_setsecurity(inode, res.fattr, res.label);
@@ -4697,8 +4714,9 @@ static int nfs4_do_create(struct inode *dir, struct dentry *dentry, struct nfs4_
 				    &data->arg.seq_args, &data->res.seq_res, 1);
 	if (status == 0) {
 		spin_lock(&dir->i_lock);
-		update_changeattr_locked(dir, &data->res.dir_cinfo,
-				data->res.fattr->time_start, 0);
+		nfs4_update_changeattr_locked(dir, &data->res.dir_cinfo,
+				data->res.fattr->time_start,
+				NFS_INO_INVALID_DATA);
 		/* Creating a directory bumps nlink in the parent */
 		if (data->arg.ftype == NF4DIR)
 			nfs4_inc_nlink_locked(dir);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 14/35] nfs: define and use the NFS_INO_INVALID_XATTR flag
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (11 preceding siblings ...)
  2019-08-27 22:51 ` [RFC PATCH 13/35] nfs: modify update_changeattr to deal with regular files Frank van der Linden
@ 2019-08-30 22:48 ` Frank van der Linden
  2019-08-30 22:56 ` [RFC PATCH 15/35] nfs: make the buf_to_pages_noslab function available to the nfs code Frank van der Linden
                   ` (22 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 22:48 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Define the NFS_INO_INVALID_XATTR flag, to be used for the NFSv4.2 xattr
cache, and use it where appropriate.

No functional change as yet.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/inode.c         | 7 ++++++-
 fs/nfs/nfs4proc.c      | 3 ++-
 fs/nfs/nfstrace.h      | 3 ++-
 include/linux/nfs_fs.h | 1 +
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 2a03bfeec10a..1632a97a6dfc 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -205,7 +205,8 @@ static void nfs_set_cache_invalid(struct inode *inode, unsigned long flags)
 			flags &= ~NFS_INO_INVALID_OTHER;
 		flags &= ~(NFS_INO_INVALID_CHANGE
 				| NFS_INO_INVALID_SIZE
-				| NFS_INO_REVAL_PAGECACHE);
+				| NFS_INO_REVAL_PAGECACHE
+				| NFS_INO_INVALID_XATTR);
 	}
 
 	if (inode->i_mapping->nrpages == 0)
@@ -535,6 +536,8 @@ nfs_fhget(struct super_block *sb, struct nfs_fh *fh, struct nfs_fattr *fattr, st
 			inode->i_gid = fattr->gid;
 		else if (nfs_server_capable(inode, NFS_CAP_OWNER_GROUP))
 			nfs_set_cache_invalid(inode, NFS_INO_INVALID_OTHER);
+		if (nfs_server_capable(inode, NFS_CAP_XATTR))
+			nfs_set_cache_invalid(inode, NFS_INO_INVALID_XATTR);
 		if (fattr->valid & NFS_ATTR_FATTR_BLOCKS_USED)
 			inode->i_blocks = fattr->du.nfs2.blocks;
 		if (fattr->valid & NFS_ATTR_FATTR_SPACE_USED) {
@@ -1359,6 +1362,8 @@ static void nfs_wcc_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 		inode_set_iversion_raw(inode, fattr->change_attr);
 		if (S_ISDIR(inode->i_mode))
 			nfs_set_cache_invalid(inode, NFS_INO_INVALID_DATA);
+		else if (nfs_server_capable(inode, NFS_CAP_XATTR))
+			nfs_set_cache_invalid(inode, NFS_INO_INVALID_XATTR);
 	}
 	/* If we have atomic WCC data, we may update some attributes */
 	ts = timespec64_to_timespec(inode->i_ctime);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a8a6ddb34ad7..30dd92d6e759 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1155,7 +1155,8 @@ nfs4_update_changeattr_locked(struct inode *inode,
 
 		if (cinfo->before != inode_peek_iversion_raw(inode))
 			nfsi->cache_validity |= NFS_INO_INVALID_ACCESS |
-						NFS_INO_INVALID_ACL;
+						NFS_INO_INVALID_ACL |
+						NFS_INO_INVALID_XATTR;
 	}
 	inode_set_iversion_raw(inode, cinfo->after);
 	nfsi->read_cache_jiffies = timestamp;
diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
index 976d4089e267..094b7d59866d 100644
--- a/fs/nfs/nfstrace.h
+++ b/fs/nfs/nfstrace.h
@@ -59,7 +59,8 @@ TRACE_DEFINE_ENUM(NFS_INO_INVALID_OTHER);
 			{ NFS_INO_INVALID_CTIME, "INVALID_CTIME" }, \
 			{ NFS_INO_INVALID_MTIME, "INVALID_MTIME" }, \
 			{ NFS_INO_INVALID_SIZE, "INVALID_SIZE" }, \
-			{ NFS_INO_INVALID_OTHER, "INVALID_OTHER" })
+			{ NFS_INO_INVALID_OTHER, "INVALID_OTHER" }, \
+			{ NFS_INO_INVALID_XATTR, "INVALID_XATTR" })
 
 TRACE_DEFINE_ENUM(NFS_INO_ADVISE_RDPLUS);
 TRACE_DEFINE_ENUM(NFS_INO_STALE);
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index dec76ec9808c..85719db3eeea 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -228,6 +228,7 @@ struct nfs4_copy_state {
 #define NFS_INO_INVALID_OTHER	BIT(12)		/* other attrs are invalid */
 #define NFS_INO_DATA_INVAL_DEFER	\
 				BIT(13)		/* Deferred cache invalidation */
+#define NFS_INO_INVALID_XATTR	BIT(14)		/* xattrs are invalid */
 
 #define NFS_INO_INVALID_ATTR	(NFS_INO_INVALID_CHANGE \
 		| NFS_INO_INVALID_CTIME \
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 15/35] nfs: make the buf_to_pages_noslab function available to the nfs code
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (12 preceding siblings ...)
  2019-08-30 22:48 ` [RFC PATCH 14/35] nfs: define and use the NFS_INO_INVALID_XATTR flag Frank van der Linden
@ 2019-08-30 22:56 ` Frank van der Linden
  2019-08-30 23:15 ` [RFC PATCH 16/35] NFSv4.2: add the extended attribute proc functions Frank van der Linden
                   ` (21 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 22:56 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Make the buf_to_pages_noslab function available to the rest of the NFS
code. Rename it to nfs4_buf_to_pages_noslab to be consistent.

This will be used later in the NFSv4.2 xattr code.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/internal.h | 3 +++
 fs/nfs/nfs4proc.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index a1464bf8d178..75645d9ff10b 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -306,6 +306,9 @@ extern const u32 nfs42_maxlistxattrs_overhead;
 extern const struct rpc_procinfo nfs4_procedures[];
 #endif
 
+extern int nfs4_buf_to_pages_noslab(const void *buf, size_t buflen,
+				    struct page **pages);
+
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 extern struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags);
 static inline struct nfs4_label *
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 30dd92d6e759..19d8fd087bf8 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5453,7 +5453,7 @@ static inline int nfs4_server_supports_acls(struct nfs_server *server)
  */
 #define NFS4ACL_MAXPAGES DIV_ROUND_UP(XATTR_SIZE_MAX, PAGE_SIZE)
 
-static int buf_to_pages_noslab(const void *buf, size_t buflen,
+int nfs4_buf_to_pages_noslab(const void *buf, size_t buflen,
 		struct page **pages)
 {
 	struct page *newpage, **spages;
@@ -5687,7 +5687,7 @@ static int __nfs4_proc_set_acl(struct inode *inode, const void *buf, size_t bufl
 		return -EOPNOTSUPP;
 	if (npages > ARRAY_SIZE(pages))
 		return -ERANGE;
-	i = buf_to_pages_noslab(buf, buflen, arg.acl_pages);
+	i = nfs4_buf_to_pages_noslab(buf, buflen, arg.acl_pages);
 	if (i < 0)
 		return i;
 	nfs4_inode_make_writeable(inode);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 16/35] NFSv4.2: add the extended attribute proc functions.
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (13 preceding siblings ...)
  2019-08-30 22:56 ` [RFC PATCH 15/35] nfs: make the buf_to_pages_noslab function available to the nfs code Frank van der Linden
@ 2019-08-30 23:15 ` Frank van der Linden
  2019-08-30 23:31 ` [RFC PATCH 17/35] NFSv4.2: hook in the user extended attribute handlers Frank van der Linden
                   ` (20 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 23:15 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Implement the extended attribute procedures for NFSv4.2 extended
attribute support (RFC 8276).

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs42.h     |   9 ++
 fs/nfs/nfs42proc.c | 242 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 251 insertions(+)

diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
index 935651345be7..96834d172269 100644
--- a/fs/nfs/nfs42.h
+++ b/fs/nfs/nfs42.h
@@ -29,6 +29,15 @@ int nfs42_proc_layouterror(struct pnfs_layout_segment *lseg,
 			   size_t n);
 
 #ifdef CONFIG_NFS_V4_XATTR
+
+ssize_t nfs42_proc_getxattr(struct inode *inode, const char *name,
+			void *buf, size_t buflen);
+int nfs42_proc_setxattr(struct inode *inode, const char *name,
+			const void *buf, size_t buflen, int flags);
+ssize_t nfs42_proc_listxattrs(struct inode *inode, void *buf,
+			  size_t buflen, u64 *cookiep, bool *eofp);
+int nfs42_proc_removexattr(struct inode *inode, const char *name);
+
 /*
  * Maximum XDR buffer size needed for a listxattr buffer of buflen size.
  *
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 5196bfa7894d..1c7081f35401 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -933,3 +933,245 @@ int nfs42_proc_clone(struct file *src_f, struct file *dst_f,
 	nfs_put_lock_context(src_lock);
 	return err;
 }
+
+#ifdef CONFIG_NFS_V4_XATTR
+
+#define NFS4XATTR_MAXPAGES DIV_ROUND_UP(XATTR_SIZE_MAX, PAGE_SIZE)
+
+static int _nfs42_proc_removexattr(struct inode *inode, const char *name)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct nfs42_removexattrargs args = {
+		.fh = NFS_FH(inode),
+		.xattr_name = name,
+	};
+	struct nfs42_removexattrres res;
+	struct rpc_message msg = {
+		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_REMOVEXATTR],
+		.rpc_argp = &args,
+		.rpc_resp = &res,
+	};
+	int ret;
+	unsigned long timestamp = jiffies;
+
+	ret = nfs4_call_sync(server->client, server, &msg, &args.seq_args,
+	    &res.seq_res, 1);
+	if (!ret)
+		nfs4_update_changeattr(inode, &res.cinfo, timestamp,
+		    NFS_INO_INVALID_XATTR);
+
+	return ret;
+}
+
+static int _nfs42_proc_setxattr(struct inode *inode, const char *name,
+				const void *buf, size_t buflen, int flags)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct page *pages[NFS4XATTR_MAXPAGES];
+	struct nfs42_setxattrargs arg = {
+		.fh		= NFS_FH(inode),
+		.xattr_pages	= pages,
+		.xattr_len	= buflen,
+		.xattr_name	= name,
+		.xattr_flags	= flags,
+	};
+	struct nfs42_setxattrres res;
+	struct rpc_message msg = {
+		.rpc_proc	= &nfs4_procedures[NFSPROC4_CLNT_SETXATTR],
+		.rpc_argp	= &arg,
+		.rpc_resp	= &res,
+	};
+	int ret, np;
+	unsigned long timestamp = jiffies;
+
+	if (buflen > server->sxasize)
+		return -ERANGE;
+
+	if (buflen > 0) {
+		np = nfs4_buf_to_pages_noslab(buf, buflen, arg.xattr_pages);
+		if (np < 0)
+			return np;
+	} else
+		np = 0;
+
+	ret = nfs4_call_sync(server->client, server, &msg, &arg.seq_args,
+	    &res.seq_res, 1);
+
+	for (; np > 0; np--)
+		put_page(pages[np - 1]);
+
+	if (!ret)
+		nfs4_update_changeattr(inode, &res.cinfo, timestamp,
+		    NFS_INO_INVALID_XATTR);
+
+	return ret;
+}
+
+static ssize_t _nfs42_proc_getxattr(struct inode *inode, const char *name,
+				void *buf, size_t buflen)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct page *pages[NFS4XATTR_MAXPAGES] = {};
+	struct nfs42_getxattrargs arg = {
+		.fh		= NFS_FH(inode),
+		.xattr_pages	= pages,
+		.xattr_len	= buflen,
+		.xattr_name	= name,
+	};
+	struct nfs42_getxattrres res;
+	struct rpc_message msg = {
+		.rpc_proc	= &nfs4_procedures[NFSPROC4_CLNT_GETXATTR],
+		.rpc_argp	= &arg,
+		.rpc_resp	= &res,
+	};
+	int ret, np;
+
+	ret = nfs4_call_sync(server->client, server, &msg, &arg.seq_args,
+	    &res.seq_res, 0);
+	if (ret < 0)
+		return ret;
+
+	if (buflen) {
+		if (res.xattr_len > buflen)
+			return -ERANGE;
+		_copy_from_pages(buf, pages, 0, res.xattr_len);
+	}
+
+	np = DIV_ROUND_UP(res.xattr_len, PAGE_SIZE);
+	while (--np >= 0)
+		__free_page(pages[np]);
+
+	return res.xattr_len;
+}
+
+static ssize_t _nfs42_proc_listxattrs(struct inode *inode, void *buf,
+				 size_t buflen, u64 *cookiep, bool *eofp)
+{
+	struct nfs_server *server = NFS_SERVER(inode);
+	struct page **pages;
+	struct nfs42_listxattrsargs arg = {
+		.fh		= NFS_FH(inode),
+		.cookie		= *cookiep,
+	};
+	struct nfs42_listxattrsres res = {
+		.eof = false,
+		.xattr_buf = buf,
+		.xattr_len = buflen,
+	};
+	struct rpc_message msg = {
+		.rpc_proc	= &nfs4_procedures[NFSPROC4_CLNT_LISTXATTRS],
+		.rpc_argp	= &arg,
+		.rpc_resp	= &res,
+	};
+	u32 xdrlen;
+	int ret, np;
+
+
+	res.scratch = alloc_page(GFP_KERNEL);
+	if (!res.scratch)
+		return -ENOMEM;
+
+	xdrlen = nfs42_listxattr_xdrsize(buflen);
+	if (xdrlen > server->lxasize)
+		xdrlen = server->lxasize;
+	np = xdrlen / PAGE_SIZE + 1;
+
+	pages = kzalloc(np * sizeof (struct page *), GFP_KERNEL);
+	if (pages == NULL) {
+		__free_page(res.scratch);
+		return -ENOMEM;
+	}
+
+	arg.xattr_pages = pages;
+	arg.count = xdrlen;
+
+	ret = nfs4_call_sync(server->client, server, &msg, &arg.seq_args,
+	    &res.seq_res, 0);
+
+	if (ret >= 0) {
+		ret = res.copied;
+		*cookiep = res.cookie;
+		*eofp = res.eof;
+	}
+
+	while (--np >= 0) {
+		if (pages[np])
+			__free_page(pages[np]);
+	}
+
+	__free_page(res.scratch);
+	kfree(pages);
+
+	return ret;
+
+}
+
+ssize_t nfs42_proc_getxattr(struct inode *inode, const char *name,
+			      void *buf, size_t buflen)
+{
+	struct nfs4_exception exception = { };
+	ssize_t err;
+
+	do {
+		err = _nfs42_proc_getxattr(inode, name, buf, buflen);
+		if (err >= 0)
+			break;
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+
+	return err;
+}
+
+int nfs42_proc_setxattr(struct inode *inode, const char *name,
+			      const void *buf, size_t buflen, int flags)
+{
+	struct nfs4_exception exception = { };
+	int err;
+
+	do {
+		err = _nfs42_proc_setxattr(inode, name, buf, buflen, flags);
+		if (!err)
+			break;
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+
+	return err;
+}
+
+ssize_t nfs42_proc_listxattrs(struct inode *inode, void *buf,
+			      size_t buflen, u64 *cookiep, bool *eofp)
+{
+	struct nfs4_exception exception = { };
+	ssize_t err;
+
+	do {
+		err = _nfs42_proc_listxattrs(inode, buf, buflen,
+		    cookiep, eofp);
+		if (err >= 0)
+			break;
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+
+	return err;
+}
+
+int nfs42_proc_removexattr(struct inode *inode, const char *name)
+{
+	struct nfs4_exception exception = { };
+	int err;
+
+	do {
+		err = _nfs42_proc_removexattr(inode, name);
+		if (!err)
+			break;
+		err = nfs4_handle_exception(NFS_SERVER(inode), err,
+				&exception);
+	} while (exception.retry);
+
+	return err;
+}
+
+#endif /* CONFIG_NFS_V4_XATTR */
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 17/35] NFSv4.2: hook in the user extended attribute handlers
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (14 preceding siblings ...)
  2019-08-30 23:15 ` [RFC PATCH 16/35] NFSv4.2: add the extended attribute proc functions Frank van der Linden
@ 2019-08-30 23:31 ` Frank van der Linden
  2019-08-30 23:38 ` [RFC PATCH 18/35] NFSv4.2: add client side xattr caching functions Frank van der Linden
                   ` (19 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 23:31 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Now that all the lower level code is there to make the RPC calls, hook
it in to the xattr handlers and the listxattr entry point, to make them
available.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs4proc.c | 123 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 121 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 19d8fd087bf8..36cca076ccdb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -66,6 +66,7 @@
 #include "nfs4idmap.h"
 #include "nfs4session.h"
 #include "fscache.h"
+#include "nfs42.h"
 
 #include "nfs4trace.h"
 
@@ -7318,6 +7319,103 @@ nfs4_listxattr_nfs4_label(struct inode *inode, char *list, size_t list_len)
 
 #endif
 
+#ifdef CONFIG_NFS_V4_XATTR
+static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler,
+				    struct dentry *unused, struct inode *inode,
+				    const char *key, const void *buf,
+				    size_t buflen, int flags)
+{
+	struct nfs_access_entry cache;
+
+	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
+		return -EOPNOTSUPP;
+
+	/*
+	 * There is no mapping from the MAY_* flags to the NFS_ACCESS_XA*
+	 * flags right now. Handling of xattr operations use the normal
+	 * file read/write permissions.
+	 *
+	 * Just in case the server has other ideas (which RFC 8276 allows),
+	 * do a cached access check for the XA* flags to possibly avoid
+	 * doing an RPC and getting EACCES back.
+	 */
+	if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) {
+		if (!(cache.mask & NFS_ACCESS_XAWRITE))
+			return -EACCES;
+	}
+
+	if (buf == NULL)
+		return nfs42_proc_removexattr(inode, key);
+	else
+		return nfs42_proc_setxattr(inode, key, buf, buflen, flags);
+}
+
+static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler,
+				    struct dentry *unused, struct inode *inode,
+				    const char *key, void *buf, size_t buflen)
+{
+	struct nfs_access_entry cache;
+
+	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
+		return -EOPNOTSUPP;
+
+	if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) {
+		if (!(cache.mask & NFS_ACCESS_XAREAD))
+			return -EACCES;
+	}
+
+	return nfs42_proc_getxattr(inode, key, buf, buflen);
+}
+
+static ssize_t
+nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
+{
+	u64 cookie;
+	bool eof;
+	int ret, size;
+	char *buf;
+	size_t buflen;
+	struct nfs_access_entry cache;
+
+	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
+		return 0;
+
+	if (!nfs_access_get_cached(inode, current_cred(), &cache, true)) {
+		if (!(cache.mask & NFS_ACCESS_XALIST))
+			return 0;
+	}
+
+	cookie = 0;
+	eof = false;
+	buflen = list_len ? list_len : XATTR_LIST_MAX;
+	buf = list_len ? list : NULL;
+	size = 0;
+
+	while (!eof) {
+		ret = nfs42_proc_listxattrs(inode, buf, buflen,
+		    &cookie, &eof);
+		if (ret < 0) {
+			return ret;
+		}
+		if (list_len) {
+			buf += ret;
+			buflen -= ret;
+		}
+		size += ret;
+	}
+
+	return size;
+}
+
+#else
+
+static ssize_t
+nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
+{
+	return 0;
+}
+#endif /* CONFIG_NFS_V4_XATTR */
+
 /*
  * nfs_fhget will use either the mounted_on_fileid or the fileid
  */
@@ -9886,7 +9984,7 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
 
 static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
 {
-	ssize_t error, error2;
+	ssize_t error, error2, error3;
 
 	error = generic_listxattr(dentry, list, size);
 	if (error < 0)
@@ -9899,7 +9997,17 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
 	error2 = nfs4_listxattr_nfs4_label(d_inode(dentry), list, size);
 	if (error2 < 0)
 		return error2;
-	return error + error2;
+
+	if (list) {
+		list += error2;
+		size -= error2;
+	}
+
+	error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, size);
+	if (error3 < 0)
+		return error3;
+
+	return error + error2 + error3;
 }
 
 static const struct inode_operations nfs4_dir_inode_operations = {
@@ -9987,10 +10095,21 @@ static const struct xattr_handler nfs4_xattr_nfs4_acl_handler = {
 	.set	= nfs4_xattr_set_nfs4_acl,
 };
 
+#ifdef CONFIG_NFS_V4_XATTR
+static const struct xattr_handler nfs4_xattr_nfs4_user_handler = {
+	.prefix	= XATTR_USER_PREFIX,
+	.get	= nfs4_xattr_get_nfs4_user,
+	.set	= nfs4_xattr_set_nfs4_user,
+};
+#endif
+
 const struct xattr_handler *nfs4_xattr_handlers[] = {
 	&nfs4_xattr_nfs4_acl_handler,
 #ifdef CONFIG_NFS_V4_SECURITY_LABEL
 	&nfs4_xattr_nfs4_label_handler,
+#endif
+#ifdef CONFIG_NFS_V4_XATTR
+	&nfs4_xattr_nfs4_user_handler,
 #endif
 	NULL
 };
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 18/35] NFSv4.2: add client side xattr caching functions
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (15 preceding siblings ...)
  2019-08-30 23:31 ` [RFC PATCH 17/35] NFSv4.2: hook in the user extended attribute handlers Frank van der Linden
@ 2019-08-30 23:38 ` Frank van der Linden
  2019-08-30 23:45 ` [RFC PATCH 19/35] NFSv4.2: call the xattr cache functions Frank van der Linden
                   ` (18 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 23:38 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Implement client side caching for NFSv4.2 extended attributes.

[note: this currently does nothing]

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/Makefile             |  1 +
 fs/nfs/internal.h           | 16 +++++++++
 fs/nfs/nfs42xattr.c         | 72 +++++++++++++++++++++++++++++++++++++
 include/uapi/linux/nfs_fs.h |  1 +
 4 files changed, 90 insertions(+)
 create mode 100644 fs/nfs/nfs42xattr.c

diff --git a/fs/nfs/Makefile b/fs/nfs/Makefile
index 34cdeaecccf6..0917598db270 100644
--- a/fs/nfs/Makefile
+++ b/fs/nfs/Makefile
@@ -31,6 +31,7 @@ nfsv4-$(CONFIG_NFS_USE_LEGACY_DNS) += cache_lib.o
 nfsv4-$(CONFIG_SYSCTL)	+= nfs4sysctl.o
 nfsv4-$(CONFIG_NFS_V4_1)	+= pnfs.o pnfs_dev.o pnfs_nfs.o
 nfsv4-$(CONFIG_NFS_V4_2)	+= nfs42proc.o
+nfsv4-$(CONFIG_NFS_V4_XATTR)	+= nfs42xattr.o
 
 obj-$(CONFIG_PNFS_FILE_LAYOUT) += filelayout/
 obj-$(CONFIG_PNFS_BLOCK) += blocklayout/
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 75645d9ff10b..558b9c8ddfbf 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -583,6 +583,22 @@ extern void nfs4_test_session_trunk(struct rpc_clnt *clnt,
 				struct rpc_xprt *xprt,
 				void *data);
 
+#ifdef CONFIG_NFS_V4_XATTR
+extern void nfs4_xattr_cache_add(struct inode *, const char *name,
+				 const char *buf, ssize_t buflen);
+extern void nfs4_xattr_cache_remove(struct inode *, const char *name);
+extern ssize_t nfs4_xattr_cache_get(struct inode *inode, const char *name,
+				char *buf, ssize_t buflen);
+extern void nfs4_xattr_cache_set_list(struct inode *, const char *buf,
+				      ssize_t buflen);
+extern ssize_t nfs4_xattr_cache_list(struct inode *, char *buf, ssize_t buflen);
+extern void nfs4_xattr_cache_zap(struct inode *);
+#else
+static inline void nfs4_xattr_cache_zap(struct inode *)
+{
+}
+#endif
+
 static inline struct inode *nfs_igrab_and_active(struct inode *inode)
 {
 	inode = igrab(inode);
diff --git a/fs/nfs/nfs42xattr.c b/fs/nfs/nfs42xattr.c
new file mode 100644
index 000000000000..40c190cfaa75
--- /dev/null
+++ b/fs/nfs/nfs42xattr.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates. All rights reserved.
+ *
+ * User extended attribute client side cache functions.
+ *
+ * Author: Frank van der Linden <fllinden@amazon.com>
+ */
+
+#include <linux/errno.h>
+#include <linux/nfs_fs.h>
+
+#include "nfs4_fs.h"
+#include "internal.h"
+
+#define NFSDBG_FACILITY         NFSDBG_XATTRCACHE
+
+
+/*
+ * Retrieve an xattr from the cache.
+ */
+ssize_t nfs4_xattr_cache_get(struct inode *inode, const char *name, char *buf,
+			 ssize_t buflen)
+{
+	return -ENOENT;
+}
+
+/*
+ * Retrieve a cached list of xattrs from the cache.
+ */
+ssize_t nfs4_xattr_cache_list(struct inode *inode, char *buf, ssize_t buflen)
+{
+	return -ENOENT;
+}
+
+/*
+ * Add an xattr to the cache.
+ *
+ * This also invalidates the xattr list cache.
+ */
+void nfs4_xattr_cache_add(struct inode *inode, const char *name,
+			  const char *buf, ssize_t buflen)
+{
+}
+
+
+/*
+ * Remove an xattr to the cache.
+ *
+ * This also invalidates the xattr list cache.
+ */
+void nfs4_xattr_cache_remove(struct inode *inode, const char *name)
+{
+}
+
+/*
+ * Cache listxattr output, replacing any possible old one.
+ *
+ * Should validate existing cache entries.
+ */
+void nfs4_xattr_cache_set_list(struct inode *inode, const char *buf,
+			       ssize_t buflen)
+{
+}
+
+/*
+ * Zap the entire cache.
+ */
+void nfs4_xattr_cache_zap(struct inode *inode)
+{
+}
diff --git a/include/uapi/linux/nfs_fs.h b/include/uapi/linux/nfs_fs.h
index 7bcc8cd6831d..3afe3767c55d 100644
--- a/include/uapi/linux/nfs_fs.h
+++ b/include/uapi/linux/nfs_fs.h
@@ -56,6 +56,7 @@
 #define NFSDBG_PNFS		0x1000
 #define NFSDBG_PNFS_LD		0x2000
 #define NFSDBG_STATE		0x4000
+#define NFSDBG_XATTRCACHE	0x8000
 #define NFSDBG_ALL		0xFFFF
 
 
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 19/35] NFSv4.2: call the xattr cache functions
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (16 preceding siblings ...)
  2019-08-30 23:38 ` [RFC PATCH 18/35] NFSv4.2: add client side xattr caching functions Frank van der Linden
@ 2019-08-30 23:45 ` Frank van der Linden
  2019-08-30 23:59 ` [RFC PATCH 20/35] nfs: add the NFS_V4_XATTR config option Frank van der Linden
                   ` (17 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 23:45 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Call the xattr caching functions from the nfs42_proc_*xattr functions
to query/add/remove extended attributes.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/nfs4proc.c  | 36 ++++++++++++++++++++++++++++++------
 fs/nfs/nfs4super.c |  1 +
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 36cca076ccdb..6d4758c35760 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7326,6 +7326,7 @@ static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler,
 				    size_t buflen, int flags)
 {
 	struct nfs_access_entry cache;
+	int ret;
 
 	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
 		return -EOPNOTSUPP;
@@ -7344,10 +7345,17 @@ static int nfs4_xattr_set_nfs4_user(const struct xattr_handler *handler,
 			return -EACCES;
 	}
 
-	if (buf == NULL)
-		return nfs42_proc_removexattr(inode, key);
-	else
-		return nfs42_proc_setxattr(inode, key, buf, buflen, flags);
+	if (buf == NULL) {
+		ret = nfs42_proc_removexattr(inode, key);
+		if (!ret)
+			nfs4_xattr_cache_remove(inode, key);
+	} else {
+		ret = nfs42_proc_setxattr(inode, key, buf, buflen, flags);
+		if (!ret)
+			nfs4_xattr_cache_add(inode, key, buf, buflen);
+	}
+
+	return ret;
 }
 
 static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler,
@@ -7355,6 +7363,7 @@ static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler,
 				    const char *key, void *buf, size_t buflen)
 {
 	struct nfs_access_entry cache;
+	ssize_t ret;
 
 	if (!nfs_server_capable(inode, NFS_CAP_XATTR))
 		return -EOPNOTSUPP;
@@ -7364,7 +7373,15 @@ static int nfs4_xattr_get_nfs4_user(const struct xattr_handler *handler,
 			return -EACCES;
 	}
 
-	return nfs42_proc_getxattr(inode, key, buf, buflen);
+	ret = nfs4_xattr_cache_get(inode, key, buf, buflen);
+	if (ret >= 0 || (ret < 0 && ret != -ENOENT))
+		return ret;
+
+	ret = nfs42_proc_getxattr(inode, key, buf, buflen);
+	if (ret >= 0)
+		nfs4_xattr_cache_add(inode, key, buf, ret);
+
+	return ret;
 }
 
 static ssize_t
@@ -7372,7 +7389,7 @@ nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
 {
 	u64 cookie;
 	bool eof;
-	int ret, size;
+	ssize_t ret, size;
 	char *buf;
 	size_t buflen;
 	struct nfs_access_entry cache;
@@ -7385,6 +7402,10 @@ nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
 			return 0;
 	}
 
+	ret = nfs4_xattr_cache_list(inode, list, list_len);
+	if (ret >= 0 || (ret < 0 && ret != -ENOENT))
+		return ret;
+
 	cookie = 0;
 	eof = false;
 	buflen = list_len ? list_len : XATTR_LIST_MAX;
@@ -7404,6 +7425,9 @@ nfs4_listxattr_nfs4_user(struct inode *inode, char *list, size_t list_len)
 		size += ret;
 	}
 
+	if (list_len)
+		nfs4_xattr_cache_set_list(inode, list, size);
+
 	return size;
 }
 
diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
index 04c57066a11a..5c2cb5505ec7 100644
--- a/fs/nfs/nfs4super.c
+++ b/fs/nfs/nfs4super.c
@@ -99,6 +99,7 @@ static void nfs4_evict_inode(struct inode *inode)
 	pnfs_destroy_layout(NFS_I(inode));
 	/* First call standard NFS clear_inode() code */
 	nfs_clear_inode(inode);
+	nfs4_xattr_cache_zap(inode);
 }
 
 /*
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 20/35] nfs: add the NFS_V4_XATTR config option
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (17 preceding siblings ...)
  2019-08-30 23:45 ` [RFC PATCH 19/35] NFSv4.2: call the xattr cache functions Frank van der Linden
@ 2019-08-30 23:59 ` Frank van der Linden
  2019-08-31  2:12 ` [RFC PATCH 21/35] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
                   ` (16 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-30 23:59 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Add the CONFIG_NFS_V4_XATTR option to enable client side user extended
attributes support for NFSv4.2

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfs/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index 295a7a21b774..067689ea9e3f 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -152,6 +152,15 @@ config NFS_V4_1_MIGRATION
           The NFSv4.1 pieces of the Linux NFSv4 migration implementation are
           still experimental.  If you are not an NFSv4 developer, say N here.
 
+config NFS_V4_XATTR
+	bool "NFSv4.2 client support for extended attributes"
+	depends on NFS_V4_2
+	default n
+	help
+	  This option enables the optional NFSv4 client extended attributes
+	  support (https://tools.ietf.org/html/rfc8276). User namespace
+	  xattr support only, as specificed in the RFC.
+
 config NFS_V4_SECURITY_LABEL
 	bool
 	depends on NFS_V4_2 && SECURITY
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 21/35] xattr: modify vfs_{set,remove}xattr for NFS server use
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (18 preceding siblings ...)
  2019-08-30 23:59 ` [RFC PATCH 20/35] nfs: add the NFS_V4_XATTR config option Frank van der Linden
@ 2019-08-31  2:12 ` Frank van der Linden
  2019-08-31 19:00 ` [RFC PATCH 22/35] nfsd: split off the write decode code in to a seperate function Frank van der Linden
                   ` (15 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-31  2:12 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

To be called from the upcoming NFS server xattr code, the vfs_removexattr
and vfs_setxattr need some modifications.

First, they need to grow a _locked variant, since the NFS server code
will call this with i_rwsem held. It needs to do that in fh_lock to be
able to atomically provide the before and after change attributes.

Second, RFC 8276 (NFSv4 extended attribute support) specifies that
delegations should be recalled (8.4.2.4, 8.4.4.4) when a SETXATTR
or REMOVEXATTR operation is performed. So, like with other fs
operations, try to break the delegation. The _locked version of
these operations will not wait for the delegation to be successfully
broken, instead returning an error if it wasn't, so that the NFS
server code can return NFS4ERR_DELAY to the client (similar to
what e.g. vfs_link does).

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/xattr.c            | 63 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/xattr.h |  2 ++
 2 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 90dd78f0eb27..58013bcbc333 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -204,10 +204,10 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name,
 	return error;
 }
 
-
 int
-vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
-		size_t size, int flags)
+__vfs_setxattr_locked(struct dentry *dentry, const char *name,
+		const void *value, size_t size, int flags,
+		struct inode **delegated_inode)
 {
 	struct inode *inode = dentry->d_inode;
 	int error;
@@ -216,15 +216,40 @@ vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 	if (error)
 		return error;
 
-	inode_lock(inode);
 	error = security_inode_setxattr(dentry, name, value, size, flags);
 	if (error)
 		goto out;
 
+	error = try_break_deleg(inode, delegated_inode);
+	if (error)
+		goto out;
+
 	error = __vfs_setxattr_noperm(dentry, name, value, size, flags);
 
 out:
+	return error;
+}
+EXPORT_SYMBOL_GPL(__vfs_setxattr_locked);
+
+int
+vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
+		size_t size, int flags)
+{
+	struct inode *inode = dentry->d_inode;
+	struct inode *delegated_inode = NULL;
+	int error;
+
+retry_deleg:
+	inode_lock(inode);
+	error = __vfs_setxattr_locked(dentry, name, value, size, flags,
+	    &delegated_inode);
 	inode_unlock(inode);
+
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry_deleg;
+	}
 	return error;
 }
 EXPORT_SYMBOL_GPL(vfs_setxattr);
@@ -379,7 +404,8 @@ __vfs_removexattr(struct dentry *dentry, const char *name)
 EXPORT_SYMBOL(__vfs_removexattr);
 
 int
-vfs_removexattr(struct dentry *dentry, const char *name)
+__vfs_removexattr_locked(struct dentry *dentry, const char *name,
+		struct inode **delegated_inode)
 {
 	struct inode *inode = dentry->d_inode;
 	int error;
@@ -388,11 +414,14 @@ vfs_removexattr(struct dentry *dentry, const char *name)
 	if (error)
 		return error;
 
-	inode_lock(inode);
 	error = security_inode_removexattr(dentry, name);
 	if (error)
 		goto out;
 
+	error = try_break_deleg(inode, delegated_inode);
+	if (error)
+		goto out;
+
 	error = __vfs_removexattr(dentry, name);
 
 	if (!error) {
@@ -401,12 +430,32 @@ vfs_removexattr(struct dentry *dentry, const char *name)
 	}
 
 out:
+	return error;
+}
+EXPORT_SYMBOL_GPL(__vfs_removexattr_locked);
+
+int
+vfs_removexattr(struct dentry *dentry, const char *name)
+{
+	struct inode *inode = dentry->d_inode;
+	struct inode *delegated_inode = NULL;
+	int error;
+
+retry_deleg:
+	inode_lock(inode);
+	error = __vfs_removexattr_locked(dentry, name, &delegated_inode);
 	inode_unlock(inode);
+
+	if (delegated_inode) {
+		error = break_deleg_wait(&delegated_inode);
+		if (!error)
+			goto retry_deleg;
+	}
+
 	return error;
 }
 EXPORT_SYMBOL_GPL(vfs_removexattr);
 
-
 /*
  * Extended attribute SET operations
  */
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 6dad031be3c2..3a71ad716da5 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -51,8 +51,10 @@ ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
 int __vfs_setxattr(struct dentry *, struct inode *, const char *, const void *, size_t, int);
 int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int);
+int __vfs_setxattr_locked(struct dentry *, const char *, const void *, size_t, int, struct inode **);
 int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
 int __vfs_removexattr(struct dentry *, const char *);
+int __vfs_removexattr_locked(struct dentry *, const char *, struct inode **);
 int vfs_removexattr(struct dentry *, const char *);
 
 ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 22/35] nfsd: split off the write decode code in to a seperate function
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (19 preceding siblings ...)
  2019-08-31  2:12 ` [RFC PATCH 21/35] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
@ 2019-08-31 19:00 ` Frank van der Linden
  2019-08-31 19:19 ` [RFC PATCH 23/35] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
                   ` (14 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-31 19:00 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

nfs4_decode_write has code to parse incoming XDR write data in to
a kvec head, and a list of pages.

Put this code in to a seperate function, so that it can be used
later by the xattr code, for setxattr. No functional change.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 72 +++++++++++++++++++++++++++--------------------
 1 file changed, 42 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index f2090f7fed42..25cd597f7b4e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -248,6 +248,44 @@ svcxdr_dupstr(struct nfsd4_compoundargs *argp, void *buf, u32 len)
 	return p;
 }
 
+static __be32
+svcxdr_construct_vector(struct nfsd4_compoundargs *argp, struct kvec *head,
+                       struct page ***pagelist, u32 buflen)
+{
+	int avail;
+	int len;
+	int pages;
+
+	/* Sorry .. no magic macros for this.. *
+	 * READ_BUF(write->wr_buflen);
+	 * SAVEMEM(write->wr_buf, write->wr_buflen);
+	 */
+	avail = (char*)argp->end - (char*)argp->p;
+	if (avail + argp->pagelen < buflen) {
+		dprintk("NFSD: xdr error (%s:%d)\n",
+			       __FILE__, __LINE__);
+		return nfserr_bad_xdr;
+	}
+	head->iov_base = argp->p;
+	head->iov_len = avail;
+	*pagelist = argp->pagelist;
+
+	len = XDR_QUADLEN(buflen) << 2;
+	if (len >= avail) {
+	       len -= avail;
+
+	       pages = len >> PAGE_SHIFT;
+	       argp->pagelist += pages;
+	       argp->pagelen -= pages * PAGE_SIZE;
+	       len -= pages * PAGE_SIZE;
+
+	       next_decode_page(argp);
+	}
+	argp->p += XDR_QUADLEN(len);
+
+	return 0;
+}
+
 /**
  * savemem - duplicate a chunk of memory for later processing
  * @argp: NFSv4 compound argument structure to be freed with
@@ -1251,8 +1289,6 @@ nfsd4_decode_verify(struct nfsd4_compoundargs *argp, struct nfsd4_verify *verify
 static __be32
 nfsd4_decode_write(struct nfsd4_compoundargs *argp, struct nfsd4_write *write)
 {
-	int avail;
-	int len;
 	DECODE_HEAD;
 
 	status = nfsd4_decode_stateid(argp, &write->wr_stateid);
@@ -1265,34 +1301,10 @@ nfsd4_decode_write(struct nfsd4_compoundargs *argp, struct nfsd4_write *write)
 		goto xdr_error;
 	write->wr_buflen = be32_to_cpup(p++);
 
-	/* Sorry .. no magic macros for this.. *
-	 * READ_BUF(write->wr_buflen);
-	 * SAVEMEM(write->wr_buf, write->wr_buflen);
-	 */
-	avail = (char*)argp->end - (char*)argp->p;
-	if (avail + argp->pagelen < write->wr_buflen) {
-		dprintk("NFSD: xdr error (%s:%d)\n",
-				__FILE__, __LINE__);
-		goto xdr_error;
-	}
-	write->wr_head.iov_base = p;
-	write->wr_head.iov_len = avail;
-	write->wr_pagelist = argp->pagelist;
-
-	len = XDR_QUADLEN(write->wr_buflen) << 2;
-	if (len >= avail) {
-		int pages;
-
-		len -= avail;
-
-		pages = len >> PAGE_SHIFT;
-		argp->pagelist += pages;
-		argp->pagelen -= pages * PAGE_SIZE;
-		len -= pages * PAGE_SIZE;
-
-		next_decode_page(argp);
-	}
-	argp->p += XDR_QUADLEN(len);
+	status = svcxdr_construct_vector(argp, &write->wr_head,
+					 &write->wr_pagelist, write->wr_buflen);
+	if (status)
+		return status;
 
 	DECODE_TAIL;
 }
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 23/35] nfsd: add defines for NFSv4.2 extended attribute support
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (20 preceding siblings ...)
  2019-08-31 19:00 ` [RFC PATCH 22/35] nfsd: split off the write decode code in to a seperate function Frank van der Linden
@ 2019-08-31 19:19 ` Frank van der Linden
  2019-08-31 21:35 ` [RFC PATCH 26/35] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
                   ` (13 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-31 19:19 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Add defines for server-side extended attribute support. Most have
already been added as part of client support, but these are
the network order error codes for the noxattr and xattr2big errors,
and the addition of the xattr support to the supported file
attributes (if configured).

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfsd.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index af2947551e9c..7e2a2f4dcb24 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -280,7 +280,9 @@ void		nfsd_lockd_shutdown(void);
 #define nfserr_union_notsupp		cpu_to_be32(NFS4ERR_UNION_NOTSUPP)
 #define nfserr_offload_denied		cpu_to_be32(NFS4ERR_OFFLOAD_DENIED)
 #define nfserr_wrong_lfs		cpu_to_be32(NFS4ERR_WRONG_LFS)
-#define nfserr_badlabel		cpu_to_be32(NFS4ERR_BADLABEL)
+#define nfserr_badlabel			cpu_to_be32(NFS4ERR_BADLABEL)
+#define nfserr_xattr2big		cpu_to_be32(NFS4ERR_XATTR2BIG)
+#define nfserr_noxattr			cpu_to_be32(NFS4ERR_NOXATTR)
 
 /* error codes for internal use */
 /* if a request fails due to kmalloc failure, it gets dropped.
@@ -378,11 +380,18 @@ void		nfsd_lockd_shutdown(void);
 #define NFSD4_2_SECURITY_ATTRS		0
 #endif
 
+#ifdef CONFIG_NFSD_V4_XATTR
+#define NFSD4_2_XATTR			FATTR4_WORD2_XATTR_SUPPORT
+#else
+#define NFSD4_2_XATTR			0
+#endif
+
 #define NFSD4_2_SUPPORTED_ATTRS_WORD2 \
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
 	FATTR4_WORD2_CHANGE_ATTR_TYPE | \
 	FATTR4_WORD2_MODE_UMASK | \
-	NFSD4_2_SECURITY_ATTRS)
+	NFSD4_2_SECURITY_ATTRS | \
+	NFSD4_2_XATTR)
 
 extern const u32 nfsd_suppattrs[3][3];
 
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 26/35] nfsd: add structure definitions for xattr requests / responses
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (21 preceding siblings ...)
  2019-08-31 19:19 ` [RFC PATCH 23/35] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
@ 2019-08-31 21:35 ` Frank van der Linden
  2019-08-31 23:53 ` [RFC PATCH 24/35] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
                   ` (12 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-31 21:35 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Add the structures used in extended attribute request / response handling.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/xdr4.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index d64c870f998a..46df94ab2488 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -223,6 +223,32 @@ struct nfsd4_putfh {
 	char		*pf_fhval;          /* request */
 };
 
+struct nfsd4_getxattr {
+	char		*getxa_name;		/* request */
+	u32		getxa_len;		/* request */
+	void		*getxa_buf;
+};
+
+struct nfsd4_setxattr {
+	u32		setxa_flags;		/* request */
+	char		*setxa_name;		/* request */
+	char		*setxa_buf;		/* request */
+	u32		setxa_len;		/* request */
+	struct nfsd4_change_info  setxa_cinfo;	/* response */
+};
+
+struct nfsd4_removexattr {
+	char *		rmxa_name;		/* request */
+	struct nfsd4_change_info  rmxa_cinfo;	/* response */
+};
+
+struct nfsd4_listxattrs {
+	u64		lsxa_cookie;		/* request */
+	u32		lsxa_maxcount;		/* request */
+	char 		*lsxa_buf;		/* unfiltered buffer (reply) */
+	u32		lsxa_len;		/* unfiltered len (reply) */
+};
+
 struct nfsd4_open {
 	u32		op_claim_type;      /* request */
 	struct xdr_netobj op_fname;	    /* request - everything but CLAIM_PREV */
@@ -629,6 +655,11 @@ struct nfsd4_op {
 		struct nfsd4_copy		copy;
 		struct nfsd4_offload_status	offload_status;
 		struct nfsd4_seek		seek;
+
+		struct nfsd4_getxattr		getxattr;
+		struct nfsd4_setxattr		setxattr;
+		struct nfsd4_listxattrs		listxattrs;
+		struct nfsd4_removexattr	removexattr;
 	} u;
 	struct nfs4_replay *			replay;
 };
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 24/35] nfsd: define xattr functions to call in to their vfs counterparts
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (22 preceding siblings ...)
  2019-08-31 21:35 ` [RFC PATCH 26/35] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
@ 2019-08-31 23:53 ` Frank van der Linden
  2019-09-01  0:13 ` [RFC PATCH 25/35] nfsd: take xattr access bits in to account when checking Frank van der Linden
                   ` (11 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-08-31 23:53 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

This adds the filehandle based functions for the xattr operations
that call in to the vfs layer to do the actual work.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/vfs.c | 129 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/vfs.h |  10 ++++
 2 files changed, 139 insertions(+)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index c85783e536d5..99363e7ce044 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1990,6 +1990,135 @@ static int exp_rdonly(struct svc_rqst *rqstp, struct svc_export *exp)
 	return nfsexp_flags(rqstp, exp) & NFSEXP_READONLY;
 }
 
+#ifdef CONFIG_NFSD_V4_XATTR
+/*
+ * Helper function to translate error numbers. In the case of xattr operations,
+ * some error codes need to be translated outside of the standard translations.
+ *
+ * ENODATA needs to be translated to nfserr_noxattr.
+ * E2BIG to nfserr_xattr2big.
+ *
+ * Additionally, vfs_listxattr can return -ERANGE. This means that the
+ * file has too many extended attributes to retrieve inside an
+ * XATTR_LIST_MAX sized buffer. This is a bug in the xattr implementation:
+ * filesystems will allow the adding of extended attributes until they hit
+ * their own internal limit. This limit may be larger than XATTR_LIST_MAX.
+ * So, at that point, the attributes are present and valid, but can't
+ * be retrieved using listxattr, since the upper level xattr code enforces
+ * the XATTR_LIST_MAX limit.
+ *
+ * This bug means that we need to deal with listxattr returning -ERANGE. The
+ * best mapping is to return TOOSMALL.
+ */
+static __be32
+nfsd_xattr_errno(int err)
+{
+	switch (err) {
+	case -ENODATA:
+		return nfserr_noxattr;
+	case -E2BIG:
+		return nfserr_xattr2big;
+	case -ERANGE:
+		return nfserr_toosmall;
+	}
+	return nfserrno(err);
+}
+
+__be32
+nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name, void *buf, int *lenp)
+{
+	ssize_t lerr;
+	int err;
+
+	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
+	if (err)
+		return err;
+
+	lerr = vfs_getxattr(fhp->fh_dentry, name, buf, *lenp);
+	if (lerr < 0)
+		err = nfsd_xattr_errno(lerr);
+	else
+		*lenp = lerr;
+
+	return err;
+}
+
+__be32
+nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, void *buf, int *lenp)
+{
+	ssize_t lerr;
+	int err;
+
+	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_READ);
+	if (err)
+		return err;
+
+	lerr = vfs_listxattr(fhp->fh_dentry, buf, *lenp);
+
+	if (lerr < 0)
+		err = nfsd_xattr_errno(lerr);
+	else
+		*lenp = lerr;
+
+	return err;
+}
+
+/*
+ * Removexattr and setxattr need to call fh_lock to both lock the inode
+ * and set the change attribute. Since the top-level vfs_removexattr
+ * and vfs_setxattr calls already do their own inode_lock calls, call
+ * the _locked variant. Pass in a NULL pointer for delegated_inode,
+ * and let the client deal with NFS4ERR_DELAY (same as with e.g.
+ * setattr and remove).
+ */
+__be32
+nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name)
+{
+	int err, ret;
+
+	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_WRITE);
+	if (err)
+		return err;
+
+	ret = fh_want_write(fhp);
+	if (ret)
+		return nfserrno(ret);
+
+	fh_lock(fhp);
+
+	ret = __vfs_removexattr_locked(fhp->fh_dentry, name, NULL);
+
+	fh_unlock(fhp);
+	fh_drop_write(fhp);
+
+	return nfsd_xattr_errno(ret);
+}
+
+__be32
+nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
+	      void *buf, u32 len, u32 flags)
+{
+	int err, ret;
+
+	err = fh_verify(rqstp, fhp, 0, NFSD_MAY_WRITE);
+	if (err)
+		return err;
+
+	ret = fh_want_write(fhp);
+	if (ret)
+		return nfserrno(ret);
+	fh_lock(fhp);
+
+	ret = __vfs_setxattr_locked(fhp->fh_dentry, name, buf, len, flags,
+				    NULL);
+
+	fh_unlock(fhp);
+	fh_drop_write(fhp);
+
+	return nfsd_xattr_errno(ret);
+}
+#endif
+
 /*
  * Check for a user's access permissions to this inode.
  */
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index db351247892d..ef4e6eaf9c78 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -75,6 +75,16 @@ __be32		do_nfsd_create(struct svc_rqst *, struct svc_fh *,
 __be32		nfsd_commit(struct svc_rqst *, struct svc_fh *,
 				loff_t, unsigned long);
 #endif /* CONFIG_NFSD_V3 */
+#ifdef CONFIG_NFSD_V4_XATTR
+__be32         nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			     char *name, void *buf, int *lenp);
+__be32         nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			      void *buf, int *lenp);
+__be32         nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
+				char *name);
+__be32         nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
+			     char *name, void *buf, u32 len, u32 flags);
+#endif
 __be32		nfsd_open(struct svc_rqst *, struct svc_fh *, umode_t,
 				int, struct file **);
 struct raparms;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 25/35] nfsd: take xattr access bits in to account when checking
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (23 preceding siblings ...)
  2019-08-31 23:53 ` [RFC PATCH 24/35] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
@ 2019-09-01  0:13 ` Frank van der Linden
  2019-09-01  1:19 ` [RFC PATCH 27/35] nfsd: implement the xattr procedure functions Frank van der Linden
                   ` (10 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-01  0:13 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Since the NFSv4.2 extended attributes extension defines 3 new access
bits for xattr operations, take them in to account when validating
what the client is asking for, and when checking permissions.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c | 10 +++++++++-
 fs/nfsd/vfs.c      | 12 ++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 6fc960677644..6ade983dd9b2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -557,8 +557,16 @@ nfsd4_access(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	     union nfsd4_op_u *u)
 {
 	struct nfsd4_access *access = &u->access;
+	u32 access_full;
 
-	if (access->ac_req_access & ~NFS3_ACCESS_FULL)
+	access_full = NFS3_ACCESS_FULL;
+#ifdef CONFIG_NFSD_V4_XATTR
+	if (cstate->minorversion >= 2)
+		access_full |= NFS4_ACCESS_XALIST | NFS4_ACCESS_XAREAD |
+			       NFS4_ACCESS_XAWRITE;
+#endif
+
+	if (access->ac_req_access & ~access_full)
 		return nfserr_inval;
 
 	access->ac_resp_access = access->ac_req_access;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 99363e7ce044..d76e3041fa8e 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -611,6 +611,12 @@ static struct accessmap	nfs3_regaccess[] = {
     {	NFS3_ACCESS_MODIFY,	NFSD_MAY_WRITE|NFSD_MAY_TRUNC	},
     {	NFS3_ACCESS_EXTEND,	NFSD_MAY_WRITE			},
 
+#ifdef CONFIG_NFSD_V4_XATTR
+    {	NFS4_ACCESS_XAREAD,	NFSD_MAY_READ			},
+    {	NFS4_ACCESS_XAWRITE,	NFSD_MAY_WRITE			},
+    {	NFS4_ACCESS_XALIST,	NFSD_MAY_READ			},
+#endif
+
     {	0,			0				}
 };
 
@@ -621,6 +627,12 @@ static struct accessmap	nfs3_diraccess[] = {
     {	NFS3_ACCESS_EXTEND,	NFSD_MAY_EXEC|NFSD_MAY_WRITE	},
     {	NFS3_ACCESS_DELETE,	NFSD_MAY_REMOVE			},
 
+#ifdef CONFIG_NFSD_V4_XATTR
+    {	NFS4_ACCESS_XAREAD,	NFSD_MAY_READ			},
+    {	NFS4_ACCESS_XAWRITE,	NFSD_MAY_WRITE			},
+    {	NFS4_ACCESS_XALIST,	NFSD_MAY_READ			},
+#endif
+
     {	0,			0				}
 };
 
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 27/35] nfsd: implement the xattr procedure functions.
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (24 preceding siblings ...)
  2019-09-01  0:13 ` [RFC PATCH 25/35] nfsd: take xattr access bits in to account when checking Frank van der Linden
@ 2019-09-01  1:19 ` Frank van der Linden
  2019-09-01  2:46 ` [RFC PATCH 01/35] nfsd: make sure the nfsd4_ops array has the right size Frank van der Linden
                   ` (9 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-01  1:19 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Implement the main entry points for the *XATTR operations.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c  |  2 ++
 2 files changed, 76 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 6ade983dd9b2..db9f3fde164e 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1802,6 +1802,80 @@ nfsd4_layoutreturn(struct svc_rqst *rqstp,
 }
 #endif /* CONFIG_NFSD_PNFS */
 
+#ifdef CONFIG_NFSD_V4_XATTR
+static __be32
+nfsd4_getxattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	       union nfsd4_op_u *u)
+{
+	struct nfsd4_getxattr *getxattr = &u->getxattr;
+
+	return nfsd_getxattr(rqstp, &cstate->current_fh,
+			     getxattr->getxa_name, getxattr->getxa_buf,
+			     &getxattr->getxa_len);
+}
+
+static __be32
+nfsd4_setxattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	   union nfsd4_op_u *u)
+{
+	struct nfsd4_setxattr *setxattr = &u->setxattr;
+	int ret;
+
+	if (opens_in_grace(SVC_NET(rqstp)))
+		return nfserr_grace;
+
+	ret = nfsd_setxattr(rqstp, &cstate->current_fh, setxattr->setxa_name,
+			    setxattr->setxa_buf, setxattr->setxa_len,
+			    setxattr->setxa_flags);
+
+	if (!ret)
+		set_change_info(&setxattr->setxa_cinfo, &cstate->current_fh);
+
+	return ret;
+}
+
+static __be32
+nfsd4_listxattrs(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	   union nfsd4_op_u *u)
+{
+	int ret, len;
+
+	/*
+	 * Get the entire list, then copy out only the user attributes
+	 * in the encode function. lsxa_buf was previously allocated as
+	 * tmp svc space, and will be automatically freed later.
+	 */
+	len = XATTR_LIST_MAX;
+
+	ret = nfsd_listxattr(rqstp, &cstate->current_fh, u->listxattrs.lsxa_buf,
+			     &len);
+	if (ret)
+		return ret;
+
+	u->listxattrs.lsxa_len = len;
+
+	return nfs_ok;
+}
+
+static __be32
+nfsd4_removexattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
+	   union nfsd4_op_u *u)
+{
+	struct nfsd4_removexattr *removexattr = &u->removexattr;
+	int ret;
+
+	if (opens_in_grace(SVC_NET(rqstp)))
+		return nfserr_grace;
+
+	ret = nfsd_removexattr(rqstp, &cstate->current_fh,
+	    removexattr->rmxa_name);
+
+	if (!ret)
+		set_change_info(&removexattr->rmxa_cinfo, &cstate->current_fh);
+
+	return ret;
+}
+#endif
 /*
  * NULL call.
  */
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 25cd597f7b4e..c921945f0df0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -40,6 +40,8 @@
 #include <linux/utsname.h>
 #include <linux/pagemap.h>
 #include <linux/sunrpc/svcauth_gss.h>
+#include <linux/xattr.h>
+#include <uapi/linux/xattr.h>
 
 #include "idmap.h"
 #include "acl.h"
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 01/35] nfsd: make sure the nfsd4_ops array has the right size
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (25 preceding siblings ...)
  2019-09-01  1:19 ` [RFC PATCH 27/35] nfsd: implement the xattr procedure functions Frank van der Linden
@ 2019-09-01  2:46 ` Frank van der Linden
  2019-09-02 19:40 ` [RFC PATCH 28/35] nfsd: define xattr reply size functions Frank van der Linden
                   ` (8 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-01  2:46 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

The nfsd4_ops was initialized by initializing individual indices (op
numbers). So, the size of the array was determined by the largest
op number.

Some operations are enabled conditionally, based on config options.
If a conditionally enabled operation were to be the highest numbered
operation, the code (through OPDESC) would attempt to access memory
beyond the end of the array. This currently can't happen, since the
highest numbered op is not conditional, but will happen once the
XATTR operations are added.

So, always size the array with LAST_NFS4_OP + 1.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 8beda999e134..6fc960677644 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2344,7 +2344,7 @@ static inline u32 nfsd4_seek_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 	return (op_encode_hdr_size + 3) * sizeof(__be32);
 }
 
-static const struct nfsd4_operation nfsd4_ops[] = {
+static const struct nfsd4_operation nfsd4_ops[LAST_NFS4_OP + 1] = {
 	[OP_ACCESS] = {
 		.op_func = nfsd4_access,
 		.op_name = "OP_ACCESS",
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 28/35] nfsd: define xattr reply size functions
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (26 preceding siblings ...)
  2019-09-01  2:46 ` [RFC PATCH 01/35] nfsd: make sure the nfsd4_ops array has the right size Frank van der Linden
@ 2019-09-02 19:40 ` Frank van der Linden
  2019-09-02 19:58 ` [RFC PATCH 29/35] nfsd: add xattr XDR decode functions Frank van der Linden
                   ` (7 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 19:40 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Add functions to calculate the reply size for the user extended attribute
operations.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index db9f3fde164e..89cc5f7fceac 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2426,6 +2426,40 @@ static inline u32 nfsd4_seek_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
 	return (op_encode_hdr_size + 3) * sizeof(__be32);
 }
 
+#ifdef CONFIG_NFSD_V4_XATTR
+static inline u32 nfsd4_getxattr_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
+{
+	u32 maxcount, rlen;
+
+	maxcount = svc_max_payload(rqstp);
+	rlen = min_t(u32, XATTR_SIZE_MAX, maxcount);
+
+	return (op_encode_hdr_size + 1 + XDR_QUADLEN(rlen)) * sizeof(__be32);
+}
+
+static inline u32 nfsd4_setxattr_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
+{
+	return (op_encode_hdr_size + op_encode_change_info_maxsz)
+		* sizeof(__be32);
+}
+static inline u32 nfsd4_listxattrs_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
+{
+	u32 maxcount, rlen;
+
+	maxcount = svc_max_payload(rqstp);
+	rlen = min(op->u.listxattrs.lsxa_maxcount, maxcount);
+
+	return (op_encode_hdr_size + 4 + XDR_QUADLEN(rlen)) * sizeof(__be32);
+}
+
+static inline u32 nfsd4_removexattr_rsize(struct svc_rqst *rqstp, struct nfsd4_op *op)
+{
+	return (op_encode_hdr_size + op_encode_change_info_maxsz)
+		* sizeof(__be32);
+}
+#endif
+
+
 static const struct nfsd4_operation nfsd4_ops[LAST_NFS4_OP + 1] = {
 	[OP_ACCESS] = {
 		.op_func = nfsd4_access,
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 29/35] nfsd: add xattr XDR decode functions
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (27 preceding siblings ...)
  2019-09-02 19:40 ` [RFC PATCH 28/35] nfsd: define xattr reply size functions Frank van der Linden
@ 2019-09-02 19:58 ` Frank van der Linden
  2019-09-02 20:09 ` [RFC PATCH 30/35] nfsd: add xattr XDR encode functions Frank van der Linden
                   ` (6 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 19:58 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Implement the XDR decode functions for the user extended attribute
operations.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 233 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 228 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index c921945f0df0..acf606f764a2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1793,6 +1793,222 @@ nfsd4_decode_seek(struct nfsd4_compoundargs *argp, struct nfsd4_seek *seek)
 	DECODE_TAIL;
 }
 
+#ifdef CONFIG_NFSD_V4_XATTR
+/*
+ * XDR data that is more than PAGE_SIZE in size is normally part of a
+ * read or write. However, the size of extended attributes is limited
+ * by the maximum request size, and then further limited by the underlying
+ * filesystem limits. This can exceed PAGE_SIZE (currently, XATTR_SIZE_MAX
+ * is 64k). Since there is no kvec- or page-based interface to xattrs,
+ * and we're not dealing with contiguous pages, we need to do some copying.
+ */
+
+/*
+ * Decode int to buffer. Uses head and pages constructed by
+ * svcxdr_construct_vector.
+ */
+static int
+nfsd4_vbuf_from_stream(struct nfsd4_compoundargs *argp, struct kvec *head,
+		       struct page **pages, char **bufp, u32 buflen)
+{
+	char *tmp, *dp;
+	u32 len;
+
+	if (buflen <= head->iov_len) {
+		/*
+		 * We're in luck, the head has enough space. Just return
+		 * the head, no need for copying.
+		 */
+		*bufp = head->iov_base;
+		return 0;
+	}
+
+	tmp = svcxdr_tmpalloc(argp, buflen);
+	if (tmp == NULL)
+		return nfserr_jukebox;
+
+	dp = tmp;
+	memcpy(dp, head->iov_base, head->iov_len);
+	buflen -= head->iov_len;
+	dp += head->iov_len;
+
+	while (buflen > 0) {
+		len = min_t(u32, buflen, PAGE_SIZE);
+		memcpy(dp, page_address(*pages), len);
+
+		buflen -= len;
+		dp += len;
+		pages++;
+	}
+
+	*bufp = tmp;
+	return 0;
+}
+
+/*
+ * Get a user extended attribute name from the XDR buffer.
+ * It will not have the "user." prefix, so prepend it.
+ * Lastly, check for nul characters in the name.
+ */
+static int
+nfsd4_decode_xattr_name(struct nfsd4_compoundargs *argp, char **namep)
+{
+	DECODE_HEAD;
+	char *name, *sp, *dp;
+	u32 namelen, cnt;
+
+	READ_BUF(4);
+	namelen = be32_to_cpup(p++);
+
+	if (namelen > (XATTR_NAME_MAX - XATTR_USER_PREFIX_LEN))
+		return nfserr_nametoolong;
+
+	if (namelen == 0)
+		goto xdr_error;
+
+	READ_BUF(namelen);
+
+	name = svcxdr_tmpalloc(argp, namelen + XATTR_USER_PREFIX_LEN + 1);
+	if (!name)
+		return nfserr_jukebox;
+
+	memcpy(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN);
+
+	/*
+	 * Copy the extended attribute name over while checking for 0
+	 * characters.
+	 */
+	sp = (char *)p;
+	dp = name + XATTR_USER_PREFIX_LEN;
+	cnt = namelen;
+
+	while (cnt-- > 0) {
+		if (*sp == '\0')
+			goto xdr_error;
+		*dp++ = *sp++;
+	}
+	*dp = '\0';
+
+	*namep = name;
+
+	DECODE_TAIL;
+}
+
+/*
+ * A GETXATTR op request comes without a length specifier. We just set the
+ * maximum length for the reply based on XATTR_SIZE_MAX and the maximum
+ * channel reply size, allocate a buffer of that length and pass it to
+ * vfs_getxattr.
+ */
+static __be32
+nfsd4_decode_getxattr(struct nfsd4_compoundargs *argp, struct nfsd4_getxattr *getxattr)
+{
+	int status;
+	u32 maxcount;
+
+	status = nfsd4_decode_xattr_name(argp, &getxattr->getxa_name);
+	if (status)
+		return status;
+
+	maxcount = svc_max_payload(argp->rqstp);
+	maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
+
+	getxattr->getxa_buf = svcxdr_tmpalloc(argp, maxcount);
+	if (!getxattr->getxa_buf)
+		status = nfserr_jukebox;
+	getxattr->getxa_len = maxcount;
+
+	return status;
+}
+
+static __be32
+nfsd4_decode_setxattr(struct nfsd4_compoundargs *argp, struct nfsd4_setxattr *setxattr)
+{
+	DECODE_HEAD;
+	u32 flags, maxcount, size;
+	struct kvec head;
+	struct page **pagelist;
+
+	READ_BUF(4);
+	flags = be32_to_cpup(p++);
+
+	if (flags > SETXATTR4_REPLACE)
+		return nfserr_inval;
+	setxattr->setxa_flags = flags;
+
+	status = nfsd4_decode_xattr_name(argp, &setxattr->setxa_name);
+	if (status)
+		return status;
+
+	maxcount = svc_max_payload(argp->rqstp);
+	maxcount = min_t(u32, XATTR_SIZE_MAX, maxcount);
+
+	READ_BUF(4);
+	size = be32_to_cpup(p++);
+	if (size > maxcount)
+		return nfserr_xattr2big;
+
+	setxattr->setxa_len = size;
+	if (size > 0) {
+		status = svcxdr_construct_vector(argp, &head, &pagelist, size);
+		if (status)
+			return status;
+
+		status = nfsd4_vbuf_from_stream(argp, &head, pagelist,
+		    &setxattr->setxa_buf, size);
+	}
+
+	DECODE_TAIL;
+}
+
+static __be32
+nfsd4_decode_listxattrs(struct nfsd4_compoundargs *argp, struct nfsd4_listxattrs *listxattrs)
+{
+	DECODE_HEAD;
+	u32 maxcount;
+
+	READ_BUF(12);
+	p = xdr_decode_hyper(p, &listxattrs->lsxa_cookie);
+
+	/*
+	 * If the cookie  is too large to have even one user.x attribute
+	 * plus trailing '\0' left in a maximum size buffer, it's invalid.
+	 */
+	if (listxattrs->lsxa_cookie >=
+	    (XATTR_LIST_MAX / (XATTR_USER_PREFIX_LEN + 2)))
+		return nfserr_badcookie;
+
+	maxcount = be32_to_cpup(p++);
+	if (maxcount < 8)
+		/* Always need at least 2 words (length and one character) */
+		return nfserr_inval;
+
+	maxcount = min(maxcount, svc_max_payload(argp->rqstp));
+	listxattrs->lsxa_maxcount = maxcount;
+
+	/*
+	 * Unfortunately, there is no interface to only list xattrs for
+	 * one prefix. So there is no good way to convert maxcount to
+	 * a maximum value to pass to vfs_listxattr, as we don't know
+	 * how many of the returned attributes will be user attributes.
+	 *
+	 * So, always ask vfs_listxattr for the maximum size, and encode
+	 * as many as possible.
+	 */
+	listxattrs->lsxa_buf = svcxdr_tmpalloc(argp, XATTR_LIST_MAX);
+	if (!listxattrs->lsxa_buf)
+		status = nfserr_jukebox;
+
+	DECODE_TAIL;
+}
+
+static __be32
+nfsd4_decode_removexattr(struct nfsd4_compoundargs *argp, struct nfsd4_removexattr *removexattr)
+{
+	return nfsd4_decode_xattr_name(argp, &removexattr->rmxa_name);
+}
+#endif
+
 static __be32
 nfsd4_decode_noop(struct nfsd4_compoundargs *argp, void *p)
 {
@@ -1890,11 +2106,18 @@ static const nfsd4_dec nfsd4_dec_ops[] = {
 	[OP_WRITE_SAME]		= (nfsd4_dec)nfsd4_decode_notsupp,
 	[OP_CLONE]		= (nfsd4_dec)nfsd4_decode_clone,
 
-        /* Placeholders for RFC 8276 extended atributes operations */
-        [OP_GETXATTR]           = (nfsd4_dec)nfsd4_decode_notsupp,
-        [OP_SETXATTR]           = (nfsd4_dec)nfsd4_decode_notsupp,
-        [OP_LISTXATTRS]         = (nfsd4_dec)nfsd4_decode_notsupp,
-        [OP_REMOVEXATTR]        = (nfsd4_dec)nfsd4_decode_notsupp,
+#ifdef CONFIG_NFSD_V4_XATTR
+	/* RFC 8276 extended atributes operations */
+	[OP_GETXATTR]		= (nfsd4_dec)nfsd4_decode_getxattr,
+	[OP_SETXATTR]		= (nfsd4_dec)nfsd4_decode_setxattr,
+	[OP_LISTXATTRS]		= (nfsd4_dec)nfsd4_decode_listxattrs,
+	[OP_REMOVEXATTR]	= (nfsd4_dec)nfsd4_decode_removexattr,
+#else
+	[OP_GETXATTR]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_SETXATTR]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_LISTXATTRS]		= (nfsd4_dec)nfsd4_decode_notsupp,
+	[OP_REMOVEXATTR]	= (nfsd4_dec)nfsd4_decode_notsupp,
+#endif
 };
 
 static inline bool
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 30/35] nfsd: add xattr XDR encode functions
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (28 preceding siblings ...)
  2019-09-02 19:58 ` [RFC PATCH 29/35] nfsd: add xattr XDR decode functions Frank van der Linden
@ 2019-09-02 20:09 ` Frank van der Linden
  2019-09-02 20:19 ` [RFC PATCH 31/35] nfsd: add xattr operations to ops array Frank van der Linden
                   ` (5 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 20:09 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Implement the XDR encode functions for the user extended attribute
operations.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 231 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 231 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index acf606f764a2..be57dc09a468 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4547,6 +4547,224 @@ nfsd4_encode_noop(struct nfsd4_compoundres *resp, __be32 nfserr, void *p)
 	return nfserr;
 }
 
+#ifdef CONFIG_NFSD_V4_XATTR
+/*
+ * Encode kmalloc-ed buffer in to XDR stream.
+ */
+static int
+nfsd4_vbuf_to_stream(struct xdr_stream *xdr, char *buf, u32 buflen)
+{
+	u32 cplen;
+	__be32 *p;
+
+	cplen = min_t(unsigned long, buflen,
+		      ((void *)xdr->end - (void *)xdr->p));
+	p = xdr_reserve_space(xdr, cplen);
+	if (!p)
+		return nfserr_resource;
+
+	memcpy(p, buf, cplen);
+	buf += cplen;
+	buflen -= cplen;
+
+	while (buflen) {
+		cplen = min_t(u32, buflen, PAGE_SIZE);
+		p = xdr_reserve_space(xdr, cplen);
+		if (!p)
+			return nfserr_resource;
+
+		memcpy(p, buf, cplen);
+
+		if (cplen < PAGE_SIZE) {
+			/*
+			 * We're done, with a length that wasn't page
+			 * aligned, so possibly not word aligned. Pad
+			 * any trailing bytes with 0.
+			 */
+			xdr_encode_opaque_fixed(p, NULL, cplen);
+			break;
+		}
+
+		buflen -= PAGE_SIZE;
+		buf += PAGE_SIZE;
+	}
+
+	return 0;
+}
+
+static __be32
+nfsd4_encode_getxattr(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_getxattr *getxattr)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 4);
+	if (!p)
+		return nfserr_resource;
+
+	*p = cpu_to_be32(getxattr->getxa_len);
+
+	if (getxattr->getxa_len == 0)
+		return 0;
+
+	return nfsd4_vbuf_to_stream(xdr, getxattr->getxa_buf,
+				    getxattr->getxa_len);
+}
+
+static __be32
+nfsd4_encode_setxattr(struct nfsd4_compoundres *resp, __be32 nfserr, struct
+		      nfsd4_setxattr *setxattr)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+
+	encode_cinfo(p, &setxattr->setxa_cinfo);
+
+	return 0;
+}
+
+/*
+ * See if there are cookie values that can be rejected outright.
+ */
+static int
+nfsd4_listxattr_validate_cookie(struct nfsd4_listxattrs *listxattrs,
+				u32 *offsetp)
+{
+	u64 cookie = listxattrs->lsxa_cookie;
+
+	/*
+	 * If the cookie is larger than the maximum number we can fit
+	 * in either the buffer we just got back from vfs_listxattr, or,
+	 * XDR-encoded, in the return buffer, it's invalid.
+	 */
+	if (cookie > (listxattrs->lsxa_len) / (XATTR_USER_PREFIX_LEN + 2))
+		return nfserr_badcookie;
+
+	if (cookie > (listxattrs->lsxa_maxcount /
+		      (XDR_QUADLEN(XATTR_USER_PREFIX_LEN + 2) + 4)))
+		return nfserr_badcookie;
+
+	*offsetp = (u32)cookie;
+	return 0;
+}
+
+static __be32
+nfsd4_encode_listxattrs(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_listxattrs *listxattrs)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	u32 cookie_offset, count_offset, eof;
+	u32 left, xdrleft, slen, count;
+	u32 xdrlen, offset;
+	u64 cookie;
+	char *sp;
+	int status;
+	__be32 *p;
+	u32 nuser;
+
+	eof = 1;
+
+	status = nfsd4_listxattr_validate_cookie(listxattrs, &offset);
+	if (status)
+		return status;
+
+	/*
+	 * Reserve space for the cookie and the name array count. Record
+	 * the offsets to save them later.
+	 */
+	cookie_offset = xdr->buf->len;
+	count_offset = cookie_offset + 8;
+	p = xdr_reserve_space(xdr, 12);
+	if (!p)
+		return nfserr_resource;
+
+	count = 0;
+	left = listxattrs->lsxa_len;
+	sp = listxattrs->lsxa_buf;
+	nuser = 0;
+
+	xdrleft = listxattrs->lsxa_maxcount;
+
+	while (left > 0 && xdrleft > 0) {
+		slen = strlen(sp);
+
+		/*
+		 * Check if this a user. attribute, skip it if not.
+		 */
+		if (strncmp(sp, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
+			goto contloop;
+
+		slen -= XATTR_USER_PREFIX_LEN;
+		xdrlen = 4 + ((slen + 3) & ~3);
+		if (xdrlen > xdrleft) {
+			if (count == 0) {
+				/*
+				 * Can't even fit the first attribute name.
+				 */
+				return nfserr_toosmall;
+			}
+			eof = 0;
+			goto wreof;
+		}
+
+		left -= XATTR_USER_PREFIX_LEN;
+		sp += XATTR_USER_PREFIX_LEN;
+		if (nuser++ < offset)
+			goto contloop;
+
+
+		p = xdr_reserve_space(xdr, xdrlen);
+		if (!p)
+			return nfserr_resource;
+
+		p = xdr_encode_opaque(p, sp, slen);
+
+		xdrleft -= xdrlen;
+		count++;
+contloop:
+		sp += slen + 1;
+		left -= slen + 1;
+	}
+
+	/*
+	 * If there were user attributes to copy, but we didn't copy
+	 * any, the offset was too large (e.g. the cookie was invalid).
+	 */
+	if (nuser > 0 && count == 0)
+		return nfserr_badcookie;
+
+wreof:
+	p = xdr_reserve_space(xdr, 4);
+	if (!p)
+		return nfserr_resource;
+	*p = cpu_to_be32(eof);
+
+	cookie = offset + count;
+
+	write_bytes_to_xdr_buf(xdr->buf, cookie_offset, &cookie, 8);
+	count = htonl(count);
+	write_bytes_to_xdr_buf(xdr->buf, count_offset, &count, 4);
+	return 0;
+}
+
+static __be32
+nfsd4_encode_removexattr(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_removexattr *removexattr)
+{
+	struct xdr_stream *xdr = &resp->xdr;
+	__be32 *p;
+
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+
+	p = encode_cinfo(p, &removexattr->rmxa_cinfo);
+	return 0;
+}
+#endif
+
 typedef __be32(* nfsd4_enc)(struct nfsd4_compoundres *, __be32, void *);
 
 /*
@@ -4636,6 +4854,19 @@ static const nfsd4_enc nfsd4_enc_ops[] = {
 	[OP_SEEK]		= (nfsd4_enc)nfsd4_encode_seek,
 	[OP_WRITE_SAME]		= (nfsd4_enc)nfsd4_encode_noop,
 	[OP_CLONE]		= (nfsd4_enc)nfsd4_encode_noop,
+
+#ifdef CONFIG_NFSD_V4_XATTR
+	/* RFC 8276 extended atributes operations */
+	[OP_GETXATTR]		= (nfsd4_enc)nfsd4_encode_getxattr,
+	[OP_SETXATTR]		= (nfsd4_enc)nfsd4_encode_setxattr,
+	[OP_LISTXATTRS]		= (nfsd4_enc)nfsd4_encode_listxattrs,
+	[OP_REMOVEXATTR]	= (nfsd4_enc)nfsd4_encode_removexattr,
+#else
+	[OP_GETXATTR]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_SETXATTR]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_LISTXATTRS]		= (nfsd4_enc)nfsd4_encode_noop,
+	[OP_REMOVEXATTR]	= (nfsd4_enc)nfsd4_encode_noop,
+#endif
 };
 
 /*
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 31/35] nfsd: add xattr operations to ops array
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (29 preceding siblings ...)
  2019-09-02 20:09 ` [RFC PATCH 30/35] nfsd: add xattr XDR encode functions Frank van der Linden
@ 2019-09-02 20:19 ` Frank van der Linden
  2019-09-02 20:34 ` [RFC PATCH 32/35] xattr: add a function to check if a namespace is supported Frank van der Linden
                   ` (4 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 20:19 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

With all underlying code implemented, let's add the user extended
attributes operations to nfsd4_ops.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4proc.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 89cc5f7fceac..27f7be739c99 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2835,6 +2835,31 @@ static const struct nfsd4_operation nfsd4_ops[LAST_NFS4_OP + 1] = {
 		.op_name = "OP_OFFLOAD_CANCEL",
 		.op_rsize_bop = nfsd4_only_status_rsize,
 	},
+
+#ifdef CONFIG_NFSD_V4_XATTR
+	[OP_GETXATTR] = {
+		.op_func = nfsd4_getxattr,
+		.op_name = "OP_GETXATTR",
+		.op_rsize_bop = nfsd4_getxattr_rsize,
+	},
+	[OP_SETXATTR] = {
+		.op_func = nfsd4_setxattr,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_name = "OP_SETXATTR",
+		.op_rsize_bop = nfsd4_setxattr_rsize,
+	},
+	[OP_LISTXATTRS] = {
+		.op_func = nfsd4_listxattrs,
+		.op_name = "OP_LISTXATTRS",
+		.op_rsize_bop = nfsd4_listxattrs_rsize,
+	},
+	[OP_REMOVEXATTR] = {
+		.op_func = nfsd4_removexattr,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_name = "OP_REMOVEXATTR",
+		.op_rsize_bop = nfsd4_removexattr_rsize,
+	},
+#endif
 };
 
 /**
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 32/35] xattr: add a function to check if a namespace is supported
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (30 preceding siblings ...)
  2019-09-02 20:19 ` [RFC PATCH 31/35] nfsd: add xattr operations to ops array Frank van der Linden
@ 2019-09-02 20:34 ` Frank van der Linden
  2019-09-02 21:30 ` [RFC PATCH 33/35] nfsd: add fattr support for user extended attributes Frank van der Linden
                   ` (3 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 20:34 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Add a function that checks is an extended attribute namespace is
supported for an inode.

To be used by the nfs server code when being queried for extended
attributes support.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/xattr.c            | 27 +++++++++++++++++++++++++++
 include/linux/xattr.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/fs/xattr.c b/fs/xattr.c
index 58013bcbc333..7d0ebab1df30 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -134,6 +134,33 @@ xattr_permission(struct inode *inode, const char *name, int mask)
 	return inode_permission(inode, mask);
 }
 
+/*
+ * Look for any handler that deals with the specified namespace.
+ */
+int
+xattr_supported_namespace(struct inode *inode, const char *prefix)
+{
+	const struct xattr_handler **handlers = inode->i_sb->s_xattr;
+	const struct xattr_handler *handler;
+	size_t preflen;
+
+	if (!(inode->i_opflags & IOP_XATTR)) {
+		if (unlikely(is_bad_inode(inode)))
+			return -EIO;
+		return -EOPNOTSUPP;
+	}
+
+	preflen = strlen(prefix);
+
+	for_each_xattr_handler(handlers, handler) {
+		if (!strncmp(xattr_prefix(handler), prefix, preflen))
+			return 0;
+	}
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(xattr_supported_namespace);
+
 int
 __vfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name,
 	       const void *value, size_t size, int flags)
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 3a71ad716da5..32e377602068 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -61,6 +61,8 @@ ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_siz
 ssize_t vfs_getxattr_alloc(struct dentry *dentry, const char *name,
 			   char **xattr_value, size_t size, gfp_t flags);
 
+int xattr_supported_namespace(struct inode *inode, const char *prefix);
+
 static inline const char *xattr_prefix(const struct xattr_handler *handler)
 {
 	return handler->prefix ?: handler->name;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 33/35] nfsd: add fattr support for user extended attributes
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (31 preceding siblings ...)
  2019-09-02 20:34 ` [RFC PATCH 32/35] xattr: add a function to check if a namespace is supported Frank van der Linden
@ 2019-09-02 21:30 ` Frank van der Linden
  2019-09-02 23:06 ` [RFC PATCH 34/35] nfsd: add export flag to disable " Frank van der Linden
                   ` (2 subsequent siblings)
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 21:30 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Check if user extended attributes are supported for an inode,
and return the answer when being queried for file attributes.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/nfs4xdr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index be57dc09a468..92b9a067c744 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3149,6 +3149,17 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 	}
 #endif
 
+#ifdef CONFIG_NFSD_V4_XATTR
+	if (bmval2 & FATTR4_WORD2_XATTR_SUPPORT) {
+		p = xdr_reserve_space(xdr, 4);
+		if (!p)
+			goto out_resource;
+		err = xattr_supported_namespace(d_inode(dentry),
+						XATTR_USER_PREFIX);
+		*p++ = cpu_to_be32(err == 0);
+	}
+#endif
+
 	attrlen = htonl(xdr->buf->len - attrlen_offset - 4);
 	write_bytes_to_xdr_buf(xdr->buf, attrlen_offset, &attrlen, 4);
 	status = nfs_ok;
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 34/35] nfsd: add export flag to disable user extended attributes
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (32 preceding siblings ...)
  2019-09-02 21:30 ` [RFC PATCH 33/35] nfsd: add fattr support for user extended attributes Frank van der Linden
@ 2019-09-02 23:06 ` " Frank van der Linden
  2019-09-02 23:17 ` [RFC PATCH 35/35] nfsd: add NFSD_V4_XATTR config option Frank van der Linden
  2019-10-24 20:16 ` [RFC PATCH 00/35] user xattr support (RFC8276) Chuck Lever
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 23:06 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

Like with some other features, add a flag that allows a filesystem
to be exported without extended user attribute support, even if
support is compiled in.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/export.c                 |  1 +
 fs/nfsd/nfs4xdr.c                |  7 +++++--
 fs/nfsd/vfs.c                    | 12 ++++++++++++
 include/uapi/linux/nfsd/export.h |  3 ++-
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index baa01956a5b3..4e363272f757 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1105,6 +1105,7 @@ static struct flags {
 	{ NFSEXP_V4ROOT, {"v4root", ""}},
 	{ NFSEXP_PNFS, {"pnfs", ""}},
 	{ NFSEXP_SECURITY_LABEL, {"security_label", ""}},
+	{ NFSEXP_NOUSERXATTR, {"no_userxattr", ""}},
 	{ 0, {"", ""}}
 };
 
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 92b9a067c744..4ed0fb023ee1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3154,8 +3154,11 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 		p = xdr_reserve_space(xdr, 4);
 		if (!p)
 			goto out_resource;
-		err = xattr_supported_namespace(d_inode(dentry),
-						XATTR_USER_PREFIX);
+		if (exp->ex_flags & NFSEXP_NOUSERXATTR)
+			err = -EOPNOTSUPP;
+		else
+			err = xattr_supported_namespace(d_inode(dentry),
+							XATTR_USER_PREFIX);
 		*p++ = cpu_to_be32(err == 0);
 	}
 #endif
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index d76e3041fa8e..97a40dbd53a7 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -2046,6 +2046,9 @@ nfsd_getxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name, void *buf,
 	if (err)
 		return err;
 
+	if (fhp->fh_export->ex_flags & NFSEXP_NOUSERXATTR)
+		return nfserr_opnotsupp;
+
 	lerr = vfs_getxattr(fhp->fh_dentry, name, buf, *lenp);
 	if (lerr < 0)
 		err = nfsd_xattr_errno(lerr);
@@ -2065,6 +2068,9 @@ nfsd_listxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, void *buf, int *lenp)
 	if (err)
 		return err;
 
+	if (fhp->fh_export->ex_flags & NFSEXP_NOUSERXATTR)
+		return nfserr_opnotsupp;
+
 	lerr = vfs_listxattr(fhp->fh_dentry, buf, *lenp);
 
 	if (lerr < 0)
@@ -2092,6 +2098,9 @@ nfsd_removexattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name)
 	if (err)
 		return err;
 
+	if (fhp->fh_export->ex_flags & NFSEXP_NOUSERXATTR)
+		return nfserr_opnotsupp;
+
 	ret = fh_want_write(fhp);
 	if (ret)
 		return nfserrno(ret);
@@ -2116,6 +2125,9 @@ nfsd_setxattr(struct svc_rqst *rqstp, struct svc_fh *fhp, char *name,
 	if (err)
 		return err;
 
+	if (fhp->fh_export->ex_flags & NFSEXP_NOUSERXATTR)
+		return nfserr_opnotsupp;
+
 	ret = fh_want_write(fhp);
 	if (ret)
 		return nfserrno(ret);
diff --git a/include/uapi/linux/nfsd/export.h b/include/uapi/linux/nfsd/export.h
index 2124ba904779..b46ee0240fca 100644
--- a/include/uapi/linux/nfsd/export.h
+++ b/include/uapi/linux/nfsd/export.h
@@ -53,9 +53,10 @@
  */
 #define	NFSEXP_V4ROOT		0x10000
 #define NFSEXP_PNFS		0x20000
+#define NFSEXP_NOUSERXATTR	0x40000
 
 /* All flags that we claim to support.  (Note we don't support NOACL.) */
-#define NFSEXP_ALLFLAGS		0x3FEFF
+#define NFSEXP_ALLFLAGS		0x7FEFF
 
 /* The flags that may vary depending on security flavor: */
 #define NFSEXP_SECINFO_FLAGS	(NFSEXP_READONLY | NFSEXP_ROOTSQUASH \
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 35/35] nfsd: add NFSD_V4_XATTR config option
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (33 preceding siblings ...)
  2019-09-02 23:06 ` [RFC PATCH 34/35] nfsd: add export flag to disable " Frank van der Linden
@ 2019-09-02 23:17 ` Frank van der Linden
  2019-10-24 20:16 ` [RFC PATCH 00/35] user xattr support (RFC8276) Chuck Lever
  35 siblings, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-02 23:17 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

With everything in place, add the NFSD_V4_XATTR config option to enable
user extended attribute support.

Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
 fs/nfsd/Kconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig
index d25f6bbe7006..a42d7c8bdb03 100644
--- a/fs/nfsd/Kconfig
+++ b/fs/nfsd/Kconfig
@@ -145,6 +145,16 @@ config NFSD_V4_SECURITY_LABEL
 	If you do not wish to enable fine-grained security labels SELinux or
 	Smack policies on NFSv4 files, say N.
 
+config NFSD_V4_XATTR
+	bool "NFSv4.2 server extended attribute support (RFC8276)"
+	depends on NFSD_V4
+	help
+
+	This option enables extended attributed support, as defined
+	by RFC8726, for the kernel NFS server. This is not to be
+	confused with the "named attributers" extension. It supports
+	the xattr user namespace only, by design.
+
 config NFSD_FAULT_INJECTION
 	bool "NFS server manual fault injection"
 	depends on NFSD_V4 && DEBUG_KERNEL && DEBUG_FS
-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [RFC PATCH 00/35] user xattr support (RFC8276)
@ 2019-09-12 17:25 Frank van der Linden
  2019-07-01 17:56 ` [RFC PATCH 02/35] nfs/nfsd: basic NFS4 extended attribute definitions Frank van der Linden
                   ` (35 more replies)
  0 siblings, 36 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-09-12 17:25 UTC (permalink / raw)
  To: linux-nfs, linux-fsdevel

This RFC patch series implements both client and server side support
for RFC8276 user extended attributes.

The server side should be complete (except for modifications made
after comments, of course). The client side lacks caching. I am
working on that, but am not happy with my current implementation.
So I'm sending this out for input. Having a reviewed base to work
from will make adding client-side caching support easier to add,
in any case.

Thanks for any input!

Some notable issues:

* RFC8276 explicitly only concerns itself with the user xattr namespace.
  Linux is the only OS that encodes the namespace in the attribute name,
  adding "user." for the user namespace. Others, like FreeBSD and MacOS,
  pass the namespace as a system call argument. Since the "user." prefix
  is specific to Linux, it is stripped off on the client side, and added
  back on the server side.

* Extended attributes tend to be small in size, but they may be somewhat
  large. The Linux-imposed limit is 64k. The sunrpc XDR interfaces only
  allow sizes > PAGE_SIZE to be encoded using pages. That works out
  great for reads/writes, but since there are no page-based xattr
  FS interfaces, it means some extra explicit copying.

* A quirk of the Linux xattr implementation is that you can create
  more extended attributes per file than listxattr can handle in
  its maximum size, meaning that you get E2BIG back. There is no
  1-1 translation for that error to the LISTXATTR op. I chose to
  return NFS4ERR_TOOSMALL (which is a valid error for the operation),
  and have the client code check for it and return E2BIG. Not
  great, but it seemed to be the best option.

* The LISTXATTR op uses cookies to support multiple calls to retrieve
  the complete list, like READDDIR. This means that there's the old
  "how to deal with non-0 cookies" issue.

  In the vast majority of cases, LISTXATTR should not need multiple
  calls. However, you never know what a client will do. READDIR
  uses the cookie as a seek offset in to the directory. It's not
  quite as simple with LISTXATTR. First, there is no seperate
  FS interface to list only user xattrs. Which means that, based
  on permissions, the buffer returned by vfs_listxattr might turn
  out differently (e.g. if you have no permission to read some
  system. attributes). That means that using just a hard offset
  in to the buffer (with verifications) can't work, as its a
  context-specific value, and the client couldn't cache it if
  it wanted to.

  My code returns the attribute count as a cookie. The server
  always asks vfs_listxattr for a XATTR_LIST_MAX buffer (see
  below), and then starts XDR encoding at the Nth user xattr
  attribute, where N is the cookie value. There are bounds
  checks of course.

  This isn't great, but probably the best you can do.

* There is no FS interface to only read user extended attributes,
  and the server code needs to strip off the "user." prefix.
  Additionally, the RFC specifies that the max length field for the
  LISTXATTR op refers to the total XDR-encoded length. Given all
  this, it is not possible to know what the length is it should pass
  to vfs_listxattr. So it just passes an XATTR_LIST_MAX sized buffer
  to vfs_listxattr, and then encodes as many user xattrs as it can
  from the returned buffer.

* Given that this is an extension to v4.2, it is only supported
  if 4.2 is used (even though it has no dependencies on 4.2
  specific features).

* There is a new mount option, already known for other filesystems,
  "user_xattr", which must be passed explicitly as it stands
  right now, together with vers=4.2

* There is a new export option to turn off extended attributes for
  a filesystem.

* Modifications outside of the NFS(D) code were minor. They were
  all in the xattr code, to export versions of vfs_setxattr and
  vfs_removexattr that the NFS code should use (inode rwsem taken
  because of atomic change info), and to have them break delegations,
  as specified by RFC8276.

* As I mentioned, this has no client caching. I have one implementation,
  but I'm not that happy with it.

  The main issue with client caching is that, for virtually all expected
  cases, the number of user extended attributes per file will be small,
  and their data small. But, theoretically, you can, within current Linux
  limitations, create some 9000 user extended attributes per inode, each
  64k in size.

  My current implementation uses an inode LRU chain (much like the access
  cache), and an rhashtable per inode (I picked rhashtables because
  they automatically grow). This seems like overkill, though.

  If there are any suggestions on better implementations, I'll be happy
  to take them. The client caching I mention here is not in this series,
  as I need to clean it up a bit (and am not sure if I really want to use
  it). But I can share it if asked. It will be in the next iteration,
  in whatever shape or form it might take.

Frank van der Linden (35):
  nfsd: make sure the nfsd4_ops array has the right size
  nfs/nfsd: basic NFS4 extended attribute definitions
  NFSv4.2: query the server for extended attribute support
  nfs: parse the {no}user_xattr option
  NFSv4.2: define a function to compute the maximum XDR size for
    listxattr
  NFSv4.2: define and set initial limits for extended attributes
  NFSv4.2: define argument and response structures for xattr operations
  NFSv4.2: define the encode/decode sizes for the XATTR operations
  NFSv4.2: define and use extended attribute overhead sizes
  NFSv4.2: add client side XDR handling for extended attributes
  nfs: define nfs_access_get_cached function
  NFSv4.2: query the extended attribute access bits
  nfs: modify update_changeattr to deal with regular files
  nfs: define and use the NFS_INO_INVALID_XATTR flag
  nfs: make the buf_to_pages_noslab function available to the nfs code
  NFSv4.2: add the extended attribute proc functions.
  NFSv4.2: hook in the user extended attribute handlers
  NFSv4.2: add client side xattr caching functions
  NFSv4.2: call the xattr cache functions
  nfs: add the NFS_V4_XATTR config option
  xattr: modify vfs_{set,remove}xattr for NFS server use
  nfsd: split off the write decode code in to a seperate function
  nfsd: add defines for NFSv4.2 extended attribute support
  nfsd: define xattr functions to call in to their vfs counterparts
  nfsd: take xattr access bits in to account when checking
  nfsd: add structure definitions for xattr requests / responses
  nfsd: implement the xattr procedure functions.
  nfsd: define xattr reply size functions
  nfsd: add xattr XDR decode functions
  nfsd: add xattr XDR encode functions
  nfsd: add xattr operations to ops array
  xattr: add a function to check if a namespace is supported
  nfsd: add fattr support for user extended attributes
  nfsd: add export flag to disable user extended attributes
  nfsd: add NFSD_V4_XATTR config option

 fs/nfs/Kconfig                   |   9 +
 fs/nfs/Makefile                  |   1 +
 fs/nfs/client.c                  |  17 +-
 fs/nfs/dir.c                     |  24 +-
 fs/nfs/inode.c                   |   7 +-
 fs/nfs/internal.h                |  24 ++
 fs/nfs/nfs42.h                   |  29 ++
 fs/nfs/nfs42proc.c               | 242 ++++++++++++++
 fs/nfs/nfs42xattr.c              |  72 ++++
 fs/nfs/nfs42xdr.c                | 446 +++++++++++++++++++++++++
 fs/nfs/nfs4_fs.h                 |   5 +
 fs/nfs/nfs4client.c              |  31 ++
 fs/nfs/nfs4proc.c                | 246 ++++++++++++--
 fs/nfs/nfs4super.c               |   1 +
 fs/nfs/nfs4xdr.c                 |  35 ++
 fs/nfs/nfstrace.h                |   3 +-
 fs/nfs/super.c                   |  11 +
 fs/nfsd/Kconfig                  |  10 +
 fs/nfsd/export.c                 |   1 +
 fs/nfsd/nfs4proc.c               | 145 +++++++-
 fs/nfsd/nfs4xdr.c                | 548 +++++++++++++++++++++++++++++--
 fs/nfsd/nfsd.h                   |  13 +-
 fs/nfsd/vfs.c                    | 153 +++++++++
 fs/nfsd/vfs.h                    |  10 +
 fs/nfsd/xdr4.h                   |  31 ++
 fs/xattr.c                       |  90 ++++-
 include/linux/nfs4.h             |  27 +-
 include/linux/nfs_fs.h           |   6 +
 include/linux/nfs_fs_sb.h        |   7 +
 include/linux/nfs_xdr.h          |  62 +++-
 include/linux/xattr.h            |   4 +
 include/uapi/linux/nfs4.h        |   3 +
 include/uapi/linux/nfs_fs.h      |   1 +
 include/uapi/linux/nfsd/export.h |   3 +-
 34 files changed, 2233 insertions(+), 84 deletions(-)
 create mode 100644 fs/nfs/nfs42xattr.c

-- 
2.17.2


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
                   ` (34 preceding siblings ...)
  2019-09-02 23:17 ` [RFC PATCH 35/35] nfsd: add NFSD_V4_XATTR config option Frank van der Linden
@ 2019-10-24 20:16 ` Chuck Lever
  2019-10-24 23:15   ` Frank van der Linden
  35 siblings, 1 reply; 45+ messages in thread
From: Chuck Lever @ 2019-10-24 20:16 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Linux NFS Mailing List, linux-fsdevel

Hi Frank-

> On Sep 12, 2019, at 1:25 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> 
> This RFC patch series implements both client and server side support
> for RFC8276 user extended attributes.
> 
> The server side should be complete (except for modifications made
> after comments, of course). The client side lacks caching. I am
> working on that, but am not happy with my current implementation.
> So I'm sending this out for input. Having a reviewed base to work
> from will make adding client-side caching support easier to add,
> in any case.
> 
> Thanks for any input!

This prototype looks very good. I have some general comments as I'm
getting to know your code. I assume your end goal is to get this
work merged into the upstream Linux kernel.

First some comments about patch submission and review:

- IMO you can post future updates just to linux-nfs. Note that the
kernel NFS client and server are maintained separately, so when it
comes time to submit final patches, you will send the client work
to Trond and Anna, and the server work to Bruce (and maybe me).

- We like patches that are as small as possible but no smaller.
Some of these might be too small. For example, you don't need to add
the XDR encoders, decoders, and reply size macros in separate patches.
That might help get the total patch count down without reducing
review-ability. I can help offline with other examples.

- Please run scripts/checkpatch.pl on each patch before you post
again. This will help identify coding convention issues that should
be addressed before merge. sparse is also a good idea too.
clang-format is also nice but is entirely optional.

- I was not able to get 34/35 to apply. The series might be missing
a patch that adds nfsd_getxattr and friends.

- Do you have man page updates for the new mount and export options?


Now some comments about code design:

- I'm not clear why new CONFIG options are necessary. These days we
try to avoid adding new CONFIG options if possible. I can't think of
a reason someone would need to compile user xattr support out if
NFSv4.2 is enabled.

- Can you explain why an NFS server administrator might want to
disable user xattr support on a share?

- Probably you are correct that the design choices you made regarding
multi-message LISTXATTR are the best that can be done. Hopefully that
is not a frequent operation, but for something like "tar" it might be.
Do you have any feeling for how to assess performance?

- Regarding client caching... the RFC is notably vague about what
is needed there. You might be able to get away with no caching, just
as a start. Do you (and others) think that this series would be
acceptable and mergeable without any client caching support?

- Do you have access to an RDMA-capable platform? RPC/RDMA needs to
be able to predict how large each reply will be, in order to reserve
appropriate memory resources to land the whole RPC reply on the client.
I'm wondering if you've found any particular areas where that might be
a challenge.


Testing:

- Does fstests already have user xattr functional tests? If not, how
do you envision testing this new code?

- How should we test the logic that deals with delegation recall?

- Do you have plans to submit patches to pyNFS?


> Some notable issues:
> 
> * RFC8276 explicitly only concerns itself with the user xattr namespace.
>  Linux is the only OS that encodes the namespace in the attribute name,
>  adding "user." for the user namespace. Others, like FreeBSD and MacOS,
>  pass the namespace as a system call argument. Since the "user." prefix
>  is specific to Linux, it is stripped off on the client side, and added
>  back on the server side.
> 
> * Extended attributes tend to be small in size, but they may be somewhat
>  large. The Linux-imposed limit is 64k. The sunrpc XDR interfaces only
>  allow sizes > PAGE_SIZE to be encoded using pages. That works out
>  great for reads/writes, but since there are no page-based xattr
>  FS interfaces, it means some extra explicit copying.
> 
> * A quirk of the Linux xattr implementation is that you can create
>  more extended attributes per file than listxattr can handle in
>  its maximum size, meaning that you get E2BIG back. There is no
>  1-1 translation for that error to the LISTXATTR op. I chose to
>  return NFS4ERR_TOOSMALL (which is a valid error for the operation),
>  and have the client code check for it and return E2BIG. Not
>  great, but it seemed to be the best option.
> 
> * The LISTXATTR op uses cookies to support multiple calls to retrieve
>  the complete list, like READDDIR. This means that there's the old
>  "how to deal with non-0 cookies" issue.
> 
>  In the vast majority of cases, LISTXATTR should not need multiple
>  calls. However, you never know what a client will do. READDIR
>  uses the cookie as a seek offset in to the directory. It's not
>  quite as simple with LISTXATTR. First, there is no seperate
>  FS interface to list only user xattrs. Which means that, based
>  on permissions, the buffer returned by vfs_listxattr might turn
>  out differently (e.g. if you have no permission to read some
>  system. attributes). That means that using just a hard offset
>  in to the buffer (with verifications) can't work, as its a
>  context-specific value, and the client couldn't cache it if
>  it wanted to.
> 
>  My code returns the attribute count as a cookie. The server
>  always asks vfs_listxattr for a XATTR_LIST_MAX buffer (see
>  below), and then starts XDR encoding at the Nth user xattr
>  attribute, where N is the cookie value. There are bounds
>  checks of course.
> 
>  This isn't great, but probably the best you can do.
> 
> * There is no FS interface to only read user extended attributes,
>  and the server code needs to strip off the "user." prefix.
>  Additionally, the RFC specifies that the max length field for the
>  LISTXATTR op refers to the total XDR-encoded length. Given all
>  this, it is not possible to know what the length is it should pass
>  to vfs_listxattr. So it just passes an XATTR_LIST_MAX sized buffer
>  to vfs_listxattr, and then encodes as many user xattrs as it can
>  from the returned buffer.
> 
> * Given that this is an extension to v4.2, it is only supported
>  if 4.2 is used (even though it has no dependencies on 4.2
>  specific features).
> 
> * There is a new mount option, already known for other filesystems,
>  "user_xattr", which must be passed explicitly as it stands
>  right now, together with vers=4.2
> 
> * There is a new export option to turn off extended attributes for
>  a filesystem.
> 
> * Modifications outside of the NFS(D) code were minor. They were
>  all in the xattr code, to export versions of vfs_setxattr and
>  vfs_removexattr that the NFS code should use (inode rwsem taken
>  because of atomic change info), and to have them break delegations,
>  as specified by RFC8276.
> 
> * As I mentioned, this has no client caching. I have one implementation,
>  but I'm not that happy with it.
> 
>  The main issue with client caching is that, for virtually all expected
>  cases, the number of user extended attributes per file will be small,
>  and their data small. But, theoretically, you can, within current Linux
>  limitations, create some 9000 user extended attributes per inode, each
>  64k in size.
> 
>  My current implementation uses an inode LRU chain (much like the access
>  cache), and an rhashtable per inode (I picked rhashtables because
>  they automatically grow). This seems like overkill, though.
> 
>  If there are any suggestions on better implementations, I'll be happy
>  to take them. The client caching I mention here is not in this series,
>  as I need to clean it up a bit (and am not sure if I really want to use
>  it). But I can share it if asked. It will be in the next iteration,
>  in whatever shape or form it might take.
> 
> Frank van der Linden (35):
>  nfsd: make sure the nfsd4_ops array has the right size
>  nfs/nfsd: basic NFS4 extended attribute definitions
>  NFSv4.2: query the server for extended attribute support
>  nfs: parse the {no}user_xattr option
>  NFSv4.2: define a function to compute the maximum XDR size for
>    listxattr
>  NFSv4.2: define and set initial limits for extended attributes
>  NFSv4.2: define argument and response structures for xattr operations
>  NFSv4.2: define the encode/decode sizes for the XATTR operations
>  NFSv4.2: define and use extended attribute overhead sizes
>  NFSv4.2: add client side XDR handling for extended attributes
>  nfs: define nfs_access_get_cached function
>  NFSv4.2: query the extended attribute access bits
>  nfs: modify update_changeattr to deal with regular files
>  nfs: define and use the NFS_INO_INVALID_XATTR flag
>  nfs: make the buf_to_pages_noslab function available to the nfs code
>  NFSv4.2: add the extended attribute proc functions.
>  NFSv4.2: hook in the user extended attribute handlers
>  NFSv4.2: add client side xattr caching functions
>  NFSv4.2: call the xattr cache functions
>  nfs: add the NFS_V4_XATTR config option
>  xattr: modify vfs_{set,remove}xattr for NFS server use
>  nfsd: split off the write decode code in to a seperate function
>  nfsd: add defines for NFSv4.2 extended attribute support
>  nfsd: define xattr functions to call in to their vfs counterparts
>  nfsd: take xattr access bits in to account when checking
>  nfsd: add structure definitions for xattr requests / responses
>  nfsd: implement the xattr procedure functions.
>  nfsd: define xattr reply size functions
>  nfsd: add xattr XDR decode functions
>  nfsd: add xattr XDR encode functions
>  nfsd: add xattr operations to ops array
>  xattr: add a function to check if a namespace is supported
>  nfsd: add fattr support for user extended attributes
>  nfsd: add export flag to disable user extended attributes
>  nfsd: add NFSD_V4_XATTR config option
> 
> fs/nfs/Kconfig                   |   9 +
> fs/nfs/Makefile                  |   1 +
> fs/nfs/client.c                  |  17 +-
> fs/nfs/dir.c                     |  24 +-
> fs/nfs/inode.c                   |   7 +-
> fs/nfs/internal.h                |  24 ++
> fs/nfs/nfs42.h                   |  29 ++
> fs/nfs/nfs42proc.c               | 242 ++++++++++++++
> fs/nfs/nfs42xattr.c              |  72 ++++
> fs/nfs/nfs42xdr.c                | 446 +++++++++++++++++++++++++
> fs/nfs/nfs4_fs.h                 |   5 +
> fs/nfs/nfs4client.c              |  31 ++
> fs/nfs/nfs4proc.c                | 246 ++++++++++++--
> fs/nfs/nfs4super.c               |   1 +
> fs/nfs/nfs4xdr.c                 |  35 ++
> fs/nfs/nfstrace.h                |   3 +-
> fs/nfs/super.c                   |  11 +
> fs/nfsd/Kconfig                  |  10 +
> fs/nfsd/export.c                 |   1 +
> fs/nfsd/nfs4proc.c               | 145 +++++++-
> fs/nfsd/nfs4xdr.c                | 548 +++++++++++++++++++++++++++++--
> fs/nfsd/nfsd.h                   |  13 +-
> fs/nfsd/vfs.c                    | 153 +++++++++
> fs/nfsd/vfs.h                    |  10 +
> fs/nfsd/xdr4.h                   |  31 ++
> fs/xattr.c                       |  90 ++++-
> include/linux/nfs4.h             |  27 +-
> include/linux/nfs_fs.h           |   6 +
> include/linux/nfs_fs_sb.h        |   7 +
> include/linux/nfs_xdr.h          |  62 +++-
> include/linux/xattr.h            |   4 +
> include/uapi/linux/nfs4.h        |   3 +
> include/uapi/linux/nfs_fs.h      |   1 +
> include/uapi/linux/nfsd/export.h |   3 +-
> 34 files changed, 2233 insertions(+), 84 deletions(-)
> create mode 100644 fs/nfs/nfs42xattr.c
> 
> -- 
> 2.17.2
> 

--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-10-24 20:16 ` [RFC PATCH 00/35] user xattr support (RFC8276) Chuck Lever
@ 2019-10-24 23:15   ` Frank van der Linden
  2019-10-25 19:55     ` Chuck Lever
  0 siblings, 1 reply; 45+ messages in thread
From: Frank van der Linden @ 2019-10-24 23:15 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List, linux-fsdevel

Hi Chuck,

Thanks for your comments.

On Thu, Oct 24, 2019 at 04:16:33PM -0400, Chuck Lever wrote:
> - IMO you can post future updates just to linux-nfs. Note that the
> kernel NFS client and server are maintained separately, so when it
> comes time to submit final patches, you will send the client work
> to Trond and Anna, and the server work to Bruce (and maybe me).

Sure, I'll do that.

> 
> - We like patches that are as small as possible but no smaller.
> Some of these might be too small. For example, you don't need to add
> the XDR encoders, decoders, and reply size macros in separate patches.

True, I might have gone overboard there :-) If you can send further
suggestions offline, that'd be great!

> - Please run scripts/checkpatch.pl on each patch before you post
> again. This will help identify coding convention issues that should
> be addressed before merge. sparse is also a good idea too.
> clang-format is also nice but is entirely optional.

No problem. I think there shouldn't be many issues, but I'm sure
I mixed up some of the coding styles I've had to adhere to over
the decades..

> 
> - I was not able to get 34/35 to apply. The series might be missing
> a patch that adds nfsd_getxattr and friends.

Hm, odd. I'll check on that - I might have messed up there.

> 
> - Do you have man page updates for the new mount and export options?

I don't, but I can easily write them. They go in nfs-utils, right?

> - I'm not clear why new CONFIG options are necessary. These days we
> try to avoid adding new CONFIG options if possible. I can't think of
> a reason someone would need to compile user xattr support out if
> NFSv4.2 is enabled.
> 
> - Can you explain why an NFS server administrator might want to
> disable user xattr support on a share?

I think both of these are cases of being careful. E.g. don't enable
something by default and allow it to be disabled at runtime in
case something goes terribly wrong.

I didn't have any other reasons, really. I'm happy do to away with
the CONFIG options if that's the consensus, as well as the
nouser_xattr export option.

> - Probably you are correct that the design choices you made regarding
> multi-message LISTXATTR are the best that can be done. Hopefully that
> is not a frequent operation, but for something like "tar" it might be.
> Do you have any feeling for how to assess performance?

So far, my performance testing has been based on synthetic workloads,
which I'm also using to test some boundary conditions. E.g. create
as many xattrs as the Linux limit allows, list them all, now do
this for many files, etc. I'll definitely add testing with code
that uses xattrs. tar is on the list, but I'm happy to test anything
that exercises the code.

I don't think a multi-message LISTXATTR will happen a lot in practice,
if at all.

> 
> - Regarding client caching... the RFC is notably vague about what
> is needed there. You might be able to get away with no caching, just
> as a start. Do you (and others) think that this series would be
> acceptable and mergeable without any client caching support?

The performance is, obviously, not great without client side caching.
But then again, that's on my synthetic workloads. In cases like GNU
tar, it won't matter a whole lot because of the way that code is
structured.

I would prefer to have client side caching enabled from the start.
I have an implementation that works, but, like I mentioned, I
have some misgivings about it. But I should just include it when
I re-post - I might simply be worrying too much.

> 
> - Do you have access to an RDMA-capable platform? RPC/RDMA needs to
> be able to predict how large each reply will be, in order to reserve
> appropriate memory resources to land the whole RPC reply on the client.
> I'm wondering if you've found any particular areas where that might be
> a challenge.

Hm. I might be able to set something up. If not, I'd be relying
on someone you might know to test it for me :-)

I am not too familiar with the RDMA RPC code. From what I posted, is 
there any specific part of how the RPC layer is used that would
be of concern with RDMA?

I don't do anything other parts of the code don't do. The only special
case is using on-demand page allocation on receive, which the ACL code
also does (XDRBUF_SPARSE_PAGES - used for LISTXATTR and GETXATTR).

> 
> 
> Testing:
> 
> - Does fstests already have user xattr functional tests? If not, how
> do you envision testing this new code?

https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/ has some xattr
tests. I've, so far, been using my own set of tests that I'm happy
to contribute to any testsuite.

> 
> - How should we test the logic that deals with delegation recall?

I believe pyNFS has some logic do test this. What I have been doing
is manual testing, either using 2 clients, or, simpler, setting
xattrs on a file on the server itself, and verifying that client
delegations were recalled.

> 
> - Do you have plans to submit patches to pyNFS?

It wasn't in my plans, but I certainly could. One issue I've noticed,
with pyNFS and some other tests, is that they go no further than 4.1.
They'll need some more work to do 4.2 - although that shouldn't be
a lot of work, as most (or was it all?) features in 4.2 are optional.

I've not had much time to work on this in the past few weeks, but
next week is looking much better. Here's my plan:

* address any issues flagged by checkpatch
* merge some patches, with your input
* clean up my nfs-ganesha patches and test some more against that
* test against Rick's FreeBSD prototype
* repost the series, split in to client and server

In general, what do people do with code changes that affect both
client and server (e.g. generic defines)?

Thanks,

- Frank

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-10-24 23:15   ` Frank van der Linden
@ 2019-10-25 19:55     ` Chuck Lever
  2019-11-04  3:01       ` bfields
  0 siblings, 1 reply; 45+ messages in thread
From: Chuck Lever @ 2019-10-25 19:55 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Linux NFS Mailing List, linux-fsdevel

Hi Frank-


> On Oct 24, 2019, at 7:15 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> 
> Hi Chuck,
> 
> Thanks for your comments.
> 
> On Thu, Oct 24, 2019 at 04:16:33PM -0400, Chuck Lever wrote:
>> - IMO you can post future updates just to linux-nfs. Note that the
>> kernel NFS client and server are maintained separately, so when it
>> comes time to submit final patches, you will send the client work
>> to Trond and Anna, and the server work to Bruce (and maybe me).
> 
> Sure, I'll do that.
> 
>> 
>> - We like patches that are as small as possible but no smaller.
>> Some of these might be too small. For example, you don't need to add
>> the XDR encoders, decoders, and reply size macros in separate patches.
> 
> True, I might have gone overboard there :-) If you can send further
> suggestions offline, that'd be great!
> 
>> - Please run scripts/checkpatch.pl on each patch before you post
>> again. This will help identify coding convention issues that should
>> be addressed before merge. sparse is also a good idea too.
>> clang-format is also nice but is entirely optional.
> 
> No problem. I think there shouldn't be many issues, but I'm sure
> I mixed up some of the coding styles I've had to adhere to over
> the decades..
> 
>> 
>> - I was not able to get 34/35 to apply. The series might be missing
>> a patch that adds nfsd_getxattr and friends.
> 
> Hm, odd. I'll check on that - I might have messed up there.
> 
>> 
>> - Do you have man page updates for the new mount and export options?
> 
> I don't, but I can easily write them. They go in nfs-utils, right?

Yes. utils/mount/nfs.man for the mount option.


>> - I'm not clear why new CONFIG options are necessary. These days we
>> try to avoid adding new CONFIG options if possible. I can't think of
>> a reason someone would need to compile user xattr support out if
>> NFSv4.2 is enabled.
>> 
>> - Can you explain why an NFS server administrator might want to
>> disable user xattr support on a share?
> 
> I think both of these are cases of being careful. E.g. don't enable
> something by default and allow it to be disabled at runtime in
> case something goes terribly wrong.
> 
> I didn't have any other reasons, really. I'm happy do to away with
> the CONFIG options if that's the consensus, as well as the
> nouser_xattr export option.

I have similar patches adding support for access to a couple of
security xattrs. I initially wrapped the new code with CONFIG
but after some discussion it was decided there was really no
need to be so cautious.

The user_xattr export option is a separate matter, but again,
if we don't know of a use case for it, I would leave it out for
the moment.


>> - Probably you are correct that the design choices you made regarding
>> multi-message LISTXATTR are the best that can be done. Hopefully that
>> is not a frequent operation, but for something like "tar" it might be.
>> Do you have any feeling for how to assess performance?
> 
> So far, my performance testing has been based on synthetic workloads,
> which I'm also using to test some boundary conditions. E.g. create
> as many xattrs as the Linux limit allows, list them all, now do
> this for many files, etc. I'll definitely add testing with code
> that uses xattrs. tar is on the list, but I'm happy to test anything
> that exercises the code.
> 
> I don't think a multi-message LISTXATTR will happen a lot in practice,
> if at all.
> 
>> 
>> - Regarding client caching... the RFC is notably vague about what
>> is needed there. You might be able to get away with no caching, just
>> as a start. Do you (and others) think that this series would be
>> acceptable and mergeable without any client caching support?
> 
> The performance is, obviously, not great without client side caching.
> But then again, that's on my synthetic workloads. In cases like GNU
> tar, it won't matter a whole lot because of the way that code is
> structured.
> 
> I would prefer to have client side caching enabled from the start.
> I have an implementation that works, but, like I mentioned, I
> have some misgivings about it. But I should just include it when
> I re-post - I might simply be worrying too much.

After the patches are cleaner (checkpatch and squashing) I think
you will get more direct review of the caching heuristics.

I'll send some suggestions via private e-mail.


>> - Do you have access to an RDMA-capable platform? RPC/RDMA needs to
>> be able to predict how large each reply will be, in order to reserve
>> appropriate memory resources to land the whole RPC reply on the client.
>> I'm wondering if you've found any particular areas where that might be
>> a challenge.
> 
> Hm. I might be able to set something up. If not, I'd be relying
> on someone you might know to test it for me :-)
> 
> I am not too familiar with the RDMA RPC code. From what I posted, is 
> there any specific part of how the RPC layer is used that would
> be of concern with RDMA?
> 
> I don't do anything other parts of the code don't do. The only special
> case is using on-demand page allocation on receive, which the ACL code
> also does (XDRBUF_SPARSE_PAGES - used for LISTXATTR and GETXATTR).

That's exactly what's of concern. RDMA has similar logic as TCP
here to allocate pages on demand for this case. The problem
arises when the server needs to return a bigger reply than will
fit in this buffer. Rare.


>> Testing:
>> 
>> - Does fstests already have user xattr functional tests? If not, how
>> do you envision testing this new code?
> 
> https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/ has some xattr
> tests. I've, so far, been using my own set of tests that I'm happy
> to contribute to any testsuite.

fstests would be the one.


>> - How should we test the logic that deals with delegation recall?
> 
> I believe pyNFS has some logic do test this. What I have been doing
> is manual testing, either using 2 clients, or, simpler, setting
> xattrs on a file on the server itself, and verifying that client
> delegations were recalled.
> 
>> 
>> - Do you have plans to submit patches to pyNFS?
> 
> It wasn't in my plans, but I certainly could. One issue I've noticed,
> with pyNFS and some other tests, is that they go no further than 4.1.
> They'll need some more work to do 4.2 - although that shouldn't be
> a lot of work, as most (or was it all?) features in 4.2 are optional.

OK, if v4.2 is not supported in the test suite, then there is
a pre-requisite discussion to be had.


> I've not had much time to work on this in the past few weeks, but
> next week is looking much better. Here's my plan:
> 
> * address any issues flagged by checkpatch
> * merge some patches, with your input
> * clean up my nfs-ganesha patches and test some more against that
> * test against Rick's FreeBSD prototype
> * repost the series, split in to client and server
> 
> In general, what do people do with code changes that affect both
> client and server (e.g. generic defines)?

For generic defines, include the same patches in both the client
and server series. When git merges the two separate branches, it
should recognize that the incoming files are identical and do
nothing.


--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-10-25 19:55     ` Chuck Lever
@ 2019-11-04  3:01       ` bfields
  2019-11-04 15:36         ` Chuck Lever
  0 siblings, 1 reply; 45+ messages in thread
From: bfields @ 2019-11-04  3:01 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Frank van der Linden, Linux NFS Mailing List, linux-fsdevel

On Fri, Oct 25, 2019 at 03:55:09PM -0400, Chuck Lever wrote:
> > On Oct 24, 2019, at 7:15 PM, Frank van der Linden <fllinden@amazon.com> wrote:
> > I think both of these are cases of being careful. E.g. don't enable
> > something by default and allow it to be disabled at runtime in
> > case something goes terribly wrong.
> > 
> > I didn't have any other reasons, really. I'm happy do to away with
> > the CONFIG options if that's the consensus, as well as the
> > nouser_xattr export option.
> 
> I have similar patches adding support for access to a couple of
> security xattrs. I initially wrapped the new code with CONFIG
> but after some discussion it was decided there was really no
> need to be so cautious.
> 
> The user_xattr export option is a separate matter, but again,
> if we don't know of a use case for it, I would leave it out for
> the moment.

Agreed.

Do ext4, xfs, etc. have an option to turn off xattrs?  If so, maybe it
would be good enough to turn off xattrs on the exported filesystem
rathre than on the export.

If not, maybe that's a sign that hasn't been a need.

--b.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-11-04  3:01       ` bfields
@ 2019-11-04 15:36         ` Chuck Lever
  2019-11-04 16:21           ` Frank van der Linden
  0 siblings, 1 reply; 45+ messages in thread
From: Chuck Lever @ 2019-11-04 15:36 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Bruce Fields, Linux NFS Mailing List, linux-fsdevel



> On Nov 3, 2019, at 10:01 PM, bfields@fieldses.org wrote:
> 
> On Fri, Oct 25, 2019 at 03:55:09PM -0400, Chuck Lever wrote:
>>> On Oct 24, 2019, at 7:15 PM, Frank van der Linden <fllinden@amazon.com> wrote:
>>> I think both of these are cases of being careful. E.g. don't enable
>>> something by default and allow it to be disabled at runtime in
>>> case something goes terribly wrong.
>>> 
>>> I didn't have any other reasons, really. I'm happy do to away with
>>> the CONFIG options if that's the consensus, as well as the
>>> nouser_xattr export option.
>> 
>> I have similar patches adding support for access to a couple of
>> security xattrs. I initially wrapped the new code with CONFIG
>> but after some discussion it was decided there was really no
>> need to be so cautious.
>> 
>> The user_xattr export option is a separate matter, but again,
>> if we don't know of a use case for it, I would leave it out for
>> the moment.
> 
> Agreed.
> 
> Do ext4, xfs, etc. have an option to turn off xattrs?  If so, maybe it
> would be good enough to turn off xattrs on the exported filesystem
> rather than on the export.

Following the server's local file systems' mount options seems like a
good way to go. In particular, is there a need to expose user xattrs
on the server host, but prevent NFS clients' access to them? I can't
think of one.


> If not, maybe that's a sign that hasn't been a need.
> 
> --b.

--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-11-04 15:36         ` Chuck Lever
@ 2019-11-04 16:21           ` Frank van der Linden
  2019-11-04 22:58             ` Bruce Fields
  0 siblings, 1 reply; 45+ messages in thread
From: Frank van der Linden @ 2019-11-04 16:21 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Bruce Fields, Linux NFS Mailing List, linux-fsdevel

On Mon, Nov 04, 2019 at 10:36:03AM -0500, Chuck Lever wrote:
> 
> Following the server's local file systems' mount options seems like a
> good way to go. In particular, is there a need to expose user xattrs
> on the server host, but prevent NFS clients' access to them? I can't
> think of one.

Ok, that sounds fine to me - I'll remove the user_xattr export flag,
and we had already agreed to do away with the CONFIGs.

That leaves one last item with regard to enabling support: the client side
mount option. I assume the [no]user_xattr option should work the same as
with other filesystems. What about the default setting?

Also, currently, my code does not fail the mount operation if user xattrs
are asked for, but the server does not support them. It just doesn't
set NFS_CAP_XATTR for the server, and the xattr handler entry points
eturn -EOPNOTSUPP, as they check for NFS_CAP_XATTR. What's the preferred
behavior there?

- Frank

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-11-04 16:21           ` Frank van der Linden
@ 2019-11-04 22:58             ` Bruce Fields
  2019-11-05  0:06               ` Frank van der Linden
  2019-11-05 15:44               ` Chuck Lever
  0 siblings, 2 replies; 45+ messages in thread
From: Bruce Fields @ 2019-11-04 22:58 UTC (permalink / raw)
  To: Frank van der Linden; +Cc: Chuck Lever, Linux NFS Mailing List, linux-fsdevel

On Mon, Nov 04, 2019 at 04:21:47PM +0000, Frank van der Linden wrote:
> On Mon, Nov 04, 2019 at 10:36:03AM -0500, Chuck Lever wrote:
> > 
> > Following the server's local file systems' mount options seems like a
> > good way to go. In particular, is there a need to expose user xattrs
> > on the server host, but prevent NFS clients' access to them? I can't
> > think of one.
> 
> Ok, that sounds fine to me - I'll remove the user_xattr export flag,
> and we had already agreed to do away with the CONFIGs.
> 
> That leaves one last item with regard to enabling support: the client side
> mount option. I assume the [no]user_xattr option should work the same as
> with other filesystems. What about the default setting?

Just checking code for other filesystems quickly; if I understand right:

	- ext4 has user_xattr and nouser_xattr options, but you get a
	  deprecation warning if you try to use the latter;
	- xfs doesn't support either option;
	- cifs supports both, with xattr support the default.

Not necessarily my call, but just for simplicity's sake, I'd probably
leave out the option and see if anybody complains.

> Also, currently, my code does not fail the mount operation if user xattrs
> are asked for, but the server does not support them. It just doesn't
> set NFS_CAP_XATTR for the server, and the xattr handler entry points
> eturn -EOPNOTSUPP, as they check for NFS_CAP_XATTR. What's the preferred
> behavior there?

getxattr(2) under ERRORS says:

	ENOTSUP
	      Extended attributes are not supported by the filesystem,
	      or  are disabled.

so I'm guessing just erroring out is clearest.

I also see there's an EOPNOTSUPP return in the nouser_xattr case in
ext4_xattr_user_get.  (errno(3) says ENOTSUP and EOPNOTSUPP are the same
value on Linux.)

--b.

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-11-04 22:58             ` Bruce Fields
@ 2019-11-05  0:06               ` Frank van der Linden
  2019-11-05 15:44               ` Chuck Lever
  1 sibling, 0 replies; 45+ messages in thread
From: Frank van der Linden @ 2019-11-05  0:06 UTC (permalink / raw)
  To: Bruce Fields; +Cc: Chuck Lever, Linux NFS Mailing List, linux-fsdevel

On Mon, Nov 04, 2019 at 05:58:46PM -0500, Bruce Fields wrote:
> Just checking code for other filesystems quickly; if I understand right:
> 
> 	- ext4 has user_xattr and nouser_xattr options, but you get a
> 	  deprecation warning if you try to use the latter;
> 	- xfs doesn't support either option;
> 	- cifs supports both, with xattr support the default.
> 
> Not necessarily my call, but just for simplicity's sake, I'd probably
> leave out the option and see if anybody complains.

Sounds good, I'll go with that.
> 
> > Also, currently, my code does not fail the mount operation if user xattrs
> > are asked for, but the server does not support them. It just doesn't
> > set NFS_CAP_XATTR for the server, and the xattr handler entry points
> > eturn -EOPNOTSUPP, as they check for NFS_CAP_XATTR. What's the preferred
> > behavior there?
> 
> getxattr(2) under ERRORS says:
> 
> 	ENOTSUP
> 	      Extended attributes are not supported by the filesystem,
> 	      or  are disabled.
> 
> so I'm guessing just erroring out is clearest.
> 
> I also see there's an EOPNOTSUPP return in the nouser_xattr case in
> ext4_xattr_user_get.  (errno(3) says ENOTSUP and EOPNOTSUPP are the same
> value on Linux.)

Sure. So, to recap, here's what I'll do:

* Remove the CONFIG options - enable the code by default.
* Server side:
	* Remove the export file option to not export extended attributes
* Client side:
	* Always probe user xattr support, and use it when available
	  (returning EOPNOTSUPP, as expected, for all xattr syscalls if
	   it is not available).

Thanks for the feedback so far - much appreciated. I'll submit the separate
client and server patch sets this week.

- Frank

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [RFC PATCH 00/35] user xattr support (RFC8276)
  2019-11-04 22:58             ` Bruce Fields
  2019-11-05  0:06               ` Frank van der Linden
@ 2019-11-05 15:44               ` Chuck Lever
  1 sibling, 0 replies; 45+ messages in thread
From: Chuck Lever @ 2019-11-05 15:44 UTC (permalink / raw)
  To: Bruce Fields, Frank van der Linden; +Cc: Linux NFS Mailing List, linux-fsdevel



> On Nov 4, 2019, at 5:58 PM, Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Mon, Nov 04, 2019 at 04:21:47PM +0000, Frank van der Linden wrote:
>> On Mon, Nov 04, 2019 at 10:36:03AM -0500, Chuck Lever wrote:
>>> 
>>> Following the server's local file systems' mount options seems like a
>>> good way to go. In particular, is there a need to expose user xattrs
>>> on the server host, but prevent NFS clients' access to them? I can't
>>> think of one.
>> 
>> Ok, that sounds fine to me - I'll remove the user_xattr export flag,
>> and we had already agreed to do away with the CONFIGs.
>> 
>> That leaves one last item with regard to enabling support: the client side
>> mount option. I assume the [no]user_xattr option should work the same as
>> with other filesystems. What about the default setting?
> 
> Just checking code for other filesystems quickly; if I understand right:
> 
> 	- ext4 has user_xattr and nouser_xattr options, but you get a
> 	  deprecation warning if you try to use the latter;
> 	- xfs doesn't support either option;
> 	- cifs supports both, with xattr support the default.
> 
> Not necessarily my call, but just for simplicity's sake, I'd probably
> leave out the option and see if anybody complains.

Agree, I would leave it out to begin with. Anyone on linux-fsdevel,
feel free to chime in here about why some other file systems have
this mount option. History lessons welcome.


>> Also, currently, my code does not fail the mount operation if user xattrs
>> are asked for, but the server does not support them. It just doesn't
>> set NFS_CAP_XATTR for the server, and the xattr handler entry points
>> eturn -EOPNOTSUPP, as they check for NFS_CAP_XATTR. What's the preferred
>> behavior there?
> 
> getxattr(2) under ERRORS says:
> 
> 	ENOTSUP
> 	      Extended attributes are not supported by the filesystem,
> 	      or  are disabled.
> 
> so I'm guessing just erroring out is clearest.

IMO on the client, we want getxattr failure behavior to be consistent among:

- A version of NFS that does not support xattrs at all (say, v3)

- A version of NFS that can support them but doesn't (say, NFSv4.2 before these patches)

- A version of NFS that can support them, but the server doesn't

- A version of NFS that can support them, a server that can support them, but it's filesystem doesn't


> I also see there's an EOPNOTSUPP return in the nouser_xattr case in
> ext4_xattr_user_get.  (errno(3) says ENOTSUP and EOPNOTSUPP are the same
> value on Linux.)
> 
> --b.

--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, back to index

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 17:25 [RFC PATCH 00/35] user xattr support (RFC8276) Frank van der Linden
2019-07-01 17:56 ` [RFC PATCH 02/35] nfs/nfsd: basic NFS4 extended attribute definitions Frank van der Linden
2019-08-26 21:38 ` [RFC PATCH 03/35] NFSv4.2: query the server for extended attribute support Frank van der Linden
2019-08-26 21:53 ` [RFC PATCH 04/35] nfs: parse the {no}user_xattr option Frank van der Linden
2019-08-26 22:06 ` [RFC PATCH 05/35] NFSv4.2: define a function to compute the maximum XDR size for listxattr Frank van der Linden
2019-08-26 22:23 ` [RFC PATCH 06/35] NFSv4.2: define and set initial limits for extended attributes Frank van der Linden
2019-08-26 22:32 ` [RFC PATCH 07/35] NFSv4.2: define argument and response structures for xattr operations Frank van der Linden
2019-08-26 22:44 ` [RFC PATCH 08/35] NFSv4.2: define the encode/decode sizes for the XATTR operations Frank van der Linden
2019-08-26 23:09 ` [RFC PATCH 09/35] NFSv4.2: define and use extended attribute overhead sizes Frank van der Linden
2019-08-27 15:34 ` [RFC PATCH 10/35] NFSv4.2: add client side XDR handling for extended attributes Frank van der Linden
2019-08-27 15:46 ` [RFC PATCH 11/35] nfs: define nfs_access_get_cached function Frank van der Linden
2019-08-27 16:01 ` [RFC PATCH 12/35] NFSv4.2: query the extended attribute access bits Frank van der Linden
2019-08-27 22:51 ` [RFC PATCH 13/35] nfs: modify update_changeattr to deal with regular files Frank van der Linden
2019-08-30 22:48 ` [RFC PATCH 14/35] nfs: define and use the NFS_INO_INVALID_XATTR flag Frank van der Linden
2019-08-30 22:56 ` [RFC PATCH 15/35] nfs: make the buf_to_pages_noslab function available to the nfs code Frank van der Linden
2019-08-30 23:15 ` [RFC PATCH 16/35] NFSv4.2: add the extended attribute proc functions Frank van der Linden
2019-08-30 23:31 ` [RFC PATCH 17/35] NFSv4.2: hook in the user extended attribute handlers Frank van der Linden
2019-08-30 23:38 ` [RFC PATCH 18/35] NFSv4.2: add client side xattr caching functions Frank van der Linden
2019-08-30 23:45 ` [RFC PATCH 19/35] NFSv4.2: call the xattr cache functions Frank van der Linden
2019-08-30 23:59 ` [RFC PATCH 20/35] nfs: add the NFS_V4_XATTR config option Frank van der Linden
2019-08-31  2:12 ` [RFC PATCH 21/35] xattr: modify vfs_{set,remove}xattr for NFS server use Frank van der Linden
2019-08-31 19:00 ` [RFC PATCH 22/35] nfsd: split off the write decode code in to a seperate function Frank van der Linden
2019-08-31 19:19 ` [RFC PATCH 23/35] nfsd: add defines for NFSv4.2 extended attribute support Frank van der Linden
2019-08-31 21:35 ` [RFC PATCH 26/35] nfsd: add structure definitions for xattr requests / responses Frank van der Linden
2019-08-31 23:53 ` [RFC PATCH 24/35] nfsd: define xattr functions to call in to their vfs counterparts Frank van der Linden
2019-09-01  0:13 ` [RFC PATCH 25/35] nfsd: take xattr access bits in to account when checking Frank van der Linden
2019-09-01  1:19 ` [RFC PATCH 27/35] nfsd: implement the xattr procedure functions Frank van der Linden
2019-09-01  2:46 ` [RFC PATCH 01/35] nfsd: make sure the nfsd4_ops array has the right size Frank van der Linden
2019-09-02 19:40 ` [RFC PATCH 28/35] nfsd: define xattr reply size functions Frank van der Linden
2019-09-02 19:58 ` [RFC PATCH 29/35] nfsd: add xattr XDR decode functions Frank van der Linden
2019-09-02 20:09 ` [RFC PATCH 30/35] nfsd: add xattr XDR encode functions Frank van der Linden
2019-09-02 20:19 ` [RFC PATCH 31/35] nfsd: add xattr operations to ops array Frank van der Linden
2019-09-02 20:34 ` [RFC PATCH 32/35] xattr: add a function to check if a namespace is supported Frank van der Linden
2019-09-02 21:30 ` [RFC PATCH 33/35] nfsd: add fattr support for user extended attributes Frank van der Linden
2019-09-02 23:06 ` [RFC PATCH 34/35] nfsd: add export flag to disable " Frank van der Linden
2019-09-02 23:17 ` [RFC PATCH 35/35] nfsd: add NFSD_V4_XATTR config option Frank van der Linden
2019-10-24 20:16 ` [RFC PATCH 00/35] user xattr support (RFC8276) Chuck Lever
2019-10-24 23:15   ` Frank van der Linden
2019-10-25 19:55     ` Chuck Lever
2019-11-04  3:01       ` bfields
2019-11-04 15:36         ` Chuck Lever
2019-11-04 16:21           ` Frank van der Linden
2019-11-04 22:58             ` Bruce Fields
2019-11-05  0:06               ` Frank van der Linden
2019-11-05 15:44               ` Chuck Lever

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git