From: Chuck Lever <chuck.lever@oracle.com>
To: Jeff Layton <jlayton@kernel.org>, NeilBrown <neilb@suse.de>
Cc: Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
"J. Bruce Fields" <bfields@fieldses.org>,
Kinglong Mee <kinglongmee@gmail.com>,
Trond Myklebust <trondmy@kernel.org>,
Anna Schumaker <anna@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Simon Horman <horms@kernel.org>,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH 1/8] nfsd: don't restart v4.1+ callback when RPC_SIGNALLED is set
Date: Mon, 27 Jan 2025 12:00:53 -0500 [thread overview]
Message-ID: <f3928cc6-66d5-4dfc-8178-d4b960550dc7@oracle.com> (raw)
In-Reply-To: <7bf69557c04b6c9084cdc153f30bd3ac7cb48065.camel@kernel.org>
On 1/27/25 10:43 AM, Jeff Layton wrote:
> On Sun, 2025-01-26 at 11:41 -0500, Chuck Lever wrote:
>> - ESERVERFAULT: SEQUENCE was decoded but failed sanity checking. The
>> reply should be dropped now, and the session marked FAULT. No requeue
>> is ever needed here.
>>
>> [ I question whether the sequence number should be bumped in this
>> case -- the client's callback server replied with the identity of
>> some other slot. And anyway, this session is about to become
>> toast. ]
>
> It didn't necessarily reply with the ID of a different slot. It's just
> that the decoding failed in some way.
My read is that if the XDR decode failed in any way, the decoder sets
cb_seq_status to -EIO.
-ESERVERFAULT means the decoding went fine, but one or more of the
session ID, slot number, or sequence did not match what NFSD's callback
client expected.
It's not the same slot if either the session ID or slot number doesn't
match what the server sent in its CB_SEQUENCE Call. That seems
equivalent to BAD_SLOT without any question.
If the sequence number is wrong, then it's equivalent to SEQ_MISORDERED,
maybe?
> It could have been any of the
> cases in decode_cb_sequence4resok(). Maybe that function needs to
> return more distinct error codes so we know what was mangled.
My preference would be that decode_cb_sequence() should simply
decode these fields, and let nfsd4_cb_sequence_done() do the sanity
checking. I don't think decode_cb_sequence4resok() should be doing
any sanity checking beyond "does this unmarshal in the space allowed?"
--
Chuck Lever
next prev parent reply other threads:[~2025-01-27 17:01 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-23 20:25 [PATCH 0/8] nfsd: CB_SEQUENCE error handling fixes and cleanups Jeff Layton
2025-01-23 20:25 ` [PATCH 1/8] nfsd: don't restart v4.1+ callback when RPC_SIGNALLED is set Jeff Layton
2025-01-25 16:24 ` Chuck Lever
2025-01-25 22:04 ` Jeff Layton
2025-01-25 23:01 ` NeilBrown
2025-01-26 11:18 ` Jeff Layton
2025-01-26 16:41 ` Chuck Lever
2025-01-27 15:43 ` Jeff Layton
2025-01-27 17:00 ` Chuck Lever [this message]
2025-01-23 20:25 ` [PATCH 2/8] nfsd: fix CB_SEQUENCE error handling of NFS4ERR_{BADSLOT,BADSESSION,SEQ_MISORDERED} Jeff Layton
2025-01-24 14:32 ` Chuck Lever
2025-01-24 14:46 ` Jeff Layton
2025-01-24 15:31 ` Chuck Lever
2025-01-24 16:04 ` Jeff Layton
2025-01-24 16:08 ` Jeff Layton
2025-01-23 20:25 ` [PATCH 3/8] nfsd: when CB_SEQUENCE gets NFS4ERR_DELAY, release the slot Jeff Layton
2025-01-23 22:18 ` Chuck Lever
2025-01-23 23:20 ` Jeff Layton
2025-01-24 1:30 ` Tom Talpey
2025-01-24 14:00 ` J. Bruce Fields
2025-01-24 14:11 ` Jeff Layton
2025-01-24 20:29 ` Tom Talpey
2025-01-24 17:45 ` Olga Kornievskaia
2025-01-24 17:47 ` Olga Kornievskaia
2025-01-23 20:25 ` [PATCH 4/8] nfsd: fix default case in nfsd4_cb_sequence_done() Jeff Layton
2025-01-23 20:25 ` [PATCH 5/8] nfsd: reverse default of "ret" variable " Jeff Layton
2025-01-23 20:25 ` [PATCH 6/8] nfsd: remove unneeded forward declaration of nfsd4_mark_cb_fault() Jeff Layton
2025-01-23 20:25 ` [PATCH 7/8] nfsd: clean up and amend comments around nfsd4_cb_sequence_done() Jeff Layton
2025-01-24 14:43 ` Chuck Lever
2025-01-24 14:50 ` Jeff Layton
2025-01-24 15:05 ` Chuck Lever
2025-01-24 15:31 ` Jeff Layton
2025-01-24 15:42 ` Chuck Lever
2025-01-26 16:50 ` Chuck Lever
2025-01-23 20:25 ` [PATCH 8/8] sunrpc: make rpc_restart_call() and rpc_restart_call_prepare() void return Jeff Layton
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=f3928cc6-66d5-4dfc-8178-d4b960550dc7@oracle.com \
--to=chuck.lever@oracle.com \
--cc=Dai.Ngo@oracle.com \
--cc=anna@kernel.org \
--cc=bfields@fieldses.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=jlayton@kernel.org \
--cc=kinglongmee@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
--cc=netdev@vger.kernel.org \
--cc=okorniev@redhat.com \
--cc=pabeni@redhat.com \
--cc=tom@talpey.com \
--cc=trondmy@kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox