netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands
@ 2024-01-20 17:33 Lorenzo Bianconi
  2024-01-20 17:33 ` [PATCH v6 1/3] NFSD: convert write_threads to netlink command Lorenzo Bianconi
                   ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-20 17:33 UTC (permalink / raw)
  To: linux-nfs
  Cc: lorenzo.bianconi, neilb, jlayton, kuba, chuck.lever, horms, netdev

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

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

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

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

 Documentation/netlink/specs/nfsd.yaml |  94 ++++++
 fs/nfsd/netlink.c                     |  63 ++++
 fs/nfsd/netlink.h                     |  10 +
 fs/nfsd/nfsctl.c                      | 396 ++++++++++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  44 +++
 tools/net/ynl/generated/nfsd-user.c   | 460 ++++++++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   | 155 +++++++++
 7 files changed, 1222 insertions(+)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v6 1/3] NFSD: convert write_threads to netlink command
  2024-01-20 17:33 [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Lorenzo Bianconi
@ 2024-01-20 17:33 ` Lorenzo Bianconi
  2024-01-20 17:33 ` [PATCH v6 2/3] NFSD: add write_version " Lorenzo Bianconi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-20 17:33 UTC (permalink / raw)
  To: linux-nfs
  Cc: lorenzo.bianconi, neilb, jlayton, kuba, chuck.lever, horms, netdev

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

Reviewed-by: Jeff Layton <jlayton@kernel.org>
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                      | 60 +++++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  9 +++
 tools/net/ynl/generated/nfsd-user.c   | 93 +++++++++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   | 47 ++++++++++++++
 7 files changed, 251 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 8e6dbe9e0b65..68718448d660 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1696,6 +1696,66 @@ int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb)
 	return 0;
 }
 
+/**
+ * nfsd_nl_threads_set_doit - set the number of running threads
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	u32 nthreads;
+	int ret;
+
+	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_WORKER_THREADS))
+		return -EINVAL;
+
+	nthreads = nla_get_u32(info->attrs[NFSD_A_SERVER_WORKER_THREADS]);
+	ret = nfsd_svc(nthreads,
+		       genl_info_net(info), get_current_cred());
+
+	return ret == nthreads ? 0 : -EINVAL;
+}
+
+/**
+ * nfsd_nl_threads_get_doit - get the number of running threads
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	void *hdr;
+	int err;
+
+	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_iput(skb, info);
+	if (!hdr) {
+		err = -EMSGSIZE;
+		goto err_free_msg;
+	}
+
+	if (nla_put_u32(skb, NFSD_A_SERVER_WORKER_THREADS,
+			nfsd_nrthreads(genl_info_net(info)))) {
+		err = -EINVAL;
+		goto err_free_msg;
+	}
+
+	genlmsg_end(skb, hdr);
+
+	return genlmsg_reply(skb, info);
+
+err_free_msg:
+	nlmsg_free(skb);
+
+	return err;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index 3cd044edee5d..1b6fe1f9ed0e 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -29,8 +29,17 @@ enum {
 	NFSD_A_RPC_STATUS_MAX = (__NFSD_A_RPC_STATUS_MAX - 1)
 };
 
+enum {
+	NFSD_A_SERVER_WORKER_THREADS = 1,
+
+	__NFSD_A_SERVER_WORKER_MAX,
+	NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
+};
+
 enum {
 	NFSD_CMD_RPC_STATUS_GET = 1,
+	NFSD_CMD_THREADS_SET,
+	NFSD_CMD_THREADS_GET,
 
 	__NFSD_CMD_MAX,
 	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
index 360b6448c6e9..f4121a52ccf8 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 */
@@ -198,6 +209,88 @@ 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 ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	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, &yrs);
+	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 989c6e209ced..e162a4f20d91 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -64,4 +64,51 @@ nfsd_rpc_status_get_rsp_list_free(struct nfsd_rpc_status_get_rsp_list *rsp);
 struct nfsd_rpc_status_get_rsp_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.43.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v6 2/3] NFSD: add write_version to netlink command
  2024-01-20 17:33 [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Lorenzo Bianconi
  2024-01-20 17:33 ` [PATCH v6 1/3] NFSD: convert write_threads to netlink command Lorenzo Bianconi
@ 2024-01-20 17:33 ` Lorenzo Bianconi
  2024-01-22 13:27   ` Jeff Layton
  2024-01-20 17:33 ` [PATCH v6 3/3] NFSD: add write_ports " Lorenzo Bianconi
  2024-01-22 13:49 ` [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Jeff Layton
  3 siblings, 1 reply; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-20 17:33 UTC (permalink / raw)
  To: linux-nfs
  Cc: lorenzo.bianconi, neilb, jlayton, kuba, chuck.lever, horms, netdev

Introduce write_version netlink command. For version-set, userspace is
expected to provide a NFS major/minor version list it wants to enable
(all the other ones will be disabled).

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/netlink/specs/nfsd.yaml |  34 +++++
 fs/nfsd/netlink.c                     |  23 ++++
 fs/nfsd/netlink.h                     |   5 +
 fs/nfsd/nfsctl.c                      | 140 ++++++++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  17 +++
 tools/net/ynl/generated/nfsd-user.c   | 176 ++++++++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   |  53 ++++++++
 7 files changed, 448 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index c92e1425d316..30f18798e84e 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -68,6 +68,23 @@ attribute-sets:
       -
         name: threads
         type: u32
+  -
+    name: nfs-version
+    attributes:
+      -
+        name: major
+        type: u32
+      -
+        name: minor
+        type: u32
+  -
+    name: server-proto
+    attributes:
+      -
+        name: version
+        type: nest
+        nested-attributes: nfs-version
+        multi-attr: true
 
 operations:
   list:
@@ -110,3 +127,20 @@ operations:
         reply:
           attributes:
             - threads
+    -
+      name: version-set
+      doc: set nfs enabled versions
+      attribute-set: server-proto
+      flags: [ admin-perm ]
+      do:
+        request:
+          attributes:
+            - version
+    -
+      name: version-get
+      doc: get nfs enabled versions
+      attribute-set: server-proto
+      do:
+        reply:
+          attributes:
+            - version
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 1a59a8e6c7e2..5cbbd3295543 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -10,11 +10,22 @@
 
 #include <uapi/linux/nfsd_netlink.h>
 
+/* Common nested types */
+const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1] = {
+	[NFSD_A_NFS_VERSION_MAJOR] = { .type = NLA_U32, },
+	[NFSD_A_NFS_VERSION_MINOR] = { .type = NLA_U32, },
+};
+
 /* NFSD_CMD_THREADS_SET - do */
 static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
 	[NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
 };
 
+/* NFSD_CMD_VERSION_SET - do */
+static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VERSION + 1] = {
+	[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_nfs_version_nl_policy),
+};
+
 /* Ops table for nfsd */
 static const struct genl_split_ops nfsd_nl_ops[] = {
 	{
@@ -36,6 +47,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
 		.doit	= nfsd_nl_threads_get_doit,
 		.flags	= GENL_CMD_CAP_DO,
 	},
+	{
+		.cmd		= NFSD_CMD_VERSION_SET,
+		.doit		= nfsd_nl_version_set_doit,
+		.policy		= nfsd_version_set_nl_policy,
+		.maxattr	= NFSD_A_SERVER_PROTO_VERSION,
+		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd	= NFSD_CMD_VERSION_GET,
+		.doit	= nfsd_nl_version_get_doit,
+		.flags	= GENL_CMD_CAP_DO,
+	},
 };
 
 struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index 4137fac477e4..c9a1be693fef 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -11,6 +11,9 @@
 
 #include <uapi/linux/nfsd_netlink.h>
 
+/* Common nested types */
+extern const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1];
+
 int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
 int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
 
@@ -18,6 +21,8 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
 				  struct netlink_callback *cb);
 int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
 
 extern struct genl_family nfsd_nl_family;
 
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 68718448d660..53af82303f93 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1756,6 +1756,146 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+/**
+ * nfsd_nl_version_set_doit - set the nfs enabled versions
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	const struct nlattr *attr;
+	struct nfsd_net *nn;
+	int i, rem;
+
+	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_PROTO_VERSION))
+		return -EINVAL;
+
+	mutex_lock(&nfsd_mutex);
+
+	nn = net_generic(genl_info_net(info), nfsd_net_id);
+	if (nn->nfsd_serv) {
+		mutex_unlock(&nfsd_mutex);
+		return -EBUSY;
+	}
+
+	/* clear current supported versions. */
+	nfsd_vers(nn, 2, NFSD_CLEAR);
+	nfsd_vers(nn, 3, NFSD_CLEAR);
+	for (i = 0; i <= NFSD_SUPPORTED_MINOR_VERSION; i++)
+		nfsd_minorversion(nn, i, NFSD_CLEAR);
+
+	nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+		struct nlattr *tb[ARRAY_SIZE(nfsd_nfs_version_nl_policy)];
+		u32 major, minor = 0;
+
+		if (nla_type(attr) != NFSD_A_SERVER_PROTO_VERSION)
+			continue;
+
+		if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
+				     nfsd_nfs_version_nl_policy,
+				     info->extack) < 0)
+			continue;
+
+		if (!tb[NFSD_A_NFS_VERSION_MAJOR])
+			continue;
+
+		major = nla_get_u32(tb[NFSD_A_NFS_VERSION_MAJOR]);
+		if (tb[NFSD_A_NFS_VERSION_MINOR])
+			minor = nla_get_u32(tb[NFSD_A_NFS_VERSION_MINOR]);
+
+		switch (major) {
+		case 4:
+			nfsd_minorversion(nn, minor, NFSD_SET);
+			break;
+		case 3:
+		case 2:
+			if (!minor)
+				nfsd_vers(nn, major, NFSD_SET);
+			break;
+		default:
+			break;
+		}
+	}
+
+	mutex_unlock(&nfsd_mutex);
+
+	return 0;
+}
+
+/**
+ * nfsd_nl_version_get_doit - get the nfs enabled versions
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nfsd_net *nn;
+	int i, err;
+	void *hdr;
+
+	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_iput(skb, info);
+	if (!hdr) {
+		err = -EMSGSIZE;
+		goto err_free_msg;
+	}
+
+	mutex_lock(&nfsd_mutex);
+	nn = net_generic(genl_info_net(info), nfsd_net_id);
+
+	for (i = 2; i <= 4; i++) {
+		int j;
+
+		for (j = 0; j <= NFSD_SUPPORTED_MINOR_VERSION; j++) {
+			struct nlattr *attr;
+
+			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;
+
+			attr = nla_nest_start_noflag(skb,
+					NFSD_A_SERVER_PROTO_VERSION);
+			if (!attr) {
+				err = -EINVAL;
+				goto err_nfsd_unlock;
+			}
+
+			if (nla_put_u32(skb, NFSD_A_NFS_VERSION_MAJOR, i) ||
+			    nla_put_u32(skb, NFSD_A_NFS_VERSION_MINOR, j)) {
+				err = -EINVAL;
+				goto err_nfsd_unlock;
+			}
+
+			nla_nest_end(skb, attr);
+		}
+	}
+
+	mutex_unlock(&nfsd_mutex);
+	genlmsg_end(skb, hdr);
+
+	return genlmsg_reply(skb, info);
+
+err_nfsd_unlock:
+	mutex_unlock(&nfsd_mutex);
+err_free_msg:
+	nlmsg_free(skb);
+
+	return err;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index 1b6fe1f9ed0e..2a06f9fe6fe9 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -36,10 +36,27 @@ enum {
 	NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
 };
 
+enum {
+	NFSD_A_NFS_VERSION_MAJOR = 1,
+	NFSD_A_NFS_VERSION_MINOR,
+
+	__NFSD_A_NFS_VERSION_MAX,
+	NFSD_A_NFS_VERSION_MAX = (__NFSD_A_NFS_VERSION_MAX - 1)
+};
+
+enum {
+	NFSD_A_SERVER_PROTO_VERSION = 1,
+
+	__NFSD_A_SERVER_PROTO_MAX,
+	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
+};
+
 enum {
 	NFSD_CMD_RPC_STATUS_GET = 1,
 	NFSD_CMD_THREADS_SET,
 	NFSD_CMD_THREADS_GET,
+	NFSD_CMD_VERSION_SET,
+	NFSD_CMD_VERSION_GET,
 
 	__NFSD_CMD_MAX,
 	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
index f4121a52ccf8..ad498543f464 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)
@@ -27,6 +29,16 @@ const char *nfsd_op_str(int op)
 }
 
 /* Policies */
+struct ynl_policy_attr nfsd_nfs_version_policy[NFSD_A_NFS_VERSION_MAX + 1] = {
+	[NFSD_A_NFS_VERSION_MAJOR] = { .name = "major", .type = YNL_PT_U32, },
+	[NFSD_A_NFS_VERSION_MINOR] = { .name = "minor", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest nfsd_nfs_version_nest = {
+	.max_attr = NFSD_A_NFS_VERSION_MAX,
+	.table = nfsd_nfs_version_policy,
+};
+
 struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
 	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
 	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
@@ -58,7 +70,60 @@ struct ynl_policy_nest nfsd_server_worker_nest = {
 	.table = nfsd_server_worker_policy,
 };
 
+struct ynl_policy_attr nfsd_server_proto_policy[NFSD_A_SERVER_PROTO_MAX + 1] = {
+	[NFSD_A_SERVER_PROTO_VERSION] = { .name = "version", .type = YNL_PT_NEST, .nest = &nfsd_nfs_version_nest, },
+};
+
+struct ynl_policy_nest nfsd_server_proto_nest = {
+	.max_attr = NFSD_A_SERVER_PROTO_MAX,
+	.table = nfsd_server_proto_policy,
+};
+
 /* Common nested types */
+void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
+{
+}
+
+int nfsd_nfs_version_put(struct nlmsghdr *nlh, unsigned int attr_type,
+			 struct nfsd_nfs_version *obj)
+{
+	struct nlattr *nest;
+
+	nest = mnl_attr_nest_start(nlh, attr_type);
+	if (obj->_present.major)
+		mnl_attr_put_u32(nlh, NFSD_A_NFS_VERSION_MAJOR, obj->major);
+	if (obj->_present.minor)
+		mnl_attr_put_u32(nlh, NFSD_A_NFS_VERSION_MINOR, obj->minor);
+	mnl_attr_nest_end(nlh, nest);
+
+	return 0;
+}
+
+int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
+			   const struct nlattr *nested)
+{
+	struct nfsd_nfs_version *dst = yarg->data;
+	const struct nlattr *attr;
+
+	mnl_attr_for_each_nested(attr, nested) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NFSD_A_NFS_VERSION_MAJOR) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.major = 1;
+			dst->major = mnl_attr_get_u32(attr);
+		} else if (type == NFSD_A_NFS_VERSION_MINOR) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.minor = 1;
+			dst->minor = mnl_attr_get_u32(attr);
+		}
+	}
+
+	return 0;
+}
+
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
 int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
@@ -291,6 +356,117 @@ 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)
+{
+	unsigned int i;
+
+	for (i = 0; i < req->n_version; i++)
+		nfsd_nfs_version_free(&req->version[i]);
+	free(req->version);
+	free(req);
+}
+
+int nfsd_version_set(struct ynl_sock *ys, struct nfsd_version_set_req *req)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_VERSION_SET, 1);
+	ys->req_policy = &nfsd_server_proto_nest;
+
+	for (unsigned int i = 0; i < req->n_version; i++)
+		nfsd_nfs_version_put(nlh, NFSD_A_SERVER_PROTO_VERSION, &req->version[i]);
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		return -1;
+
+	return 0;
+}
+
+/* ============== NFSD_CMD_VERSION_GET ============== */
+/* NFSD_CMD_VERSION_GET - do */
+void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp)
+{
+	unsigned int i;
+
+	for (i = 0; i < rsp->n_version; i++)
+		nfsd_nfs_version_free(&rsp->version[i]);
+	free(rsp->version);
+	free(rsp);
+}
+
+int nfsd_version_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
+{
+	struct ynl_parse_arg *yarg = data;
+	struct nfsd_version_get_rsp *dst;
+	unsigned int n_version = 0;
+	const struct nlattr *attr;
+	struct ynl_parse_arg parg;
+	int i;
+
+	dst = yarg->data;
+	parg.ys = yarg->ys;
+
+	if (dst->version)
+		return ynl_error_parse(yarg, "attribute already present (server-proto.version)");
+
+	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NFSD_A_SERVER_PROTO_VERSION) {
+			n_version++;
+		}
+	}
+
+	if (n_version) {
+		dst->version = calloc(n_version, sizeof(*dst->version));
+		dst->n_version = n_version;
+		i = 0;
+		parg.rsp_policy = &nfsd_nfs_version_nest;
+		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_PROTO_VERSION) {
+				parg.data = &dst->version[i];
+				if (nfsd_nfs_version_parse(&parg, attr))
+					return MNL_CB_ERROR;
+				i++;
+			}
+		}
+	}
+
+	return MNL_CB_OK;
+}
+
+struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct nfsd_version_get_rsp *rsp;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_VERSION_GET, 1);
+	ys->req_policy = &nfsd_server_proto_nest;
+	yrs.yarg.rsp_policy = &nfsd_server_proto_nest;
+
+	rsp = calloc(1, sizeof(*rsp));
+	yrs.yarg.data = rsp;
+	yrs.cb = nfsd_version_get_rsp_parse;
+	yrs.rsp_cmd = NFSD_CMD_VERSION_GET;
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		goto err_free;
+
+	return rsp;
+
+err_free:
+	nfsd_version_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 e162a4f20d91..d062ee8fa8b6 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -19,6 +19,16 @@ extern const struct ynl_family ynl_nfsd_family;
 const char *nfsd_op_str(int op);
 
 /* Common nested types */
+struct nfsd_nfs_version {
+	struct {
+		__u32 major:1;
+		__u32 minor:1;
+	} _present;
+
+	__u32 major;
+	__u32 minor;
+};
+
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
 struct nfsd_rpc_status_get_rsp_dump {
@@ -111,4 +121,47 @@ 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 {
+	unsigned int n_version;
+	struct nfsd_nfs_version *version;
+};
+
+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_version(struct nfsd_version_set_req *req,
+				   struct nfsd_nfs_version *version,
+				   unsigned int n_version)
+{
+	free(req->version);
+	req->version = version;
+	req->n_version = n_version;
+}
+
+/*
+ * set nfs enabled versions
+ */
+int nfsd_version_set(struct ynl_sock *ys, struct nfsd_version_set_req *req);
+
+/* ============== NFSD_CMD_VERSION_GET ============== */
+/* NFSD_CMD_VERSION_GET - do */
+
+struct nfsd_version_get_rsp {
+	unsigned int n_version;
+	struct nfsd_nfs_version *version;
+};
+
+void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
+
+/*
+ * get nfs enabled versions
+ */
+struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
+
 #endif /* _LINUX_NFSD_GEN_H */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-20 17:33 [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Lorenzo Bianconi
  2024-01-20 17:33 ` [PATCH v6 1/3] NFSD: convert write_threads to netlink command Lorenzo Bianconi
  2024-01-20 17:33 ` [PATCH v6 2/3] NFSD: add write_version " Lorenzo Bianconi
@ 2024-01-20 17:33 ` Lorenzo Bianconi
  2024-01-22 13:41   ` Jeff Layton
  2024-01-22 16:31   ` kernel test robot
  2024-01-22 13:49 ` [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Jeff Layton
  3 siblings, 2 replies; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-20 17:33 UTC (permalink / raw)
  To: linux-nfs
  Cc: lorenzo.bianconi, neilb, jlayton, kuba, chuck.lever, horms, netdev

Introduce write_ports netlink command. For listener-set, userspace is
expected to provide a NFS listeners list it wants to enable (all the
other ports will be closed).

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 Documentation/netlink/specs/nfsd.yaml |  37 +++++
 fs/nfsd/netlink.c                     |  23 +++
 fs/nfsd/netlink.h                     |   3 +
 fs/nfsd/nfsctl.c                      | 196 ++++++++++++++++++++++++++
 include/uapi/linux/nfsd_netlink.h     |  18 +++
 tools/net/ynl/generated/nfsd-user.c   | 191 +++++++++++++++++++++++++
 tools/net/ynl/generated/nfsd-user.h   |  55 ++++++++
 7 files changed, 523 insertions(+)

diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
index 30f18798e84e..296ff24b23ac 100644
--- a/Documentation/netlink/specs/nfsd.yaml
+++ b/Documentation/netlink/specs/nfsd.yaml
@@ -85,6 +85,26 @@ attribute-sets:
         type: nest
         nested-attributes: nfs-version
         multi-attr: true
+  -
+    name: server-instance
+    attributes:
+      -
+        name: transport-name
+        type: string
+      -
+        name: port
+        type: u32
+      -
+        name: inet-proto
+        type: u16
+  -
+    name: server-listener
+    attributes:
+      -
+        name: instance
+        type: nest
+        nested-attributes: server-instance
+        multi-attr: true
 
 operations:
   list:
@@ -144,3 +164,20 @@ operations:
         reply:
           attributes:
             - version
+    -
+      name: listener-set
+      doc: set nfs running listeners
+      attribute-set: server-listener
+      flags: [ admin-perm ]
+      do:
+        request:
+          attributes:
+            - instance
+    -
+      name: listener-get
+      doc: get nfs running listeners
+      attribute-set: server-listener
+      do:
+        reply:
+          attributes:
+            - instance
diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
index 5cbbd3295543..c772f9e14761 100644
--- a/fs/nfsd/netlink.c
+++ b/fs/nfsd/netlink.c
@@ -16,6 +16,12 @@ const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1]
 	[NFSD_A_NFS_VERSION_MINOR] = { .type = NLA_U32, },
 };
 
+const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1] = {
+	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
+	[NFSD_A_SERVER_INSTANCE_PORT] = { .type = NLA_U32, },
+	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .type = NLA_U16, },
+};
+
 /* 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, },
@@ -26,6 +32,11 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VE
 	[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_nfs_version_nl_policy),
 };
 
+/* NFSD_CMD_LISTENER_SET - do */
+static const struct nla_policy nfsd_listener_set_nl_policy[NFSD_A_SERVER_LISTENER_INSTANCE + 1] = {
+	[NFSD_A_SERVER_LISTENER_INSTANCE] = NLA_POLICY_NESTED(nfsd_server_instance_nl_policy),
+};
+
 /* Ops table for nfsd */
 static const struct genl_split_ops nfsd_nl_ops[] = {
 	{
@@ -59,6 +70,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
 		.doit	= nfsd_nl_version_get_doit,
 		.flags	= GENL_CMD_CAP_DO,
 	},
+	{
+		.cmd		= NFSD_CMD_LISTENER_SET,
+		.doit		= nfsd_nl_listener_set_doit,
+		.policy		= nfsd_listener_set_nl_policy,
+		.maxattr	= NFSD_A_SERVER_LISTENER_INSTANCE,
+		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd	= NFSD_CMD_LISTENER_GET,
+		.doit	= nfsd_nl_listener_get_doit,
+		.flags	= GENL_CMD_CAP_DO,
+	},
 };
 
 struct genl_family nfsd_nl_family __ro_after_init = {
diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
index c9a1be693fef..10a26ad32cd0 100644
--- a/fs/nfsd/netlink.h
+++ b/fs/nfsd/netlink.h
@@ -13,6 +13,7 @@
 
 /* Common nested types */
 extern const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1];
+extern const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1];
 
 int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
 int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
@@ -23,6 +24,8 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
 int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info);
+int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info);
 
 extern struct genl_family nfsd_nl_family;
 
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 53af82303f93..562b209f2921 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1896,6 +1896,202 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+/**
+ * nfsd_nl_listener_set_doit - set the nfs running listeners
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct nlattr *tb[ARRAY_SIZE(nfsd_server_instance_nl_policy)];
+	struct net *net = genl_info_net(info);
+	struct svc_xprt *xprt, *tmp_xprt;
+	const struct nlattr *attr;
+	struct svc_serv *serv;
+	const char *xcl_name;
+	struct nfsd_net *nn;
+	int port, err, rem;
+	sa_family_t af;
+
+	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_INSTANCE))
+		return -EINVAL;
+
+	mutex_lock(&nfsd_mutex);
+
+	err = nfsd_create_serv(net);
+	if (err) {
+		mutex_unlock(&nfsd_mutex);
+		return err;
+	}
+
+	nn = net_generic(net, nfsd_net_id);
+	serv = nn->nfsd_serv;
+
+	/* 1- create brand new listeners */
+	nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+		if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
+			continue;
+
+		if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
+				     nfsd_server_instance_nl_policy,
+				     info->extack) < 0)
+			continue;
+
+		if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
+		    !tb[NFSD_A_SERVER_INSTANCE_PORT])
+			continue;
+
+		xcl_name = nla_data(tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
+		port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
+		if (port < 1 || port > USHRT_MAX)
+			continue;
+
+		af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
+		if (af != PF_INET && af != PF_INET6)
+			continue;
+
+		xprt = svc_find_xprt(serv, xcl_name, net, PF_INET, port);
+		if (xprt) {
+			svc_xprt_put(xprt);
+			continue;
+		}
+
+		/* create new listerner */
+		if (svc_xprt_create(serv, xcl_name, net, af, port,
+				    SVC_SOCK_ANONYMOUS, get_current_cred()))
+			continue;
+	}
+
+	/* 2- remove stale listeners */
+	spin_lock_bh(&serv->sv_lock);
+	list_for_each_entry_safe(xprt, tmp_xprt, &serv->sv_permsocks,
+				 xpt_list) {
+		struct svc_xprt *rqt_xprt = NULL;
+
+		nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
+			if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
+				continue;
+
+			if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
+					     nfsd_server_instance_nl_policy,
+					     info->extack) < 0)
+				continue;
+
+			if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
+			    !tb[NFSD_A_SERVER_INSTANCE_PORT])
+				continue;
+
+			xcl_name = nla_data(
+				tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
+			port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
+			if (port < 1 || port > USHRT_MAX)
+				continue;
+
+			af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
+			if (af != PF_INET && af != PF_INET6)
+				continue;
+
+			if (!strcmp(xprt->xpt_class->xcl_name, xcl_name) &&
+			    port == svc_xprt_local_port(xprt) &&
+			    af == xprt->xpt_local.ss_family &&
+			    xprt->xpt_net == net) {
+				rqt_xprt = xprt;
+				break;
+			}
+		}
+
+		/* remove stale listener */
+		if (!rqt_xprt) {
+			spin_unlock_bh(&serv->sv_lock);
+			svc_xprt_close(xprt);
+			spin_lock_bh(&serv->sv_lock);
+		}
+	}
+	spin_unlock_bh(&serv->sv_lock);
+
+	if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
+		nfsd_destroy_serv(net);
+
+	mutex_unlock(&nfsd_mutex);
+
+	return 0;
+}
+
+/**
+ * nfsd_nl_listener_get_doit - get the nfs running listeners
+ * @skb: reply buffer
+ * @info: netlink metadata and command arguments
+ *
+ * Return 0 on success or a negative errno.
+ */
+int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	struct svc_xprt *xprt;
+	struct svc_serv *serv;
+	struct nfsd_net *nn;
+	void *hdr;
+	int err;
+
+	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hdr = genlmsg_iput(skb, info);
+	if (!hdr) {
+		err = -EMSGSIZE;
+		goto err_free_msg;
+	}
+
+	mutex_lock(&nfsd_mutex);
+	nn = net_generic(genl_info_net(info), nfsd_net_id);
+	if (!nn->nfsd_serv) {
+		err = -EINVAL;
+		goto err_nfsd_unlock;
+	}
+
+	serv = nn->nfsd_serv;
+	spin_lock_bh(&serv->sv_lock);
+	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
+		struct nlattr *attr;
+
+		attr = nla_nest_start_noflag(skb,
+					     NFSD_A_SERVER_LISTENER_INSTANCE);
+		if (!attr) {
+			err = -EINVAL;
+			goto err_serv_unlock;
+		}
+
+		if (nla_put_string(skb, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME,
+				   xprt->xpt_class->xcl_name) ||
+		    nla_put_u32(skb, NFSD_A_SERVER_INSTANCE_PORT,
+				svc_xprt_local_port(xprt)) ||
+		    nla_put_u16(skb, NFSD_A_SERVER_INSTANCE_INET_PROTO,
+				xprt->xpt_local.ss_family)) {
+			err = -EINVAL;
+			goto err_serv_unlock;
+		}
+
+		nla_nest_end(skb, attr);
+	}
+	spin_unlock_bh(&serv->sv_lock);
+	mutex_unlock(&nfsd_mutex);
+
+	genlmsg_end(skb, hdr);
+
+	return genlmsg_reply(skb, info);
+
+err_serv_unlock:
+	spin_unlock_bh(&serv->sv_lock);
+err_nfsd_unlock:
+	mutex_unlock(&nfsd_mutex);
+err_free_msg:
+	nlmsg_free(skb);
+
+	return err;
+}
+
 /**
  * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
  * @net: a freshly-created network namespace
diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
index 2a06f9fe6fe9..659ab76b8840 100644
--- a/include/uapi/linux/nfsd_netlink.h
+++ b/include/uapi/linux/nfsd_netlink.h
@@ -51,12 +51,30 @@ enum {
 	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
 };
 
+enum {
+	NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME = 1,
+	NFSD_A_SERVER_INSTANCE_PORT,
+	NFSD_A_SERVER_INSTANCE_INET_PROTO,
+
+	__NFSD_A_SERVER_INSTANCE_MAX,
+	NFSD_A_SERVER_INSTANCE_MAX = (__NFSD_A_SERVER_INSTANCE_MAX - 1)
+};
+
+enum {
+	NFSD_A_SERVER_LISTENER_INSTANCE = 1,
+
+	__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_SET,
+	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 ad498543f464..d52f392c7f59 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_SET] = "listener-set",
+	[NFSD_CMD_LISTENER_GET] = "listener-get",
 };
 
 const char *nfsd_op_str(int op)
@@ -39,6 +41,17 @@ struct ynl_policy_nest nfsd_nfs_version_nest = {
 	.table = nfsd_nfs_version_policy,
 };
 
+struct ynl_policy_attr nfsd_server_instance_policy[NFSD_A_SERVER_INSTANCE_MAX + 1] = {
+	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
+	[NFSD_A_SERVER_INSTANCE_PORT] = { .name = "port", .type = YNL_PT_U32, },
+	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .name = "inet-proto", .type = YNL_PT_U16, },
+};
+
+struct ynl_policy_nest nfsd_server_instance_nest = {
+	.max_attr = NFSD_A_SERVER_INSTANCE_MAX,
+	.table = nfsd_server_instance_policy,
+};
+
 struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
 	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
 	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
@@ -79,6 +92,15 @@ struct ynl_policy_nest nfsd_server_proto_nest = {
 	.table = nfsd_server_proto_policy,
 };
 
+struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
+	[NFSD_A_SERVER_LISTENER_INSTANCE] = { .name = "instance", .type = YNL_PT_NEST, .nest = &nfsd_server_instance_nest, },
+};
+
+struct ynl_policy_nest nfsd_server_listener_nest = {
+	.max_attr = NFSD_A_SERVER_LISTENER_MAX,
+	.table = nfsd_server_listener_policy,
+};
+
 /* Common nested types */
 void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
 {
@@ -124,6 +146,64 @@ int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
 	return 0;
 }
 
+void nfsd_server_instance_free(struct nfsd_server_instance *obj)
+{
+	free(obj->transport_name);
+}
+
+int nfsd_server_instance_put(struct nlmsghdr *nlh, unsigned int attr_type,
+			     struct nfsd_server_instance *obj)
+{
+	struct nlattr *nest;
+
+	nest = mnl_attr_nest_start(nlh, attr_type);
+	if (obj->_present.transport_name_len)
+		mnl_attr_put_strz(nlh, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME, obj->transport_name);
+	if (obj->_present.port)
+		mnl_attr_put_u32(nlh, NFSD_A_SERVER_INSTANCE_PORT, obj->port);
+	if (obj->_present.inet_proto)
+		mnl_attr_put_u16(nlh, NFSD_A_SERVER_INSTANCE_INET_PROTO, obj->inet_proto);
+	mnl_attr_nest_end(nlh, nest);
+
+	return 0;
+}
+
+int nfsd_server_instance_parse(struct ynl_parse_arg *yarg,
+			       const struct nlattr *nested)
+{
+	struct nfsd_server_instance *dst = yarg->data;
+	const struct nlattr *attr;
+
+	mnl_attr_for_each_nested(attr, nested) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME) {
+			unsigned int len;
+
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+
+			len = strnlen(mnl_attr_get_str(attr), mnl_attr_get_payload_len(attr));
+			dst->_present.transport_name_len = len;
+			dst->transport_name = malloc(len + 1);
+			memcpy(dst->transport_name, mnl_attr_get_str(attr), len);
+			dst->transport_name[len] = 0;
+		} else if (type == NFSD_A_SERVER_INSTANCE_PORT) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.port = 1;
+			dst->port = mnl_attr_get_u32(attr);
+		} else if (type == NFSD_A_SERVER_INSTANCE_INET_PROTO) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.inet_proto = 1;
+			dst->inet_proto = mnl_attr_get_u16(attr);
+		}
+	}
+
+	return 0;
+}
+
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
 int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
@@ -467,6 +547,117 @@ struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
 	return NULL;
 }
 
+/* ============== NFSD_CMD_LISTENER_SET ============== */
+/* NFSD_CMD_LISTENER_SET - do */
+void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req)
+{
+	unsigned int i;
+
+	for (i = 0; i < req->n_instance; i++)
+		nfsd_server_instance_free(&req->instance[i]);
+	free(req->instance);
+	free(req);
+}
+
+int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_SET, 1);
+	ys->req_policy = &nfsd_server_listener_nest;
+
+	for (unsigned int i = 0; i < req->n_instance; i++)
+		nfsd_server_instance_put(nlh, NFSD_A_SERVER_LISTENER_INSTANCE, &req->instance[i]);
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		return -1;
+
+	return 0;
+}
+
+/* ============== NFSD_CMD_LISTENER_GET ============== */
+/* NFSD_CMD_LISTENER_GET - do */
+void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp)
+{
+	unsigned int i;
+
+	for (i = 0; i < rsp->n_instance; i++)
+		nfsd_server_instance_free(&rsp->instance[i]);
+	free(rsp->instance);
+	free(rsp);
+}
+
+int nfsd_listener_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
+{
+	struct nfsd_listener_get_rsp *dst;
+	struct ynl_parse_arg *yarg = data;
+	unsigned int n_instance = 0;
+	const struct nlattr *attr;
+	struct ynl_parse_arg parg;
+	int i;
+
+	dst = yarg->data;
+	parg.ys = yarg->ys;
+
+	if (dst->instance)
+		return ynl_error_parse(yarg, "attribute already present (server-listener.instance)");
+
+	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NFSD_A_SERVER_LISTENER_INSTANCE) {
+			n_instance++;
+		}
+	}
+
+	if (n_instance) {
+		dst->instance = calloc(n_instance, sizeof(*dst->instance));
+		dst->n_instance = n_instance;
+		i = 0;
+		parg.rsp_policy = &nfsd_server_instance_nest;
+		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_LISTENER_INSTANCE) {
+				parg.data = &dst->instance[i];
+				if (nfsd_server_instance_parse(&parg, attr))
+					return MNL_CB_ERROR;
+				i++;
+			}
+		}
+	}
+
+	return MNL_CB_OK;
+}
+
+struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct nfsd_listener_get_rsp *rsp;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
+	ys->req_policy = &nfsd_server_listener_nest;
+	yrs.yarg.rsp_policy = &nfsd_server_listener_nest;
+
+	rsp = calloc(1, sizeof(*rsp));
+	yrs.yarg.data = rsp;
+	yrs.cb = nfsd_listener_get_rsp_parse;
+	yrs.rsp_cmd = NFSD_CMD_LISTENER_GET;
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		goto err_free;
+
+	return rsp;
+
+err_free:
+	nfsd_listener_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 d062ee8fa8b6..5765fb6f2ef5 100644
--- a/tools/net/ynl/generated/nfsd-user.h
+++ b/tools/net/ynl/generated/nfsd-user.h
@@ -29,6 +29,18 @@ struct nfsd_nfs_version {
 	__u32 minor;
 };
 
+struct nfsd_server_instance {
+	struct {
+		__u32 transport_name_len;
+		__u32 port:1;
+		__u32 inet_proto:1;
+	} _present;
+
+	char *transport_name;
+	__u32 port;
+	__u16 inet_proto;
+};
+
 /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
 /* NFSD_CMD_RPC_STATUS_GET - dump */
 struct nfsd_rpc_status_get_rsp_dump {
@@ -164,4 +176,47 @@ void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
  */
 struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
 
+/* ============== NFSD_CMD_LISTENER_SET ============== */
+/* NFSD_CMD_LISTENER_SET - do */
+struct nfsd_listener_set_req {
+	unsigned int n_instance;
+	struct nfsd_server_instance *instance;
+};
+
+static inline struct nfsd_listener_set_req *nfsd_listener_set_req_alloc(void)
+{
+	return calloc(1, sizeof(struct nfsd_listener_set_req));
+}
+void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req);
+
+static inline void
+__nfsd_listener_set_req_set_instance(struct nfsd_listener_set_req *req,
+				     struct nfsd_server_instance *instance,
+				     unsigned int n_instance)
+{
+	free(req->instance);
+	req->instance = instance;
+	req->n_instance = n_instance;
+}
+
+/*
+ * set nfs running listeners
+ */
+int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req);
+
+/* ============== NFSD_CMD_LISTENER_GET ============== */
+/* NFSD_CMD_LISTENER_GET - do */
+
+struct nfsd_listener_get_rsp {
+	unsigned int n_instance;
+	struct nfsd_server_instance *instance;
+};
+
+void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp);
+
+/*
+ * get nfs running listeners
+ */
+struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys);
+
 #endif /* _LINUX_NFSD_GEN_H */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 2/3] NFSD: add write_version to netlink command
  2024-01-20 17:33 ` [PATCH v6 2/3] NFSD: add write_version " Lorenzo Bianconi
@ 2024-01-22 13:27   ` Jeff Layton
  0 siblings, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-22 13:27 UTC (permalink / raw)
  To: Lorenzo Bianconi, linux-nfs
  Cc: lorenzo.bianconi, neilb, kuba, chuck.lever, horms, netdev

On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> Introduce write_version netlink command. For version-set, userspace is
> expected to provide a NFS major/minor version list it wants to enable
> (all the other ones will be disabled).
> 

To be clear, this is a departure from how the existing interface works.
The "versions" file was an imperative interface. You write "+3" to it,
and that just enables v3.

This is a declarative interface. If a version is not listed in the set,
then it will be _disabled_. I think this is a better way to do it, given
the way that rpc.nfsd works, but we should all be aware that this is a
change in behavior.

> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  Documentation/netlink/specs/nfsd.yaml |  34 +++++
>  fs/nfsd/netlink.c                     |  23 ++++
>  fs/nfsd/netlink.h                     |   5 +
>  fs/nfsd/nfsctl.c                      | 140 ++++++++++++++++++++
>  include/uapi/linux/nfsd_netlink.h     |  17 +++
>  tools/net/ynl/generated/nfsd-user.c   | 176 ++++++++++++++++++++++++++
>  tools/net/ynl/generated/nfsd-user.h   |  53 ++++++++
>  7 files changed, 448 insertions(+)
> 
> diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> index c92e1425d316..30f18798e84e 100644
> --- a/Documentation/netlink/specs/nfsd.yaml
> +++ b/Documentation/netlink/specs/nfsd.yaml
> @@ -68,6 +68,23 @@ attribute-sets:
>        -
>          name: threads
>          type: u32
> +  -
> +    name: nfs-version
> +    attributes:
> +      -
> +        name: major
> +        type: u32
> +      -
> +        name: minor
> +        type: u32
> +  -
> +    name: server-proto
> +    attributes:
> +      -
> +        name: version
> +        type: nest
> +        nested-attributes: nfs-version
> +        multi-attr: true
>  
>  operations:
>    list:
> @@ -110,3 +127,20 @@ operations:
>          reply:
>            attributes:
>              - threads
> +    -
> +      name: version-set
> +      doc: set nfs enabled versions
> +      attribute-set: server-proto
> +      flags: [ admin-perm ]
> +      do:
> +        request:
> +          attributes:
> +            - version
> +    -
> +      name: version-get
> +      doc: get nfs enabled versions
> +      attribute-set: server-proto
> +      do:
> +        reply:
> +          attributes:
> +            - version
> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> index 1a59a8e6c7e2..5cbbd3295543 100644
> --- a/fs/nfsd/netlink.c
> +++ b/fs/nfsd/netlink.c
> @@ -10,11 +10,22 @@
>  
>  #include <uapi/linux/nfsd_netlink.h>
>  
> +/* Common nested types */
> +const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1] = {
> +	[NFSD_A_NFS_VERSION_MAJOR] = { .type = NLA_U32, },
> +	[NFSD_A_NFS_VERSION_MINOR] = { .type = NLA_U32, },
> +};
> +
>  /* NFSD_CMD_THREADS_SET - do */
>  static const struct nla_policy nfsd_threads_set_nl_policy[NFSD_A_SERVER_WORKER_THREADS + 1] = {
>  	[NFSD_A_SERVER_WORKER_THREADS] = { .type = NLA_U32, },
>  };
>  
> +/* NFSD_CMD_VERSION_SET - do */
> +static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VERSION + 1] = {
> +	[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_nfs_version_nl_policy),
> +};
> +
>  /* Ops table for nfsd */
>  static const struct genl_split_ops nfsd_nl_ops[] = {
>  	{
> @@ -36,6 +47,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
>  		.doit	= nfsd_nl_threads_get_doit,
>  		.flags	= GENL_CMD_CAP_DO,
>  	},
> +	{
> +		.cmd		= NFSD_CMD_VERSION_SET,
> +		.doit		= nfsd_nl_version_set_doit,
> +		.policy		= nfsd_version_set_nl_policy,
> +		.maxattr	= NFSD_A_SERVER_PROTO_VERSION,
> +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> +	},
> +	{
> +		.cmd	= NFSD_CMD_VERSION_GET,
> +		.doit	= nfsd_nl_version_get_doit,
> +		.flags	= GENL_CMD_CAP_DO,
> +	},
>  };
>  
>  struct genl_family nfsd_nl_family __ro_after_init = {
> diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
> index 4137fac477e4..c9a1be693fef 100644
> --- a/fs/nfsd/netlink.h
> +++ b/fs/nfsd/netlink.h
> @@ -11,6 +11,9 @@
>  
>  #include <uapi/linux/nfsd_netlink.h>
>  
> +/* Common nested types */
> +extern const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1];
> +
>  int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
>  int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
>  
> @@ -18,6 +21,8 @@ int nfsd_nl_rpc_status_get_dumpit(struct sk_buff *skb,
>  				  struct netlink_callback *cb);
>  int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
>  int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
> +int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
> +int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
>  
>  extern struct genl_family nfsd_nl_family;
>  
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 68718448d660..53af82303f93 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1756,6 +1756,146 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info)
>  	return err;
>  }
>  
> +/**
> + * nfsd_nl_version_set_doit - set the nfs enabled versions
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	const struct nlattr *attr;
> +	struct nfsd_net *nn;
> +	int i, rem;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_PROTO_VERSION))
> +		return -EINVAL;
> +
> +	mutex_lock(&nfsd_mutex);
> +
> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> +	if (nn->nfsd_serv) {
> +		mutex_unlock(&nfsd_mutex);
> +		return -EBUSY;
> +	}
> +
> +	/* clear current supported versions. */
> +	nfsd_vers(nn, 2, NFSD_CLEAR);
> +	nfsd_vers(nn, 3, NFSD_CLEAR);
> +	for (i = 0; i <= NFSD_SUPPORTED_MINOR_VERSION; i++)
> +		nfsd_minorversion(nn, i, NFSD_CLEAR);
> +
> +	nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> +		struct nlattr *tb[ARRAY_SIZE(nfsd_nfs_version_nl_policy)];
> +		u32 major, minor = 0;
> +
> +		if (nla_type(attr) != NFSD_A_SERVER_PROTO_VERSION)
> +			continue;
> +
> +		if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> +				     nfsd_nfs_version_nl_policy,
> +				     info->extack) < 0)
> +			continue;
> +
> +		if (!tb[NFSD_A_NFS_VERSION_MAJOR])
> +			continue;
> +
> +		major = nla_get_u32(tb[NFSD_A_NFS_VERSION_MAJOR]);
> +		if (tb[NFSD_A_NFS_VERSION_MINOR])
> +			minor = nla_get_u32(tb[NFSD_A_NFS_VERSION_MINOR]);
> +
> +		switch (major) {
> +		case 4:
> +			nfsd_minorversion(nn, minor, NFSD_SET);
> +			break;
> +		case 3:
> +		case 2:
> +			if (!minor)
> +				nfsd_vers(nn, major, NFSD_SET);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&nfsd_mutex);
> +
> +	return 0;
> +}
> +
> +/**
> + * nfsd_nl_version_get_doit - get the nfs enabled versions
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nfsd_net *nn;
> +	int i, err;
> +	void *hdr;
> +
> +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_iput(skb, info);
> +	if (!hdr) {
> +		err = -EMSGSIZE;
> +		goto err_free_msg;
> +	}
> +
> +	mutex_lock(&nfsd_mutex);
> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> +
> +	for (i = 2; i <= 4; i++) {
> +		int j;
> +
> +		for (j = 0; j <= NFSD_SUPPORTED_MINOR_VERSION; j++) {
> +			struct nlattr *attr;
> +
> +			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;
> +
> +			attr = nla_nest_start_noflag(skb,
> +					NFSD_A_SERVER_PROTO_VERSION);
> +			if (!attr) {
> +				err = -EINVAL;
> +				goto err_nfsd_unlock;
> +			}
> +
> +			if (nla_put_u32(skb, NFSD_A_NFS_VERSION_MAJOR, i) ||
> +			    nla_put_u32(skb, NFSD_A_NFS_VERSION_MINOR, j)) {
> +				err = -EINVAL;
> +				goto err_nfsd_unlock;
> +			}
> +
> +			nla_nest_end(skb, attr);
> +		}
> +	}
> +
> +	mutex_unlock(&nfsd_mutex);
> +	genlmsg_end(skb, hdr);
> +
> +	return genlmsg_reply(skb, info);
> +
> +err_nfsd_unlock:
> +	mutex_unlock(&nfsd_mutex);
> +err_free_msg:
> +	nlmsg_free(skb);
> +
> +	return err;
> +}
> +
>  /**
>   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
>   * @net: a freshly-created network namespace
> diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> index 1b6fe1f9ed0e..2a06f9fe6fe9 100644
> --- a/include/uapi/linux/nfsd_netlink.h
> +++ b/include/uapi/linux/nfsd_netlink.h
> @@ -36,10 +36,27 @@ enum {
>  	NFSD_A_SERVER_WORKER_MAX = (__NFSD_A_SERVER_WORKER_MAX - 1)
>  };
>  
> +enum {
> +	NFSD_A_NFS_VERSION_MAJOR = 1,
> +	NFSD_A_NFS_VERSION_MINOR,
> +
> +	__NFSD_A_NFS_VERSION_MAX,
> +	NFSD_A_NFS_VERSION_MAX = (__NFSD_A_NFS_VERSION_MAX - 1)
> +};
> +
> +enum {
> +	NFSD_A_SERVER_PROTO_VERSION = 1,
> +
> +	__NFSD_A_SERVER_PROTO_MAX,
> +	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
> +};
> +
>  enum {
>  	NFSD_CMD_RPC_STATUS_GET = 1,
>  	NFSD_CMD_THREADS_SET,
>  	NFSD_CMD_THREADS_GET,
> +	NFSD_CMD_VERSION_SET,
> +	NFSD_CMD_VERSION_GET,
>  
>  	__NFSD_CMD_MAX,
>  	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
> diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
> index f4121a52ccf8..ad498543f464 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)
> @@ -27,6 +29,16 @@ const char *nfsd_op_str(int op)
>  }
>  
>  /* Policies */
> +struct ynl_policy_attr nfsd_nfs_version_policy[NFSD_A_NFS_VERSION_MAX + 1] = {
> +	[NFSD_A_NFS_VERSION_MAJOR] = { .name = "major", .type = YNL_PT_U32, },
> +	[NFSD_A_NFS_VERSION_MINOR] = { .name = "minor", .type = YNL_PT_U32, },
> +};
> +
> +struct ynl_policy_nest nfsd_nfs_version_nest = {
> +	.max_attr = NFSD_A_NFS_VERSION_MAX,
> +	.table = nfsd_nfs_version_policy,
> +};
> +
>  struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
>  	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
>  	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
> @@ -58,7 +70,60 @@ struct ynl_policy_nest nfsd_server_worker_nest = {
>  	.table = nfsd_server_worker_policy,
>  };
>  
> +struct ynl_policy_attr nfsd_server_proto_policy[NFSD_A_SERVER_PROTO_MAX + 1] = {
> +	[NFSD_A_SERVER_PROTO_VERSION] = { .name = "version", .type = YNL_PT_NEST, .nest = &nfsd_nfs_version_nest, },
> +};
> +
> +struct ynl_policy_nest nfsd_server_proto_nest = {
> +	.max_attr = NFSD_A_SERVER_PROTO_MAX,
> +	.table = nfsd_server_proto_policy,
> +};
> +
>  /* Common nested types */
> +void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
> +{
> +}
> +
> +int nfsd_nfs_version_put(struct nlmsghdr *nlh, unsigned int attr_type,
> +			 struct nfsd_nfs_version *obj)
> +{
> +	struct nlattr *nest;
> +
> +	nest = mnl_attr_nest_start(nlh, attr_type);
> +	if (obj->_present.major)
> +		mnl_attr_put_u32(nlh, NFSD_A_NFS_VERSION_MAJOR, obj->major);
> +	if (obj->_present.minor)
> +		mnl_attr_put_u32(nlh, NFSD_A_NFS_VERSION_MINOR, obj->minor);
> +	mnl_attr_nest_end(nlh, nest);
> +
> +	return 0;
> +}
> +
> +int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
> +			   const struct nlattr *nested)
> +{
> +	struct nfsd_nfs_version *dst = yarg->data;
> +	const struct nlattr *attr;
> +
> +	mnl_attr_for_each_nested(attr, nested) {
> +		unsigned int type = mnl_attr_get_type(attr);
> +
> +		if (type == NFSD_A_NFS_VERSION_MAJOR) {
> +			if (ynl_attr_validate(yarg, attr))
> +				return MNL_CB_ERROR;
> +			dst->_present.major = 1;
> +			dst->major = mnl_attr_get_u32(attr);
> +		} else if (type == NFSD_A_NFS_VERSION_MINOR) {
> +			if (ynl_attr_validate(yarg, attr))
> +				return MNL_CB_ERROR;
> +			dst->_present.minor = 1;
> +			dst->minor = mnl_attr_get_u32(attr);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
>  /* NFSD_CMD_RPC_STATUS_GET - dump */
>  int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
> @@ -291,6 +356,117 @@ 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)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < req->n_version; i++)
> +		nfsd_nfs_version_free(&req->version[i]);
> +	free(req->version);
> +	free(req);
> +}
> +
> +int nfsd_version_set(struct ynl_sock *ys, struct nfsd_version_set_req *req)
> +{
> +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_VERSION_SET, 1);
> +	ys->req_policy = &nfsd_server_proto_nest;
> +
> +	for (unsigned int i = 0; i < req->n_version; i++)
> +		nfsd_nfs_version_put(nlh, NFSD_A_SERVER_PROTO_VERSION, &req->version[i]);
> +
> +	err = ynl_exec(ys, nlh, &yrs);
> +	if (err < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +/* ============== NFSD_CMD_VERSION_GET ============== */
> +/* NFSD_CMD_VERSION_GET - do */
> +void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < rsp->n_version; i++)
> +		nfsd_nfs_version_free(&rsp->version[i]);
> +	free(rsp->version);
> +	free(rsp);
> +}
> +
> +int nfsd_version_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> +{
> +	struct ynl_parse_arg *yarg = data;
> +	struct nfsd_version_get_rsp *dst;
> +	unsigned int n_version = 0;
> +	const struct nlattr *attr;
> +	struct ynl_parse_arg parg;
> +	int i;
> +
> +	dst = yarg->data;
> +	parg.ys = yarg->ys;
> +
> +	if (dst->version)
> +		return ynl_error_parse(yarg, "attribute already present (server-proto.version)");
> +
> +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> +		unsigned int type = mnl_attr_get_type(attr);
> +
> +		if (type == NFSD_A_SERVER_PROTO_VERSION) {
> +			n_version++;
> +		}
> +	}
> +
> +	if (n_version) {
> +		dst->version = calloc(n_version, sizeof(*dst->version));
> +		dst->n_version = n_version;
> +		i = 0;
> +		parg.rsp_policy = &nfsd_nfs_version_nest;
> +		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> +			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_PROTO_VERSION) {
> +				parg.data = &dst->version[i];
> +				if (nfsd_nfs_version_parse(&parg, attr))
> +					return MNL_CB_ERROR;
> +				i++;
> +			}
> +		}
> +	}
> +
> +	return MNL_CB_OK;
> +}
> +
> +struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
> +{
> +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> +	struct nfsd_version_get_rsp *rsp;
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_VERSION_GET, 1);
> +	ys->req_policy = &nfsd_server_proto_nest;
> +	yrs.yarg.rsp_policy = &nfsd_server_proto_nest;
> +
> +	rsp = calloc(1, sizeof(*rsp));
> +	yrs.yarg.data = rsp;
> +	yrs.cb = nfsd_version_get_rsp_parse;
> +	yrs.rsp_cmd = NFSD_CMD_VERSION_GET;
> +
> +	err = ynl_exec(ys, nlh, &yrs);
> +	if (err < 0)
> +		goto err_free;
> +
> +	return rsp;
> +
> +err_free:
> +	nfsd_version_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 e162a4f20d91..d062ee8fa8b6 100644
> --- a/tools/net/ynl/generated/nfsd-user.h
> +++ b/tools/net/ynl/generated/nfsd-user.h
> @@ -19,6 +19,16 @@ extern const struct ynl_family ynl_nfsd_family;
>  const char *nfsd_op_str(int op);
>  
>  /* Common nested types */
> +struct nfsd_nfs_version {
> +	struct {
> +		__u32 major:1;
> +		__u32 minor:1;
> +	} _present;
> +
> +	__u32 major;
> +	__u32 minor;
> +};
> +
>  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
>  /* NFSD_CMD_RPC_STATUS_GET - dump */
>  struct nfsd_rpc_status_get_rsp_dump {
> @@ -111,4 +121,47 @@ 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 {
> +	unsigned int n_version;
> +	struct nfsd_nfs_version *version;
> +};
> +
> +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_version(struct nfsd_version_set_req *req,
> +				   struct nfsd_nfs_version *version,
> +				   unsigned int n_version)
> +{
> +	free(req->version);
> +	req->version = version;
> +	req->n_version = n_version;
> +}
> +
> +/*
> + * set nfs enabled versions
> + */
> +int nfsd_version_set(struct ynl_sock *ys, struct nfsd_version_set_req *req);
> +
> +/* ============== NFSD_CMD_VERSION_GET ============== */
> +/* NFSD_CMD_VERSION_GET - do */
> +
> +struct nfsd_version_get_rsp {
> +	unsigned int n_version;
> +	struct nfsd_nfs_version *version;
> +};
> +
> +void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
> +
> +/*
> + * get nfs enabled versions
> + */
> +struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
> +
>  #endif /* _LINUX_NFSD_GEN_H */

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-20 17:33 ` [PATCH v6 3/3] NFSD: add write_ports " Lorenzo Bianconi
@ 2024-01-22 13:41   ` Jeff Layton
  2024-01-22 15:35     ` Lorenzo Bianconi
  2024-01-22 21:35     ` NeilBrown
  2024-01-22 16:31   ` kernel test robot
  1 sibling, 2 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-22 13:41 UTC (permalink / raw)
  To: Lorenzo Bianconi, linux-nfs
  Cc: lorenzo.bianconi, neilb, kuba, chuck.lever, horms, netdev

On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> Introduce write_ports netlink command. For listener-set, userspace is
> expected to provide a NFS listeners list it wants to enable (all the
> other ports will be closed).
> 

Ditto here. This is a change to a declarative interface, which I think
is a better way to handle this, but we should be aware of the change.

> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  Documentation/netlink/specs/nfsd.yaml |  37 +++++
>  fs/nfsd/netlink.c                     |  23 +++
>  fs/nfsd/netlink.h                     |   3 +
>  fs/nfsd/nfsctl.c                      | 196 ++++++++++++++++++++++++++
>  include/uapi/linux/nfsd_netlink.h     |  18 +++
>  tools/net/ynl/generated/nfsd-user.c   | 191 +++++++++++++++++++++++++
>  tools/net/ynl/generated/nfsd-user.h   |  55 ++++++++
>  7 files changed, 523 insertions(+)
> 
> diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> index 30f18798e84e..296ff24b23ac 100644
> --- a/Documentation/netlink/specs/nfsd.yaml
> +++ b/Documentation/netlink/specs/nfsd.yaml
> @@ -85,6 +85,26 @@ attribute-sets:
>          type: nest
>          nested-attributes: nfs-version
>          multi-attr: true
> +  -
> +    name: server-instance
> +    attributes:
> +      -
> +        name: transport-name
> +        type: string
> +      -
> +        name: port
> +        type: u32
> +      -
> +        name: inet-proto
> +        type: u16
> +  -
> +    name: server-listener
> +    attributes:
> +      -
> +        name: instance
> +        type: nest
> +        nested-attributes: server-instance
> +        multi-attr: true
>  
>  operations:
>    list:
> @@ -144,3 +164,20 @@ operations:
>          reply:
>            attributes:
>              - version
> +    -
> +      name: listener-set
> +      doc: set nfs running listeners
> +      attribute-set: server-listener
> +      flags: [ admin-perm ]
> +      do:
> +        request:
> +          attributes:
> +            - instance
> +    -
> +      name: listener-get
> +      doc: get nfs running listeners
> +      attribute-set: server-listener
> +      do:
> +        reply:
> +          attributes:
> +            - instance
> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> index 5cbbd3295543..c772f9e14761 100644
> --- a/fs/nfsd/netlink.c
> +++ b/fs/nfsd/netlink.c
> @@ -16,6 +16,12 @@ const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1]
>  	[NFSD_A_NFS_VERSION_MINOR] = { .type = NLA_U32, },
>  };
>  
> +const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1] = {
> +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
> +	[NFSD_A_SERVER_INSTANCE_PORT] = { .type = NLA_U32, },
> +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .type = NLA_U16, },
> +};
> +
>  /* 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, },
> @@ -26,6 +32,11 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VE
>  	[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_nfs_version_nl_policy),
>  };
>  
> +/* NFSD_CMD_LISTENER_SET - do */
> +static const struct nla_policy nfsd_listener_set_nl_policy[NFSD_A_SERVER_LISTENER_INSTANCE + 1] = {
> +	[NFSD_A_SERVER_LISTENER_INSTANCE] = NLA_POLICY_NESTED(nfsd_server_instance_nl_policy),
> +};
> +
>  /* Ops table for nfsd */
>  static const struct genl_split_ops nfsd_nl_ops[] = {
>  	{
> @@ -59,6 +70,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
>  		.doit	= nfsd_nl_version_get_doit,
>  		.flags	= GENL_CMD_CAP_DO,
>  	},
> +	{
> +		.cmd		= NFSD_CMD_LISTENER_SET,
> +		.doit		= nfsd_nl_listener_set_doit,
> +		.policy		= nfsd_listener_set_nl_policy,
> +		.maxattr	= NFSD_A_SERVER_LISTENER_INSTANCE,
> +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> +	},
> +	{
> +		.cmd	= NFSD_CMD_LISTENER_GET,
> +		.doit	= nfsd_nl_listener_get_doit,
> +		.flags	= GENL_CMD_CAP_DO,
> +	},
>  };
>  
>  struct genl_family nfsd_nl_family __ro_after_init = {
> diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
> index c9a1be693fef..10a26ad32cd0 100644
> --- a/fs/nfsd/netlink.h
> +++ b/fs/nfsd/netlink.h
> @@ -13,6 +13,7 @@
>  
>  /* Common nested types */
>  extern const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1];
> +extern const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1];
>  
>  int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
>  int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
> @@ -23,6 +24,8 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
>  int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
>  int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
>  int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
> +int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info);
> +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info);
>  
>  extern struct genl_family nfsd_nl_family;
>  
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 53af82303f93..562b209f2921 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -1896,6 +1896,202 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
>  	return err;
>  }
>  
> +/**
> + * nfsd_nl_listener_set_doit - set the nfs running listeners
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct nlattr *tb[ARRAY_SIZE(nfsd_server_instance_nl_policy)];
> +	struct net *net = genl_info_net(info);
> +	struct svc_xprt *xprt, *tmp_xprt;
> +	const struct nlattr *attr;
> +	struct svc_serv *serv;
> +	const char *xcl_name;
> +	struct nfsd_net *nn;
> +	int port, err, rem;
> +	sa_family_t af;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_INSTANCE))
> +		return -EINVAL;
> +
> +	mutex_lock(&nfsd_mutex);
> +
> +	err = nfsd_create_serv(net);
> +	if (err) {
> +		mutex_unlock(&nfsd_mutex);
> +		return err;
> +	}
> +
> +	nn = net_generic(net, nfsd_net_id);
> +	serv = nn->nfsd_serv;
> +
> +	/* 1- create brand new listeners */
> +	nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> +		if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
> +			continue;
> +
> +		if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> +				     nfsd_server_instance_nl_policy,
> +				     info->extack) < 0)
> +			continue;
> +
> +		if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
> +		    !tb[NFSD_A_SERVER_INSTANCE_PORT])
> +			continue;
> +
> +		xcl_name = nla_data(tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
> +		port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
> +		if (port < 1 || port > USHRT_MAX)
> +			continue;
> +
> +		af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
> +		if (af != PF_INET && af != PF_INET6)
> +			continue;
> +
> +		xprt = svc_find_xprt(serv, xcl_name, net, PF_INET, port);
> +		if (xprt) {
> +			svc_xprt_put(xprt);
> +			continue;
> +		}
> +
> +		/* create new listerner */
> +		if (svc_xprt_create(serv, xcl_name, net, af, port,
> +				    SVC_SOCK_ANONYMOUS, get_current_cred()))
> +			continue;
> +	}
> +
> +	/* 2- remove stale listeners */


The old portlist interface was weird, in that it was only additive. You
couldn't use it to close a listening socket (AFAICT). We may be able to
support that now with this interface, but we'll need to test that case
carefully.



> +	spin_lock_bh(&serv->sv_lock);
> +	list_for_each_entry_safe(xprt, tmp_xprt, &serv->sv_permsocks,
> +				 xpt_list) {
> +		struct svc_xprt *rqt_xprt = NULL;
> +
> +		nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> +			if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
> +				continue;
> +
> +			if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> +					     nfsd_server_instance_nl_policy,
> +					     info->extack) < 0)
> +				continue;
> +
> +			if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
> +			    !tb[NFSD_A_SERVER_INSTANCE_PORT])
> +				continue;
> +
> +			xcl_name = nla_data(
> +				tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
> +			port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
> +			if (port < 1 || port > USHRT_MAX)
> +				continue;
> +
> +			af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
> +			if (af != PF_INET && af != PF_INET6)
> +				continue;
> +
> +			if (!strcmp(xprt->xpt_class->xcl_name, xcl_name) &&
> +			    port == svc_xprt_local_port(xprt) &&
> +			    af == xprt->xpt_local.ss_family &&
> +			    xprt->xpt_net == net) {
> +				rqt_xprt = xprt;
> +				break;
> +			}
> +		}
> +
> +		/* remove stale listener */
> +		if (!rqt_xprt) {
> +			spin_unlock_bh(&serv->sv_lock);
> +			svc_xprt_close(xprt);
> 

I'm not sure this is safe. Can anything else modify sv_permsocks while
you're not holding the lock? Maybe not since you're holding the
nfsd_mutex, but it's still probably best to restart the list walk if you
have to drop the lock here.

You're typically only going to have a few sockets here anyway -- usually
just one each for TCP, UDP and maybe RDMA.


> +			spin_lock_bh(&serv->sv_lock);
> +		}
> +	}
> +	spin_unlock_bh(&serv->sv_lock);
> +
> +	if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
> +		nfsd_destroy_serv(net);
> +
> +	mutex_unlock(&nfsd_mutex);
> +
> +	return 0;
> +}
> +
> +/**
> + * nfsd_nl_listener_get_doit - get the nfs running listeners
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return 0 on success or a negative errno.
> + */
> +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct svc_xprt *xprt;
> +	struct svc_serv *serv;
> +	struct nfsd_net *nn;
> +	void *hdr;
> +	int err;
> +
> +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_iput(skb, info);
> +	if (!hdr) {
> +		err = -EMSGSIZE;
> +		goto err_free_msg;
> +	}
> +
> +	mutex_lock(&nfsd_mutex);
> +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> +	if (!nn->nfsd_serv) {
> +		err = -EINVAL;
> +		goto err_nfsd_unlock;
> +	}
> +
> +	serv = nn->nfsd_serv;
> +	spin_lock_bh(&serv->sv_lock);
> +	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> +		struct nlattr *attr;
> +
> +		attr = nla_nest_start_noflag(skb,
> +					     NFSD_A_SERVER_LISTENER_INSTANCE);
> +		if (!attr) {
> +			err = -EINVAL;
> +			goto err_serv_unlock;
> +		}
> +
> +		if (nla_put_string(skb, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME,
> +				   xprt->xpt_class->xcl_name) ||
> +		    nla_put_u32(skb, NFSD_A_SERVER_INSTANCE_PORT,
> +				svc_xprt_local_port(xprt)) ||
> +		    nla_put_u16(skb, NFSD_A_SERVER_INSTANCE_INET_PROTO,
> +				xprt->xpt_local.ss_family)) {
> +			err = -EINVAL;
> +			goto err_serv_unlock;
> +		}
> +
> +		nla_nest_end(skb, attr);
> +	}
> +	spin_unlock_bh(&serv->sv_lock);
> +	mutex_unlock(&nfsd_mutex);
> +
> +	genlmsg_end(skb, hdr);
> +
> +	return genlmsg_reply(skb, info);
> +
> +err_serv_unlock:
> +	spin_unlock_bh(&serv->sv_lock);
> +err_nfsd_unlock:
> +	mutex_unlock(&nfsd_mutex);
> +err_free_msg:
> +	nlmsg_free(skb);
> +
> +	return err;
> +}
> +
>  /**
>   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
>   * @net: a freshly-created network namespace
> diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> index 2a06f9fe6fe9..659ab76b8840 100644
> --- a/include/uapi/linux/nfsd_netlink.h
> +++ b/include/uapi/linux/nfsd_netlink.h
> @@ -51,12 +51,30 @@ enum {
>  	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
>  };
>  
> +enum {
> +	NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME = 1,
> +	NFSD_A_SERVER_INSTANCE_PORT,
> +	NFSD_A_SERVER_INSTANCE_INET_PROTO,
> +
> +	__NFSD_A_SERVER_INSTANCE_MAX,
> +	NFSD_A_SERVER_INSTANCE_MAX = (__NFSD_A_SERVER_INSTANCE_MAX - 1)
> +};
> +
> +enum {
> +	NFSD_A_SERVER_LISTENER_INSTANCE = 1,
> +
> +	__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_SET,
> +	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 ad498543f464..d52f392c7f59 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_SET] = "listener-set",
> +	[NFSD_CMD_LISTENER_GET] = "listener-get",
>  };
>  
>  const char *nfsd_op_str(int op)
> @@ -39,6 +41,17 @@ struct ynl_policy_nest nfsd_nfs_version_nest = {
>  	.table = nfsd_nfs_version_policy,
>  };
>  
> +struct ynl_policy_attr nfsd_server_instance_policy[NFSD_A_SERVER_INSTANCE_MAX + 1] = {
> +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
> +	[NFSD_A_SERVER_INSTANCE_PORT] = { .name = "port", .type = YNL_PT_U32, },
> +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .name = "inet-proto", .type = YNL_PT_U16, },
> +};
> +
> +struct ynl_policy_nest nfsd_server_instance_nest = {
> +	.max_attr = NFSD_A_SERVER_INSTANCE_MAX,
> +	.table = nfsd_server_instance_policy,
> +};
> +
>  struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
>  	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
>  	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
> @@ -79,6 +92,15 @@ struct ynl_policy_nest nfsd_server_proto_nest = {
>  	.table = nfsd_server_proto_policy,
>  };
>  
> +struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
> +	[NFSD_A_SERVER_LISTENER_INSTANCE] = { .name = "instance", .type = YNL_PT_NEST, .nest = &nfsd_server_instance_nest, },
> +};
> +
> +struct ynl_policy_nest nfsd_server_listener_nest = {
> +	.max_attr = NFSD_A_SERVER_LISTENER_MAX,
> +	.table = nfsd_server_listener_policy,
> +};
> +
>  /* Common nested types */
>  void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
>  {
> @@ -124,6 +146,64 @@ int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
>  	return 0;
>  }
>  
> +void nfsd_server_instance_free(struct nfsd_server_instance *obj)
> +{
> +	free(obj->transport_name);
> +}
> +
> +int nfsd_server_instance_put(struct nlmsghdr *nlh, unsigned int attr_type,
> +			     struct nfsd_server_instance *obj)
> +{
> +	struct nlattr *nest;
> +
> +	nest = mnl_attr_nest_start(nlh, attr_type);
> +	if (obj->_present.transport_name_len)
> +		mnl_attr_put_strz(nlh, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME, obj->transport_name);
> +	if (obj->_present.port)
> +		mnl_attr_put_u32(nlh, NFSD_A_SERVER_INSTANCE_PORT, obj->port);
> +	if (obj->_present.inet_proto)
> +		mnl_attr_put_u16(nlh, NFSD_A_SERVER_INSTANCE_INET_PROTO, obj->inet_proto);
> +	mnl_attr_nest_end(nlh, nest);
> +
> +	return 0;
> +}
> +
> +int nfsd_server_instance_parse(struct ynl_parse_arg *yarg,
> +			       const struct nlattr *nested)
> +{
> +	struct nfsd_server_instance *dst = yarg->data;
> +	const struct nlattr *attr;
> +
> +	mnl_attr_for_each_nested(attr, nested) {
> +		unsigned int type = mnl_attr_get_type(attr);
> +
> +		if (type == NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME) {
> +			unsigned int len;
> +
> +			if (ynl_attr_validate(yarg, attr))
> +				return MNL_CB_ERROR;
> +
> +			len = strnlen(mnl_attr_get_str(attr), mnl_attr_get_payload_len(attr));
> +			dst->_present.transport_name_len = len;
> +			dst->transport_name = malloc(len + 1);
> +			memcpy(dst->transport_name, mnl_attr_get_str(attr), len);
> +			dst->transport_name[len] = 0;
> +		} else if (type == NFSD_A_SERVER_INSTANCE_PORT) {
> +			if (ynl_attr_validate(yarg, attr))
> +				return MNL_CB_ERROR;
> +			dst->_present.port = 1;
> +			dst->port = mnl_attr_get_u32(attr);
> +		} else if (type == NFSD_A_SERVER_INSTANCE_INET_PROTO) {
> +			if (ynl_attr_validate(yarg, attr))
> +				return MNL_CB_ERROR;
> +			dst->_present.inet_proto = 1;
> +			dst->inet_proto = mnl_attr_get_u16(attr);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
>  /* NFSD_CMD_RPC_STATUS_GET - dump */
>  int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
> @@ -467,6 +547,117 @@ struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
>  	return NULL;
>  }
>  
> +/* ============== NFSD_CMD_LISTENER_SET ============== */
> +/* NFSD_CMD_LISTENER_SET - do */
> +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < req->n_instance; i++)
> +		nfsd_server_instance_free(&req->instance[i]);
> +	free(req->instance);
> +	free(req);
> +}
> +
> +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req)
> +{
> +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_SET, 1);
> +	ys->req_policy = &nfsd_server_listener_nest;
> +
> +	for (unsigned int i = 0; i < req->n_instance; i++)
> +		nfsd_server_instance_put(nlh, NFSD_A_SERVER_LISTENER_INSTANCE, &req->instance[i]);
> +
> +	err = ynl_exec(ys, nlh, &yrs);
> +	if (err < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +/* ============== NFSD_CMD_LISTENER_GET ============== */
> +/* NFSD_CMD_LISTENER_GET - do */
> +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < rsp->n_instance; i++)
> +		nfsd_server_instance_free(&rsp->instance[i]);
> +	free(rsp->instance);
> +	free(rsp);
> +}
> +
> +int nfsd_listener_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> +{
> +	struct nfsd_listener_get_rsp *dst;
> +	struct ynl_parse_arg *yarg = data;
> +	unsigned int n_instance = 0;
> +	const struct nlattr *attr;
> +	struct ynl_parse_arg parg;
> +	int i;
> +
> +	dst = yarg->data;
> +	parg.ys = yarg->ys;
> +
> +	if (dst->instance)
> +		return ynl_error_parse(yarg, "attribute already present (server-listener.instance)");
> +
> +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> +		unsigned int type = mnl_attr_get_type(attr);
> +
> +		if (type == NFSD_A_SERVER_LISTENER_INSTANCE) {
> +			n_instance++;
> +		}
> +	}
> +
> +	if (n_instance) {
> +		dst->instance = calloc(n_instance, sizeof(*dst->instance));
> +		dst->n_instance = n_instance;
> +		i = 0;
> +		parg.rsp_policy = &nfsd_server_instance_nest;
> +		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> +			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_LISTENER_INSTANCE) {
> +				parg.data = &dst->instance[i];
> +				if (nfsd_server_instance_parse(&parg, attr))
> +					return MNL_CB_ERROR;
> +				i++;
> +			}
> +		}
> +	}
> +
> +	return MNL_CB_OK;
> +}
> +
> +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys)
> +{
> +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> +	struct nfsd_listener_get_rsp *rsp;
> +	struct nlmsghdr *nlh;
> +	int err;
> +
> +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
> +	ys->req_policy = &nfsd_server_listener_nest;
> +	yrs.yarg.rsp_policy = &nfsd_server_listener_nest;
> +
> +	rsp = calloc(1, sizeof(*rsp));
> +	yrs.yarg.data = rsp;
> +	yrs.cb = nfsd_listener_get_rsp_parse;
> +	yrs.rsp_cmd = NFSD_CMD_LISTENER_GET;
> +
> +	err = ynl_exec(ys, nlh, &yrs);
> +	if (err < 0)
> +		goto err_free;
> +
> +	return rsp;
> +
> +err_free:
> +	nfsd_listener_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 d062ee8fa8b6..5765fb6f2ef5 100644
> --- a/tools/net/ynl/generated/nfsd-user.h
> +++ b/tools/net/ynl/generated/nfsd-user.h
> @@ -29,6 +29,18 @@ struct nfsd_nfs_version {
>  	__u32 minor;
>  };
>  
> +struct nfsd_server_instance {
> +	struct {
> +		__u32 transport_name_len;
> +		__u32 port:1;
> +		__u32 inet_proto:1;
> +	} _present;
> +
> +	char *transport_name;
> +	__u32 port;
> +	__u16 inet_proto;
> +};
> +
>  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
>  /* NFSD_CMD_RPC_STATUS_GET - dump */
>  struct nfsd_rpc_status_get_rsp_dump {
> @@ -164,4 +176,47 @@ void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
>   */
>  struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
>  
> +/* ============== NFSD_CMD_LISTENER_SET ============== */
> +/* NFSD_CMD_LISTENER_SET - do */
> +struct nfsd_listener_set_req {
> +	unsigned int n_instance;
> +	struct nfsd_server_instance *instance;
> +};
> +
> +static inline struct nfsd_listener_set_req *nfsd_listener_set_req_alloc(void)
> +{
> +	return calloc(1, sizeof(struct nfsd_listener_set_req));
> +}
> +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req);
> +
> +static inline void
> +__nfsd_listener_set_req_set_instance(struct nfsd_listener_set_req *req,
> +				     struct nfsd_server_instance *instance,
> +				     unsigned int n_instance)
> +{
> +	free(req->instance);
> +	req->instance = instance;
> +	req->n_instance = n_instance;
> +}
> +
> +/*
> + * set nfs running listeners
> + */
> +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req);
> +
> +/* ============== NFSD_CMD_LISTENER_GET ============== */
> +/* NFSD_CMD_LISTENER_GET - do */
> +
> +struct nfsd_listener_get_rsp {
> +	unsigned int n_instance;
> +	struct nfsd_server_instance *instance;
> +};
> +
> +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp);
> +
> +/*
> + * get nfs running listeners
> + */
> +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys);
> +
>  #endif /* _LINUX_NFSD_GEN_H */

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands
  2024-01-20 17:33 [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Lorenzo Bianconi
                   ` (2 preceding siblings ...)
  2024-01-20 17:33 ` [PATCH v6 3/3] NFSD: add write_ports " Lorenzo Bianconi
@ 2024-01-22 13:49 ` Jeff Layton
  2024-01-22 17:01   ` Lorenzo Bianconi
  3 siblings, 1 reply; 27+ messages in thread
From: Jeff Layton @ 2024-01-22 13:49 UTC (permalink / raw)
  To: Lorenzo Bianconi, linux-nfs
  Cc: lorenzo.bianconi, neilb, kuba, chuck.lever, horms, netdev, Steve Dickson

On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> Introduce write_threads, write_version and write_ports netlink
> commands similar to the ones available through the procfs.
> 
> Changes since v5:
> - for write_ports and write_version commands, userspace is expected to provide
>   a NFS listeners/supported versions list it want to enable (all the other
>   ports/versions will be disabled).
> - fix comments
> - rebase on top of nfsd-next
> Changes since v4:
> - rebase on top of nfsd-next tree
> Changes since v3:
> - drop write_maxconn and write_maxblksize for the moment
> - add write_version and write_ports commands
> Changes since v2:
> - use u32 to store nthreads in nfsd_nl_threads_set_doit
> - rename server-attr in control-plane in nfsd.yaml specs
> Changes since v1:
> - remove write_v4_end_grace command
> - add write_maxblksize and write_maxconn netlink commands
> 
> This patch can be tested with user-space tool reported below:
> https://github.com/LorenzoBianconi/nfsd-netlink.git
> 
> Lorenzo Bianconi (3):
>   NFSD: convert write_threads to netlink command
>   NFSD: add write_version to netlink command
>   NFSD: add write_ports to netlink command
> 
>  Documentation/netlink/specs/nfsd.yaml |  94 ++++++
>  fs/nfsd/netlink.c                     |  63 ++++
>  fs/nfsd/netlink.h                     |  10 +
>  fs/nfsd/nfsctl.c                      | 396 ++++++++++++++++++++++
>  include/uapi/linux/nfsd_netlink.h     |  44 +++
>  tools/net/ynl/generated/nfsd-user.c   | 460 ++++++++++++++++++++++++++
>  tools/net/ynl/generated/nfsd-user.h   | 155 +++++++++
>  7 files changed, 1222 insertions(+)
> 


I think this is really close and coming together! Before we merge this
though, I'd _really_ like to see some patches for rpc.nfsd in nfs-utils.
Until we try to implement the userland bits, we won't know if we've
gotten this interface right.

...and before that, we really need to have some sort of userland program
packaged and available for querying the new netlink RPC stats from nfsd.
You have the simple userland one on github, but I think we need omething
packaged, ideally as part of nfs-utils.

Doing that first would allow you to add the necessary autoconf/libtool
stuff to pull in the netlink libraries, which will be a prerequisite for
doing the userland rpc.nfsd work, and will probably be a bit simpler
than modifying rpc.nfsd.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 13:41   ` Jeff Layton
@ 2024-01-22 15:35     ` Lorenzo Bianconi
  2024-01-22 15:50       ` Jeff Layton
  2024-01-22 21:35     ` NeilBrown
  1 sibling, 1 reply; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-22 15:35 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-nfs, lorenzo.bianconi, neilb, kuba, chuck.lever, horms, netdev

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

> On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > Introduce write_ports netlink command. For listener-set, userspace is
> > expected to provide a NFS listeners list it wants to enable (all the
> > other ports will be closed).
> > 
> 
> Ditto here. This is a change to a declarative interface, which I think
> is a better way to handle this, but we should be aware of the change.
> 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  Documentation/netlink/specs/nfsd.yaml |  37 +++++
> >  fs/nfsd/netlink.c                     |  23 +++
> >  fs/nfsd/netlink.h                     |   3 +
> >  fs/nfsd/nfsctl.c                      | 196 ++++++++++++++++++++++++++
> >  include/uapi/linux/nfsd_netlink.h     |  18 +++
> >  tools/net/ynl/generated/nfsd-user.c   | 191 +++++++++++++++++++++++++
> >  tools/net/ynl/generated/nfsd-user.h   |  55 ++++++++
> >  7 files changed, 523 insertions(+)
> > 
> > diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> > index 30f18798e84e..296ff24b23ac 100644
> > --- a/Documentation/netlink/specs/nfsd.yaml
> > +++ b/Documentation/netlink/specs/nfsd.yaml
> > @@ -85,6 +85,26 @@ attribute-sets:
> >          type: nest
> >          nested-attributes: nfs-version
> >          multi-attr: true
> > +  -
> > +    name: server-instance
> > +    attributes:
> > +      -
> > +        name: transport-name
> > +        type: string
> > +      -
> > +        name: port
> > +        type: u32
> > +      -
> > +        name: inet-proto
> > +        type: u16
> > +  -
> > +    name: server-listener
> > +    attributes:
> > +      -
> > +        name: instance
> > +        type: nest
> > +        nested-attributes: server-instance
> > +        multi-attr: true
> >  
> >  operations:
> >    list:
> > @@ -144,3 +164,20 @@ operations:
> >          reply:
> >            attributes:
> >              - version
> > +    -
> > +      name: listener-set
> > +      doc: set nfs running listeners
> > +      attribute-set: server-listener
> > +      flags: [ admin-perm ]
> > +      do:
> > +        request:
> > +          attributes:
> > +            - instance
> > +    -
> > +      name: listener-get
> > +      doc: get nfs running listeners
> > +      attribute-set: server-listener
> > +      do:
> > +        reply:
> > +          attributes:
> > +            - instance
> > diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> > index 5cbbd3295543..c772f9e14761 100644
> > --- a/fs/nfsd/netlink.c
> > +++ b/fs/nfsd/netlink.c
> > @@ -16,6 +16,12 @@ const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1]
> >  	[NFSD_A_NFS_VERSION_MINOR] = { .type = NLA_U32, },
> >  };
> >  
> > +const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1] = {
> > +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
> > +	[NFSD_A_SERVER_INSTANCE_PORT] = { .type = NLA_U32, },
> > +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .type = NLA_U16, },
> > +};
> > +
> >  /* 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, },
> > @@ -26,6 +32,11 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VE
> >  	[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_nfs_version_nl_policy),
> >  };
> >  
> > +/* NFSD_CMD_LISTENER_SET - do */
> > +static const struct nla_policy nfsd_listener_set_nl_policy[NFSD_A_SERVER_LISTENER_INSTANCE + 1] = {
> > +	[NFSD_A_SERVER_LISTENER_INSTANCE] = NLA_POLICY_NESTED(nfsd_server_instance_nl_policy),
> > +};
> > +
> >  /* Ops table for nfsd */
> >  static const struct genl_split_ops nfsd_nl_ops[] = {
> >  	{
> > @@ -59,6 +70,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> >  		.doit	= nfsd_nl_version_get_doit,
> >  		.flags	= GENL_CMD_CAP_DO,
> >  	},
> > +	{
> > +		.cmd		= NFSD_CMD_LISTENER_SET,
> > +		.doit		= nfsd_nl_listener_set_doit,
> > +		.policy		= nfsd_listener_set_nl_policy,
> > +		.maxattr	= NFSD_A_SERVER_LISTENER_INSTANCE,
> > +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > +	},
> > +	{
> > +		.cmd	= NFSD_CMD_LISTENER_GET,
> > +		.doit	= nfsd_nl_listener_get_doit,
> > +		.flags	= GENL_CMD_CAP_DO,
> > +	},
> >  };
> >  
> >  struct genl_family nfsd_nl_family __ro_after_init = {
> > diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
> > index c9a1be693fef..10a26ad32cd0 100644
> > --- a/fs/nfsd/netlink.h
> > +++ b/fs/nfsd/netlink.h
> > @@ -13,6 +13,7 @@
> >  
> >  /* Common nested types */
> >  extern const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1];
> > +extern const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1];
> >  
> >  int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
> >  int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
> > @@ -23,6 +24,8 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
> >  int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
> >  int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
> >  int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
> > +int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info);
> > +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info);
> >  
> >  extern struct genl_family nfsd_nl_family;
> >  
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index 53af82303f93..562b209f2921 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -1896,6 +1896,202 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
> >  	return err;
> >  }
> >  
> > +/**
> > + * nfsd_nl_listener_set_doit - set the nfs running listeners
> > + * @skb: reply buffer
> > + * @info: netlink metadata and command arguments
> > + *
> > + * Return 0 on success or a negative errno.
> > + */
> > +int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > +	struct nlattr *tb[ARRAY_SIZE(nfsd_server_instance_nl_policy)];
> > +	struct net *net = genl_info_net(info);
> > +	struct svc_xprt *xprt, *tmp_xprt;
> > +	const struct nlattr *attr;
> > +	struct svc_serv *serv;
> > +	const char *xcl_name;
> > +	struct nfsd_net *nn;
> > +	int port, err, rem;
> > +	sa_family_t af;
> > +
> > +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_INSTANCE))
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&nfsd_mutex);
> > +
> > +	err = nfsd_create_serv(net);
> > +	if (err) {
> > +		mutex_unlock(&nfsd_mutex);
> > +		return err;
> > +	}
> > +
> > +	nn = net_generic(net, nfsd_net_id);
> > +	serv = nn->nfsd_serv;
> > +
> > +	/* 1- create brand new listeners */
> > +	nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> > +		if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
> > +			continue;
> > +
> > +		if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> > +				     nfsd_server_instance_nl_policy,
> > +				     info->extack) < 0)
> > +			continue;
> > +
> > +		if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
> > +		    !tb[NFSD_A_SERVER_INSTANCE_PORT])
> > +			continue;
> > +
> > +		xcl_name = nla_data(tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
> > +		port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
> > +		if (port < 1 || port > USHRT_MAX)
> > +			continue;
> > +
> > +		af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
> > +		if (af != PF_INET && af != PF_INET6)
> > +			continue;
> > +
> > +		xprt = svc_find_xprt(serv, xcl_name, net, PF_INET, port);
> > +		if (xprt) {
> > +			svc_xprt_put(xprt);
> > +			continue;
> > +		}
> > +
> > +		/* create new listerner */
> > +		if (svc_xprt_create(serv, xcl_name, net, af, port,
> > +				    SVC_SOCK_ANONYMOUS, get_current_cred()))
> > +			continue;
> > +	}
> > +
> > +	/* 2- remove stale listeners */
> 
> 
> The old portlist interface was weird, in that it was only additive. You
> couldn't use it to close a listening socket (AFAICT). We may be able to
> support that now with this interface, but we'll need to test that case
> carefully.
> 
> 
> 
> > +	spin_lock_bh(&serv->sv_lock);
> > +	list_for_each_entry_safe(xprt, tmp_xprt, &serv->sv_permsocks,
> > +				 xpt_list) {
> > +		struct svc_xprt *rqt_xprt = NULL;
> > +
> > +		nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> > +			if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
> > +				continue;
> > +
> > +			if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> > +					     nfsd_server_instance_nl_policy,
> > +					     info->extack) < 0)
> > +				continue;
> > +
> > +			if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
> > +			    !tb[NFSD_A_SERVER_INSTANCE_PORT])
> > +				continue;
> > +
> > +			xcl_name = nla_data(
> > +				tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
> > +			port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
> > +			if (port < 1 || port > USHRT_MAX)
> > +				continue;
> > +
> > +			af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
> > +			if (af != PF_INET && af != PF_INET6)
> > +				continue;
> > +
> > +			if (!strcmp(xprt->xpt_class->xcl_name, xcl_name) &&
> > +			    port == svc_xprt_local_port(xprt) &&
> > +			    af == xprt->xpt_local.ss_family &&
> > +			    xprt->xpt_net == net) {
> > +				rqt_xprt = xprt;
> > +				break;
> > +			}
> > +		}
> > +
> > +		/* remove stale listener */
> > +		if (!rqt_xprt) {
> > +			spin_unlock_bh(&serv->sv_lock);
> > +			svc_xprt_close(xprt);
> > 
> 
> I'm not sure this is safe. Can anything else modify sv_permsocks while
> you're not holding the lock? Maybe not since you're holding the
> nfsd_mutex, but it's still probably best to restart the list walk if you
> have to drop the lock here.
> 
> You're typically only going to have a few sockets here anyway -- usually
> just one each for TCP, UDP and maybe RDMA.

what about beeing a bit proactive and set XPT_CLOSE bit before releasing the
spinlock (as we already do in svc_xprt_close)?

Regards,
Lorenzo

> 
> 
> > +			spin_lock_bh(&serv->sv_lock);
> > +		}
> > +	}
> > +	spin_unlock_bh(&serv->sv_lock);
> > +
> > +	if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
> > +		nfsd_destroy_serv(net);
> > +
> > +	mutex_unlock(&nfsd_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * nfsd_nl_listener_get_doit - get the nfs running listeners
> > + * @skb: reply buffer
> > + * @info: netlink metadata and command arguments
> > + *
> > + * Return 0 on success or a negative errno.
> > + */
> > +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > +	struct svc_xprt *xprt;
> > +	struct svc_serv *serv;
> > +	struct nfsd_net *nn;
> > +	void *hdr;
> > +	int err;
> > +
> > +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > +	if (!skb)
> > +		return -ENOMEM;
> > +
> > +	hdr = genlmsg_iput(skb, info);
> > +	if (!hdr) {
> > +		err = -EMSGSIZE;
> > +		goto err_free_msg;
> > +	}
> > +
> > +	mutex_lock(&nfsd_mutex);
> > +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> > +	if (!nn->nfsd_serv) {
> > +		err = -EINVAL;
> > +		goto err_nfsd_unlock;
> > +	}
> > +
> > +	serv = nn->nfsd_serv;
> > +	spin_lock_bh(&serv->sv_lock);
> > +	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> > +		struct nlattr *attr;
> > +
> > +		attr = nla_nest_start_noflag(skb,
> > +					     NFSD_A_SERVER_LISTENER_INSTANCE);
> > +		if (!attr) {
> > +			err = -EINVAL;
> > +			goto err_serv_unlock;
> > +		}
> > +
> > +		if (nla_put_string(skb, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME,
> > +				   xprt->xpt_class->xcl_name) ||
> > +		    nla_put_u32(skb, NFSD_A_SERVER_INSTANCE_PORT,
> > +				svc_xprt_local_port(xprt)) ||
> > +		    nla_put_u16(skb, NFSD_A_SERVER_INSTANCE_INET_PROTO,
> > +				xprt->xpt_local.ss_family)) {
> > +			err = -EINVAL;
> > +			goto err_serv_unlock;
> > +		}
> > +
> > +		nla_nest_end(skb, attr);
> > +	}
> > +	spin_unlock_bh(&serv->sv_lock);
> > +	mutex_unlock(&nfsd_mutex);
> > +
> > +	genlmsg_end(skb, hdr);
> > +
> > +	return genlmsg_reply(skb, info);
> > +
> > +err_serv_unlock:
> > +	spin_unlock_bh(&serv->sv_lock);
> > +err_nfsd_unlock:
> > +	mutex_unlock(&nfsd_mutex);
> > +err_free_msg:
> > +	nlmsg_free(skb);
> > +
> > +	return err;
> > +}
> > +
> >  /**
> >   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
> >   * @net: a freshly-created network namespace
> > diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> > index 2a06f9fe6fe9..659ab76b8840 100644
> > --- a/include/uapi/linux/nfsd_netlink.h
> > +++ b/include/uapi/linux/nfsd_netlink.h
> > @@ -51,12 +51,30 @@ enum {
> >  	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
> >  };
> >  
> > +enum {
> > +	NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME = 1,
> > +	NFSD_A_SERVER_INSTANCE_PORT,
> > +	NFSD_A_SERVER_INSTANCE_INET_PROTO,
> > +
> > +	__NFSD_A_SERVER_INSTANCE_MAX,
> > +	NFSD_A_SERVER_INSTANCE_MAX = (__NFSD_A_SERVER_INSTANCE_MAX - 1)
> > +};
> > +
> > +enum {
> > +	NFSD_A_SERVER_LISTENER_INSTANCE = 1,
> > +
> > +	__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_SET,
> > +	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 ad498543f464..d52f392c7f59 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_SET] = "listener-set",
> > +	[NFSD_CMD_LISTENER_GET] = "listener-get",
> >  };
> >  
> >  const char *nfsd_op_str(int op)
> > @@ -39,6 +41,17 @@ struct ynl_policy_nest nfsd_nfs_version_nest = {
> >  	.table = nfsd_nfs_version_policy,
> >  };
> >  
> > +struct ynl_policy_attr nfsd_server_instance_policy[NFSD_A_SERVER_INSTANCE_MAX + 1] = {
> > +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
> > +	[NFSD_A_SERVER_INSTANCE_PORT] = { .name = "port", .type = YNL_PT_U32, },
> > +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .name = "inet-proto", .type = YNL_PT_U16, },
> > +};
> > +
> > +struct ynl_policy_nest nfsd_server_instance_nest = {
> > +	.max_attr = NFSD_A_SERVER_INSTANCE_MAX,
> > +	.table = nfsd_server_instance_policy,
> > +};
> > +
> >  struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
> >  	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
> >  	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
> > @@ -79,6 +92,15 @@ struct ynl_policy_nest nfsd_server_proto_nest = {
> >  	.table = nfsd_server_proto_policy,
> >  };
> >  
> > +struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
> > +	[NFSD_A_SERVER_LISTENER_INSTANCE] = { .name = "instance", .type = YNL_PT_NEST, .nest = &nfsd_server_instance_nest, },
> > +};
> > +
> > +struct ynl_policy_nest nfsd_server_listener_nest = {
> > +	.max_attr = NFSD_A_SERVER_LISTENER_MAX,
> > +	.table = nfsd_server_listener_policy,
> > +};
> > +
> >  /* Common nested types */
> >  void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
> >  {
> > @@ -124,6 +146,64 @@ int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
> >  	return 0;
> >  }
> >  
> > +void nfsd_server_instance_free(struct nfsd_server_instance *obj)
> > +{
> > +	free(obj->transport_name);
> > +}
> > +
> > +int nfsd_server_instance_put(struct nlmsghdr *nlh, unsigned int attr_type,
> > +			     struct nfsd_server_instance *obj)
> > +{
> > +	struct nlattr *nest;
> > +
> > +	nest = mnl_attr_nest_start(nlh, attr_type);
> > +	if (obj->_present.transport_name_len)
> > +		mnl_attr_put_strz(nlh, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME, obj->transport_name);
> > +	if (obj->_present.port)
> > +		mnl_attr_put_u32(nlh, NFSD_A_SERVER_INSTANCE_PORT, obj->port);
> > +	if (obj->_present.inet_proto)
> > +		mnl_attr_put_u16(nlh, NFSD_A_SERVER_INSTANCE_INET_PROTO, obj->inet_proto);
> > +	mnl_attr_nest_end(nlh, nest);
> > +
> > +	return 0;
> > +}
> > +
> > +int nfsd_server_instance_parse(struct ynl_parse_arg *yarg,
> > +			       const struct nlattr *nested)
> > +{
> > +	struct nfsd_server_instance *dst = yarg->data;
> > +	const struct nlattr *attr;
> > +
> > +	mnl_attr_for_each_nested(attr, nested) {
> > +		unsigned int type = mnl_attr_get_type(attr);
> > +
> > +		if (type == NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME) {
> > +			unsigned int len;
> > +
> > +			if (ynl_attr_validate(yarg, attr))
> > +				return MNL_CB_ERROR;
> > +
> > +			len = strnlen(mnl_attr_get_str(attr), mnl_attr_get_payload_len(attr));
> > +			dst->_present.transport_name_len = len;
> > +			dst->transport_name = malloc(len + 1);
> > +			memcpy(dst->transport_name, mnl_attr_get_str(attr), len);
> > +			dst->transport_name[len] = 0;
> > +		} else if (type == NFSD_A_SERVER_INSTANCE_PORT) {
> > +			if (ynl_attr_validate(yarg, attr))
> > +				return MNL_CB_ERROR;
> > +			dst->_present.port = 1;
> > +			dst->port = mnl_attr_get_u32(attr);
> > +		} else if (type == NFSD_A_SERVER_INSTANCE_INET_PROTO) {
> > +			if (ynl_attr_validate(yarg, attr))
> > +				return MNL_CB_ERROR;
> > +			dst->_present.inet_proto = 1;
> > +			dst->inet_proto = mnl_attr_get_u16(attr);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> >  int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
> > @@ -467,6 +547,117 @@ struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
> >  	return NULL;
> >  }
> >  
> > +/* ============== NFSD_CMD_LISTENER_SET ============== */
> > +/* NFSD_CMD_LISTENER_SET - do */
> > +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < req->n_instance; i++)
> > +		nfsd_server_instance_free(&req->instance[i]);
> > +	free(req->instance);
> > +	free(req);
> > +}
> > +
> > +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req)
> > +{
> > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > +	struct nlmsghdr *nlh;
> > +	int err;
> > +
> > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_SET, 1);
> > +	ys->req_policy = &nfsd_server_listener_nest;
> > +
> > +	for (unsigned int i = 0; i < req->n_instance; i++)
> > +		nfsd_server_instance_put(nlh, NFSD_A_SERVER_LISTENER_INSTANCE, &req->instance[i]);
> > +
> > +	err = ynl_exec(ys, nlh, &yrs);
> > +	if (err < 0)
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > +/* NFSD_CMD_LISTENER_GET - do */
> > +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp)
> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < rsp->n_instance; i++)
> > +		nfsd_server_instance_free(&rsp->instance[i]);
> > +	free(rsp->instance);
> > +	free(rsp);
> > +}
> > +
> > +int nfsd_listener_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> > +{
> > +	struct nfsd_listener_get_rsp *dst;
> > +	struct ynl_parse_arg *yarg = data;
> > +	unsigned int n_instance = 0;
> > +	const struct nlattr *attr;
> > +	struct ynl_parse_arg parg;
> > +	int i;
> > +
> > +	dst = yarg->data;
> > +	parg.ys = yarg->ys;
> > +
> > +	if (dst->instance)
> > +		return ynl_error_parse(yarg, "attribute already present (server-listener.instance)");
> > +
> > +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > +		unsigned int type = mnl_attr_get_type(attr);
> > +
> > +		if (type == NFSD_A_SERVER_LISTENER_INSTANCE) {
> > +			n_instance++;
> > +		}
> > +	}
> > +
> > +	if (n_instance) {
> > +		dst->instance = calloc(n_instance, sizeof(*dst->instance));
> > +		dst->n_instance = n_instance;
> > +		i = 0;
> > +		parg.rsp_policy = &nfsd_server_instance_nest;
> > +		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > +			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_LISTENER_INSTANCE) {
> > +				parg.data = &dst->instance[i];
> > +				if (nfsd_server_instance_parse(&parg, attr))
> > +					return MNL_CB_ERROR;
> > +				i++;
> > +			}
> > +		}
> > +	}
> > +
> > +	return MNL_CB_OK;
> > +}
> > +
> > +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys)
> > +{
> > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > +	struct nfsd_listener_get_rsp *rsp;
> > +	struct nlmsghdr *nlh;
> > +	int err;
> > +
> > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
> > +	ys->req_policy = &nfsd_server_listener_nest;
> > +	yrs.yarg.rsp_policy = &nfsd_server_listener_nest;
> > +
> > +	rsp = calloc(1, sizeof(*rsp));
> > +	yrs.yarg.data = rsp;
> > +	yrs.cb = nfsd_listener_get_rsp_parse;
> > +	yrs.rsp_cmd = NFSD_CMD_LISTENER_GET;
> > +
> > +	err = ynl_exec(ys, nlh, &yrs);
> > +	if (err < 0)
> > +		goto err_free;
> > +
> > +	return rsp;
> > +
> > +err_free:
> > +	nfsd_listener_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 d062ee8fa8b6..5765fb6f2ef5 100644
> > --- a/tools/net/ynl/generated/nfsd-user.h
> > +++ b/tools/net/ynl/generated/nfsd-user.h
> > @@ -29,6 +29,18 @@ struct nfsd_nfs_version {
> >  	__u32 minor;
> >  };
> >  
> > +struct nfsd_server_instance {
> > +	struct {
> > +		__u32 transport_name_len;
> > +		__u32 port:1;
> > +		__u32 inet_proto:1;
> > +	} _present;
> > +
> > +	char *transport_name;
> > +	__u32 port;
> > +	__u16 inet_proto;
> > +};
> > +
> >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> >  struct nfsd_rpc_status_get_rsp_dump {
> > @@ -164,4 +176,47 @@ void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
> >   */
> >  struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
> >  
> > +/* ============== NFSD_CMD_LISTENER_SET ============== */
> > +/* NFSD_CMD_LISTENER_SET - do */
> > +struct nfsd_listener_set_req {
> > +	unsigned int n_instance;
> > +	struct nfsd_server_instance *instance;
> > +};
> > +
> > +static inline struct nfsd_listener_set_req *nfsd_listener_set_req_alloc(void)
> > +{
> > +	return calloc(1, sizeof(struct nfsd_listener_set_req));
> > +}
> > +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req);
> > +
> > +static inline void
> > +__nfsd_listener_set_req_set_instance(struct nfsd_listener_set_req *req,
> > +				     struct nfsd_server_instance *instance,
> > +				     unsigned int n_instance)
> > +{
> > +	free(req->instance);
> > +	req->instance = instance;
> > +	req->n_instance = n_instance;
> > +}
> > +
> > +/*
> > + * set nfs running listeners
> > + */
> > +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req);
> > +
> > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > +/* NFSD_CMD_LISTENER_GET - do */
> > +
> > +struct nfsd_listener_get_rsp {
> > +	unsigned int n_instance;
> > +	struct nfsd_server_instance *instance;
> > +};
> > +
> > +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp);
> > +
> > +/*
> > + * get nfs running listeners
> > + */
> > +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys);
> > +
> >  #endif /* _LINUX_NFSD_GEN_H */
> 
> -- 
> Jeff Layton <jlayton@kernel.org>
> 

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 15:35     ` Lorenzo Bianconi
@ 2024-01-22 15:50       ` Jeff Layton
  2024-01-22 15:59         ` Lorenzo Bianconi
  0 siblings, 1 reply; 27+ messages in thread
From: Jeff Layton @ 2024-01-22 15:50 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-nfs, lorenzo.bianconi, neilb, kuba, chuck.lever, horms, netdev

On Mon, 2024-01-22 at 16:35 +0100, Lorenzo Bianconi wrote:
> > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > Introduce write_ports netlink command. For listener-set, userspace is
> > > expected to provide a NFS listeners list it wants to enable (all the
> > > other ports will be closed).
> > > 
> > 
> > Ditto here. This is a change to a declarative interface, which I think
> > is a better way to handle this, but we should be aware of the change.
> > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > >  Documentation/netlink/specs/nfsd.yaml |  37 +++++
> > >  fs/nfsd/netlink.c                     |  23 +++
> > >  fs/nfsd/netlink.h                     |   3 +
> > >  fs/nfsd/nfsctl.c                      | 196 ++++++++++++++++++++++++++
> > >  include/uapi/linux/nfsd_netlink.h     |  18 +++
> > >  tools/net/ynl/generated/nfsd-user.c   | 191 +++++++++++++++++++++++++
> > >  tools/net/ynl/generated/nfsd-user.h   |  55 ++++++++
> > >  7 files changed, 523 insertions(+)
> > > 
> > > diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> > > index 30f18798e84e..296ff24b23ac 100644
> > > --- a/Documentation/netlink/specs/nfsd.yaml
> > > +++ b/Documentation/netlink/specs/nfsd.yaml
> > > @@ -85,6 +85,26 @@ attribute-sets:
> > >          type: nest
> > >          nested-attributes: nfs-version
> > >          multi-attr: true
> > > +  -
> > > +    name: server-instance
> > > +    attributes:
> > > +      -
> > > +        name: transport-name
> > > +        type: string
> > > +      -
> > > +        name: port
> > > +        type: u32
> > > +      -
> > > +        name: inet-proto
> > > +        type: u16
> > > +  -
> > > +    name: server-listener
> > > +    attributes:
> > > +      -
> > > +        name: instance
> > > +        type: nest
> > > +        nested-attributes: server-instance
> > > +        multi-attr: true
> > >  
> > > 
> > > 
> > > 
> > >  operations:
> > >    list:
> > > @@ -144,3 +164,20 @@ operations:
> > >          reply:
> > >            attributes:
> > >              - version
> > > +    -
> > > +      name: listener-set
> > > +      doc: set nfs running listeners
> > > +      attribute-set: server-listener
> > > +      flags: [ admin-perm ]
> > > +      do:
> > > +        request:
> > > +          attributes:
> > > +            - instance
> > > +    -
> > > +      name: listener-get
> > > +      doc: get nfs running listeners
> > > +      attribute-set: server-listener
> > > +      do:
> > > +        reply:
> > > +          attributes:
> > > +            - instance
> > > diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> > > index 5cbbd3295543..c772f9e14761 100644
> > > --- a/fs/nfsd/netlink.c
> > > +++ b/fs/nfsd/netlink.c
> > > @@ -16,6 +16,12 @@ const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1]
> > >  	[NFSD_A_NFS_VERSION_MINOR] = { .type = NLA_U32, },
> > >  };
> > >  
> > > 
> > > 
> > > 
> > > +const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1] = {
> > > +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
> > > +	[NFSD_A_SERVER_INSTANCE_PORT] = { .type = NLA_U32, },
> > > +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .type = NLA_U16, },
> > > +};
> > > +
> > >  /* 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, },
> > > @@ -26,6 +32,11 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_PROTO_VE
> > >  	[NFSD_A_SERVER_PROTO_VERSION] = NLA_POLICY_NESTED(nfsd_nfs_version_nl_policy),
> > >  };
> > >  
> > > 
> > > 
> > > 
> > > +/* NFSD_CMD_LISTENER_SET - do */
> > > +static const struct nla_policy nfsd_listener_set_nl_policy[NFSD_A_SERVER_LISTENER_INSTANCE + 1] = {
> > > +	[NFSD_A_SERVER_LISTENER_INSTANCE] = NLA_POLICY_NESTED(nfsd_server_instance_nl_policy),
> > > +};
> > > +
> > >  /* Ops table for nfsd */
> > >  static const struct genl_split_ops nfsd_nl_ops[] = {
> > >  	{
> > > @@ -59,6 +70,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> > >  		.doit	= nfsd_nl_version_get_doit,
> > >  		.flags	= GENL_CMD_CAP_DO,
> > >  	},
> > > +	{
> > > +		.cmd		= NFSD_CMD_LISTENER_SET,
> > > +		.doit		= nfsd_nl_listener_set_doit,
> > > +		.policy		= nfsd_listener_set_nl_policy,
> > > +		.maxattr	= NFSD_A_SERVER_LISTENER_INSTANCE,
> > > +		.flags		= GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > > +	},
> > > +	{
> > > +		.cmd	= NFSD_CMD_LISTENER_GET,
> > > +		.doit	= nfsd_nl_listener_get_doit,
> > > +		.flags	= GENL_CMD_CAP_DO,
> > > +	},
> > >  };
> > >  
> > > 
> > > 
> > > 
> > >  struct genl_family nfsd_nl_family __ro_after_init = {
> > > diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
> > > index c9a1be693fef..10a26ad32cd0 100644
> > > --- a/fs/nfsd/netlink.h
> > > +++ b/fs/nfsd/netlink.h
> > > @@ -13,6 +13,7 @@
> > >  
> > > 
> > > 
> > > 
> > >  /* Common nested types */
> > >  extern const struct nla_policy nfsd_nfs_version_nl_policy[NFSD_A_NFS_VERSION_MINOR + 1];
> > > +extern const struct nla_policy nfsd_server_instance_nl_policy[NFSD_A_SERVER_INSTANCE_INET_PROTO + 1];
> > >  
> > > 
> > > 
> > > 
> > >  int nfsd_nl_rpc_status_get_start(struct netlink_callback *cb);
> > >  int nfsd_nl_rpc_status_get_done(struct netlink_callback *cb);
> > > @@ -23,6 +24,8 @@ int nfsd_nl_threads_set_doit(struct sk_buff *skb, struct genl_info *info);
> > >  int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
> > >  int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
> > >  int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info);
> > > +int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info);
> > > +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info);
> > >  
> > > 
> > > 
> > > 
> > >  extern struct genl_family nfsd_nl_family;
> > >  
> > > 
> > > 
> > > 
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 53af82303f93..562b209f2921 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -1896,6 +1896,202 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info)
> > >  	return err;
> > >  }
> > >  
> > > 
> > > 
> > > 
> > > +/**
> > > + * nfsd_nl_listener_set_doit - set the nfs running listeners
> > > + * @skb: reply buffer
> > > + * @info: netlink metadata and command arguments
> > > + *
> > > + * Return 0 on success or a negative errno.
> > > + */
> > > +int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
> > > +{
> > > +	struct nlattr *tb[ARRAY_SIZE(nfsd_server_instance_nl_policy)];
> > > +	struct net *net = genl_info_net(info);
> > > +	struct svc_xprt *xprt, *tmp_xprt;
> > > +	const struct nlattr *attr;
> > > +	struct svc_serv *serv;
> > > +	const char *xcl_name;
> > > +	struct nfsd_net *nn;
> > > +	int port, err, rem;
> > > +	sa_family_t af;
> > > +
> > > +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_INSTANCE))
> > > +		return -EINVAL;
> > > +
> > > +	mutex_lock(&nfsd_mutex);
> > > +
> > > +	err = nfsd_create_serv(net);
> > > +	if (err) {
> > > +		mutex_unlock(&nfsd_mutex);
> > > +		return err;
> > > +	}
> > > +
> > > +	nn = net_generic(net, nfsd_net_id);
> > > +	serv = nn->nfsd_serv;
> > > +
> > > +	/* 1- create brand new listeners */
> > > +	nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> > > +		if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
> > > +			continue;
> > > +
> > > +		if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> > > +				     nfsd_server_instance_nl_policy,
> > > +				     info->extack) < 0)
> > > +			continue;
> > > +
> > > +		if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
> > > +		    !tb[NFSD_A_SERVER_INSTANCE_PORT])
> > > +			continue;
> > > +
> > > +		xcl_name = nla_data(tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
> > > +		port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
> > > +		if (port < 1 || port > USHRT_MAX)
> > > +			continue;
> > > +
> > > +		af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
> > > +		if (af != PF_INET && af != PF_INET6)
> > > +			continue;
> > > +
> > > +		xprt = svc_find_xprt(serv, xcl_name, net, PF_INET, port);
> > > +		if (xprt) {
> > > +			svc_xprt_put(xprt);
> > > +			continue;
> > > +		}
> > > +
> > > +		/* create new listerner */
> > > +		if (svc_xprt_create(serv, xcl_name, net, af, port,
> > > +				    SVC_SOCK_ANONYMOUS, get_current_cred()))
> > > +			continue;
> > > +	}
> > > +
> > > +	/* 2- remove stale listeners */
> > 
> > 
> > The old portlist interface was weird, in that it was only additive. You
> > couldn't use it to close a listening socket (AFAICT). We may be able to
> > support that now with this interface, but we'll need to test that case
> > carefully.
> > 
> > 
> > 
> > > +	spin_lock_bh(&serv->sv_lock);
> > > +	list_for_each_entry_safe(xprt, tmp_xprt, &serv->sv_permsocks,
> > > +				 xpt_list) {
> > > +		struct svc_xprt *rqt_xprt = NULL;
> > > +
> > > +		nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
> > > +			if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
> > > +				continue;
> > > +
> > > +			if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
> > > +					     nfsd_server_instance_nl_policy,
> > > +					     info->extack) < 0)
> > > +				continue;
> > > +
> > > +			if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
> > > +			    !tb[NFSD_A_SERVER_INSTANCE_PORT])
> > > +				continue;
> > > +
> > > +			xcl_name = nla_data(
> > > +				tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
> > > +			port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
> > > +			if (port < 1 || port > USHRT_MAX)
> > > +				continue;
> > > +
> > > +			af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
> > > +			if (af != PF_INET && af != PF_INET6)
> > > +				continue;
> > > +
> > > +			if (!strcmp(xprt->xpt_class->xcl_name, xcl_name) &&
> > > +			    port == svc_xprt_local_port(xprt) &&
> > > +			    af == xprt->xpt_local.ss_family &&
> > > +			    xprt->xpt_net == net) {
> > > +				rqt_xprt = xprt;
> > > +				break;
> > > +			}
> > > +		}
> > > +
> > > +		/* remove stale listener */
> > > +		if (!rqt_xprt) {
> > > +			spin_unlock_bh(&serv->sv_lock);
> > > +			svc_xprt_close(xprt);
> > > 
> > 
> > I'm not sure this is safe. Can anything else modify sv_permsocks while
> > you're not holding the lock? Maybe not since you're holding the
> > nfsd_mutex, but it's still probably best to restart the list walk if you
> > have to drop the lock here.
> > 
> > You're typically only going to have a few sockets here anyway -- usually
> > just one each for TCP, UDP and maybe RDMA.
> 
> what about beeing a bit proactive and set XPT_CLOSE bit before releasing the
> spinlock (as we already do in svc_xprt_close)?
> 

That does sound better, actually. You might have to open-code parts of
svc_xprt_close, but it's not that big anyway.


> > 
> > 
> > > +			spin_lock_bh(&serv->sv_lock);
> > > +		}
> > > +	}
> > > +	spin_unlock_bh(&serv->sv_lock);
> > > +
> > > +	if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
> > > +		nfsd_destroy_serv(net);
> > > +
> > > +	mutex_unlock(&nfsd_mutex);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/**
> > > + * nfsd_nl_listener_get_doit - get the nfs running listeners
> > > + * @skb: reply buffer
> > > + * @info: netlink metadata and command arguments
> > > + *
> > > + * Return 0 on success or a negative errno.
> > > + */
> > > +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
> > > +{
> > > +	struct svc_xprt *xprt;
> > > +	struct svc_serv *serv;
> > > +	struct nfsd_net *nn;
> > > +	void *hdr;
> > > +	int err;
> > > +
> > > +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > > +	if (!skb)
> > > +		return -ENOMEM;
> > > +
> > > +	hdr = genlmsg_iput(skb, info);
> > > +	if (!hdr) {
> > > +		err = -EMSGSIZE;
> > > +		goto err_free_msg;
> > > +	}
> > > +
> > > +	mutex_lock(&nfsd_mutex);
> > > +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> > > +	if (!nn->nfsd_serv) {
> > > +		err = -EINVAL;
> > > +		goto err_nfsd_unlock;
> > > +	}
> > > +
> > > +	serv = nn->nfsd_serv;
> > > +	spin_lock_bh(&serv->sv_lock);
> > > +	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> > > +		struct nlattr *attr;
> > > +
> > > +		attr = nla_nest_start_noflag(skb,
> > > +					     NFSD_A_SERVER_LISTENER_INSTANCE);
> > > +		if (!attr) {
> > > +			err = -EINVAL;
> > > +			goto err_serv_unlock;
> > > +		}
> > > +
> > > +		if (nla_put_string(skb, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME,
> > > +				   xprt->xpt_class->xcl_name) ||
> > > +		    nla_put_u32(skb, NFSD_A_SERVER_INSTANCE_PORT,
> > > +				svc_xprt_local_port(xprt)) ||
> > > +		    nla_put_u16(skb, NFSD_A_SERVER_INSTANCE_INET_PROTO,
> > > +				xprt->xpt_local.ss_family)) {
> > > +			err = -EINVAL;
> > > +			goto err_serv_unlock;
> > > +		}
> > > +
> > > +		nla_nest_end(skb, attr);
> > > +	}
> > > +	spin_unlock_bh(&serv->sv_lock);
> > > +	mutex_unlock(&nfsd_mutex);
> > > +
> > > +	genlmsg_end(skb, hdr);
> > > +
> > > +	return genlmsg_reply(skb, info);
> > > +
> > > +err_serv_unlock:
> > > +	spin_unlock_bh(&serv->sv_lock);
> > > +err_nfsd_unlock:
> > > +	mutex_unlock(&nfsd_mutex);
> > > +err_free_msg:
> > > +	nlmsg_free(skb);
> > > +
> > > +	return err;
> > > +}
> > > +
> > >  /**
> > >   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
> > >   * @net: a freshly-created network namespace
> > > diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> > > index 2a06f9fe6fe9..659ab76b8840 100644
> > > --- a/include/uapi/linux/nfsd_netlink.h
> > > +++ b/include/uapi/linux/nfsd_netlink.h
> > > @@ -51,12 +51,30 @@ enum {
> > >  	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
> > >  };
> > >  
> > > 
> > > +enum {
> > > +	NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME = 1,
> > > +	NFSD_A_SERVER_INSTANCE_PORT,
> > > +	NFSD_A_SERVER_INSTANCE_INET_PROTO,
> > > +
> > > +	__NFSD_A_SERVER_INSTANCE_MAX,
> > > +	NFSD_A_SERVER_INSTANCE_MAX = (__NFSD_A_SERVER_INSTANCE_MAX - 1)
> > > +};
> > > +
> > > +enum {
> > > +	NFSD_A_SERVER_LISTENER_INSTANCE = 1,
> > > +
> > > +	__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_SET,
> > > +	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 ad498543f464..d52f392c7f59 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_SET] = "listener-set",
> > > +	[NFSD_CMD_LISTENER_GET] = "listener-get",
> > >  };
> > >  
> > > 
> > >  const char *nfsd_op_str(int op)
> > > @@ -39,6 +41,17 @@ struct ynl_policy_nest nfsd_nfs_version_nest = {
> > >  	.table = nfsd_nfs_version_policy,
> > >  };
> > >  
> > > 
> > > +struct ynl_policy_attr nfsd_server_instance_policy[NFSD_A_SERVER_INSTANCE_MAX + 1] = {
> > > +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
> > > +	[NFSD_A_SERVER_INSTANCE_PORT] = { .name = "port", .type = YNL_PT_U32, },
> > > +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .name = "inet-proto", .type = YNL_PT_U16, },
> > > +};
> > > +
> > > +struct ynl_policy_nest nfsd_server_instance_nest = {
> > > +	.max_attr = NFSD_A_SERVER_INSTANCE_MAX,
> > > +	.table = nfsd_server_instance_policy,
> > > +};
> > > +
> > >  struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
> > >  	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
> > >  	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
> > > @@ -79,6 +92,15 @@ struct ynl_policy_nest nfsd_server_proto_nest = {
> > >  	.table = nfsd_server_proto_policy,
> > >  };
> > >  
> > > 
> > > +struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
> > > +	[NFSD_A_SERVER_LISTENER_INSTANCE] = { .name = "instance", .type = YNL_PT_NEST, .nest = &nfsd_server_instance_nest, },
> > > +};
> > > +
> > > +struct ynl_policy_nest nfsd_server_listener_nest = {
> > > +	.max_attr = NFSD_A_SERVER_LISTENER_MAX,
> > > +	.table = nfsd_server_listener_policy,
> > > +};
> > > +
> > >  /* Common nested types */
> > >  void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
> > >  {
> > > @@ -124,6 +146,64 @@ int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
> > >  	return 0;
> > >  }
> > >  
> > > 
> > > +void nfsd_server_instance_free(struct nfsd_server_instance *obj)
> > > +{
> > > +	free(obj->transport_name);
> > > +}
> > > +
> > > +int nfsd_server_instance_put(struct nlmsghdr *nlh, unsigned int attr_type,
> > > +			     struct nfsd_server_instance *obj)
> > > +{
> > > +	struct nlattr *nest;
> > > +
> > > +	nest = mnl_attr_nest_start(nlh, attr_type);
> > > +	if (obj->_present.transport_name_len)
> > > +		mnl_attr_put_strz(nlh, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME, obj->transport_name);
> > > +	if (obj->_present.port)
> > > +		mnl_attr_put_u32(nlh, NFSD_A_SERVER_INSTANCE_PORT, obj->port);
> > > +	if (obj->_present.inet_proto)
> > > +		mnl_attr_put_u16(nlh, NFSD_A_SERVER_INSTANCE_INET_PROTO, obj->inet_proto);
> > > +	mnl_attr_nest_end(nlh, nest);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int nfsd_server_instance_parse(struct ynl_parse_arg *yarg,
> > > +			       const struct nlattr *nested)
> > > +{
> > > +	struct nfsd_server_instance *dst = yarg->data;
> > > +	const struct nlattr *attr;
> > > +
> > > +	mnl_attr_for_each_nested(attr, nested) {
> > > +		unsigned int type = mnl_attr_get_type(attr);
> > > +
> > > +		if (type == NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME) {
> > > +			unsigned int len;
> > > +
> > > +			if (ynl_attr_validate(yarg, attr))
> > > +				return MNL_CB_ERROR;
> > > +
> > > +			len = strnlen(mnl_attr_get_str(attr), mnl_attr_get_payload_len(attr));
> > > +			dst->_present.transport_name_len = len;
> > > +			dst->transport_name = malloc(len + 1);
> > > +			memcpy(dst->transport_name, mnl_attr_get_str(attr), len);
> > > +			dst->transport_name[len] = 0;
> > > +		} else if (type == NFSD_A_SERVER_INSTANCE_PORT) {
> > > +			if (ynl_attr_validate(yarg, attr))
> > > +				return MNL_CB_ERROR;
> > > +			dst->_present.port = 1;
> > > +			dst->port = mnl_attr_get_u32(attr);
> > > +		} else if (type == NFSD_A_SERVER_INSTANCE_INET_PROTO) {
> > > +			if (ynl_attr_validate(yarg, attr))
> > > +				return MNL_CB_ERROR;
> > > +			dst->_present.inet_proto = 1;
> > > +			dst->inet_proto = mnl_attr_get_u16(attr);
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> > >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> > >  int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
> > > @@ -467,6 +547,117 @@ struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
> > >  	return NULL;
> > >  }
> > >  
> > > 
> > > +/* ============== NFSD_CMD_LISTENER_SET ============== */
> > > +/* NFSD_CMD_LISTENER_SET - do */
> > > +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req)
> > > +{
> > > +	unsigned int i;
> > > +
> > > +	for (i = 0; i < req->n_instance; i++)
> > > +		nfsd_server_instance_free(&req->instance[i]);
> > > +	free(req->instance);
> > > +	free(req);
> > > +}
> > > +
> > > +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req)
> > > +{
> > > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > > +	struct nlmsghdr *nlh;
> > > +	int err;
> > > +
> > > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_SET, 1);
> > > +	ys->req_policy = &nfsd_server_listener_nest;
> > > +
> > > +	for (unsigned int i = 0; i < req->n_instance; i++)
> > > +		nfsd_server_instance_put(nlh, NFSD_A_SERVER_LISTENER_INSTANCE, &req->instance[i]);
> > > +
> > > +	err = ynl_exec(ys, nlh, &yrs);
> > > +	if (err < 0)
> > > +		return -1;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > > +/* NFSD_CMD_LISTENER_GET - do */
> > > +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp)
> > > +{
> > > +	unsigned int i;
> > > +
> > > +	for (i = 0; i < rsp->n_instance; i++)
> > > +		nfsd_server_instance_free(&rsp->instance[i]);
> > > +	free(rsp->instance);
> > > +	free(rsp);
> > > +}
> > > +
> > > +int nfsd_listener_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> > > +{
> > > +	struct nfsd_listener_get_rsp *dst;
> > > +	struct ynl_parse_arg *yarg = data;
> > > +	unsigned int n_instance = 0;
> > > +	const struct nlattr *attr;
> > > +	struct ynl_parse_arg parg;
> > > +	int i;
> > > +
> > > +	dst = yarg->data;
> > > +	parg.ys = yarg->ys;
> > > +
> > > +	if (dst->instance)
> > > +		return ynl_error_parse(yarg, "attribute already present (server-listener.instance)");
> > > +
> > > +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > > +		unsigned int type = mnl_attr_get_type(attr);
> > > +
> > > +		if (type == NFSD_A_SERVER_LISTENER_INSTANCE) {
> > > +			n_instance++;
> > > +		}
> > > +	}
> > > +
> > > +	if (n_instance) {
> > > +		dst->instance = calloc(n_instance, sizeof(*dst->instance));
> > > +		dst->n_instance = n_instance;
> > > +		i = 0;
> > > +		parg.rsp_policy = &nfsd_server_instance_nest;
> > > +		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > > +			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_LISTENER_INSTANCE) {
> > > +				parg.data = &dst->instance[i];
> > > +				if (nfsd_server_instance_parse(&parg, attr))
> > > +					return MNL_CB_ERROR;
> > > +				i++;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > > +	return MNL_CB_OK;
> > > +}
> > > +
> > > +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys)
> > > +{
> > > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > > +	struct nfsd_listener_get_rsp *rsp;
> > > +	struct nlmsghdr *nlh;
> > > +	int err;
> > > +
> > > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
> > > +	ys->req_policy = &nfsd_server_listener_nest;
> > > +	yrs.yarg.rsp_policy = &nfsd_server_listener_nest;
> > > +
> > > +	rsp = calloc(1, sizeof(*rsp));
> > > +	yrs.yarg.data = rsp;
> > > +	yrs.cb = nfsd_listener_get_rsp_parse;
> > > +	yrs.rsp_cmd = NFSD_CMD_LISTENER_GET;
> > > +
> > > +	err = ynl_exec(ys, nlh, &yrs);
> > > +	if (err < 0)
> > > +		goto err_free;
> > > +
> > > +	return rsp;
> > > +
> > > +err_free:
> > > +	nfsd_listener_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 d062ee8fa8b6..5765fb6f2ef5 100644
> > > --- a/tools/net/ynl/generated/nfsd-user.h
> > > +++ b/tools/net/ynl/generated/nfsd-user.h
> > > @@ -29,6 +29,18 @@ struct nfsd_nfs_version {
> > >  	__u32 minor;
> > >  };
> > >  
> > > 
> > > +struct nfsd_server_instance {
> > > +	struct {
> > > +		__u32 transport_name_len;
> > > +		__u32 port:1;
> > > +		__u32 inet_proto:1;
> > > +	} _present;
> > > +
> > > +	char *transport_name;
> > > +	__u32 port;
> > > +	__u16 inet_proto;
> > > +};
> > > +
> > >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> > >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> > >  struct nfsd_rpc_status_get_rsp_dump {
> > > @@ -164,4 +176,47 @@ void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
> > >   */
> > >  struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
> > >  
> > > 
> > > +/* ============== NFSD_CMD_LISTENER_SET ============== */
> > > +/* NFSD_CMD_LISTENER_SET - do */
> > > +struct nfsd_listener_set_req {
> > > +	unsigned int n_instance;
> > > +	struct nfsd_server_instance *instance;
> > > +};
> > > +
> > > +static inline struct nfsd_listener_set_req *nfsd_listener_set_req_alloc(void)
> > > +{
> > > +	return calloc(1, sizeof(struct nfsd_listener_set_req));
> > > +}
> > > +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req);
> > > +
> > > +static inline void
> > > +__nfsd_listener_set_req_set_instance(struct nfsd_listener_set_req *req,
> > > +				     struct nfsd_server_instance *instance,
> > > +				     unsigned int n_instance)
> > > +{
> > > +	free(req->instance);
> > > +	req->instance = instance;
> > > +	req->n_instance = n_instance;
> > > +}
> > > +
> > > +/*
> > > + * set nfs running listeners
> > > + */
> > > +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req);
> > > +
> > > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > > +/* NFSD_CMD_LISTENER_GET - do */
> > > +
> > > +struct nfsd_listener_get_rsp {
> > > +	unsigned int n_instance;
> > > +	struct nfsd_server_instance *instance;
> > > +};
> > > +
> > > +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp);
> > > +
> > > +/*
> > > + * get nfs running listeners
> > > + */
> > > +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys);
> > > +
> > >  #endif /* _LINUX_NFSD_GEN_H */
> > 
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> > 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 15:50       ` Jeff Layton
@ 2024-01-22 15:59         ` Lorenzo Bianconi
  0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-22 15:59 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-nfs, lorenzo.bianconi, neilb, kuba, chuck.lever, horms, netdev

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

[...]
> > > 
> > > I'm not sure this is safe. Can anything else modify sv_permsocks while
> > > you're not holding the lock? Maybe not since you're holding the
> > > nfsd_mutex, but it's still probably best to restart the list walk if you
> > > have to drop the lock here.
> > > 
> > > You're typically only going to have a few sockets here anyway -- usually
> > > just one each for TCP, UDP and maybe RDMA.
> > 
> > what about beeing a bit proactive and set XPT_CLOSE bit before releasing the
> > spinlock (as we already do in svc_xprt_close)?
> > 
> 
> That does sound better, actually. You might have to open-code parts of
> svc_xprt_close, but it's not that big anyway.

or even just set XPT_CLOSE before releasing the spinlock since svc_xprt_close()
will not be affected anyway and we are not in the hotpath.

Regards,
Lorenzo

> 
> 
> > > 
> > > 
> > > > +			spin_lock_bh(&serv->sv_lock);
> > > > +		}
> > > > +	}
> > > > +	spin_unlock_bh(&serv->sv_lock);
> > > > +
> > > > +	if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
> > > > +		nfsd_destroy_serv(net);
> > > > +
> > > > +	mutex_unlock(&nfsd_mutex);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +/**
> > > > + * nfsd_nl_listener_get_doit - get the nfs running listeners
> > > > + * @skb: reply buffer
> > > > + * @info: netlink metadata and command arguments
> > > > + *
> > > > + * Return 0 on success or a negative errno.
> > > > + */
> > > > +int nfsd_nl_listener_get_doit(struct sk_buff *skb, struct genl_info *info)
> > > > +{
> > > > +	struct svc_xprt *xprt;
> > > > +	struct svc_serv *serv;
> > > > +	struct nfsd_net *nn;
> > > > +	void *hdr;
> > > > +	int err;
> > > > +
> > > > +	skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > > > +	if (!skb)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	hdr = genlmsg_iput(skb, info);
> > > > +	if (!hdr) {
> > > > +		err = -EMSGSIZE;
> > > > +		goto err_free_msg;
> > > > +	}
> > > > +
> > > > +	mutex_lock(&nfsd_mutex);
> > > > +	nn = net_generic(genl_info_net(info), nfsd_net_id);
> > > > +	if (!nn->nfsd_serv) {
> > > > +		err = -EINVAL;
> > > > +		goto err_nfsd_unlock;
> > > > +	}
> > > > +
> > > > +	serv = nn->nfsd_serv;
> > > > +	spin_lock_bh(&serv->sv_lock);
> > > > +	list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> > > > +		struct nlattr *attr;
> > > > +
> > > > +		attr = nla_nest_start_noflag(skb,
> > > > +					     NFSD_A_SERVER_LISTENER_INSTANCE);
> > > > +		if (!attr) {
> > > > +			err = -EINVAL;
> > > > +			goto err_serv_unlock;
> > > > +		}
> > > > +
> > > > +		if (nla_put_string(skb, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME,
> > > > +				   xprt->xpt_class->xcl_name) ||
> > > > +		    nla_put_u32(skb, NFSD_A_SERVER_INSTANCE_PORT,
> > > > +				svc_xprt_local_port(xprt)) ||
> > > > +		    nla_put_u16(skb, NFSD_A_SERVER_INSTANCE_INET_PROTO,
> > > > +				xprt->xpt_local.ss_family)) {
> > > > +			err = -EINVAL;
> > > > +			goto err_serv_unlock;
> > > > +		}
> > > > +
> > > > +		nla_nest_end(skb, attr);
> > > > +	}
> > > > +	spin_unlock_bh(&serv->sv_lock);
> > > > +	mutex_unlock(&nfsd_mutex);
> > > > +
> > > > +	genlmsg_end(skb, hdr);
> > > > +
> > > > +	return genlmsg_reply(skb, info);
> > > > +
> > > > +err_serv_unlock:
> > > > +	spin_unlock_bh(&serv->sv_lock);
> > > > +err_nfsd_unlock:
> > > > +	mutex_unlock(&nfsd_mutex);
> > > > +err_free_msg:
> > > > +	nlmsg_free(skb);
> > > > +
> > > > +	return err;
> > > > +}
> > > > +
> > > >  /**
> > > >   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
> > > >   * @net: a freshly-created network namespace
> > > > diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> > > > index 2a06f9fe6fe9..659ab76b8840 100644
> > > > --- a/include/uapi/linux/nfsd_netlink.h
> > > > +++ b/include/uapi/linux/nfsd_netlink.h
> > > > @@ -51,12 +51,30 @@ enum {
> > > >  	NFSD_A_SERVER_PROTO_MAX = (__NFSD_A_SERVER_PROTO_MAX - 1)
> > > >  };
> > > >  
> > > > 
> > > > +enum {
> > > > +	NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME = 1,
> > > > +	NFSD_A_SERVER_INSTANCE_PORT,
> > > > +	NFSD_A_SERVER_INSTANCE_INET_PROTO,
> > > > +
> > > > +	__NFSD_A_SERVER_INSTANCE_MAX,
> > > > +	NFSD_A_SERVER_INSTANCE_MAX = (__NFSD_A_SERVER_INSTANCE_MAX - 1)
> > > > +};
> > > > +
> > > > +enum {
> > > > +	NFSD_A_SERVER_LISTENER_INSTANCE = 1,
> > > > +
> > > > +	__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_SET,
> > > > +	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 ad498543f464..d52f392c7f59 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_SET] = "listener-set",
> > > > +	[NFSD_CMD_LISTENER_GET] = "listener-get",
> > > >  };
> > > >  
> > > > 
> > > >  const char *nfsd_op_str(int op)
> > > > @@ -39,6 +41,17 @@ struct ynl_policy_nest nfsd_nfs_version_nest = {
> > > >  	.table = nfsd_nfs_version_policy,
> > > >  };
> > > >  
> > > > 
> > > > +struct ynl_policy_attr nfsd_server_instance_policy[NFSD_A_SERVER_INSTANCE_MAX + 1] = {
> > > > +	[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
> > > > +	[NFSD_A_SERVER_INSTANCE_PORT] = { .name = "port", .type = YNL_PT_U32, },
> > > > +	[NFSD_A_SERVER_INSTANCE_INET_PROTO] = { .name = "inet-proto", .type = YNL_PT_U16, },
> > > > +};
> > > > +
> > > > +struct ynl_policy_nest nfsd_server_instance_nest = {
> > > > +	.max_attr = NFSD_A_SERVER_INSTANCE_MAX,
> > > > +	.table = nfsd_server_instance_policy,
> > > > +};
> > > > +
> > > >  struct ynl_policy_attr nfsd_rpc_status_policy[NFSD_A_RPC_STATUS_MAX + 1] = {
> > > >  	[NFSD_A_RPC_STATUS_XID] = { .name = "xid", .type = YNL_PT_U32, },
> > > >  	[NFSD_A_RPC_STATUS_FLAGS] = { .name = "flags", .type = YNL_PT_U32, },
> > > > @@ -79,6 +92,15 @@ struct ynl_policy_nest nfsd_server_proto_nest = {
> > > >  	.table = nfsd_server_proto_policy,
> > > >  };
> > > >  
> > > > 
> > > > +struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
> > > > +	[NFSD_A_SERVER_LISTENER_INSTANCE] = { .name = "instance", .type = YNL_PT_NEST, .nest = &nfsd_server_instance_nest, },
> > > > +};
> > > > +
> > > > +struct ynl_policy_nest nfsd_server_listener_nest = {
> > > > +	.max_attr = NFSD_A_SERVER_LISTENER_MAX,
> > > > +	.table = nfsd_server_listener_policy,
> > > > +};
> > > > +
> > > >  /* Common nested types */
> > > >  void nfsd_nfs_version_free(struct nfsd_nfs_version *obj)
> > > >  {
> > > > @@ -124,6 +146,64 @@ int nfsd_nfs_version_parse(struct ynl_parse_arg *yarg,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > 
> > > > +void nfsd_server_instance_free(struct nfsd_server_instance *obj)
> > > > +{
> > > > +	free(obj->transport_name);
> > > > +}
> > > > +
> > > > +int nfsd_server_instance_put(struct nlmsghdr *nlh, unsigned int attr_type,
> > > > +			     struct nfsd_server_instance *obj)
> > > > +{
> > > > +	struct nlattr *nest;
> > > > +
> > > > +	nest = mnl_attr_nest_start(nlh, attr_type);
> > > > +	if (obj->_present.transport_name_len)
> > > > +		mnl_attr_put_strz(nlh, NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME, obj->transport_name);
> > > > +	if (obj->_present.port)
> > > > +		mnl_attr_put_u32(nlh, NFSD_A_SERVER_INSTANCE_PORT, obj->port);
> > > > +	if (obj->_present.inet_proto)
> > > > +		mnl_attr_put_u16(nlh, NFSD_A_SERVER_INSTANCE_INET_PROTO, obj->inet_proto);
> > > > +	mnl_attr_nest_end(nlh, nest);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +int nfsd_server_instance_parse(struct ynl_parse_arg *yarg,
> > > > +			       const struct nlattr *nested)
> > > > +{
> > > > +	struct nfsd_server_instance *dst = yarg->data;
> > > > +	const struct nlattr *attr;
> > > > +
> > > > +	mnl_attr_for_each_nested(attr, nested) {
> > > > +		unsigned int type = mnl_attr_get_type(attr);
> > > > +
> > > > +		if (type == NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME) {
> > > > +			unsigned int len;
> > > > +
> > > > +			if (ynl_attr_validate(yarg, attr))
> > > > +				return MNL_CB_ERROR;
> > > > +
> > > > +			len = strnlen(mnl_attr_get_str(attr), mnl_attr_get_payload_len(attr));
> > > > +			dst->_present.transport_name_len = len;
> > > > +			dst->transport_name = malloc(len + 1);
> > > > +			memcpy(dst->transport_name, mnl_attr_get_str(attr), len);
> > > > +			dst->transport_name[len] = 0;
> > > > +		} else if (type == NFSD_A_SERVER_INSTANCE_PORT) {
> > > > +			if (ynl_attr_validate(yarg, attr))
> > > > +				return MNL_CB_ERROR;
> > > > +			dst->_present.port = 1;
> > > > +			dst->port = mnl_attr_get_u32(attr);
> > > > +		} else if (type == NFSD_A_SERVER_INSTANCE_INET_PROTO) {
> > > > +			if (ynl_attr_validate(yarg, attr))
> > > > +				return MNL_CB_ERROR;
> > > > +			dst->_present.inet_proto = 1;
> > > > +			dst->inet_proto = mnl_attr_get_u16(attr);
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> > > >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> > > >  int nfsd_rpc_status_get_rsp_dump_parse(const struct nlmsghdr *nlh, void *data)
> > > > @@ -467,6 +547,117 @@ struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys)
> > > >  	return NULL;
> > > >  }
> > > >  
> > > > 
> > > > +/* ============== NFSD_CMD_LISTENER_SET ============== */
> > > > +/* NFSD_CMD_LISTENER_SET - do */
> > > > +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req)
> > > > +{
> > > > +	unsigned int i;
> > > > +
> > > > +	for (i = 0; i < req->n_instance; i++)
> > > > +		nfsd_server_instance_free(&req->instance[i]);
> > > > +	free(req->instance);
> > > > +	free(req);
> > > > +}
> > > > +
> > > > +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req)
> > > > +{
> > > > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > > > +	struct nlmsghdr *nlh;
> > > > +	int err;
> > > > +
> > > > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_SET, 1);
> > > > +	ys->req_policy = &nfsd_server_listener_nest;
> > > > +
> > > > +	for (unsigned int i = 0; i < req->n_instance; i++)
> > > > +		nfsd_server_instance_put(nlh, NFSD_A_SERVER_LISTENER_INSTANCE, &req->instance[i]);
> > > > +
> > > > +	err = ynl_exec(ys, nlh, &yrs);
> > > > +	if (err < 0)
> > > > +		return -1;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > > > +/* NFSD_CMD_LISTENER_GET - do */
> > > > +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp)
> > > > +{
> > > > +	unsigned int i;
> > > > +
> > > > +	for (i = 0; i < rsp->n_instance; i++)
> > > > +		nfsd_server_instance_free(&rsp->instance[i]);
> > > > +	free(rsp->instance);
> > > > +	free(rsp);
> > > > +}
> > > > +
> > > > +int nfsd_listener_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
> > > > +{
> > > > +	struct nfsd_listener_get_rsp *dst;
> > > > +	struct ynl_parse_arg *yarg = data;
> > > > +	unsigned int n_instance = 0;
> > > > +	const struct nlattr *attr;
> > > > +	struct ynl_parse_arg parg;
> > > > +	int i;
> > > > +
> > > > +	dst = yarg->data;
> > > > +	parg.ys = yarg->ys;
> > > > +
> > > > +	if (dst->instance)
> > > > +		return ynl_error_parse(yarg, "attribute already present (server-listener.instance)");
> > > > +
> > > > +	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > > > +		unsigned int type = mnl_attr_get_type(attr);
> > > > +
> > > > +		if (type == NFSD_A_SERVER_LISTENER_INSTANCE) {
> > > > +			n_instance++;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (n_instance) {
> > > > +		dst->instance = calloc(n_instance, sizeof(*dst->instance));
> > > > +		dst->n_instance = n_instance;
> > > > +		i = 0;
> > > > +		parg.rsp_policy = &nfsd_server_instance_nest;
> > > > +		mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
> > > > +			if (mnl_attr_get_type(attr) == NFSD_A_SERVER_LISTENER_INSTANCE) {
> > > > +				parg.data = &dst->instance[i];
> > > > +				if (nfsd_server_instance_parse(&parg, attr))
> > > > +					return MNL_CB_ERROR;
> > > > +				i++;
> > > > +			}
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return MNL_CB_OK;
> > > > +}
> > > > +
> > > > +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys)
> > > > +{
> > > > +	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
> > > > +	struct nfsd_listener_get_rsp *rsp;
> > > > +	struct nlmsghdr *nlh;
> > > > +	int err;
> > > > +
> > > > +	nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
> > > > +	ys->req_policy = &nfsd_server_listener_nest;
> > > > +	yrs.yarg.rsp_policy = &nfsd_server_listener_nest;
> > > > +
> > > > +	rsp = calloc(1, sizeof(*rsp));
> > > > +	yrs.yarg.data = rsp;
> > > > +	yrs.cb = nfsd_listener_get_rsp_parse;
> > > > +	yrs.rsp_cmd = NFSD_CMD_LISTENER_GET;
> > > > +
> > > > +	err = ynl_exec(ys, nlh, &yrs);
> > > > +	if (err < 0)
> > > > +		goto err_free;
> > > > +
> > > > +	return rsp;
> > > > +
> > > > +err_free:
> > > > +	nfsd_listener_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 d062ee8fa8b6..5765fb6f2ef5 100644
> > > > --- a/tools/net/ynl/generated/nfsd-user.h
> > > > +++ b/tools/net/ynl/generated/nfsd-user.h
> > > > @@ -29,6 +29,18 @@ struct nfsd_nfs_version {
> > > >  	__u32 minor;
> > > >  };
> > > >  
> > > > 
> > > > +struct nfsd_server_instance {
> > > > +	struct {
> > > > +		__u32 transport_name_len;
> > > > +		__u32 port:1;
> > > > +		__u32 inet_proto:1;
> > > > +	} _present;
> > > > +
> > > > +	char *transport_name;
> > > > +	__u32 port;
> > > > +	__u16 inet_proto;
> > > > +};
> > > > +
> > > >  /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> > > >  /* NFSD_CMD_RPC_STATUS_GET - dump */
> > > >  struct nfsd_rpc_status_get_rsp_dump {
> > > > @@ -164,4 +176,47 @@ void nfsd_version_get_rsp_free(struct nfsd_version_get_rsp *rsp);
> > > >   */
> > > >  struct nfsd_version_get_rsp *nfsd_version_get(struct ynl_sock *ys);
> > > >  
> > > > 
> > > > +/* ============== NFSD_CMD_LISTENER_SET ============== */
> > > > +/* NFSD_CMD_LISTENER_SET - do */
> > > > +struct nfsd_listener_set_req {
> > > > +	unsigned int n_instance;
> > > > +	struct nfsd_server_instance *instance;
> > > > +};
> > > > +
> > > > +static inline struct nfsd_listener_set_req *nfsd_listener_set_req_alloc(void)
> > > > +{
> > > > +	return calloc(1, sizeof(struct nfsd_listener_set_req));
> > > > +}
> > > > +void nfsd_listener_set_req_free(struct nfsd_listener_set_req *req);
> > > > +
> > > > +static inline void
> > > > +__nfsd_listener_set_req_set_instance(struct nfsd_listener_set_req *req,
> > > > +				     struct nfsd_server_instance *instance,
> > > > +				     unsigned int n_instance)
> > > > +{
> > > > +	free(req->instance);
> > > > +	req->instance = instance;
> > > > +	req->n_instance = n_instance;
> > > > +}
> > > > +
> > > > +/*
> > > > + * set nfs running listeners
> > > > + */
> > > > +int nfsd_listener_set(struct ynl_sock *ys, struct nfsd_listener_set_req *req);
> > > > +
> > > > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > > > +/* NFSD_CMD_LISTENER_GET - do */
> > > > +
> > > > +struct nfsd_listener_get_rsp {
> > > > +	unsigned int n_instance;
> > > > +	struct nfsd_server_instance *instance;
> > > > +};
> > > > +
> > > > +void nfsd_listener_get_rsp_free(struct nfsd_listener_get_rsp *rsp);
> > > > +
> > > > +/*
> > > > + * get nfs running listeners
> > > > + */
> > > > +struct nfsd_listener_get_rsp *nfsd_listener_get(struct ynl_sock *ys);
> > > > +
> > > >  #endif /* _LINUX_NFSD_GEN_H */
> > > 
> > > -- 
> > > Jeff Layton <jlayton@kernel.org>
> > > 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-20 17:33 ` [PATCH v6 3/3] NFSD: add write_ports " Lorenzo Bianconi
  2024-01-22 13:41   ` Jeff Layton
@ 2024-01-22 16:31   ` kernel test robot
  1 sibling, 0 replies; 27+ messages in thread
From: kernel test robot @ 2024-01-22 16:31 UTC (permalink / raw)
  To: Lorenzo Bianconi, linux-nfs
  Cc: oe-kbuild-all, lorenzo.bianconi, neilb, jlayton, kuba,
	chuck.lever, horms, netdev

Hi Lorenzo,

kernel test robot noticed the following build errors:

[auto build test ERROR on v6.7]
[cannot apply to linus/master trondmy-nfs/linux-next next-20240122]
[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/20240121-013808
base:   v6.7
patch link:    https://lore.kernel.org/r/f7c42dae2b232b3b06e54ceb3f00725893973e02.1705771400.git.lorenzo%40kernel.org
patch subject: [PATCH v6 3/3] NFSD: add write_ports to netlink command
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20240123/202401230032.Sx6BKQgl-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240123/202401230032.Sx6BKQgl-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/202401230032.Sx6BKQgl-lkp@intel.com/

All errors (new ones prefixed by >>):

   fs/nfsd/nfsctl.c: In function 'nfsd_nl_listener_set_doit':
>> fs/nfsd/nfsctl.c:2017:17: error: implicit declaration of function 'nfsd_destroy_serv'; did you mean 'nfsd4_destroy_session'? [-Werror=implicit-function-declaration]
    2017 |                 nfsd_destroy_serv(net);
         |                 ^~~~~~~~~~~~~~~~~
         |                 nfsd4_destroy_session
   cc1: some warnings being treated as errors


vim +2017 fs/nfsd/nfsctl.c

  1900	
  1901	/**
  1902	 * nfsd_nl_listener_set_doit - set the nfs running listeners
  1903	 * @skb: reply buffer
  1904	 * @info: netlink metadata and command arguments
  1905	 *
  1906	 * Return 0 on success or a negative errno.
  1907	 */
  1908	int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info)
  1909	{
  1910		struct nlattr *tb[ARRAY_SIZE(nfsd_server_instance_nl_policy)];
  1911		struct net *net = genl_info_net(info);
  1912		struct svc_xprt *xprt, *tmp_xprt;
  1913		const struct nlattr *attr;
  1914		struct svc_serv *serv;
  1915		const char *xcl_name;
  1916		struct nfsd_net *nn;
  1917		int port, err, rem;
  1918		sa_family_t af;
  1919	
  1920		if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_INSTANCE))
  1921			return -EINVAL;
  1922	
  1923		mutex_lock(&nfsd_mutex);
  1924	
  1925		err = nfsd_create_serv(net);
  1926		if (err) {
  1927			mutex_unlock(&nfsd_mutex);
  1928			return err;
  1929		}
  1930	
  1931		nn = net_generic(net, nfsd_net_id);
  1932		serv = nn->nfsd_serv;
  1933	
  1934		/* 1- create brand new listeners */
  1935		nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
  1936			if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
  1937				continue;
  1938	
  1939			if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
  1940					     nfsd_server_instance_nl_policy,
  1941					     info->extack) < 0)
  1942				continue;
  1943	
  1944			if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
  1945			    !tb[NFSD_A_SERVER_INSTANCE_PORT])
  1946				continue;
  1947	
  1948			xcl_name = nla_data(tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
  1949			port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
  1950			if (port < 1 || port > USHRT_MAX)
  1951				continue;
  1952	
  1953			af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
  1954			if (af != PF_INET && af != PF_INET6)
  1955				continue;
  1956	
  1957			xprt = svc_find_xprt(serv, xcl_name, net, PF_INET, port);
  1958			if (xprt) {
  1959				svc_xprt_put(xprt);
  1960				continue;
  1961			}
  1962	
  1963			/* create new listerner */
  1964			if (svc_xprt_create(serv, xcl_name, net, af, port,
  1965					    SVC_SOCK_ANONYMOUS, get_current_cred()))
  1966				continue;
  1967		}
  1968	
  1969		/* 2- remove stale listeners */
  1970		spin_lock_bh(&serv->sv_lock);
  1971		list_for_each_entry_safe(xprt, tmp_xprt, &serv->sv_permsocks,
  1972					 xpt_list) {
  1973			struct svc_xprt *rqt_xprt = NULL;
  1974	
  1975			nlmsg_for_each_attr(attr, info->nlhdr, GENL_HDRLEN, rem) {
  1976				if (nla_type(attr) != NFSD_A_SERVER_LISTENER_INSTANCE)
  1977					continue;
  1978	
  1979				if (nla_parse_nested(tb, ARRAY_SIZE(tb), attr,
  1980						     nfsd_server_instance_nl_policy,
  1981						     info->extack) < 0)
  1982					continue;
  1983	
  1984				if (!tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME] ||
  1985				    !tb[NFSD_A_SERVER_INSTANCE_PORT])
  1986					continue;
  1987	
  1988				xcl_name = nla_data(
  1989					tb[NFSD_A_SERVER_INSTANCE_TRANSPORT_NAME]);
  1990				port = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_PORT]);
  1991				if (port < 1 || port > USHRT_MAX)
  1992					continue;
  1993	
  1994				af = nla_get_u32(tb[NFSD_A_SERVER_INSTANCE_INET_PROTO]);
  1995				if (af != PF_INET && af != PF_INET6)
  1996					continue;
  1997	
  1998				if (!strcmp(xprt->xpt_class->xcl_name, xcl_name) &&
  1999				    port == svc_xprt_local_port(xprt) &&
  2000				    af == xprt->xpt_local.ss_family &&
  2001				    xprt->xpt_net == net) {
  2002					rqt_xprt = xprt;
  2003					break;
  2004				}
  2005			}
  2006	
  2007			/* remove stale listener */
  2008			if (!rqt_xprt) {
  2009				spin_unlock_bh(&serv->sv_lock);
  2010				svc_xprt_close(xprt);
  2011				spin_lock_bh(&serv->sv_lock);
  2012			}
  2013		}
  2014		spin_unlock_bh(&serv->sv_lock);
  2015	
  2016		if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks))
> 2017			nfsd_destroy_serv(net);
  2018	
  2019		mutex_unlock(&nfsd_mutex);
  2020	
  2021		return 0;
  2022	}
  2023	

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands
  2024-01-22 13:49 ` [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Jeff Layton
@ 2024-01-22 17:01   ` Lorenzo Bianconi
  0 siblings, 0 replies; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-22 17:01 UTC (permalink / raw)
  To: Jeff Layton
  Cc: linux-nfs, lorenzo.bianconi, neilb, kuba, chuck.lever, horms,
	netdev, Steve Dickson

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

> On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > Introduce write_threads, write_version and write_ports netlink
> > commands similar to the ones available through the procfs.
> > 
> > Changes since v5:
> > - for write_ports and write_version commands, userspace is expected to provide
> >   a NFS listeners/supported versions list it want to enable (all the other
> >   ports/versions will be disabled).
> > - fix comments
> > - rebase on top of nfsd-next
> > Changes since v4:
> > - rebase on top of nfsd-next tree
> > Changes since v3:
> > - drop write_maxconn and write_maxblksize for the moment
> > - add write_version and write_ports commands
> > Changes since v2:
> > - use u32 to store nthreads in nfsd_nl_threads_set_doit
> > - rename server-attr in control-plane in nfsd.yaml specs
> > Changes since v1:
> > - remove write_v4_end_grace command
> > - add write_maxblksize and write_maxconn netlink commands
> > 
> > This patch can be tested with user-space tool reported below:
> > https://github.com/LorenzoBianconi/nfsd-netlink.git
> > 
> > Lorenzo Bianconi (3):
> >   NFSD: convert write_threads to netlink command
> >   NFSD: add write_version to netlink command
> >   NFSD: add write_ports to netlink command
> > 
> >  Documentation/netlink/specs/nfsd.yaml |  94 ++++++
> >  fs/nfsd/netlink.c                     |  63 ++++
> >  fs/nfsd/netlink.h                     |  10 +
> >  fs/nfsd/nfsctl.c                      | 396 ++++++++++++++++++++++
> >  include/uapi/linux/nfsd_netlink.h     |  44 +++
> >  tools/net/ynl/generated/nfsd-user.c   | 460 ++++++++++++++++++++++++++
> >  tools/net/ynl/generated/nfsd-user.h   | 155 +++++++++
> >  7 files changed, 1222 insertions(+)
> > 
> 
> 
> I think this is really close and coming together! Before we merge this
> though, I'd _really_ like to see some patches for rpc.nfsd in nfs-utils.
> Until we try to implement the userland bits, we won't know if we've
> gotten this interface right.
> 
> ...and before that, we really need to have some sort of userland program
> packaged and available for querying the new netlink RPC stats from nfsd.
> You have the simple userland one on github, but I think we need omething
> packaged, ideally as part of nfs-utils.

Hi Jeff,

I guess we can experiment on the new APIs very easily with ynl cli.py.
Something like:

$ ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/nfsd.yaml --dump rpc-status-get
[{'compound-ops': [53, 22, 9],
 'daddr4': 3232266828,
 'dport': 2049,
 'flags': 5,
 'proc': 1,
 'prog': 100003,
 'saddr4': 3232266753,
 'service_time': 81705129,
 'sport': 908,
 'version': 4,
 'xid': 0},
{'compound-ops': [53, 22, 9],
 'daddr4': 3232266828,
 'dport': 2049,
 'flags': 5,
 'proc': 1,
 'prog': 100003,
 'saddr4': 3232266753,
 'service_time': 81700496,
 'sport': 908,
 'version': 4,
 'xid': 0}]

or 

./tools/net/ynl/cli.py --spec Documentation/netlink/specs/nfsd.yaml --do threads-get
{'threads': 8}

(the only required package is jsonschema iirc).

Regards,
Lorenzo

> 
> Doing that first would allow you to add the necessary autoconf/libtool
> stuff to pull in the netlink libraries, which will be a prerequisite for
> doing the userland rpc.nfsd work, and will probably be a bit simpler
> than modifying rpc.nfsd.
> -- 
> Jeff Layton <jlayton@kernel.org>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 13:41   ` Jeff Layton
  2024-01-22 15:35     ` Lorenzo Bianconi
@ 2024-01-22 21:35     ` NeilBrown
  2024-01-22 21:37       ` Jeff Layton
  2024-01-22 22:58       ` Chuck Lever
  1 sibling, 2 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-22 21:35 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Lorenzo Bianconi, linux-nfs, lorenzo.bianconi, kuba, chuck.lever,
	horms, netdev

On Tue, 23 Jan 2024, Jeff Layton wrote:
> On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > Introduce write_ports netlink command. For listener-set, userspace is
> > expected to provide a NFS listeners list it wants to enable (all the
> > other ports will be closed).
> > 
> 
> Ditto here. This is a change to a declarative interface, which I think
> is a better way to handle this, but we should be aware of the change.

I agree it is better, and thanks for highlighting the change.

> > +	/* 2- remove stale listeners */
> 
> 
> The old portlist interface was weird, in that it was only additive. You
> couldn't use it to close a listening socket (AFAICT). We may be able to
> support that now with this interface, but we'll need to test that case
> carefully.

Do we ever want/need to remove listening sockets?
Normal practice when making any changes is to stop and restart where
"stop" removes all sockets, unexports all filesystems, disables all
versions.
I don't exactly object to supporting fine-grained changes, but I suspect
anything that is not used by normal service start will hardly ever be
used in practice, so will not be tested.

So if it is easiest to support reverting previous configuration (as it
probably is for version setting), then do so.  But if there is any
complexity (as maybe there is with listening sockets), then don't
add complexity that won't be used.

Thanks,
NeilBrown

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 21:35     ` NeilBrown
@ 2024-01-22 21:37       ` Jeff Layton
  2024-01-22 22:58       ` Chuck Lever
  1 sibling, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-22 21:37 UTC (permalink / raw)
  To: NeilBrown
  Cc: Lorenzo Bianconi, linux-nfs, lorenzo.bianconi, kuba, chuck.lever,
	horms, netdev

On Tue, 2024-01-23 at 08:35 +1100, NeilBrown wrote:
> On Tue, 23 Jan 2024, Jeff Layton wrote:
> > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > Introduce write_ports netlink command. For listener-set, userspace is
> > > expected to provide a NFS listeners list it wants to enable (all the
> > > other ports will be closed).
> > > 
> > 
> > Ditto here. This is a change to a declarative interface, which I think
> > is a better way to handle this, but we should be aware of the change.
> 
> I agree it is better, and thanks for highlighting the change.
> 
> > > +	/* 2- remove stale listeners */
> > 
> > 
> > The old portlist interface was weird, in that it was only additive. You
> > couldn't use it to close a listening socket (AFAICT). We may be able to
> > support that now with this interface, but we'll need to test that case
> > carefully.
> 
> Do we ever want/need to remove listening sockets?
> Normal practice when making any changes is to stop and restart where
> "stop" removes all sockets, unexports all filesystems, disables all
> versions.
> I don't exactly object to supporting fine-grained changes, but I suspect
> anything that is not used by normal service start will hardly ever be
> used in practice, so will not be tested.
> 
> So if it is easiest to support reverting previous configuration (as it
> probably is for version setting), then do so.  But if there is any
> complexity (as maybe there is with listening sockets), then don't
> add complexity that won't be used.
> 


I completely agree here. It's probably simplest to just prevent this for
now unless and until there is some need for it.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 21:35     ` NeilBrown
  2024-01-22 21:37       ` Jeff Layton
@ 2024-01-22 22:58       ` Chuck Lever
  2024-01-23  9:59         ` Lorenzo Bianconi
  1 sibling, 1 reply; 27+ messages in thread
From: Chuck Lever @ 2024-01-22 22:58 UTC (permalink / raw)
  To: NeilBrown
  Cc: Jeff Layton, Lorenzo Bianconi, linux-nfs, lorenzo.bianconi, kuba,
	horms, netdev

On Tue, Jan 23, 2024 at 08:35:07AM +1100, NeilBrown wrote:
> On Tue, 23 Jan 2024, Jeff Layton wrote:
> > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > Introduce write_ports netlink command. For listener-set, userspace is
> > > expected to provide a NFS listeners list it wants to enable (all the
> > > other ports will be closed).
> > > 
> > 
> > Ditto here. This is a change to a declarative interface, which I think
> > is a better way to handle this, but we should be aware of the change.
> 
> I agree it is better, and thanks for highlighting the change.
> 
> > > +	/* 2- remove stale listeners */
> > 
> > 
> > The old portlist interface was weird, in that it was only additive. You
> > couldn't use it to close a listening socket (AFAICT). We may be able to
> > support that now with this interface, but we'll need to test that case
> > carefully.
> 
> Do we ever want/need to remove listening sockets?

I think that might be an interesting use case. Disabling RDMA, for
example, should kill the RDMA listening endpoints but leave
listening sockets in place.

But for now, our socket listeners are "any". Wondering how net
namespaces play into this.


> Normal practice when making any changes is to stop and restart where
> "stop" removes all sockets, unexports all filesystems, disables all
> versions.
> I don't exactly object to supporting fine-grained changes, but I suspect
> anything that is not used by normal service start will hardly ever be
> used in practice, so will not be tested.

Well, there is that. I guess until we have test coverage for NFSD
administrative interfaces, we should leave well enough alone.


> So if it is easiest to support reverting previous configuration (as it
> probably is for version setting), then do so.  But if there is any
> complexity (as maybe there is with listening sockets), then don't
> add complexity that won't be used.
> 
> Thanks,
> NeilBrown

-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-22 22:58       ` Chuck Lever
@ 2024-01-23  9:59         ` Lorenzo Bianconi
  2024-01-23 11:17           ` Jeff Layton
  0 siblings, 1 reply; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-23  9:59 UTC (permalink / raw)
  To: Chuck Lever
  Cc: NeilBrown, Jeff Layton, linux-nfs, lorenzo.bianconi, kuba, horms, netdev

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

> On Tue, Jan 23, 2024 at 08:35:07AM +1100, NeilBrown wrote:
> > On Tue, 23 Jan 2024, Jeff Layton wrote:
> > > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > > Introduce write_ports netlink command. For listener-set, userspace is
> > > > expected to provide a NFS listeners list it wants to enable (all the
> > > > other ports will be closed).
> > > > 
> > > 
> > > Ditto here. This is a change to a declarative interface, which I think
> > > is a better way to handle this, but we should be aware of the change.
> > 
> > I agree it is better, and thanks for highlighting the change.
> > 
> > > > +	/* 2- remove stale listeners */
> > > 
> > > 
> > > The old portlist interface was weird, in that it was only additive. You
> > > couldn't use it to close a listening socket (AFAICT). We may be able to
> > > support that now with this interface, but we'll need to test that case
> > > carefully.
> > 
> > Do we ever want/need to remove listening sockets?
> 
> I think that might be an interesting use case. Disabling RDMA, for
> example, should kill the RDMA listening endpoints but leave
> listening sockets in place.
> 
> But for now, our socket listeners are "any". Wondering how net
> namespaces play into this.
> 
> 
> > Normal practice when making any changes is to stop and restart where
> > "stop" removes all sockets, unexports all filesystems, disables all
> > versions.
> > I don't exactly object to supporting fine-grained changes, but I suspect
> > anything that is not used by normal service start will hardly ever be
> > used in practice, so will not be tested.
> 
> Well, there is that. I guess until we have test coverage for NFSD
> administrative interfaces, we should leave well enough alone.

So to summarize it:
- we will allow to remove enabled versions (as it is in patch v6 2/3)
- we will allow to add new listening sockets but we will not allow to remove
  them (the user/admin will need to stop/start the server).

Agree? If so I will work on it and post v7.

Regards,
Lorenzo

> 
> 
> > So if it is easiest to support reverting previous configuration (as it
> > probably is for version setting), then do so.  But if there is any
> > complexity (as maybe there is with listening sockets), then don't
> > add complexity that won't be used.
> > 
> > Thanks,
> > NeilBrown
> 
> -- 
> Chuck Lever

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-23  9:59         ` Lorenzo Bianconi
@ 2024-01-23 11:17           ` Jeff Layton
  2024-01-23 13:21             ` Lorenzo Bianconi
  0 siblings, 1 reply; 27+ messages in thread
From: Jeff Layton @ 2024-01-23 11:17 UTC (permalink / raw)
  To: Lorenzo Bianconi, Chuck Lever
  Cc: NeilBrown, linux-nfs, lorenzo.bianconi, kuba, horms, netdev

On Tue, 2024-01-23 at 10:59 +0100, Lorenzo Bianconi wrote:
> > On Tue, Jan 23, 2024 at 08:35:07AM +1100, NeilBrown wrote:
> > > On Tue, 23 Jan 2024, Jeff Layton wrote:
> > > > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > > > Introduce write_ports netlink command. For listener-set, userspace is
> > > > > expected to provide a NFS listeners list it wants to enable (all the
> > > > > other ports will be closed).
> > > > > 
> > > > 
> > > > Ditto here. This is a change to a declarative interface, which I think
> > > > is a better way to handle this, but we should be aware of the change.
> > > 
> > > I agree it is better, and thanks for highlighting the change.
> > > 
> > > > > +	/* 2- remove stale listeners */
> > > > 
> > > > 
> > > > The old portlist interface was weird, in that it was only additive. You
> > > > couldn't use it to close a listening socket (AFAICT). We may be able to
> > > > support that now with this interface, but we'll need to test that case
> > > > carefully.
> > > 
> > > Do we ever want/need to remove listening sockets?
> > 
> > I think that might be an interesting use case. Disabling RDMA, for
> > example, should kill the RDMA listening endpoints but leave
> > listening sockets in place.
> > 
> > But for now, our socket listeners are "any". Wondering how net
> > namespaces play into this.
> > 
> > 
> > > Normal practice when making any changes is to stop and restart where
> > > "stop" removes all sockets, unexports all filesystems, disables all
> > > versions.
> > > I don't exactly object to supporting fine-grained changes, but I suspect
> > > anything that is not used by normal service start will hardly ever be
> > > used in practice, so will not be tested.
> > 
> > Well, there is that. I guess until we have test coverage for NFSD
> > administrative interfaces, we should leave well enough alone.
> 
> So to summarize it:
> - we will allow to remove enabled versions (as it is in patch v6 2/3)
> - we will allow to add new listening sockets but we will not allow to remove
>   them (the user/admin will need to stop/start the server).
> 
> Agree? If so I will work on it and post v7.
> 
> 

That sounds about right to me. We could eventually relax the restriction
about removing sockets later, but for now it's probably best to prohibit
it (like Neil suggests).


> 
> > 
> > 
> > > So if it is easiest to support reverting previous configuration (as it
> > > probably is for version setting), then do so.  But if there is any
> > > complexity (as maybe there is with listening sockets), then don't
> > > add complexity that won't be used.
> > > 
> > > Thanks,
> > > NeilBrown
> > 
> > -- 
> > Chuck Lever

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-23 11:17           ` Jeff Layton
@ 2024-01-23 13:21             ` Lorenzo Bianconi
  2024-01-23 14:28               ` Jeff Layton
  0 siblings, 1 reply; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-23 13:21 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Chuck Lever, NeilBrown, linux-nfs, lorenzo.bianconi, kuba, horms, netdev

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

> On Tue, 2024-01-23 at 10:59 +0100, Lorenzo Bianconi wrote:
> > > On Tue, Jan 23, 2024 at 08:35:07AM +1100, NeilBrown wrote:
> > > > On Tue, 23 Jan 2024, Jeff Layton wrote:
> > > > > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > > > > Introduce write_ports netlink command. For listener-set, userspace is
> > > > > > expected to provide a NFS listeners list it wants to enable (all the
> > > > > > other ports will be closed).
> > > > > > 
> > > > > 
> > > > > Ditto here. This is a change to a declarative interface, which I think
> > > > > is a better way to handle this, but we should be aware of the change.
> > > > 
> > > > I agree it is better, and thanks for highlighting the change.
> > > > 
> > > > > > +	/* 2- remove stale listeners */
> > > > > 
> > > > > 
> > > > > The old portlist interface was weird, in that it was only additive. You
> > > > > couldn't use it to close a listening socket (AFAICT). We may be able to
> > > > > support that now with this interface, but we'll need to test that case
> > > > > carefully.
> > > > 
> > > > Do we ever want/need to remove listening sockets?
> > > 
> > > I think that might be an interesting use case. Disabling RDMA, for
> > > example, should kill the RDMA listening endpoints but leave
> > > listening sockets in place.
> > > 
> > > But for now, our socket listeners are "any". Wondering how net
> > > namespaces play into this.
> > > 
> > > 
> > > > Normal practice when making any changes is to stop and restart where
> > > > "stop" removes all sockets, unexports all filesystems, disables all
> > > > versions.
> > > > I don't exactly object to supporting fine-grained changes, but I suspect
> > > > anything that is not used by normal service start will hardly ever be
> > > > used in practice, so will not be tested.
> > > 
> > > Well, there is that. I guess until we have test coverage for NFSD
> > > administrative interfaces, we should leave well enough alone.
> > 
> > So to summarize it:
> > - we will allow to remove enabled versions (as it is in patch v6 2/3)
> > - we will allow to add new listening sockets but we will not allow to remove
> >   them (the user/admin will need to stop/start the server).
> > 
> > Agree? If so I will work on it and post v7.
> > 
> > 
> 
> That sounds about right to me. We could eventually relax the restriction
> about removing sockets later, but for now it's probably best to prohibit
> it (like Neil suggests).

Do we want to add even the capability to specify the socket file descriptor
(similar to what we do in __write_ports_addfd())?

Regards,
Lorenzo

> 
> 
> > 
> > > 
> > > 
> > > > So if it is easiest to support reverting previous configuration (as it
> > > > probably is for version setting), then do so.  But if there is any
> > > > complexity (as maybe there is with listening sockets), then don't
> > > > add complexity that won't be used.
> > > > 
> > > > Thanks,
> > > > NeilBrown
> > > 
> > > -- 
> > > Chuck Lever
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-23 13:21             ` Lorenzo Bianconi
@ 2024-01-23 14:28               ` Jeff Layton
  2024-01-24  9:52                 ` Lorenzo Bianconi
  0 siblings, 1 reply; 27+ messages in thread
From: Jeff Layton @ 2024-01-23 14:28 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Chuck Lever, NeilBrown, linux-nfs, lorenzo.bianconi, kuba, horms, netdev

On Tue, 2024-01-23 at 14:21 +0100, Lorenzo Bianconi wrote:
> > On Tue, 2024-01-23 at 10:59 +0100, Lorenzo Bianconi wrote:
> > > > On Tue, Jan 23, 2024 at 08:35:07AM +1100, NeilBrown wrote:
> > > > > On Tue, 23 Jan 2024, Jeff Layton wrote:
> > > > > > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > > > > > Introduce write_ports netlink command. For listener-set, userspace is
> > > > > > > expected to provide a NFS listeners list it wants to enable (all the
> > > > > > > other ports will be closed).
> > > > > > > 
> > > > > > 
> > > > > > Ditto here. This is a change to a declarative interface, which I think
> > > > > > is a better way to handle this, but we should be aware of the change.
> > > > > 
> > > > > I agree it is better, and thanks for highlighting the change.
> > > > > 
> > > > > > > +	/* 2- remove stale listeners */
> > > > > > 
> > > > > > 
> > > > > > The old portlist interface was weird, in that it was only additive. You
> > > > > > couldn't use it to close a listening socket (AFAICT). We may be able to
> > > > > > support that now with this interface, but we'll need to test that case
> > > > > > carefully.
> > > > > 
> > > > > Do we ever want/need to remove listening sockets?
> > > > 
> > > > I think that might be an interesting use case. Disabling RDMA, for
> > > > example, should kill the RDMA listening endpoints but leave
> > > > listening sockets in place.
> > > > 
> > > > But for now, our socket listeners are "any". Wondering how net
> > > > namespaces play into this.
> > > > 
> > > > 
> > > > > Normal practice when making any changes is to stop and restart where
> > > > > "stop" removes all sockets, unexports all filesystems, disables all
> > > > > versions.
> > > > > I don't exactly object to supporting fine-grained changes, but I suspect
> > > > > anything that is not used by normal service start will hardly ever be
> > > > > used in practice, so will not be tested.
> > > > 
> > > > Well, there is that. I guess until we have test coverage for NFSD
> > > > administrative interfaces, we should leave well enough alone.
> > > 
> > > So to summarize it:
> > > - we will allow to remove enabled versions (as it is in patch v6 2/3)
> > > - we will allow to add new listening sockets but we will not allow to remove
> > >   them (the user/admin will need to stop/start the server).
> > > 
> > > Agree? If so I will work on it and post v7.
> > > 
> > > 
> > 
> > That sounds about right to me. We could eventually relax the restriction
> > about removing sockets later, but for now it's probably best to prohibit
> > it (like Neil suggests).
> 
> Do we want to add even the capability to specify the socket file descriptor
> (similar to what we do in __write_ports_addfd())?
> 

That's a great question. We do need to properly support the -H option to
rpc.nfsd. What we do today is look up the hostname or address using
getaddrinfo, and then open a listening socket for that address and then
pass that fd down to the kernel, which I think then takes the socket and
sticks it on sv_permsocks.

All of that seems a bit klunky. Ideally, I'd say the best thing would be
to allow userland to pass the sockaddr we look up directly via netlink,
and then let the kernel open the socket. That will probably mean
refactoring some of the svc_xprt_create machinery to take a sockaddr,
but I don't think it looks too hard to do.

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-23 14:28               ` Jeff Layton
@ 2024-01-24  9:52                 ` Lorenzo Bianconi
  2024-01-24 11:24                   ` Jeff Layton
  0 siblings, 1 reply; 27+ messages in thread
From: Lorenzo Bianconi @ 2024-01-24  9:52 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Chuck Lever, NeilBrown, linux-nfs, lorenzo.bianconi, kuba, horms, netdev

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

[...]
> 
> That's a great question. We do need to properly support the -H option to
> rpc.nfsd. What we do today is look up the hostname or address using
> getaddrinfo, and then open a listening socket for that address and then
> pass that fd down to the kernel, which I think then takes the socket and
> sticks it on sv_permsocks.
> 
> All of that seems a bit klunky. Ideally, I'd say the best thing would be
> to allow userland to pass the sockaddr we look up directly via netlink,
> and then let the kernel open the socket. That will probably mean
> refactoring some of the svc_xprt_create machinery to take a sockaddr,
> but I don't think it looks too hard to do.

Do we already have a specific use case for it? I think we can even add it
later when we have a defined use case for it on top of the current series.

Regards,
Lorenzo

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

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-24  9:52                 ` Lorenzo Bianconi
@ 2024-01-24 11:24                   ` Jeff Layton
  2024-01-24 13:55                     ` Chuck Lever III
  0 siblings, 1 reply; 27+ messages in thread
From: Jeff Layton @ 2024-01-24 11:24 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Chuck Lever, NeilBrown, linux-nfs, lorenzo.bianconi, kuba, horms, netdev

On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
> [...]
> > 
> > That's a great question. We do need to properly support the -H option to
> > rpc.nfsd. What we do today is look up the hostname or address using
> > getaddrinfo, and then open a listening socket for that address and then
> > pass that fd down to the kernel, which I think then takes the socket and
> > sticks it on sv_permsocks.
> > 
> > All of that seems a bit klunky. Ideally, I'd say the best thing would be
> > to allow userland to pass the sockaddr we look up directly via netlink,
> > and then let the kernel open the socket. That will probably mean
> > refactoring some of the svc_xprt_create machinery to take a sockaddr,
> > but I don't think it looks too hard to do.
> 
> Do we already have a specific use case for it? I think we can even add it
> later when we have a defined use case for it on top of the current series.
> 

Yes:

rpc.nfsd -H makes nfsd listen on a particular address and port. By
passing down the sockaddr instead of an already-opened socket
descriptor, we can achieve the goal without having to open sockets in
userland.

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-24 11:24                   ` Jeff Layton
@ 2024-01-24 13:55                     ` Chuck Lever III
  2024-01-24 18:10                       ` Jeff Layton
  2024-01-25 22:44                       ` NeilBrown
  0 siblings, 2 replies; 27+ messages in thread
From: Chuck Lever III @ 2024-01-24 13:55 UTC (permalink / raw)
  To: Jeff Layton, Lorenzo Bianconi, Neil Brown
  Cc: Linux NFS Mailing List, Lorenzo Bianconi, Jakub Kicinski,
	Simon Horman, open list:NETWORKING [GENERAL]



> On Jan 24, 2024, at 6:24 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
>> [...]
>>> 
>>> That's a great question. We do need to properly support the -H option to
>>> rpc.nfsd. What we do today is look up the hostname or address using
>>> getaddrinfo, and then open a listening socket for that address and then
>>> pass that fd down to the kernel, which I think then takes the socket and
>>> sticks it on sv_permsocks.
>>> 
>>> All of that seems a bit klunky. Ideally, I'd say the best thing would be
>>> to allow userland to pass the sockaddr we look up directly via netlink,
>>> and then let the kernel open the socket. That will probably mean
>>> refactoring some of the svc_xprt_create machinery to take a sockaddr,
>>> but I don't think it looks too hard to do.
>> 
>> Do we already have a specific use case for it? I think we can even add it
>> later when we have a defined use case for it on top of the current series.
>> 
> 
> Yes:
> 
> rpc.nfsd -H makes nfsd listen on a particular address and port. By
> passing down the sockaddr instead of an already-opened socket
> descriptor, we can achieve the goal without having to open sockets in
> userland.

Tearing down a listener that was created that way would be a
use case for:

> Do we ever want/need to remove listening sockets?
> Normal practice when making any changes is to stop and restart where
> "stop" removes all sockets, unexports all filesystems, disables all
> versions.



--
Chuck Lever



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-24 13:55                     ` Chuck Lever III
@ 2024-01-24 18:10                       ` Jeff Layton
  2024-01-25 22:44                       ` NeilBrown
  1 sibling, 0 replies; 27+ messages in thread
From: Jeff Layton @ 2024-01-24 18:10 UTC (permalink / raw)
  To: Chuck Lever III, Lorenzo Bianconi, Neil Brown
  Cc: Linux NFS Mailing List, Lorenzo Bianconi, Jakub Kicinski,
	Simon Horman, open list:NETWORKING [GENERAL]

On Wed, 2024-01-24 at 13:55 +0000, Chuck Lever III wrote:
> 
> > On Jan 24, 2024, at 6:24 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
> > > [...]
> > > > 
> > > > That's a great question. We do need to properly support the -H option to
> > > > rpc.nfsd. What we do today is look up the hostname or address using
> > > > getaddrinfo, and then open a listening socket for that address and then
> > > > pass that fd down to the kernel, which I think then takes the socket and
> > > > sticks it on sv_permsocks.
> > > > 
> > > > All of that seems a bit klunky. Ideally, I'd say the best thing would be
> > > > to allow userland to pass the sockaddr we look up directly via netlink,
> > > > and then let the kernel open the socket. That will probably mean
> > > > refactoring some of the svc_xprt_create machinery to take a sockaddr,
> > > > but I don't think it looks too hard to do.
> > > 
> > > Do we already have a specific use case for it? I think we can even add it
> > > later when we have a defined use case for it on top of the current series.
> > > 
> > 
> > Yes:
> > 
> > rpc.nfsd -H makes nfsd listen on a particular address and port. By
> > passing down the sockaddr instead of an already-opened socket
> > descriptor, we can achieve the goal without having to open sockets in
> > userland.
> 
> Tearing down a listener that was created that way would be a
> use case for:
> 
> > Do we ever want/need to remove listening sockets?
> > Normal practice when making any changes is to stop and restart where
> > "stop" removes all sockets, unexports all filesystems, disables all
> > versions.
> 

Good point. Even if we don't want to allow it today, passing down a
sockaddr over netlink gives us an interface to properly match a
listening socket for shutting them down without taking down the server. 

It would be a little silly to make userland open a socket in order to
close the one that nfsd is listening on.
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-24 13:55                     ` Chuck Lever III
  2024-01-24 18:10                       ` Jeff Layton
@ 2024-01-25 22:44                       ` NeilBrown
  2024-01-26  2:38                         ` Chuck Lever III
  2024-01-26 13:58                         ` Chuck Lever III
  1 sibling, 2 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-25 22:44 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: Jeff Layton, Lorenzo Bianconi, Linux NFS Mailing List,
	Lorenzo Bianconi, Jakub Kicinski, Simon Horman,
	open list:NETWORKING [GENERAL]

On Thu, 25 Jan 2024, Chuck Lever III wrote:
> 
> 
> > On Jan 24, 2024, at 6:24 AM, Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
> >> [...]
> >>> 
> >>> That's a great question. We do need to properly support the -H option to
> >>> rpc.nfsd. What we do today is look up the hostname or address using
> >>> getaddrinfo, and then open a listening socket for that address and then
> >>> pass that fd down to the kernel, which I think then takes the socket and
> >>> sticks it on sv_permsocks.
> >>> 
> >>> All of that seems a bit klunky. Ideally, I'd say the best thing would be
> >>> to allow userland to pass the sockaddr we look up directly via netlink,
> >>> and then let the kernel open the socket. That will probably mean
> >>> refactoring some of the svc_xprt_create machinery to take a sockaddr,
> >>> but I don't think it looks too hard to do.
> >> 
> >> Do we already have a specific use case for it? I think we can even add it
> >> later when we have a defined use case for it on top of the current series.
> >> 
> > 
> > Yes:
> > 
> > rpc.nfsd -H makes nfsd listen on a particular address and port. By
> > passing down the sockaddr instead of an already-opened socket
> > descriptor, we can achieve the goal without having to open sockets in
> > userland.
> 
> Tearing down a listener that was created that way would be a
> use case for:

Only if it was actually useful.
Have you *ever* wanted to do that?  Or heard from anyone else who did?

NeilBrown


> 
> > Do we ever want/need to remove listening sockets?
> > Normal practice when making any changes is to stop and restart where
> > "stop" removes all sockets, unexports all filesystems, disables all
> > versions.
> 
> 
> 
> --
> Chuck Lever
> 
> 
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-25 22:44                       ` NeilBrown
@ 2024-01-26  2:38                         ` Chuck Lever III
  2024-01-26  7:27                           ` NeilBrown
  2024-01-26 13:58                         ` Chuck Lever III
  1 sibling, 1 reply; 27+ messages in thread
From: Chuck Lever III @ 2024-01-26  2:38 UTC (permalink / raw)
  To: Neil Brown
  Cc: Jeff Layton, Lorenzo Bianconi, Linux NFS Mailing List,
	Lorenzo Bianconi, Jakub Kicinski, Simon Horman,
	open list:NETWORKING [GENERAL]



> On Jan 25, 2024, at 5:44 PM, NeilBrown <neilb@suse.de> wrote:
> 
> On Thu, 25 Jan 2024, Chuck Lever III wrote:
>> 
>> 
>>> On Jan 24, 2024, at 6:24 AM, Jeff Layton <jlayton@kernel.org> wrote:
>>> 
>>> On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
>>>> [...]
>>>>> 
>>>>> That's a great question. We do need to properly support the -H option to
>>>>> rpc.nfsd. What we do today is look up the hostname or address using
>>>>> getaddrinfo, and then open a listening socket for that address and then
>>>>> pass that fd down to the kernel, which I think then takes the socket and
>>>>> sticks it on sv_permsocks.
>>>>> 
>>>>> All of that seems a bit klunky. Ideally, I'd say the best thing would be
>>>>> to allow userland to pass the sockaddr we look up directly via netlink,
>>>>> and then let the kernel open the socket. That will probably mean
>>>>> refactoring some of the svc_xprt_create machinery to take a sockaddr,
>>>>> but I don't think it looks too hard to do.
>>>> 
>>>> Do we already have a specific use case for it? I think we can even add it
>>>> later when we have a defined use case for it on top of the current series.
>>>> 
>>> 
>>> Yes:
>>> 
>>> rpc.nfsd -H makes nfsd listen on a particular address and port. By
>>> passing down the sockaddr instead of an already-opened socket
>>> descriptor, we can achieve the goal without having to open sockets in
>>> userland.
>> 
>> Tearing down a listener that was created that way would be a
>> use case for:
> 
> Only if it was actually useful.
> Have you *ever* wanted to do that?  Or heard from anyone else who did?

Container shutdown will want to clear out any listener
that might have been created during the container's
lifetime. How is that done today? Is that simply handled
by net namespace tear-down?


>>> Do we ever want/need to remove listening sockets?
>>> Normal practice when making any changes is to stop and restart where
>>> "stop" removes all sockets, unexports all filesystems, disables all
>>> versions.
>> 
>> 
>> 
>> --
>> Chuck Lever


--
Chuck Lever



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-26  2:38                         ` Chuck Lever III
@ 2024-01-26  7:27                           ` NeilBrown
  0 siblings, 0 replies; 27+ messages in thread
From: NeilBrown @ 2024-01-26  7:27 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: Jeff Layton, Lorenzo Bianconi, Linux NFS Mailing List,
	Lorenzo Bianconi, Jakub Kicinski, Simon Horman,
	open list:NETWORKING [GENERAL]

On Fri, 26 Jan 2024, Chuck Lever III wrote:
> 
> 
> > On Jan 25, 2024, at 5:44 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > On Thu, 25 Jan 2024, Chuck Lever III wrote:
> >> 
> >> 
> >>> On Jan 24, 2024, at 6:24 AM, Jeff Layton <jlayton@kernel.org> wrote:
> >>> 
> >>> On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
> >>>> [...]
> >>>>> 
> >>>>> That's a great question. We do need to properly support the -H option to
> >>>>> rpc.nfsd. What we do today is look up the hostname or address using
> >>>>> getaddrinfo, and then open a listening socket for that address and then
> >>>>> pass that fd down to the kernel, which I think then takes the socket and
> >>>>> sticks it on sv_permsocks.
> >>>>> 
> >>>>> All of that seems a bit klunky. Ideally, I'd say the best thing would be
> >>>>> to allow userland to pass the sockaddr we look up directly via netlink,
> >>>>> and then let the kernel open the socket. That will probably mean
> >>>>> refactoring some of the svc_xprt_create machinery to take a sockaddr,
> >>>>> but I don't think it looks too hard to do.
> >>>> 
> >>>> Do we already have a specific use case for it? I think we can even add it
> >>>> later when we have a defined use case for it on top of the current series.
> >>>> 
> >>> 
> >>> Yes:
> >>> 
> >>> rpc.nfsd -H makes nfsd listen on a particular address and port. By
> >>> passing down the sockaddr instead of an already-opened socket
> >>> descriptor, we can achieve the goal without having to open sockets in
> >>> userland.
> >> 
> >> Tearing down a listener that was created that way would be a
> >> use case for:
> > 
> > Only if it was actually useful.
> > Have you *ever* wanted to do that?  Or heard from anyone else who did?
> 
> Container shutdown will want to clear out any listener
> that might have been created during the container's
> lifetime. How is that done today? Is that simply handled
> by net namespace tear-down?

Yes.  When the last thread in a netns exits, nfsd_last_thread() is
called which closes all sockets.

NeilBrown

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
  2024-01-25 22:44                       ` NeilBrown
  2024-01-26  2:38                         ` Chuck Lever III
@ 2024-01-26 13:58                         ` Chuck Lever III
  1 sibling, 0 replies; 27+ messages in thread
From: Chuck Lever III @ 2024-01-26 13:58 UTC (permalink / raw)
  To: Neil Brown
  Cc: Jeff Layton, Lorenzo Bianconi, Linux NFS Mailing List,
	Lorenzo Bianconi, Jakub Kicinski, Simon Horman,
	open list:NETWORKING [GENERAL]



> On Jan 25, 2024, at 5:44 PM, NeilBrown <neilb@suse.de> wrote:
> 
> On Thu, 25 Jan 2024, Chuck Lever III wrote:
>> 
>> 
>>> On Jan 24, 2024, at 6:24 AM, Jeff Layton <jlayton@kernel.org> wrote:
>>> 
>>> On Wed, 2024-01-24 at 10:52 +0100, Lorenzo Bianconi wrote:
>>>> [...]
>>>>> 
>>>>> That's a great question. We do need to properly support the -H option to
>>>>> rpc.nfsd. What we do today is look up the hostname or address using
>>>>> getaddrinfo, and then open a listening socket for that address and then
>>>>> pass that fd down to the kernel, which I think then takes the socket and
>>>>> sticks it on sv_permsocks.
>>>>> 
>>>>> All of that seems a bit klunky. Ideally, I'd say the best thing would be
>>>>> to allow userland to pass the sockaddr we look up directly via netlink,
>>>>> and then let the kernel open the socket. That will probably mean
>>>>> refactoring some of the svc_xprt_create machinery to take a sockaddr,
>>>>> but I don't think it looks too hard to do.
>>>> 
>>>> Do we already have a specific use case for it? I think we can even add it
>>>> later when we have a defined use case for it on top of the current series.
>>>> 
>>> 
>>> Yes:
>>> 
>>> rpc.nfsd -H makes nfsd listen on a particular address and port. By
>>> passing down the sockaddr instead of an already-opened socket
>>> descriptor, we can achieve the goal without having to open sockets in
>>> userland.
>> 
>> Tearing down a listener that was created that way would be a
>> use case for:
> 
> Only if it was actually useful.
> Have you *ever* wanted to do that?  Or heard from anyone else who did?

Another possibility is removing a listener when unplugging a
network device. That also might be automatic already.

But hey, we don't have this kind of administrative capability
today, so there's no need to add it in a first pass of this
new interface either. I'm happy to wait and see.


>>> Do we ever want/need to remove listening sockets?
>>> Normal practice when making any changes is to stop and restart where
>>> "stop" removes all sockets, unexports all filesystems, disables all
>>> versions.


--
Chuck Lever



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2024-01-26 13:58 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-20 17:33 [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Lorenzo Bianconi
2024-01-20 17:33 ` [PATCH v6 1/3] NFSD: convert write_threads to netlink command Lorenzo Bianconi
2024-01-20 17:33 ` [PATCH v6 2/3] NFSD: add write_version " Lorenzo Bianconi
2024-01-22 13:27   ` Jeff Layton
2024-01-20 17:33 ` [PATCH v6 3/3] NFSD: add write_ports " Lorenzo Bianconi
2024-01-22 13:41   ` Jeff Layton
2024-01-22 15:35     ` Lorenzo Bianconi
2024-01-22 15:50       ` Jeff Layton
2024-01-22 15:59         ` Lorenzo Bianconi
2024-01-22 21:35     ` NeilBrown
2024-01-22 21:37       ` Jeff Layton
2024-01-22 22:58       ` Chuck Lever
2024-01-23  9:59         ` Lorenzo Bianconi
2024-01-23 11:17           ` Jeff Layton
2024-01-23 13:21             ` Lorenzo Bianconi
2024-01-23 14:28               ` Jeff Layton
2024-01-24  9:52                 ` Lorenzo Bianconi
2024-01-24 11:24                   ` Jeff Layton
2024-01-24 13:55                     ` Chuck Lever III
2024-01-24 18:10                       ` Jeff Layton
2024-01-25 22:44                       ` NeilBrown
2024-01-26  2:38                         ` Chuck Lever III
2024-01-26  7:27                           ` NeilBrown
2024-01-26 13:58                         ` Chuck Lever III
2024-01-22 16:31   ` kernel test robot
2024-01-22 13:49 ` [PATCH v6 0/3] convert write_threads, write_version and write_ports to netlink commands Jeff Layton
2024-01-22 17:01   ` Lorenzo Bianconi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).