Re: [PATCH v4 2/6] NFSD: Add NFSD_CMD_UNLOCK netlink command with ip scope

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2026-03-18 at 10:15 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> The existing write_unlock_ip procfs interface releases NLM file
> locks held by a specific client IP address, but procfs provides
> no structured way to extend that operation to other scopes such
> as revoking NFSv4 state. A netlink command allows the operation
> to carry typed, validated attributes and supports future scope
> values without interface proliferation.
> 
> NFSD_CMD_UNLOCK accepts an unlock-type attribute selecting the
> scope and an address attribute carrying a binary sockaddr_in
> or sockaddr_in6. The handler validates the address family
> and length, then calls nlmsvc_unlock_all_by_ip() to release
> matching NLM locks. Because lockd is a single global instance,
> that call operates across all network namespaces regardless of
> which namespace the caller inhabits. The command requires admin
> privileges via GENL_ADMIN_PERM.
> 
> The unlock-type enum begins with a single value, ip, and is
> defined with render-max so that future values can be added
> without breaking existing userspace.
> 
> The nfsd_ctl_unlock_ip tracepoint is updated from string-based
> address logging to __sockaddr, which stores the binary sockaddr
> and formats it with %pISpc. This affects both the new netlink
> path and the existing procfs write_unlock_ip path, giving
> consistent structured output in both cases.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  Documentation/netlink/specs/nfsd.yaml | 32 ++++++++++++++++++
>  fs/nfsd/netlink.c                     | 13 ++++++++
>  fs/nfsd/netlink.h                     |  1 +
>  fs/nfsd/nfsctl.c                      | 63 ++++++++++++++++++++++++++++++++++-
>  fs/nfsd/trace.h                       | 13 ++++----
>  include/uapi/linux/nfsd_netlink.h     | 17 ++++++++++
>  6 files changed, 132 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/netlink/specs/nfsd.yaml b/Documentation/netlink/specs/nfsd.yaml
> index f87b5a05e5e9..02fadfca22ba 100644
> --- a/Documentation/netlink/specs/nfsd.yaml
> +++ b/Documentation/netlink/specs/nfsd.yaml
> @@ -6,6 +6,13 @@ uapi-header: linux/nfsd_netlink.h
>  
>  doc: NFSD configuration over generic netlink.
>  
> +definitions:
> +  -
> +    type: enum
> +    name: unlock-type
> +    render-max: true
> +    entries: [ip]
> +
>  attribute-sets:
>    -
>      name: rpc-status
> @@ -127,6 +134,21 @@ attribute-sets:
>        -
>          name: npools
>          type: u32
> +  -
> +    name: unlock
> +    attributes:
> +      -
> +        name: type
> +        type: u32
> +        enum: unlock-type
> +      -
> +        name: address
> +        type: binary
> +        doc: >-
> +          struct sockaddr_in or struct sockaddr_in6.
> +          Required when type is ip.
> +        checks:
> +          min-len: 16
>  
>  operations:
>    list:
> @@ -227,3 +249,13 @@ operations:
>            attributes:
>              - mode
>              - npools
> +    -
> +      name: unlock
> +      doc: release NLM locks by scope
> +      attribute-set: unlock
> +      flags: [admin-perm]
> +      do:
> +        request:
> +          attributes:
> +            - type
> +            - address

I wonder if we'd be better served with different commands instead of
passing a type value to a single command? Different types are going to
require different attributes, and it'll be easier to validate those if
they use different commands.

> diff --git a/fs/nfsd/netlink.c b/fs/nfsd/netlink.c
> index 887525964451..9ec0d56eaa21 100644
> --- a/fs/nfsd/netlink.c
> +++ b/fs/nfsd/netlink.c
> @@ -47,6 +47,12 @@ static const struct nla_policy nfsd_pool_mode_set_nl_policy[NFSD_A_POOL_MODE_MOD
>  	[NFSD_A_POOL_MODE_MODE] = { .type = NLA_NUL_STRING, },
>  };
>  
> +/* NFSD_CMD_UNLOCK - do */
> +static const struct nla_policy nfsd_unlock_nl_policy[NFSD_A_UNLOCK_ADDRESS + 1] = {
> +	[NFSD_A_UNLOCK_TYPE] = NLA_POLICY_MAX(NLA_U32, 0),
> +	[NFSD_A_UNLOCK_ADDRESS] = NLA_POLICY_MIN_LEN(16),
> +};
> +
>  /* Ops table for nfsd */
>  static const struct genl_split_ops nfsd_nl_ops[] = {
>  	{
> @@ -102,6 +108,13 @@ static const struct genl_split_ops nfsd_nl_ops[] = {
>  		.doit	= nfsd_nl_pool_mode_get_doit,
>  		.flags	= GENL_CMD_CAP_DO,
>  	},
> +	{
> +		.cmd		= NFSD_CMD_UNLOCK,
> +		.doit		= nfsd_nl_unlock_doit,
> +		.policy		= nfsd_unlock_nl_policy,
> +		.maxattr	= NFSD_A_UNLOCK_ADDRESS,
> +		.flags		= GENL_ADMIN_PERM | 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 478117ff6b8c..3ece774e5f52 100644
> --- a/fs/nfsd/netlink.h
> +++ b/fs/nfsd/netlink.h
> @@ -26,6 +26,7 @@ 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);
>  int nfsd_nl_pool_mode_set_doit(struct sk_buff *skb, struct genl_info *info);
>  int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info);
> +int nfsd_nl_unlock_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 4cc8a58fa56a..858f3803c490 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -236,7 +236,7 @@ static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
>  	if (rpc_pton(net, fo_path, size, sap, salen) == 0)
>  		return -EINVAL;
>  
> -	trace_nfsd_ctl_unlock_ip(net, buf);
> +	trace_nfsd_ctl_unlock_ip(net, sap, svc_addr_len(sap));
>  	return nlmsvc_unlock_all_by_ip(sap);
>  }
>  
> @@ -2142,6 +2142,67 @@ int nfsd_nl_pool_mode_get_doit(struct sk_buff *skb, struct genl_info *info)
>  	return err;
>  }
>  
> +/**
> + * nfsd_nl_unlock_by_ip - release NLM locks held by an IP address
> + * @info: netlink metadata and command arguments
> + *
> + * Return: 0 on success or a negative errno.
> + */
> +static int nfsd_nl_unlock_by_ip(struct genl_info *info)
> +{
> +	struct sockaddr *sap;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_UNLOCK_ADDRESS))
> +		return -EINVAL;
> +	sap = nla_data(info->attrs[NFSD_A_UNLOCK_ADDRESS]);
> +	switch (sap->sa_family) {
> +	case AF_INET:
> +		if (nla_len(info->attrs[NFSD_A_UNLOCK_ADDRESS]) <
> +		    sizeof(struct sockaddr_in))
> +			return -EINVAL;
> +		break;
> +	case AF_INET6:
> +		if (nla_len(info->attrs[NFSD_A_UNLOCK_ADDRESS]) <
> +		    sizeof(struct sockaddr_in6))
> +			return -EINVAL;
> +		break;
> +	default:
> +		return -EAFNOSUPPORT;
> +	}
> +	/*
> +	 * nlmsvc_unlock_all_by_ip() releases matching locks
> +	 * across all network namespaces because lockd operates
> +	 * a single global instance.
> +	 */
> +	trace_nfsd_ctl_unlock_ip(genl_info_net(info), sap,
> +				 svc_addr_len(sap));
> +	return nlmsvc_unlock_all_by_ip(sap);
> +}
> +
> +/**
> + * nfsd_nl_unlock_doit - release NLM locks by scope
> + * @skb: reply buffer
> + * @info: netlink metadata and command arguments
> + *
> + * Return: 0 on success or a negative errno.
> + */
> +int nfsd_nl_unlock_doit(struct sk_buff *skb, struct genl_info *info)
> +{
> +	u32 type;
> +
> +	if (GENL_REQ_ATTR_CHECK(info, NFSD_A_UNLOCK_TYPE))
> +		return -EINVAL;
> +
> +	type = nla_get_u32(info->attrs[NFSD_A_UNLOCK_TYPE]);
> +
> +	switch (type) {
> +	case NFSD_UNLOCK_TYPE_IP:
> +		return nfsd_nl_unlock_by_ip(info);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  /**
>   * nfsd_net_init - Prepare the nfsd_net portion of a new net namespace
>   * @net: a freshly-created network namespace
> diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> index d1d0b0dd0545..c770c5e6b1e7 100644
> --- a/fs/nfsd/trace.h
> +++ b/fs/nfsd/trace.h
> @@ -1984,19 +1984,20 @@ TRACE_EVENT(nfsd_cb_recall_any_done,
>  TRACE_EVENT(nfsd_ctl_unlock_ip,
>  	TP_PROTO(
>  		const struct net *net,
> -		const char *address
> +		const struct sockaddr *addr,
> +		const unsigned int addrlen
>  	),
> -	TP_ARGS(net, address),
> +	TP_ARGS(net, addr, addrlen),
>  	TP_STRUCT__entry(
>  		__field(unsigned int, netns_ino)
> -		__string(address, address)
> +		__sockaddr(addr, addrlen)
>  	),
>  	TP_fast_assign(
>  		__entry->netns_ino = net->ns.inum;
> -		__assign_str(address);
> +		__assign_sockaddr(addr, addr, addrlen);
>  	),
> -	TP_printk("address=%s",
> -		__get_str(address)
> +	TP_printk("addr=%pISpc",
> +		__get_sockaddr(addr)
>  	)
>  );
>  
> diff --git a/include/uapi/linux/nfsd_netlink.h b/include/uapi/linux/nfsd_netlink.h
> index e9efbc9e63d8..8edd75590f31 100644
> --- a/include/uapi/linux/nfsd_netlink.h
> +++ b/include/uapi/linux/nfsd_netlink.h
> @@ -10,6 +10,14 @@
>  #define NFSD_FAMILY_NAME	"nfsd"
>  #define NFSD_FAMILY_VERSION	1
>  
> +enum nfsd_unlock_type {
> +	NFSD_UNLOCK_TYPE_IP,
> +
> +	/* private: */
> +	__NFSD_UNLOCK_TYPE_MAX,
> +	NFSD_UNLOCK_TYPE_MAX = (__NFSD_UNLOCK_TYPE_MAX - 1)
> +};
> +
>  enum {
>  	NFSD_A_RPC_STATUS_XID = 1,
>  	NFSD_A_RPC_STATUS_FLAGS,
> @@ -80,6 +88,14 @@ enum {
>  	NFSD_A_POOL_MODE_MAX = (__NFSD_A_POOL_MODE_MAX - 1)
>  };
>  
> +enum {
> +	NFSD_A_UNLOCK_TYPE = 1,
> +	NFSD_A_UNLOCK_ADDRESS,
> +
> +	__NFSD_A_UNLOCK_MAX,
> +	NFSD_A_UNLOCK_MAX = (__NFSD_A_UNLOCK_MAX - 1)
> +};
> +
>  enum {
>  	NFSD_CMD_RPC_STATUS_GET = 1,
>  	NFSD_CMD_THREADS_SET,
> @@ -90,6 +106,7 @@ enum {
>  	NFSD_CMD_LISTENER_GET,
>  	NFSD_CMD_POOL_MODE_SET,
>  	NFSD_CMD_POOL_MODE_GET,
> +	NFSD_CMD_UNLOCK,
>  
>  	__NFSD_CMD_MAX,
>  	NFSD_CMD_MAX = (__NFSD_CMD_MAX - 1)

-- 
Jeff Layton <jlayton@kernel.org>