LinuxLists.cc - [PATCH v6 0/3] convert write_threads, write_version and write

[permalink] [raw]

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

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

Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Documentation/netlink/specs/nfsd.yaml | 23 +++++++
fs/nfsd/netlink.c | 17 +++++
fs/nfsd/netlink.h | 2 +
fs/nfsd/nfsctl.c | 60 +++++++++++++++++
include/uapi/linux/nfsd_netlink.h | 9 +++
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

2024-01-20 17:34:25

[permalink] [raw]

Subject: [PATCH v6 2/3] NFSD: add write_version to netlink command

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 <[email protected]>
---
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

2024-01-20 17:34:33

[permalink] [raw]

Subject: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>
---
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

2024-01-22 13:27:10

[permalink] [raw]

Subject: Re: [PATCH v6 2/3] NFSD: add write_version to netlink command

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 <[email protected]>
> ---
> 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 <[email protected]>

2024-01-22 13:44:49

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>
> ---
> 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 <[email protected]>

2024-01-22 13:50:04

[permalink] [raw]

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

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 <[email protected]>

2024-01-22 16:59:45

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>

Attachments:

(No filename) (23.85 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-22 17:05:47

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>
> > > ---
> > > ?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 <[email protected]>
> >

--
Jeff Layton <[email protected]>

2024-01-22 17:42:40

[permalink] [raw]

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

> 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 <[email protected]>

Attachments:

(No filename) (3.31 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-22 17:53:54

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

[...]
> > >
> > > 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 <[email protected]>
> > >
>
> --
> Jeff Layton <[email protected]>

Attachments:

(No filename) (16.44 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-22 18:00:20

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

2024-01-22 21:36:41

by NeilBrown

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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

2024-01-22 21:39:56

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>

2024-01-22 23:00:33

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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

2024-01-23 10:09:54

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

> 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

Attachments:

(No filename) (2.34 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-23 11:30:42

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>

2024-01-23 13:38:41

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

> 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 <[email protected]>

Attachments:

(No filename) (3.01 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-23 14:28:59

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>

2024-01-24 09:54:48

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

[...]
>
> 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 <[email protected]>
>

Attachments:

(No filename) (908.00 B)
signature.asc (235.00 B)
Download all attachments

2024-01-24 11:24:29

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

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 <[email protected]>

2024-01-24 13:55:34

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

> On Jan 24, 2024, at 6:24 AM, Jeff Layton <[email protected]> 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

2024-01-24 18:10:59

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

On Wed, 2024-01-24 at 13:55 +0000, Chuck Lever III wrote:
>
> > On Jan 24, 2024, at 6:24 AM, Jeff Layton <[email protected]> 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 <[email protected]>

2024-01-25 22:44:20

by NeilBrown

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

On Thu, 25 Jan 2024, Chuck Lever III wrote:
>
>
> > On Jan 24, 2024, at 6:24 AM, Jeff Layton <[email protected]> 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
>
>
>

2024-01-26 02:39:06

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

> On Jan 25, 2024, at 5:44 PM, NeilBrown <[email protected]> wrote:
>
> On Thu, 25 Jan 2024, Chuck Lever III wrote:
>>
>>
>>> On Jan 24, 2024, at 6:24 AM, Jeff Layton <[email protected]> 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

2024-01-26 08:58:40

by NeilBrown

[permalink] [raw]

Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command

On Fri, 26 Jan 2024, Chuck Lever III wrote:
>
>
> > On Jan 25, 2024, at 5:44 PM, NeilBrown <[email protected]> wrote:
> >
> > On Thu, 25 Jan 2024, Chuck Lever III wrote:
> >>
> >>
> >>> On Jan 24, 2024, at 6:24 AM, Jeff Layton <[email protected]> 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

2024-01-26 14:14:05