mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Trond Myklebust <trondmy@kernel.org>,
	Anna Schumaker <anna@kernel.org>,  Chuck Lever <cel@kernel.org>,
	NeilBrown <neil@brown.name>,
	 Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <Dai.Ngo@oracle.com>,  Tom Talpey <tom@talpey.com>,
	Shuah Khan <shuah@kernel.org>
Cc: Scott Mayhew <smayhew@redhat.com>,
	linux-nfs@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	 Jeff Layton <jlayton@kernel.org>
Subject: [PATCH 2/2] selftests/nfsd: add lockd netlink configuration tests
Date: Wed, 23 Sep 2026 06:58:57 -0400	[thread overview]
Message-ID: <20260923-nfsd-testing-v1-2-a793f54a4dd1@kernel.org> (raw)
In-Reply-To: <20260923-nfsd-testing-v1-0-a793f54a4dd1@kernel.org>

Cover LOCKD_CMD_SERVER_SET / SERVER_GET, in particular that each of the
three attributes can be set on its own. Without the preceding fix, four
of the eight tests fail with EINVAL -- every one that omits the grace
time, which is what nfsdctl sends for a [lockd] port with no
grace-time.

Also pinned down here:
- an empty SERVER_SET is a no-op, not an error
- a gracetime over nlm_grace_period_max is still rejected, and the
  rejected request applies none of the ports it came with
- the settings are per-netns

Everything runs under unshare(CLONE_NEWNET): the values are per-netns,
and a SERVER_SET in init_net would overwrite the host's module-wide
nlm_grace_period/nlm_tcpport/nlm_udpport.

nfsd_netlink.h grows genl_resolve() and *_to() request helpers that take
a family id, so the plumbing can drive the lockd family too;
genl_resolve_nfsd() and the existing helpers are thin wrappers and the
other tests are unchanged.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tools/testing/selftests/nfsd/.gitignore           |   1 +
 tools/testing/selftests/nfsd/Makefile             |   1 +
 tools/testing/selftests/nfsd/nfsd_lockd_netlink.c | 249 ++++++++++++++++++++++
 tools/testing/selftests/nfsd/nfsd_netlink.h       |  90 ++++++--
 4 files changed, 326 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/nfsd/.gitignore b/tools/testing/selftests/nfsd/.gitignore
index 2347491c634d..7ac844d0fd57 100644
--- a/tools/testing/selftests/nfsd/.gitignore
+++ b/tools/testing/selftests/nfsd/.gitignore
@@ -1,3 +1,4 @@
+nfsd_lockd_netlink
 nfsd_netlink_listener
 nfsd_netns_isolation
 nfsd_netns_stress
diff --git a/tools/testing/selftests/nfsd/Makefile b/tools/testing/selftests/nfsd/Makefile
index b29bf642c0ad..e74bb3424d45 100644
--- a/tools/testing/selftests/nfsd/Makefile
+++ b/tools/testing/selftests/nfsd/Makefile
@@ -2,6 +2,7 @@
 CFLAGS += $(KHDR_INCLUDES) -Wall
 
 TEST_GEN_PROGS := nfsd_netlink_listener
+TEST_GEN_PROGS += nfsd_lockd_netlink
 TEST_GEN_PROGS += nfsd_netns_isolation
 TEST_GEN_PROGS += nfsd_netns_stress
 
diff --git a/tools/testing/selftests/nfsd/nfsd_lockd_netlink.c b/tools/testing/selftests/nfsd/nfsd_lockd_netlink.c
new file mode 100644
index 000000000000..89f68c5708ad
--- /dev/null
+++ b/tools/testing/selftests/nfsd/nfsd_lockd_netlink.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Regression tests for lockd's generic-netlink configuration interface
+ * (LOCKD_CMD_SERVER_SET / LOCKD_CMD_SERVER_GET).
+ *
+ * All three attributes are optional in the spec and each is applied on its
+ * own by the kernel, but SERVER_SET used to demand a grace time and reject
+ * anything else with -EINVAL. That made the tcp and udp ports unsettable by
+ * themselves, which is exactly what nfsdctl asks for when /etc/nfs.conf has
+ * a [lockd] port but no grace-time -- "nfsdctl autostart" then failed before
+ * it had configured anything.
+ *
+ * Every test runs in a private network namespace. The settings are per-netns,
+ * and a SERVER_SET in init_net would also overwrite the host's module-wide
+ * nlm_grace_period/nlm_tcpport/nlm_udpport.
+ */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <sched.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <linux/lockd_netlink.h>
+
+#include "../kselftest_harness.h"
+#include "nfsd_netlink.h"
+
+/* fs/lockd/svc.c: nlm_grace_period_max */
+#define GRACE_MAX		240
+
+#define TEST_TCP_PORT		32531
+#define TEST_UDP_PORT		32532
+
+struct lockd_cfg {
+	uint32_t gracetime;
+	uint16_t tcp_port;
+	uint16_t udp_port;
+};
+
+static int lockd_family = -1;
+
+static int lockd_set(const char *attrs, int len)
+{
+	return genl_request_to(lockd_family, LOCKD_CMD_SERVER_SET, attrs, len);
+}
+
+static int lockd_get(struct lockd_cfg *cfg)
+{
+	const struct nlattr *grace, *tcp, *udp;
+	char rbuf[4096];
+	int n;
+
+	n = genl_request_reply_attrs_to(lockd_family, LOCKD_CMD_SERVER_GET,
+					NULL, 0, rbuf, sizeof(rbuf));
+	if (n < 0)
+		return n;
+
+	grace = genl_find_attr(rbuf, n, LOCKD_A_SERVER_GRACETIME);
+	tcp = genl_find_attr(rbuf, n, LOCKD_A_SERVER_TCP_PORT);
+	udp = genl_find_attr(rbuf, n, LOCKD_A_SERVER_UDP_PORT);
+	if (!grace || !tcp || !udp)
+		return -ENOENT;
+
+	cfg->gracetime = nla_u32(grace);
+	cfg->tcp_port = nla_u16(tcp);
+	cfg->udp_port = nla_u16(udp);
+	return 0;
+}
+
+/* ------------------- SERVER_SET request builders ------------------- */
+
+static int put_gracetime(char *buf, int off, uint32_t grace)
+{
+	return put_attr(buf, off, LOCKD_A_SERVER_GRACETIME, &grace,
+			sizeof(grace));
+}
+
+static int put_ports(char *buf, int off, uint16_t tcp, uint16_t udp)
+{
+	off = put_attr(buf, off, LOCKD_A_SERVER_TCP_PORT, &tcp, sizeof(tcp));
+	return put_attr(buf, off, LOCKD_A_SERVER_UDP_PORT, &udp, sizeof(udp));
+}
+
+FIXTURE(lockd_netlink) {
+};
+
+FIXTURE_SETUP(lockd_netlink)
+{
+	if (geteuid() != 0)
+		SKIP(return, "must be run as root");
+	if (unshare(CLONE_NEWNET) < 0)
+		SKIP(return, "unshare(NEWNET): %s", strerror(errno));
+
+	lockd_family = genl_resolve(LOCKD_FAMILY_NAME, sizeof(LOCKD_FAMILY_NAME));
+	if (lockd_family < 0)
+		SKIP(return, "lockd netlink family not registered");
+}
+
+FIXTURE_TEARDOWN(lockd_netlink)
+{
+	/* Nothing to undo: the settings die with the namespace. */
+}
+
+/* A fresh namespace starts out with everything at zero. */
+TEST_F(lockd_netlink, defaults_are_zero)
+{
+	struct lockd_cfg cfg;
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(0, cfg.gracetime);
+	EXPECT_EQ(0, cfg.tcp_port);
+	EXPECT_EQ(0, cfg.udp_port);
+}
+
+/*
+ * The regression: ports on their own, no grace time. This is the request
+ * nfsdctl builds from a [lockd] section that only sets the ports.
+ */
+TEST_F(lockd_netlink, set_ports_without_gracetime)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	int off;
+
+	off = put_ports(attrs, 0, TEST_TCP_PORT, TEST_UDP_PORT);
+
+	ASSERT_EQ(0, lockd_set(attrs, off))
+		TH_LOG("SERVER_SET rejected a request with no gracetime");
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(TEST_TCP_PORT, cfg.tcp_port);
+	EXPECT_EQ(TEST_UDP_PORT, cfg.udp_port);
+	EXPECT_EQ(0, cfg.gracetime);
+}
+
+/* The mirror image: a grace time with no ports. */
+TEST_F(lockd_netlink, set_gracetime_without_ports)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	int off;
+
+	off = put_gracetime(attrs, 0, 90);
+
+	ASSERT_EQ(0, lockd_set(attrs, off));
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(90, cfg.gracetime);
+	EXPECT_EQ(0, cfg.tcp_port);
+	EXPECT_EQ(0, cfg.udp_port);
+}
+
+/* One attribute at a time leaves the others alone. */
+TEST_F(lockd_netlink, attributes_are_set_independently)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	uint16_t tcp = TEST_TCP_PORT;
+	int off;
+
+	off = put_attr(attrs, 0, LOCKD_A_SERVER_TCP_PORT, &tcp, sizeof(tcp));
+	ASSERT_EQ(0, lockd_set(attrs, off));
+
+	off = put_gracetime(attrs, 0, 30);
+	ASSERT_EQ(0, lockd_set(attrs, off));
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(30, cfg.gracetime);
+	EXPECT_EQ(TEST_TCP_PORT, cfg.tcp_port)
+		TH_LOG("a gracetime-only SET clobbered the tcp port");
+	EXPECT_EQ(0, cfg.udp_port);
+}
+
+TEST_F(lockd_netlink, set_all_three)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	int off;
+
+	off = put_gracetime(attrs, 0, GRACE_MAX);
+	off = put_ports(attrs, off, TEST_TCP_PORT, TEST_UDP_PORT);
+
+	ASSERT_EQ(0, lockd_set(attrs, off));
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(GRACE_MAX, cfg.gracetime);
+	EXPECT_EQ(TEST_TCP_PORT, cfg.tcp_port);
+	EXPECT_EQ(TEST_UDP_PORT, cfg.udp_port);
+}
+
+/* An empty SERVER_SET has nothing to do, but is not an error. */
+TEST_F(lockd_netlink, empty_set_is_a_noop)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	int off;
+
+	off = put_ports(attrs, 0, TEST_TCP_PORT, TEST_UDP_PORT);
+	ASSERT_EQ(0, lockd_set(attrs, off));
+
+	ASSERT_EQ(0, lockd_set(NULL, 0));
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(TEST_TCP_PORT, cfg.tcp_port);
+	EXPECT_EQ(TEST_UDP_PORT, cfg.udp_port);
+}
+
+/*
+ * The grace time is still range-checked, and the check runs before anything
+ * is stored: a rejected request must not apply the ports it came with.
+ */
+TEST_F(lockd_netlink, gracetime_above_max_rejected)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	int off;
+
+	off = put_gracetime(attrs, 0, GRACE_MAX + 1);
+	off = put_ports(attrs, off, TEST_TCP_PORT, TEST_UDP_PORT);
+
+	EXPECT_EQ(-EINVAL, lockd_set(attrs, off));
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(0, cfg.gracetime);
+	EXPECT_EQ(0, cfg.tcp_port)
+		TH_LOG("a rejected SERVER_SET applied the tcp port anyway");
+	EXPECT_EQ(0, cfg.udp_port);
+}
+
+/* Settings belong to the namespace that made them. */
+TEST_F(lockd_netlink, settings_are_per_netns)
+{
+	struct lockd_cfg cfg;
+	char attrs[64];
+	int off;
+
+	off = put_ports(attrs, 0, TEST_TCP_PORT, TEST_UDP_PORT);
+	ASSERT_EQ(0, lockd_set(attrs, off));
+
+	if (unshare(CLONE_NEWNET) < 0)
+		SKIP(return, "second unshare(NEWNET): %s", strerror(errno));
+
+	ASSERT_EQ(0, lockd_get(&cfg));
+	EXPECT_EQ(0, cfg.tcp_port)
+		TH_LOG("tcp port leaked out of the namespace that set it");
+	EXPECT_EQ(0, cfg.udp_port);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/nfsd/nfsd_netlink.h b/tools/testing/selftests/nfsd/nfsd_netlink.h
index c66a980bfc01..bb9327109e8f 100644
--- a/tools/testing/selftests/nfsd/nfsd_netlink.h
+++ b/tools/testing/selftests/nfsd/nfsd_netlink.h
@@ -4,7 +4,9 @@
  *
  * Header-only: every helper is static inline, so each test binary gets its
  * own copy and there is nothing extra to link. nfsd_family must be set by
- * calling genl_resolve_nfsd() before any of the request helpers are used.
+ * calling genl_resolve_nfsd() before any of the request helpers are used;
+ * the *_to() variants take a family id instead, for the other families in
+ * the NFS server stack.
  */
 #ifndef __SELFTESTS_NFSD_NETLINK_H__
 #define __SELFTESTS_NFSD_NETLINK_H__
@@ -100,6 +102,45 @@ static inline int put_attr(char *buf, int off, uint16_t type,
 	return off + NLA_ALIGN4(NLA_HDRLEN + len);
 }
 
+/* Payload accessors; the payload is only 4-byte aligned, so no direct load. */
+static inline uint32_t nla_u32(const struct nlattr *na)
+{
+	uint32_t v;
+
+	memcpy(&v, (const char *)na + NLA_HDRLEN, sizeof(v));
+	return v;
+}
+
+static inline uint16_t nla_u16(const struct nlattr *na)
+{
+	uint16_t v;
+
+	memcpy(&v, (const char *)na + NLA_HDRLEN, sizeof(v));
+	return v;
+}
+
+/* Find top-level attribute @type in a genl reply of @len bytes; NULL if absent. */
+static inline const struct nlattr *genl_find_attr(const char *rbuf, int len,
+						  uint16_t type)
+{
+	const struct nlmsghdr *nlh = (const void *)rbuf;
+	const struct nlattr *na;
+	int left;
+
+	if (len < (int)(NLMSG_HDRLEN + GENL_HDRLEN))
+		return NULL;
+	na = (const void *)(rbuf + NLMSG_HDRLEN + GENL_HDRLEN);
+	left = nlh->nlmsg_len - NLMSG_HDRLEN - GENL_HDRLEN;
+
+	while (left >= (int)NLA_HDRLEN) {
+		if ((na->nla_type & NLA_TYPE_MASK) == type)
+			return na;
+		left -= NLA_ALIGN4(na->nla_len);
+		na = (const void *)((const char *)na + NLA_ALIGN4(na->nla_len));
+	}
+	return NULL;
+}
+
 /* Build a genl message header into @buf; return the offset past it. */
 static inline int genl_hdr(char *buf, uint16_t type, uint16_t flags, uint8_t cmd)
 {
@@ -115,15 +156,16 @@ static inline int genl_hdr(char *buf, uint16_t type, uint16_t flags, uint8_t cmd
 	return NLMSG_HDRLEN + GENL_HDRLEN;
 }
 
-/* Send an nfsd command with an ACK; return the ACK errno (<= 0). */
-static inline int genl_request(uint8_t cmd, const char *attrs, int attrs_len)
+/* Send a command to @family with an ACK; return the ACK errno (<= 0). */
+static inline int genl_request_to(uint16_t family, uint8_t cmd,
+				  const char *attrs, int attrs_len)
 {
 	char buf[1 << 20], rbuf[4096];
 	struct nlmsghdr *nlh = (void *)buf;
 	int fd = genl_open();
 	int off, n, ret;
 
-	off = genl_hdr(buf, nfsd_family, NLM_F_REQUEST | NLM_F_ACK, cmd);
+	off = genl_hdr(buf, family, NLM_F_REQUEST | NLM_F_ACK, cmd);
 	if (attrs_len) {
 		memcpy(buf + off, attrs, attrs_len);
 		off += attrs_len;
@@ -147,21 +189,27 @@ static inline int genl_request(uint8_t cmd, const char *attrs, int attrs_len)
 	return ret;
 }
 
+static inline int genl_request(uint8_t cmd, const char *attrs, int attrs_len)
+{
+	return genl_request_to(nfsd_family, cmd, attrs, attrs_len);
+}
+
 /*
- * Send a command with attributes and return the full reply message; -errno
- * on failure. NLM_F_ACK is left off: the kernel reports an error either way,
- * so the first message back is the reply whenever there is one.
+ * Send a command to @family with attributes and return the full reply
+ * message; -errno on failure. NLM_F_ACK is left off: the kernel reports an
+ * error either way, so the first message back is the reply whenever there
+ * is one.
  */
-static inline int genl_request_reply_attrs(uint8_t cmd, const char *attrs,
-					   int attrs_len, char *rbuf,
-					   size_t rlen)
+static inline int genl_request_reply_attrs_to(uint16_t family, uint8_t cmd,
+					      const char *attrs, int attrs_len,
+					      char *rbuf, size_t rlen)
 {
 	char buf[1 << 20];
 	struct nlmsghdr *nlh = (void *)buf;
 	int fd = genl_open();
 	int off, n, ret;
 
-	off = genl_hdr(buf, nfsd_family, NLM_F_REQUEST, cmd);
+	off = genl_hdr(buf, family, NLM_F_REQUEST, cmd);
 	if (attrs_len) {
 		memcpy(buf + off, attrs, attrs_len);
 		off += attrs_len;
@@ -182,13 +230,21 @@ static inline int genl_request_reply_attrs(uint8_t cmd, const char *attrs,
 	return ret;
 }
 
+static inline int genl_request_reply_attrs(uint8_t cmd, const char *attrs,
+					   int attrs_len, char *rbuf,
+					   size_t rlen)
+{
+	return genl_request_reply_attrs_to(nfsd_family, cmd, attrs, attrs_len,
+					   rbuf, rlen);
+}
+
 static inline int genl_request_reply(uint8_t cmd, char *rbuf, size_t rlen)
 {
 	return genl_request_reply_attrs(cmd, NULL, 0, rbuf, rlen);
 }
 
-/* Resolve the "nfsd" genl family id; -1 if not registered. */
-static inline int genl_resolve_nfsd(void)
+/* Resolve a genl family id by name; -1 if not registered. */
+static inline int genl_resolve(const char *name, size_t namelen)
 {
 	char buf[1024], rbuf[4096];
 	struct nlmsghdr *nlh = (void *)buf;
@@ -198,8 +254,7 @@ static inline int genl_resolve_nfsd(void)
 
 	fd = genl_open();
 	off = genl_hdr(buf, GENL_ID_CTRL, NLM_F_REQUEST, CTRL_CMD_GETFAMILY);
-	off = put_attr(buf, off, CTRL_ATTR_FAMILY_NAME,
-		       NFSD_FAMILY_NAME, sizeof(NFSD_FAMILY_NAME));
+	off = put_attr(buf, off, CTRL_ATTR_FAMILY_NAME, name, namelen);
 	nlh->nlmsg_len = off;
 
 	if (send(fd, buf, off, 0) < 0)
@@ -224,6 +279,11 @@ static inline int genl_resolve_nfsd(void)
 	return id;
 }
 
+static inline int genl_resolve_nfsd(void)
+{
+	return genl_resolve(NFSD_FAMILY_NAME, sizeof(NFSD_FAMILY_NAME));
+}
+
 /* ------------------- listener request builders ------------------- */
 
 /* Fine-grained control for negative tests: any field can be omitted/malformed. */

-- 
2.55.0


      parent reply	other threads:[~2026-09-23 10:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-23 10:58 [PATCH 0/2] lockd: allow setting ports without setting gracetime Jeff Layton
2026-09-23 10:58 ` [PATCH 1/2] lockd: allow SERVER_SET without a gracetime attribute Jeff Layton
2026-09-23 10:58 ` Jeff Layton [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260923-nfsd-testing-v1-2-a793f54a4dd1@kernel.org \
    --to=jlayton@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=anna@kernel.org \
    --cc=cel@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=shuah@kernel.org \
    --cc=smayhew@redhat.com \
    --cc=tom@talpey.com \
    --cc=trondmy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®