Re: [PATCH v5 09/10] nfsd: handle delegated timestamps in SETATTR

Chuck Lever
2024-12-12T22:26:10Z

On 12/9/24 4:14 PM, Jeff Layton wrote:
> Allow SETATTR to handle delegated timestamps. This patch assumes that
> only the delegation holder has the ability to set the timestamps in this
> way, so we allow this only if the SETATTR stateid refers to a
> *_ATTRS_DELEG delegation.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/nfs4proc.c | 31 ++++++++++++++++++++++++++++---
> fs/nfsd/nfs4state.c | 2 +-
> fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
> fs/nfsd/nfsd.h | 5 ++++-
> 4 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index f8a10f90bc7a4b288c20d2733c85f331cc0a8dba..fea171ffed623818c61886b786339b0b73f1053d 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1135,18 +1135,43 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> .na_iattr = &setattr->sa_iattr,
> .na_seclabel = &setattr->sa_label,
> };
> + bool save_no_wcc, deleg_attrs;
> + struct nfs4_stid *st = NULL;
> struct inode *inode;
> __be32 status = nfs_ok;
> - bool save_no_wcc;
> int err;
>
> - if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
> + deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
> + FATTR4_WORD2_TIME_DELEG_MODIFY);
> +
> + if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
> + int flags = WR_STATE;
> +
> + if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS)
> + flags |= RD_STATE;
> +
> status = nfs4_preprocess_stateid_op(rqstp, cstate,
> &cstate->current_fh, &setattr->sa_stateid,
> - WR_STATE, NULL, NULL);
> + flags, NULL, &st);
> if (status)
> return status;
> }
> +
> + if (deleg_attrs) {
> + status = nfserr_bad_stateid;
> + if (st->sc_type & SC_TYPE_DELEG) {
> + struct nfs4_delegation *dp = delegstateid(st);
> +
> + /* Only for *_ATTRS_DELEG flavors */
> + if (deleg_attrs_deleg(dp->dl_type))
> + status = nfs_ok;
> + }
> + }
> + if (st)
> + nfs4_put_stid(st);
> + if (status)
> + return status;
> +
> err = fh_want_write(&cstate->current_fh);
> if (err)
> return nfserrno(err);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c882eeba7830b0249ccd74654f81e63b12a30f14..a76e35f86021c5657e31e4fddf08cb5781f01e32 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5486,7 +5486,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
> static inline __be32
> nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
> {
> - if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
> + if (!(flags & RD_STATE) && deleg_is_read(dp->dl_type))
> return nfserr_openmode;
> else
> return nfs_ok;
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 0561c99b5def2eccf679bf3ea0e5b1a57d5d8374..ce93a31ac5cec75b0f944d288e796e7a73641572 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
> *umask = mask & S_IRWXUGO;
> iattr->ia_valid |= ATTR_MODE;
> }
> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> + fattr4_time_deleg_access access;
> +
> + if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
> + return nfserr_bad_xdr;
> + iattr->ia_atime.tv_sec = access.seconds;
> + iattr->ia_atime.tv_nsec = access.nseconds;
> + iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
> + }
> + if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> + fattr4_time_deleg_modify modify;
> +
> + if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
> + return nfserr_bad_xdr;
> + iattr->ia_mtime.tv_sec = modify.seconds;
> + iattr->ia_mtime.tv_nsec = modify.nseconds;
> + iattr->ia_ctime.tv_sec = modify.seconds;
> + iattr->ia_ctime.tv_nsec = modify.seconds;
> + iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
> + }
>
> /* request sanity: did attrlist4 contain the expected number of words? */
> if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 004415651295891b3440f52a4c986e3a668a48cb..f007699aa397fe39042d80ccd568db4654d19dd5 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -531,7 +531,10 @@ static inline bool nfsd_attrs_supported(u32 minorversion, const u32 *bmval)
> #endif
> #define NFSD_WRITEABLE_ATTRS_WORD2 \
> (FATTR4_WORD2_MODE_UMASK \
> - | MAYBE_FATTR4_WORD2_SECURITY_LABEL)
> + | MAYBE_FATTR4_WORD2_SECURITY_LABEL \
> + | FATTR4_WORD2_TIME_DELEG_ACCESS \
> + | FATTR4_WORD2_TIME_DELEG_MODIFY \
> + )
>
> #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
> NFSD_WRITEABLE_ATTRS_WORD0
>

Hi Jeff-

After this patch is applied, I see failures of the git regression suite
on NFSv4.2 mounts.

Test Summary Report
-------------------
./t3412-rebase-root.sh (Wstat: 256 (exited
1) Tests: 25 Failed: 5)
Failed tests: 6, 19, 21-22, 24
Non-zero exit status: 1
./t3400-rebase.sh (Wstat: 256 (exited
1) Tests: 38 Failed: 1)
Failed test: 31
Non-zero exit status: 1
./t3406-rebase-message.sh (Wstat: 256 (exited
1) Tests: 32 Failed: 2)
Failed tests: 15, 20
Non-zero exit status: 1
./t3428-rebase-signoff.sh (Wstat: 256 (exited
1) Tests: 7 Failed: 2)
Failed tests: 6-7
Non-zero exit status: 1
./t3418-rebase-continue.sh (Wstat: 256 (exited
1) Tests: 29 Failed: 1)
Failed test: 7
Non-zero exit status: 1
./t3415-rebase-autosquash.sh (Wstat: 256 (exited
1) Tests: 27 Failed: 2)
Failed tests: 3-4
Non-zero exit status: 1
./t3404-rebase-interactive.sh (Wstat: 256 (exited
1) Tests: 131 Failed: 15)
Failed tests: 32, 34-43, 45, 121-123
Non-zero exit status: 1
./t1013-read-tree-submodule.sh (Wstat: 256 (exited
1) Tests: 68 Failed: 1)
Failed test: 34
Non-zero exit status: 1
./t2013-checkout-submodule.sh (Wstat: 256 (exited
1) Tests: 74 Failed: 4)
Failed tests: 26-27, 30-31
Non-zero exit status: 1
./t5500-fetch-pack.sh (Wstat: 256 (exited
1) Tests: 375 Failed: 1)
Failed test: 28
Non-zero exit status: 1
./t5572-pull-submodule.sh (Wstat: 256 (exited
1) Tests: 67 Failed: 2)
Failed tests: 5, 7
Non-zero exit status: 1
Files=1007, Tests=30810, 1417 wallclock secs (11.18 usr 10.17 sys +
1037.05 cusr 6529.12 csys = 7587.52 CPU)
Result: FAIL

The NFS client and NFS server under test are running the same v6.13-rc2
kernel from my git.kernel.org nfsd-testing branch.

--
Chuck Lever