On Mon, 2025-02-10 at 11:25 -0500, cel@xxxxxxxxxx wrote:
> From: Mike Snitzer <snitzer@xxxxxxxxxx>
>
> We do not and cannot support file locking with NFS reexport over
> NFSv4.x for the same reason we don't do it for NFSv3: NFS reexport
> server reboot cannot allow clients to recover locks because the source
> NFS server has not rebooted, and so it is not in grace.
>
Refresh my memory: how do we prevent this in v3? It seems like NLM
should use the same mechanism.
> Since the
> source NFS server is not in grace, it cannot offer any guarantees that
> the file won't have been changed between the locks getting lost and
> any attempt to recover/reclaim them. The same applies to delegations
> and any associated locks, so disallow them too.
>
> Clients are no longer allowed to get file locks or delegations from a
> reexport server, any attempts will fail with operation not supported.
>
> Update the "Reboot recovery" section accordingly in
> Documentation/filesystems/nfs/reexport.rst
>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> Documentation/filesystems/nfs/reexport.rst | 10 +++++++---
> fs/nfs/export.c | 3 ++-
> fs/nfsd/nfs4state.c | 18 ++++++++++++++++++
> include/linux/exportfs.h | 14 +++++++++++++-
> 4 files changed, 40 insertions(+), 5 deletions(-)
>
>
> Jeff and I believe that it is better to prevent data corruption for
> now than to continue to enable risky use cases. Thus I've forward-
> ported this patch to v6.14-rc2 and intend to include it in the
> nfsd-6.15 pull request.
>
> We remain open to considering solutions that enable locking through
> to the back-end NFS server with appropriate lock recovery.
>
>
> diff --git a/Documentation/filesystems/nfs/reexport.rst b/Documentation/filesystems/nfs/reexport.rst
> index ff9ae4a46530..044be965d75e 100644
> --- a/Documentation/filesystems/nfs/reexport.rst
> +++ b/Documentation/filesystems/nfs/reexport.rst
> @@ -26,9 +26,13 @@ Reboot recovery
> ---------------
>
> The NFS protocol's normal reboot recovery mechanisms don't work for the
> -case when the reexport server reboots. Clients will lose any locks
> -they held before the reboot, and further IO will result in errors.
> -Closing and reopening files should clear the errors.
> +case when the reexport server reboots because the source server has not
> +rebooted, and so it is not in grace. Since the source server is not in
> +grace, it cannot offer any guarantees that the file won't have been
> +changed between the locks getting lost and any attempt to recover them.
> +The same applies to delegations and any associated locks. Clients are
> +not allowed to get file locks or delegations from a reexport server, any
> +attempts will fail with operation not supported.
>
> Filehandle limits
> -----------------
> diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> index be686b8e0c54..e9c233b6fd20 100644
> --- a/fs/nfs/export.c
> +++ b/fs/nfs/export.c
> @@ -154,5 +154,6 @@ const struct export_operations nfs_export_ops = {
> EXPORT_OP_CLOSE_BEFORE_UNLINK |
> EXPORT_OP_REMOTE_FS |
> EXPORT_OP_NOATOMIC_ATTR |
> - EXPORT_OP_FLUSH_ON_CLOSE,
> + EXPORT_OP_FLUSH_ON_CLOSE |
> + EXPORT_OP_NOLOCKS,
> };
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b42e2ab7a042..f0fb6ca4b70c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6000,6 +6000,15 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> if (!nf)
> return ERR_PTR(-EAGAIN);
>
> + /*
> + * File delegations and associated locks cannot be recovered if the
> + * export is from an NFS proxy server.
> + */
> + if (exportfs_cannot_lock(nf->nf_file->f_path.mnt->mnt_sb->s_export_op)) {
> + nfsd_file_put(nf);
> + return ERR_PTR(-EOPNOTSUPP);
> + }
> +
> spin_lock(&state_lock);
> spin_lock(&fp->fi_lock);
> if (nfs4_delegation_exists(clp, fp))
> @@ -8140,6 +8149,10 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0);
> if (status != nfs_ok)
> return status;
> + if (exportfs_cannot_lock(cstate->current_fh.fh_dentry->d_sb->s_export_op)) {
> + status = nfserr_notsupp;
> + goto out;
> + }
>
> if (lock->lk_is_new) {
> if (nfsd4_has_session(cstate))
> @@ -8479,6 +8492,11 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> status = nfserr_lock_range;
> goto put_stateid;
> }
> + if (exportfs_cannot_lock(nf->nf_file->f_path.mnt->mnt_sb->s_export_op)) {
> + status = nfserr_notsupp;
> + goto put_file;
> + }
> +
> file_lock = locks_alloc_lock();
> if (!file_lock) {
> dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 4cc8801e50e3..ca6c605072c7 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -259,10 +259,22 @@ struct export_operations {
> atomic attribute updates
> */
> #define EXPORT_OP_FLUSH_ON_CLOSE (0x20) /* fs flushes file data on close */
> -#define EXPORT_OP_ASYNC_LOCK (0x40) /* fs can do async lock request */
> +#define EXPORT_OP_NOLOCKS (0x40) /* no file locking support */
> unsigned long flags;
> };
>
> +/**
> + * exportfs_cannot_lock() - check if export implements file locking
> + * @export_ops: the nfs export operations to check
> + *
> + * Returns true if the export does not support file locking.
> + */
> +static inline bool
> +exportfs_cannot_lock(const struct export_operations *export_ops)
> +{
> + return export_ops->flags & EXPORT_OP_NOLOCKS;
> +}
> +
> extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
> int *max_len, struct inode *parent,
> int flags);
Aside from the question about NLM, this looks good:
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
