Re: [PATCH - v2] NFSv4: handle EINVAL from EXCHANGE_ID better.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2018-03-16 at 10:31 +0100, Mkrtchyan, Tigran wrote:
> Hi Neil,
> 
> according to rfc5661, NFS4ERR_INVAL is returned by the server if it
> thinks that client sends an invalid request (e.g. points to a client
> bug)
> or server misinterpret it (broken server).
> 
> With your change instead of failing the mount, client will silently
> go for
> v4.0, even v4.1 mount was requested and produce undesirable behavior,
> e.g.
> proxy-io instead of pnfs. I fill prefer fail-fast instead of long
> debug
> sessions.
> 
> On the other hand, I understand, that it's not always possible to fix
> server
> or clients in production environment and time-to-time workarounds are
> necessary.
> 
> 

I'd tend to agree with Tigran. Hiding server bugs, should not be a
priority and particularly not in this case, where the workaround is
simple: either turn off version negotiation altogether, or edit
/etc/nfsmount.conf to negotiate a different set of versions.

What we might want to do, is make it easier to allow the user to detect
that this is indeed a server bug and is not a problem with the
arguments supplied to the "mount" utility. Perhaps we might have the
kernel log something in the syslogs?

Cheers
  Trond

> Tigran.
> 
> 
> ----- Original Message -----
> > From: "NeilBrown" <neilb@xxxxxxxx>
> > To: "Trond Myklebust" <trond.myklebust@xxxxxxxxxxxxxxx>, "Anna
> > Schumaker" <anna.schumaker@xxxxxxxxxx>
> > Cc: "linux-nfs" <linux-nfs@xxxxxxxxxxxxxxx>
> > Sent: Friday, March 16, 2018 12:44:23 AM
> > Subject: [PATCH - v2] NFSv4: handle EINVAL from EXCHANGE_ID better.
> > nfs4_proc_exchange_id() can return -EINVAL if the server
> > reported NFS4INVAL (which I have seen in a packet trace),
> > or nfs4_check_cl_exchange_flags() exchange flags detects
> > a problem.
> > Each of these mean that NFSv4.1 later cannot be use, but
> > they should not prevent fallback to NFSv4.0.
> > 
> > Currently this EINVAL error is returned by nfs4_proc_exchange_id()
> > to
> > nfs41_discover_server_trunking(), and thence to
> > nfs4_discover_server_trunking().
> > nfs4_discover_server_trunking() doesn't understand EINVAL, so
> > converts
> > it to EIO which causes mount.nfs to think something is horribly
> > wrong
> > and to give up.
> > 
> > It would be a more graceful failure if
> > nfs4_discover_server_trunking()
> > mapped -EINVAL to -EPROTONOSUPPORT - a failure to negotiate a
> > client
> > ID clearly shows that NFSv4.1 cannot be supported, but isn't as
> > general a failure as EIO.
> > 
> > Signed-off-by: NeilBrown <neilb@xxxxxxxx>
> > ---
> > 
> > Sorry - a bit too trigger-happy with that first version of the
> > patch.
> > 
> > NeilBrown
> > 
> > fs/nfs/nfs4state.c | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index 91a4d4eeb235..b988e460553d 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -2219,6 +2219,8 @@ int nfs4_discover_server_trunking(struct
> > nfs_client *clp,
> > 		clnt = clp->cl_rpcclient;
> > 		goto again;
> > 
> > +	case -NFS4ERR_INVAL:
> > +		/* Server confused - assume this minor isn't
> > supported */
> > 	case -NFS4ERR_MINOR_VERS_MISMATCH:
> > 		status = -EPROTONOSUPPORT;
> > 		break;
> > --
> > 2.14.0.rc0.dirty
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx
��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux