* [PATCH v2 2/2] selftests/nfsd: add lockd netlink configuration tests
2026-09-23 15:30 [PATCH v2 0/2] lockd: allow setting ports without setting gracetime Jeff Layton
2026-09-23 15:30 ` [PATCH v2 1/2] lockd: allow SERVER_SET without a gracetime attribute Jeff Layton
@ 2026-09-23 15:30 ` Jeff Layton
2026-09-23 18:25 ` [PATCH v2 0/2] lockd: allow setting ports without setting gracetime Chuck Lever
2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2026-09-23 15:30 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, Chuck Lever, NeilBrown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Shuah Khan
Cc: Scott Mayhew, linux-nfs, linux-kernel, linux-kselftest, Jeff Layton
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 | 268 ++++++++++++++++++++++
tools/testing/selftests/nfsd/nfsd_netlink.h | 90 ++++++--
4 files changed, 345 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..47a1dee5a1f5
--- /dev/null
+++ b/tools/testing/selftests/nfsd/nfsd_lockd_netlink.c
@@ -0,0 +1,268 @@
+// 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_tcp_port(char *buf, int off, uint16_t tcp)
+{
+ return put_attr(buf, off, LOCKD_A_SERVER_TCP_PORT, &tcp, sizeof(tcp));
+}
+
+static int put_udp_port(char *buf, int off, uint16_t udp)
+{
+ return put_attr(buf, off, LOCKD_A_SERVER_UDP_PORT, &udp, sizeof(udp));
+}
+
+static int put_ports(char *buf, int off, uint16_t tcp, uint16_t udp)
+{
+ off = put_tcp_port(buf, off, tcp);
+ return put_udp_port(buf, off, 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];
+ int off;
+
+ off = put_tcp_port(attrs, 0, TEST_TCP_PORT);
+ ASSERT_EQ(0, lockd_set(attrs, off));
+
+ off = put_udp_port(attrs, 0, TEST_UDP_PORT);
+ ASSERT_EQ(0, lockd_set(attrs, off))
+ TH_LOG("SERVER_SET rejected a UDP-only request");
+
+ 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 later SET clobbered the tcp port");
+ EXPECT_EQ(TEST_UDP_PORT, cfg.udp_port)
+ TH_LOG("a gracetime-only SET clobbered the 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_gracetime(attrs, 0, 30);
+ off = put_ports(attrs, off, 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(30, cfg.gracetime);
+ 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_gracetime(attrs, 0, 30);
+ off = put_ports(attrs, off, 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.gracetime)
+ TH_LOG("grace time leaked out of the namespace that set it");
+ 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
^ permalink raw reply [flat|nested] 4+ messages in thread