| Message ID | 20200311195613.26108-4-fllinden@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | client side user xattr (RFC8276) support | expand |
Hi Frank, ----- Original Message ----- > From: "Frank van der Linden" <fllinden@amazon.com> > To: "Trond Myklebust" <trond.myklebust@hammerspace.com>, "Anna Schumaker" <anna.schumaker@netapp.com>, "linux-nfs" > <linux-nfs@vger.kernel.org> > Cc: "Frank van der Linden" <fllinden@amazon.com> > Sent: Wednesday, March 11, 2020 8:56:03 PM > Subject: [PATCH 03/13] NFSv4.2: query the server for extended attribute support > 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 | 14 ++++++++++++-- > fs/nfs/nfs4xdr.c | 23 +++++++++++++++++++++++ > include/linux/nfs_fs_sb.h | 1 + > include/linux/nfs_xdr.h | 1 + > 4 files changed, 37 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 69b7ab7a5815..47bbd7db9d18 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -3742,6 +3742,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, > @@ -3763,6 +3764,13 @@ 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; > + > + if (minorversion >= 2) { I am not sure you need this extra check as by querying for FATTR4_WORD0_SUPPORTED_ATTRS server already will return FATTR4_WORD2_XATTR_SUPPORT if supported. Tigran. > + bitmask[2] |= FATTR4_WORD2_XATTR_SUPPORT; > + fattr4_word2_nfs42_mask |= FATTR4_WORD2_XATTR_SUPPORT; > + } > + > status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, > &res.seq_res, 0); > if (status == 0) { > /* Sanity check the server answers */ > @@ -3775,7 +3783,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| > @@ -3783,7 +3791,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; > @@ -3811,6 +3819,8 @@ static int _nfs4_server_capabilities(struct nfs_server > *server, struct nfs_fh *f > if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL) > server->caps |= NFS_CAP_SECURITY_LABEL; > #endif > + if (res.has_xattr) > + server->caps |= NFS_CAP_XATTR; > memcpy(server->attr_bitmask_nl, res.attr_bitmask, > sizeof(server->attr_bitmask)); > server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL; > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c > index 47817ef0aadb..bebc087a1433 100644 > --- a/fs/nfs/nfs4xdr.c > +++ b/fs/nfs/nfs4xdr.c > @@ -4201,6 +4201,26 @@ static int decode_attr_time_modify(struct xdr_stream > *xdr, uint32_t *bitmap, str > return status; > } > > +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; > +} > + > static int verify_attr_len(struct xdr_stream *xdr, unsigned int savep, uint32_t > attrlen) > { > unsigned int attrwords = XDR_QUADLEN(attrlen); > @@ -4371,6 +4391,9 @@ 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; > + status = decode_attr_xattrsupport(xdr, bitmap, &res->has_xattr); > + if (status != 0) > + goto xdr_error; > 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 465fa98258a3..d881f7a38bc9 100644 > --- a/include/linux/nfs_fs_sb.h > +++ b/include/linux/nfs_fs_sb.h > @@ -281,5 +281,6 @@ struct nfs_server { > #define NFS_CAP_OFFLOAD_CANCEL (1U << 25) > #define NFS_CAP_LAYOUTERROR (1U << 26) > #define NFS_CAP_COPY_NOTIFY (1U << 27) > +#define NFS_CAP_XATTR (1U << 28) > > #endif > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h > index 94c77ed55ce1..5076fe42c693 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.16.6
On Thu, Mar 12, 2020 at 05:15:10PM +0100, Mkrtchyan, Tigran wrote: > > + fattr4_word2_nfs42_mask = FATTR4_WORD2_NFS42_MASK; > > + > > + if (minorversion >= 2) { > > I am not sure you need this extra check as by querying for FATTR4_WORD0_SUPPORTED_ATTRS > server already will return FATTR4_WORD2_XATTR_SUPPORT if supported. There used to be a mount option check here, which I later removed. That means that fattr4_word2_nfs42_mask is no longer needed, so I'll remove that. As for the attribute itself: I suppose the question here is what the client should use to assume the server has RFC 8276 support: 1) The xattr_support attribute exists 2) The xattr support attribute exists *and* it's true for the root fh Currently the code does 2) in one operation. That might not be 100% correct - the RFC does mention that (section 8.2): "Before interrogating this attribute using GETATTR, a client should determine whether it is a supported attribute by interrogating the supported_attrs attribute." That's a "should", not a "MUST", but it's still waving its finger at you not to do this. Since 8.2.1 says: "However, a client may reasonably assume that a server (or file system) that does not support the xattr_support attribute does not provide xattr support, and it acts on that basis." ..I think you're right, and the code should just use the existence of the attribute as a signal that the server knows about xattrs - operations should still error out correctly if it doesn't. I'll make that change, thanks. Frank
On Thu, Mar 12, 2020 at 08:51:39PM +0000, Frank van der Linden wrote: > 1) The xattr_support attribute exists > 2) The xattr support attribute exists *and* it's true for the root fh > > Currently the code does 2) in one operation. That might not be 100% > correct - the RFC does mention that (section 8.2): > > "Before interrogating this attribute using GETATTR, a client should > determine whether it is a supported attribute by interrogating the > supported_attrs attribute." > > That's a "should", not a "MUST", but it's still waving its finger > at you not to do this. > > Since 8.2.1 says: > > "However, a client may reasonably assume that a server > (or file system) that does not support the xattr_support attribute > does not provide xattr support, and it acts on that basis." > > ..I think you're right, and the code should just use the existence > of the attribute as a signal that the server knows about xattrs - > operations should still error out correctly if it doesn't. > > I'll make that change, thanks. ..or, alternatively, only query xattr_support in nfs4_server_capabilities, and then its actual value, if it exists, in nfs4_fs_info. Any opinions on this? - Frank
Hi Frank, I think the way how you have implemented is almost correct. You query server for supported attributes. As result client will get all attributes supported bu the server and if FATTR4_XATTR_SUPPORT is returned, then client adds xattr capability. This the way how I read rfc8276. Do you have a different opinion? Regards, Tigran. ----- Original Message ----- > From: "Frank van der Linden" <fllinden@amazon.com> > To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> > Cc: "Trond Myklebust" <trond.myklebust@hammerspace.com>, "Anna Schumaker" <anna.schumaker@netapp.com>, "linux-nfs" > <linux-nfs@vger.kernel.org> > Sent: Thursday, March 12, 2020 10:15:55 PM > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended attribute support > On Thu, Mar 12, 2020 at 08:51:39PM +0000, Frank van der Linden wrote: >> 1) The xattr_support attribute exists >> 2) The xattr support attribute exists *and* it's true for the root fh >> >> Currently the code does 2) in one operation. That might not be 100% >> correct - the RFC does mention that (section 8.2): >> >> "Before interrogating this attribute using GETATTR, a client should >> determine whether it is a supported attribute by interrogating the >> supported_attrs attribute." >> >> That's a "should", not a "MUST", but it's still waving its finger >> at you not to do this. >> >> Since 8.2.1 says: >> >> "However, a client may reasonably assume that a server >> (or file system) that does not support the xattr_support attribute >> does not provide xattr support, and it acts on that basis." >> >> ..I think you're right, and the code should just use the existence >> of the attribute as a signal that the server knows about xattrs - >> operations should still error out correctly if it doesn't. >> >> I'll make that change, thanks. > > ..or, alternatively, only query xattr_support in nfs4_server_capabilities, > and then its actual value, if it exists, in nfs4_fs_info. > > Any opinions on this? > > - Frank
On Fri, 2020-03-13 at 12:11 +0100, Mkrtchyan, Tigran wrote: > Hi Frank, > > I think the way how you have implemented is almost correct. You query > server for supported attributes. As result client will get all > attributes > supported bu the server and if FATTR4_XATTR_SUPPORT is returned, then > client > adds xattr capability. This the way how I read rfc8276. Do you have a > different > opinion? > 'xattr_support' seems like a protocol hack to allow the client to determine whether or not the xattr operations are supported. The reason why it is a hack is that 'supported_attrs' is also a per- filesystem attribute, and there is no value in advertising 'xattr_support' there unless your filesystem also supports xattrs. IOW: the protocol forces you to do 2 round trips to the server in order to figure out something that really should be obvious with 1 round trip. > Regards, > Tigran. > > ----- Original Message ----- > > From: "Frank van der Linden" <fllinden@amazon.com> > > To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> > > Cc: "Trond Myklebust" <trond.myklebust@hammerspace.com>, "Anna > > Schumaker" <anna.schumaker@netapp.com>, "linux-nfs" > > <linux-nfs@vger.kernel.org> > > Sent: Thursday, March 12, 2020 10:15:55 PM > > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended > > attribute support > > On Thu, Mar 12, 2020 at 08:51:39PM +0000, Frank van der Linden > > wrote: > > > 1) The xattr_support attribute exists > > > 2) The xattr support attribute exists *and* it's true for the > > > root fh > > > > > > Currently the code does 2) in one operation. That might not be > > > 100% > > > correct - the RFC does mention that (section 8.2): > > > > > > "Before interrogating this attribute using GETATTR, a client > > > should > > > determine whether it is a supported attribute by interrogating > > > the > > > supported_attrs attribute." > > > > > > That's a "should", not a "MUST", but it's still waving its finger > > > at you not to do this. > > > > > > Since 8.2.1 says: > > > > > > "However, a client may reasonably assume that a server > > > (or file system) that does not support the xattr_support > > > attribute > > > does not provide xattr support, and it acts on that basis." > > > > > > ..I think you're right, and the code should just use the > > > existence > > > of the attribute as a signal that the server knows about xattrs - > > > operations should still error out correctly if it doesn't. > > > > > > I'll make that change, thanks. > > > > ..or, alternatively, only query xattr_support in > > nfs4_server_capabilities, > > and then its actual value, if it exists, in nfs4_fs_info. > > > > Any opinions on this? > > > > - Frank
----- Original Message ----- > From: "trondmy" <trondmy@hammerspace.com> > To: "Frank van der Linden" <fllinden@amazon.com>, "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> > Cc: "linux-nfs" <linux-nfs@vger.kernel.org>, "Anna Schumaker" <anna.schumaker@netapp.com> > Sent: Friday, March 13, 2020 2:50:38 PM > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended attribute support > On Fri, 2020-03-13 at 12:11 +0100, Mkrtchyan, Tigran wrote: >> Hi Frank, >> >> I think the way how you have implemented is almost correct. You query >> server for supported attributes. As result client will get all >> attributes >> supported bu the server and if FATTR4_XATTR_SUPPORT is returned, then >> client >> adds xattr capability. This the way how I read rfc8276. Do you have a >> different >> opinion? >> > > 'xattr_support' seems like a protocol hack to allow the client to > determine whether or not the xattr operations are supported. > > The reason why it is a hack is that 'supported_attrs' is also a per- > filesystem attribute, and there is no value in advertising > 'xattr_support' there unless your filesystem also supports xattrs. > > IOW: the protocol forces you to do 2 round trips to the server in order > to figure out something that really should be obvious with 1 round > trip. > So you say that client have to query for xattr_support every time the fsid is changing? Tigran. >> Regards, >> Tigran. >> >> ----- Original Message ----- >> > From: "Frank van der Linden" <fllinden@amazon.com> >> > To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> >> > Cc: "Trond Myklebust" <trond.myklebust@hammerspace.com>, "Anna >> > Schumaker" <anna.schumaker@netapp.com>, "linux-nfs" >> > <linux-nfs@vger.kernel.org> >> > Sent: Thursday, March 12, 2020 10:15:55 PM >> > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended >> > attribute support >> > On Thu, Mar 12, 2020 at 08:51:39PM +0000, Frank van der Linden >> > wrote: >> > > 1) The xattr_support attribute exists >> > > 2) The xattr support attribute exists *and* it's true for the >> > > root fh >> > > >> > > Currently the code does 2) in one operation. That might not be >> > > 100% >> > > correct - the RFC does mention that (section 8.2): >> > > >> > > "Before interrogating this attribute using GETATTR, a client >> > > should >> > > determine whether it is a supported attribute by interrogating >> > > the >> > > supported_attrs attribute." >> > > >> > > That's a "should", not a "MUST", but it's still waving its finger >> > > at you not to do this. >> > > >> > > Since 8.2.1 says: >> > > >> > > "However, a client may reasonably assume that a server >> > > (or file system) that does not support the xattr_support >> > > attribute >> > > does not provide xattr support, and it acts on that basis." >> > > >> > > ..I think you're right, and the code should just use the >> > > existence >> > > of the attribute as a signal that the server knows about xattrs - >> > > operations should still error out correctly if it doesn't. >> > > >> > > I'll make that change, thanks. >> > >> > ..or, alternatively, only query xattr_support in >> > nfs4_server_capabilities, >> > and then its actual value, if it exists, in nfs4_fs_info. >> > >> > Any opinions on this? >> > >> > - Frank > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com
On Fri, 2020-03-13 at 15:19 +0100, Mkrtchyan, Tigran wrote: > > ----- Original Message ----- > > From: "trondmy" <trondmy@hammerspace.com> > > To: "Frank van der Linden" <fllinden@amazon.com>, "Tigran > > Mkrtchyan" <tigran.mkrtchyan@desy.de> > > Cc: "linux-nfs" <linux-nfs@vger.kernel.org>, "Anna Schumaker" < > > anna.schumaker@netapp.com> > > Sent: Friday, March 13, 2020 2:50:38 PM > > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for extended > > attribute support > > On Fri, 2020-03-13 at 12:11 +0100, Mkrtchyan, Tigran wrote: > > > Hi Frank, > > > > > > I think the way how you have implemented is almost correct. You > > > query > > > server for supported attributes. As result client will get all > > > attributes > > > supported bu the server and if FATTR4_XATTR_SUPPORT is returned, > > > then > > > client > > > adds xattr capability. This the way how I read rfc8276. Do you > > > have a > > > different > > > opinion? > > > > > > > 'xattr_support' seems like a protocol hack to allow the client to > > determine whether or not the xattr operations are supported. > > > > The reason why it is a hack is that 'supported_attrs' is also a > > per- > > filesystem attribute, and there is no value in advertising > > 'xattr_support' there unless your filesystem also supports xattrs. > > > > IOW: the protocol forces you to do 2 round trips to the server in > > order > > to figure out something that really should be obvious with 1 round > > trip. > > > > So you say that client have to query for xattr_support every time > the > fsid is changing? > According to the spec it is a per-filesystem attribute, just like supported_attrs... Cheers Trond > Tigran. > > > > Regards, > > > Tigran. > > > > > > ----- Original Message ----- > > > > From: "Frank van der Linden" <fllinden@amazon.com> > > > > To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de> > > > > Cc: "Trond Myklebust" <trond.myklebust@hammerspace.com>, "Anna > > > > Schumaker" <anna.schumaker@netapp.com>, "linux-nfs" > > > > <linux-nfs@vger.kernel.org> > > > > Sent: Thursday, March 12, 2020 10:15:55 PM > > > > Subject: Re: [PATCH 03/13] NFSv4.2: query the server for > > > > extended > > > > attribute support > > > > On Thu, Mar 12, 2020 at 08:51:39PM +0000, Frank van der Linden > > > > wrote: > > > > > 1) The xattr_support attribute exists > > > > > 2) The xattr support attribute exists *and* it's true for the > > > > > root fh > > > > > > > > > > Currently the code does 2) in one operation. That might not > > > > > be > > > > > 100% > > > > > correct - the RFC does mention that (section 8.2): > > > > > > > > > > "Before interrogating this attribute using GETATTR, a client > > > > > should > > > > > determine whether it is a supported attribute by > > > > > interrogating > > > > > the > > > > > supported_attrs attribute." > > > > > > > > > > That's a "should", not a "MUST", but it's still waving its > > > > > finger > > > > > at you not to do this. > > > > > > > > > > Since 8.2.1 says: > > > > > > > > > > "However, a client may reasonably assume that a server > > > > > (or file system) that does not support the xattr_support > > > > > attribute > > > > > does not provide xattr support, and it acts on that basis." > > > > > > > > > > ..I think you're right, and the code should just use the > > > > > existence > > > > > of the attribute as a signal that the server knows about > > > > > xattrs - > > > > > operations should still error out correctly if it doesn't. > > > > > > > > > > I'll make that change, thanks. > > > > > > > > ..or, alternatively, only query xattr_support in > > > > nfs4_server_capabilities, > > > > and then its actual value, if it exists, in nfs4_fs_info. > > > > > > > > Any opinions on this? > > > > > > > > - Frank > > -- > > Trond Myklebust > > Linux NFS client maintainer, Hammerspace > > trond.myklebust@hammerspace.com
On Fri, Mar 13, 2020 at 01:50:38PM +0000, Trond Myklebust wrote: > 'xattr_support' seems like a protocol hack to allow the client to > determine whether or not the xattr operations are supported. > > The reason why it is a hack is that 'supported_attrs' is also a per- > filesystem attribute, and there is no value in advertising > 'xattr_support' there unless your filesystem also supports xattrs. > > IOW: the protocol forces you to do 2 round trips to the server in order > to figure out something that really should be obvious with 1 round > trip. Right, that's the annoying part. I mean, theoretically, a server can legally reject a GETATTR because you're asking for an unknown attribute (e.g. xattr_support in this case). So then what I have in my current patch (asking for both the supported_attrs and xattr_support at once) might fail, and the mount would fail. Which means I should split it up and have nfs4_do_fsinfo do the 2nd part, just in case. - Frank
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 69b7ab7a5815..47bbd7db9d18 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3742,6 +3742,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, @@ -3763,6 +3764,13 @@ 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; + + if (minorversion >= 2) { + bitmask[2] |= FATTR4_WORD2_XATTR_SUPPORT; + fattr4_word2_nfs42_mask |= FATTR4_WORD2_XATTR_SUPPORT; + } + status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, &res.seq_res, 0); if (status == 0) { /* Sanity check the server answers */ @@ -3775,7 +3783,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| @@ -3783,7 +3791,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; @@ -3811,6 +3819,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL) server->caps |= NFS_CAP_SECURITY_LABEL; #endif + if (res.has_xattr) + server->caps |= NFS_CAP_XATTR; memcpy(server->attr_bitmask_nl, res.attr_bitmask, sizeof(server->attr_bitmask)); server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL; diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 47817ef0aadb..bebc087a1433 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -4201,6 +4201,26 @@ static int decode_attr_time_modify(struct xdr_stream *xdr, uint32_t *bitmap, str return status; } +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; +} + static int verify_attr_len(struct xdr_stream *xdr, unsigned int savep, uint32_t attrlen) { unsigned int attrwords = XDR_QUADLEN(attrlen); @@ -4371,6 +4391,9 @@ 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; + status = decode_attr_xattrsupport(xdr, bitmap, &res->has_xattr); + if (status != 0) + goto xdr_error; 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 465fa98258a3..d881f7a38bc9 100644 --- a/include/linux/nfs_fs_sb.h +++ b/include/linux/nfs_fs_sb.h @@ -281,5 +281,6 @@ struct nfs_server { #define NFS_CAP_OFFLOAD_CANCEL (1U << 25) #define NFS_CAP_LAYOUTERROR (1U << 26) #define NFS_CAP_COPY_NOTIFY (1U << 27) +#define NFS_CAP_XATTR (1U << 28) #endif diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 94c77ed55ce1..5076fe42c693 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
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 | 14 ++++++++++++-- fs/nfs/nfs4xdr.c | 23 +++++++++++++++++++++++ include/linux/nfs_fs_sb.h | 1 + include/linux/nfs_xdr.h | 1 + 4 files changed, 37 insertions(+), 2 deletions(-)