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

Lorenzo Bianconi
2023-11-30T09:57:11Z

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

> On Wed, 2023-11-29 at 18:12 +0100, Lorenzo Bianconi wrote:
> > Introduce write_ports netlink command similar to the ones available
> > through the procfs.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > Documentation/netlink/specs/nfsd.yaml | 28 +++++++
> > fs/nfsd/netlink.c | 18 +++++
> > fs/nfsd/netlink.h | 3 +
> > fs/nfsd/nfsctl.c | 104 ++++++++++++++++++++++++--
> > include/uapi/linux/nfsd_netlink.h | 10 +++
> > tools/net/ynl/generated/nfsd-user.c | 81 ++++++++++++++++++++
> > tools/net/ynl/generated/nfsd-user.h | 54 +++++++++++++
> > 7 files changed, 291 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> > index 6c5e42bb20f6..1c342ad3c5fa 100644
> > --- a/Documentation/netlink/specs/nfsd.yaml
> > +++ b/Documentation/netlink/specs/nfsd.yaml
> > @@ -80,6 +80,15 @@ attribute-sets:
> > -
> > name: status
> > type: u8
> > + -
> > + name: server-listener
> > + attributes:
> > + -
> > + name: transport-name
> > + type: string
> > + -
> > + name: port
> > + type: u32
> >
> > operations:
> > list:
> > @@ -142,3 +151,22 @@ operations:
> > attributes:
> > - major
> > - minor
> > + -
> > + name: listener-start
> > + doc: start server listener
> > + attribute-set: server-listener
> > + flags: [ admin-perm ]
> > + do:
> > + request:
> > + attributes:
> > + - transport-name
> > + - port
> > + -
> > + name: listener-get
> > + doc: dump server listeners
> > + attribute-set: server-listener
> > + dump:
> > + reply:
> > + attributes:
> > + - transport-name
> > + - port
> > diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> > index 0608a7bd193b..cd51393ede72 100644
> > --- a/fs/nfsd/netlink.c
> > +++ b/fs/nfsd/netlink.c
> > @@ -22,6 +22,12 @@ static const struct nla_policy nfsd_version_set_nl_policy[NFSD_A_SERVER_VERSION_
> > [NFSD_A_SERVER_VERSION_STATUS] = { .type = NLA_U8, },
> > };
> >
> > +/* NFSD_CMD_LISTENER_START - do */
> > +static const struct nla_policy nfsd_listener_start_nl_policy[NFSD_A_SERVER_LISTENER_PORT + 1] = {
> > + [NFSD_A_SERVER_LISTENER_TRANSPORT_NAME] = { .type = NLA_NUL_STRING, },
> > + [NFSD_A_SERVER_LISTENER_PORT] = { .type = NLA_U32, },
> > +};
> > +
> > /* Ops table for nfsd */
> > static const struct genl_split_ops nfsd_nl_ops[] = {
> > {
> > @@ -55,6 +61,18 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
> > .dumpit = nfsd_nl_version_get_dumpit,
> > .flags = GENL_CMD_CAP_DUMP,
> > },
> > + {
> > + .cmd = NFSD_CMD_LISTENER_START,
> > + .doit = nfsd_nl_listener_start_doit,
> > + .policy = nfsd_listener_start_nl_policy,
> > + .maxattr = NFSD_A_SERVER_LISTENER_PORT,
> > + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > + },
> > + {
> > + .cmd = NFSD_CMD_LISTENER_GET,
> > + .dumpit = nfsd_nl_listener_get_dumpit,
> > + .flags = GENL_CMD_CAP_DUMP,
> > + },
> > };
> >
> > struct genl_family nfsd_nl_family __ro_after_init = {
> > diff --git a/fs/nfsd/netlink.h b/fs/nfsd/netlink.h
> > index 7d203cec08e4..9a51cb83f343 100644
> > --- a/fs/nfsd/netlink.h
> > +++ b/fs/nfsd/netlink.h
> > @@ -21,6 +21,9 @@ int nfsd_nl_threads_get_doit(struct sk_buff *skb, struct genl_info *info);
> > int nfsd_nl_version_set_doit(struct sk_buff *skb, struct genl_info *info);
> > int nfsd_nl_version_get_dumpit(struct sk_buff *skb,
> > struct netlink_callback *cb);
> > +int nfsd_nl_listener_start_doit(struct sk_buff *skb, struct genl_info *info);
> > +int nfsd_nl_listener_get_dumpit(struct sk_buff *skb,
> > + struct netlink_callback *cb);
> >
> > extern struct genl_family nfsd_nl_family;
> >
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index f04430f79687..53129b5b7d3c 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -721,18 +721,16 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred
> > * A transport listener is added by writing its transport name and
> > * a port number.
> > */
> > -static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cred *cred)
> > +static ssize_t ___write_ports_addxprt(struct net *net, const struct cred *cred,
> > + const char *transport, const int port)
> > {
> > - char transport[16];
> > - struct svc_xprt *xprt;
> > - int port, err;
> > struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > -
> > - if (sscanf(buf, "%15s %5u", transport, &port) != 2)
> > - return -EINVAL;
> > + struct svc_xprt *xprt;
> > + int err;
> >
> > if (port < 1 || port > USHRT_MAX)
> > return -EINVAL;
> > +
> > trace_nfsd_ctl_ports_addxprt(net, transport, port);
> >
> > err = nfsd_create_serv(net);
> > @@ -765,6 +763,17 @@ static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cr
> > return err;
> > }
> >
> > +static ssize_t __write_ports_addxprt(char *buf, struct net *net, const struct cred *cred)
> > +{
> > + char transport[16];
> > + int port;
> > +
> > + if (sscanf(buf, "%15s %5u", transport, &port) != 2)
> > + return -EINVAL;
> > +
> > + return ___write_ports_addxprt(net, cred, transport, port);
> > +}
> > +
> > static ssize_t __write_ports(struct file *file, char *buf, size_t size,
> > struct net *net)
> > {
> > @@ -1862,6 +1871,87 @@ int nfsd_nl_version_get_dumpit(struct sk_buff *skb,
> > return ret;
> > }
> >
> > +/**
> > + * nfsd_nl_listener_start_doit - start the provided nfs server listener
> > + * @skb: reply buffer
> > + * @info: netlink metadata and command arguments
> > + *
> > + * Return 0 on success or a negative errno.
> > + */
> > +int nfsd_nl_listener_start_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + int ret;
> > +
> > + if (GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME) ||
> > + GENL_REQ_ATTR_CHECK(info, NFSD_A_SERVER_LISTENER_PORT))
> > + return -EINVAL;
> > +
> > + mutex_lock(&nfsd_mutex);
> > + ret = ___write_ports_addxprt(genl_info_net(info), get_current_cred(),
> > + nla_data(info->attrs[NFSD_A_SERVER_LISTENER_TRANSPORT_NAME]),
> > + nla_get_u32(info->attrs[NFSD_A_SERVER_LISTENER_PORT]));
> > + mutex_unlock(&nfsd_mutex);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * nfsd_nl_version_get_dumpit - Handle listener_get dumpit
> > + * @skb: reply buffer
> > + * @cb: netlink metadata and command arguments
> > + *
> > + * Returns the size of the reply or a negative errno.
> > + */
> > +int nfsd_nl_listener_get_dumpit(struct sk_buff *skb,
> > + struct netlink_callback *cb)
> > +{
> > + struct nfsd_net *nn = net_generic(sock_net(skb->sk), nfsd_net_id);
> > + int i = 0, ret = -ENOMEM;
> > + struct svc_xprt *xprt;
> > + struct svc_serv *serv;
> > +
> > + mutex_lock(&nfsd_mutex);
> > +
> > + serv = nn->nfsd_serv;
> > + if (!serv) {
> > + mutex_unlock(&nfsd_mutex);
> > + return 0;
> > + }
> > +
> > + spin_lock_bh(&serv->sv_lock);
> > + list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> > + void *hdr;
> > +
> > + if (i < cb->args[0]) /* already consumed */
> > + continue;
> > +
> > + hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid,
> > + cb->nlh->nlmsg_seq, &nfsd_nl_family,
> > + 0, NFSD_CMD_LISTENER_GET);
> > + if (!hdr)
> > + goto out;
> > +
> > + if (nla_put_string(skb, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME,
> > + xprt->xpt_class->xcl_name))
> > + goto out;
> > +
> > + if (nla_put_u32(skb, NFSD_A_SERVER_LISTENER_PORT,
> > + svc_xprt_local_port(xprt)))
> > + goto out;
> > +
> > + genlmsg_end(skb, hdr);
> > + i++;
> > + }
> > + cb->args[0] = i;
> > + ret = skb->len;
> > +out:
> > + spin_unlock_bh(&serv->sv_lock);
> > +
> > + mutex_unlock(&nfsd_mutex);
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
> > * @net: a freshly-created network namespace
> > diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> > index 1b3340f31baa..61f4c5b50ecb 100644
> > --- a/include/uapi/linux/nfsd_netlink.h
> > +++ b/include/uapi/linux/nfsd_netlink.h
> > @@ -45,12 +45,22 @@ enum {
> > NFSD_A_SERVER_VERSION_MAX = (__NFSD_A_SERVER_VERSION_MAX - 1)
> > };
> >
> > +enum {
> > + NFSD_A_SERVER_LISTENER_TRANSPORT_NAME = 1,
> > + NFSD_A_SERVER_LISTENER_PORT,
> > +
> > + __NFSD_A_SERVER_LISTENER_MAX,
> > + NFSD_A_SERVER_LISTENER_MAX = (__NFSD_A_SERVER_LISTENER_MAX - 1)
> > +};
> > +
> > enum {
> > NFSD_CMD_RPC_STATUS_GET = 1,
> > NFSD_CMD_THREADS_SET,
> > NFSD_CMD_THREADS_GET,
> > NFSD_CMD_VERSION_SET,
> > NFSD_CMD_VERSION_GET,
> > + NFSD_CMD_LISTENER_START,
> > + NFSD_CMD_LISTENER_GET,
> >
> > __NFSD_CMD_MAX,
> > NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)
> > diff --git a/tools/net/ynl/generated/nfsd-user.c b/tools/net/ynl/generated/nfsd-user.c
> > index 4cb71c3cd18d..167e404c9e20 100644
> > --- a/tools/net/ynl/generated/nfsd-user.c
> > +++ b/tools/net/ynl/generated/nfsd-user.c
> > @@ -19,6 +19,8 @@ static const char * const nfsd_op_strmap[] = {
> > [NFSD_CMD_THREADS_GET] = "threads-get",
> > [NFSD_CMD_VERSION_SET] = "version-set",
> > [NFSD_CMD_VERSION_GET] = "version-get",
> > + [NFSD_CMD_LISTENER_START] = "listener-start",
> > + [NFSD_CMD_LISTENER_GET] = "listener-get",
> > };
> >
> > const char *nfsd_op_str(int op)
> > @@ -71,6 +73,16 @@ struct ynl_policy_nest nfsd_server_version_nest = {
> > .table = nfsd_server_version_policy,
> > };
> >
> > +struct ynl_policy_attr nfsd_server_listener_policy[NFSD_A_SERVER_LISTENER_MAX + 1] = {
> > + [NFSD_A_SERVER_LISTENER_TRANSPORT_NAME] = { .name = "transport-name", .type = YNL_PT_NUL_STR, },
> > + [NFSD_A_SERVER_LISTENER_PORT] = { .name = "port", .type = YNL_PT_U32, },
> > +};
> > +
> > +struct ynl_policy_nest nfsd_server_listener_nest = {
> > + .max_attr = NFSD_A_SERVER_LISTENER_MAX,
> > + .table = nfsd_server_listener_policy,
> > +};
> > +
> > /* Common nested types */
> > /* ============== NFSD_CMD_RPC_STATUS_GET ============== */
> > /* NFSD_CMD_RPC_STATUS_GET - dump */
> > @@ -371,6 +383,75 @@ struct nfsd_version_get_list *nfsd_version_get_dump(struct ynl_sock *ys)
> > return NULL;
> > }
> >
> > +/* ============== NFSD_CMD_LISTENER_START ============== */
> > +/* NFSD_CMD_LISTENER_START - do */
> > +void nfsd_listener_start_req_free(struct nfsd_listener_start_req *req)
> > +{
> > + free(req->transport_name);
> > + free(req);
> > +}
> > +
> > +int nfsd_listener_start(struct ynl_sock *ys,
> > + struct nfsd_listener_start_req *req)
> > +{
> > + struct nlmsghdr *nlh;
> > + int err;
> > +
> > + nlh = ynl_gemsg_start_req(ys, ys->family_id, NFSD_CMD_LISTENER_START, 1);
> > + ys->req_policy = &nfsd_server_listener_nest;
> > +
> > + if (req->_present.transport_name_len)
> > + mnl_attr_put_strz(nlh, NFSD_A_SERVER_LISTENER_TRANSPORT_NAME, req->transport_name);
> > + if (req->_present.port)
> > + mnl_attr_put_u32(nlh, NFSD_A_SERVER_LISTENER_PORT, req->port);
> > +
> > + err = ynl_exec(ys, nlh, NULL);
> > + if (err < 0)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > +/* NFSD_CMD_LISTENER_GET - dump */
> > +void nfsd_listener_get_list_free(struct nfsd_listener_get_list *rsp)
> > +{
> > + struct nfsd_listener_get_list *next = rsp;
> > +
> > + while ((void *)next != YNL_LIST_END) {
> > + rsp = next;
> > + next = rsp->next;
> > +
> > + free(rsp->obj.transport_name);
> > + free(rsp);
> > + }
> > +}
> > +
> > +struct nfsd_listener_get_list *nfsd_listener_get_dump(struct ynl_sock *ys)
> > +{
> > + struct ynl_dump_state yds = {};
> > + struct nlmsghdr *nlh;
> > + int err;
> > +
> > + yds.ys = ys;
> > + yds.alloc_sz = sizeof(struct nfsd_listener_get_list);
> > + yds.cb = nfsd_listener_get_rsp_parse;
> > + yds.rsp_cmd = NFSD_CMD_LISTENER_GET;
> > + yds.rsp_policy = &nfsd_server_listener_nest;
> > +
> > + nlh = ynl_gemsg_start_dump(ys, ys->family_id, NFSD_CMD_LISTENER_GET, 1);
> > +
> > + err = ynl_exec_dump(ys, nlh, &yds);
> > + if (err < 0)
> > + goto free_list;
> > +
> > + return yds.first;
> > +
> > +free_list:
> > + nfsd_listener_get_list_free(yds.first);
> > + return NULL;
> > +}
> > +
> > const struct ynl_family ynl_nfsd_family = {
> > .name = "nfsd",
> > };
> > diff --git a/tools/net/ynl/generated/nfsd-user.h b/tools/net/ynl/generated/nfsd-user.h
> > index e61c5a9e46fb..da3aaaf3f6c0 100644
> > --- a/tools/net/ynl/generated/nfsd-user.h
> > +++ b/tools/net/ynl/generated/nfsd-user.h
> > @@ -166,4 +166,58 @@ void nfsd_version_get_list_free(struct nfsd_version_get_list *rsp);
> >
> > struct nfsd_version_get_list *nfsd_version_get_dump(struct ynl_sock *ys);
> >
> > +/* ============== NFSD_CMD_LISTENER_START ============== */
> > +/* NFSD_CMD_LISTENER_START - do */
> > +struct nfsd_listener_start_req {
> > + struct {
> > + __u32 transport_name_len;
> > + __u32 port:1;
> > + } _present;
> > +
> > + char *transport_name;
> > + __u32 port;
> > +};
>
> How do you deconfigure a listener with this interface? i.e. suppose I
> want to stop nfsd from listening on a particular port? I think this too
> is a place where a declarative interface would be better:

Is it possible with current APIs? as for 2/3 so far I have just added netlink
counter for current implementation but I am fine to change the logic here to
better APIs.

Regards,
Lorenzo

>
> Have userland send down a list of the ports that we should currently be
> listening on, and let the kernel do the work to match the request. Again
> too, an empty list could mean "close everything".
>
> > +
> > +static inline struct nfsd_listener_start_req *
> > +nfsd_listener_start_req_alloc(void)
> > +{
> > + return calloc(1, sizeof(struct nfsd_listener_start_req));
> > +}
> > +void nfsd_listener_start_req_free(struct nfsd_listener_start_req *req);
> > +
> > +static inline void
> > +nfsd_listener_start_req_set_transport_name(struct nfsd_listener_start_req *req,
> > + const char *transport_name)
> > +{
> > + free(req->transport_name);
> > + req->_present.transport_name_len = strlen(transport_name);
> > + req->transport_name = malloc(req->_present.transport_name_len + 1);
> > + memcpy(req->transport_name, transport_name, req->_present.transport_name_len);
> > + req->transport_name[req->_present.transport_name_len] = 0;
> > +}
> > +static inline void
> > +nfsd_listener_start_req_set_port(struct nfsd_listener_start_req *req,
> > + __u32 port)
> > +{
> > + req->_present.port = 1;
> > + req->port = port;
> > +}
> > +
> > +/*
> > + * start server listener
> > + */
> > +int nfsd_listener_start(struct ynl_sock *ys,
> > + struct nfsd_listener_start_req *req);
> > +
> > +/* ============== NFSD_CMD_LISTENER_GET ============== */
> > +/* NFSD_CMD_LISTENER_GET - dump */
> > +struct nfsd_listener_get_list {
> > + struct nfsd_listener_get_list *next;
> > + struct nfsd_listener_get_rsp obj __attribute__ ((aligned (8)));
> > +};
> > +
> > +void nfsd_listener_get_list_free(struct nfsd_listener_get_list *rsp);
> > +
> > +struct nfsd_listener_get_list *nfsd_listener_get_dump(struct ynl_sock *ys);
> > +
> > #endif /* _LINUX_NFSD_GEN_H */
>
> --
> Jeff Layton <jlayton@kernel.org>
>

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