From: "Schumaker, Anna" <Anna.Schumaker@netapp.com>
To: "fllinden@amazon.com" <fllinden@amazon.com>,
"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
"trond.myklebust@hammerspace.com"
<trond.myklebust@hammerspace.com>
Subject: Re: [PATCH 04/13] NFSv4.2: define limits and sizes for user xattr handling
Date: Thu, 12 Mar 2020 20:35:06 +0000 [thread overview]
Message-ID: <4cdfd8fa4a0fa75dd58ae278b5329b97f639527b.camel@netapp.com> (raw)
In-Reply-To: <20200311195613.26108-5-fllinden@amazon.com>
Hi Frank,
On Wed, 2020-03-11 at 19:56 +0000, Frank van der Linden wrote:
> Set limits for extended attributes (attribute value size and listxattr
> buffer size), based on the fs-independent limits (XATTR_*_MAX).
>
> 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 separate pages, like other operations do.
>
> 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/client.c | 19 ++++++++++--
> fs/nfs/internal.h | 5 ++++
> fs/nfs/nfs42.h | 16 ++++++++++
> fs/nfs/nfs42xdr.c | 74
> +++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfs/nfs4client.c | 31 ++++++++++++++++++++
> include/linux/nfs_fs_sb.h | 5 ++++
> 6 files changed, 148 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 989c30c98511..eef39a4ec114 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
>
> @@ -748,7 +749,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)
> @@ -761,7 +762,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)
> @@ -794,6 +797,18 @@ 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_2
> + /*
> + * Defaults until limited by the session parameters.
> + */
> + server->gxasize = min_t(unsigned int, raw_max_rpc_payload,
> + XATTR_SIZE_MAX);
> + server->sxasize = min_t(unsigned int, raw_max_rpc_payload,
> + XATTR_SIZE_MAX);
> + server->lxasize = min_t(unsigned int, raw_max_rpc_payload,
> + nfs42_listxattr_xdrsize(XATTR_LIST_MAX));
> +#endif
> }
>
> /*
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index f80c47d5ff27..68f235a571e1 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -306,6 +306,11 @@ extern const u32 nfs41_maxread_overhead;
> extern const u32 nfs41_maxwrite_overhead;
> extern const u32 nfs41_maxgetdevinfo_overhead;
> #endif
> +#ifdef CONFIG_NFS_V4_2
> +extern const u32 nfs42_maxsetxattr_overhead;
> +extern const u32 nfs42_maxgetxattr_overhead;
> +extern const u32 nfs42_maxlistxattrs_overhead;
> +#endif
As far as I can tell, these are only used by the NFS v4 code. Can you put these
definitions in nfs4_fs.h instead of internal.h, since the generic client doesn't
really need to know about it?
Thanks,
Anna
>
> /* nfs4proc.c */
> #if IS_ENABLED(CONFIG_NFS_V4)
> diff --git a/fs/nfs/nfs42.h b/fs/nfs/nfs42.h
> index c891af949886..51de8ddc7d88 100644
> --- a/fs/nfs/nfs42.h
> +++ b/fs/nfs/nfs42.h
> @@ -6,6 +6,8 @@
> #ifndef __LINUX_FS_NFS_NFS4_2_H
> #define __LINUX_FS_NFS_NFS4_2_H
>
> +#include <linux/xattr.h>
> +
> /*
> * 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.
> @@ -36,5 +38,19 @@ static inline bool nfs42_files_from_same_server(struct file
> *in,
> return nfs4_check_serverowner_major_id(c_in->cl_serverowner,
> c_out->cl_serverowner);
> }
> +
> +/*
> + * 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 /* CONFIG_NFS_V4_2 */
> #endif /* __LINUX_FS_NFS_NFS4_2_H */
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index c03f3246d6c5..6712daa9d85b 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -169,6 +169,80 @@
> decode_clone_maxsz + \
> decode_getattr_maxsz)
>
> +#ifdef CONFIG_NFS_V4_2
> +/* 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)
> +
> +/*
> + * 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,
> const struct nfs42_falloc_args *args)
> {
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 0cd767e5c977..8b320ef0e8a3 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -993,6 +993,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_2
> + 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)
> {
> @@ -1040,6 +1070,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;
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index d881f7a38bc9..7eae72a8762e 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -163,6 +163,11 @@ struct nfs_server {
> unsigned int dtsize; /* readdir size */
> unsigned short port; /* "port=" setting */
> unsigned int bsize; /* server block size */
> +#ifdef CONFIG_NFS_V4_2
> + 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;
next prev parent reply other threads:[~2020-03-12 20:35 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-11 19:56 [PATCH 00/13] client side user xattr (RFC8276) support Frank van der Linden
2020-03-11 19:56 ` [PATCH 01/13] nfs,nfsd: NFSv4.2 extended attribute protocol definitions Frank van der Linden
2020-03-11 19:56 ` [PATCH 02/13] nfs: add client side only definitions for user xattrs Frank van der Linden
2020-03-11 19:56 ` [PATCH 03/13] NFSv4.2: query the server for extended attribute support Frank van der Linden
2020-03-12 16:15 ` Mkrtchyan, Tigran
2020-03-12 20:51 ` Frank van der Linden
2020-03-12 21:15 ` Frank van der Linden
2020-03-13 11:11 ` Mkrtchyan, Tigran
2020-03-13 13:50 ` Trond Myklebust
2020-03-13 14:19 ` Mkrtchyan, Tigran
2020-03-13 17:10 ` Trond Myklebust
2020-03-13 17:55 ` Frank van der Linden
2020-03-11 19:56 ` [PATCH 04/13] NFSv4.2: define limits and sizes for user xattr handling Frank van der Linden
2020-03-12 20:35 ` Schumaker, Anna [this message]
2020-03-11 19:56 ` [PATCH 05/13] NFSv4.2: add client side XDR handling for extended attributes Frank van der Linden
2020-03-12 20:49 ` Schumaker, Anna
2020-03-11 19:56 ` [PATCH 06/13] nfs: define nfs_access_get_cached function Frank van der Linden
2020-03-11 19:56 ` [PATCH 07/13] NFSv4.2: query the extended attribute access bits Frank van der Linden
2020-03-11 19:56 ` [PATCH 08/13] nfs: modify update_changeattr to deal with regular files Frank van der Linden
2020-03-11 19:56 ` [PATCH 09/13] nfs: define and use the NFS_INO_INVALID_XATTR flag Frank van der Linden
[not found] ` <20200324060215.GD11705@shao2-debian>
2020-03-24 16:21 ` [nfs] c5654df66d: stress-ng.msg.ops_per_sec 15.5% improvement Frank van der Linden
2020-03-11 19:56 ` [PATCH 10/13] nfs: make the buf_to_pages_noslab function available to the nfs code Frank van der Linden
2020-03-12 20:36 ` Schumaker, Anna
2020-03-11 19:56 ` [PATCH 11/13] NFSv4.2: add the extended attribute proc functions Frank van der Linden
2020-03-11 19:56 ` [PATCH 12/13] NFSv4.2: hook in the user extended attribute handlers Frank van der Linden
2020-03-11 19:56 ` [PATCH 13/13] NFSv4.2: add client side xattr caching Frank van der Linden
2020-03-12 20:39 ` Schumaker, Anna
2020-03-12 20:48 ` Schumaker, Anna
2020-03-12 19:06 ` [PATCH 00/13] client side user xattr (RFC8276) support Mkrtchyan, Tigran
2020-03-12 20:09 ` Anna Schumaker
2020-03-16 15:50 ` Frank van der Linden
2020-03-17 23:03 ` Frank van der Linden
2020-03-19 14:39 ` J. Bruce Fields
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4cdfd8fa4a0fa75dd58ae278b5329b97f639527b.camel@netapp.com \
--to=anna.schumaker@netapp.com \
--cc=fllinden@amazon.com \
--cc=linux-nfs@vger.kernel.org \
--cc=trond.myklebust@hammerspace.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).