Re: [PATCH/RFC] NFS: handle NFSv4.1 server that doesn't support NFS4_OPEN_CLAIM_DELEG_CUR_FH

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2019-12-19 at 13:56 +1100, NeilBrown wrote:
> On Wed, Dec 18 2019, Trond Myklebust wrote:
> 
> > On Thu, 2019-12-19 at 09:47 +1100, NeilBrown wrote:
> > > If an NFSv4.1 server doesn't support NFS4_OPEN_CLAIM_DELEG_CUR_FH
> > > (e.g. Linux 3.0), and a newer NFS client tries to use it to claim
> > > an open before returning a delegation, the server might return
> > > NFS4ERR_BADXDR.
> > > That is what Linux 3.0 does, though the RFC doesn't seem to be
> > > explicit
> > > on which flags must be supported, and what error can be returned
> > > for
> > > unsupported flags.
> > 
> > NFS4ERR_BADXDR is defined in RFC5661, section 15.1.1.1 as meaning
> > 
> > "The arguments for this operation do not match those specified in
> > the
> > XDR definition."
> > 
> > That's clearly not the case here, so I'd chalk this down to a
> > fairly
> > blatant server bug, at which point it makes no sense to fix it in
> > the
> > client.
> 
> Ok, but the RFC seems to suggest it is OK to not support this flag,
> so
> suppose I fixed the server to return NFS4ERR_NOTSUPP instead.
> The client still wouldn't handle this response gracefully.
> 

NFS4ERR_NOTSUPP is wrong too as the OPEN operation is clearly
supported. The only error that might make sense is NFS4ERR_INVAL:

"15.1.1.4.  NFS4ERR_INVAL (Error Code 22)

   The arguments for this operation are not valid for some reason, even
   though they do match those specified in the XDR definition for the
   request."

That said, why do we care about supporting NFSv4.1 on this server? It
is clearly broken.


> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index caacf5e7f5e1..14f958d16648 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2174,6 +2174,13 @@ static int nfs4_open_reclaim(struct
> nfs4_state_owner *sp, struct nfs4_state *sta
>  static int nfs4_handle_delegation_recall_error(struct nfs_server
> *server, struct nfs4_state *state, const
> nfs4_stateid *stateid, struct file_lock *fl, int err)
>  {
> 	switch (err) {
> +		case -NFS4ERR_NOTSUPP: {
> +			struct nfs4_exception exception;
> +			if (nfs4_clear_cap_atomic_open_v1(server,
> -EINVAL,
> +							  &exception))
> +				return -EAGAIN;
> +		}
> +			/* fallthrough */
> 		default:
> 			printk(KERN_ERR "NFS: %s: unhandled error "
> 					"%d.\n", __func__, err);
> 
-- 
Trond Myklebust
CTO, Hammerspace Inc
4300 El Camino Real, Suite 105
Los Altos, CA 94022
www.hammer.space