[PATCH v2 3/4] NFSD: Do not cache solo SEQUENCE operations

[Chuck Lever <cel@kernel.org>]

From: Chuck Lever <chuck.lever@oracle.com>

 RFC 8881 Section 2.10.6.1.3 says:

> On a per-request basis, the requester can choose to direct the
> replier to cache the reply to all operations after the first
> operation (SEQUENCE or CB_SEQUENCE) via the sa_cachethis or
> csa_cachethis fields of the arguments to SEQUENCE or CB_SEQUENCE.

RFC 8881 Section 2.10.6.4 further says:

> If sa_cachethis or csa_cachethis is TRUE, then the replier MUST
> cache a reply except if an error is returned by the SEQUENCE or
> CB_SEQUENCE operation (see Section 2.10.6.1.2).

This suggests to me that the spec authors do not expect an NFSv4.1
server implementation to ever cache the result of a SEQUENCE
operation (except perhaps as part of a successful multi-operation
COMPOUND).

NFSD attempts to cache the result of solo SEQUENCE operations,
however. This is because the protocol does not permit servers to
respond to a SEQUENCE with NFS4ERR_RETRY_UNCACHED_REP. If the server
always caches solo SEQUENCE operations, then it never has occasion
to return that status code.

However, clients use solo SEQUENCE to query the current status of a
session slot. A cached reply will return stale information to the
client, and could result in an infinite loop.

Change the check in nfsd4_

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/xdr4.h | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index d1837a10b0c2..9619e78f0ed2 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -931,18 +931,19 @@ static inline bool nfsd4_is_solo_sequence(struct nfsd4_compoundres *resp)
 }
 
 /*
- * The session reply cache only needs to cache replies that the client
- * actually asked us to.  But it's almost free for us to cache compounds
- * consisting of only a SEQUENCE op, so we may as well cache those too.
- * Also, the protocol doesn't give us a convenient response in the case
- * of a replay of a solo SEQUENCE op that wasn't cached
- * (RETRY_UNCACHED_REP can only be returned in the second op of a
- * compound).
+ * Solo SEQUENCE operations are not supposed respect the setting in the
+ * sa_cachethis field, since that field controls whether the operations
+ * /after/ the SEQUENCE are preserved in the slot reply cache. Because
+ * clients might use a solo SEQUENCE to query the current state of the
+ * session or slot, a cached reply would return stale data to the client.
+ *
+ * Therefore NFSD treats solo SEQUENCE as an uncached operation no matter
+ * how the sa_cachethis field is set.
  */
 static inline bool nfsd4_cache_this(struct nfsd4_compoundres *resp)
 {
-	return (resp->cstate.slot->sl_flags & NFSD4_SLOT_CACHETHIS)
-		|| nfsd4_is_solo_sequence(resp);
+	return (resp->cstate.slot->sl_flags & NFSD4_SLOT_CACHETHIS) &&
+		!nfsd4_is_solo_sequence(resp);
 }
 
 static inline bool nfsd4_last_compound_op(struct svc_rqst *rqstp)
-- 
2.51.0