[PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[Lorenzo Bianconi]

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

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/nfsd-netlink.git
This series is based on the commit below available in net-next tree

commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
Author: Jakub Kicinski <kuba@kernel.org>
Date:   Fri Oct 6 06:50:32 2023 -0700

    tools: ynl-gen: handle do ops with no input attrs

    The code supports dumps with no input attributes currently
    thru a combination of special-casing and luck.
    Clean up the handling of ops with no inputs. Create empty
    Structs, and skip printing of empty types.
    This makes dos with no inputs work.

Lorenzo Bianconi (3):
  NFSD: convert write_threads to netlink commands
  NFSD: convert write_version to netlink commands
  NFSD: convert write_ports to netlink commands

 Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
 fs/nfsd/netlink.c                     |  54 ++++++
 fs/nfsd/netlink.h                     |   8 +
 fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
 include/uapi/linux/nfsd_netlink.h     |  30 +++
 tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
 7 files changed, 845 insertions(+), 7 deletions(-)

-- 
2.41.0

[PATCH v4 1/3] NFSD: convert write_threads to netlink command

[Lorenzo Bianconi]

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

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/netlink/specs/nfsd.yaml | 23 +++++++
 fs/nfsd/netlink.c                     | 17 +++++
 fs/nfsd/netlink.h                     |  2 +
 fs/nfsd/nfsctl.c                      | 58 +++++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  9 +++
 tools/net/ynl/generated/nfsd-user.c   | 92 +++++++++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   | 47 ++++++++++++++
 7 files changed, 248 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 739ed5bf71cd..0d0394887506 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1693,6 +1693,64 @@ 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 : ret;
+}
+
+/**
+ * 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 c8ae72466ee6..99f7855852a1 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)
diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
index fec6828680ce..342a00b0474a 100644
--- a/tools/net/ynl/generated/nfsd-user.c
+++ b/tools/net/ynl/generated/nfsd-user.c
@@ -15,6 +15,8 @@
 /* Enums */
 static const char * const nfsd_op_strmap[] = {
 	[NFSD_CMD_RPC_STATUS_GET] = "rpc-status-get",
+	[NFSD_CMD_THREADS_SET] = "threads-set",
+	[NFSD_CMD_THREADS_GET] = "threads-get",
 };
 
 const char *nfsd_op_str(int op)
@@ -47,6 +49,15 @@ struct ynl_policy_nest nfsd_rpc_status_nest = {
 	.table = nfsd_rpc_status_policy,
 };
 
+struct ynl_policy_attr nfsd_server_worker_policy[NFSD_A_SERVER_WORKER_MAX + 1] = {
+	[NFSD_A_SERVER_WORKER_THREADS] = { .name = "threads", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest nfsd_server_worker_nest = {
+	.max_attr = NFSD_A_SERVER_WORKER_MAX,
+	.table = nfsd_server_worker_policy,
+};
+
 /* Common nested types */
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
@@ -90,6 +101,87 @@ struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys)
 	return NULL;
 }
 
+/* ============== NFSD_CMD_THREADS_SET ============== */
+/* NFSD_CMD_THREADS_SET - do */
+void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req)
+{
+	free(req);
+}
+
+int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_SET, 1);
+	ys->req_policy = &nfsd_server_worker_nest;
+
+	if (req->_present.threads)
+		mnl_attr_put_u32(nlh, NFSD_A_SERVER_WORKER_THREADS, req->threads);
+
+	err = ynl_exec(ys, nlh, NULL);
+	if (err < 0)
+		return -1;
+
+	return 0;
+}
+
+/* ============== NFSD_CMD_THREADS_GET ============== */
+/* NFSD_CMD_THREADS_GET - do */
+void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp)
+{
+	free(rsp);
+}
+
+int nfsd_threads_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
+{
+	struct ynl_parse_arg *yarg = data;
+	struct nfsd_threads_get_rsp *dst;
+	const struct nlattr *attr;
+
+	dst = yarg->data;
+
+	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NFSD_A_SERVER_WORKER_THREADS) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.threads = 1;
+			dst->threads = mnl_attr_get_u32(attr);
+		}
+	}
+
+	return MNL_CB_OK;
+}
+
+struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct nfsd_threads_get_rsp *rsp;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_GET, 1);
+	ys->req_policy = &nfsd_server_worker_nest;
+	yrs.yarg.rsp_policy = &nfsd_server_worker_nest;
+
+	rsp = calloc(1, sizeof(*rsp));
+	yrs.yarg.data = rsp;
+	yrs.cb = nfsd_threads_get_rsp_parse;
+	yrs.rsp_cmd = NFSD_CMD_THREADS_GET;
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		goto err_free;
+
+	return rsp;
+
+err_free:
+	nfsd_threads_get_rsp_free(rsp);
+	return NULL;
+}
+
 const struct ynl_family ynl_nfsd_family =  {
 	.name		= "nfsd",
 };
diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
index b6b69501031a..4c11119217f1 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -30,4 +30,51 @@ void nfsd_rpc_status_get_list_free(struct nfsd_rpc_status_get_list *rsp);
 
 struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys);
 
+/* ============== NFSD_CMD_THREADS_SET ============== */
+/* NFSD_CMD_THREADS_SET - do */
+struct nfsd_threads_set_req {
+	struct {
+		__u32 threads:1;
+	} _present;
+
+	__u32 threads;
+};
+
+static inline struct nfsd_threads_set_req *nfsd_threads_set_req_alloc(void)
+{
+	return calloc(1, sizeof(struct nfsd_threads_set_req));
+}
+void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req);
+
+static inline void
+nfsd_threads_set_req_set_threads(struct nfsd_threads_set_req *req,
+				 __u32 threads)
+{
+	req->_present.threads = 1;
+	req->threads = threads;
+}
+
+/*
+ * set the number of running threads
+ */
+int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req);
+
+/* ============== NFSD_CMD_THREADS_GET ============== */
+/* NFSD_CMD_THREADS_GET - do */
+
+struct nfsd_threads_get_rsp {
+	struct {
+		__u32 threads:1;
+	} _present;
+
+	__u32 threads;
+};
+
+void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp);
+
+/*
+ * get the number of running threads
+ */
+struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys);
+
 #endif /* _LINUX_NFSD_GEN_H */
-- 
2.41.0

[PATCH v4 2/3] NFSD: convert write_version to netlink command

[Lorenzo Bianconi]

Introduce write_version netlink command similar to the ones available
through the procfs.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/netlink/specs/nfsd.yaml |  32 ++++++++
 fs/nfsd/netlink.c                     |  19 +++++
 fs/nfsd/netlink.h                     |   3 +
 fs/nfsd/nfsctl.c                      | 105 ++++++++++++++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  11 +++
 tools/net/ynl/generated/nfsd-user.c   |  81 ++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   |  55 ++++++++++++++
 7 files changed, 306 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index c92e1425d316..6c5e42bb20f6 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -68,6 +68,18 @@ attribute-sets:
       -
         name: threads
         type: u32
+  -
+    name: server-version
+    attributes:
+      -
+        name: major
+        type: u32
+      -
+        name: minor
+        type: u32
+      -
+        name: status
+        type: u8
 
 operations:
   list:
@@ -110,3 +122,23 @@ operations:
         reply:
           attributes:
             - threads
+    -
+      name: version-set
+      doc: enable/disable server version
+      attribute-set: server-version
+      flags: [ admin-perm ]
+      do:
+        request:
+          attributes:
+            - major
+            - minor
+            - status
+    -
+      name: version-get
+      doc: dump server versions
+      attribute-set: server-version
+      dump:
+        reply:
+          attributes:
+            - major
+            - minor
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 1a59a8e6c7e2..0608a7bd193b 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -15,6 +15,13 @@ static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_T
 	[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_VERSION_STATUS + 1] = {
+	[NFSD_A_SERVER_VERSION_MAJOR] = { .type = NLA_U32, },
+	[NFSD_A_SERVER_VERSION_MINOR] = { .type = NLA_U32, },
+	[NFSD_A_SERVER_VERSION_STATUS] = { .type = NLA_U8, },
+};
+
 /* Ops table for nfsd */
 static const struct genl_split_ops nfsd_nl_ops[] = {
 	{
@@ -36,6 +43,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_VERSION_STATUS,
+		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd	= NFSD_CMD_VERSION_GET,
+		.dumpit	= nfsd_nl_version_get_dumpit,
+		.flags	= GENL_CMD_CAP_DUMP,
+	},
 };
 
 struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index 4137fac477e4..7d203cec08e4 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -18,6 +18,9 @@ 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_dumpit(struct sk_buff *skb,
+			       struct netlink_callback *cb);
 
 extern struct genl_family nfsd_nl_family;
 
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 0d0394887506..5cf6238617d9 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1751,6 +1751,111 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+/**
+ * nfsd_nl_version_set_doit - enable/disable the provided nfs server version
+ * @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)
+{
+	struct nfsd_net *nn = net_generic(genl_info_net(info), nfsd_net_id);
+	enum vers_op cmd;
+	u32 major, minor;
+	u8 status;
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_VERSION_MAJOR) ||
+	    GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_VERSION_MINOR) ||
+	    GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_VERSION_STATUS))
+		return -EINVAL;
+
+	major = nla_get_u32(info->attrs[NFSD_A_SERVER_VERSION_MAJOR]);
+	minor = nla_get_u32(info->attrs[NFSD_A_SERVER_VERSION_MINOR]);
+
+	status = nla_get_u32(info->attrs[NFSD_A_SERVER_VERSION_STATUS]);
+	cmd = !!status ? NFSD_SET : NFSD_CLEAR;
+
+	mutex_lock(&nfsd_mutex);
+	switch (major) {
+	case 4:
+		ret = nfsd_minorversion(nn, minor, cmd);
+		break;
+	case 2:
+	case 3:
+		if (!minor) {
+			ret = nfsd_vers(nn, major, cmd);
+			break;
+		}
+		fallthrough;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
+}
+
+/**
+ * nfsd_nl_version_get_doit - Handle verion_get dumpit
+ * @skb: reply buffer
+ * @cb: netlink metadata and command arguments
+ *
+ * Returns the size of the reply or a negative errno.
+ */
+int nfsd_nl_version_get_dumpit(struct sk_buff *skb,
+			       struct netlink_callback *cb)
+{
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i, ret = -ENOMEM;
+
+	mutex_lock(&nfsd_mutex);
+
+	for (i = 2; i <= 4; i++) {
+		int j;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		if (!nfsd_vers(nn, i, NFSD_AVAIL))
+			continue;
+
+		for (j = 0; j <= NFSD_SUPPORTED_MINOR_VERSION; j++) {
+			void *hdr;
+
+			if (!nfsd_vers(nn, i, NFSD_TEST))
+				continue;
+
+			/* NFSv{2,3} does not support minor numbers */
+			if (i < 4 && j)
+				continue;
+
+			if (i == 4 && !nfsd_minorversion(nn, j, NFSD_TEST))
+				continue;
+
+			hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+					  cb->nlh->nlmsg_seq, &nfsd_nl_family,
+					  0, NFSD_CMD_VERSION_GET);
+			if (!hdr)
+				goto out;
+
+			if (nla_put_u32(skb, NFSD_A_SERVER_VERSION_MAJOR, i) ||
+			    nla_put_u32(skb, NFSD_A_SERVER_VERSION_MINOR, j))
+				goto out;
+
+			genlmsg_end(skb, hdr);
+		}
+	}
+	cb->args[0] = i;
+	ret = skb->len;
+out:
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
+}
+
 /**
  * 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 99f7855852a1..51f078b26af4 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -36,10 +36,21 @@ enum {
 	NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
 };
 
+enum {
+	NFSD_A_SERVER_VERSION_MAJOR = 1,
+	NFSD_A_SERVER_VERSION_MINOR,
+	NFSD_A_SERVER_VERSION_STATUS,
+
+	__NFSD_A_SERVER_VERSION_MAX,
+	NFSD_A_SERVER_VERSION_MAX = (__NFSD_A_SERVER_VERSION_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)
diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
index 342a00b0474a..dceaccbdfb69 100644
--- a/tools/net/ynl/generated/nfsd-user.c
+++ b/tools/net/ynl/generated/nfsd-user.c
@@ -17,6 +17,8 @@ static const char * const nfsd_op_strmap[] = {
 	[NFSD_CMD_RPC_STATUS_GET] = "rpc-status-get",
 	[NFSD_CMD_THREADS_SET] = "threads-set",
 	[NFSD_CMD_THREADS_GET] = "threads-get",
+	[NFSD_CMD_VERSION_SET] = "version-set",
+	[NFSD_CMD_VERSION_GET] = "version-get",
 };
 
 const char *nfsd_op_str(int op)
@@ -58,6 +60,17 @@ struct ynl_policy_nest nfsd_server_worker_nest = {
 	.table = nfsd_server_worker_policy,
 };
 
+struct ynl_policy_attr nfsd_server_version_policy[NFSD_A_SERVER_VERSION_MAX + 1] = {
+	[NFSD_A_SERVER_VERSION_MAJOR] = { .name = "major", .type = YNL_PT_U32, },
+	[NFSD_A_SERVER_VERSION_MINOR] = { .name = "minor", .type = YNL_PT_U32, },
+	[NFSD_A_SERVER_VERSION_STATUS] = { .name = "status", .type = YNL_PT_U8, },
+};
+
+struct ynl_policy_nest nfsd_server_version_nest = {
+	.max_attr = NFSD_A_SERVER_VERSION_MAX,
+	.table = nfsd_server_version_policy,
+};
+
 /* Common nested types */
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
@@ -182,6 +195,74 @@ struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys)
 	return NULL;
 }
 
+/* ============== NFSD_CMD_VERSION_SET ============== */
+/* NFSD_CMD_VERSION_SET - do */
+void nfsd_version_set_req_free(struct nfsd_version_set_req *req)
+{
+	free(req);
+}
+
+int nfsd_version_set(struct ynl_sock *ys, struct nfsd_version_set_req *req)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_VERSION_SET, 1);
+	ys->req_policy = &nfsd_server_version_nest;
+
+	if (req->_present.major)
+		mnl_attr_put_u32(nlh, NFSD_A_SERVER_VERSION_MAJOR, req->major);
+	if (req->_present.minor)
+		mnl_attr_put_u32(nlh, NFSD_A_SERVER_VERSION_MINOR, req->minor);
+	if (req->_present.status)
+		mnl_attr_put_u8(nlh, NFSD_A_SERVER_VERSION_STATUS, req->status);
+
+	err = ynl_exec(ys, nlh, NULL);
+	if (err < 0)
+		return -1;
+
+	return 0;
+}
+
+/* ============== NFSD_CMD_VERSION_GET ============== */
+/* NFSD_CMD_VERSION_GET - dump */
+void nfsd_version_get_list_free(struct nfsd_version_get_list *rsp)
+{
+	struct nfsd_version_get_list *next = rsp;
+
+	while ((void *)next != YNL_LIST_END) {
+		rsp = next;
+		next = rsp->next;
+
+		free(rsp);
+	}
+}
+
+struct nfsd_version_get_list *nfsd_version_get_dump(struct ynl_sock *ys)
+{
+	struct ynl_dump_state yds = {};
+	struct nlmsghdr *nlh;
+	int err;
+
+	yds.ys = ys;
+	yds.alloc_sz = sizeof(struct nfsd_version_get_list);
+	yds.cb = nfsd_version_get_rsp_parse;
+	yds.rsp_cmd = NFSD_CMD_VERSION_GET;
+	yds.rsp_policy = &nfsd_server_version_nest;
+
+	nlh = ynl_gemsg_start_dump(ys, ys->family_id, NFSD_CMD_VERSION_GET, 1);
+
+	err = ynl_exec_dump(ys, nlh, &yds);
+	if (err < 0)
+		goto free_list;
+
+	return yds.first;
+
+free_list:
+	nfsd_version_get_list_free(yds.first);
+	return NULL;
+}
+
 const struct ynl_family ynl_nfsd_family =  {
 	.name		= "nfsd",
 };
diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
index 4c11119217f1..2272cb25c364 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -77,4 +77,59 @@ void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp);
  */
 struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys);
 
+/* ============== NFSD_CMD_VERSION_SET ============== */
+/* NFSD_CMD_VERSION_SET - do */
+struct nfsd_version_set_req {
+	struct {
+		__u32 major:1;
+		__u32 minor:1;
+		__u32 status:1;
+	} _present;
+
+	__u32 major;
+	__u32 minor;
+	__u8 status;
+};
+
+static inline struct nfsd_version_set_req *nfsd_version_set_req_alloc(void)
+{
+	return calloc(1, sizeof(struct nfsd_version_set_req));
+}
+void nfsd_version_set_req_free(struct nfsd_version_set_req *req);
+
+static inline void
+nfsd_version_set_req_set_major(struct nfsd_version_set_req *req, __u32 major)
+{
+	req->_present.major = 1;
+	req->major = major;
+}
+static inline void
+nfsd_version_set_req_set_minor(struct nfsd_version_set_req *req, __u32 minor)
+{
+	req->_present.minor = 1;
+	req->minor = minor;
+}
+static inline void
+nfsd_version_set_req_set_status(struct nfsd_version_set_req *req, __u8 status)
+{
+	req->_present.status = 1;
+	req->status = status;
+}
+
+/*
+ * enable/disable server version
+ */
+int nfsd_version_set(struct ynl_sock *ys, struct nfsd_version_set_req *req);
+
+/* ============== NFSD_CMD_VERSION_GET ============== */
+/* NFSD_CMD_VERSION_GET - dump */
+struct nfsd_version_get_list {
+	struct nfsd_version_get_list *next;
+	struct nfsd_version_get_rsp obj __attribute__ ((aligned (8)));
+};
+
+void nfsd_version_get_list_free(struct nfsd_version_get_list *rsp);
+
+struct nfsd_version_get_list *nfsd_version_get_dump(struct ynl_sock *ys);
+
 #endif /* _LINUX_NFSD_GEN_H */
-- 
2.41.0

[PATCH v4 3/3] NFSD: convert write_ports to netlink command

[Lorenzo Bianconi]

Introduce write_ports netlink command similar to the ones available
through the procfs.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/netlink/specs/nfsd.yaml |  28 +++++++
 fs/nfsd/netlink.c                     |  18 +++++
 fs/nfsd/netlink.h                     |   3 +
 fs/nfsd/nfsctl.c                      | 104 ++++++++++++++++++++++++--
 include/uapi/linux/nfsd_netlink.h     |  10 +++
 tools/net/ynl/generated/nfsd-user.c   |  81 ++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   |  54 +++++++++++++
 7 files changed, 291 insertions(+), 7 deletions(-)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index 6c5e42bb20f6..1c342ad3c5fa 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -80,6 +80,15 @@ attribute-sets:
       -
         name: status
         type: u8
+  -
+    name: server-listener
+    attributes:
+      -
+        name: transport-name
+        type: string
+      -
+        name: port
+        type: u32
 
 operations:
   list:
@@ -142,3 +151,22 @@ operations:
           attributes:
             - major
             - minor
+    -
+      name: listener-start
+      doc: start server listener
+      attribute-set: server-listener
+      flags: [ admin-perm ]
+      do:
+        request:
+          attributes:
+            - transport-name
+            - port
+    -
+      name: listener-get
+      doc: dump server listeners
+      attribute-set: server-listener
+      dump:
+        reply:
+          attributes:
+            - transport-name
+            - port
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 0608a7bd193b..cd51393ede72 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -22,6 +22,12 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_VERSION_
 	[NFSD_A_SERVER_VERSION_STATUS] = { .type = NLA_U8, },
 };
 
+/* NFSD_CMD_LISTENER_START - do */
+static const struct nla_policy nfsd_listener_start_nl_policy[NFSD_A_SERVER_LISTENER_PORT + 1] = {
+	[NFSD_A_SERVER_LISTENER_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
+	[NFSD_A_SERVER_LISTENER_PORT] = { .type = NLA_U32, },
+};
+
 /* Ops table for nfsd */
 static const struct genl_split_ops nfsd_nl_ops[] = {
 	{
@@ -55,6 +61,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
 		.dumpit	= nfsd_nl_version_get_dumpit,
 		.flags	= GENL_CMD_CAP_DUMP,
 	},
+	{
+		.cmd		= NFSD_CMD_LISTENER_START,
+		.doit		= nfsd_nl_listener_start_doit,
+		.policy		= nfsd_listener_start_nl_policy,
+		.maxattr	= NFSD_A_SERVER_LISTENER_PORT,
+		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd	= NFSD_CMD_LISTENER_GET,
+		.dumpit	= nfsd_nl_listener_get_dumpit,
+		.flags	= GENL_CMD_CAP_DUMP,
+	},
 };
 
 struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index 7d203cec08e4..9a51cb83f343 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -21,6 +21,9 @@ 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_dumpit(struct sk_buff *skb,
 			       struct netlink_callback *cb);
+int nfsd_nl_listener_start_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_listener_get_dumpit(struct sk_buff *skb,
+				struct netlink_callback *cb);
 
 extern struct genl_family nfsd_nl_family;
 
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 5cf6238617d9..471021b92519 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -717,18 +717,16 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
  * A transport listener is added by writing its transport name and
  * a port number.
  */
-static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cred *cred)
+static ssize_t ___write_ports_addxprt(struct net *net, const struct cred *cred,
+				      const char *transport, const int port)
 {
-	char transport[16];
-	struct svc_xprt *xprt;
-	int port, err;
 	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
-
-	if (sscanf(buf, "%15s %5u", transport, &port) != 2)
-		return -EINVAL;
+	struct svc_xprt *xprt;
+	int err;
 
 	if (port < 1 || port > USHRT_MAX)
 		return -EINVAL;
+
 	trace_nfsd_ctl_ports_addxprt(net, transport, port);
 
 	err = nfsd_create_serv(net);
@@ -761,6 +759,17 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
 	return err;
 }
 
+static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cred *cred)
+{
+	char transport[16];
+	int port;
+
+	if (sscanf(buf, "%15s %5u", transport, &port) != 2)
+		return -EINVAL;
+
+	return ___write_ports_addxprt(net, cred, transport, port);
+}
+
 static ssize_t __write_ports(struct file *file, char *buf, size_t size,
 			     struct net *net)
 {
@@ -1856,6 +1865,87 @@ int nfsd_nl_version_get_dumpit(struct sk_buff *skb,
 	return ret;
 }
 
+/**
+ * nfsd_nl_listener_start_doit - start the provided nfs server listener
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_listener_start_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME) ||
+	    GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_PORT))
+		return -EINVAL;
+
+	mutex_lock(&nfsd_mutex);
+	ret = ___write_ports_addxprt(genl_info_net(info), get_current_cred(),
+			nla_data(info->attrs[NFSD_A_SERVER_LISTENER_TRANSPORT_NAME]),
+			nla_get_u32(info->attrs[NFSD_A_SERVER_LISTENER_PORT]));
+	mutex_unlock(&nfsd_mutex);
+
+	return 0;
+}
+
+/**
+ * nfsd_nl_version_get_dumpit - Handle listener_get dumpit
+ * @skb: reply buffer
+ * @cb: netlink metadata and command arguments
+ *
+ * Returns the size of the reply or a negative errno.
+ */
+int nfsd_nl_listener_get_dumpit(struct sk_buff *skb,
+				struct netlink_callback *cb)
+{
+	struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
+	int i = 0, ret = -ENOMEM;
+	struct svc_xprt *xprt;
+	struct svc_serv *serv;
+
+	mutex_lock(&nfsd_mutex);
+
+	serv = nn->nfsd_serv;
+	if (!serv) {
+		mutex_unlock(&nfsd_mutex);
+		return 0;
+	}
+
+	spin_lock_bh(&serv->sv_lock);
+	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
+		void *hdr;
+
+		if (i < cb->args[0]) /* already consumed */
+			continue;
+
+		hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
+				  cb->nlh->nlmsg_seq, &nfsd_nl_family,
+				  0, NFSD_CMD_LISTENER_GET);
+		if (!hdr)
+			goto out;
+
+		if (nla_put_string(skb, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME,
+				   xprt->xpt_class->xcl_name))
+			goto out;
+
+		if (nla_put_u32(skb, NFSD_A_SERVER_LISTENER_PORT,
+				svc_xprt_local_port(xprt)))
+			goto out;
+
+		genlmsg_end(skb, hdr);
+		i++;
+	}
+	cb->args[0] = i;
+	ret = skb->len;
+out:
+	spin_unlock_bh(&serv->sv_lock);
+
+	mutex_unlock(&nfsd_mutex);
+
+	return ret;
+}
+
 /**
  * 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 51f078b26af4..2690bab6cef2 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -45,12 +45,22 @@ enum {
 	NFSD_A_SERVER_VERSION_MAX = (__NFSD_A_SERVER_VERSION_MAX - 1)
 };
 
+enum {
+	NFSD_A_SERVER_LISTENER_TRANSPORT_NAME = 1,
+	NFSD_A_SERVER_LISTENER_PORT,
+
+	__NFSD_A_SERVER_LISTENER_MAX,
+	NFSD_A_SERVER_LISTENER_MAX = (__NFSD_A_SERVER_LISTENER_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_START,
+	NFSD_CMD_LISTENER_GET,
 
 	__NFSD_CMD_MAX,
 	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
index dceaccbdfb69..8a7cef300b9b 100644
--- a/tools/net/ynl/generated/nfsd-user.c
+++ b/tools/net/ynl/generated/nfsd-user.c
@@ -19,6 +19,8 @@ static const char * const nfsd_op_strmap[] = {
 	[NFSD_CMD_THREADS_GET] = "threads-get",
 	[NFSD_CMD_VERSION_SET] = "version-set",
 	[NFSD_CMD_VERSION_GET] = "version-get",
+	[NFSD_CMD_LISTENER_START] = "listener-start",
+	[NFSD_CMD_LISTENER_GET] = "listener-get",
 };
 
 const char *nfsd_op_str(int op)
@@ -71,6 +73,16 @@ struct ynl_policy_nest nfsd_server_version_nest = {
 	.table = nfsd_server_version_policy,
 };
 
+struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
+	[NFSD_A_SERVER_LISTENER_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
+	[NFSD_A_SERVER_LISTENER_PORT] = { .name = "port", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest nfsd_server_listener_nest = {
+	.max_attr = NFSD_A_SERVER_LISTENER_MAX,
+	.table = nfsd_server_listener_policy,
+};
+
 /* Common nested types */
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
@@ -263,6 +275,75 @@ struct nfsd_version_get_list *nfsd_version_get_dump(struct ynl_sock *ys)
 	return NULL;
 }
 
+/* ============== NFSD_CMD_LISTENER_START ============== */
+/* NFSD_CMD_LISTENER_START - do */
+void nfsd_listener_start_req_free(struct nfsd_listener_start_req *req)
+{
+	free(req->transport_name);
+	free(req);
+}
+
+int nfsd_listener_start(struct ynl_sock *ys,
+			struct nfsd_listener_start_req *req)
+{
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_START, 1);
+	ys->req_policy = &nfsd_server_listener_nest;
+
+	if (req->_present.transport_name_len)
+		mnl_attr_put_strz(nlh, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME, req->transport_name);
+	if (req->_present.port)
+		mnl_attr_put_u32(nlh, NFSD_A_SERVER_LISTENER_PORT, req->port);
+
+	err = ynl_exec(ys, nlh, NULL);
+	if (err < 0)
+		return -1;
+
+	return 0;
+}
+
+/* ============== NFSD_CMD_LISTENER_GET ============== */
+/* NFSD_CMD_LISTENER_GET - dump */
+void nfsd_listener_get_list_free(struct nfsd_listener_get_list *rsp)
+{
+	struct nfsd_listener_get_list *next = rsp;
+
+	while ((void *)next != YNL_LIST_END) {
+		rsp = next;
+		next = rsp->next;
+
+		free(rsp->obj.transport_name);
+		free(rsp);
+	}
+}
+
+struct nfsd_listener_get_list *nfsd_listener_get_dump(struct ynl_sock *ys)
+{
+	struct ynl_dump_state yds = {};
+	struct nlmsghdr *nlh;
+	int err;
+
+	yds.ys = ys;
+	yds.alloc_sz = sizeof(struct nfsd_listener_get_list);
+	yds.cb = nfsd_listener_get_rsp_parse;
+	yds.rsp_cmd = NFSD_CMD_LISTENER_GET;
+	yds.rsp_policy = &nfsd_server_listener_nest;
+
+	nlh = ynl_gemsg_start_dump(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
+
+	err = ynl_exec_dump(ys, nlh, &yds);
+	if (err < 0)
+		goto free_list;
+
+	return yds.first;
+
+free_list:
+	nfsd_listener_get_list_free(yds.first);
+	return NULL;
+}
+
 const struct ynl_family ynl_nfsd_family =  {
 	.name		= "nfsd",
 };
diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
index 2272cb25c364..c757387cd60d 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -132,4 +132,58 @@ void nfsd_version_get_list_free(struct nfsd_version_get_list *rsp);
 
 struct nfsd_version_get_list *nfsd_version_get_dump(struct ynl_sock *ys);
 
+/* ============== NFSD_CMD_LISTENER_START ============== */
+/* NFSD_CMD_LISTENER_START - do */
+struct nfsd_listener_start_req {
+	struct {
+		__u32 transport_name_len;
+		__u32 port:1;
+	} _present;
+
+	char *transport_name;
+	__u32 port;
+};
+
+static inline struct nfsd_listener_start_req *
+nfsd_listener_start_req_alloc(void)
+{
+	return calloc(1, sizeof(struct nfsd_listener_start_req));
+}
+void nfsd_listener_start_req_free(struct nfsd_listener_start_req *req);
+
+static inline void
+nfsd_listener_start_req_set_transport_name(struct nfsd_listener_start_req *req,
+					   const char *transport_name)
+{
+	free(req->transport_name);
+	req->_present.transport_name_len = strlen(transport_name);
+	req->transport_name = malloc(req->_present.transport_name_len + 1);
+	memcpy(req->transport_name, transport_name, req->_present.transport_name_len);
+	req->transport_name[req->_present.transport_name_len] = 0;
+}
+static inline void
+nfsd_listener_start_req_set_port(struct nfsd_listener_start_req *req,
+				 __u32 port)
+{
+	req->_present.port = 1;
+	req->port = port;
+}
+
+/*
+ * start server listener
+ */
+int nfsd_listener_start(struct ynl_sock *ys,
+			struct nfsd_listener_start_req *req);
+
+/* ============== NFSD_CMD_LISTENER_GET ============== */
+/* NFSD_CMD_LISTENER_GET - dump */
+struct nfsd_listener_get_list {
+	struct nfsd_listener_get_list *next;
+	struct nfsd_listener_get_rsp obj __attribute__ ((aligned (8)));
+};
+
+void nfsd_listener_get_list_free(struct nfsd_listener_get_list *rsp);
+
+struct nfsd_listener_get_list *nfsd_listener_get_dump(struct ynl_sock *ys);
+
 #endif /* _LINUX_NFSD_GEN_H */
-- 
2.41.0

Re: [PATCH v4 2/3] NFSD: convert write_version to netlink command

[kernel test robot]

Hi Lorenzo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20231103]
[cannot apply to trondmy-nfs/linux-next v6.6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Bianconi/NFSD-convert-write_threads-to-netlink-command/20231104-202515
base:   linus/master
patch link:    https://lore.kernel.org/r/3785da26e14c13e194510eaad9c6bd846d691d5f.1699095665.git.lorenzo%40kernel.org
patch subject: [PATCH v4 2/3] NFSD: convert write_version to netlink command
config: alpha-defconfig (https://download.01.org/0day-ci/archive/20231104/202311042320.nQOTYxhs-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311042320.nQOTYxhs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311042320.nQOTYxhs-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/nfsd/nfsctl.c:1810: warning: expecting prototype for nfsd_nl_version_get_doit(). Prototype was for nfsd_nl_version_get_dumpit() instead


vim +1810 fs/nfsd/nfsctl.c

  1800	
  1801	/**
  1802	 * nfsd_nl_version_get_doit - Handle verion_get dumpit
  1803	 * @skb: reply buffer
  1804	 * @cb: netlink metadata and command arguments
  1805	 *
  1806	 * Returns the size of the reply or a negative errno.
  1807	 */
  1808	int nfsd_nl_version_get_dumpit(struct sk_buff *skb,
  1809				       struct netlink_callback *cb)
> 1810	{
  1811		struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
  1812		int i, ret = -ENOMEM;
  1813	
  1814		mutex_lock(&nfsd_mutex);
  1815	
  1816		for (i = 2; i <= 4; i++) {
  1817			int j;
  1818	
  1819			if (i < cb->args[0]) /* already consumed */
  1820				continue;
  1821	
  1822			if (!nfsd_vers(nn, i, NFSD_AVAIL))
  1823				continue;
  1824	
  1825			for (j = 0; j <= NFSD_SUPPORTED_MINOR_VERSION; j++) {
  1826				void *hdr;
  1827	
  1828				if (!nfsd_vers(nn, i, NFSD_TEST))
  1829					continue;
  1830	
  1831				/* NFSv{2,3} does not support minor numbers */
  1832				if (i < 4 && j)
  1833					continue;
  1834	
  1835				if (i == 4 && !nfsd_minorversion(nn, j, NFSD_TEST))
  1836					continue;
  1837	
  1838				hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
  1839						  cb->nlh->nlmsg_seq, &nfsd_nl_family,
  1840						  0, NFSD_CMD_VERSION_GET);
  1841				if (!hdr)
  1842					goto out;
  1843	
  1844				if (nla_put_u32(skb, NFSD_A_SERVER_VERSION_MAJOR, i) ||
  1845				    nla_put_u32(skb, NFSD_A_SERVER_VERSION_MINOR, j))
  1846					goto out;
  1847	
  1848				genlmsg_end(skb, hdr);
  1849			}
  1850		}
  1851		cb->args[0] = i;
  1852		ret = skb->len;
  1853	out:
  1854		mutex_unlock(&nfsd_mutex);
  1855	
  1856		return ret;
  1857	}
  1858	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 3/3] NFSD: convert write_ports to netlink command

[kernel test robot]

Hi Lorenzo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on next-20231103]
[cannot apply to trondmy-nfs/linux-next v6.6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Bianconi/NFSD-convert-write_threads-to-netlink-command/20231104-202515
base:   linus/master
patch link:    https://lore.kernel.org/r/153b94db12b5c8fff270706673afffad5d84938c.1699095665.git.lorenzo%40kernel.org
patch subject: [PATCH v4 3/3] NFSD: convert write_ports to netlink command
config: alpha-defconfig (https://download.01.org/0day-ci/archive/20231105/202311050409.dPLvgiwN-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231105/202311050409.dPLvgiwN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311050409.dPLvgiwN-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/nfsd/nfsctl.c: In function 'nfsd_nl_listener_start_doit':
>> fs/nfsd/nfsctl.c:1877:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
    1877 |         int ret;
         |             ^~~
--
   fs/nfsd/nfsctl.c:1819: warning: expecting prototype for nfsd_nl_version_get_doit(). Prototype was for nfsd_nl_version_get_dumpit() instead
>> fs/nfsd/nfsctl.c:1901: warning: expecting prototype for nfsd_nl_version_get_dumpit(). Prototype was for nfsd_nl_listener_get_dumpit() instead


vim +/ret +1877 fs/nfsd/nfsctl.c

  1867	
  1868	/**
  1869	 * nfsd_nl_listener_start_doit - start the provided nfs server listener
  1870	 * @skb: reply buffer
  1871	 * @info: netlink metadata and command arguments
  1872	 *
  1873	 * Return 0 on success or a negative errno.
  1874	 */
  1875	int nfsd_nl_listener_start_doit(struct sk_buff *skb, struct genl_info *info)
  1876	{
> 1877		int ret;
  1878	
  1879		if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME) ||
  1880		    GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_PORT))
  1881			return -EINVAL;
  1882	
  1883		mutex_lock(&nfsd_mutex);
  1884		ret = ___write_ports_addxprt(genl_info_net(info), get_current_cred(),
  1885				nla_data(info->attrs[NFSD_A_SERVER_LISTENER_TRANSPORT_NAME]),
  1886				nla_get_u32(info->attrs[NFSD_A_SERVER_LISTENER_PORT]));
  1887		mutex_unlock(&nfsd_mutex);
  1888	
  1889		return 0;
  1890	}
  1891	
  1892	/**
  1893	 * nfsd_nl_version_get_dumpit - Handle listener_get dumpit
  1894	 * @skb: reply buffer
  1895	 * @cb: netlink metadata and command arguments
  1896	 *
  1897	 * Returns the size of the reply or a negative errno.
  1898	 */
  1899	int nfsd_nl_listener_get_dumpit(struct sk_buff *skb,
  1900					struct netlink_callback *cb)
> 1901	{
  1902		struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
  1903		int i = 0, ret = -ENOMEM;
  1904		struct svc_xprt *xprt;
  1905		struct svc_serv *serv;
  1906	
  1907		mutex_lock(&nfsd_mutex);
  1908	
  1909		serv = nn->nfsd_serv;
  1910		if (!serv) {
  1911			mutex_unlock(&nfsd_mutex);
  1912			return 0;
  1913		}
  1914	
  1915		spin_lock_bh(&serv->sv_lock);
  1916		list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
  1917			void *hdr;
  1918	
  1919			if (i < cb->args[0]) /* already consumed */
  1920				continue;
  1921	
  1922			hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
  1923					  cb->nlh->nlmsg_seq, &nfsd_nl_family,
  1924					  0, NFSD_CMD_LISTENER_GET);
  1925			if (!hdr)
  1926				goto out;
  1927	
  1928			if (nla_put_string(skb, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME,
  1929					   xprt->xpt_class->xcl_name))
  1930				goto out;
  1931	
  1932			if (nla_put_u32(skb, NFSD_A_SERVER_LISTENER_PORT,
  1933					svc_xprt_local_port(xprt)))
  1934				goto out;
  1935	
  1936			genlmsg_end(skb, hdr);
  1937			i++;
  1938		}
  1939		cb->args[0] = i;
  1940		ret = skb->len;
  1941	out:
  1942		spin_unlock_bh(&serv->sv_lock);
  1943	
  1944		mutex_unlock(&nfsd_mutex);
  1945	
  1946		return ret;
  1947	}
  1948	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 1/3] NFSD: convert write_threads to netlink command

[Chuck Lever]

On Sat, Nov 04, 2023 at 12:13:45PM +0100, Lorenzo Bianconi wrote:
> Introduce write_threads netlink command similar to the ones available
> through the procfs.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  Documentation/netlink/specs/nfsd.yaml | 23 +++++++
>  fs/nfsd/netlink.c                     | 17 +++++
>  fs/nfsd/netlink.h                     |  2 +
>  fs/nfsd/nfsctl.c                      | 58 +++++++++++++++++
>  include/uapi/linux/nfsd_netlink.h     |  9 +++
>  tools/net/ynl/generated/nfsd-user.c   | 92 +++++++++++++++++++++++++++
>  tools/net/ynl/generated/nfsd-user.h   | 47 ++++++++++++++
>  7 files changed, 248 insertions(+)

Hi Lorenzo -

This doesn't apply to my private nfsd-next branch. I don't believe
that's your fault... We've got some things in flight here that
conflict with what is in net-next.

Jakub tells me there is some ynl churn coming in v6.7-rc1 that
might resolve the conflicts.

I plan to rebase my private nfsd-next on v6.7-rc1 and push the first
set of patches on Monday or Tuesday. Please rebase this series on
top of that, regen the ynl code, and then send a v5. I will then
apply that to nfsd-next for more testing and review.


> 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 739ed5bf71cd..0d0394887506 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1693,6 +1693,64 @@ 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 : ret;
> +}
> +
> +/**
> + * 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 c8ae72466ee6..99f7855852a1 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)
> diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
> index fec6828680ce..342a00b0474a 100644
> --- a/tools/net/ynl/generated/nfsd-user.c
> +++ b/tools/net/ynl/generated/nfsd-user.c
> @@ -15,6 +15,8 @@
>  /* Enums */
>  static const char * const nfsd_op_strmap[] = {
>  	[NFSD_CMD_RPC_STATUS_GET] = "rpc-status-get",
> +	[NFSD_CMD_THREADS_SET] = "threads-set",
> +	[NFSD_CMD_THREADS_GET] = "threads-get",
>  };
>  
>  const char *nfsd_op_str(int op)
> @@ -47,6 +49,15 @@ struct ynl_policy_nest nfsd_rpc_status_nest = {
>  	.table = nfsd_rpc_status_policy,
>  };
>  
> +struct ynl_policy_attr nfsd_server_worker_policy[NFSD_A_SERVER_WORKER_MAX + 1] = {
> +	[NFSD_A_SERVER_WORKER_THREADS] = { .name = "threads", .type = YNL_PT_U32, },
> +};
> +
> +struct ynl_policy_nest nfsd_server_worker_nest = {
> +	.max_attr = NFSD_A_SERVER_WORKER_MAX,
> +	.table = nfsd_server_worker_policy,
> +};
> +
>  /* Common nested types */
>  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
>  /* NFSD_CMD_RPC_STATUS_GET - dump */
> @@ -90,6 +101,87 @@ struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys)
>  	return NULL;
>  }
>  
> +/* ============== NFSD_CMD_THREADS_SET ============== */
> +/* NFSD_CMD_THREADS_SET - do */
> +void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req)
> +{
> +	free(req);
> +}
> +
> +int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req)
> +{
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_SET, 1);
> +	ys->req_policy = &nfsd_server_worker_nest;
> +
> +	if (req->_present.threads)
> +		mnl_attr_put_u32(nlh, NFSD_A_SERVER_WORKER_THREADS, req->threads);
> +
> +	err = ynl_exec(ys, nlh, NULL);
> +	if (err < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +/* ============== NFSD_CMD_THREADS_GET ============== */
> +/* NFSD_CMD_THREADS_GET - do */
> +void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp)
> +{
> +	free(rsp);
> +}
> +
> +int nfsd_threads_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> +{
> +	struct ynl_parse_arg *yarg = data;
> +	struct nfsd_threads_get_rsp *dst;
> +	const struct nlattr *attr;
> +
> +	dst = yarg->data;
> +
> +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> +		unsigned int type = mnl_attr_get_type(attr);
> +
> +		if (type == NFSD_A_SERVER_WORKER_THREADS) {
> +			if (ynl_attr_validate(yarg, attr))
> +				return MNL_CB_ERROR;
> +			dst->_present.threads = 1;
> +			dst->threads = mnl_attr_get_u32(attr);
> +		}
> +	}
> +
> +	return MNL_CB_OK;
> +}
> +
> +struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys)
> +{
> +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> +	struct nfsd_threads_get_rsp *rsp;
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_GET, 1);
> +	ys->req_policy = &nfsd_server_worker_nest;
> +	yrs.yarg.rsp_policy = &nfsd_server_worker_nest;
> +
> +	rsp = calloc(1, sizeof(*rsp));
> +	yrs.yarg.data = rsp;
> +	yrs.cb = nfsd_threads_get_rsp_parse;
> +	yrs.rsp_cmd = NFSD_CMD_THREADS_GET;
> +
> +	err = ynl_exec(ys, nlh, &yrs);
> +	if (err < 0)
> +		goto err_free;
> +
> +	return rsp;
> +
> +err_free:
> +	nfsd_threads_get_rsp_free(rsp);
> +	return NULL;
> +}
> +
>  const struct ynl_family ynl_nfsd_family =  {
>  	.name		= "nfsd",
>  };
> diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
> index b6b69501031a..4c11119217f1 100644
> --- a/tools/net/ynl/generated/nfsd-user.h
> +++ b/tools/net/ynl/generated/nfsd-user.h
> @@ -30,4 +30,51 @@ void nfsd_rpc_status_get_list_free(struct nfsd_rpc_status_get_list *rsp);
>  
>  struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys);
>  
> +/* ============== NFSD_CMD_THREADS_SET ============== */
> +/* NFSD_CMD_THREADS_SET - do */
> +struct nfsd_threads_set_req {
> +	struct {
> +		__u32 threads:1;
> +	} _present;
> +
> +	__u32 threads;
> +};
> +
> +static inline struct nfsd_threads_set_req *nfsd_threads_set_req_alloc(void)
> +{
> +	return calloc(1, sizeof(struct nfsd_threads_set_req));
> +}
> +void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req);
> +
> +static inline void
> +nfsd_threads_set_req_set_threads(struct nfsd_threads_set_req *req,
> +				 __u32 threads)
> +{
> +	req->_present.threads = 1;
> +	req->threads = threads;
> +}
> +
> +/*
> + * set the number of running threads
> + */
> +int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req);
> +
> +/* ============== NFSD_CMD_THREADS_GET ============== */
> +/* NFSD_CMD_THREADS_GET - do */
> +
> +struct nfsd_threads_get_rsp {
> +	struct {
> +		__u32 threads:1;
> +	} _present;
> +
> +	__u32 threads;
> +};
> +
> +void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp);
> +
> +/*
> + * get the number of running threads
> + */
> +struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys);
> +
>  #endif /* _LINUX_NFSD_GEN_H */
> -- 
> 2.41.0
> 

-- 
Chuck Lever

Re: [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[Jeff Layton]

On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote:
> Introduce write_threads, write_version and write_ports netlink
> commands similar to the ones available through the procfs.
> 
> 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/nfsd-netlink.git
> This series is based on the commit below available in net-next tree
> 
> commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
> Author: Jakub Kicinski <kuba@kernel.org>
> Date:   Fri Oct 6 06:50:32 2023 -0700
> 
>     tools: ynl-gen: handle do ops with no input attrs
> 
>     The code supports dumps with no input attributes currently
>     thru a combination of special-casing and luck.
>     Clean up the handling of ops with no inputs. Create empty
>     Structs, and skip printing of empty types.
>     This makes dos with no inputs work.
> 
> Lorenzo Bianconi (3):
>   NFSD: convert write_threads to netlink commands
>   NFSD: convert write_version to netlink commands
>   NFSD: convert write_ports to netlink commands
> 
>  Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
>  fs/nfsd/netlink.c                     |  54 ++++++
>  fs/nfsd/netlink.h                     |   8 +
>  fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
>  include/uapi/linux/nfsd_netlink.h     |  30 +++
>  tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
>  tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
>  7 files changed, 845 insertions(+), 7 deletions(-)
> 

Nice work, Lorenzo! Now comes the bikeshedding...

With the nfsdfs interface, we sort of had to split things up into
multiple files like this, but it has some drawbacks, in particular with
weird behavior when people do things out of order.

Would it make more sense to instead have a single netlink command that
sets up ports and versions, and then spawns the requisite amount of
threads, all in one fell swoop?

That does presuppose we can send down a variable-length frame though,
but I assume that is possible with netlink.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 1/3] NFSD: convert write_threads to netlink command

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 12507 bytes --]

> On Sat, Nov 04, 2023 at 12:13:45PM +0100, Lorenzo Bianconi wrote:
> > Introduce write_threads netlink command similar to the ones available
> > through the procfs.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  Documentation/netlink/specs/nfsd.yaml | 23 +++++++
> >  fs/nfsd/netlink.c                     | 17 +++++
> >  fs/nfsd/netlink.h                     |  2 +
> >  fs/nfsd/nfsctl.c                      | 58 +++++++++++++++++
> >  include/uapi/linux/nfsd_netlink.h     |  9 +++
> >  tools/net/ynl/generated/nfsd-user.c   | 92 +++++++++++++++++++++++++++
> >  tools/net/ynl/generated/nfsd-user.h   | 47 ++++++++++++++
> >  7 files changed, 248 insertions(+)
> 
> Hi Lorenzo -
> 
> This doesn't apply to my private nfsd-next branch. I don't believe
> that's your fault... We've got some things in flight here that
> conflict with what is in net-next.
> 
> Jakub tells me there is some ynl churn coming in v6.7-rc1 that
> might resolve the conflicts.
> 
> I plan to rebase my private nfsd-next on v6.7-rc1 and push the first
> set of patches on Monday or Tuesday. Please rebase this series on
> top of that, regen the ynl code, and then send a v5. I will then
> apply that to nfsd-next for more testing and review.

ack, will do.

Regards,
Lorenzo

> 
> 
> > 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 739ed5bf71cd..0d0394887506 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -1693,6 +1693,64 @@ 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 : ret;
> > +}
> > +
> > +/**
> > + * 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 c8ae72466ee6..99f7855852a1 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)
> > diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
> > index fec6828680ce..342a00b0474a 100644
> > --- a/tools/net/ynl/generated/nfsd-user.c
> > +++ b/tools/net/ynl/generated/nfsd-user.c
> > @@ -15,6 +15,8 @@
> >  /* Enums */
> >  static const char * const nfsd_op_strmap[] = {
> >  	[NFSD_CMD_RPC_STATUS_GET] = "rpc-status-get",
> > +	[NFSD_CMD_THREADS_SET] = "threads-set",
> > +	[NFSD_CMD_THREADS_GET] = "threads-get",
> >  };
> >  
> >  const char *nfsd_op_str(int op)
> > @@ -47,6 +49,15 @@ struct ynl_policy_nest nfsd_rpc_status_nest = {
> >  	.table = nfsd_rpc_status_policy,
> >  };
> >  
> > +struct ynl_policy_attr nfsd_server_worker_policy[NFSD_A_SERVER_WORKER_MAX + 1] = {
> > +	[NFSD_A_SERVER_WORKER_THREADS] = { .name = "threads", .type = YNL_PT_U32, },
> > +};
> > +
> > +struct ynl_policy_nest nfsd_server_worker_nest = {
> > +	.max_attr = NFSD_A_SERVER_WORKER_MAX,
> > +	.table = nfsd_server_worker_policy,
> > +};
> > +
> >  /* Common nested types */
> >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> > @@ -90,6 +101,87 @@ struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys)
> >  	return NULL;
> >  }
> >  
> > +/* ============== NFSD_CMD_THREADS_SET ============== */
> > +/* NFSD_CMD_THREADS_SET - do */
> > +void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req)
> > +{
> > +	free(req);
> > +}
> > +
> > +int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req)
> > +{
> > +	struct nlmsghdr *nlh;
> > +	int err;
> > +
> > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_SET, 1);
> > +	ys->req_policy = &nfsd_server_worker_nest;
> > +
> > +	if (req->_present.threads)
> > +		mnl_attr_put_u32(nlh, NFSD_A_SERVER_WORKER_THREADS, req->threads);
> > +
> > +	err = ynl_exec(ys, nlh, NULL);
> > +	if (err < 0)
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> > +/* ============== NFSD_CMD_THREADS_GET ============== */
> > +/* NFSD_CMD_THREADS_GET - do */
> > +void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp)
> > +{
> > +	free(rsp);
> > +}
> > +
> > +int nfsd_threads_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> > +{
> > +	struct ynl_parse_arg *yarg = data;
> > +	struct nfsd_threads_get_rsp *dst;
> > +	const struct nlattr *attr;
> > +
> > +	dst = yarg->data;
> > +
> > +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > +		unsigned int type = mnl_attr_get_type(attr);
> > +
> > +		if (type == NFSD_A_SERVER_WORKER_THREADS) {
> > +			if (ynl_attr_validate(yarg, attr))
> > +				return MNL_CB_ERROR;
> > +			dst->_present.threads = 1;
> > +			dst->threads = mnl_attr_get_u32(attr);
> > +		}
> > +	}
> > +
> > +	return MNL_CB_OK;
> > +}
> > +
> > +struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys)
> > +{
> > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > +	struct nfsd_threads_get_rsp *rsp;
> > +	struct nlmsghdr *nlh;
> > +	int err;
> > +
> > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_THREADS_GET, 1);
> > +	ys->req_policy = &nfsd_server_worker_nest;
> > +	yrs.yarg.rsp_policy = &nfsd_server_worker_nest;
> > +
> > +	rsp = calloc(1, sizeof(*rsp));
> > +	yrs.yarg.data = rsp;
> > +	yrs.cb = nfsd_threads_get_rsp_parse;
> > +	yrs.rsp_cmd = NFSD_CMD_THREADS_GET;
> > +
> > +	err = ynl_exec(ys, nlh, &yrs);
> > +	if (err < 0)
> > +		goto err_free;
> > +
> > +	return rsp;
> > +
> > +err_free:
> > +	nfsd_threads_get_rsp_free(rsp);
> > +	return NULL;
> > +}
> > +
> >  const struct ynl_family ynl_nfsd_family =  {
> >  	.name		= "nfsd",
> >  };
> > diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
> > index b6b69501031a..4c11119217f1 100644
> > --- a/tools/net/ynl/generated/nfsd-user.h
> > +++ b/tools/net/ynl/generated/nfsd-user.h
> > @@ -30,4 +30,51 @@ void nfsd_rpc_status_get_list_free(struct nfsd_rpc_status_get_list *rsp);
> >  
> >  struct nfsd_rpc_status_get_list *nfsd_rpc_status_get_dump(struct ynl_sock *ys);
> >  
> > +/* ============== NFSD_CMD_THREADS_SET ============== */
> > +/* NFSD_CMD_THREADS_SET - do */
> > +struct nfsd_threads_set_req {
> > +	struct {
> > +		__u32 threads:1;
> > +	} _present;
> > +
> > +	__u32 threads;
> > +};
> > +
> > +static inline struct nfsd_threads_set_req *nfsd_threads_set_req_alloc(void)
> > +{
> > +	return calloc(1, sizeof(struct nfsd_threads_set_req));
> > +}
> > +void nfsd_threads_set_req_free(struct nfsd_threads_set_req *req);
> > +
> > +static inline void
> > +nfsd_threads_set_req_set_threads(struct nfsd_threads_set_req *req,
> > +				 __u32 threads)
> > +{
> > +	req->_present.threads = 1;
> > +	req->threads = threads;
> > +}
> > +
> > +/*
> > + * set the number of running threads
> > + */
> > +int nfsd_threads_set(struct ynl_sock *ys, struct nfsd_threads_set_req *req);
> > +
> > +/* ============== NFSD_CMD_THREADS_GET ============== */
> > +/* NFSD_CMD_THREADS_GET - do */
> > +
> > +struct nfsd_threads_get_rsp {
> > +	struct {
> > +		__u32 threads:1;
> > +	} _present;
> > +
> > +	__u32 threads;
> > +};
> > +
> > +void nfsd_threads_get_rsp_free(struct nfsd_threads_get_rsp *rsp);
> > +
> > +/*
> > + * get the number of running threads
> > + */
> > +struct nfsd_threads_get_rsp *nfsd_threads_get(struct ynl_sock *ys);
> > +
> >  #endif /* _LINUX_NFSD_GEN_H */
> > -- 
> > 2.41.0
> > 
> 
> -- 
> Chuck Lever

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 3021 bytes --]

> On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote:
> > Introduce write_threads, write_version and write_ports netlink
> > commands similar to the ones available through the procfs.
> > 
> > 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/nfsd-netlink.git
> > This series is based on the commit below available in net-next tree
> > 
> > commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
> > Author: Jakub Kicinski <kuba@kernel.org>
> > Date:   Fri Oct 6 06:50:32 2023 -0700
> > 
> >     tools: ynl-gen: handle do ops with no input attrs
> > 
> >     The code supports dumps with no input attributes currently
> >     thru a combination of special-casing and luck.
> >     Clean up the handling of ops with no inputs. Create empty
> >     Structs, and skip printing of empty types.
> >     This makes dos with no inputs work.
> > 
> > Lorenzo Bianconi (3):
> >   NFSD: convert write_threads to netlink commands
> >   NFSD: convert write_version to netlink commands
> >   NFSD: convert write_ports to netlink commands
> > 
> >  Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
> >  fs/nfsd/netlink.c                     |  54 ++++++
> >  fs/nfsd/netlink.h                     |   8 +
> >  fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
> >  include/uapi/linux/nfsd_netlink.h     |  30 +++
> >  tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
> >  tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
> >  7 files changed, 845 insertions(+), 7 deletions(-)
> > 
> 
> Nice work, Lorenzo! Now comes the bikeshedding...

Hi Jeff,

> 
> With the nfsdfs interface, we sort of had to split things up into
> multiple files like this, but it has some drawbacks, in particular with
> weird behavior when people do things out of order.

what do you mean with 'weird behavior'? Something not expected?

> 
> Would it make more sense to instead have a single netlink command that
> sets up ports and versions, and then spawns the requisite amount of
> threads, all in one fell swoop?

I do not have a strong opinion about it but I would say having a dedicated
set/get for each paramater allow us to have more granularity (e.g. if you want
to change just a parameter we do not need to send all of them to the kernel).
What do you think?

> 
> That does presuppose we can send down a variable-length frame though,
> but I assume that is possible with netlink.

sure, we can do it.

Regards,
Lorenzo

> -- 
> Jeff Layton <jlayton@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[Jeff Layton]

On Sun, 2023-11-12 at 11:02 +0100, Lorenzo Bianconi wrote:
> > On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote:
> > > Introduce write_threads, write_version and write_ports netlink
> > > commands similar to the ones available through the procfs.
> > > 
> > > 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/nfsd-netlink.git
> > > This series is based on the commit below available in net-next tree
> > > 
> > > commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
> > > Author: Jakub Kicinski <kuba@kernel.org>
> > > Date:   Fri Oct 6 06:50:32 2023 -0700
> > > 
> > >     tools: ynl-gen: handle do ops with no input attrs
> > > 
> > >     The code supports dumps with no input attributes currently
> > >     thru a combination of special-casing and luck.
> > >     Clean up the handling of ops with no inputs. Create empty
> > >     Structs, and skip printing of empty types.
> > >     This makes dos with no inputs work.
> > > 
> > > Lorenzo Bianconi (3):
> > >   NFSD: convert write_threads to netlink commands
> > >   NFSD: convert write_version to netlink commands
> > >   NFSD: convert write_ports to netlink commands
> > > 
> > >  Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
> > >  fs/nfsd/netlink.c                     |  54 ++++++
> > >  fs/nfsd/netlink.h                     |   8 +
> > >  fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
> > >  include/uapi/linux/nfsd_netlink.h     |  30 +++
> > >  tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
> > >  tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
> > >  7 files changed, 845 insertions(+), 7 deletions(-)
> > > 
> > 
> > Nice work, Lorenzo! Now comes the bikeshedding...
> 
> Hi Jeff,
> 
> > 
> > With the nfsdfs interface, we sort of had to split things up into
> > multiple files like this, but it has some drawbacks, in particular with
> > weird behavior when people do things out of order.
> 
> what do you mean with 'weird behavior'? Something not expected?
> 

Yeah.

For instance, if you set up sockets but never write anything to the
"threads" file, those sockets will sit around in perpetuity. Granted
most people use rpc.nfsd to start the server, so this generally doesn't
happen often, but it's always been a klunky interface regardless.

> > 
> > Would it make more sense to instead have a single netlink command that
> > sets up ports and versions, and then spawns the requisite amount of
> > threads, all in one fell swoop?
> 
> I do not have a strong opinion about it but I would say having a dedicated
> set/get for each paramater allow us to have more granularity (e.g. if you want
> to change just a parameter we do not need to send all of them to the kernel).
> What do you think?
> 

It's pretty rare to need to twiddle settings on the server while it's up
and running. Restarting the server in the face of even minor changes is
not generally a huge problem, so I don't see this as necessary.

Also, it's always been a bit hit and miss as to which settings take
immediate effect. For instance, if I (e.g.) turn off NFSv4 serving
altogether on a running server, it doesn't purge the existing NFSv4
state, but v4 RPCs would be immediately rejected. Eventually it would
time out, but it is odd.

Personally, I think this is amenable to a declarative interface:

Have userland always send down a complete description of what the server
should look like, and then the kernel can do what it needs to make that
happen (starting/stopping threads, opening/closing sockets, changing
versions served, etc.).

> > 
> > That does presuppose we can send down a variable-length frame though,
> > but I assume that is possible with netlink.
> 
> sure, we can do it.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[Chuck Lever III]

> On Nov 12, 2023, at 6:09 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Sun, 2023-11-12 at 11:02 +0100, Lorenzo Bianconi wrote:
>>> On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote:
>>>> Introduce write_threads, write_version and write_ports netlink
>>>> commands similar to the ones available through the procfs.
>>>> 
>>>> 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/nfsd-netlink.git
>>>> This series is based on the commit below available in net-next tree
>>>> 
>>>> commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
>>>> Author: Jakub Kicinski <kuba@kernel.org>
>>>> Date:   Fri Oct 6 06:50:32 2023 -0700
>>>> 
>>>>     tools: ynl-gen: handle do ops with no input attrs
>>>> 
>>>>     The code supports dumps with no input attributes currently
>>>>     thru a combination of special-casing and luck.
>>>>     Clean up the handling of ops with no inputs. Create empty
>>>>     Structs, and skip printing of empty types.
>>>>     This makes dos with no inputs work.
>>>> 
>>>> Lorenzo Bianconi (3):
>>>>   NFSD: convert write_threads to netlink commands
>>>>   NFSD: convert write_version to netlink commands
>>>>   NFSD: convert write_ports to netlink commands
>>>> 
>>>>  Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
>>>>  fs/nfsd/netlink.c                     |  54 ++++++
>>>>  fs/nfsd/netlink.h                     |   8 +
>>>>  fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
>>>>  include/uapi/linux/nfsd_netlink.h     |  30 +++
>>>>  tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
>>>>  tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
>>>>  7 files changed, 845 insertions(+), 7 deletions(-)
>>>> 
>>> 
>>> Nice work, Lorenzo! Now comes the bikeshedding...
>> 
>> Hi Jeff,
>> 
>>> 
>>> With the nfsdfs interface, we sort of had to split things up into
>>> multiple files like this, but it has some drawbacks, in particular with
>>> weird behavior when people do things out of order.
>> 
>> what do you mean with 'weird behavior'? Something not expected?
>> 
> 
> Yeah.
> 
> For instance, if you set up sockets but never write anything to the
> "threads" file, those sockets will sit around in perpetuity. Granted
> most people use rpc.nfsd to start the server, so this generally doesn't
> happen often, but it's always been a klunky interface regardless.
> 
>>> 
>>> Would it make more sense to instead have a single netlink command that
>>> sets up ports and versions, and then spawns the requisite amount of
>>> threads, all in one fell swoop?
>> 
>> I do not have a strong opinion about it but I would say having a dedicated
>> set/get for each paramater allow us to have more granularity (e.g. if you want
>> to change just a parameter we do not need to send all of them to the kernel).
>> What do you think?
>> 
> 
> It's pretty rare to need to twiddle settings on the server while it's up
> and running. Restarting the server in the face of even minor changes is
> not generally a huge problem, so I don't see this as necessary.

I don't have a problem creating a single "set" netlink command
that takes a number of optional arguments to adjust nfsd's
configuration in a single operation.

And a matching "get" command to query all of the server settings
at one time.


> Also, it's always been a bit hit and miss as to which settings take
> immediate effect. For instance, if I (e.g.) turn off NFSv4 serving
> altogether on a running server, it doesn't purge the existing NFSv4
> state, but v4 RPCs would be immediately rejected. Eventually it would
> time out, but it is odd.
> 
> Personally, I think this is amenable to a declarative interface:
> 
> Have userland always send down a complete description of what the server
> should look like, and then the kernel can do what it needs to make that
> happen (starting/stopping threads, opening/closing sockets, changing
> versions served, etc.).
> 
>>> 
>>> That does presuppose we can send down a variable-length frame though,
>>> but I assume that is possible with netlink.
>> 
>> sure, we can do it.
> -- 
> Jeff Layton <jlayton@kernel.org>


--
Chuck Lever

Re: [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[NeilBrown]

On Sun, 12 Nov 2023, Jeff Layton wrote:
> On Sun, 2023-11-12 at 11:02 +0100, Lorenzo Bianconi wrote:
> > > On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote:
> > > > Introduce write_threads, write_version and write_ports netlink
> > > > commands similar to the ones available through the procfs.
> > > > 
> > > > 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/nfsd-netlink.git
> > > > This series is based on the commit below available in net-next tree
> > > > 
> > > > commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
> > > > Author: Jakub Kicinski <kuba@kernel.org>
> > > > Date:   Fri Oct 6 06:50:32 2023 -0700
> > > > 
> > > >     tools: ynl-gen: handle do ops with no input attrs
> > > > 
> > > >     The code supports dumps with no input attributes currently
> > > >     thru a combination of special-casing and luck.
> > > >     Clean up the handling of ops with no inputs. Create empty
> > > >     Structs, and skip printing of empty types.
> > > >     This makes dos with no inputs work.
> > > > 
> > > > Lorenzo Bianconi (3):
> > > >   NFSD: convert write_threads to netlink commands
> > > >   NFSD: convert write_version to netlink commands
> > > >   NFSD: convert write_ports to netlink commands
> > > > 
> > > >  Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
> > > >  fs/nfsd/netlink.c                     |  54 ++++++
> > > >  fs/nfsd/netlink.h                     |   8 +
> > > >  fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
> > > >  include/uapi/linux/nfsd_netlink.h     |  30 +++
> > > >  tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
> > > >  tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
> > > >  7 files changed, 845 insertions(+), 7 deletions(-)
> > > > 
> > > 
> > > Nice work, Lorenzo! Now comes the bikeshedding...
> > 
> > Hi Jeff,
> > 
> > > 
> > > With the nfsdfs interface, we sort of had to split things up into
> > > multiple files like this, but it has some drawbacks, in particular with
> > > weird behavior when people do things out of order.
> > 
> > what do you mean with 'weird behavior'? Something not expected?
> > 
> 
> Yeah.
> 
> For instance, if you set up sockets but never write anything to the
> "threads" file, those sockets will sit around in perpetuity. Granted
> most people use rpc.nfsd to start the server, so this generally doesn't
> happen often, but it's always been a klunky interface regardless.

If you set up sockets but *do* write something to the "threads" file,
then those sockets will *still* sit around in perpetuity.
i.e. until you shut down the NFS server (rpc.nfsd 0).

I don't really see the problem.

It is true that you can use use the interface to ask for meaningless
things.  The maxim that applies is "If you make it fool-proof, only a
fool will use it". :-)

I'm not against exploring changes to the interface style in conjunction
with moving from nfsd-fs to netlink, but I would want a bit more
justification for any change.

Thanks,
NeilBrown

> 
> > > 
> > > Would it make more sense to instead have a single netlink command that
> > > sets up ports and versions, and then spawns the requisite amount of
> > > threads, all in one fell swoop?
> > 
> > I do not have a strong opinion about it but I would say having a dedicated
> > set/get for each paramater allow us to have more granularity (e.g. if you want
> > to change just a parameter we do not need to send all of them to the kernel).
> > What do you think?
> > 
> 
> It's pretty rare to need to twiddle settings on the server while it's up
> and running. Restarting the server in the face of even minor changes is
> not generally a huge problem, so I don't see this as necessary.

Restarting the server is not zero-cost.  It restarts the grace period.
So I would rather not require it for minor changes.

NeilBrown


> 
> Also, it's always been a bit hit and miss as to which settings take
> immediate effect. For instance, if I (e.g.) turn off NFSv4 serving
> altogether on a running server, it doesn't purge the existing NFSv4
> state, but v4 RPCs would be immediately rejected. Eventually it would
> time out, but it is odd.
> 
> Personally, I think this is amenable to a declarative interface:
> 
> Have userland always send down a complete description of what the server
> should look like, and then the kernel can do what it needs to make that
> happen (starting/stopping threads, opening/closing sockets, changing
> versions served, etc.).
> 
> > > 
> > > That does presuppose we can send down a variable-length frame though,
> > > but I assume that is possible with netlink.
> > 
> > sure, we can do it.
> -- 
> Jeff Layton <jlayton@kernel.org>
> 

Re: [PATCH v4 0/3] convert write_threads, write_version and write_ports to netlink commands

[Jeff Layton]

On Mon, 2023-11-13 at 07:22 +1100, NeilBrown wrote:
> On Sun, 12 Nov 2023, Jeff Layton wrote:
> > On Sun, 2023-11-12 at 11:02 +0100, Lorenzo Bianconi wrote:
> > > > On Sat, 2023-11-04 at 12:13 +0100, Lorenzo Bianconi wrote:
> > > > > Introduce write_threads, write_version and write_ports netlink
> > > > > commands similar to the ones available through the procfs.
> > > > > 
> > > > > 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/nfsd-netlink.git
> > > > > This series is based on the commit below available in net-next tree
> > > > > 
> > > > > commit e0fadcffdd172d3a762cb3d0e2e185b8198532d9
> > > > > Author: Jakub Kicinski <kuba@kernel.org>
> > > > > Date:   Fri Oct 6 06:50:32 2023 -0700
> > > > > 
> > > > >     tools: ynl-gen: handle do ops with no input attrs
> > > > > 
> > > > >     The code supports dumps with no input attributes currently
> > > > >     thru a combination of special-casing and luck.
> > > > >     Clean up the handling of ops with no inputs. Create empty
> > > > >     Structs, and skip printing of empty types.
> > > > >     This makes dos with no inputs work.
> > > > > 
> > > > > Lorenzo Bianconi (3):
> > > > >   NFSD: convert write_threads to netlink commands
> > > > >   NFSD: convert write_version to netlink commands
> > > > >   NFSD: convert write_ports to netlink commands
> > > > > 
> > > > >  Documentation/netlink/specs/nfsd.yaml |  83 ++++++++
> > > > >  fs/nfsd/netlink.c                     |  54 ++++++
> > > > >  fs/nfsd/netlink.h                     |   8 +
> > > > >  fs/nfsd/nfsctl.c                      | 267 +++++++++++++++++++++++++-
> > > > >  include/uapi/linux/nfsd_netlink.h     |  30 +++
> > > > >  tools/net/ynl/generated/nfsd-user.c   | 254 ++++++++++++++++++++++++
> > > > >  tools/net/ynl/generated/nfsd-user.h   | 156 +++++++++++++++
> > > > >  7 files changed, 845 insertions(+), 7 deletions(-)
> > > > > 
> > > > 
> > > > Nice work, Lorenzo! Now comes the bikeshedding...
> > > 
> > > Hi Jeff,
> > > 
> > > > 
> > > > With the nfsdfs interface, we sort of had to split things up into
> > > > multiple files like this, but it has some drawbacks, in particular with
> > > > weird behavior when people do things out of order.
> > > 
> > > what do you mean with 'weird behavior'? Something not expected?
> > > 
> > 
> > Yeah.
> > 
> > For instance, if you set up sockets but never write anything to the
> > "threads" file, those sockets will sit around in perpetuity. Granted
> > most people use rpc.nfsd to start the server, so this generally doesn't
> > happen often, but it's always been a klunky interface regardless.
> 
> If you set up sockets but *do* write something to the "threads" file,
> then those sockets will *still* sit around in perpetuity.
> i.e. until you shut down the NFS server (rpc.nfsd 0).
> 
> I don't really see the problem.
> 
> It is true that you can use use the interface to ask for meaningless
> things.  The maxim that applies is "If you make it fool-proof, only a
> fool will use it". :-)
> 
> I'm not against exploring changes to the interface style in conjunction
> with moving from nfsd-fs to netlink, but I would want a bit more
> justification for any change.
> 
> 

Mostly my justification is that that occasionally we do add new settings
for nfsd, and having a single extensible command may make that simpler
to deal with.

> > 
> > > > 
> > > > Would it make more sense to instead have a single netlink command that
> > > > sets up ports and versions, and then spawns the requisite amount of
> > > > threads, all in one fell swoop?
> > > 
> > > I do not have a strong opinion about it but I would say having a dedicated
> > > set/get for each paramater allow us to have more granularity (e.g. if you want
> > > to change just a parameter we do not need to send all of them to the kernel).
> > > What do you think?
> > > 
> > 
> > It's pretty rare to need to twiddle settings on the server while it's up
> > and running. Restarting the server in the face of even minor changes is
> > not generally a huge problem, so I don't see this as necessary.
> 
> Restarting the server is not zero-cost.  It restarts the grace period.
> So I would rather not require it for minor changes.
> 
> 

That is a good point. That said, we wouldn't necessarily need to require
restarting the server on a reconfigure. Some settings could be changed
without a server restart.

In any case, I'll withdraw my objection and we can do this with multiple
commands for now. We can always add a single command later, and just fix
up rpc.nfsd to hide all of the details of the different interfaces at
that time if we think it's helpful.

Cheers,
--
Jeff Layton <jlayton@kernel.org>