[PATCH 0/3] NFSD patches to support junctions

[PATCH 0/3] NFSD patches to support junctions

[Chuck Lever]

Sometime soon we are going to have easy-to-install user space FedFS
components.  Here are kernel patches needed to make server-side FedFS
support work.  Please consider these for the 3.2 kernel.

The third patch introduces a potentially expensive check to see if
a junction has been encountered during a mountpoint lookup.  An object
is a junction iff it has the requisite set of extended attributes.
However, reading an extended attribute is expensive on some file
systems.

To mitigate the cost of this check, junctions always have their sticky
bit set.  The expensive extended attribute part of the junction test
is done only if the sticky bit is present.

Note that today junctions are directories, but someday symlinks might
also act as junctions (for SMB2 support).  And very few files have the
sticky bit set.  So we avoid doing a directory test here.

Also, junctions ostensibly have all zero mode bits to hide their local
contents.  I don't think the kernel needs to be concerned about the
permissions as long as the sticky bit is set.  This allows some
flexibility in how junctions are represented.  However, Jeff thinks
that having nfsd4_is_junction() also consider mode bits would make the
expensive part of this test still less frequent.

Any thoughts about this?

---

Trond Myklebust (3):
      NFSD: Add a cache for fs_locations information
      NFSD: Remove the ex_pathname field from struct svc_export
      NFSD: Cleanup for nfsd4_path()


 fs/nfsd/export.c            |   15 +-----
 fs/nfsd/nfs4xdr.c           |  106 ++++++++++++++++++++++++++++++++-----------
 fs/nfsd/nfsd.h              |    7 +++
 fs/nfsd/vfs.c               |   16 ++++++
 include/linux/nfsd/export.h |    2 -
 5 files changed, 104 insertions(+), 42 deletions(-)

-- 
Chuck Lever

[PATCH 1/3] NFSD: Cleanup for nfsd4_path()

[Chuck Lever]

From: Trond Myklebust <Trond.Myklebust@netapp.com>

The current code is sort of hackish in that it assumes a referral is always
matched to an export. When we add support for junctions that may not be the
case.
We can replace nfsd4_path() with a function that encodes the components
directly from the dentries. Since nfsd4_path is currently the only user of
the 'ex_pathname' field in struct svc_export, this has the added benefit
of allowing us to get rid of that.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfsd/export.c            |    4 +-
 fs/nfsd/nfs4xdr.c           |  106 ++++++++++++++++++++++++++++++++-----------
 include/linux/nfsd/export.h |    1 
 3 files changed, 81 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index f4cc1e2..d9a6611 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -1010,7 +1010,7 @@ rqst_exp_parent(struct svc_rqst *rqstp, struct path *path)
 	return exp;
 }
 
-static struct svc_export *find_fsidzero_export(struct svc_rqst *rqstp)
+struct svc_export *rqst_find_fsidzero_export(struct svc_rqst *rqstp)
 {
 	u32 fsidv[2];
 
@@ -1030,7 +1030,7 @@ exp_pseudoroot(struct svc_rqst *rqstp, struct svc_fh *fhp)
 	struct svc_export *exp;
 	__be32 rv;
 
-	exp = find_fsidzero_export(rqstp);
+	exp = rqst_find_fsidzero_export(rqstp);
 	if (IS_ERR(exp))
 		return nfserrno(PTR_ERR(exp));
 	rv = fh_compose(fhp, exp, exp->ex_path.dentry, NULL);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index c8bf405..f7b068e 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1697,36 +1697,89 @@ static __be32 nfsd4_encode_fs_location4(struct nfsd4_fs_location *location,
 }
 
 /*
- * Return the path to an export point in the pseudo filesystem namespace
- * Returned string is safe to use as long as the caller holds a reference
- * to @exp.
+ * Encode a path in RFC3530 'pathname4' format
  */
-static char *nfsd4_path(struct svc_rqst *rqstp, struct svc_export *exp, __be32 *stat)
+static __be32 nfsd4_encode_path(const struct path *root,
+		const struct path *path, __be32 **pp, int *buflen)
 {
-	struct svc_fh tmp_fh;
-	char *path = NULL, *rootpath;
-	size_t rootlen;
+	struct path cur = {
+		.mnt = path->mnt,
+		.dentry = path->dentry,
+	};
+	__be32 *p = *pp;
+	struct dentry **components = NULL;
+	unsigned int ncomponents = 0;
+	__be32 err = nfserr_resource;
 
-	fh_init(&tmp_fh, NFS4_FHSIZE);
-	*stat = exp_pseudoroot(rqstp, &tmp_fh);
-	if (*stat)
-		return NULL;
-	rootpath = tmp_fh.fh_export->ex_pathname;
+	dprintk("nfsd4_encode_components(");
 
-	path = exp->ex_pathname;
+	path_get(&cur);
+	/* First walk the path up to the nfsd root, and store the
+	 * dentries/path components in an array.
+	 */
+	for (;;) {
+		if (cur.dentry == root->dentry && cur.mnt == root->mnt)
+			break;
+		if (cur.dentry == cur.mnt->mnt_root) {
+			if (follow_up(&cur))
+				continue;
+			goto out_free;
+		}
+		if ((ncomponents & 15) == 0) {
+			struct dentry **new;
+			new = krealloc(components,
+					sizeof(*new) * (ncomponents + 16),
+					GFP_KERNEL);
+			if (!new)
+				goto out_free;
+			components = new;
+		}
+		components[ncomponents++] = cur.dentry;
+		cur.dentry = dget_parent(cur.dentry);
+	}
 
-	rootlen = strlen(rootpath);
-	if (strncmp(path, rootpath, rootlen)) {
-		dprintk("nfsd: fs_locations failed;"
-			"%s is not contained in %s\n", path, rootpath);
-		*stat = nfserr_notsupp;
-		path = NULL;
-		goto out;
+	*buflen -= 4;
+	if (*buflen < 0)
+		goto out_free;
+	WRITE32(ncomponents);
+
+	while (ncomponents) {
+		struct dentry *dentry = components[ncomponents - 1];
+		unsigned int len = dentry->d_name.len;
+
+		*buflen -= 4 + (XDR_QUADLEN(len) << 2);
+		if (*buflen < 0)
+			goto out_free;
+		WRITE32(len);
+		WRITEMEM(dentry->d_name.name, len);
+		dprintk("/%s", dentry->d_name.name);
+		dput(dentry);
+		ncomponents--;
 	}
-	path += rootlen;
-out:
-	fh_put(&tmp_fh);
-	return path;
+
+	*pp = p;
+	err = 0;
+out_free:
+	dprintk(")\n");
+	while (ncomponents)
+		dput(components[--ncomponents]);
+	kfree(components);
+	path_put(&cur);
+	return err;
+}
+
+static __be32 nfsd4_encode_fsloc_fsroot(struct svc_rqst *rqstp,
+		const struct path *path, __be32 **pp, int *buflen)
+{
+	struct svc_export *exp_ps;
+	__be32 res;
+
+	exp_ps = rqst_find_fsidzero_export(rqstp);
+	if (IS_ERR(exp_ps))
+		return nfserrno(PTR_ERR(exp_ps));
+	res = nfsd4_encode_path(&exp_ps->ex_path, path, pp, buflen);
+	exp_put(exp_ps);
+	return res;
 }
 
 /*
@@ -1740,11 +1793,8 @@ static __be32 nfsd4_encode_fs_locations(struct svc_rqst *rqstp,
 	int i;
 	__be32 *p = *pp;
 	struct nfsd4_fs_locations *fslocs = &exp->ex_fslocs;
-	char *root = nfsd4_path(rqstp, exp, &status);
 
-	if (status)
-		return status;
-	status = nfsd4_encode_components('/', root, &p, buflen);
+	status = nfsd4_encode_fsloc_fsroot(rqstp, &exp->ex_path, &p, buflen);
 	if (status)
 		return status;
 	if ((*buflen -= 4) < 0)
diff --git a/include/linux/nfsd/export.h b/include/linux/nfsd/export.h
index 8a31a20..7ba3fd4 100644
--- a/include/linux/nfsd/export.h
+++ b/include/linux/nfsd/export.h
@@ -137,6 +137,7 @@ struct svc_export *	rqst_exp_get_by_name(struct svc_rqst *,
 					     struct path *);
 struct svc_export *	rqst_exp_parent(struct svc_rqst *,
 					struct path *);
+struct svc_export *	rqst_find_fsidzero_export(struct svc_rqst *);
 int			exp_rootfh(struct auth_domain *, 
 					char *path, struct knfsd_fh *, int maxsize);
 __be32			exp_pseudoroot(struct svc_rqst *, struct svc_fh *);

Re: [PATCH 1/3] NFSD: Cleanup for nfsd4_path()

[J. Bruce Fields]

On Fri, Sep 02, 2011 at 12:38:23PM -0400, Chuck Lever wrote:
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> The current code is sort of hackish in that it assumes a referral is always
> matched to an export. When we add support for junctions that may not be the
> case.
> We can replace nfsd4_path() with a function that encodes the components
> directly from the dentries. Since nfsd4_path is currently the only user of
> the 'ex_pathname' field in struct svc_export, this has the added benefit
> of allowing us to get rid of that.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> 
>  fs/nfsd/export.c            |    4 +-
>  fs/nfsd/nfs4xdr.c           |  106 ++++++++++++++++++++++++++++++++-----------
>  include/linux/nfsd/export.h |    1 
>  3 files changed, 81 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index f4cc1e2..d9a6611 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -1010,7 +1010,7 @@ rqst_exp_parent(struct svc_rqst *rqstp, struct path *path)
>  	return exp;
>  }
>  
> -static struct svc_export *find_fsidzero_export(struct svc_rqst *rqstp)
> +struct svc_export *rqst_find_fsidzero_export(struct svc_rqst *rqstp)
>  {
>  	u32 fsidv[2];
>  
> @@ -1030,7 +1030,7 @@ exp_pseudoroot(struct svc_rqst *rqstp, struct svc_fh *fhp)
>  	struct svc_export *exp;
>  	__be32 rv;
>  
> -	exp = find_fsidzero_export(rqstp);
> +	exp = rqst_find_fsidzero_export(rqstp);
>  	if (IS_ERR(exp))
>  		return nfserrno(PTR_ERR(exp));
>  	rv = fh_compose(fhp, exp, exp->ex_path.dentry, NULL);
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index c8bf405..f7b068e 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1697,36 +1697,89 @@ static __be32 nfsd4_encode_fs_location4(struct nfsd4_fs_location *location,
>  }
>  
>  /*
> - * Return the path to an export point in the pseudo filesystem namespace
> - * Returned string is safe to use as long as the caller holds a reference
> - * to @exp.
> + * Encode a path in RFC3530 'pathname4' format
>   */
> -static char *nfsd4_path(struct svc_rqst *rqstp, struct svc_export *exp, __be32 *stat)
> +static __be32 nfsd4_encode_path(const struct path *root,
> +		const struct path *path, __be32 **pp, int *buflen)
>  {
> -	struct svc_fh tmp_fh;
> -	char *path = NULL, *rootpath;
> -	size_t rootlen;
> +	struct path cur = {
> +		.mnt = path->mnt,
> +		.dentry = path->dentry,
> +	};
> +	__be32 *p = *pp;
> +	struct dentry **components = NULL;
> +	unsigned int ncomponents = 0;
> +	__be32 err = nfserr_resource;

I've been fixing nfsd to return nfserr_jukebox for allocation errors,
and reserving nfserr_resource (not even available in 4.1) for cases
where the error is permanent (e.g. I just can't process a compound with
1000 ops).

Patch looks fine otherwise.

--b.

>  
> -	fh_init(&tmp_fh, NFS4_FHSIZE);
> -	*stat = exp_pseudoroot(rqstp, &tmp_fh);
> -	if (*stat)
> -		return NULL;
> -	rootpath = tmp_fh.fh_export->ex_pathname;
> +	dprintk("nfsd4_encode_components(");
>  
> -	path = exp->ex_pathname;
> +	path_get(&cur);
> +	/* First walk the path up to the nfsd root, and store the
> +	 * dentries/path components in an array.
> +	 */
> +	for (;;) {
> +		if (cur.dentry == root->dentry && cur.mnt == root->mnt)
> +			break;
> +		if (cur.dentry == cur.mnt->mnt_root) {
> +			if (follow_up(&cur))
> +				continue;
> +			goto out_free;
> +		}
> +		if ((ncomponents & 15) == 0) {
> +			struct dentry **new;
> +			new = krealloc(components,
> +					sizeof(*new) * (ncomponents + 16),
> +					GFP_KERNEL);
> +			if (!new)
> +				goto out_free;
> +			components = new;
> +		}
> +		components[ncomponents++] = cur.dentry;
> +		cur.dentry = dget_parent(cur.dentry);
> +	}
>  
> -	rootlen = strlen(rootpath);
> -	if (strncmp(path, rootpath, rootlen)) {
> -		dprintk("nfsd: fs_locations failed;"
> -			"%s is not contained in %s\n", path, rootpath);
> -		*stat = nfserr_notsupp;
> -		path = NULL;
> -		goto out;
> +	*buflen -= 4;
> +	if (*buflen < 0)
> +		goto out_free;
> +	WRITE32(ncomponents);
> +
> +	while (ncomponents) {
> +		struct dentry *dentry = components[ncomponents - 1];
> +		unsigned int len = dentry->d_name.len;
> +
> +		*buflen -= 4 + (XDR_QUADLEN(len) << 2);
> +		if (*buflen < 0)
> +			goto out_free;
> +		WRITE32(len);
> +		WRITEMEM(dentry->d_name.name, len);
> +		dprintk("/%s", dentry->d_name.name);
> +		dput(dentry);
> +		ncomponents--;
>  	}
> -	path += rootlen;
> -out:
> -	fh_put(&tmp_fh);
> -	return path;
> +
> +	*pp = p;
> +	err = 0;
> +out_free:
> +	dprintk(")\n");
> +	while (ncomponents)
> +		dput(components[--ncomponents]);
> +	kfree(components);
> +	path_put(&cur);
> +	return err;
> +}
> +
> +static __be32 nfsd4_encode_fsloc_fsroot(struct svc_rqst *rqstp,
> +		const struct path *path, __be32 **pp, int *buflen)
> +{
> +	struct svc_export *exp_ps;
> +	__be32 res;
> +
> +	exp_ps = rqst_find_fsidzero_export(rqstp);
> +	if (IS_ERR(exp_ps))
> +		return nfserrno(PTR_ERR(exp_ps));
> +	res = nfsd4_encode_path(&exp_ps->ex_path, path, pp, buflen);
> +	exp_put(exp_ps);
> +	return res;
>  }
>  
>  /*
> @@ -1740,11 +1793,8 @@ static __be32 nfsd4_encode_fs_locations(struct svc_rqst *rqstp,
>  	int i;
>  	__be32 *p = *pp;
>  	struct nfsd4_fs_locations *fslocs = &exp->ex_fslocs;
> -	char *root = nfsd4_path(rqstp, exp, &status);
>  
> -	if (status)
> -		return status;
> -	status = nfsd4_encode_components('/', root, &p, buflen);
> +	status = nfsd4_encode_fsloc_fsroot(rqstp, &exp->ex_path, &p, buflen);
>  	if (status)
>  		return status;
>  	if ((*buflen -= 4) < 0)
> diff --git a/include/linux/nfsd/export.h b/include/linux/nfsd/export.h
> index 8a31a20..7ba3fd4 100644
> --- a/include/linux/nfsd/export.h
> +++ b/include/linux/nfsd/export.h
> @@ -137,6 +137,7 @@ struct svc_export *	rqst_exp_get_by_name(struct svc_rqst *,
>  					     struct path *);
>  struct svc_export *	rqst_exp_parent(struct svc_rqst *,
>  					struct path *);
> +struct svc_export *	rqst_find_fsidzero_export(struct svc_rqst *);
>  int			exp_rootfh(struct auth_domain *, 
>  					char *path, struct knfsd_fh *, int maxsize);
>  __be32			exp_pseudoroot(struct svc_rqst *, struct svc_fh *);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] NFSD: Remove the ex_pathname field from struct svc_export

[Chuck Lever]

From: Trond Myklebust <Trond.Myklebust@netapp.com>

There are no more users...

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfsd/export.c            |   11 -----------
 include/linux/nfsd/export.h |    1 -
 2 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index d9a6611..bf985f7 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -318,7 +318,6 @@ static void svc_export_put(struct kref *ref)
 	struct svc_export *exp = container_of(ref, struct svc_export, h.ref);
 	path_put(&exp->ex_path);
 	auth_domain_put(exp->ex_client);
-	kfree(exp->ex_pathname);
 	nfsd4_fslocs_free(&exp->ex_fslocs);
 	kfree(exp);
 }
@@ -528,11 +527,6 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen)
 
 	exp.ex_client = dom;
 
-	err = -ENOMEM;
-	exp.ex_pathname = kstrdup(buf, GFP_KERNEL);
-	if (!exp.ex_pathname)
-		goto out2;
-
 	/* expiry */
 	err = -EINVAL;
 	exp.h.expiry_time = get_expiry(&mesg);
@@ -613,8 +607,6 @@ out4:
 	nfsd4_fslocs_free(&exp.ex_fslocs);
 	kfree(exp.ex_uuid);
 out3:
-	kfree(exp.ex_pathname);
-out2:
 	path_put(&exp.ex_path);
 out1:
 	auth_domain_put(dom);
@@ -678,7 +670,6 @@ static void svc_export_init(struct cache_head *cnew, struct cache_head *citem)
 	new->ex_client = item->ex_client;
 	new->ex_path.dentry = dget(item->ex_path.dentry);
 	new->ex_path.mnt = mntget(item->ex_path.mnt);
-	new->ex_pathname = NULL;
 	new->ex_fslocs.locations = NULL;
 	new->ex_fslocs.locations_count = 0;
 	new->ex_fslocs.migrated = 0;
@@ -696,8 +687,6 @@ static void export_update(struct cache_head *cnew, struct cache_head *citem)
 	new->ex_fsid = item->ex_fsid;
 	new->ex_uuid = item->ex_uuid;
 	item->ex_uuid = NULL;
-	new->ex_pathname = item->ex_pathname;
-	item->ex_pathname = NULL;
 	new->ex_fslocs.locations = item->ex_fslocs.locations;
 	item->ex_fslocs.locations = NULL;
 	new->ex_fslocs.locations_count = item->ex_fslocs.locations_count;
diff --git a/include/linux/nfsd/export.h b/include/linux/nfsd/export.h
index 7ba3fd4..f85308e 100644
--- a/include/linux/nfsd/export.h
+++ b/include/linux/nfsd/export.h
@@ -96,7 +96,6 @@ struct svc_export {
 	struct auth_domain *	ex_client;
 	int			ex_flags;
 	struct path		ex_path;
-	char			*ex_pathname;
 	uid_t			ex_anon_uid;
 	gid_t			ex_anon_gid;
 	int			ex_fsid;

[PATCH 3/3] NFSD: Add a cache for fs_locations information

[Chuck Lever]

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
[ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
[ cel: implement S_ISVTX filter in bfields-normal form ]
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsd.h |    7 +++++++
 fs/nfsd/vfs.c  |   16 ++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7ecfa24..d314812 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
 #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
 	NFSD_WRITEABLE_ATTRS_WORD2
 
+extern int nfsd4_is_junction(struct dentry *dentry);
+#else
+static inline int nfsd4_is_junction(struct dentry *dentry)
+{
+	return 0;
+}
+
 #endif /* CONFIG_NFSD_V4 */
 
 #endif /* LINUX_NFSD_NFSD_H */
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index fd0acca..1f2d5bf 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
 {
 	if (d_mountpoint(dentry))
 		return 1;
+	if (nfsd4_is_junction(dentry))
+		return 1;
 	if (!(exp->ex_flags & NFSEXP_V4ROOT))
 		return 0;
 	return dentry->d_inode != NULL;
@@ -592,6 +594,20 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
 	return error;
 }
 
+#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
+#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
+int nfsd4_is_junction(struct dentry *dentry)
+{
+	struct inode *inode = dentry->d_inode;
+
+	if (inode == NULL)
+		return 0;
+	if (!(inode->i_mode & S_ISVTX))
+		return 0;
+	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
+		return 0;
+	return 1;
+}
 #endif /* defined(CONFIG_NFSD_V4) */
 
 #ifdef CONFIG_NFSD_V3

Re: [PATCH 3/3] NFSD: Add a cache for fs_locations information

[J. Bruce Fields]

On Fri, Sep 02, 2011 at 12:38:42PM -0400, Chuck Lever wrote:
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
> [ cel: implement S_ISVTX filter in bfields-normal form ]
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

OK, and this is all it takes to get entries for these added to the
export cache, so we're done?

--b.

> ---
> 
>  fs/nfsd/nfsd.h |    7 +++++++
>  fs/nfsd/vfs.c  |   16 ++++++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 7ecfa24..d314812 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
>  #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
>  	NFSD_WRITEABLE_ATTRS_WORD2
>  
> +extern int nfsd4_is_junction(struct dentry *dentry);
> +#else
> +static inline int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
>  #endif /* CONFIG_NFSD_V4 */
>  
>  #endif /* LINUX_NFSD_NFSD_H */
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index fd0acca..1f2d5bf 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
>  {
>  	if (d_mountpoint(dentry))
>  		return 1;
> +	if (nfsd4_is_junction(dentry))
> +		return 1;
>  	if (!(exp->ex_flags & NFSEXP_V4ROOT))
>  		return 0;
>  	return dentry->d_inode != NULL;
> @@ -592,6 +594,20 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
>  	return error;
>  }
>  
> +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
> +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
> +int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	struct inode *inode = dentry->d_inode;
> +
> +	if (inode == NULL)
> +		return 0;
> +	if (!(inode->i_mode & S_ISVTX))
> +		return 0;
> +	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
> +		return 0;
> +	return 1;
> +}
>  #endif /* defined(CONFIG_NFSD_V4) */
>  
>  #ifdef CONFIG_NFSD_V3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] NFSD: Add a cache for fs_locations information

[Chuck Lever]

On Sep 8, 2011, at 11:21 AM, J. Bruce Fields wrote:

> On Fri, Sep 02, 2011 at 12:38:42PM -0400, Chuck Lever wrote:
>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
>> 
>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>> [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
>> [ cel: implement S_ISVTX filter in bfields-normal form ]
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> 
> OK, and this is all it takes to get entries for these added to the
> export cache, so we're done?

The two important future NFSD work items are:

1.  Make the "refer=" mountd reply rich enough to return fs_locations_info data
2.  Server-side support for fs_locations_info operations in NFSv4 compounds

But neither of these are necessary for basic junction support.

> 
> --b.
> 
>> ---
>> 
>> fs/nfsd/nfsd.h |    7 +++++++
>> fs/nfsd/vfs.c  |   16 ++++++++++++++++
>> 2 files changed, 23 insertions(+), 0 deletions(-)
>> 
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 7ecfa24..d314812 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
>> #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
>> 	NFSD_WRITEABLE_ATTRS_WORD2
>> 
>> +extern int nfsd4_is_junction(struct dentry *dentry);
>> +#else
>> +static inline int nfsd4_is_junction(struct dentry *dentry)
>> +{
>> +	return 0;
>> +}
>> +
>> #endif /* CONFIG_NFSD_V4 */
>> 
>> #endif /* LINUX_NFSD_NFSD_H */
>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>> index fd0acca..1f2d5bf 100644
>> --- a/fs/nfsd/vfs.c
>> +++ b/fs/nfsd/vfs.c
>> @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
>> {
>> 	if (d_mountpoint(dentry))
>> 		return 1;
>> +	if (nfsd4_is_junction(dentry))
>> +		return 1;
>> 	if (!(exp->ex_flags & NFSEXP_V4ROOT))
>> 		return 0;
>> 	return dentry->d_inode != NULL;
>> @@ -592,6 +594,20 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
>> 	return error;
>> }
>> 
>> +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
>> +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
>> +int nfsd4_is_junction(struct dentry *dentry)
>> +{
>> +	struct inode *inode = dentry->d_inode;
>> +
>> +	if (inode == NULL)
>> +		return 0;
>> +	if (!(inode->i_mode & S_ISVTX))
>> +		return 0;
>> +	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
>> +		return 0;
>> +	return 1;
>> +}
>> #endif /* defined(CONFIG_NFSD_V4) */
>> 
>> #ifdef CONFIG_NFSD_V3
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/3] NFSD patches to support junctions

[J. Bruce Fields]

On Fri, Sep 02, 2011 at 12:38:13PM -0400, Chuck Lever wrote:
> Sometime soon we are going to have easy-to-install user space FedFS
> components.  Here are kernel patches needed to make server-side FedFS
> support work.  Please consider these for the 3.2 kernel.
> 
> The third patch introduces a potentially expensive check to see if
> a junction has been encountered during a mountpoint lookup.  An object
> is a junction iff it has the requisite set of extended attributes.
> However, reading an extended attribute is expensive on some file
> systems.
> 
> To mitigate the cost of this check, junctions always have their sticky
> bit set.  The expensive extended attribute part of the junction test
> is done only if the sticky bit is present.
> 
> Note that today junctions are directories, but someday symlinks might
> also act as junctions (for SMB2 support).  And very few files have the
> sticky bit set.  So we avoid doing a directory test here.
> 
> Also, junctions ostensibly have all zero mode bits to hide their local
> contents.  I don't think the kernel needs to be concerned about the
> permissions as long as the sticky bit is set.  This allows some
> flexibility in how junctions are represented.  However, Jeff thinks
> that having nfsd4_is_junction() also consider mode bits would make the
> expensive part of this test still less frequent.
> 
> Any thoughts about this?

Hm, right, a sticky bit set on a directory is a normal thing.  I thought
Trond's original idea was to check for the sticky bit and not
executable?  Which is a pointless combination so should be very rare.

On a typical system maybe directories with the sticky bit are normally
somewhat rare, but that's a question of policy and there could be cases
where it's common.

--b.

> 
> ---
> 
> Trond Myklebust (3):
>       NFSD: Add a cache for fs_locations information
>       NFSD: Remove the ex_pathname field from struct svc_export
>       NFSD: Cleanup for nfsd4_path()
> 
> 
>  fs/nfsd/export.c            |   15 +-----
>  fs/nfsd/nfs4xdr.c           |  106 ++++++++++++++++++++++++++++++++-----------
>  fs/nfsd/nfsd.h              |    7 +++
>  fs/nfsd/vfs.c               |   16 ++++++
>  include/linux/nfsd/export.h |    2 -
>  5 files changed, 104 insertions(+), 42 deletions(-)
> 
> -- 
> Chuck Lever
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] NFSD patches to support junctions

[J. Bruce Fields]

On Thu, Sep 08, 2011 at 01:59:46PM -0400, bfields wrote:
> On Fri, Sep 02, 2011 at 12:38:13PM -0400, Chuck Lever wrote:
> > Sometime soon we are going to have easy-to-install user space FedFS
> > components.  Here are kernel patches needed to make server-side FedFS
> > support work.  Please consider these for the 3.2 kernel.
> > 
> > The third patch introduces a potentially expensive check to see if
> > a junction has been encountered during a mountpoint lookup.  An object
> > is a junction iff it has the requisite set of extended attributes.
> > However, reading an extended attribute is expensive on some file
> > systems.
> > 
> > To mitigate the cost of this check, junctions always have their sticky
> > bit set.  The expensive extended attribute part of the junction test
> > is done only if the sticky bit is present.
> > 
> > Note that today junctions are directories, but someday symlinks might
> > also act as junctions (for SMB2 support).  And very few files have the
> > sticky bit set.  So we avoid doing a directory test here.
> > 
> > Also, junctions ostensibly have all zero mode bits to hide their local
> > contents.  I don't think the kernel needs to be concerned about the
> > permissions as long as the sticky bit is set.  This allows some
> > flexibility in how junctions are represented.  However, Jeff thinks
> > that having nfsd4_is_junction() also consider mode bits would make the
> > expensive part of this test still less frequent.
> > 
> > Any thoughts about this?
> 
> Hm, right, a sticky bit set on a directory is a normal thing.  I thought
> Trond's original idea was to check for the sticky bit and not
> executable?  Which is a pointless combination so should be very rare.
> 
> On a typical system maybe directories with the sticky bit are normally
> somewhat rare, but that's a question of policy and there could be cases
> where it's common.

Except for that and the one gripe about an error return, it looks fine
to me.

Remind me where the corresponding userland code is?  Would it make sense
to add some documentation with a link to that?

--b.

Re: [PATCH 0/3] NFSD patches to support junctions

[Chuck Lever]

On Sep 8, 2011, at 11:24 AM, J. Bruce Fields wrote:

> On Thu, Sep 08, 2011 at 01:59:46PM -0400, bfields wrote:
>> On Fri, Sep 02, 2011 at 12:38:13PM -0400, Chuck Lever wrote:
>>> Sometime soon we are going to have easy-to-install user space FedFS
>>> components.  Here are kernel patches needed to make server-side FedFS
>>> support work.  Please consider these for the 3.2 kernel.
>>> 
>>> The third patch introduces a potentially expensive check to see if
>>> a junction has been encountered during a mountpoint lookup.  An object
>>> is a junction iff it has the requisite set of extended attributes.
>>> However, reading an extended attribute is expensive on some file
>>> systems.
>>> 
>>> To mitigate the cost of this check, junctions always have their sticky
>>> bit set.  The expensive extended attribute part of the junction test
>>> is done only if the sticky bit is present.
>>> 
>>> Note that today junctions are directories, but someday symlinks might
>>> also act as junctions (for SMB2 support).  And very few files have the
>>> sticky bit set.  So we avoid doing a directory test here.
>>> 
>>> Also, junctions ostensibly have all zero mode bits to hide their local
>>> contents.  I don't think the kernel needs to be concerned about the
>>> permissions as long as the sticky bit is set.  This allows some
>>> flexibility in how junctions are represented.  However, Jeff thinks
>>> that having nfsd4_is_junction() also consider mode bits would make the
>>> expensive part of this test still less frequent.
>>> 
>>> Any thoughts about this?
>> 
>> Hm, right, a sticky bit set on a directory is a normal thing.  I thought
>> Trond's original idea was to check for the sticky bit and not
>> executable?  Which is a pointless combination so should be very rare.
>> 
>> On a typical system maybe directories with the sticky bit are normally
>> somewhat rare, but that's a question of policy and there could be cases
>> where it's common.
> 
> Except for that and the one gripe about an error return, it looks fine
> to me.

Noted.  I can repost these with requested fixes in a day or two.

> Remind me where the corresponding userland code is?

http://oss.oracle.com/projects/fedfs-utils

> Would it make sense to add some documentation with a link to that?

I don't see strong utility for documenting it here, but others may disagree.  One problem is that the user space components can move, and then the URL is out of date and long forgotten.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/3] NFSD patches to support junctions

[J. Bruce Fields]

On Thu, Sep 08, 2011 at 12:03:49PM -0700, Chuck Lever wrote:
> 
> On Sep 8, 2011, at 11:24 AM, J. Bruce Fields wrote:
> 
> > On Thu, Sep 08, 2011 at 01:59:46PM -0400, bfields wrote:
> >> On Fri, Sep 02, 2011 at 12:38:13PM -0400, Chuck Lever wrote:
> >>> Sometime soon we are going to have easy-to-install user space FedFS
> >>> components.  Here are kernel patches needed to make server-side FedFS
> >>> support work.  Please consider these for the 3.2 kernel.
> >>> 
> >>> The third patch introduces a potentially expensive check to see if
> >>> a junction has been encountered during a mountpoint lookup.  An object
> >>> is a junction iff it has the requisite set of extended attributes.
> >>> However, reading an extended attribute is expensive on some file
> >>> systems.
> >>> 
> >>> To mitigate the cost of this check, junctions always have their sticky
> >>> bit set.  The expensive extended attribute part of the junction test
> >>> is done only if the sticky bit is present.
> >>> 
> >>> Note that today junctions are directories, but someday symlinks might
> >>> also act as junctions (for SMB2 support).  And very few files have the
> >>> sticky bit set.  So we avoid doing a directory test here.
> >>> 
> >>> Also, junctions ostensibly have all zero mode bits to hide their local
> >>> contents.  I don't think the kernel needs to be concerned about the
> >>> permissions as long as the sticky bit is set.  This allows some
> >>> flexibility in how junctions are represented.  However, Jeff thinks
> >>> that having nfsd4_is_junction() also consider mode bits would make the
> >>> expensive part of this test still less frequent.
> >>> 
> >>> Any thoughts about this?
> >> 
> >> Hm, right, a sticky bit set on a directory is a normal thing.  I thought
> >> Trond's original idea was to check for the sticky bit and not
> >> executable?  Which is a pointless combination so should be very rare.
> >> 
> >> On a typical system maybe directories with the sticky bit are normally
> >> somewhat rare, but that's a question of policy and there could be cases
> >> where it's common.
> > 
> > Except for that and the one gripe about an error return, it looks fine
> > to me.
> 
> Noted.  I can repost these with requested fixes in a day or two.
> 
> > Remind me where the corresponding userland code is?
> 
> http://oss.oracle.com/projects/fedfs-utils

I can find gitweb, but not a git url to clone from.

> 
> > Would it make sense to add some documentation with a link to that?
> 
> I don't see strong utility for documenting it here, but others may disagree.  One problem is that the user space components can move, and then the URL is out of date and long forgotten.

OK.

--b.

Re: [PATCH 0/3] NFSD patches to support junctions

[Chuck Lever]

On Sep 8, 2011, at 12:21 PM, J. Bruce Fields wrote:

> On Thu, Sep 08, 2011 at 12:03:49PM -0700, Chuck Lever wrote:
>> 
>> On Sep 8, 2011, at 11:24 AM, J. Bruce Fields wrote:
>> 
>>> On Thu, Sep 08, 2011 at 01:59:46PM -0400, bfields wrote:
>>>> On Fri, Sep 02, 2011 at 12:38:13PM -0400, Chuck Lever wrote:
>>>>> Sometime soon we are going to have easy-to-install user space FedFS
>>>>> components.  Here are kernel patches needed to make server-side FedFS
>>>>> support work.  Please consider these for the 3.2 kernel.
>>>>> 
>>>>> The third patch introduces a potentially expensive check to see if
>>>>> a junction has been encountered during a mountpoint lookup.  An object
>>>>> is a junction iff it has the requisite set of extended attributes.
>>>>> However, reading an extended attribute is expensive on some file
>>>>> systems.
>>>>> 
>>>>> To mitigate the cost of this check, junctions always have their sticky
>>>>> bit set.  The expensive extended attribute part of the junction test
>>>>> is done only if the sticky bit is present.
>>>>> 
>>>>> Note that today junctions are directories, but someday symlinks might
>>>>> also act as junctions (for SMB2 support).  And very few files have the
>>>>> sticky bit set.  So we avoid doing a directory test here.
>>>>> 
>>>>> Also, junctions ostensibly have all zero mode bits to hide their local
>>>>> contents.  I don't think the kernel needs to be concerned about the
>>>>> permissions as long as the sticky bit is set.  This allows some
>>>>> flexibility in how junctions are represented.  However, Jeff thinks
>>>>> that having nfsd4_is_junction() also consider mode bits would make the
>>>>> expensive part of this test still less frequent.
>>>>> 
>>>>> Any thoughts about this?
>>>> 
>>>> Hm, right, a sticky bit set on a directory is a normal thing.  I thought
>>>> Trond's original idea was to check for the sticky bit and not
>>>> executable?  Which is a pointless combination so should be very rare.
>>>> 
>>>> On a typical system maybe directories with the sticky bit are normally
>>>> somewhat rare, but that's a question of policy and there could be cases
>>>> where it's common.
>>> 
>>> Except for that and the one gripe about an error return, it looks fine
>>> to me.
>> 
>> Noted.  I can repost these with requested fixes in a day or two.
>> 
>>> Remind me where the corresponding userland code is?
>> 
>> http://oss.oracle.com/projects/fedfs-utils
> 
> I can find gitweb, but not a git url to clone from.

Try

  git clone git://oss.oracle.com/git/cel/fedfs-utils.git

> 
>> 
>>> Would it make sense to add some documentation with a link to that?
>> 
>> I don't see strong utility for documenting it here, but others may disagree.  One problem is that the user space components can move, and then the URL is out of date and long forgotten.
> 
> OK.
> 
> --b.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/3] NFSD patches to support junctions

[J. Bruce Fields]

On Thu, Sep 08, 2011 at 09:47:50PM -0700, Chuck Lever wrote:
> 
> On Sep 8, 2011, at 12:21 PM, J. Bruce Fields wrote:
> 
> > On Thu, Sep 08, 2011 at 12:03:49PM -0700, Chuck Lever wrote:
> >> 
> >> On Sep 8, 2011, at 11:24 AM, J. Bruce Fields wrote:
> >> 
> >>> On Thu, Sep 08, 2011 at 01:59:46PM -0400, bfields wrote:
> >>>> On Fri, Sep 02, 2011 at 12:38:13PM -0400, Chuck Lever wrote:
> >>>>> Sometime soon we are going to have easy-to-install user space FedFS
> >>>>> components.  Here are kernel patches needed to make server-side FedFS
> >>>>> support work.  Please consider these for the 3.2 kernel.
> >>>>> 
> >>>>> The third patch introduces a potentially expensive check to see if
> >>>>> a junction has been encountered during a mountpoint lookup.  An object
> >>>>> is a junction iff it has the requisite set of extended attributes.
> >>>>> However, reading an extended attribute is expensive on some file
> >>>>> systems.
> >>>>> 
> >>>>> To mitigate the cost of this check, junctions always have their sticky
> >>>>> bit set.  The expensive extended attribute part of the junction test
> >>>>> is done only if the sticky bit is present.
> >>>>> 
> >>>>> Note that today junctions are directories, but someday symlinks might
> >>>>> also act as junctions (for SMB2 support).  And very few files have the
> >>>>> sticky bit set.  So we avoid doing a directory test here.
> >>>>> 
> >>>>> Also, junctions ostensibly have all zero mode bits to hide their local
> >>>>> contents.  I don't think the kernel needs to be concerned about the
> >>>>> permissions as long as the sticky bit is set.  This allows some
> >>>>> flexibility in how junctions are represented.  However, Jeff thinks
> >>>>> that having nfsd4_is_junction() also consider mode bits would make the
> >>>>> expensive part of this test still less frequent.
> >>>>> 
> >>>>> Any thoughts about this?
> >>>> 
> >>>> Hm, right, a sticky bit set on a directory is a normal thing.  I thought
> >>>> Trond's original idea was to check for the sticky bit and not
> >>>> executable?  Which is a pointless combination so should be very rare.
> >>>> 
> >>>> On a typical system maybe directories with the sticky bit are normally
> >>>> somewhat rare, but that's a question of policy and there could be cases
> >>>> where it's common.
> >>> 
> >>> Except for that and the one gripe about an error return, it looks fine
> >>> to me.
> >> 
> >> Noted.  I can repost these with requested fixes in a day or two.
> >> 
> >>> Remind me where the corresponding userland code is?
> >> 
> >> http://oss.oracle.com/projects/fedfs-utils
> > 
> > I can find gitweb, but not a git url to clone from.
> 
> Try
> 
>   git clone git://oss.oracle.com/git/cel/fedfs-utils.git

Got it, thanks!

(There should be some way to configure your gitweb installation so it
includes that.)

--b.

[PATCH 0/3] NFSD support for FedFS junctions [v3]

[Chuck Lever]

Sometime soon we are going to have easy-to-install user space FedFS
components.  Here are kernel patches needed to make server-side FedFS
support work.  Please consider these for the 3.2 kernel.

This respin addresses recent review comments.

Note that because NFSD previously didn't care about the execute bits
of junctions, neither does fedfs-utils-0.7.0 (released September 3,
2011).  Junctions created by this release and previous releases have
the sticky bit set, but the other mode bits are left untouched during
junction creation.

I will publish fedfs-utils-0.7.1 soon with a commit that makes new
junctions non-executable.

---

Trond Myklebust (3):
      NFSD: Add a cache for fs_locations information
      NFSD: Remove the ex_pathname field from struct svc_export
      NFSD: Cleanup for nfsd4_path()


 fs/nfsd/export.c            |   15 +-----
 fs/nfsd/nfs4xdr.c           |  106 ++++++++++++++++++++++++++++++++-----------
 fs/nfsd/nfsd.h              |    7 +++
 fs/nfsd/vfs.c               |   18 +++++++
 include/linux/nfsd/export.h |    2 -
 5 files changed, 106 insertions(+), 42 deletions(-)

-- 
Chuck Lever

[PATCH 3/3] NFSD: Add a cache for fs_locations information

[Chuck Lever]

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
[ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
[ cel: implement S_ISVTX filter in bfields-normal form ]
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsd.h |    7 +++++++
 fs/nfsd/vfs.c  |   18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7ecfa24..d314812 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
 #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
 	NFSD_WRITEABLE_ATTRS_WORD2
 
+extern int nfsd4_is_junction(struct dentry *dentry);
+#else
+static inline int nfsd4_is_junction(struct dentry *dentry)
+{
+	return 0;
+}
+
 #endif /* CONFIG_NFSD_V4 */
 
 #endif /* LINUX_NFSD_NFSD_H */
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index fd0acca..d1d4d5e 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
 {
 	if (d_mountpoint(dentry))
 		return 1;
+	if (nfsd4_is_junction(dentry))
+		return 1;
 	if (!(exp->ex_flags & NFSEXP_V4ROOT))
 		return 0;
 	return dentry->d_inode != NULL;
@@ -592,6 +594,22 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
 	return error;
 }
 
+#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
+#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
+int nfsd4_is_junction(struct dentry *dentry)
+{
+	struct inode *inode = dentry->d_inode;
+
+	if (inode == NULL)
+		return 0;
+	if (inode->i_mode & S_IXUGO)
+		return 0;
+	if (!(inode->i_mode & S_ISVTX))
+		return 0;
+	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
+		return 0;
+	return 1;
+}
 #endif /* defined(CONFIG_NFSD_V4) */
 
 #ifdef CONFIG_NFSD_V3

Re: [PATCH 3/3] NFSD: Add a cache for fs_locations information

[Jeff Layton]

On Mon, 12 Sep 2011 19:37:26 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
> [ cel: implement S_ISVTX filter in bfields-normal form ]
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfsd/nfsd.h |    7 +++++++
>  fs/nfsd/vfs.c  |   18 ++++++++++++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 7ecfa24..d314812 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
>  #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
>  	NFSD_WRITEABLE_ATTRS_WORD2
>  
> +extern int nfsd4_is_junction(struct dentry *dentry);
> +#else
> +static inline int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
>  #endif /* CONFIG_NFSD_V4 */
>  
>  #endif /* LINUX_NFSD_NFSD_H */
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index fd0acca..d1d4d5e 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
>  {
>  	if (d_mountpoint(dentry))
>  		return 1;
> +	if (nfsd4_is_junction(dentry))
> +		return 1;
>  	if (!(exp->ex_flags & NFSEXP_V4ROOT))
>  		return 0;
>  	return dentry->d_inode != NULL;
> @@ -592,6 +594,22 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
>  	return error;
>  }
>  
> +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
> +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
> +int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	struct inode *inode = dentry->d_inode;
> +
> +	if (inode == NULL)
> +		return 0;
> +	if (inode->i_mode & S_IXUGO)
> +		return 0;
> +	if (!(inode->i_mode & S_ISVTX))
> +		return 0;

	^^^^^^^^^^^^^^^^^^
Minor nit: it's more likely that a directory will have the execute bit
set everywhere than the sticky bit. Checking for the sticky bit first
seems like it might cut out some unnecessary checks for S_IXUGO.

> +	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
> +		return 0;
> +	return 1;
> +}
>  #endif /* defined(CONFIG_NFSD_V4) */
>  
>  #ifdef CONFIG_NFSD_V3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Other than that, this (and the previous patches) look fine...
-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 3/3] NFSD: Add a cache for fs_locations information

[Jeff Layton]

On Tue, 13 Sep 2011 09:16:54 -0400
Jeff Layton <jlayton@redhat.com> wrote:

> On Mon, 12 Sep 2011 19:37:26 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> > From: Trond Myklebust <Trond.Myklebust@netapp.com>
> > 
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
> > [ cel: implement S_ISVTX filter in bfields-normal form ]
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> > 
> >  fs/nfsd/nfsd.h |    7 +++++++
> >  fs/nfsd/vfs.c  |   18 ++++++++++++++++++
> >  2 files changed, 25 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 7ecfa24..d314812 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
> >  #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
> >  	NFSD_WRITEABLE_ATTRS_WORD2
> >  
> > +extern int nfsd4_is_junction(struct dentry *dentry);
> > +#else
> > +static inline int nfsd4_is_junction(struct dentry *dentry)
> > +{
> > +	return 0;
> > +}
> > +
> >  #endif /* CONFIG_NFSD_V4 */
> >  
> >  #endif /* LINUX_NFSD_NFSD_H */
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index fd0acca..d1d4d5e 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
> >  {
> >  	if (d_mountpoint(dentry))
> >  		return 1;
> > +	if (nfsd4_is_junction(dentry))
> > +		return 1;
> >  	if (!(exp->ex_flags & NFSEXP_V4ROOT))
> >  		return 0;
> >  	return dentry->d_inode != NULL;
> > @@ -592,6 +594,22 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
> >  	return error;
> >  }
> >  
> > +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
> > +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
> > +int nfsd4_is_junction(struct dentry *dentry)
> > +{
> > +	struct inode *inode = dentry->d_inode;
> > +
> > +	if (inode == NULL)
> > +		return 0;
> > +	if (inode->i_mode & S_IXUGO)
> > +		return 0;
> > +	if (!(inode->i_mode & S_ISVTX))
> > +		return 0;
> 
> 	^^^^^^^^^^^^^^^^^^
> Minor nit: it's more likely that a directory will have the execute bit
> set everywhere than the sticky bit. Checking for the sticky bit first
> seems like it might cut out some unnecessary checks for S_IXUGO.
> 

Chuck pointed out on IRC that he's checking for an absence of execute
bits in the first check, which is probably more rare than having the
sticky bit set. So my nit above isn't correct...

> > +	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
> > +		return 0;
> > +	return 1;
> > +}
> >  #endif /* defined(CONFIG_NFSD_V4) */
> >  
> >  #ifdef CONFIG_NFSD_V3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Other than that, this (and the previous patches) look fine...

You can add my reviewed by on this patch and the others...

Reviewed-by: Jeff Layton <jlayton@redhat.com>

Re: [PATCH 3/3] NFSD: Add a cache for fs_locations information

[J. Bruce Fields]

On Tue, Sep 13, 2011 at 11:48:30AM -0400, Jeff Layton wrote:
> You can add my reviewed by on this patch and the others...

OK, thanks to both of you, applying for 3.2.--b.

> 
> Reviewed-by: Jeff Layton <jlayton@redhat.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] NFSD: Add a cache for fs_locations information

[J. Bruce Fields]

On Mon, Sep 12, 2011 at 07:37:26PM -0400, Chuck Lever wrote:
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
> [ cel: implement S_ISVTX filter in bfields-normal form ]

Hm.--b.

> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfsd/nfsd.h |    7 +++++++
>  fs/nfsd/vfs.c  |   18 ++++++++++++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 7ecfa24..d314812 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
>  #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
>  	NFSD_WRITEABLE_ATTRS_WORD2
>  
> +extern int nfsd4_is_junction(struct dentry *dentry);
> +#else
> +static inline int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
>  #endif /* CONFIG_NFSD_V4 */
>  
>  #endif /* LINUX_NFSD_NFSD_H */
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index fd0acca..d1d4d5e 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
>  {
>  	if (d_mountpoint(dentry))
>  		return 1;
> +	if (nfsd4_is_junction(dentry))
> +		return 1;
>  	if (!(exp->ex_flags & NFSEXP_V4ROOT))
>  		return 0;
>  	return dentry->d_inode != NULL;
> @@ -592,6 +594,22 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
>  	return error;
>  }
>  
> +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
> +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
> +int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	struct inode *inode = dentry->d_inode;
> +
> +	if (inode == NULL)
> +		return 0;
> +	if (inode->i_mode & S_IXUGO)
> +		return 0;
> +	if (!(inode->i_mode & S_ISVTX))
> +		return 0;
> +	if (vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0) <= 0)
> +		return 0;
> +	return 1;
> +}
>  #endif /* defined(CONFIG_NFSD_V4) */
>  
>  #ifdef CONFIG_NFSD_V3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html