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.

MFC after:	2 weeks
This commit is contained in:
Rick Macklem 2021-04-26 16:24:10 -07:00
parent 43bbae1948
commit 8759773148
4 changed files with 26 additions and 7 deletions

View File

@ -871,7 +871,7 @@ newnfs_request(struct nfsrv_descript *nd, struct nfsmount *nmp,
sep->nfsess_slotseq[nd->nd_slotid] += 10; sep->nfsess_slotseq[nd->nd_slotid] += 10;
mtx_unlock(&sep->nfsess_mtx); mtx_unlock(&sep->nfsess_mtx);
/* And free the slot. */ /* And free the slot. */
nfsv4_freeslot(sep, nd->nd_slotid); nfsv4_freeslot(sep, nd->nd_slotid, false);
} }
NFSINCRGLOBAL(nfsstatsv1.rpcinvalid); NFSINCRGLOBAL(nfsstatsv1.rpcinvalid);
error = ENXIO; error = ENXIO;
@ -1108,7 +1108,7 @@ newnfs_request(struct nfsrv_descript *nd, struct nfsmount *nmp,
if ((nd->nd_flag & ND_NFSV4) != 0) { if ((nd->nd_flag & ND_NFSV4) != 0) {
/* Free the slot, as required. */ /* Free the slot, as required. */
if (freeslot != -1) if (freeslot != -1)
nfsv4_freeslot(sep, freeslot); nfsv4_freeslot(sep, freeslot, false);
/* /*
* If this op is Putfh, throw its results away. * 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. * Free a session slot.
*/ */
void void
nfsv4_freeslot(struct nfsclsession *sep, int slot) nfsv4_freeslot(struct nfsclsession *sep, int slot, bool resetseq)
{ {
uint64_t bitval; uint64_t bitval;
@ -4819,6 +4819,8 @@ nfsv4_freeslot(struct nfsclsession *sep, int slot)
if (slot > 0) if (slot > 0)
bitval <<= slot; bitval <<= slot;
mtx_lock(&sep->nfsess_mtx); mtx_lock(&sep->nfsess_mtx);
if (resetseq)
sep->nfsess_slotseq[slot]--;
if ((bitval & sep->nfsess_slots) == 0) if ((bitval & sep->nfsess_slots) == 0)
printf("freeing free slot!!\n"); printf("freeing free slot!!\n");
sep->nfsess_slots &= ~bitval; sep->nfsess_slots &= ~bitval;

View File

@ -355,7 +355,7 @@ void nfsv4_setsequence(struct nfsmount *, struct nfsrv_descript *,
struct nfsclsession *, int); struct nfsclsession *, int);
int nfsv4_sequencelookup(struct nfsmount *, struct nfsclsession *, int *, int nfsv4_sequencelookup(struct nfsmount *, struct nfsclsession *, int *,
int *, uint32_t *, uint8_t *); 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 ucred *nfsrv_getgrpscred(struct ucred *);
struct nfsdevice *nfsv4_findmirror(struct nfsmount *); struct nfsdevice *nfsv4_findmirror(struct nfsmount *);
void nfsm_set(struct nfsrv_descript *, u_int); 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) { if ((clp->lc_flags & LCL_NFSV41) != 0) {
error = ECONNREFUSED; error = ECONNREFUSED;
if (procnum != NFSV4PROC_CBNULL) if (procnum != NFSV4PROC_CBNULL)
nfsv4_freeslot(&sep->sess_cbsess, slotpos); nfsv4_freeslot(&sep->sess_cbsess, slotpos,
true);
nfsrv_freesession(sep, NULL); nfsrv_freesession(sep, NULL);
} else if (nd->nd_procnum == NFSV4PROC_CBNULL) } else if (nd->nd_procnum == NFSV4PROC_CBNULL)
error = newnfs_connect(NULL, &clp->lc_req, cred, error = newnfs_connect(NULL, &clp->lc_req, cred,
@ -4604,8 +4605,24 @@ nfsrv_docallback(struct nfsclient *clp, int procnum, nfsv4stateid_t *stateidp,
error = ECONNREFUSED; error = ECONNREFUSED;
} }
NFSD_DEBUG(4, "aft newnfs_request=%d\n", error); NFSD_DEBUG(4, "aft newnfs_request=%d\n", error);
if (error != 0 && procnum != NFSV4PROC_CBNULL) if (error != 0 && procnum != NFSV4PROC_CBNULL) {
nfsv4_freeslot(&sep->sess_cbsess, slotpos); /*
* 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); nfsrv_freesession(sep, NULL);
} else } else
error = newnfs_request(nd, NULL, clp, &clp->lc_req, error = newnfs_request(nd, NULL, clp, &clp->lc_req,