Date: Wed, 22 Apr 2020 21:00:15 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r360205 - in head/sys/fs: nfs nfsclient Message-ID: <202004222100.03ML0Fwd050816@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rmacklem Date: Wed Apr 22 21:00:14 2020 New Revision: 360205 URL: https://svnweb.freebsd.org/changeset/base/360205 Log: Make the NFSv4.n client's recovery from NFSERR_BADSESSION RFC5661 conformant. RFC5661 specifies that a client's recovery upon receipt of NFSERR_BADSESSION should first consist of a CreateSession operation using the extant ClientID. If that fails, then a full recovery beginning with the ExchangeID operation is to be done. Without this patch, the FreeBSD client did not attempt the CreateSession operation with the extant ClientID and went directly to a full recovery beginning with ExchangeID. I have had this patch several years, but since no extant NFSv4.n server required the CreateSession with extant ClientID, I have never committed it. I an committing it now, since I suspect some future NFSv4.n server will require this and it should not negatively impact recovery for extant NFSv4.n servers, since they should all return NFSERR_STATECLIENTID for this first CreateSession. The patched client has been tested for recovery against both the FreeBSD and Linux NFSv4.n servers and no problems have been observed. MFC after: 1 month Modified: head/sys/fs/nfs/nfs_var.h head/sys/fs/nfsclient/nfs_clrpcops.c head/sys/fs/nfsclient/nfs_clstate.c Modified: head/sys/fs/nfs/nfs_var.h ============================================================================== --- head/sys/fs/nfs/nfs_var.h Wed Apr 22 20:50:24 2020 (r360204) +++ head/sys/fs/nfs/nfs_var.h Wed Apr 22 21:00:14 2020 (r360205) @@ -454,7 +454,7 @@ int nfsrpc_closerpc(struct nfsrv_descript *, struct nf int nfsrpc_openconfirm(vnode_t, u_int8_t *, int, struct nfsclopen *, struct ucred *, NFSPROC_T *); int nfsrpc_setclient(struct nfsmount *, struct nfsclclient *, int, - struct ucred *, NFSPROC_T *); + bool *, struct ucred *, NFSPROC_T *); int nfsrpc_getattr(vnode_t, struct ucred *, NFSPROC_T *, struct nfsvattr *, void *); int nfsrpc_getattrnovp(struct nfsmount *, u_int8_t *, int, int, Modified: head/sys/fs/nfsclient/nfs_clrpcops.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clrpcops.c Wed Apr 22 20:50:24 2020 (r360204) +++ head/sys/fs/nfsclient/nfs_clrpcops.c Wed Apr 22 21:00:14 2020 (r360205) @@ -932,7 +932,7 @@ nfsmout: */ APPLESTATIC int nfsrpc_setclient(struct nfsmount *nmp, struct nfsclclient *clp, int reclaim, - struct ucred *cred, NFSPROC_T *p) + bool *retokp, struct ucred *cred, NFSPROC_T *p) { u_int32_t *tl; struct nfsrv_descript nfsd; @@ -944,26 +944,81 @@ nfsrpc_setclient(struct nfsmount *nmp, struct nfsclcli nfsquad_t confirm; u_int32_t lease; static u_int32_t rev = 0; - struct nfsclds *dsp; + struct nfsclds *dsp, *odsp; struct in6_addr a6; struct nfsclsession *tsep; if (nfsboottime.tv_sec == 0) NFSSETBOOTTIME(nfsboottime); - clp->nfsc_rev = rev++; if (NFSHASNFSV4N(nmp)) { - /* - * Either there was no previous session or the - * previous session has failed, so... - * do an ExchangeID followed by the CreateSession. - */ - error = nfsrpc_exchangeid(nmp, clp, &nmp->nm_sockreq, 0, - NFSV4EXCH_USEPNFSMDS | NFSV4EXCH_USENONPNFS, &dsp, cred, p); - NFSCL_DEBUG(1, "aft exch=%d\n", error); - if (error == 0) + error = NFSERR_BADSESSION; + odsp = dsp = NULL; + if (retokp != NULL) { + NFSLOCKMNT(nmp); + odsp = TAILQ_FIRST(&nmp->nm_sess); + NFSUNLOCKMNT(nmp); + } + if (odsp != NULL) { + /* + * When a session already exists, first try a + * CreateSession with the extant ClientID. + */ + dsp = malloc(sizeof(struct nfsclds) + + odsp->nfsclds_servownlen + 1, M_NFSCLDS, + M_WAITOK | M_ZERO); + dsp->nfsclds_expire = NFSD_MONOSEC + clp->nfsc_renew; + dsp->nfsclds_servownlen = odsp->nfsclds_servownlen; + dsp->nfsclds_sess.nfsess_clientid = + odsp->nfsclds_sess.nfsess_clientid; + dsp->nfsclds_sess.nfsess_sequenceid = + odsp->nfsclds_sess.nfsess_sequenceid; + dsp->nfsclds_flags = odsp->nfsclds_flags; + if (dsp->nfsclds_servownlen > 0) + memcpy(dsp->nfsclds_serverown, + odsp->nfsclds_serverown, + dsp->nfsclds_servownlen + 1); + mtx_init(&dsp->nfsclds_mtx, "nfsds", NULL, MTX_DEF); + mtx_init(&dsp->nfsclds_sess.nfsess_mtx, "nfssession", + NULL, MTX_DEF); + nfscl_initsessionslots(&dsp->nfsclds_sess); error = nfsrpc_createsession(nmp, &dsp->nfsclds_sess, &nmp->nm_sockreq, NULL, dsp->nfsclds_sess.nfsess_sequenceid, 1, cred, p); + NFSCL_DEBUG(1, "create session for extant " + "ClientID=%d\n", error); + if (error != 0) { + nfscl_freenfsclds(dsp); + dsp = NULL; + /* + * If *retokp is true, return any error other + * than NFSERR_STALECLIENTID, + * NFSERR_BADSESSION or NFSERR_STALEDONTRECOVER + * so that nfscl_recover() will not loop. + */ + if (*retokp) + return (NFSERR_IO); + } else + *retokp = true; + } else if (retokp != NULL && *retokp) + return (NFSERR_IO); + if (error != 0) { + /* + * Either there was no previous session or the + * CreateSession attempt failed, so... + * do an ExchangeID followed by the CreateSession. + */ + clp->nfsc_rev = rev++; + error = nfsrpc_exchangeid(nmp, clp, &nmp->nm_sockreq, 0, + NFSV4EXCH_USEPNFSMDS | NFSV4EXCH_USENONPNFS, &dsp, + cred, p); + NFSCL_DEBUG(1, "aft exch=%d\n", error); + if (error == 0) + error = nfsrpc_createsession(nmp, + &dsp->nfsclds_sess, &nmp->nm_sockreq, NULL, + dsp->nfsclds_sess.nfsess_sequenceid, 1, + cred, p); + NFSCL_DEBUG(1, "aft createsess=%d\n", error); + } if (error == 0) { NFSLOCKMNT(nmp); /* @@ -988,9 +1043,8 @@ nfsrpc_setclient(struct nfsmount *nmp, struct nfsclcli wakeup(&tsep->nfsess_slots); wakeup(&nmp->nm_sess); NFSUNLOCKMNT(nmp); - } else + } else if (dsp != NULL) nfscl_freenfsclds(dsp); - NFSCL_DEBUG(1, "aft createsess=%d\n", error); if (error == 0 && reclaim == 0) { error = nfsrpc_reclaimcomplete(nmp, cred, p); NFSCL_DEBUG(1, "aft reclaimcomp=%d\n", error); @@ -1000,7 +1054,9 @@ nfsrpc_setclient(struct nfsmount *nmp, struct nfsclcli error = 0; } return (error); - } + } else if (retokp != NULL && *retokp) + return (NFSERR_IO); + clp->nfsc_rev = rev++; /* * Allocate a single session structure for NFSv4.0, because some of Modified: head/sys/fs/nfsclient/nfs_clstate.c ============================================================================== --- head/sys/fs/nfsclient/nfs_clstate.c Wed Apr 22 20:50:24 2020 (r360204) +++ head/sys/fs/nfsclient/nfs_clstate.c Wed Apr 22 21:00:14 2020 (r360205) @@ -110,7 +110,8 @@ static void nfscl_expireclient(struct nfsclclient *, s struct ucred *, NFSPROC_T *); static int nfscl_expireopen(struct nfsclclient *, struct nfsclopen *, struct nfsmount *, struct ucred *, NFSPROC_T *); -static void nfscl_recover(struct nfsclclient *, struct ucred *, NFSPROC_T *); +static void nfscl_recover(struct nfsclclient *, bool *, struct ucred *, + NFSPROC_T *); static void nfscl_insertlock(struct nfscllockowner *, struct nfscllock *, struct nfscllock *, int); static int nfscl_updatelock(struct nfscllockowner *, struct nfscllock **, @@ -907,7 +908,7 @@ nfscl_getcl(struct mount *mp, struct ucred *cred, NFSP clidinusedelay = 120; trystalecnt = 3; do { - error = nfsrpc_setclient(nmp, clp, 0, cred, p); + error = nfsrpc_setclient(nmp, clp, 0, NULL, cred, p); if (error == NFSERR_STALECLIENTID || error == NFSERR_STALEDONTRECOVER || error == NFSERR_BADSESSION || @@ -1950,7 +1951,7 @@ nfscl_umount(struct nfsmount *nmp, NFSPROC_T *p) (void)nfsrpc_destroysession(nmp, clp, cred, p); (void)nfsrpc_destroyclient(nmp, clp, cred, p); } else - (void)nfsrpc_setclient(nmp, clp, 0, cred, p); + (void)nfsrpc_setclient(nmp, clp, 0, NULL, cred, p); nfscl_cleanclient(clp); nmp->nm_clp = NULL; NFSFREECRED(cred); @@ -1966,7 +1967,8 @@ nfscl_umount(struct nfsmount *nmp, NFSPROC_T *p) * corresponding state. */ static void -nfscl_recover(struct nfsclclient *clp, struct ucred *cred, NFSPROC_T *p) +nfscl_recover(struct nfsclclient *clp, bool *retokp, struct ucred *cred, + NFSPROC_T *p) { struct nfsclowner *owp, *nowp; struct nfsclopen *op, *nop; @@ -2010,8 +2012,9 @@ nfscl_recover(struct nfsclclient *clp, struct ucred *c LIST_INIT(&clp->nfsc_layouthash[i]); trycnt = 5; + tcred = NULL; do { - error = nfsrpc_setclient(nmp, clp, 1, cred, p); + error = nfsrpc_setclient(nmp, clp, 1, retokp, cred, p); } while ((error == NFSERR_STALECLIENTID || error == NFSERR_BADSESSION || error == NFSERR_STALEDONTRECOVER) && --trycnt > 0); @@ -2044,6 +2047,13 @@ nfscl_recover(struct nfsclclient *clp, struct ucred *c NFSUNLOCKREQ(); /* + * If nfsrpc_setclient() returns *retokp == true, + * no more recovery is needed. + */ + if (*retokp) + goto out; + + /* * Now, mark all delegations "need reclaim". */ TAILQ_FOREACH(dp, &clp->nfsc_deleg, nfsdl_list) @@ -2276,12 +2286,14 @@ nfscl_recover(struct nfsclclient *clp, struct ucred *c if (NFSHASNFSV4N(nmp)) (void)nfsrpc_reclaimcomplete(nmp, cred, p); +out: NFSLOCKCLSTATE(); clp->nfsc_flags &= ~NFSCLFLAGS_RECVRINPROG; wakeup(&clp->nfsc_flags); nfsv4_unlock(&clp->nfsc_lock, 0); NFSUNLOCKCLSTATE(); - NFSFREECRED(tcred); + if (tcred != NULL) + NFSFREECRED(tcred); } /* @@ -2330,7 +2342,7 @@ nfscl_hasexpired(struct nfsclclient *clp, u_int32_t cl cred = newnfs_getcred(); trycnt = 5; do { - error = nfsrpc_setclient(nmp, clp, 0, cred, p); + error = nfsrpc_setclient(nmp, clp, 0, NULL, cred, p); } while ((error == NFSERR_STALECLIENTID || error == NFSERR_BADSESSION || error == NFSERR_STALEDONTRECOVER) && --trycnt > 0); @@ -2539,6 +2551,7 @@ nfscl_renewthread(struct nfsclclient *clp, NFSPROC_T * struct nfscllayouthead rlh; struct nfsclrecalllayout *recallp; struct nfsclds *dsp; + bool retok; cred = newnfs_getcred(); NFSLOCKCLSTATE(); @@ -2549,21 +2562,23 @@ nfscl_renewthread(struct nfsclclient *clp, NFSPROC_T * cbpathdown = 0; if (clp->nfsc_flags & NFSCLFLAGS_RECOVER) { /* - * Only allow one recover within 1/2 of the lease + * Only allow one full recover within 1/2 of the lease * duration (nfsc_renew). + * retok is value/result. If passed in set to true, + * it indicates only a CreateSession operation should + * be attempted. + * If it is returned true, it indicates that the + * recovery only required a CreateSession. */ + retok = true; if (recover_done_time < NFSD_MONOSEC) { recover_done_time = NFSD_MONOSEC + clp->nfsc_renew; - NFSCL_DEBUG(1, "Doing recovery..\n"); - nfscl_recover(clp, cred, p); - } else { - NFSCL_DEBUG(1, "Clear Recovery dt=%u ms=%jd\n", - recover_done_time, (intmax_t)NFSD_MONOSEC); - NFSLOCKCLSTATE(); - clp->nfsc_flags &= ~NFSCLFLAGS_RECOVER; - NFSUNLOCKCLSTATE(); + retok = false; } + NFSCL_DEBUG(1, "Doing recovery, only " + "createsession=%d\n", retok); + nfscl_recover(clp, &retok, cred, p); } if (clp->nfsc_expire <= NFSD_MONOSEC && (clp->nfsc_flags & NFSCLFLAGS_HASCLIENTID)) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202004222100.03ML0Fwd050816>