Re: [PATCH v2 3/3] nfsdctl: add necessary bits to configure lockd

Tom Talpey <tom@xxxxxxxxxx> · Fri, 10 Jan 2025 10:40:28 -0500

On 1/10/2025 10:21 AM, Jeff Layton wrote:
On Fri, 2025-01-10 at 10:05 -0500, Tom Talpey wrote:
On 1/10/2025 8:46 AM, Jeff Layton wrote:
The legacy rpc.nfsd program would configure the nlm_grace_period to
match the NFSv4 grace period when starting the server. This adds similar
functionality to the autostart subcommand using the new lockd netlink
configuration interfaces. In addition, if lockd's udp or tcp listener
ports are specified, also configure them before starting the server.

A "nlm" subcommand is also added that will display the current lockd
config settings in the current net ns. In the future, we could add the
ability to set these values using the "nlm" subcommand, but for now I
didn't see a real use-case for that, so I left it out.

It seems unnatural that the nlm_grace_period is tied to the netns.

It seems to me it's more dependent on the network and its likely
failure modes, the backend storage/filesystem, and perhaps the
scale of clients performing possibly-conflicting locks. Oh, and
also perhaps the minor version, since 4.1+ have the RECLAIM_COMPLETE
termination event.

Food for thought, perhaps.

Tom.


Fair point. More food:

My guess is that nlm_grace_period handling is likely horribly broken in
multi-container scenarios anyway. If you have multiple nfs server
containers with different grace period settings, then they will all be
competing to set the global nlm_grace_period.

Most clients end up reclaiming quickly enough that we don't hit major
problems here, but it would be good to make this more "airtight".

The grace period can has been kicked down the road since nineteen
ninety ump so, sure, what the heck :)

I wonder how many sysadmins are actually changing it, and why. It's
not truly a correctness thing, more of an availability setting, so
the choice of value is pretty soft. As you say, if the clients all
reclaim promptly, nobody's the wiser. Do we have any data on how
often the setting actually gets used?

Tom.



Link: https://issues.redhat.com/browse/RHEL-71698
Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
---
   configure.ac                  |   4 +
   utils/nfsdctl/lockd_netlink.h |  29 ++++++
   utils/nfsdctl/nfsdctl.8       |   6 ++
   utils/nfsdctl/nfsdctl.adoc    |   5 +
   utils/nfsdctl/nfsdctl.c       | 218 +++++++++++++++++++++++++++++++++++++++---
   5 files changed, 249 insertions(+), 13 deletions(-)

diff --git a/configure.ac b/configure.ac
index 561e894dc46f48997df4ba6dc3e7691876589fdb..1d865fbc1c7f79e3ac6152bc59995e99fe10a38e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -268,6 +268,10 @@ AC_ARG_ENABLE(nfsdctl,
   				                   [[int foo = NFSD_CMD_POOL_MODE_GET;]])],
   				   [AC_DEFINE([USE_SYSTEM_NFSD_NETLINK_H], 1,
   					      ["Use system's linux/nfsd_netlink.h"])])
+		AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/lockd_netlink.h>]],
+				                   [[int foo = LOCKD_CMD_SERVER_GET;]])],
+				   [AC_DEFINE([USE_SYSTEM_LOCKD_NETLINK_H], 1,
+					      ["Use system's linux/lockd_netlink.h"])])
   	fi
   
   AC_ARG_ENABLE(nfsv4server,
diff --git a/utils/nfsdctl/lockd_netlink.h b/utils/nfsdctl/lockd_netlink.h
new file mode 100644
index 0000000000000000000000000000000000000000..21c65aec3bc6d1839961937081e6d16540332179
--- /dev/null
+++ b/utils/nfsdctl/lockd_netlink.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/* Do not edit directly, auto-generated from: */
+/*	Documentation/netlink/specs/lockd.yaml */
+/* YNL-GEN uapi header */
+
+#ifndef _UAPI_LINUX_LOCKD_NETLINK_H
+#define _UAPI_LINUX_LOCKD_NETLINK_H
+
+#define LOCKD_FAMILY_NAME	"lockd"
+#define LOCKD_FAMILY_VERSION	1
+
+enum {
+	LOCKD_A_SERVER_GRACETIME = 1,
+	LOCKD_A_SERVER_TCP_PORT,
+	LOCKD_A_SERVER_UDP_PORT,
+
+	__LOCKD_A_SERVER_MAX,
+	LOCKD_A_SERVER_MAX = (__LOCKD_A_SERVER_MAX - 1)
+};
+
+enum {
+	LOCKD_CMD_SERVER_SET = 1,
+	LOCKD_CMD_SERVER_GET,
+
+	__LOCKD_CMD_MAX,
+	LOCKD_CMD_MAX = (__LOCKD_CMD_MAX - 1)
+};
+
+#endif /* _UAPI_LINUX_LOCKD_NETLINK_H */
diff --git a/utils/nfsdctl/nfsdctl.8 b/utils/nfsdctl/nfsdctl.8
index 39ae1855ae50e94da113981d5e8cf8ac14440c3a..d69922448eb17fb155f05dc4ddc9aefffbf966e4 100644
--- a/utils/nfsdctl/nfsdctl.8
+++ b/utils/nfsdctl/nfsdctl.8
@@ -127,6 +127,12 @@ colon separated form, and must be enclosed in square brackets.
   .if n .RE
   .RE
   .sp
+\fBnlm\fP
+.RS 4
+Get information about NLM (lockd) settings in the current net namespace. This
+subcommand takes no arguments.
+.RE
+.sp
   \fBstatus\fP
   .RS 4
   Get information about RPCs currently executing in the server. This subcommand
diff --git a/utils/nfsdctl/nfsdctl.adoc b/utils/nfsdctl/nfsdctl.adoc
index 2114693042594297b7c5d8600ca16813a0f2356c..0207eff6118d2dcc5a794d2013c039d9beb11ddc 100644
--- a/utils/nfsdctl/nfsdctl.adoc
+++ b/utils/nfsdctl/nfsdctl.adoc
@@ -67,6 +67,11 @@ Each subcommand can also accept its own set of options and arguments. The
     addresses must be in dotted-quad form. IPv6 addresses should be in standard
     colon separated form, and must be enclosed in square brackets.
   
+*nlm*::
+
+  Get information about NLM (lockd) settings in the current net namespace. This
+  subcommand takes no arguments.
+
   *status*::
   
     Get information about RPCs currently executing in the server. This subcommand
diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 5c319a74273fd2bbe84003c1261043c4b2f1ff29..003daba5f30a403eb4168f6103e5a496d96968a4 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -35,6 +35,12 @@
   #include "nfsd_netlink.h"
   #endif
   
+#ifdef USE_SYSTEM_LOCKD_NETLINK_H
+#include <linux/lockd_netlink.h>
+#else
+#include "lockd_netlink.h"
+#endif
+
   #include "nfsdctl.h"
   #include "conffile.h"
   #include "xlog.h"
@@ -348,6 +354,28 @@ static void parse_pool_mode_get(struct genlmsghdr *gnlh)
   	}
   }
   
+static void parse_lockd_get(struct genlmsghdr *gnlh)
+{
+	struct nlattr *attr;
+	int rem;
+
+	nla_for_each_attr(attr, genlmsg_attrdata(gnlh, 0),
+			  genlmsg_attrlen(gnlh, 0), rem) {
+		switch (nla_type(attr)) {
+		case LOCKD_A_SERVER_GRACETIME:
+			printf("gracetime: %u\n", nla_get_u32(attr));
+			break;
+		case LOCKD_A_SERVER_TCP_PORT:
+			printf("tcp_port: %hu\n", nla_get_u16(attr));
+			break;
+		case LOCKD_A_SERVER_UDP_PORT:
+			printf("udp_port: %hu\n", nla_get_u16(attr));
+			break;
+		default:
+			break;
+		}
+	}
+}
   static int recv_handler(struct nl_msg *msg, void *arg)
   {
   	struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
@@ -368,6 +396,9 @@ static int recv_handler(struct nl_msg *msg, void *arg)
   	case NFSD_CMD_POOL_MODE_GET:
   		parse_pool_mode_get(gnlh);
   		break;
+	case LOCKD_CMD_SERVER_GET:
+		parse_lockd_get(gnlh);
+		break;
   	default:
   		break;
   	}
@@ -398,12 +429,12 @@ static struct nl_sock *netlink_sock_alloc(void)
   	return sock;
   }
   
-static struct nl_msg *netlink_msg_alloc(struct nl_sock *sock)
+static struct nl_msg *netlink_msg_alloc(struct nl_sock *sock, const char *family)
   {
   	struct nl_msg *msg;
   	int id;
   
-	id = genl_ctrl_resolve(sock, NFSD_FAMILY_NAME);
+	id = genl_ctrl_resolve(sock, family);
   	if (id < 0) {
   		xlog(L_ERROR, "%s not found", NFSD_FAMILY_NAME);
   		return NULL;
@@ -447,7 +478,7 @@ static int status_func(struct nl_sock *sock, int argc, char ** argv)
   		}
   	}
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -495,7 +526,7 @@ static int threads_doit(struct nl_sock *sock, int cmd, int grace, int lease,
   	struct nl_cb *cb;
   	int ret;
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -607,7 +638,7 @@ static int fetch_nfsd_versions(struct nl_sock *sock)
   	struct nl_cb *cb;
   	int ret;
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -672,7 +703,7 @@ static int set_nfsd_versions(struct nl_sock *sock)
   	struct nl_cb *cb;
   	int i, ret;
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -825,7 +856,7 @@ static int fetch_current_listeners(struct nl_sock *sock)
   	struct nl_cb *cb;
   	int ret;
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -1054,7 +1085,7 @@ static int set_listeners(struct nl_sock *sock)
   	struct nl_cb *cb;
   	int i, ret;
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -1170,7 +1201,7 @@ static int pool_mode_doit(struct nl_sock *sock, int cmd, const char *pool_mode)
   	struct nl_cb *cb;
   	int ret;
   
-	msg = netlink_msg_alloc(sock);
+	msg = netlink_msg_alloc(sock, NFSD_FAMILY_NAME);
   	if (!msg)
   		return 1;
   
@@ -1249,6 +1280,131 @@ static int pool_mode_func(struct nl_sock *sock, int argc, char **argv)
   	return pool_mode_doit(sock, cmd, pool_mode);
   }
   
+static int lockd_config_doit(struct nl_sock *sock, int cmd, int grace, int tcpport, int udpport)
+{
+	struct genlmsghdr *ghdr;
+	struct nlmsghdr *nlh;
+	struct nl_msg *msg;
+	struct nl_cb *cb;
+	int ret;
+
+	if (cmd == LOCKD_CMD_SERVER_SET) {
+		/* Do nothing if there is nothing to set */
+		if (!grace && !tcpport && !udpport)
+			return 0;
+	}
+
+	msg = netlink_msg_alloc(sock, LOCKD_FAMILY_NAME);
+	if (!msg)
+		return 1;
+
+	nlh = nlmsg_hdr(msg);
+	if (cmd == LOCKD_CMD_SERVER_SET) {
+		if (grace)
+			nla_put_u32(msg, LOCKD_A_SERVER_GRACETIME, grace);
+		if (tcpport)
+			nla_put_u16(msg, LOCKD_A_SERVER_TCP_PORT, tcpport);
+		if (udpport)
+			nla_put_u16(msg, LOCKD_A_SERVER_UDP_PORT, udpport);
+	}
+
+	ghdr = nlmsg_data(nlh);
+	ghdr->cmd = cmd;
+
+	cb = nl_cb_alloc(NL_CB_CUSTOM);
+	if (!cb) {
+		xlog(L_ERROR, "failed to allocate netlink callbacks\n");
+		ret = 1;
+		goto out;
+	}
+
+	ret = nl_send_auto(sock, msg);
+	if (ret < 0) {
+		xlog(L_ERROR, "send failed (%d)!\n", ret);
+		goto out_cb;
+	}
+
+	ret = 1;
+	nl_cb_err(cb, NL_CB_CUSTOM, error_handler, &ret);
+	nl_cb_set(cb, NL_CB_FINISH, NL_CB_CUSTOM, finish_handler, &ret);
+	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, ack_handler, &ret);
+	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, recv_handler, NULL);
+
+	while (ret > 0)
+		nl_recvmsgs(sock, cb);
+	if (ret < 0) {
+		xlog(L_ERROR, "Error: %s\n", strerror(-ret));
+		ret = 1;
+	}
+out_cb:
+	nl_cb_put(cb);
+out:
+	nlmsg_free(msg);
+	return ret;
+}
+
+static int get_service(const char *svc)
+{
+	struct addrinfo *res, hints = { .ai_flags = AI_PASSIVE };
+	int ret, port;
+
+	if (!svc)
+		return 0;
+
+	ret = getaddrinfo(NULL, svc, &hints, &res);
+	if (ret) {
+		xlog(L_ERROR, "getaddrinfo of \"%s\" failed: %s\n",
+			svc, gai_strerror(ret));
+		return -1;
+	}
+
+	switch (res->ai_family) {
+	case AF_INET:
+		{
+			struct sockaddr_in *sin = (struct sockaddr_in *)res->ai_addr;
+
+			port = ntohs(sin->sin_port);
+		}
+		break;
+	case AF_INET6:
+		{
+			struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)res->ai_addr;
+
+			port = ntohs(sin6->sin6_port);
+		}
+		break;
+	default:
+		xlog(L_ERROR, "Bad address family: %d\n", res->ai_family);
+		port = -1;
+	}
+	freeaddrinfo(res);
+	return port;
+}
+
+static int lockd_configure(struct nl_sock *sock, int grace)
+{
+	char *tcp_svc, *udp_svc;
+	int tcpport = 0, udpport = 0;
+	int ret;
+
+	tcp_svc = conf_get_str("lockd", "port");
+	if (tcp_svc) {
+		tcpport = get_service(tcp_svc);
+		if (tcpport < 0)
+			return 1;
+	}
+
+	udp_svc = conf_get_str("lockd", "udp-port");
+	if (udp_svc) {
+		udpport = get_service(udp_svc);
+		if (udpport < 0)
+			return 1;
+	}
+
+	return lockd_config_doit(sock, LOCKD_CMD_SERVER_SET, grace, tcpport, udpport);
+}
+
+
   #define MAX_LISTENER_LEN (64 * 2 + 16)
   
   static void
@@ -1355,6 +1511,13 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
   
   	read_nfsd_conf();
   
+	grace = conf_get_num("nfsd", "grace-time", 0);
+	ret = lockd_configure(sock, grace);
+	if (ret) {
+		xlog(L_ERROR, "lockd configuration failure");
+		return ret;
+	}
+
   	pool_mode = conf_get_str("nfsd", "pool-mode");
   	if (pool_mode) {
   		ret = pool_mode_doit(sock, NFSD_CMD_POOL_MODE_SET, pool_mode);
@@ -1370,15 +1533,12 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
   	if (ret)
   		return ret;
   
+	xlog(D_GENERAL, "configuring listeners");
   	configure_listeners();
   	ret = set_listeners(sock);
   	if (ret)
   		return ret;
   
-	grace = conf_get_num("nfsd", "grace-time", 0);
-	lease = conf_get_num("nfsd", "lease-time", 0);
-	scope = conf_get_str("nfsd", "scope");
-
   	thread_str = conf_get_list("nfsd", "threads");
   	pools = thread_str ? thread_str->cnt : 1;
   
@@ -1402,6 +1562,9 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
   		threads[0] = DEFAULT_AUTOSTART_THREADS;
   	}
   
+	lease = conf_get_num("nfsd", "lease-time", 0);
+	scope = conf_get_str("nfsd", "scope");
+
   	ret = threads_doit(sock, NFSD_CMD_THREADS_SET, grace, lease, pools,
   			   threads, scope);
   out:
@@ -1409,6 +1572,30 @@ out:
   	return ret;
   }
   
+static void nlm_usage(void)
+{
+	printf("Usage: %s nlm\n", taskname);
+	printf("    Get the current settings for the NLM (lockd) server.\n");
+}
+
+static int nlm_func(struct nl_sock *sock, int argc, char ** argv)
+{
+	int *threads, grace, lease, idx, ret, opt, pools;
+	struct conf_list *thread_str;
+	struct conf_list_node *n;
+	char *scope, *pool_mode;
+
+	optind = 1;
+	while ((opt = getopt_long(argc, argv, "h", help_only_options, NULL)) != -1) {
+		switch (opt) {
+		case 'h':
+			nlm_usage();
+			return 0;
+		}
+	}
+	return lockd_config_doit(sock, LOCKD_CMD_SERVER_GET, 0, 0, 0);
+}
+
   enum nfsdctl_commands {
   	NFSDCTL_STATUS,
   	NFSDCTL_THREADS,
@@ -1416,6 +1603,7 @@ enum nfsdctl_commands {
   	NFSDCTL_LISTENER,
   	NFSDCTL_AUTOSTART,
   	NFSDCTL_POOL_MODE,
+	NFSDCTL_NLM,
   };
   
   static int parse_command(char *str)
@@ -1432,6 +1620,8 @@ static int parse_command(char *str)
   		return NFSDCTL_AUTOSTART;
   	if (!strcmp(str, "pool-mode"))
   		return NFSDCTL_POOL_MODE;
+	if (!strcmp(str, "nlm"))
+		return NFSDCTL_NLM;
   	return -1;
   }
   
@@ -1444,6 +1634,7 @@ static nfsdctl_func func[] = {
   	[NFSDCTL_LISTENER] = listener_func,
   	[NFSDCTL_AUTOSTART] = autostart_func,
   	[NFSDCTL_POOL_MODE] = pool_mode_func,
+	[NFSDCTL_NLM] = nlm_func,
   };
   
   static void usage(void)
@@ -1460,6 +1651,7 @@ static void usage(void)
   	printf("    listener             get/set listener info\n");
   	printf("    version              get/set supported NFS versions\n");
   	printf("    threads              get/set nfsd thread settings\n");
+	printf("    nlm                  get current nlm settings\n");
   	printf("    status               get current RPC processing info\n");
   	printf("    autostart            start server with settings from /etc/nfs.conf\n");
   }











