[RFC PATCH] NFS: add a sysfs file for enabling & disabling nfs features

[RFC PATCH] NFS: add a sysfs file for enabling & disabling nfs features

[Anna Schumaker]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

And add some basic checking so we only enable features that are present
in a given NFS version.

Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/sysfs.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/nfs/sysfs.h |  6 +++
 2 files changed, 105 insertions(+)

diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
index 39bfcbcf916c..667c3e544b23 100644
--- a/fs/nfs/sysfs.c
+++ b/fs/nfs/sysfs.c
@@ -216,7 +216,106 @@ static void nfs_sysfs_sb_release(struct kobject *kobj)
 	/* no-op: why? see lib/kobject.c kobject_cleanup() */
 }
 
+static inline char nfs_sysfs_server_capable(struct nfs_server *server,
+					    unsigned int capability)
+{
+	return (server->caps & capability) ? '+' : '-';
+}
+
+static ssize_t nfs_netns_sb_features_show(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  char *buf)
+{
+	struct nfs_server *server = container_of(kobj, struct nfs_server, kobj);
+
+	return sysfs_emit(buf, "%creaddirplus\n"
+				"%csecurity_label\n"
+				"%cseek\n"
+				"%callocate\n"
+				"%cdeallocate\n"
+				"%clayoutstats\n"
+				"%cclone\n"
+				"%ccopy\n"
+				"%coffload_cancel\n"
+				"%clayouterror\n"
+				"%ccopy_notify\n"
+				"%cxattr\n"
+				"%cread_plus\n",
+			nfs_sysfs_server_capable(server, NFS_CAP_READDIRPLUS),
+			nfs_sysfs_server_capable(server, NFS_CAP_SECURITY_LABEL),
+			nfs_sysfs_server_capable(server, NFS_CAP_SEEK),
+			nfs_sysfs_server_capable(server, NFS_CAP_ALLOCATE),
+			nfs_sysfs_server_capable(server, NFS_CAP_DEALLOCATE),
+			nfs_sysfs_server_capable(server, NFS_CAP_LAYOUTSTATS),
+			nfs_sysfs_server_capable(server, NFS_CAP_CLONE),
+			nfs_sysfs_server_capable(server, NFS_CAP_COPY),
+			nfs_sysfs_server_capable(server, NFS_CAP_OFFLOAD_CANCEL),
+			nfs_sysfs_server_capable(server, NFS_CAP_LAYOUTERROR),
+			nfs_sysfs_server_capable(server, NFS_CAP_COPY_NOTIFY),
+			nfs_sysfs_server_capable(server, NFS_CAP_XATTR),
+			nfs_sysfs_server_capable(server, NFS_CAP_READ_PLUS));
+}
+
+static ssize_t nfs_netns_sb_features_store(struct kobject *kobj,
+					   struct kobj_attribute *attr,
+					   const char *buf, size_t count)
+{
+	struct nfs_server *server = container_of(kobj, struct nfs_server, kobj);
+	unsigned int cap;
+
+	if (!strncmp(buf + 1, "readdirplus", count - 2))
+		cap = NFS_CAP_READDIRPLUS;
+	else if (!strncmp(buf + 1, "security_label", count - 2))
+		cap = NFS_CAP_SECURITY_LABEL;
+	else if (!strncmp(buf + 1, "seek", count - 2))
+		cap = NFS_CAP_SEEK;
+	else if (!strncmp(buf + 1, "allocate", count - 2))
+		cap = NFS_CAP_ALLOCATE;
+	else if (!strncmp(buf + 1, "deallocate", count - 2))
+		cap = NFS_CAP_DEALLOCATE;
+	else if (!strncmp(buf + 1, "layoutstats", count - 2))
+		cap = NFS_CAP_LAYOUTSTATS;
+	else if (!strncmp(buf + 1, "clone", count - 2))
+		cap = NFS_CAP_CLONE;
+	else if (!strncmp(buf + 1, "copy", count - 2))
+		cap = NFS_CAP_COPY;
+	else if (!strncmp(buf + 1, "offload_cancel", count - 2))
+		cap = NFS_CAP_OFFLOAD_CANCEL;
+	else if (!strncmp(buf + 1, "layouterror", count - 2))
+		cap = NFS_CAP_LAYOUTERROR;
+	else if (!strncmp(buf + 1, "copy_notify", count - 2))
+		cap = NFS_CAP_COPY_NOTIFY;
+	else if (!strncmp(buf + 1, "xattr", count - 2))
+		cap = NFS_CAP_XATTR;
+	else if (!strncmp(buf + 1, "read_plus", count - 2))
+		cap = NFS_CAP_READ_PLUS;
+	else
+		return -EINVAL;
+
+	switch (cap) {
+	case NFS_CAP_READDIRPLUS:
+		if (server->nfs_client->rpc_ops->version == 2)
+			return -EINVAL;
+		break;
+	default:
+		if (server->nfs_client->rpc_ops->version != 4 ||
+		    server->nfs_client->cl_minorversion < 2)
+			return -EINVAL;
+	}
+
+	if (buf[0] == '+')
+		server->caps |= cap;
+	else
+		server->caps &= ~cap;
+		
+	return count;
+}
+
+static struct kobj_attribute nfs_netns_sb_features = __ATTR(features,
+		0644, nfs_netns_sb_features_show, nfs_netns_sb_features_store);
+
 static struct attribute *nfs_mp_attrs[] = {
+	&nfs_netns_sb_features.attr,
         NULL,
 };
 
diff --git a/fs/nfs/sysfs.h b/fs/nfs/sysfs.h
index 34e40f3a14cb..ed89fc873c68 100644
--- a/fs/nfs/sysfs.h
+++ b/fs/nfs/sysfs.h
@@ -14,6 +14,12 @@ struct nfs_netns_client {
 	const char __rcu *identifier;
 };
 
+struct nfs_netns_server {
+	struct kobject kobject;
+	struct net *net;
+	unsigned int caps;
+};
+
 extern struct kobject *nfs_client_kobj;
 
 extern int nfs_sysfs_init(void);
-- 
2.40.0

Re: [RFC PATCH] NFS: add a sysfs file for enabling & disabling nfs features

[Benjamin Coddington]

On 21 Apr 2023, at 14:27, Anna Schumaker wrote:

> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>
> And add some basic checking so we only enable features that are present
> in a given NFS version.
>
> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> ---

This is great, I like how you've kept the +/- notation similar to knfsd
supported versions.

Another way to do this would be an attribute file per capability, setting it
to 0 or 1 which is more inline with sysfs usage.

I think if we do use this, we ought to leave readdir plus out of it because
there's already a mount option for it.  Readdir plus can be turned on and
off with a remount already.  The issue for me would be how to work out what
the behavior should be when we have a mount that has "nordirplus" and then
someone tries to toggle it via sysfs.

Any other thoughts?

I'll add this patch to my future postings of sysfs work.

Ben

Re: [RFC PATCH] NFS: add a sysfs file for enabling & disabling nfs features

[Anna Schumaker]

On Thu, May 4, 2023 at 8:56 AM Benjamin Coddington <bcodding@redhat.com> wrote:
>
> On 21 Apr 2023, at 14:27, Anna Schumaker wrote:
>
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >
> > And add some basic checking so we only enable features that are present
> > in a given NFS version.
> >
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > ---
>
> This is great, I like how you've kept the +/- notation similar to knfsd
> supported versions.
>
> Another way to do this would be an attribute file per capability, setting it
> to 0 or 1 which is more inline with sysfs usage.
>
> I think if we do use this, we ought to leave readdir plus out of it because
> there's already a mount option for it.  Readdir plus can be turned on and
> off with a remount already.  The issue for me would be how to work out what
> the behavior should be when we have a mount that has "nordirplus" and then
> someone tries to toggle it via sysfs.

That makes sense!
>
> Any other thoughts?

I guess if we remove the readdir plus option, then we could make it so
this file only shows up on v4.2 mounts.
>
> I'll add this patch to my future postings of sysfs work.

Do you want a v2 without the readdir plus line, and with the v4.2 restriction?

Anna

>
> Ben
>

Re: [RFC PATCH] NFS: add a sysfs file for enabling & disabling nfs features

[Benjamin Coddington]

On 4 May 2023, at 16:51, Anna Schumaker wrote:
>
> Do you want a v2 without the readdir plus line, and with the v4.2 restriction?

Sure, that'd be great!

Ben