diff mbox series

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

Message ID 20230421182738.901701-1-anna@kernel.org (mailing list archive)
State New, archived
Headers show
Series [RFC] NFS: add a sysfs file for enabling & disabling nfs features | expand

Commit Message

Anna Schumaker April 21, 2023, 6:27 p.m. UTC 
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(+)

Comments

Benjamin Coddington May 4, 2023, 12:56 p.m. UTC | #1 
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
Anna Schumaker May 4, 2023, 8:51 p.m. UTC | #2 
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
>
Benjamin Coddington May 4, 2023, 8:58 p.m. UTC | #3 
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
diff mbox series

Patch

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);