2024-04-11 18:02:53

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH v7 0/5] convert write_threads, write_version and write_ports to netlink commands

Introduce write_threads, write_version and write_ports netlink
commands similar to the ones available through the procfs.

Changes since v6:
- add the capability to pass sockaddr from userspace through listener-set
command
- rebase on top of nfsd-next
Changes since v5:
- for write_ports and write_version commands, userspace is expected to provide
a NFS listeners/supported versions list it want to enable (all the other
ports/versions will be disabled).
- fix comments
- rebase on top of nfsd-next
Changes since v4:
- rebase on top of nfsd-next tree
Changes since v3:
- drop write_maxconn and write_maxblksize for the moment
- add write_version and write_ports commands
Changes since v2:
- use u32 to store nthreads in nfsd_nl_threads_set_doit
- rename server-attr in control-plane in nfsd.yaml specs
Changes since v1:
- remove write_v4_end_grace command
- add write_maxblksize and write_maxconn netlink commands

This patch can be tested with user-space tool reported below:
https://github.com/LorenzoBianconi/nfsdctl

Jeff Layton (1):
SUNRPC: add a new svc_find_listener helper

Lorenzo Bianconi (4):
NFSD: convert write_threads to netlink command
NFSD: add write_version to netlink command
SUNRPC: introduce svc_xprt_create_from_sa utility routine
NFSD: add listener-{set,get} netlink command

Documentation/netlink/specs/nfsd.yaml | 94 ++++++
fs/nfsd/netlink.c | 63 ++++
fs/nfsd/netlink.h | 10 +
fs/nfsd/netns.h | 1 +
fs/nfsd/nfsctl.c | 433 ++++++++++++++++++++++++++
fs/nfsd/nfssvc.c | 3 +-
include/linux/sunrpc/svc_xprt.h | 5 +
include/uapi/linux/nfsd_netlink.h | 44 +++
net/sunrpc/svc_xprt.c | 167 ++++++----
9 files changed, 760 insertions(+), 60 deletions(-)

--
2.44.0



2024-04-11 18:03:09

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH v7 2/5] NFSD: add write_version to netlink command

Introduce write_version netlink command through a "declarative" interface.
This patch introduces a change in behavior since for version-set userspace
is expected to provide a NFS major/minor version list it wants to enable
while all the other ones will be disabled. (procfs write_version
command implements imperative interface where the admin writes +3/-3 to
enable/disable a single version.

Tested-by: Jeff Layton <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Documentation/netlink/specs/nfsd.yaml | 37 +++++++
fs/nfsd/netlink.c | 24 ++++
fs/nfsd/netlink.h | 5 +
fs/nfsd/netns.h | 1 +
fs/nfsd/nfsctl.c | 153 ++++++++++++++++++++++++++
fs/nfsd/nfssvc.c | 3 +-
include/uapi/linux/nfsd_netlink.h | 18 +++
7 files changed, 239 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index c92e1425d316..cb93e3e37119 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -68,6 +68,26 @@ attribute-sets:
-
name: threads
type: u32
+ -
+ name: version
+ attributes:
+ -
+ name: major
+ type: u32
+ -
+ name: minor
+ type: u32
+ -
+ name: enabled
+ type: flag
+ -
+ name: server-proto
+ attributes:
+ -
+ name: version
+ type: nest
+ nested-attributes: version
+ multi-attr: true

operations:
list:
@@ -110,3 +130,20 @@ operations:
reply:
attributes:
- threads
+ -
+ name: version-set
+ doc: set nfs enabled versions
+ attribute-set: server-proto
+ flags: [ admin-perm ]
+ do:
+ request:
+ attributes:
+ - version
+ -
+ name: version-get
+ doc: get nfs enabled versions
+ attribute-set: server-proto
+ do:
+ reply:
+ attributes:
+ - version
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 1a59a8e6c7e2..75f609b57ceb 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -10,11 +10,23 @@

#include <uapi/linux/nfsd_netlink.h>

+/* Common nested types */
+const struct nla_policy nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1] = {
+ [NFSD_A_VERSION_MAJOR] = { .type = NLA_U32, },
+ [NFSD_A_VERSION_MINOR] = { .type = NLA_U32, },
+ [NFSD_A_VERSION_ENABLED] = { .type = NLA_FLAG, },
+};
+
/* NFSD_CMD_THREADS_SET - do */
static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
[NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
};

+/* NFSD_CMD_VERSION_SET - do */
+static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VERSION + 1] = {
+ [NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_version_nl_policy),
+};
+
/* Ops table for nfsd */
static const struct genl_split_ops nfsd_nl_ops[] = {
{
@@ -36,6 +48,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
.doit = nfsd_nl_threads_get_doit,
.flags = GENL_CMD_CAP_DO,
},
+ {
+ .cmd = NFSD_CMD_VERSION_SET,
+ .doit = nfsd_nl_version_set_doit,
+ .policy = nfsd_version_set_nl_policy,
+ .maxattr = NFSD_A_SERVER_PROTO_VERSION,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NFSD_CMD_VERSION_GET,
+ .doit = nfsd_nl_version_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 4137fac477e4..c7c0da275481 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -11,6 +11,9 @@

#include <uapi/linux/nfsd_netlink.h>

+/* Common nested types */
+extern const struct nla_policy nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1];
+
int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);

@@ -18,6 +21,8 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
struct netlink_callback *cb);
int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
+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);

extern struct genl_family nfsd_nl_family;

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index d4be519b5734..14ec15656320 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -218,6 +218,7 @@ struct nfsd_net {
/* Simple check to find out if a given net was properly initialized */
#define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl)

+extern bool nfsd_support_version(int vers);
extern void nfsd_netns_free_versions(struct nfsd_net *nn);

extern unsigned int nfsd_net_id;
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 71608744e67f..341efab4eaa7 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1711,6 +1711,159 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
return err;
}

+/**
+ * nfsd_nl_version_set_doit - set the nfs enabled versions
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ const struct nlattr *attr;
+ struct nfsd_net *nn;
+ int i, rem;
+
+ if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_PROTO_VERSION))
+ return -EINVAL;
+
+ mutex_lock(&nfsd_mutex);
+
+ nn = net_generic(genl_info_net(info), nfsd_net_id);
+ if (nn->nfsd_serv) {
+ mutex_unlock(&nfsd_mutex);
+ return -EBUSY;
+ }
+
+ /* clear current supported versions. */
+ nfsd_vers(nn, 2, NFSD_CLEAR);
+ nfsd_vers(nn, 3, NFSD_CLEAR);
+ for (i = 0; i <= NFSD_SUPPORTED_MINOR_VERSION; i++)
+ nfsd_minorversion(nn, i, NFSD_CLEAR);
+
+ nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+ struct nlattr *tb[NFSD_A_VERSION_MAX + 1];
+ u32 major, minor = 0;
+ bool enabled;
+
+ if (nla_type(attr) != NFSD_A_SERVER_PROTO_VERSION)
+ continue;
+
+ if (nla_parse_nested(tb, NFSD_A_VERSION_MAX, attr,
+ nfsd_version_nl_policy, info->extack) < 0)
+ continue;
+
+ if (!tb[NFSD_A_VERSION_MAJOR])
+ continue;
+
+ major = nla_get_u32(tb[NFSD_A_VERSION_MAJOR]);
+ if (tb[NFSD_A_VERSION_MINOR])
+ minor = nla_get_u32(tb[NFSD_A_VERSION_MINOR]);
+
+ enabled = nla_get_flag(tb[NFSD_A_VERSION_ENABLED]);
+
+ switch (major) {
+ case 4:
+ nfsd_minorversion(nn, minor, enabled ? NFSD_SET : NFSD_CLEAR);
+ break;
+ case 3:
+ case 2:
+ if (!minor)
+ nfsd_vers(nn, major, enabled ? NFSD_SET : NFSD_CLEAR);
+ break;
+ default:
+ break;
+ }
+ }
+
+ mutex_unlock(&nfsd_mutex);
+
+ return 0;
+}
+
+/**
+ * nfsd_nl_version_get_doit - get the nfs enabled versions
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nfsd_net *nn;
+ int i, err;
+ void *hdr;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_iput(skb, info);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto err_free_msg;
+ }
+
+ mutex_lock(&nfsd_mutex);
+ nn = net_generic(genl_info_net(info), nfsd_net_id);
+
+ for (i = 2; i <= 4; i++) {
+ int j;
+
+ for (j = 0; j <= NFSD_SUPPORTED_MINOR_VERSION; j++) {
+ struct nlattr *attr;
+
+ /* Don't record any versions the kernel doesn't have
+ * compiled in
+ */
+ if (!nfsd_support_version(i))
+ continue;
+
+ /* NFSv{2,3} does not support minor numbers */
+ if (i < 4 && j)
+ continue;
+
+ if (i == 4 && !nfsd_minorversion(nn, j, NFSD_TEST))
+ continue;
+
+ attr = nla_nest_start_noflag(skb,
+ NFSD_A_SERVER_PROTO_VERSION);
+ if (!attr) {
+ err = -EINVAL;
+ goto err_nfsd_unlock;
+ }
+
+ if (nla_put_u32(skb, NFSD_A_VERSION_MAJOR, i) ||
+ nla_put_u32(skb, NFSD_A_VERSION_MINOR, j)) {
+ err = -EINVAL;
+ goto err_nfsd_unlock;
+ }
+
+ /* Set the enabled flag if the version is enabled */
+ if (nfsd_vers(nn, i, NFSD_TEST) &&
+ (i < 4 || nfsd_minorversion(nn, j, NFSD_TEST)) &&
+ nla_put_flag(skb, NFSD_A_VERSION_ENABLED)) {
+ err = -EINVAL;
+ goto err_nfsd_unlock;
+ }
+
+ nla_nest_end(skb, attr);
+ }
+ }
+
+ mutex_unlock(&nfsd_mutex);
+ genlmsg_end(skb, hdr);
+
+ return genlmsg_reply(skb, info);
+
+err_nfsd_unlock:
+ mutex_unlock(&nfsd_mutex);
+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/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index c0d17b92b249..4452a9bb9bbb 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -133,8 +133,7 @@ struct svc_program nfsd_program = {
.pg_rpcbind_set = nfsd_rpcbind_set,
};

-static bool
-nfsd_support_version(int vers)
+bool nfsd_support_version(int vers)
{
if (vers >= NFSD_MINVERS && vers < NFSD_NRVERS)
return nfsd_version[vers] != NULL;
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index 1b6fe1f9ed0e..ccf3e15fe160 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -36,10 +36,28 @@ enum {
NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
};

+enum {
+ NFSD_A_VERSION_MAJOR = 1,
+ NFSD_A_VERSION_MINOR,
+ NFSD_A_VERSION_ENABLED,
+
+ __NFSD_A_VERSION_MAX,
+ NFSD_A_VERSION_MAX = (__NFSD_A_VERSION_MAX - 1)
+};
+
+enum {
+ NFSD_A_SERVER_PROTO_VERSION = 1,
+
+ __NFSD_A_SERVER_PROTO_MAX,
+ NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
+};
+
enum {
NFSD_CMD_RPC_STATUS_GET = 1,
NFSD_CMD_THREADS_SET,
NFSD_CMD_THREADS_GET,
+ NFSD_CMD_VERSION_SET,
+ NFSD_CMD_VERSION_GET,

__NFSD_CMD_MAX,
NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
--
2.44.0


2024-04-11 18:03:17

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH v7 3/5] SUNRPC: introduce svc_xprt_create_from_sa utility routine

Add svc_xprt_create_from_sa utility routine and refactor
svc_xprt_create() codebase in order to introduce the capability to
create a svc port from socket address.

Tested-by: Jeff Layton <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/linux/sunrpc/svc_xprt.h | 3 +
net/sunrpc/svc_xprt.c | 133 ++++++++++++++++++--------------
2 files changed, 78 insertions(+), 58 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 8e20cd60e2e7..0d9b10dbe07d 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -135,6 +135,9 @@ int svc_reg_xprt_class(struct svc_xprt_class *);
void svc_unreg_xprt_class(struct svc_xprt_class *);
void svc_xprt_init(struct net *, struct svc_xprt_class *, struct svc_xprt *,
struct svc_serv *);
+int svc_xprt_create_from_sa(struct svc_serv *serv, const char *xprt_name,
+ struct net *net, struct sockaddr *sap,
+ int flags, const struct cred *cred);
int svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
struct net *net, const int family,
const unsigned short port, int flags,
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index b4a85a227bd7..463fe544ae28 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -211,51 +211,6 @@ void svc_xprt_init(struct net *net, struct svc_xprt_class *xcl,
}
EXPORT_SYMBOL_GPL(svc_xprt_init);

-static struct svc_xprt *__svc_xpo_create(struct svc_xprt_class *xcl,
- struct svc_serv *serv,
- struct net *net,
- const int family,
- const unsigned short port,
- int flags)
-{
- struct sockaddr_in sin = {
- .sin_family = AF_INET,
- .sin_addr.s_addr = htonl(INADDR_ANY),
- .sin_port = htons(port),
- };
-#if IS_ENABLED(CONFIG_IPV6)
- struct sockaddr_in6 sin6 = {
- .sin6_family = AF_INET6,
- .sin6_addr = IN6ADDR_ANY_INIT,
- .sin6_port = htons(port),
- };
-#endif
- struct svc_xprt *xprt;
- struct sockaddr *sap;
- size_t len;
-
- switch (family) {
- case PF_INET:
- sap = (struct sockaddr *)&sin;
- len = sizeof(sin);
- break;
-#if IS_ENABLED(CONFIG_IPV6)
- case PF_INET6:
- sap = (struct sockaddr *)&sin6;
- len = sizeof(sin6);
- break;
-#endif
- default:
- return ERR_PTR(-EAFNOSUPPORT);
- }
-
- xprt = xcl->xcl_ops->xpo_create(serv, net, sap, len, flags);
- if (IS_ERR(xprt))
- trace_svc_xprt_create_err(serv->sv_program->pg_name,
- xcl->xcl_name, sap, len, xprt);
- return xprt;
-}
-
/**
* svc_xprt_received - start next receiver thread
* @xprt: controlling transport
@@ -294,9 +249,8 @@ void svc_add_new_perm_xprt(struct svc_serv *serv, struct svc_xprt *new)
}

static int _svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
- struct net *net, const int family,
- const unsigned short port, int flags,
- const struct cred *cred)
+ struct net *net, struct sockaddr *sap,
+ size_t len, int flags, const struct cred *cred)
{
struct svc_xprt_class *xcl;

@@ -312,8 +266,11 @@ static int _svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
goto err;

spin_unlock(&svc_xprt_class_lock);
- newxprt = __svc_xpo_create(xcl, serv, net, family, port, flags);
+ newxprt = xcl->xcl_ops->xpo_create(serv, net, sap, len, flags);
if (IS_ERR(newxprt)) {
+ trace_svc_xprt_create_err(serv->sv_program->pg_name,
+ xcl->xcl_name, sap, len,
+ newxprt);
module_put(xcl->xcl_owner);
return PTR_ERR(newxprt);
}
@@ -329,6 +286,48 @@ static int _svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
return -EPROTONOSUPPORT;
}

+/**
+ * svc_xprt_create_from_sa - Add a new listener to @serv from socket address
+ * @serv: target RPC service
+ * @xprt_name: transport class name
+ * @net: network namespace
+ * @sap: socket address pointer
+ * @flags: SVC_SOCK flags
+ * @cred: credential to bind to this transport
+ *
+ * Return local xprt port on success or %-EPROTONOSUPPORT on failure
+ */
+int svc_xprt_create_from_sa(struct svc_serv *serv, const char *xprt_name,
+ struct net *net, struct sockaddr *sap,
+ int flags, const struct cred *cred)
+{
+ size_t len;
+ int err;
+
+ switch (sap->sa_family) {
+ case AF_INET:
+ len = sizeof(struct sockaddr_in);
+ break;
+#if IS_ENABLED(CONFIG_IPV6)
+ case AF_INET6:
+ len = sizeof(struct sockaddr_in6);
+ break;
+#endif
+ default:
+ return -EAFNOSUPPORT;
+ }
+
+ err = _svc_xprt_create(serv, xprt_name, net, sap, len, flags, cred);
+ if (err == -EPROTONOSUPPORT) {
+ request_module("svc%s", xprt_name);
+ err = _svc_xprt_create(serv, xprt_name, net, sap, len, flags,
+ cred);
+ }
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(svc_xprt_create_from_sa);
+
/**
* svc_xprt_create - Add a new listener to @serv
* @serv: target RPC service
@@ -339,23 +338,41 @@ static int _svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
* @flags: SVC_SOCK flags
* @cred: credential to bind to this transport
*
- * Return values:
- * %0: New listener added successfully
- * %-EPROTONOSUPPORT: Requested transport type not supported
+ * Return local xprt port on success or %-EPROTONOSUPPORT on failure
*/
int svc_xprt_create(struct svc_serv *serv, const char *xprt_name,
struct net *net, const int family,
const unsigned short port, int flags,
const struct cred *cred)
{
- int err;
+ struct sockaddr_in sin = {
+ .sin_family = AF_INET,
+ .sin_addr.s_addr = htonl(INADDR_ANY),
+ .sin_port = htons(port),
+ };
+#if IS_ENABLED(CONFIG_IPV6)
+ struct sockaddr_in6 sin6 = {
+ .sin6_family = AF_INET6,
+ .sin6_addr = IN6ADDR_ANY_INIT,
+ .sin6_port = htons(port),
+ };
+#endif
+ struct sockaddr *sap;

- err = _svc_xprt_create(serv, xprt_name, net, family, port, flags, cred);
- if (err == -EPROTONOSUPPORT) {
- request_module("svc%s", xprt_name);
- err = _svc_xprt_create(serv, xprt_name, net, family, port, flags, cred);
+ switch (family) {
+ case PF_INET:
+ sap = (struct sockaddr *)&sin;
+ break;
+#if IS_ENABLED(CONFIG_IPV6)
+ case PF_INET6:
+ sap = (struct sockaddr *)&sin6;
+ break;
+#endif
+ default:
+ return -EAFNOSUPPORT;
}
- return err;
+
+ return svc_xprt_create_from_sa(serv, xprt_name, net, sap, flags, cred);
}
EXPORT_SYMBOL_GPL(svc_xprt_create);

--
2.44.0


2024-04-11 18:03:25

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH v7 4/5] SUNRPC: add a new svc_find_listener helper

From: Jeff Layton <[email protected]>

svc_find_listener will return the transport instance pointer for the
endpoint accepting connections/peer traffic from the specified transport
class, and matching sockaddr.

Signed-off-by: Jeff Layton <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
include/linux/sunrpc/svc_xprt.h | 2 ++
net/sunrpc/svc_xprt.c | 34 +++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 0d9b10dbe07d..0981e35a9fed 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -150,6 +150,8 @@ void svc_xprt_copy_addrs(struct svc_rqst *rqstp, struct svc_xprt *xprt);
void svc_xprt_close(struct svc_xprt *xprt);
int svc_port_is_privileged(struct sockaddr *sin);
int svc_print_xprts(char *buf, int maxlen);
+struct svc_xprt *svc_find_listener(struct svc_serv *serv, const char *xcl_name,
+ struct net *net, const struct sockaddr *sa);
struct svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
struct net *net, const sa_family_t af,
const unsigned short port);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 463fe544ae28..34a3626c56b1 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -1276,6 +1276,40 @@ static struct svc_deferred_req *svc_deferred_dequeue(struct svc_xprt *xprt)
return dr;
}

+/**
+ * svc_find_listener - find an RPC transport instance
+ * @serv: pointer to svc_serv to search
+ * @xcl_name: C string containing transport's class name
+ * @net: owner net pointer
+ * @sa: sockaddr containing address
+ *
+ * Return the transport instance pointer for the endpoint accepting
+ * connections/peer traffic from the specified transport class,
+ * and matching sockaddr.
+ */
+struct svc_xprt *svc_find_listener(struct svc_serv *serv, const char *xcl_name,
+ struct net *net, const struct sockaddr *sa)
+{
+ struct svc_xprt *xprt;
+ struct svc_xprt *found = NULL;
+
+ spin_lock_bh(&serv->sv_lock);
+ list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
+ if (xprt->xpt_net != net)
+ continue;
+ if (strcmp(xprt->xpt_class->xcl_name, xcl_name))
+ continue;
+ if (!rpc_cmp_addr_port(sa, (struct sockaddr *)&xprt->xpt_local))
+ continue;
+ found = xprt;
+ svc_xprt_get(xprt);
+ break;
+ }
+ spin_unlock_bh(&serv->sv_lock);
+ return found;
+}
+EXPORT_SYMBOL_GPL(svc_find_listener);
+
/**
* svc_find_xprt - find an RPC transport instance
* @serv: pointer to svc_serv to search
--
2.44.0


2024-04-11 18:03:32

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH v7 5/5] NFSD: add listener-{set,get} netlink command

Introduce write_ports netlink command. For listener-set, userspace is
expected to provide a NFS listeners list it wants enabled. All other
sockets will be closed.

Co-developed-by: Jeff Layton <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Documentation/netlink/specs/nfsd.yaml | 34 ++++
fs/nfsd/netlink.c | 22 +++
fs/nfsd/netlink.h | 3 +
fs/nfsd/nfsctl.c | 220 ++++++++++++++++++++++++++
include/uapi/linux/nfsd_netlink.h | 17 ++
5 files changed, 296 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index cb93e3e37119..5b8645abb007 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -88,6 +88,23 @@ attribute-sets:
type: nest
nested-attributes: version
multi-attr: true
+ -
+ name: sock
+ attributes:
+ -
+ name: addr
+ type: binary
+ -
+ name: transport-name
+ type: string
+ -
+ name: server-sock
+ attributes:
+ -
+ name: addr
+ type: nest
+ nested-attributes: sock
+ multi-attr: true

operations:
list:
@@ -147,3 +164,20 @@ operations:
reply:
attributes:
- version
+ -
+ name: listener-set
+ doc: set nfs running sockets
+ attribute-set: server-sock
+ flags: [ admin-perm ]
+ do:
+ request:
+ attributes:
+ - addr
+ -
+ name: listener-get
+ doc: get nfs running listeners
+ attribute-set: server-sock
+ do:
+ reply:
+ attributes:
+ - addr
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 75f609b57ceb..51863043552e 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -11,6 +11,11 @@
#include <uapi/linux/nfsd_netlink.h>

/* Common nested types */
+const struct nla_policy nfsd_sock_nl_policy[NFSD_A_SOCK_TRANSPORT_NAME + 1] = {
+ [NFSD_A_SOCK_ADDR] = { .type = NLA_BINARY, },
+ [NFSD_A_SOCK_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
+};
+
const struct nla_policy nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1] = {
[NFSD_A_VERSION_MAJOR] = { .type = NLA_U32, },
[NFSD_A_VERSION_MINOR] = { .type = NLA_U32, },
@@ -27,6 +32,11 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VE
[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_version_nl_policy),
};

+/* NFSD_CMD_LISTENER_SET - do */
+static const struct nla_policy nfsd_listener_set_nl_policy[NFSD_A_SERVER_SOCK_ADDR + 1] = {
+ [NFSD_A_SERVER_SOCK_ADDR] = NLA_POLICY_NESTED(nfsd_sock_nl_policy),
+};
+
/* Ops table for nfsd */
static const struct genl_split_ops nfsd_nl_ops[] = {
{
@@ -60,6 +70,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
.doit = nfsd_nl_version_get_doit,
.flags = GENL_CMD_CAP_DO,
},
+ {
+ .cmd = NFSD_CMD_LISTENER_SET,
+ .doit = nfsd_nl_listener_set_doit,
+ .policy = nfsd_listener_set_nl_policy,
+ .maxattr = NFSD_A_SERVER_SOCK_ADDR,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NFSD_CMD_LISTENER_GET,
+ .doit = nfsd_nl_listener_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 c7c0da275481..e3724637d64d 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -12,6 +12,7 @@
#include <uapi/linux/nfsd_netlink.h>

/* Common nested types */
+extern const struct nla_policy nfsd_sock_nl_policy[NFSD_A_SOCK_TRANSPORT_NAME + 1];
extern const struct nla_policy nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1];

int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
@@ -23,6 +24,8 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
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);

extern struct genl_family nfsd_nl_family;

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 341efab4eaa7..5ccdfe9a10a5 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1864,6 +1864,226 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
return err;
}

+/**
+ * nfsd_nl_listener_set_doit - set the nfs running sockets
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct net *net = genl_info_net(info);
+ struct svc_xprt *xprt, *tmp;
+ const struct nlattr *attr;
+ struct svc_serv *serv;
+ LIST_HEAD(permsocks);
+ struct nfsd_net *nn;
+ int err, rem;
+
+ mutex_lock(&nfsd_mutex);
+
+ err = nfsd_create_serv(net);
+ if (err) {
+ mutex_unlock(&nfsd_mutex);
+ return err;
+ }
+
+ nn = net_generic(net, nfsd_net_id);
+ serv = nn->nfsd_serv;
+
+ spin_lock_bh(&serv->sv_lock);
+
+ /* Move all of the old listener sockets to a temp list */
+ list_splice_init(&serv->sv_permsocks, &permsocks);
+
+ /*
+ * Walk the list of server_socks from userland and move any that match
+ * back to sv_permsocks
+ */
+ nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+ struct nlattr *tb[NFSD_A_SOCK_MAX + 1];
+ const char *xcl_name;
+ struct sockaddr *sa;
+
+ if (nla_type(attr) != NFSD_A_SERVER_SOCK_ADDR)
+ continue;
+
+ if (nla_parse_nested(tb, NFSD_A_SOCK_MAX, attr,
+ nfsd_sock_nl_policy, info->extack) < 0)
+ continue;
+
+ if (!tb[NFSD_A_SOCK_ADDR] || !tb[NFSD_A_SOCK_TRANSPORT_NAME])
+ continue;
+
+ if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(*sa))
+ continue;
+
+ xcl_name = nla_data(tb[NFSD_A_SOCK_TRANSPORT_NAME]);
+ sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
+
+ /* Put back any matching sockets */
+ list_for_each_entry_safe(xprt, tmp, &permsocks, xpt_list) {
+ /* This shouldn't be possible */
+ if (WARN_ON_ONCE(xprt->xpt_net != net)) {
+ list_move(&xprt->xpt_list, &serv->sv_permsocks);
+ continue;
+ }
+
+ /* If everything matches, put it back */
+ if (!strcmp(xprt->xpt_class->xcl_name, xcl_name) &&
+ rpc_cmp_addr_port(sa, (struct sockaddr *)&xprt->xpt_local)) {
+ list_move(&xprt->xpt_list, &serv->sv_permsocks);
+ break;
+ }
+ }
+ }
+
+ /* For now, no removing old sockets while server is running */
+ if (serv->sv_nrthreads && !list_empty(&permsocks)) {
+ list_splice_init(&permsocks, &serv->sv_permsocks);
+ spin_unlock_bh(&serv->sv_lock);
+ err = -EBUSY;
+ goto out_unlock_mtx;
+ }
+
+ /* Close the remaining sockets on the permsocks list */
+ while (!list_empty(&permsocks)) {
+ xprt = list_first_entry(&permsocks, struct svc_xprt, xpt_list);
+ list_move(&xprt->xpt_list, &serv->sv_permsocks);
+
+ /*
+ * Newly-created sockets are born with the BUSY bit set. Clear
+ * it if there are no threads, since nothing can pick it up
+ * in that case.
+ */
+ if (!serv->sv_nrthreads)
+ clear_bit(XPT_BUSY, &xprt->xpt_flags);
+
+ set_bit(XPT_CLOSE, &xprt->xpt_flags);
+ spin_unlock_bh(&serv->sv_lock);
+ svc_xprt_close(xprt);
+ spin_lock_bh(&serv->sv_lock);
+ }
+
+ spin_unlock_bh(&serv->sv_lock);
+
+ /* walk list of addrs again, open any that still don't exist */
+ nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+ struct nlattr *tb[NFSD_A_SOCK_MAX + 1];
+ const char *xcl_name;
+ struct sockaddr *sa;
+ int ret;
+
+ if (nla_type(attr) != NFSD_A_SERVER_SOCK_ADDR)
+ continue;
+
+ if (nla_parse_nested(tb, NFSD_A_SOCK_MAX, attr,
+ nfsd_sock_nl_policy, info->extack) < 0)
+ continue;
+
+ if (!tb[NFSD_A_SOCK_ADDR] || !tb[NFSD_A_SOCK_TRANSPORT_NAME])
+ continue;
+
+ if (nla_len(tb[NFSD_A_SOCK_ADDR]) < sizeof(*sa))
+ continue;
+
+ xcl_name = nla_data(tb[NFSD_A_SOCK_TRANSPORT_NAME]);
+ sa = nla_data(tb[NFSD_A_SOCK_ADDR]);
+
+ xprt = svc_find_listener(serv, xcl_name, net, sa);
+ if (xprt) {
+ svc_xprt_put(xprt);
+ continue;
+ }
+
+ ret = svc_xprt_create_from_sa(serv, xcl_name, net, sa,
+ SVC_SOCK_ANONYMOUS,
+ get_current_cred());
+ /* always save the latest error */
+ if (ret < 0)
+ err = ret;
+ }
+
+ if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
+ nfsd_destroy_serv(net);
+
+out_unlock_mtx:
+ mutex_unlock(&nfsd_mutex);
+
+ return err;
+}
+
+/**
+ * nfsd_nl_listener_get_doit - get the nfs running listeners
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct svc_xprt *xprt;
+ struct svc_serv *serv;
+ struct nfsd_net *nn;
+ void *hdr;
+ int err;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_iput(skb, info);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto err_free_msg;
+ }
+
+ mutex_lock(&nfsd_mutex);
+ nn = net_generic(genl_info_net(info), nfsd_net_id);
+
+ /* no nfs server? Just send empty socket list */
+ if (!nn->nfsd_serv)
+ goto out_unlock_mtx;
+
+ serv = nn->nfsd_serv;
+ spin_lock_bh(&serv->sv_lock);
+ list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
+ struct nlattr *attr;
+
+ attr = nla_nest_start_noflag(skb, NFSD_A_SERVER_SOCK_ADDR);
+ if (!attr) {
+ err = -EINVAL;
+ goto err_serv_unlock;
+ }
+
+ if (nla_put_string(skb, NFSD_A_SOCK_TRANSPORT_NAME,
+ xprt->xpt_class->xcl_name) ||
+ nla_put(skb, NFSD_A_SOCK_ADDR,
+ sizeof(struct sockaddr_storage),
+ &xprt->xpt_local)) {
+ err = -EINVAL;
+ goto err_serv_unlock;
+ }
+
+ nla_nest_end(skb, attr);
+ }
+ spin_unlock_bh(&serv->sv_lock);
+out_unlock_mtx:
+ mutex_unlock(&nfsd_mutex);
+ genlmsg_end(skb, hdr);
+
+ return genlmsg_reply(skb, info);
+
+err_serv_unlock:
+ spin_unlock_bh(&serv->sv_lock);
+ mutex_unlock(&nfsd_mutex);
+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 ccf3e15fe160..5253039aeb4d 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -52,12 +52,29 @@ enum {
NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
};

+enum {
+ NFSD_A_SOCK_ADDR = 1,
+ NFSD_A_SOCK_TRANSPORT_NAME,
+
+ __NFSD_A_SOCK_MAX,
+ NFSD_A_SOCK_MAX = (__NFSD_A_SOCK_MAX - 1)
+};
+
+enum {
+ NFSD_A_SERVER_SOCK_ADDR = 1,
+
+ __NFSD_A_SERVER_SOCK_MAX,
+ NFSD_A_SERVER_SOCK_MAX = (__NFSD_A_SERVER_SOCK_MAX - 1)
+};
+
enum {
NFSD_CMD_RPC_STATUS_GET = 1,
NFSD_CMD_THREADS_SET,
NFSD_CMD_THREADS_GET,
NFSD_CMD_VERSION_SET,
NFSD_CMD_VERSION_GET,
+ NFSD_CMD_LISTENER_SET,
+ NFSD_CMD_LISTENER_GET,

__NFSD_CMD_MAX,
NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
--
2.44.0


2024-04-11 18:23:23

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v7 0/5] convert write_threads, write_version and write_ports to netlink commands

On Thu, 2024-04-11 at 18:47 +0200, Lorenzo Bianconi wrote:
> Introduce write_threads, write_version and write_ports netlink
> commands similar to the ones available through the procfs.
>
> Changes since v6:
> - add the capability to pass sockaddr from userspace through listener-set
> command
> - rebase on top of nfsd-next
> Changes since v5:
> - for write_ports and write_version commands, userspace is expected to provide
> a NFS listeners/supported versions list it want to enable (all the other
> ports/versions will be disabled).
> - fix comments
> - rebase on top of nfsd-next
> Changes since v4:
> - rebase on top of nfsd-next tree
> Changes since v3:
> - drop write_maxconn and write_maxblksize for the moment
> - add write_version and write_ports commands
> Changes since v2:
> - use u32 to store nthreads in nfsd_nl_threads_set_doit
> - rename server-attr in control-plane in nfsd.yaml specs
> Changes since v1:
> - remove write_v4_end_grace command
> - add write_maxblksize and write_maxconn netlink commands
>
> This patch can be tested with user-space tool reported below:
> https://github.com/LorenzoBianconi/nfsdctl
>

I'm reworking the interface on that tool and adding it to nfs-utils as
well. Patches will be coming soon.

> Jeff Layton (1):
> SUNRPC: add a new svc_find_listener helper
>
> Lorenzo Bianconi (4):
> NFSD: convert write_threads to netlink command
> NFSD: add write_version to netlink command
> SUNRPC: introduce svc_xprt_create_from_sa utility routine
> NFSD: add listener-{set,get} netlink command
>
> Documentation/netlink/specs/nfsd.yaml | 94 ++++++
> fs/nfsd/netlink.c | 63 ++++
> fs/nfsd/netlink.h | 10 +
> fs/nfsd/netns.h | 1 +
> fs/nfsd/nfsctl.c | 433 ++++++++++++++++++++++++++
> fs/nfsd/nfssvc.c | 3 +-
> include/linux/sunrpc/svc_xprt.h | 5 +
> include/uapi/linux/nfsd_netlink.h | 44 +++
> net/sunrpc/svc_xprt.c | 167 ++++++----
> 9 files changed, 760 insertions(+), 60 deletions(-)
>

Reviewed-by: Jeff Layton <[email protected]>

2024-04-11 18:32:05

by Lorenzo Bianconi

[permalink] [raw]
Subject: [PATCH v7 1/5] NFSD: convert write_threads to netlink command

Introduce write_threads netlink command similar to the one available
through the procfs.

Tested-by: Jeff Layton <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Documentation/netlink/specs/nfsd.yaml | 23 ++++++++++
fs/nfsd/netlink.c | 17 ++++++++
fs/nfsd/netlink.h | 2 +
fs/nfsd/nfsctl.c | 60 +++++++++++++++++++++++++++
include/uapi/linux/nfsd_netlink.h | 9 ++++
5 files changed, 111 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index 05acc73e2e33..c92e1425d316 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -62,6 +62,12 @@ attribute-sets:
name: compound-ops
type: u32
multi-attr: true
+ -
+ name: server-worker
+ attributes:
+ -
+ name: threads
+ type: u32

operations:
list:
@@ -87,3 +93,20 @@ operations:
- sport
- dport
- compound-ops
+ -
+ name: threads-set
+ doc: set the number of running threads
+ attribute-set: server-worker
+ flags: [ admin-perm ]
+ do:
+ request:
+ attributes:
+ - threads
+ -
+ name: threads-get
+ doc: get the number of running threads
+ attribute-set: server-worker
+ do:
+ reply:
+ attributes:
+ - threads
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 0e1d635ec5f9..1a59a8e6c7e2 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -10,6 +10,11 @@

#include <uapi/linux/nfsd_netlink.h>

+/* NFSD_CMD_THREADS_SET - do */
+static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
+ [NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
+};
+
/* Ops table for nfsd */
static const struct genl_split_ops nfsd_nl_ops[] = {
{
@@ -19,6 +24,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
.done = nfsd_nl_rpc_status_get_done,
.flags = GENL_CMD_CAP_DUMP,
},
+ {
+ .cmd = NFSD_CMD_THREADS_SET,
+ .doit = nfsd_nl_threads_set_doit,
+ .policy = nfsd_threads_set_nl_policy,
+ .maxattr = NFSD_A_SERVER_WORKER_THREADS,
+ .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = NFSD_CMD_THREADS_GET,
+ .doit = nfsd_nl_threads_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 d83dd6bdee92..4137fac477e4 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -16,6 +16,8 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);

int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
struct netlink_callback *cb);
+int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_threads_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 93c87587e646..71608744e67f 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1651,6 +1651,66 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
return 0;
}

+/**
+ * nfsd_nl_threads_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_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ u32 nthreads;
+ int ret;
+
+ if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_WORKER_THREADS))
+ return -EINVAL;
+
+ nthreads = nla_get_u32(info->attrs[NFSD_A_SERVER_WORKER_THREADS]);
+ ret = nfsd_svc(nthreads,
+ genl_info_net(info), get_current_cred());
+
+ return ret == nthreads ? 0 : -EINVAL;
+}
+
+/**
+ * nfsd_nl_threads_get_doit - get 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_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ void *hdr;
+ int err;
+
+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = genlmsg_iput(skb, info);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto err_free_msg;
+ }
+
+ if (nla_put_u32(skb, NFSD_A_SERVER_WORKER_THREADS,
+ nfsd_nrthreads(genl_info_net(info)))) {
+ err = -EINVAL;
+ 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 3cd044edee5d..1b6fe1f9ed0e 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -29,8 +29,17 @@ enum {
NFSD_A_RPC_STATUS_MAX = (__NFSD_A_RPC_STATUS_MAX - 1)
};

+enum {
+ NFSD_A_SERVER_WORKER_THREADS = 1,
+
+ __NFSD_A_SERVER_WORKER_MAX,
+ NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
+};
+
enum {
NFSD_CMD_RPC_STATUS_GET = 1,
+ NFSD_CMD_THREADS_SET,
+ NFSD_CMD_THREADS_GET,

__NFSD_CMD_MAX,
NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
--
2.44.0


2024-04-11 22:48:52

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] NFSD: add write_version to netlink command

On Thu, 2024-04-11 at 18:47 +0200, Lorenzo Bianconi wrote:
> Introduce write_version netlink command through a "declarative" interface.
> This patch introduces a change in behavior since for version-set userspace
> is expected to provide a NFS major/minor version list it wants to enable
> while all the other ones will be disabled. (procfs write_version
> command implements imperative interface where the admin writes +3/-3 to
> enable/disable a single version.
>
> Tested-by: Jeff Layton <[email protected]>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
> Documentation/netlink/specs/nfsd.yaml | 37 +++++++
> fs/nfsd/netlink.c | 24 ++++
> fs/nfsd/netlink.h | 5 +
> fs/nfsd/netns.h | 1 +
> fs/nfsd/nfsctl.c | 153 ++++++++++++++++++++++++++
> fs/nfsd/nfssvc.c | 3 +-
> include/uapi/linux/nfsd_netlink.h | 18 +++
> 7 files changed, 239 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> index c92e1425d316..cb93e3e37119 100644
> --- a/Documentation/netlink/specs/nfsd.yaml
> +++ b/Documentation/netlink/specs/nfsd.yaml
> @@ -68,6 +68,26 @@ attribute-sets:
> -
> name: threads
> type: u32
> + -
> + name: version
> + attributes:
> + -
> + name: major
> + type: u32
> + -
> + name: minor
> + type: u32
> + -
> + name: enabled
> + type: flag
> + -
> + name: server-proto
> + attributes:
> + -
> + name: version
> + type: nest
> + nested-attributes: version
> + multi-attr: true
>
> operations:
> list:
> @@ -110,3 +130,20 @@ operations:
> reply:
> attributes:
> - threads
> + -
> + name: version-set
> + doc: set nfs enabled versions
> + attribute-set: server-proto
> + flags: [ admin-perm ]
> + do:
> + request:
> + attributes:
> + - version
> + -
> + name: version-get
> + doc: get nfs enabled versions
> + attribute-set: server-proto
> + do:
> + reply:
> + attributes:
> + - version
> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> index 1a59a8e6c7e2..75f609b57ceb 100644
> --- a/fs/nfsd/netlink.c
> +++ b/fs/nfsd/netlink.c
> @@ -10,11 +10,23 @@
>
> #include <uapi/linux/nfsd_netlink.h>
>
> +/* Common nested types */
> +const struct nla_policy nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1] = {
> + [NFSD_A_VERSION_MAJOR] = { .type = NLA_U32, },
> + [NFSD_A_VERSION_MINOR] = { .type = NLA_U32, },
> + [NFSD_A_VERSION_ENABLED] = { .type = NLA_FLAG, },
> +};
> +
> /* NFSD_CMD_THREADS_SET - do */
> static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
> [NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
> };
>
> +/* NFSD_CMD_VERSION_SET - do */
> +static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VERSION + 1] = {
> + [NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_version_nl_policy),
> +};
> +
> /* Ops table for nfsd */
> static const struct genl_split_ops nfsd_nl_ops[] = {
> {
> @@ -36,6 +48,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> .doit = nfsd_nl_threads_get_doit,
> .flags = GENL_CMD_CAP_DO,
> },
> + {
> + .cmd = NFSD_CMD_VERSION_SET,
> + .doit = nfsd_nl_version_set_doit,
> + .policy = nfsd_version_set_nl_policy,
> + .maxattr = NFSD_A_SERVER_PROTO_VERSION,
> + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> + },
> + {
> + .cmd = NFSD_CMD_VERSION_GET,
> + .doit = nfsd_nl_version_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 4137fac477e4..c7c0da275481 100644
> --- a/fs/nfsd/netlink.h
> +++ b/fs/nfsd/netlink.h
> @@ -11,6 +11,9 @@
>
> #include <uapi/linux/nfsd_netlink.h>
>
> +/* Common nested types */
> +extern const struct nla_policy nfsd_version_nl_policy[NFSD_A_VERSION_ENABLED + 1];
> +
> int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
> int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
>
> @@ -18,6 +21,8 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
> struct netlink_callback *cb);
> int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
> int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
> +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);
>
> extern struct genl_family nfsd_nl_family;
>
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index d4be519b5734..14ec15656320 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -218,6 +218,7 @@ struct nfsd_net {
> /* Simple check to find out if a given net was properly initialized */
> #define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl)
>
> +extern bool nfsd_support_version(int vers);
> extern void nfsd_netns_free_versions(struct nfsd_net *nn);
>
> extern unsigned int nfsd_net_id;
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 71608744e67f..341efab4eaa7 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1711,6 +1711,159 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
> return err;
> }
>
> +/**
> + * nfsd_nl_version_set_doit - set the nfs enabled versions
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> + const struct nlattr *attr;
> + struct nfsd_net *nn;
> + int i, rem;
> +
> + if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_PROTO_VERSION))
> + return -EINVAL;
> +
> + mutex_lock(&nfsd_mutex);
> +
> + nn = net_generic(genl_info_net(info), nfsd_net_id);
> + if (nn->nfsd_serv) {
> + mutex_unlock(&nfsd_mutex);
> + return -EBUSY;
> + }
> +
> + /* clear current supported versions. */
> + nfsd_vers(nn, 2, NFSD_CLEAR);
> + nfsd_vers(nn, 3, NFSD_CLEAR);
> + for (i = 0; i <= NFSD_SUPPORTED_MINOR_VERSION; i++)
> + nfsd_minorversion(nn, i, NFSD_CLEAR);
> +
> + nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> + struct nlattr *tb[NFSD_A_VERSION_MAX + 1];
> + u32 major, minor = 0;
> + bool enabled;
> +
> + if (nla_type(attr) != NFSD_A_SERVER_PROTO_VERSION)
> + continue;
> +
> + if (nla_parse_nested(tb, NFSD_A_VERSION_MAX, attr,
> + nfsd_version_nl_policy, info->extack) < 0)
> + continue;
> +
> + if (!tb[NFSD_A_VERSION_MAJOR])
> + continue;
> +
> + major = nla_get_u32(tb[NFSD_A_VERSION_MAJOR]);
> + if (tb[NFSD_A_VERSION_MINOR])
> + minor = nla_get_u32(tb[NFSD_A_VERSION_MINOR]);
> +
> + enabled = nla_get_flag(tb[NFSD_A_VERSION_ENABLED]);
> +
> + switch (major) {
> + case 4:
> + nfsd_minorversion(nn, minor, enabled ? NFSD_SET : NFSD_CLEAR);
> + break;
> + case 3:
> + case 2:
> + if (!minor)
> + nfsd_vers(nn, major, enabled ? NFSD_SET : NFSD_CLEAR);
> + break;
> + default:
> + break;
> + }
> + }
> +
> + mutex_unlock(&nfsd_mutex);
> +
> + return 0;
> +}
> +
> +/**
> + * nfsd_nl_version_get_doit - get the nfs enabled versions
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> + struct nfsd_net *nn;
> + int i, err;
> + void *hdr;
> +
> + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOMEM;
> +
> + hdr = genlmsg_iput(skb, info);
> + if (!hdr) {
> + err = -EMSGSIZE;
> + goto err_free_msg;
> + }
> +
> + mutex_lock(&nfsd_mutex);
> + nn = net_generic(genl_info_net(info), nfsd_net_id);
> +
> + for (i = 2; i <= 4; i++) {
> + int j;
> +
> + for (j = 0; j <= NFSD_SUPPORTED_MINOR_VERSION; j++) {
> + struct nlattr *attr;
> +
> + /* Don't record any versions the kernel doesn't have
> + * compiled in
> + */
> + if (!nfsd_support_version(i))
> + continue;
> +
> + /* NFSv{2,3} does not support minor numbers */
> + if (i < 4 && j)
> + continue;
> +
> + if (i == 4 && !nfsd_minorversion(nn, j, NFSD_TEST))
> + continue;
> +

I think this is not quite right. We don't want to skip returning a
minorversion just because it's not enabled. We want to return the entry
with the enabled flag cleared. I'd suggest just dropping the above if
statement entirely.

> + attr = nla_nest_start_noflag(skb,
> + NFSD_A_SERVER_PROTO_VERSION);
> + if (!attr) {
> + err = -EINVAL;
> + goto err_nfsd_unlock;
> + }
> +
> + if (nla_put_u32(skb, NFSD_A_VERSION_MAJOR, i) ||
> + nla_put_u32(skb, NFSD_A_VERSION_MINOR, j)) {
> + err = -EINVAL;
> + goto err_nfsd_unlock;
> + }
> +
> + /* Set the enabled flag if the version is enabled */
> + if (nfsd_vers(nn, i, NFSD_TEST) &&
> + (i < 4 || nfsd_minorversion(nn, j, NFSD_TEST)) &&
> + nla_put_flag(skb, NFSD_A_VERSION_ENABLED)) {
> + err = -EINVAL;
> + goto err_nfsd_unlock;
> + }
> +
> + nla_nest_end(skb, attr);
> + }
> + }
> +
> + mutex_unlock(&nfsd_mutex);
> + genlmsg_end(skb, hdr);
> +
> + return genlmsg_reply(skb, info);
> +
> +err_nfsd_unlock:
> + mutex_unlock(&nfsd_mutex);
> +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/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index c0d17b92b249..4452a9bb9bbb 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -133,8 +133,7 @@ struct svc_program nfsd_program = {
> .pg_rpcbind_set = nfsd_rpcbind_set,
> };
>
> -static bool
> -nfsd_support_version(int vers)
> +bool nfsd_support_version(int vers)
> {
> if (vers >= NFSD_MINVERS && vers < NFSD_NRVERS)
> return nfsd_version[vers] != NULL;
> diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> index 1b6fe1f9ed0e..ccf3e15fe160 100644
> --- a/include/uapi/linux/nfsd_netlink.h
> +++ b/include/uapi/linux/nfsd_netlink.h
> @@ -36,10 +36,28 @@ enum {
> NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
> };
>
> +enum {
> + NFSD_A_VERSION_MAJOR = 1,
> + NFSD_A_VERSION_MINOR,
> + NFSD_A_VERSION_ENABLED,
> +
> + __NFSD_A_VERSION_MAX,
> + NFSD_A_VERSION_MAX = (__NFSD_A_VERSION_MAX - 1)
> +};
> +
> +enum {
> + NFSD_A_SERVER_PROTO_VERSION = 1,
> +
> + __NFSD_A_SERVER_PROTO_MAX,
> + NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
> +};
> +
> enum {
> NFSD_CMD_RPC_STATUS_GET = 1,
> NFSD_CMD_THREADS_SET,
> NFSD_CMD_THREADS_GET,
> + NFSD_CMD_VERSION_SET,
> + NFSD_CMD_VERSION_GET,
>
> __NFSD_CMD_MAX,
> NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)

--
Jeff Layton <[email protected]>

2024-04-12 13:22:44

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] NFSD: convert write_threads to netlink command

On Thu, 2024-04-11 at 18:47 +0200, Lorenzo Bianconi wrote:
> Introduce write_threads netlink command similar to the one available
> through the procfs.
>
> Tested-by: Jeff Layton <[email protected]>
> Reviewed-by: Jeff Layton <[email protected]>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
> Documentation/netlink/specs/nfsd.yaml | 23 ++++++++++
> fs/nfsd/netlink.c | 17 ++++++++
> fs/nfsd/netlink.h | 2 +
> fs/nfsd/nfsctl.c | 60 +++++++++++++++++++++++++++
> include/uapi/linux/nfsd_netlink.h | 9 ++++
> 5 files changed, 111 insertions(+)
>
> diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> index 05acc73e2e33..c92e1425d316 100644
> --- a/Documentation/netlink/specs/nfsd.yaml
> +++ b/Documentation/netlink/specs/nfsd.yaml
> @@ -62,6 +62,12 @@ attribute-sets:
> name: compound-ops
> type: u32
> multi-attr: true
> + -
> + name: server-worker
> + attributes:
> + -
> + name: threads
> + type: u32
>
> operations:
> list:
> @@ -87,3 +93,20 @@ operations:
> - sport
> - dport
> - compound-ops
> + -
> + name: threads-set
> + doc: set the number of running threads
> + attribute-set: server-worker
> + flags: [ admin-perm ]
> + do:
> + request:
> + attributes:
> + - threads

There's only one thing I think we're missing to get feature parity with
rpc.nfsd, and that's the ability to set the lease-time and grace-time
when starting the server.

I think the simplest thing to do would be to just add 2 optional int
parameters to the threads-set command. They only matter once you're
starting the server anyway. I'll see about spinning up a patch, but let
me know if you see problems with that approach.

> + -
> + name: threads-get
> + doc: get the number of running threads
> + attribute-set: server-worker
> + do:
> + reply:
> + attributes:
> + - threads
> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> index 0e1d635ec5f9..1a59a8e6c7e2 100644
> --- a/fs/nfsd/netlink.c
> +++ b/fs/nfsd/netlink.c
> @@ -10,6 +10,11 @@
>
> #include <uapi/linux/nfsd_netlink.h>
>
> +/* NFSD_CMD_THREADS_SET - do */
> +static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
> + [NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
> +};
> +
> /* Ops table for nfsd */
> static const struct genl_split_ops nfsd_nl_ops[] = {
> {
> @@ -19,6 +24,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> .done = nfsd_nl_rpc_status_get_done,
> .flags = GENL_CMD_CAP_DUMP,
> },
> + {
> + .cmd = NFSD_CMD_THREADS_SET,
> + .doit = nfsd_nl_threads_set_doit,
> + .policy = nfsd_threads_set_nl_policy,
> + .maxattr = NFSD_A_SERVER_WORKER_THREADS,
> + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> + },
> + {
> + .cmd = NFSD_CMD_THREADS_GET,
> + .doit = nfsd_nl_threads_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 d83dd6bdee92..4137fac477e4 100644
> --- a/fs/nfsd/netlink.h
> +++ b/fs/nfsd/netlink.h
> @@ -16,6 +16,8 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
>
> int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
> struct netlink_callback *cb);
> +int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
> +int nfsd_nl_threads_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 93c87587e646..71608744e67f 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1651,6 +1651,66 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
> return 0;
> }
>
> +/**
> + * nfsd_nl_threads_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_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> + u32 nthreads;
> + int ret;
> +
> + if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_WORKER_THREADS))
> + return -EINVAL;
> +
> + nthreads = nla_get_u32(info->attrs[NFSD_A_SERVER_WORKER_THREADS]);
> + ret = nfsd_svc(nthreads,
> + genl_info_net(info), get_current_cred());
> +
> + return ret == nthreads ? 0 : -EINVAL;
> +}
> +
> +/**
> + * nfsd_nl_threads_get_doit - get 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_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> + void *hdr;
> + int err;
> +
> + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!skb)
> + return -ENOMEM;
> +
> + hdr = genlmsg_iput(skb, info);
> + if (!hdr) {
> + err = -EMSGSIZE;
> + goto err_free_msg;
> + }
> +
> + if (nla_put_u32(skb, NFSD_A_SERVER_WORKER_THREADS,
> + nfsd_nrthreads(genl_info_net(info)))) {
> + err = -EINVAL;
> + 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 3cd044edee5d..1b6fe1f9ed0e 100644
> --- a/include/uapi/linux/nfsd_netlink.h
> +++ b/include/uapi/linux/nfsd_netlink.h
> @@ -29,8 +29,17 @@ enum {
> NFSD_A_RPC_STATUS_MAX = (__NFSD_A_RPC_STATUS_MAX - 1)
> };
>
> +enum {
> + NFSD_A_SERVER_WORKER_THREADS = 1,
> +
> + __NFSD_A_SERVER_WORKER_MAX,
> + NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
> +};
> +
> enum {
> NFSD_CMD_RPC_STATUS_GET = 1,
> + NFSD_CMD_THREADS_SET,
> + NFSD_CMD_THREADS_GET,
>
> __NFSD_CMD_MAX,
> NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)

--
Jeff Layton <[email protected]>

2024-04-12 14:29:15

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH v7 1/5] NFSD: convert write_threads to netlink command

> On Thu, 2024-04-11 at 18:47 +0200, Lorenzo Bianconi wrote:
> > Introduce write_threads netlink command similar to the one available
> > through the procfs.
> >
> > Tested-by: Jeff Layton <[email protected]>
> > Reviewed-by: Jeff Layton <[email protected]>
> > Signed-off-by: Lorenzo Bianconi <[email protected]>
> > ---
> > Documentation/netlink/specs/nfsd.yaml | 23 ++++++++++
> > fs/nfsd/netlink.c | 17 ++++++++
> > fs/nfsd/netlink.h | 2 +
> > fs/nfsd/nfsctl.c | 60 +++++++++++++++++++++++++++
> > include/uapi/linux/nfsd_netlink.h | 9 ++++
> > 5 files changed, 111 insertions(+)
> >
> > diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> > index 05acc73e2e33..c92e1425d316 100644
> > --- a/Documentation/netlink/specs/nfsd.yaml
> > +++ b/Documentation/netlink/specs/nfsd.yaml
> > @@ -62,6 +62,12 @@ attribute-sets:
> > name: compound-ops
> > type: u32
> > multi-attr: true
> > + -
> > + name: server-worker
> > + attributes:
> > + -
> > + name: threads
> > + type: u32
> >
> > operations:
> > list:
> > @@ -87,3 +93,20 @@ operations:
> > - sport
> > - dport
> > - compound-ops
> > + -
> > + name: threads-set
> > + doc: set the number of running threads
> > + attribute-set: server-worker
> > + flags: [ admin-perm ]
> > + do:
> > + request:
> > + attributes:
> > + - threads
>
> There's only one thing I think we're missing to get feature parity with
> rpc.nfsd, and that's the ability to set the lease-time and grace-time
> when starting the server.
>
> I think the simplest thing to do would be to just add 2 optional int
> parameters to the threads-set command. They only matter once you're
> starting the server anyway. I'll see about spinning up a patch, but let
> me know if you see problems with that approach.

ack, I am fine with it, we can squash it in the next revision or send a
follow-up patch.

Regards,
Lorenzo

>
> > + -
> > + name: threads-get
> > + doc: get the number of running threads
> > + attribute-set: server-worker
> > + do:
> > + reply:
> > + attributes:
> > + - threads
> > diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> > index 0e1d635ec5f9..1a59a8e6c7e2 100644
> > --- a/fs/nfsd/netlink.c
> > +++ b/fs/nfsd/netlink.c
> > @@ -10,6 +10,11 @@
> >
> > #include <uapi/linux/nfsd_netlink.h>
> >
> > +/* NFSD_CMD_THREADS_SET - do */
> > +static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
> > + [NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
> > +};
> > +
> > /* Ops table for nfsd */
> > static const struct genl_split_ops nfsd_nl_ops[] = {
> > {
> > @@ -19,6 +24,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> > .done = nfsd_nl_rpc_status_get_done,
> > .flags = GENL_CMD_CAP_DUMP,
> > },
> > + {
> > + .cmd = NFSD_CMD_THREADS_SET,
> > + .doit = nfsd_nl_threads_set_doit,
> > + .policy = nfsd_threads_set_nl_policy,
> > + .maxattr = NFSD_A_SERVER_WORKER_THREADS,
> > + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > + },
> > + {
> > + .cmd = NFSD_CMD_THREADS_GET,
> > + .doit = nfsd_nl_threads_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 d83dd6bdee92..4137fac477e4 100644
> > --- a/fs/nfsd/netlink.h
> > +++ b/fs/nfsd/netlink.h
> > @@ -16,6 +16,8 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
> >
> > int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
> > struct netlink_callback *cb);
> > +int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
> > +int nfsd_nl_threads_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 93c87587e646..71608744e67f 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -1651,6 +1651,66 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
> > return 0;
> > }
> >
> > +/**
> > + * nfsd_nl_threads_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_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + u32 nthreads;
> > + int ret;
> > +
> > + if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_WORKER_THREADS))
> > + return -EINVAL;
> > +
> > + nthreads = nla_get_u32(info->attrs[NFSD_A_SERVER_WORKER_THREADS]);
> > + ret = nfsd_svc(nthreads,
> > + genl_info_net(info), get_current_cred());
> > +
> > + return ret == nthreads ? 0 : -EINVAL;
> > +}
> > +
> > +/**
> > + * nfsd_nl_threads_get_doit - get 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_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + void *hdr;
> > + int err;
> > +
> > + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + hdr = genlmsg_iput(skb, info);
> > + if (!hdr) {
> > + err = -EMSGSIZE;
> > + goto err_free_msg;
> > + }
> > +
> > + if (nla_put_u32(skb, NFSD_A_SERVER_WORKER_THREADS,
> > + nfsd_nrthreads(genl_info_net(info)))) {
> > + err = -EINVAL;
> > + 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 3cd044edee5d..1b6fe1f9ed0e 100644
> > --- a/include/uapi/linux/nfsd_netlink.h
> > +++ b/include/uapi/linux/nfsd_netlink.h
> > @@ -29,8 +29,17 @@ enum {
> > NFSD_A_RPC_STATUS_MAX = (__NFSD_A_RPC_STATUS_MAX - 1)
> > };
> >
> > +enum {
> > + NFSD_A_SERVER_WORKER_THREADS = 1,
> > +
> > + __NFSD_A_SERVER_WORKER_MAX,
> > + NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
> > +};
> > +
> > enum {
> > NFSD_CMD_RPC_STATUS_GET = 1,
> > + NFSD_CMD_THREADS_SET,
> > + NFSD_CMD_THREADS_GET,
> >
> > __NFSD_CMD_MAX,
> > NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
>
> --
> Jeff Layton <[email protected]>
>


Attachments:
(No filename) (6.78 kB)
signature.asc (235.00 B)
Download all attachments

2024-04-15 18:03:32

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v7 2/5] NFSD: add write_version to netlink command

On Thu, 11 Apr 2024 18:47:25 +0200 Lorenzo Bianconi wrote:
> + attr = nla_nest_start_noflag(skb,
> + NFSD_A_SERVER_PROTO_VERSION);

The _noflag() version is for legacy code, I think you should let
the nest be marked appropriately.