git: b1caf1920f83 - stable/13 - nfscl: Fix generation of va_fsid for a tree of NFSv4 server file systems

Rick Macklem rmacklem at FreeBSD.org
Sat Jun 26 23:02:08 UTC 2021


The branch stable/13 has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=b1caf1920f83fd6fe00602ec37a354e3ca4eba23

commit b1caf1920f83fd6fe00602ec37a354e3ca4eba23
Author:     Rick Macklem <rmacklem at FreeBSD.org>
AuthorDate: 2021-06-07 20:48:25 +0000
Commit:     Rick Macklem <rmacklem at FreeBSD.org>
CommitDate: 2021-06-26 22:58:21 +0000

    nfscl: Fix generation of va_fsid for a tree of NFSv4 server file systems
    
    Pre-r318997 the code looked like:
    if (vp->v_mount->mnt_stat.f_fsid.val[0] != (uint32_t)np->n_vattr.na_filesid[0])
             vap->va_fsid = (uint32_t)np->n_vattr.na_filesid[0];
    Doing this assignment got lost by r318997 and, as such, NFSv4 mounts
    of servers with trees of file systems on the server is broken, due to duplicate
    fileno values for the same st_dev/va_fsid.
    
    Although I could have re-introduced the assignment, since the value of
    na_filesid[0] is not guaranteed to be unique across the server file systems,
    I felt it was better to always do the hash for na_filesid[0,1].
    Since dev_t (st_dev/va_fsid) is now 64bits, I switched to a 64bit hash.
    
    There is a slight chance of a hash conflict where 2 different na_filesid
    values map to same va_fsid, which will be documented in the BUGS
    section of the man page for mount_nfs(8).  Using a table to keep track
    of mappings to catch conflicts would not easily scale to 10,000+ server file
    systems and, when the conflict occurs, it only results in fts(3) reporting
    a "directory cycle" under certain circumstances.
    
    (cherry picked from commit 03c81af24920e14bd977f34edcd3eb7fb122db19)
---
 sys/fs/nfsclient/nfs_clport.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c
index 492b93340e4e..264d99626394 100644
--- a/sys/fs/nfsclient/nfs_clport.c
+++ b/sys/fs/nfsclient/nfs_clport.c
@@ -441,6 +441,7 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
 	struct nfsmount *nmp;
 	struct timespec mtime_save;
 	int error, force_fid_err;
+	dev_t topfsid;
 
 	error = 0;
 
@@ -504,28 +505,30 @@ nfscl_loadattrcache(struct vnode **vpp, struct nfsvattr *nap, void *nvaper,
 	}
 
 	/*
-	 * For NFSv4, if the node's fsid is not equal to the mount point's
-	 * fsid, return the low order 32bits of the node's fsid. This
-	 * allows getcwd(3) to work. There is a chance that the fsid might
-	 * be the same as a local fs, but since this is in an NFS mount
-	 * point, I don't think that will cause any problems?
+	 * For NFSv4, the server's export may be a tree of file systems
+	 * where a fileno is a unique value within each file system.
+	 * na_filesid[0,1] uniquely identify the server file system
+	 * and nm_fsid[0,1] is the value for the root file system mounted.
+	 * As such, the value of va_fsid generated by vn_fsid() represents
+	 * the root file system on the server and a different value for
+	 * va_fsid is needed for the other server file systems.  This
+	 * va_fsid is ideally unique for all of the server file systems,
+	 * so a 64bit hash on na_filesid[0,1] is calculated.
+	 * Although highly unlikely that the fnv_64_hash() will be
+	 * the same as the root, test for this case and recalculate the hash.
 	 */
+	vn_fsid(vp, vap);
 	if (NFSHASNFSV4(nmp) && NFSHASHASSETFSID(nmp) &&
 	    (nmp->nm_fsid[0] != np->n_vattr.na_filesid[0] ||
 	     nmp->nm_fsid[1] != np->n_vattr.na_filesid[1])) {
-		/*
-		 * va_fsid needs to be set to some value derived from
-		 * np->n_vattr.na_filesid that is not equal
-		 * vp->v_mount->mnt_stat.f_fsid[0], so that it changes
-		 * from the value used for the top level server volume
-		 * in the mounted subtree.
-		 */
-		vn_fsid(vp, vap);
-		if ((uint32_t)vap->va_fsid == np->n_vattr.na_filesid[0])
-			vap->va_fsid = hash32_buf(
-			    np->n_vattr.na_filesid, 2 * sizeof(uint64_t), 0);
-	} else
-		vn_fsid(vp, vap);
+		topfsid = vap->va_fsid;
+		vap->va_fsid = FNV1_64_INIT;
+		do {
+			vap->va_fsid = fnv_64_buf(np->n_vattr.na_filesid,
+			    sizeof(np->n_vattr.na_filesid), vap->va_fsid);
+		} while (vap->va_fsid == topfsid);
+	}
+
 	np->n_attrstamp = time_second;
 	if (vap->va_size != np->n_size) {
 		if (vap->va_type == VREG) {


More information about the dev-commits-src-all mailing list