Re: [PATCH 05/13] NFSv4.2: add client side XDR handling for extended attributes

Schumaker, Anna
2020-03-12T20:49:48Z

Hi Frank,

On Wed, 2020-03-11 at 19:56 +0000, Frank van der Linden wrote:
> Define the argument and response structures that will be used for
> RFC 8276 extended attribute RPC calls, and 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 | 368
> ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/nfs/nfs4xdr.c | 6 +
> include/linux/nfs_xdr.h | 59 +++++++-
> 3 files changed, 432 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> index 6712daa9d85b..ac45a1027523 100644
> --- a/fs/nfs/nfs42xdr.c
> +++ b/fs/nfs/nfs42xdr.c
> @@ -407,6 +407,212 @@ static void encode_layouterror(struct xdr_stream *xdr,
> encode_device_error(xdr, &args->errors[0]);
> }
>
> +#ifdef CONFIG_NFS_V4_2

This file is only compiled when CONFIG_NFS_V4_2=y, so there is no need for the
#ifdef here.

Anna

> +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
> */
> @@ -1062,4 +1268,166 @@ static int nfs4_xdr_dec_layouterror(struct rpc_rqst
> *rqstp,
> return status;
> }
>
> +#ifdef CONFIG_NFS_V4_2
> +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 bebc087a1433..9e1ced791789 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -7479,6 +7479,8 @@ static struct {
> { NFS4ERR_SYMLINK, -ELOOP },
> { NFS4ERR_OP_ILLEGAL, -EOPNOTSUPP },
> { NFS4ERR_DEADLOCK, -EDEADLK },
> + { NFS4ERR_NOXATTR, -ENODATA },
> + { NFS4ERR_XATTR2BIG, -E2BIG },
> { -1, -EIO }
> };
>
> @@ -7607,6 +7609,10 @@ const struct rpc_procinfo nfs4_procedures[] = {
> PROC42(COPY_NOTIFY, enc_copy_notify, dec_copy_notify),
> PROC(LOOKUPP, enc_lookupp, dec_lookupp),
> PROC42(LAYOUTERROR, enc_layouterror, dec_layouterror),
> + PROC42(GETXATTR, enc_getxattr, dec_getxattr),
> + PROC42(SETXATTR, enc_setxattr, dec_setxattr),
> + PROC42(LISTXATTRS, enc_listxattrs, dec_listxattrs),
> + PROC42(REMOVEXATTR, enc_removexattr, dec_removexattr),
> };
>
> static unsigned int nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 5076fe42c693..685deed805e8 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1497,7 +1497,64 @@ struct nfs42_seek_res {
> u32 sr_eof;
> u64 sr_offset;
> };
> -#endif
> +
> +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_2 */
>
> struct nfs_page;
>