On Wed, Oct 23, 2024 at 10:54:36AM -0400, Mike Snitzer wrote:
> We do not and cannot support NFS proxy server file locking over
> NFSv4.x for the same reason we don't do it for NFSv3: NFS proxy server
> reboot cannot allow clients to recover locks because the source NFS
> server has not rebooted, and so it is not in grace. 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.
>
> Add EXPORT_OP_NOLOCKSUPPORT and exportfs_lock_op_is_unsupported(), set
> EXPORT_OP_NOLOCKSUPPORT in nfs_export_ops and check for it in
> nfsd4_lock(), nfsd4_locku() and nfs4_set_delegation().
>
> Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx>
> ---
> fs/nfs/export.c | 3 ++-
> fs/nfsd/nfs4state.c | 20 ++++++++++++++++++++
> include/linux/exportfs.h | 14 ++++++++++++++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> index be686b8e0c54..2f001a0273bc 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_NOLOCKSUPPORT,
> };
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ac1859c7cc9d..63297ea82e4e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5813,6 +5813,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
> + * export is from NFS proxy server.
> + */
> + if (exportfs_lock_op_is_unsupported(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))
> @@ -7917,6 +7926,11 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> }
> sb = cstate->current_fh.fh_dentry->d_sb;
>
> + if (exportfs_lock_op_is_unsupported(sb->s_export_op)) {
> + status = nfserr_notsupp;
> + goto out;
> + }
> +
> if (lock->lk_is_new) {
> if (nfsd4_has_session(cstate))
> /* See rfc 5661 18.10.3: given clientid is ignored: */
> @@ -8266,6 +8280,12 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> status = nfserr_lock_range;
> goto put_stateid;
> }
> +
> + if (exportfs_lock_op_is_unsupported(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 893a1d21dc1c..106fd590d323 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -247,6 +247,7 @@ struct export_operations {
> */
> #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_NOLOCKSUPPORT (0x80) /* no file locking support */
> unsigned long flags;
> };
>
> @@ -263,6 +264,19 @@ exportfs_lock_op_is_async(const struct export_operations *export_ops)
> return export_ops->flags & EXPORT_OP_ASYNC_LOCK;
> }
>
> +/**
> + * exportfs_lock_op_is_unsupported() - export does not support file locking
> + * @export_ops: the nfs export operations to check
> + *
> + * Returns true if the nfs export_operations structure has
> + * EXPORT_OP_NOLOCKSUPPORT in their flags set
> + */
> +static inline bool
> +exportfs_lock_op_is_unsupported(const struct export_operations *export_ops)
> +{
> + return export_ops->flags & EXPORT_OP_NOLOCKSUPPORT;
> +}
> +
> extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
> int *max_len, struct inode *parent,
> int flags);
> --
> 2.44.0
>
Re: this patch:
I expect that this behavior will be surprising to users and
administrators. It would be great to have some documentation for NFS
re-export / proxy somewhere prominent that explains this (and other)
limitations for NFS proxy. Any thoughts, please share.
General comment:
We have talked about adding NFS re-export to our kdevops CI for
upstream NFSD, but that hasn't happened yet. I'm wondering if there
is any other regular testing of re-export.
--
Chuck Lever
