svn commit: r360205 - in head/sys/fs: nfs nfsclient

Rick Macklem rmacklem at FreeBSD.org
Wed Apr 22 21:00:16 UTC 2020


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)) {


More information about the svn-src-all mailing list