From owner-dev-commits-src-main@freebsd.org Mon Apr 26 23:28:15 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 73B145F4592; Mon, 26 Apr 2021 23:28:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4FTh0H2vjNz4hTM; Mon, 26 Apr 2021 23:28:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5653B10456; Mon, 26 Apr 2021 23:28:15 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 13QNSFGJ025231; Mon, 26 Apr 2021 23:28:15 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 13QNSFf3025230; Mon, 26 Apr 2021 23:28:15 GMT (envelope-from git) Date: Mon, 26 Apr 2021 23:28:15 GMT Message-Id: <202104262328.13QNSFf3025230@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: 875977314881 - main - nfsd: fix the slot sequence# when a callback fails MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 87597731488105dd1ab921a95e39bb62e1abe668 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Apr 2021 23:28:15 -0000 The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=87597731488105dd1ab921a95e39bb62e1abe668 commit 87597731488105dd1ab921a95e39bb62e1abe668 Author: Rick Macklem AuthorDate: 2021-04-26 23:24:10 +0000 Commit: Rick Macklem CommitDate: 2021-04-26 23:24:10 +0000 nfsd: fix the slot sequence# when a callback fails Commit 4281bfec3628 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 --- sys/fs/nfs/nfs_commonkrpc.c | 4 ++-- sys/fs/nfs/nfs_commonsubs.c | 4 +++- sys/fs/nfs/nfs_var.h | 2 +- sys/fs/nfsserver/nfs_nfsdstate.c | 23 ++++++++++++++++++++--- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/sys/fs/nfs/nfs_commonkrpc.c b/sys/fs/nfs/nfs_commonkrpc.c index daf8082fa1c3..6e766abcf4b1 100644 --- a/sys/fs/nfs/nfs_commonkrpc.c +++ b/sys/fs/nfs/nfs_commonkrpc.c @@ -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. */ diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c index a30ee458e06c..539cbbbde7d2 100644 --- a/sys/fs/nfs/nfs_commonsubs.c +++ b/sys/fs/nfs/nfs_commonsubs.c @@ -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; diff --git a/sys/fs/nfs/nfs_var.h b/sys/fs/nfs/nfs_var.h index 201c5a5fc2b5..f23d56050449 100644 --- a/sys/fs/nfs/nfs_var.h +++ b/sys/fs/nfs/nfs_var.h @@ -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); diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c index 430f09844b83..3aebddad0962 100644 --- a/sys/fs/nfsserver/nfs_nfsdstate.c +++ b/sys/fs/nfsserver/nfs_nfsdstate.c @@ -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,