Subject: Re: file id alignment
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Martin Husemann <martin@duskware.de>
List: tech-kern
Date: 07/12/2006 21:17:14
--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Jul 12, 2006 at 11:47:11PM +0900, YAMAMOTO Takashi wrote:
> - i think you need to check nfsrv_descript.nd_fh.

As I said initialy: I'm pretty much lost in the NFS part of the change.
I've attached a modified version (only the nfs part this time)

> - why you leave VFS_MAXFIDSZ?

Oversight, I've removed it (it wasn't used nor usable anymore anyway)

> - why LC_MAXFIDSIZ is 64?  what's "LC"?

I suppose "lease cache", it's original use was lc_fiddata.

> - i'm not sure ENOMEM is an appropriate error code for too big filehandles.

Any better suggestions?

> - is it ok to use c99 feature for an userland-exported header?
>   (a flexible array member in struct fid)

IMHO it is (didn't we get rid of gcc 2.95.x now?). Are there alternatives?
I don't like "char fid_data[2]", when "2" is just a lie.


Martin

--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=patch

Index: nfs.h
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs.h,v
retrieving revision 1.57
diff -u -p -r1.57 nfs.h
--- nfs.h	7 Jun 2006 22:34:17 -0000	1.57
+++ nfs.h	12 Jul 2006 19:10:45 -0000
@@ -512,7 +512,7 @@ struct nfsrv_descript {
 	u_int32_t		nd_retxid;	/* Reply xid */
 	u_int32_t		nd_duration;	/* Lease duration */
 	struct timeval		nd_starttime;	/* Time RPC initiated */
-	fhandle_t		nd_fh;		/* File handle */
+	nfsfh_t			nd_fh;		/* File handle */
 	kauth_cred_t	 	nd_cr;		/* Credentials */
 };
 
@@ -574,7 +574,7 @@ extern int nfs_numasync;
  */
 struct nfs_public {
 	int		np_valid;	/* Do we hold valid information */
-	fhandle_t	np_handle;	/* Filehandle for pub fs (internal) */
+	fhandle_t	*np_handle;	/* Filehandle for pub fs (internal) */
 	struct mount	*np_mount;	/* Mountpoint of exported fs */
 	char		*np_index;	/* Index file */
 };
Index: nfs_export.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_export.c,v
retrieving revision 1.14
diff -u -p -r1.14 nfs_export.c
--- nfs_export.c	17 Jun 2006 07:06:51 -0000	1.14
+++ nfs_export.c	12 Jul 2006 19:10:45 -0000
@@ -223,7 +223,8 @@ mountd_set_exports_list(const struct mou
 	struct netexport *ne;
 	struct nameidata nd;
 	struct vnode *vp;
-	struct fid fid;
+	struct fid *fid;
+	size_t fid_size;
 
 	if (kauth_authorize_generic(l->l_proc->p_cred, KAUTH_GENERIC_ISSUSER,
 			      &l->l_proc->p_acflag) != 0)
@@ -239,12 +240,22 @@ mountd_set_exports_list(const struct mou
 
 	/* The selected file system may not support NFS exports, so ensure
 	 * it does. */
-	if (mp->mnt_op->vfs_vptofh == NULL || mp->mnt_op->vfs_fhtovp == NULL ||
-	    VFS_VPTOFH(vp, &fid) != 0) {
+	if (mp->mnt_op->vfs_vptofh == NULL || mp->mnt_op->vfs_fhtovp == NULL) {
+		error = EOPNOTSUPP;
+		goto out_locked;
+	}
+	fid_size = 0;
+	if ((error = VFS_VPTOFH(vp, NULL, &fid_size)) == ENOMEM) {
+		fid = malloc(fid_size, M_TEMP, M_NOWAIT);
+		if (fid != NULL) {
+			error = VFS_VPTOFH(vp, fid, &fid_size);
+			free(fid, M_TEMP);
+		}
+	}
+	if (error != 0) {
 		error = EOPNOTSUPP;
 		goto out_locked;
 	}
-	KASSERT(fid.fid_len <= _VFS_MAXFIDSZ);
 	KASSERT(mp->mnt_op->vfs_vptofh != NULL &&
 	    mp->mnt_op->vfs_fhtovp != NULL);
 
@@ -717,6 +728,7 @@ setpublicfs(struct mount *mp, struct net
 	char *cp;
 	int error;
 	struct vnode *rvp;
+	size_t fhsize;
 
 	/*
 	 * mp == NULL -> invalidate the current info, the FS is
@@ -726,6 +738,10 @@ setpublicfs(struct mount *mp, struct net
 	if (mp == NULL) {
 		if (nfs_pub.np_valid) {
 			nfs_pub.np_valid = 0;
+			if (nfs_pub.np_handle != NULL) {
+				free(nfs_pub.np_handle, M_TEMP);
+				nfs_pub.np_handle = NULL;
+			}
 			if (nfs_pub.np_index != NULL) {
 				FREE(nfs_pub.np_index, M_TEMP);
 				nfs_pub.np_index = NULL;
@@ -746,7 +762,15 @@ setpublicfs(struct mount *mp, struct net
 	if ((error = VFS_ROOT(mp, &rvp)))
 		return error;
 
-	error = vfs_composefh(rvp, &nfs_pub.np_handle);
+	fhsize = 0;
+	error = vfs_composefh(rvp, NULL, &fhsize);
+	if (error != ENOMEM)
+		return error;
+	nfs_pub.np_handle = malloc(fhsize, M_TEMP, M_NOWAIT);
+	if (nfs_pub.np_handle == NULL)
+		error = ENOMEM;
+	else
+		error = vfs_composefh(rvp, nfs_pub.np_handle, &fhsize);
 	if (error)
 		return error;
 
Index: nfs_nqlease.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_nqlease.c,v
retrieving revision 1.66
diff -u -p -r1.66 nfs_nqlease.c
--- nfs_nqlease.c	17 Jun 2006 07:06:51 -0000	1.66
+++ nfs_nqlease.c	12 Jul 2006 19:10:45 -0000
@@ -186,7 +186,11 @@ nqsrv_getlease(vp, duration, flags, slp,
 	struct nqlease *tlp;
 	struct nqm **lphp;
 	struct vattr vattr;
-	fhandle_t fh;
+	union {
+		fhandle_t fh;
+		char space[LC_MAXFIDSIZ + offsetof(fhandle_t, fh_fid)];
+	} fh;
+	size_t fh_size;
 	int i, ok, error, s;
 
 	if (vp->v_type != VREG && vp->v_type != VDIR && vp->v_type != VLNK)
@@ -208,17 +212,18 @@ nqsrv_getlease(vp, duration, flags, slp,
 		/*
 		 * Find the lease by searching the hash list.
 		 */
-		error = vfs_composefh(vp, &fh);
+		fh_size = sizeof(fh);
+		error = vfs_composefh(vp, &fh.fh, &fh_size);
 		if (error) {
 			splx(s);
 			return (error);
 		}
-		lpp = NQFHHASH(fh.fh_fid.fid_data);
+		lpp = NQFHHASH(fh.fh.fh_fid.fid_data);
 		LIST_FOREACH (lp, lpp, lc_hash) {
-			if (fh.fh_fsid.__fsid_val[0] == lp->lc_fsid.__fsid_val[0] &&
-			    fh.fh_fsid.__fsid_val[1] == lp->lc_fsid.__fsid_val[1] &&
-			    !memcmp(fh.fh_fid.fid_data, lp->lc_fiddata,
-				  fh.fh_fid.fid_len - sizeof (int32_t))) {
+			if (fh.fh.fh_fsid.__fsid_val[0] == lp->lc_fsid.__fsid_val[0] &&
+			    fh.fh.fh_fsid.__fsid_val[1] == lp->lc_fsid.__fsid_val[1] &&
+			    !memcmp(fh.fh.fh_fid.fid_data, lp->lc_fiddata,
+				  fh.fh.fh_fid.fid_len - sizeof (int32_t))) {
 				/* Found it */
 				lp->lc_vp = vp;
 				vp->v_lease = lp;
@@ -294,8 +299,9 @@ doreply:
 		return (0);
 	}
 	splx(s);
-	if (flags & ND_CHECK)
+	if (flags & ND_CHECK) {
 		return (0);
+	}
 
 	/*
 	 * Allocate new lease
@@ -315,9 +321,9 @@ doreply:
 		lp->lc_flag |= (LC_WRITE | LC_WRITTEN);
 	nqsrv_addhost(&lp->lc_host, slp, nam);
 	lp->lc_vp = vp;
-	lp->lc_fsid = fh.fh_fsid;
-	memcpy(lp->lc_fiddata, fh.fh_fid.fid_data,
-	    fh.fh_fid.fid_len - sizeof (int32_t));
+	lp->lc_fsid = fh.fh.fh_fsid;
+	memcpy(lp->lc_fiddata, fh.fh.fh_fid.fid_data,
+	    fh.fh.fh_fid.fid_len - sizeof (int32_t));
 	if(!lpp)
 		panic("nfs_nqlease.c: Phoney lpp");
 	LIST_INSERT_HEAD(lpp, lp, lc_hash);
@@ -473,6 +479,7 @@ nqsrv_send_eviction(vp, lp, slp, nam, cr
 	struct sockaddr_in *saddr;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
+	size_t fh_size;
 	caddr_t bpos, cp;
 	u_int32_t xid, *tl;
 	int len = 1, ok = 1, i = 0;
@@ -518,7 +525,8 @@ nqsrv_send_eviction(vp, lp, slp, nam, cr
 				NFSX_V3FH + NFSX_UNSIGNED);
 			memset(&nfh, 0, sizeof(nfh));
 			fhp = &nfh.fh_generic;
-			vfs_composefh(vp, fhp);
+			fh_size = NFS_SMALLFH;
+			vfs_composefh(vp, fhp, &fh_size);
 			nfsm_srvfhtom(fhp, 1);
 			m = mreq;
 			siz = 0;
@@ -800,7 +808,7 @@ nqnfsrv_vacated(nfsd, slp, lwp, mrq)
 		if (fhp->fh_fsid.__fsid_val[0] == lp->lc_fsid.__fsid_val[0] &&
 		    fhp->fh_fsid.__fsid_val[1] == lp->lc_fsid.__fsid_val[1] &&
 		    !memcmp(fhp->fh_fid.fid_data, lp->lc_fiddata,
-			  VFS_MAXFIDSZ)) {
+			  LC_MAXFIDSIZ)) {
 			/* Found it */
 			tlp = lp;
 			break;
Index: nfs_serv.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_serv.c,v
retrieving revision 1.113
diff -u -p -r1.113 nfs_serv.c
--- nfs_serv.c	30 Jun 2006 09:56:03 -0000	1.113
+++ nfs_serv.c	12 Jul 2006 19:10:45 -0000
@@ -377,8 +377,10 @@ nfsrv_lookup(nfsd, slp, lwp, mrq)
 	struct mbuf *mb, *mreq;
 	struct vattr va, dirattr;
 	u_quad_t frev;
+	size_t fh_size;
 
 	fhp = &nfh.fh_generic;
+	fh_size = NFS_SMALLFH;
 	nfsm_srvmtofh(fhp);
 	nfsm_srvnamesiz(len);
 
@@ -446,7 +448,7 @@ nfsrv_lookup(nfsd, slp, lwp, mrq)
 	vrele(ndp->ni_startdir);
 	PNBUF_PUT(nd.ni_cnd.cn_pnbuf);
 	vp = ndp->ni_vp;
-	error = vfs_composefh(vp, fhp);
+	error = vfs_composefh(vp, fhp, &fh_size);
 	if (!error)
 		error = VOP_GETATTR(vp, &va, cred, lwp);
 	vput(vp);
@@ -1133,7 +1135,7 @@ nfsmout:
 		LIST_INSERT_HEAD(&slp->ns_tq, nfsd, nd_tq);
 	    }
 	    if (nfsd->nd_mrep) {
-		wpp = NWDELAYHASH(slp, nfsd->nd_fh.fh_fid.fid_data);
+		wpp = NWDELAYHASH(slp, nfsd->nd_fh.fh_generic.fh_fid.fid_data);
 		owp = NULL;
 		wp = LIST_FIRST(wpp);
 		while (wp &&
@@ -1187,7 +1189,7 @@ loop1:
 		cred = nfsd->nd_cr;
 		v3 = (nfsd->nd_flag & ND_NFSV3);
 		forat_ret = aftat_ret = 1;
-		error = nfsrv_fhtovp(&nfsd->nd_fh, 1, &vp, cred, slp,
+		error = nfsrv_fhtovp(&nfsd->nd_fh.fh_generic, 1, &vp, cred, slp,
 		    nfsd->nd_nam, &rdonly, (nfsd->nd_flag & ND_KERBAUTH),
 		    FALSE);
 		if (!error) {
@@ -1411,12 +1413,14 @@ nfsrv_create(nfsd, slp, lwp, mrq)
 	struct vnode *vp = NULL, *dirp = NULL;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
+	size_t fh_size;
 	u_quad_t frev, tempsize;
 	u_char cverf[NFSX_V3CREATEVERF];
 	struct mount *mp = NULL;
 
 	nd.ni_cnd.cn_nameiop = 0;
 	fhp = &nfh.fh_generic;
+	fh_size = NFS_SMALLFH;
 	nfsm_srvmtofh(fhp);
 	if ((mp = vfs_getvfs(&fhp->fh_fsid)) == NULL)
 		return (ESTALE);
@@ -1564,7 +1568,7 @@ nfsrv_create(nfsd, slp, lwp, mrq)
 			vput(vp);
 	}
 	if (!error) {
-		error = vfs_composefh(vp, fhp);
+		error = vfs_composefh(vp, fhp, &fh_size);
 		if (!error)
 			error = VOP_GETATTR(vp, &va, cred, lwp);
 		vput(vp);
@@ -1640,11 +1644,13 @@ nfsrv_mknod(nfsd, slp, lwp, mrq)
 	struct vnode *vp, *dirp = (struct vnode *)0;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
+	size_t fh_size;
 	u_quad_t frev;
 	struct mount *mp = NULL;
 
 	nd.ni_cnd.cn_nameiop = 0;
 	fhp = &nfh.fh_generic;
+	fh_size = NFS_SMALLFH;
 	nfsm_srvmtofh(fhp);
 	if ((mp = vfs_getvfs(&fhp->fh_fsid)) == NULL)
 		return (ESTALE);
@@ -1728,7 +1734,7 @@ abort:
 out:
 	vp = nd.ni_vp;
 	if (!error) {
-		error = vfs_composefh(vp, fhp);
+		error = vfs_composefh(vp, fhp, &fh_size);
 		if (!error)
 			error = VOP_GETATTR(vp, &va, cred, lwp);
 		vput(vp);
@@ -2205,11 +2211,13 @@ nfsrv_symlink(nfsd, slp, lwp, mrq)
 	struct vnode *dirp = (struct vnode *)0;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
+	size_t fh_size;
 	u_quad_t frev;
 	struct mount *mp = NULL;
 
 	nd.ni_cnd.cn_nameiop = 0;
 	fhp = &nfh.fh_generic;
+	fh_size = NFS_SMALLFH;
 	nfsm_srvmtofh(fhp);
 	if ((mp = vfs_getvfs(&fhp->fh_fsid)) == NULL)
 		return (ESTALE);
@@ -2278,7 +2286,7 @@ abortop:
 	error = VOP_SYMLINK(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, &va, pathcp);
 	if (!error) {
 	    if (v3) {
-		error = vfs_composefh(nd.ni_vp, fhp);
+		error = vfs_composefh(nd.ni_vp, fhp, &fh_size);
 		if (!error)
 		    error = VOP_GETATTR(nd.ni_vp, &va, cred, lwp);
 		vput(nd.ni_vp);
@@ -2347,10 +2355,12 @@ nfsrv_mkdir(nfsd, slp, lwp, mrq)
 	struct vnode *vp, *dirp = (struct vnode *)0;
 	nfsfh_t nfh;
 	fhandle_t *fhp;
+	size_t fh_size;
 	u_quad_t frev;
 	struct mount *mp = NULL;
 
 	fhp = &nfh.fh_generic;
+	fh_size = NFS_SMALLFH;
 	nfsm_srvmtofh(fhp);
 	if ((mp = vfs_getvfs(&fhp->fh_fsid)) == NULL)
 		return (ESTALE);
@@ -2401,7 +2411,7 @@ nfsrv_mkdir(nfsd, slp, lwp, mrq)
 	error = VOP_MKDIR(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, &va);
 	if (!error) {
 		vp = nd.ni_vp;
-		error = vfs_composefh(vp, fhp);
+		error = vfs_composefh(vp, fhp, &fh_size);
 		if (!error)
 			error = VOP_GETATTR(vp, &va, cred, lwp);
 		vput(vp);
@@ -2868,6 +2878,7 @@ nfsrv_readdirplus(nfsd, slp, lwp, mrq)
 	struct flrep fl;
 	nfsfh_t nfh;
 	fhandle_t *fhp, *nfhp = (fhandle_t *)fl.fl_nfh;
+	size_t fh_size = sizeof(fl.fl_nfh);
 	struct uio io;
 	struct iovec iv;
 	struct vattr va, at, *vap = &va;
@@ -3038,7 +3049,7 @@ again:
 			 */
 			if (VFS_VGET(vp->v_mount, dp->d_fileno, &nvp))
 				goto invalid;
-			if (vfs_composefh(nvp, nfhp)) {
+			if (vfs_composefh(nvp, nfhp, &fh_size)) {
 				vput(nvp);
 				goto invalid;
 			}
Index: nfs_subs.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_subs.c,v
retrieving revision 1.165
diff -u -p -r1.165 nfs_subs.c
--- nfs_subs.c	7 Jun 2006 22:34:17 -0000	1.165
+++ nfs_subs.c	12 Jul 2006 19:10:46 -0000
@@ -2543,7 +2543,7 @@ nfsrv_fhtovp(fhp, lockflag, vpp, cred, s
 	if (nfs_ispublicfh(fhp)) {
 		if (!pubflag || !nfs_pub.np_valid)
 			return (ESTALE);
-		fhp = &nfs_pub.np_handle;
+		fhp = nfs_pub.np_handle;
 	}
 
 	error = netexport_check(&fhp->fh_fsid, nam, &mp, &exflags, &credanon);
Index: nfs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_vfsops.c,v
retrieving revision 1.157
diff -u -p -r1.157 nfs_vfsops.c
--- nfs_vfsops.c	7 Jun 2006 22:34:17 -0000	1.157
+++ nfs_vfsops.c	12 Jul 2006 19:10:46 -0000
@@ -1079,9 +1079,10 @@ nfs_fhtovp(mp, fhp, vpp)
  */
 /* ARGSUSED */
 int
-nfs_vptofh(vp, fhp)
+nfs_vptofh(vp, fhp, fh_size)
 	struct vnode *vp;
 	struct fid *fhp;
+	size_t *fh_size;
 {
 
 	return (EINVAL);
Index: nfsmount.h
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfsmount.h,v
retrieving revision 1.39
diff -u -p -r1.39 nfsmount.h
--- nfsmount.h	14 May 2006 21:32:21 -0000	1.39
+++ nfsmount.h	12 Jul 2006 19:10:46 -0000
@@ -199,7 +199,7 @@ int	nfs_sync __P((struct mount *mp, int 
 		struct lwp *p));
 int	nfs_vget __P((struct mount *, ino_t, struct vnode **));
 int	nfs_fhtovp __P((struct mount *mp, struct fid *fhp, struct vnode **vpp));
-int	nfs_vptofh __P((struct vnode *vp, struct fid *fhp));
+int	nfs_vptofh __P((struct vnode *vp, struct fid *fhp, size_t *fh_size));
 int	nfs_fsinfo __P((struct nfsmount *, struct vnode *, kauth_cred_t,
 			struct lwp *));
 void	nfs_vfs_init __P((void));
Index: nfsproto.h
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfsproto.h,v
retrieving revision 1.13
diff -u -p -r1.13 nfsproto.h
--- nfsproto.h	14 Mar 2006 03:23:23 -0000	1.13
+++ nfsproto.h	12 Jul 2006 19:10:46 -0000
@@ -113,7 +113,7 @@
 #define NFSX_V2STATFS	20
 
 /* specific to NFS Version 3 */
-#define NFSX_V3FH		(sizeof (fhandle_t)) /* size this server uses */
+#define NFSX_V3FH		(sizeof(fhandle_t)+16)
 #define	NFSX_V3FHMAX		64	/* max. allowed by protocol */
 #define NFSX_V3FATTR		84
 #define NFSX_V3SATTR		60	/* max. all fields filled in */
Index: nqnfs.h
===================================================================
RCS file: /cvsroot/src/sys/nfs/nqnfs.h,v
retrieving revision 1.19
diff -u -p -r1.19 nqnfs.h
--- nqnfs.h	7 Jun 2006 22:34:18 -0000	1.19
+++ nqnfs.h	12 Jul 2006 19:10:46 -0000
@@ -81,6 +81,7 @@
  * hashed on lc_fh.
  */
 #define	LC_MOREHOSTSIZ	10
+#define	LC_MAXFIDSIZ	64
 
 struct nqhost {
 	union {
@@ -116,7 +117,7 @@ struct nqlease {
 	struct nqhost	lc_host;	/* Host that got lease */
 	struct nqm	*lc_morehosts;	/* Other hosts that share read lease */
 	fsid_t		lc_fsid;	/* Fhandle */
-	char		lc_fiddata[VFS_MAXFIDSZ];
+	char		lc_fiddata[LC_MAXFIDSIZ];
 	struct vnode	*lc_vp;		/* Soft reference to associated vnode */
 };
 #define	lc_flag		lc_host.lph_un.un_udp.udp_flag

--huq684BweRXVnRxX--