Date: Fri, 20 Oct 2023 23:12:33 GMT From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 196787f79e67 - main - nfscl: Use Claim_Null_FH and Claim_Deleg_Cur_FH Message-ID: <202310202312.39KNCX0G056113@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=196787f79e67374527a1d528a42efa8b31acd9af commit 196787f79e67374527a1d528a42efa8b31acd9af Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2023-10-20 23:10:25 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2023-10-20 23:10:25 +0000 nfscl: Use Claim_Null_FH and Claim_Deleg_Cur_FH For NFSv4.1/4.2, there are two new options for the Open operation. These two options use the file handle for the file instead of the file handle for the directory plus a file name. By doing so, the client code is simplified (it no longer needs the "nfsv4node" structure attached to the NFS vnode). It also avoids problems caused by another NFS client (or process running locally in the NFS server) doing a rename or remove of the file name between the Lookup and Open. Unfortunately, there was a bug (fixed recently by commit X) in the NFS server which mis-parsed the Claim_Deleg_Cur_FH arguments. To allow this patch to work with the broken FreeBSD NFSv4.1/4.2 server, NFSMNTP_BUGGYFBSDSRV is defined and is set when a correctly formatted Claim_Deleg_Cur_FH fails with NFSERR_EXPIRED. (This is what the old, broken NFS server does, since it erroneously uses the Getattr arguments as a stateID.) Once this flag is set, the client fills in a stateID, to make the broken NFS server happy. Tested at a recent IETF NFSv4 Bakeathon. MFC after: 1 month --- sys/fs/nfsclient/nfs_clport.c | 4 +- sys/fs/nfsclient/nfs_clrpcops.c | 92 +++++++++++++++++++++++++++++++---------- sys/fs/nfsclient/nfs_clstate.c | 47 +++++++++++++++++---- sys/fs/nfsclient/nfs_clvnops.c | 16 ------- sys/fs/nfsclient/nfsmount.h | 1 + 5 files changed, 114 insertions(+), 46 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clport.c b/sys/fs/nfsclient/nfs_clport.c index 8ea50d80ae19..c0318b692d86 100644 --- a/sys/fs/nfsclient/nfs_clport.c +++ b/sys/fs/nfsclient/nfs_clport.c @@ -264,10 +264,10 @@ nfscl_nget(struct mount *mntp, struct vnode *dvp, struct nfsfh *nfhp, np->n_fhp = nfhp; /* - * For NFSv4, we have to attach the directory file handle and + * For NFSv4.0, we have to attach the directory file handle and * file name, so that Open Ops can be done later. */ - if (nmp->nm_flag & NFSMNT_NFSV4) { + if (NFSHASNFSV4(nmp) && !NFSHASNFSV4N(nmp)) { np->n_v4 = malloc(sizeof (struct nfsv4node) + dnp->n_fhp->nfh_len + cnp->cn_namelen - 1, M_NFSV4NODE, M_WAITOK); diff --git a/sys/fs/nfsclient/nfs_clrpcops.c b/sys/fs/nfsclient/nfs_clrpcops.c index 14351d915ba2..2276e09f6e7e 100644 --- a/sys/fs/nfsclient/nfs_clrpcops.c +++ b/sys/fs/nfsclient/nfs_clrpcops.c @@ -392,16 +392,6 @@ nfsrpc_open(vnode_t vp, int amode, struct ucred *cred, NFSPROC_T *p) nfhp = np->n_fhp; retrycnt = 0; -#ifdef notdef -{ char name[100]; int namel; -namel = (np->n_v4->n4_namelen < 100) ? np->n_v4->n4_namelen : 99; -bcopy(NFS4NODENAME(np->n_v4), name, namel); -name[namel] = '\0'; -printf("rpcopen p=0x%x name=%s",p->p_pid,name); -if (nfhp->nfh_len > 0) printf(" fh=0x%x\n",nfhp->nfh_fh[12]); -else printf(" fhl=0\n"); -} -#endif do { dp = NULL; error = nfscl_open(vp, nfhp->nfh_fh, nfhp->nfh_len, mode, 1, @@ -452,6 +442,39 @@ else printf(" fhl=0\n"); op->nfso_own->nfsow_clp, nfhp->nfh_fh, nfhp->nfh_len, cred, p, &dp); } + } else if (NFSHASNFSV4N(nmp)) { + /* + * For the first attempt, try and get a layout, if + * pNFS is enabled for the mount. + */ + if (!NFSHASPNFS(nmp) || nfscl_enablecallb == 0 || + nfs_numnfscbd == 0 || + (np->n_flag & NNOLAYOUT) != 0 || retrycnt > 0) + error = nfsrpc_openrpc(nmp, vp, nfhp->nfh_fh, + nfhp->nfh_len, nfhp->nfh_fh, nfhp->nfh_len, + mode, op, NULL, 0, &dp, 0, 0x0, cred, p, 0, + 0); + else + error = nfsrpc_getopenlayout(nmp, vp, + nfhp->nfh_fh, nfhp->nfh_len, nfhp->nfh_fh, + nfhp->nfh_len, mode, op, NULL, 0, &dp, + cred, p); + if (dp != NULL) { + NFSLOCKNODE(np); + np->n_flag &= ~NDELEGMOD; + /* + * Invalidate the attribute cache, so that + * attributes that pre-date the issue of a + * delegation are not cached, since the + * cached attributes will remain valid while + * the delegation is held. + */ + NFSINVALATTRCACHE(np); + NFSUNLOCKNODE(np); + (void) nfscl_deleg(nmp->nm_mountp, + op->nfso_own->nfsow_clp, + nfhp->nfh_fh, nfhp->nfh_len, cred, p, &dp); + } } else { error = EIO; } @@ -538,19 +561,40 @@ nfsrpc_openrpc(struct nfsmount *nmp, vnode_t vp, u_int8_t *nfhp, int fhlen, *tl = txdr_unsigned(delegtype); } else { if (dp != NULL) { - *tl = txdr_unsigned(NFSV4OPEN_CLAIMDELEGATECUR); - NFSM_BUILD(tl, u_int32_t *, NFSX_STATEID); - if (NFSHASNFSV4N(nmp)) - *tl++ = 0; - else + if (NFSHASNFSV4N(nmp)) { + *tl = txdr_unsigned( + NFSV4OPEN_CLAIMDELEGATECURFH); + NFSLOCKMNT(nmp); + if ((nmp->nm_privflag & NFSMNTP_BUGGYFBSDSRV) != + 0) { + NFSUNLOCKMNT(nmp); + /* + * Add a stateID argument to make old + * broken FreeBSD NFSv4.1/4.2 servers + * happy. + */ + NFSM_BUILD(tl, uint32_t *,NFSX_STATEID); + *tl++ = 0; + *tl++ = dp->nfsdl_stateid.other[0]; + *tl++ = dp->nfsdl_stateid.other[1]; + *tl = dp->nfsdl_stateid.other[2]; + } else + NFSUNLOCKMNT(nmp); + } else { + *tl = txdr_unsigned(NFSV4OPEN_CLAIMDELEGATECUR); + NFSM_BUILD(tl, u_int32_t *, NFSX_STATEID); *tl++ = dp->nfsdl_stateid.seqid; - *tl++ = dp->nfsdl_stateid.other[0]; - *tl++ = dp->nfsdl_stateid.other[1]; - *tl = dp->nfsdl_stateid.other[2]; + *tl++ = dp->nfsdl_stateid.other[0]; + *tl++ = dp->nfsdl_stateid.other[1]; + *tl = dp->nfsdl_stateid.other[2]; + (void)nfsm_strtom(nd, name, namelen); + } + } else if (NFSHASNFSV4N(nmp)) { + *tl = txdr_unsigned(NFSV4OPEN_CLAIMFH); } else { *tl = txdr_unsigned(NFSV4OPEN_CLAIMNULL); + (void)nfsm_strtom(nd, name, namelen); } - (void) nfsm_strtom(nd, name, namelen); } NFSM_BUILD(tl, u_int32_t *, NFSX_UNSIGNED); *tl = txdr_unsigned(NFSV4OP_GETATTR); @@ -2713,6 +2757,8 @@ nfsrpc_createv4(vnode_t dvp, char *name, int namelen, struct vattr *vap, if ((rflags & NFSV4OPEN_RESULTCONFIRM) && (owp->nfsow_clp->nfsc_flags & NFSCLFLAGS_GOTDELEG) && !error && dp == NULL) { + KASSERT(!NFSHASNFSV4N(nmp), + ("nfsrpc_createv4: result confirm")); do { ret = nfsrpc_openrpc(VFSTONFS(dvp->v_mount), dvp, np->n_fhp->nfh_fh, np->n_fhp->nfh_len, @@ -8009,8 +8055,12 @@ nfsrpc_openlayoutrpc(struct nfsmount *nmp, vnode_t vp, u_int8_t *nfhp, nfsm_strtom(nd, op->nfso_own->nfsow_owner, NFSV4CL_LOCKNAMELEN); NFSM_BUILD(tl, uint32_t *, 2 * NFSX_UNSIGNED); *tl++ = txdr_unsigned(NFSV4OPEN_NOCREATE); - *tl = txdr_unsigned(NFSV4OPEN_CLAIMNULL); - nfsm_strtom(nd, name, namelen); + if (NFSHASNFSV4N(nmp)) { + *tl = txdr_unsigned(NFSV4OPEN_CLAIMFH); + } else { + *tl = txdr_unsigned(NFSV4OPEN_CLAIMNULL); + nfsm_strtom(nd, name, namelen); + } NFSM_BUILD(tl, uint32_t *, NFSX_UNSIGNED); *tl = txdr_unsigned(NFSV4OP_GETATTR); NFSZERO_ATTRBIT(&attrbits); diff --git a/sys/fs/nfsclient/nfs_clstate.c b/sys/fs/nfsclient/nfs_clstate.c index 74d6a14fc4c4..579210941802 100644 --- a/sys/fs/nfsclient/nfs_clstate.c +++ b/sys/fs/nfsclient/nfs_clstate.c @@ -4383,9 +4383,15 @@ nfscl_moveopen(vnode_t vp, struct nfsclclient *clp, struct nfsmount *nmp, nfscl_newopen(clp, NULL, &owp, NULL, &op, &nop, owp->nfsow_owner, lop->nfso_fh, lop->nfso_fhlen, cred, &newone); ndp = dp; - error = nfscl_tryopen(nmp, vp, np->n_v4->n4_data, np->n_v4->n4_fhlen, - lop->nfso_fh, lop->nfso_fhlen, lop->nfso_mode, op, - NFS4NODENAME(np->n_v4), np->n_v4->n4_namelen, &ndp, 0, 0, cred, p); + if (NFSHASNFSV4N(nmp)) + error = nfscl_tryopen(nmp, vp, lop->nfso_fh, lop->nfso_fhlen, + lop->nfso_fh, lop->nfso_fhlen, lop->nfso_mode, op, + NULL, 0, &ndp, 0, 0, cred, p); + else + error = nfscl_tryopen(nmp, vp, np->n_v4->n4_data, + np->n_v4->n4_fhlen, lop->nfso_fh, lop->nfso_fhlen, + lop->nfso_mode, op, NFS4NODENAME(np->n_v4), + np->n_v4->n4_namelen, &ndp, 0, 0, cred, p); if (error) { if (newone) nfscl_freeopen(op, 0, true); @@ -4476,14 +4482,16 @@ nfsrpc_reopen(struct nfsmount *nmp, u_int8_t *fhp, int fhlen, if (error) return (error); vp = NFSTOV(np); - if (np->n_v4 != NULL) { + if (NFSHASNFSV4N(nmp)) + error = nfscl_tryopen(nmp, vp, fhp, fhlen, fhp, fhlen, mode, op, + NULL, 0, dpp, 0, 0, cred, p); + else if (np->n_v4 != NULL) error = nfscl_tryopen(nmp, vp, np->n_v4->n4_data, np->n_v4->n4_fhlen, fhp, fhlen, mode, op, NFS4NODENAME(np->n_v4), np->n_v4->n4_namelen, dpp, 0, 0, cred, p); - } else { + else error = EINVAL; - } vrele(vp); return (error); } @@ -4500,18 +4508,43 @@ nfscl_tryopen(struct nfsmount *nmp, vnode_t vp, u_int8_t *fhp, int fhlen, int reclaim, u_int32_t delegtype, struct ucred *cred, NFSPROC_T *p) { int error; + struct nfscldeleg *dp; + bool try_busted_xdr; + dp = *ndpp; do { + *ndpp = dp; /* *ndpp needs to be set for retries. */ error = nfsrpc_openrpc(nmp, vp, fhp, fhlen, newfhp, newfhlen, mode, op, name, namelen, ndpp, reclaim, delegtype, cred, p, 0, 0); + try_busted_xdr = false; if (error == NFSERR_DELAY) (void) nfs_catnap(PZERO, error, "nfstryop"); - } while (error == NFSERR_DELAY); + else if (error == NFSERR_EXPIRED && NFSHASNFSV4N(nmp) && + reclaim == 0 && dp != NULL) { + /* This case is a Claim_Deleg_Cur_FH Open. */ + NFSLOCKMNT(nmp); + if ((nmp->nm_privflag & NFSMNTP_BUGGYFBSDSRV) == 0) { + /* + * Old FreeBSD NFSv4.1/4.2 servers erroneously + * expect a stateID argument for Open + * Claim_Deleg_Cur_FH and interpret the + * Getattr reply as a stateID. This results + * in an NFSERR_EXPIRED failure. + * Setting NFSMNTP_BUGGYFBSDSRV makes the Open + * send a stateID, in violation of RFC8881. + */ + try_busted_xdr = true; + nmp->nm_privflag |= NFSMNTP_BUGGYFBSDSRV; + } + NFSUNLOCKMNT(nmp); + } + } while (error == NFSERR_DELAY || try_busted_xdr); if (error == EAUTH || error == EACCES) { /* Try again using system credentials */ newnfs_setroot(cred); do { + *ndpp = dp; /* *ndpp needs to be set for retries. */ error = nfsrpc_openrpc(nmp, vp, fhp, fhlen, newfhp, newfhlen, mode, op, name, namelen, ndpp, reclaim, delegtype, cred, p, 1, 0); diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c index fd88531789d9..00b6c7617101 100644 --- a/sys/fs/nfsclient/nfs_clvnops.c +++ b/sys/fs/nfsclient/nfs_clvnops.c @@ -2052,14 +2052,6 @@ nfs_rename(struct vop_rename_args *ap) tdnp->n_fhp->nfh_len != fnp->n_v4->n4_fhlen || NFSBCMP(tdnp->n_fhp->nfh_fh, fnp->n_v4->n4_data, tdnp->n_fhp->nfh_len))) { -#ifdef notdef -{ char nnn[100]; int nnnl; -nnnl = (tcnp->cn_namelen < 100) ? tcnp->cn_namelen : 99; -bcopy(tcnp->cn_nameptr, nnn, nnnl); -nnn[nnnl] = '\0'; -printf("ren replace=%s\n",nnn); -} -#endif free(fnp->n_v4, M_NFSV4NODE); fnp->n_v4 = newv4; newv4 = NULL; @@ -2713,14 +2705,6 @@ nfs_lookitup(struct vnode *dvp, char *name, int len, struct ucred *cred, dnp->n_fhp->nfh_len != np->n_v4->n4_fhlen || NFSBCMP(dnp->n_fhp->nfh_fh, np->n_v4->n4_data, dnp->n_fhp->nfh_len))) { -#ifdef notdef -{ char nnn[100]; int nnnl; -nnnl = (len < 100) ? len : 99; -bcopy(name, nnn, nnnl); -nnn[nnnl] = '\0'; -printf("replace=%s\n",nnn); -} -#endif free(np->n_v4, M_NFSV4NODE); np->n_v4 = malloc( sizeof (struct nfsv4node) + diff --git a/sys/fs/nfsclient/nfsmount.h b/sys/fs/nfsclient/nfsmount.h index 37b84a015dab..7571add27b9c 100644 --- a/sys/fs/nfsclient/nfsmount.h +++ b/sys/fs/nfsclient/nfsmount.h @@ -124,6 +124,7 @@ struct nfsmount { #define NFSMNTP_DELEGISSUED 0x00000400 #define NFSMNTP_NODEALLOCATE 0x00000800 #define NFSMNTP_FAKEROOTFH 0x00001000 +#define NFSMNTP_BUGGYFBSDSRV 0x00002000 /* New mount flags only used by the kernel via nmount(2). */ #define NFSMNT_TLS 0x00000001
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202310202312.39KNCX0G056113>