2024-06-13 18:37:18

by Jeff Layton

[permalink] [raw]
Subject: [PATCH v3 0/5] nfsd/sunrpc: allow starting/stopping pooled NFS server via netlink

This is a resend of the patchset I sent a little over a week ago, with
a couple of new patches that allow setting the pool-mode via netlink.

This patchset first attempts to detangle the pooled/non-pooled service
handling in the sunrpc layer, unifies the codepaths that start the
pooled vs. non-pooled nfsd, and then wires up the new netlink threads
interface to allow you to start a pooled server by specifying an
array of thread counts.

Signed-off-by: Jeff Layton <[email protected]>
---
Changes in v3:
- better announce the subtle change to the /sys/module interface
- add more kerneldoc comments
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- add new pool-mode set/get netlink calls

---
Jeff Layton (5):
sunrpc: fix up the special handling of sv_nrpools == 1
nfsd: make nfsd_svc take an array of thread counts
nfsd: allow passing in array of thread counts via netlink
sunrpc: refactor pool_mode setting code
nfsd: new netlink ops to get/set server pool_mode

Documentation/netlink/specs/nfsd.yaml | 27 +++++++++
fs/nfsd/netlink.c | 17 ++++++
fs/nfsd/netlink.h | 2 +
fs/nfsd/nfsctl.c | 100 ++++++++++++++++++++++++++----
fs/nfsd/nfsd.h | 3 +-
fs/nfsd/nfssvc.c | 59 +++++++++++-------
include/linux/sunrpc/svc.h | 3 +
include/uapi/linux/nfsd_netlink.h | 10 +++
net/sunrpc/svc.c | 111 ++++++++++++++++++++++------------
9 files changed, 256 insertions(+), 76 deletions(-)
---
base-commit: fec4124bac55ad92c47585fe537e646fe108b8fa
change-id: 20240604-nfsd-next-b04c0d2d89a9

Best regards,
--
Jeff Layton <[email protected]>



2024-06-13 18:39:22

by Jeff Layton

[permalink] [raw]
Subject: [PATCH v3 3/5] nfsd: allow passing in array of thread counts via netlink

Now that nfsd_svc can handle an array of thread counts, fix up the
netlink threads interface to construct one from the netlink call
and pass it through so we can start a pooled server the same way we
would start a normal one.

Note that any unspecified values in the array are considered zeroes,
so it's possible to shut down a pooled server by passing in a short
array that has only zeros, or even an empty array.

Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfsctl.c | 32 ++++++++++++++++++++------------
fs/nfsd/nfssvc.c | 12 +++++++++++-
2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index c0d84a6e5416..dde42aad5582 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1671,7 +1671,7 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
*/
int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
{
- int nthreads = 0, count = 0, nrpools, ret = -EOPNOTSUPP, rem;
+ int *nthreads, count = 0, nrpools, i, ret = -EOPNOTSUPP, rem;
struct net *net = genl_info_net(info);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
const struct nlattr *attr;
@@ -1688,15 +1688,22 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info)

mutex_lock(&nfsd_mutex);

- nrpools = nfsd_nrpools(net);
- if (nrpools && count > nrpools)
- count = nrpools;
-
- /* XXX: make this handle non-global pool-modes */
- if (count > 1)
+ nrpools = max(count, nfsd_nrpools(net));
+ nthreads = kcalloc(nrpools, sizeof(int), GFP_KERNEL);
+ if (!nthreads) {
+ ret = -ENOMEM;
goto out_unlock;
+ }
+
+ i = 0;
+ nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+ if (nla_type(attr) == NFSD_A_SERVER_THREADS) {
+ nthreads[i++] = nla_get_u32(attr);
+ if (i >= nrpools)
+ break;
+ }
+ }

- nthreads = nla_get_u32(info->attrs[NFSD_A_SERVER_THREADS]);
if (info->attrs[NFSD_A_SERVER_GRACETIME] ||
info->attrs[NFSD_A_SERVER_LEASETIME] ||
info->attrs[NFSD_A_SERVER_SCOPE]) {
@@ -1730,12 +1737,13 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
scope = nla_data(attr);
}

- ret = nfsd_svc(1, &nthreads, net, get_current_cred(), scope);
-
+ ret = nfsd_svc(nrpools, nthreads, net, get_current_cred(), scope);
+ if (ret > 0)
+ ret = 0;
out_unlock:
mutex_unlock(&nfsd_mutex);
-
- return ret == nthreads ? 0 : ret;
+ kfree(nthreads);
+ return ret;
}

/**
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 2e47fd26c9b4..9edb4f7c4cc2 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -763,8 +763,18 @@ int nfsd_set_nrthreads(int n, int *nthreads, struct net *net)
&nn->nfsd_serv->sv_pools[i],
nthreads[i]);
if (err)
- break;
+ goto out;
}
+
+ /* Anything undefined in array is considered to be 0 */
+ for (i = n; i < nn->nfsd_serv->sv_nrpools; ++i) {
+ err = svc_set_num_threads(nn->nfsd_serv,
+ &nn->nfsd_serv->sv_pools[i],
+ 0);
+ if (err)
+ goto out;
+ }
+out:
return err;
}


--
2.45.2


2024-06-13 18:40:11

by Jeff Layton

[permalink] [raw]
Subject: [PATCH v3 5/5] nfsd: new netlink ops to get/set server pool_mode

Signed-off-by: Jeff Layton <[email protected]>
---
Documentation/netlink/specs/nfsd.yaml | 27 ++++++++++++++++
fs/nfsd/netlink.c | 17 ++++++++++
fs/nfsd/netlink.h | 2 ++
fs/nfsd/nfsctl.c | 58 +++++++++++++++++++++++++++++++++++
include/uapi/linux/nfsd_netlink.h | 10 ++++++
5 files changed, 114 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index d21234097167..5a98e5a06c68 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -115,6 +115,15 @@ attribute-sets:
type: nest
nested-attributes: sock
multi-attr: true
+ -
+ name: pool-mode
+ attributes:
+ -
+ name: mode
+ type: string
+ -
+ name: npools
+ type: u32

operations:
list:
@@ -197,3 +206,21 @@ operations:
reply:
attributes:
- addr
+ -
+ name: pool-mode-set
+ doc: set the current server pool-mode
+ attribute-set: pool-mode
+ flags: [ admin-perm ]
+ do:
+ request:
+ attributes:
+ - mode
+ -
+ name: pool-mode-get
+ doc: get info about server pool-mode
+ attribute-set: pool-mode
+ do:
+ reply:
+ attributes:
+ - mode
+ - npools
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 62d2586d9902..137701153c9e 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -40,6 +40,11 @@ static const struct nla_policy nfsd_listener_set_nl_policy[NFSD_A_SERVER_SOCK_AD
[NFSD_A_SERVER_SOCK_ADDR] = NLA_POLICY_NESTED(nfsd_sock_nl_policy),
};

+/* NFSD_CMD_POOL_MODE_SET - do */
+static const struct nla_policy nfsd_pool_mode_set_nl_policy[NFSD_A_POOL_MODE_MODE + 1] = {
+ [NFSD_A_POOL_MODE_MODE] = { .type = NLA_NUL_STRING, },
+};
+
/* Ops table for nfsd */
static const struct genl_split_ops nfsd_nl_ops[] = {
{
@@ -85,6 +90,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
.doit = nfsd_nl_listener_get_doit,
.flags = GENL_CMD_CAP_DO,
},
+ {
+ .cmd = NFSD_CMD_POOL_MODE_SET,
+ .doit = nfsd_nl_pool_mode_set_doit,
+ .policy = nfsd_pool_mode_set_nl_policy,
+ .maxattr = NFSD_A_POOL_MODE_MODE,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NFSD_CMD_POOL_MODE_GET,
+ .doit = nfsd_nl_pool_mode_get_doit,
+ .flags = GENL_CMD_CAP_DO,
+ },
};

struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index e3724637d64d..9459547de04e 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -26,6 +26,8 @@ int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info);
int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_pool_mode_set_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info);

extern struct genl_family nfsd_nl_family;

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index dde42aad5582..187e9be77b78 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -2182,6 +2182,64 @@ int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
return err;
}

+/**
+ * nfsd_nl_pool_mode_set_doit - set the number of running threads
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_pool_mode_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ const struct nlattr *attr;
+
+ if (GENL_REQ_ATTR_CHECK(info, NFSD_A_POOL_MODE_MODE))
+ return -EINVAL;
+
+ attr = info->attrs[NFSD_A_POOL_MODE_MODE];
+ return sunrpc_set_pool_mode(nla_data(attr));
+}
+
+/**
+ * nfsd_nl_pool_mode_get_doit - get info about pool_mode
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct net *net = genl_info_net(info);
+ char buf[16];
+ void *hdr;
+ int err;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ err = -EMSGSIZE;
+ hdr = genlmsg_iput(skb, info);
+ if (!hdr)
+ goto err_free_msg;
+
+ err = -ERANGE;
+ if (sunrpc_get_pool_mode(buf, ARRAY_SIZE(buf)) >= ARRAY_SIZE(buf))
+ goto err_free_msg;
+
+ err = nla_put_string(skb, NFSD_A_POOL_MODE_MODE, buf) ||
+ nla_put_u32(skb, NFSD_A_POOL_MODE_NPOOLS, nfsd_nrpools(net));
+ if (err)
+ goto err_free_msg;
+
+ genlmsg_end(skb, hdr);
+ return genlmsg_reply(skb, info);
+
+err_free_msg:
+ nlmsg_free(skb);
+ return err;
+}
+
/**
* nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
* @net: a freshly-created network namespace
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index 24c86dbc7ed5..887cbd12b695 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -70,6 +70,14 @@ enum {
NFSD_A_SERVER_SOCK_MAX = (__NFSD_A_SERVER_SOCK_MAX - 1)
};

+enum {
+ NFSD_A_POOL_MODE_MODE = 1,
+ NFSD_A_POOL_MODE_NPOOLS,
+
+ __NFSD_A_POOL_MODE_MAX,
+ NFSD_A_POOL_MODE_MAX = (__NFSD_A_POOL_MODE_MAX - 1)
+};
+
enum {
NFSD_CMD_RPC_STATUS_GET = 1,
NFSD_CMD_THREADS_SET,
@@ -78,6 +86,8 @@ enum {
NFSD_CMD_VERSION_GET,
NFSD_CMD_LISTENER_SET,
NFSD_CMD_LISTENER_GET,
+ NFSD_CMD_POOL_MODE_SET,
+ NFSD_CMD_POOL_MODE_GET,

__NFSD_CMD_MAX,
NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)

--
2.45.2


2024-06-13 23:56:31

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] nfsd: new netlink ops to get/set server pool_mode

On Thu, 13 Jun 2024 14:34:34 -0400 Jeff Layton wrote:
> + err = nla_put_string(skb, NFSD_A_POOL_MODE_MODE, buf) ||
> + nla_put_u32(skb, NFSD_A_POOL_MODE_NPOOLS, nfsd_nrpools(net));

bitwise or?

Other option would be to move sunrpc_get_pool_mode() before allocation
that way all error codes past allocations are EMSGSIZE and life is
simpler.

2024-06-14 00:18:18

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] nfsd: new netlink ops to get/set server pool_mode

On Thu, 2024-06-13 at 16:56 -0700, Jakub Kicinski wrote:
> On Thu, 13 Jun 2024 14:34:34 -0400 Jeff Layton wrote:
> > + err = nla_put_string(skb, NFSD_A_POOL_MODE_MODE, buf) ||
> > + nla_put_u32(skb, NFSD_A_POOL_MODE_NPOOLS, nfsd_nrpools(net));
>
> bitwise or?
>

Yes. Good catch.

> Other option would be to move sunrpc_get_pool_mode() before allocation
> that way all error codes past allocations are EMSGSIZE and life is
> simpler.

Thanks, I'll probably do the latter.
--
Jeff Layton <[email protected]>