Re: [PATCH 1/8] nfsd: don't restart v4.1+ callback when RPC_SIGNALLED is set

Chuck Lever
2025-01-27T17:01:25Z

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