All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>
To: NeilBrown <neilb@suse.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH - v2] NFSv4: handle EINVAL from EXCHANGE_ID better.
Date: Fri, 16 Mar 2018 10:31:52 +0100 (CET)	[thread overview]
Message-ID: <584484878.12780264.1521192712528.JavaMail.zimbra@desy.de> (raw)
In-Reply-To: <878tasc3ag.fsf@notabene.neil.brown.name>


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.


Tigran.


----- Original Message -----
> From: "NeilBrown" <neilb@suse.com>
> To: "Trond Myklebust" <trond.myklebust@primarydata.com>, "Anna Schumaker" <anna.schumaker@netapp.com>
> Cc: "linux-nfs" <linux-nfs@vger.kernel.org>
> 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@suse.com>
> ---
> 
> 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

  reply	other threads:[~2018-03-16  9:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-15 23:29 [PATCH] " NeilBrown
2018-03-15 23:44 ` [PATCH - v2] " NeilBrown
2018-03-16  9:31   ` Mkrtchyan, Tigran [this message]
2018-03-16 12:10     ` Trond Myklebust
2018-03-20  0:32       ` NeilBrown
2018-03-20 14:14         ` Trond Myklebust
2018-03-20 18:44           ` NeilBrown
2018-03-20 19:43             ` Trond Myklebust
2018-03-20 19:58               ` NeilBrown
2018-03-20 20:13                 ` Trond Myklebust
2018-03-20 21:20                   ` NeilBrown
2018-03-20  0:09     ` NeilBrown
2018-03-20 21:48   ` J. Bruce Fields
2018-03-20 22:12     ` NeilBrown
2018-04-03  0:41     ` NeilBrown
2018-04-03 16:02       ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=584484878.12780264.1521192712528.JavaMail.zimbra@desy.de \
    --to=tigran.mkrtchyan@desy.de \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.com \
    --cc=trond.myklebust@primarydata.com \
    --subject='Re: [PATCH - v2] NFSv4: handle EINVAL from EXCHANGE_ID better.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.