nfsd: fix the slot sequence# when a callback fails

Commit 4281bfec36 patched the server so that the
callback session slot would be free'd for reuse when
a callback attempt fails.
However, this can often result in the sequence# for
the session slot to be advanced such that the client
end will reply NFSERR_SEQMISORDERED.

To avoid the NFSERR_SEQMISORDERED client reply,
this patch negates the sequence# advance for the
case where the callback has failed.
The common case is a failed back channel, where
the callback cannot be sent to the client, and
not advancing the sequence# is correct for this
case.  For the uncommon case where the client's
reply to the callback is lost, not advancing the
sequence# will indicate to the client that the
next callback is a retry and not a new callback.
But, since the FreeBSD server always sets "csa_cachethis"
false in the callback sequence operation, a retry
and a new callback should be handled the same way
by the client, so this should not matter.

Until you have this patch in your NFSv4.1/4.2 server,
you should consider avoiding the use of delegations.
Even with this patch, interoperation with the
Linux NFSv4.1/4.2 client in kernel versions prior
to 5.3 can result in frequent 15second delays if
delegations are enabled.  This occurs because, for
kernels prior to 5.3, the Linux client does a TCP
reconnect every time it sees multiple concurrent
callbacks and then it takes 15seconds to recover
the back channel after doing so.

(cherry picked from commit 8759773148)
This commit is contained in:
Rick Macklem 2021-04-26 16:24:10 -07:00
parent 5e1753891a
commit 272f399422
4 changed files with 26 additions and 7 deletions

View file

@ -871,7 +871,7 @@ tryagain:
sep->nfsess_slotseq[nd->nd_slotid] += 10;
mtx_unlock(&sep->nfsess_mtx);
/* And free the slot. */
nfsv4_freeslot(sep, nd->nd_slotid);
nfsv4_freeslot(sep, nd->nd_slotid, false);
}
NFSINCRGLOBAL(nfsstatsv1.rpcinvalid);
error = ENXIO;
@ -1108,7 +1108,7 @@ tryagain:
if ((nd->nd_flag & ND_NFSV4) != 0) {
/* Free the slot, as required. */
if (freeslot != -1)
nfsv4_freeslot(sep, freeslot);
nfsv4_freeslot(sep, freeslot, false);
/*
* If this op is Putfh, throw its results away.
*/

View file

@ -4811,7 +4811,7 @@ nfsv4_sequencelookup(struct nfsmount *nmp, struct nfsclsession *sep,
* Free a session slot.
*/
void
nfsv4_freeslot(struct nfsclsession *sep, int slot)
nfsv4_freeslot(struct nfsclsession *sep, int slot, bool resetseq)
{
uint64_t bitval;
@ -4819,6 +4819,8 @@ nfsv4_freeslot(struct nfsclsession *sep, int slot)
if (slot > 0)
bitval <<= slot;
mtx_lock(&sep->nfsess_mtx);
if (resetseq)
sep->nfsess_slotseq[slot]--;
if ((bitval & sep->nfsess_slots) == 0)
printf("freeing free slot!!\n");
sep->nfsess_slots &= ~bitval;

View file

@ -355,7 +355,7 @@ void nfsv4_setsequence(struct nfsmount *, struct nfsrv_descript *,
struct nfsclsession *, int);
int nfsv4_sequencelookup(struct nfsmount *, struct nfsclsession *, int *,
int *, uint32_t *, uint8_t *);
void nfsv4_freeslot(struct nfsclsession *, int);
void nfsv4_freeslot(struct nfsclsession *, int, bool);
struct ucred *nfsrv_getgrpscred(struct ucred *);
struct nfsdevice *nfsv4_findmirror(struct nfsmount *);
void nfsm_set(struct nfsrv_descript *, u_int);

View file

@ -4572,7 +4572,8 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
if ((clp->lc_flags & LCL_NFSV41) != 0) {
error = ECONNREFUSED;
if (procnum != NFSV4PROC_CBNULL)
nfsv4_freeslot(&sep->sess_cbsess, slotpos);
nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
nfsrv_freesession(sep, NULL);
} else if (nd->nd_procnum == NFSV4PROC_CBNULL)
error = newnfs_connect(NULL, &clp->lc_req, cred,
@ -4604,8 +4605,24 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
error = ECONNREFUSED;
}
NFSD_DEBUG(4, "aft newnfs_request=%d\n", error);
if (error != 0 && procnum != NFSV4PROC_CBNULL)
nfsv4_freeslot(&sep->sess_cbsess, slotpos);
if (error != 0 && procnum != NFSV4PROC_CBNULL) {
/*
* It is likely that the callback was never
* processed by the client and, as such,
* the sequence# for the session slot needs
* to be backed up by one to avoid a
* NFSERR_SEQMISORDERED error reply.
* For the unlikely case where the callback
* was processed by the client, this will
* make the next callback on the slot
* appear to be a retry.
* Since callbacks never specify that the
* reply be cached, this "apparent retry"
* should not be a problem.
*/
nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
}
nfsrv_freesession(sep, NULL);
} else
error = newnfs_request(nd, NULL, clp, &clp->lc_req,