From nobody Fri Oct 20 23:12:33 2023 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SC0jY44Fqz4yLZc; Fri, 20 Oct 2023 23:12:33 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SC0jY3YlTz3LGY; Fri, 20 Oct 2023 23:12:33 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1697843553; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kPxsOrG1WHSQm0TdLZlRwQtHrKe9ux91cFdGE8Hnlhc=; b=Jvkm9xiKbkM4ySIAG2X1D1NUU9nGMRwOf+7bHpAKNNG/IDcyL54e4CZ4WUYfsk2+wRKPCZ VBAgTRKPCArGM7If6q1Upnhf7lbtM8XYWa4rXhSTP5egiM+r2gYd75sc0G1dduVVGTiqO/ 5C+4GpYVPT/sO3ZvWVd1IvKmDowCUhT2ZbeMngB1PWGjWlqosuL99Yb1QH/xKcfcz0fPv+ dU3pDz4kfAiICf5DJC063IBUQ61vEqBxrholgBIrWFKP8ZUhDaWvaNPZrs79hOjk7w024b YEI/+Ug4gfGPqqH6WCM5pkUWIdyeDVP3tY0h1oohuMqFdssRRdjww84J3Cyelg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1697843553; a=rsa-sha256; cv=none; b=NCxpcLCVUDRQK8VeTy6yNGNWag92gAuwxVrr/VuIjQ6oP5zKtM0+U1roODUbr/GzHIQeDJ 9DUqPzx6s8Fu59/W7OVL2GM9wCgSw7jtwH/e8qwZDsNsoDeGaehH5ajRuCcyv14HYq5XGV gwlbFnsPTfsxU7YXmuROv1lR+D/WkuZOfJrnEWMRRdI3WUJmoxbdKgOL3ATC7oV/YwdoWV ye/WU2+3jh6E2IvJsQ1GIEBClbvxIjBkbps7GzLjH5lABhS4Unw6g8bt2IJSdcshptE1tB 8rgJt627MeZtAyjDwEAqK8O73nMPFmjxrqEpWnUZxhGQutYLc3KGNiLtnCxW1g== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1697843553; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kPxsOrG1WHSQm0TdLZlRwQtHrKe9ux91cFdGE8Hnlhc=; b=lSA2GWv9jWDQ53TivPEmjMpCse1B1yoySvJN8gvxSLIfi7mqJEJeLoy6/UQXaiHQ0zrU1L xyQBLue5iLMgb23EepzD0lQp/eGPnlBlhW2Z4V1xeMtMXhJOjyU6syfBbMtF/3USz0J+21 ETv7k4vIuHsqv23RGgsvGuTiIG7UYCVgneezDAy+FUt2ficA1hdGOTkRbqMZ9cQhDnXhNO MKWCeiczQjiMpeqGGXFVXysM/U+6zjSMP5TO9SB4hadC+0CEMh2Ji21PVTZibQYmIdcoex yRRHX0dIO0+Jdw3x8jKxiyI2Ttk3bGdRVQQIYl0ZWSnqOfICkWK1MR5GVpGZiQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4SC0jY2d1Szs7l; Fri, 20 Oct 2023 23:12:33 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 39KNCXsC056116; Fri, 20 Oct 2023 23:12:33 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 39KNCX0G056113; Fri, 20 Oct 2023 23:12:33 GMT (envelope-from git) Date: Fri, 20 Oct 2023 23:12:33 GMT Message-Id: <202310202312.39KNCX0G056113@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: 196787f79e67 - main - nfscl: Use Claim_Null_FH and Claim_Deleg_Cur_FH List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 196787f79e67374527a1d528a42efa8b31acd9af Auto-Submitted: auto-generated The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=196787f79e67374527a1d528a42efa8b31acd9af commit 196787f79e67374527a1d528a42efa8b31acd9af Author: Rick Macklem AuthorDate: 2023-10-20 23:10:25 +0000 Commit: Rick Macklem 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