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

Commit Message

Lorenzo Bianconi Jan. 20, 2024, 5:33 p.m. UTC

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 <[e-mail]>
---
 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(+)

Comments

Jeff Layton Jan. 22, 2024, 1:41 p.m. UTC | #1

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 <[e-mail]>
> ---
>  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 */

Lorenzo Bianconi Jan. 22, 2024, 3:35 p.m. UTC | #2

> 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 <[e-mail]>
> > ---
> >  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 <[e-mail]>
>

Jeff Layton Jan. 22, 2024, 3:50 p.m. UTC | #3

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 <[e-mail]>
> > > ---
> > >  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 <[e-mail]>
> >

Lorenzo Bianconi Jan. 22, 2024, 3:59 p.m. UTC | #4

[...]
> > > 
> > > 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 <[e-mail]>
> > > 
> 
> -- 
> Jeff Layton <[e-mail]>

kernel test robot Jan. 22, 2024, 4:31 p.m. UTC | #5

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

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

NeilBrown Jan. 22, 2024, 9:35 p.m. UTC | #6

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

Jeff Layton Jan. 22, 2024, 9:37 p.m. UTC | #7

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.

Chuck Lever III Jan. 22, 2024, 10:58 p.m. UTC | #8

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

Lorenzo Bianconi Jan. 23, 2024, 9:59 a.m. UTC | #9

> 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

Jeff Layton Jan. 23, 2024, 11:17 a.m. UTC | #10

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

Lorenzo Bianconi Jan. 23, 2024, 1:21 p.m. UTC | #11

> 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 <[e-mail]>

Jeff Layton Jan. 23, 2024, 2:28 p.m. UTC | #12

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.

Lorenzo Bianconi Jan. 24, 2024, 9:52 a.m. UTC | #13

[...]
> 
> 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 <[e-mail]>
>

Jeff Layton Jan. 24, 2024, 11:24 a.m. UTC | #14

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.

Chuck Lever III Jan. 24, 2024, 1:55 p.m. UTC | #15

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

Jeff Layton Jan. 24, 2024, 6:10 p.m. UTC | #16

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

NeilBrown Jan. 25, 2024, 10:44 p.m. UTC | #17

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

Chuck Lever III Jan. 26, 2024, 2:38 a.m. UTC | #18

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

NeilBrown Jan. 26, 2024, 7:27 a.m. UTC | #19

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

Chuck Lever III Jan. 26, 2024, 1:58 p.m. UTC | #20

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

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

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


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


--
Chuck Lever

Patch

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 */