Date: Sat, 26 Jun 2021 23:37:57 GMT From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 88878ea4e410 - stable/12 - nfscl: Fix generation of va_fsid for a tree of NFSv4 server file systems Message-ID: <202106262337.15QNbvBw009358@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=88878ea4e410d8ad8e5394cc83a019da29db8d79 commit 88878ea4e410d8ad8e5394cc83a019da29db8d79 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2021-06-07 20:48:25 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2021-06-26 23:34:30 +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 20500a290ff0..97cde6421da2 100644 --- a/sys/fs/nfsclient/nfs_clport.c +++ b/sys/fs/nfsclient/nfs_clport.c @@ -439,6 +439,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; @@ -501,28 +502,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) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202106262337.15QNbvBw009358>