2023-04-21 18:40:12

by Anna Schumaker

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

From: Anna Schumaker <[email protected]>

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

Signed-off-by: Anna Schumaker <[email protected]>
---
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


2023-05-04 13:11:59

by Benjamin Coddington

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

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

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

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

2023-05-04 21:03:57

by Anna Schumaker

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

On Thu, May 4, 2023 at 8:56 AM Benjamin Coddington <[email protected]> wrote:
>
> On 21 Apr 2023, at 14:27, Anna Schumaker wrote:
>
> > From: Anna Schumaker <[email protected]>
> >
> > And add some basic checking so we only enable features that are present
> > in a given NFS version.
> >
> > Signed-off-by: Anna Schumaker <[email protected]>
> > ---
>
> 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
>

2023-05-04 21:07:10

by Benjamin Coddington

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

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