add a clientid mount option

add a clientid mount option

[Christoph Hellwig]

Hi all,

this series adds a new clientid mount option that allows to use a
non-default clientid on a per-mount basis to make it easy to test
client-based behavior on a single client system.

Diffstat:
 fs/nfs/client.c           |   12 +++
 fs/nfs/fs_context.c       |   12 +++
 fs/nfs/internal.h         |    2 
 fs/nfs/nfs4client.c       |  152 ++++++++++++++++++++--------------------------
 fs/nfs/nfs4proc.c         |    7 +-
 include/linux/nfs_fs_sb.h |    1 
 6 files changed, 102 insertions(+), 84 deletions(-)

[PATCH 1/2] NFS: pass struct nfs_client_initdata to nfs4_set_client

[Christoph Hellwig]

Passed the partially filled out structure to nfs4_set_client instead of
11 arguments that then get stashed into the structure.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/nfs4client.c | 151 ++++++++++++++++++++------------------------
 1 file changed, 68 insertions(+), 83 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 162c85a83a14..2e623da1a787 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -895,55 +895,40 @@ nfs4_find_client_sessionid(struct net *net, const struct sockaddr *addr,
  * Set up an NFS4 client
  */
 static int nfs4_set_client(struct nfs_server *server,
-		const char *hostname,
-		const struct sockaddr_storage *addr,
-		const size_t addrlen,
-		const char *ip_addr,
-		int proto, const struct rpc_timeout *timeparms,
-		u32 minorversion, unsigned int nconnect,
-		unsigned int max_connect,
-		struct net *net,
-		struct xprtsec_parms *xprtsec)
+		struct nfs_client_initdata *cl_init)
 {
-	struct nfs_client_initdata cl_init = {
-		.hostname = hostname,
-		.addr = addr,
-		.addrlen = addrlen,
-		.ip_addr = ip_addr,
-		.nfs_mod = &nfs_v4,
-		.proto = proto,
-		.minorversion = minorversion,
-		.net = net,
-		.timeparms = timeparms,
-		.cred = server->cred,
-		.xprtsec = *xprtsec,
-	};
 	struct nfs_client *clp;
 
-	if (minorversion == 0)
-		__set_bit(NFS_CS_REUSEPORT, &cl_init.init_flags);
-	else
-		cl_init.max_connect = max_connect;
-	switch (proto) {
+	cl_init->nfs_mod = &nfs_v4;
+	cl_init->cred = server->cred;
+
+	if (cl_init->minorversion == 0) {
+		__set_bit(NFS_CS_REUSEPORT, &cl_init->init_flags);
+		cl_init->max_connect = 0;
+	}
+
+	switch (cl_init->proto) {
 	case XPRT_TRANSPORT_RDMA:
 	case XPRT_TRANSPORT_TCP:
 	case XPRT_TRANSPORT_TCP_TLS:
-		cl_init.nconnect = nconnect;
+		break;
+	default:
+		cl_init->nconnect = 0;
 	}
 
 	if (server->flags & NFS_MOUNT_NORESVPORT)
-		__set_bit(NFS_CS_NORESVPORT, &cl_init.init_flags);
+		__set_bit(NFS_CS_NORESVPORT, &cl_init->init_flags);
 	if (server->options & NFS_OPTION_MIGRATION)
-		__set_bit(NFS_CS_MIGRATION, &cl_init.init_flags);
+		__set_bit(NFS_CS_MIGRATION, &cl_init->init_flags);
 	if (test_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status))
-		__set_bit(NFS_CS_TSM_POSSIBLE, &cl_init.init_flags);
-	server->port = rpc_get_port((struct sockaddr *)addr);
+		__set_bit(NFS_CS_TSM_POSSIBLE, &cl_init->init_flags);
+	server->port = rpc_get_port((struct sockaddr *)cl_init->addr);
 
 	if (server->flags & NFS_MOUNT_NETUNREACH_FATAL)
-		__set_bit(NFS_CS_NETUNREACH_FATAL, &cl_init.init_flags);
+		__set_bit(NFS_CS_NETUNREACH_FATAL, &cl_init->init_flags);
 
 	/* Allocate or find a client reference we can use */
-	clp = nfs_get_client(&cl_init);
+	clp = nfs_get_client(cl_init);
 	if (IS_ERR(clp))
 		return PTR_ERR(clp);
 
@@ -1156,6 +1141,19 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
 {
 	struct nfs_fs_context *ctx = nfs_fc2context(fc);
 	struct rpc_timeout timeparms;
+	struct nfs_client_initdata cl_init = {
+		.hostname = ctx->nfs_server.hostname,
+		.addr = &ctx->nfs_server._address,
+		.addrlen = ctx->nfs_server.addrlen,
+		.ip_addr = ctx->client_address,
+		.proto = ctx->nfs_server.protocol,
+		.minorversion = ctx->minorversion,
+		.net = fc->net_ns,
+		.timeparms = &timeparms,
+		.xprtsec = ctx->xprtsec,
+		.nconnect = ctx->nfs_server.nconnect,
+		.max_connect = ctx->nfs_server.max_connect,
+	};
 	int error;
 
 	nfs_init_timeout_values(&timeparms, ctx->nfs_server.protocol,
@@ -1175,18 +1173,7 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
 		ctx->selected_flavor = RPC_AUTH_UNIX;
 
 	/* Get a client record */
-	error = nfs4_set_client(server,
-				ctx->nfs_server.hostname,
-				&ctx->nfs_server._address,
-				ctx->nfs_server.addrlen,
-				ctx->client_address,
-				ctx->nfs_server.protocol,
-				&timeparms,
-				ctx->minorversion,
-				ctx->nfs_server.nconnect,
-				ctx->nfs_server.max_connect,
-				fc->net_ns,
-				&ctx->xprtsec);
+	error = nfs4_set_client(server, &cl_init);
 	if (error < 0)
 		return error;
 
@@ -1246,18 +1233,28 @@ struct nfs_server *nfs4_create_server(struct fs_context *fc)
 struct nfs_server *nfs4_create_referral_server(struct fs_context *fc)
 {
 	struct nfs_fs_context *ctx = nfs_fc2context(fc);
-	struct nfs_client *parent_client;
-	struct nfs_server *server, *parent_server;
-	int proto, error;
+	struct nfs_server *parent_server = NFS_SB(ctx->clone_data.sb);
+	struct nfs_client *parent_client = parent_server->nfs_client;
+	struct nfs_client_initdata cl_init = {
+		.hostname = ctx->nfs_server.hostname,
+		.addr = &ctx->nfs_server._address,
+		.addrlen = ctx->nfs_server.addrlen,
+		.ip_addr = parent_client->cl_ipaddr,
+		.minorversion = parent_client->cl_mvops->minor_version,
+		.net = parent_client->cl_net,
+		.timeparms = parent_server->client->cl_timeout,
+		.xprtsec = parent_client->cl_xprtsec,
+		.nconnect = parent_client->cl_nconnect,
+		.max_connect = parent_client->cl_max_connect,
+	};
+	struct nfs_server *server;
 	bool auth_probe;
+	int error;
 
 	server = nfs_alloc_server();
 	if (!server)
 		return ERR_PTR(-ENOMEM);
 
-	parent_server = NFS_SB(ctx->clone_data.sb);
-	parent_client = parent_server->nfs_client;
-
 	server->cred = get_cred(parent_server->cred);
 
 	/* Initialise the client representation from the parent server */
@@ -1266,38 +1263,17 @@ struct nfs_server *nfs4_create_referral_server(struct fs_context *fc)
 	/* Get a client representation */
 #if IS_ENABLED(CONFIG_SUNRPC_XPRT_RDMA)
 	rpc_set_port(&ctx->nfs_server.address, NFS_RDMA_PORT);
-	error = nfs4_set_client(server,
-				ctx->nfs_server.hostname,
-				&ctx->nfs_server._address,
-				ctx->nfs_server.addrlen,
-				parent_client->cl_ipaddr,
-				XPRT_TRANSPORT_RDMA,
-				parent_server->client->cl_timeout,
-				parent_client->cl_mvops->minor_version,
-				parent_client->cl_nconnect,
-				parent_client->cl_max_connect,
-				parent_client->cl_net,
-				&parent_client->cl_xprtsec);
+	cl_init.proto = XPRT_TRANSPORT_RDMA;
+	error = nfs4_set_client(server, &cl_init);
 	if (!error)
 		goto init_server;
 #endif	/* IS_ENABLED(CONFIG_SUNRPC_XPRT_RDMA) */
 
-	proto = XPRT_TRANSPORT_TCP;
+	cl_init.proto = XPRT_TRANSPORT_TCP;
 	if (parent_client->cl_xprtsec.policy != RPC_XPRTSEC_NONE)
-		proto = XPRT_TRANSPORT_TCP_TLS;
+		cl_init.proto = XPRT_TRANSPORT_TCP_TLS;
 	rpc_set_port(&ctx->nfs_server.address, NFS_PORT);
-	error = nfs4_set_client(server,
-				ctx->nfs_server.hostname,
-				&ctx->nfs_server._address,
-				ctx->nfs_server.addrlen,
-				parent_client->cl_ipaddr,
-				proto,
-				parent_server->client->cl_timeout,
-				parent_client->cl_mvops->minor_version,
-				parent_client->cl_nconnect,
-				parent_client->cl_max_connect,
-				parent_client->cl_net,
-				&parent_client->cl_xprtsec);
+	error = nfs4_set_client(server, &cl_init);
 	if (error < 0)
 		goto error;
 
@@ -1353,6 +1329,19 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
 	char buf[INET6_ADDRSTRLEN + 1];
 	struct sockaddr_storage address;
 	struct sockaddr *localaddr = (struct sockaddr *)&address;
+	struct nfs_client_initdata cl_init = {
+		.hostname = hostname,
+		.addr = sap,
+		.addrlen = salen,
+		.ip_addr = buf,
+		.proto = clp->cl_proto,
+		.minorversion = clp->cl_minorversion,
+		.net = net,
+		.timeparms = clnt->cl_timeout,
+		.xprtsec = clp->cl_xprtsec,
+		.nconnect = clp->cl_nconnect,
+		.max_connect = clp->cl_max_connect,
+	};
 	int error;
 
 	error = rpc_switch_client_transport(clnt, &xargs, clnt->cl_timeout);
@@ -1368,11 +1357,7 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
 
 	nfs_server_remove_lists(server);
 	set_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status);
-	error = nfs4_set_client(server, hostname, sap, salen, buf,
-				clp->cl_proto, clnt->cl_timeout,
-				clp->cl_minorversion,
-				clp->cl_nconnect, clp->cl_max_connect,
-				net, &clp->cl_xprtsec);
+	error = nfs4_set_client(server, &cl_init);
 	clear_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status);
 	if (error != 0) {
 		nfs_server_insert_lists(server);
-- 
2.47.2

[PATCH 2/2] NFS: add a clientid mount option

[Christoph Hellwig]

Add a mount option to set a clientid, similarly to how it can be
configured through the per-netfs sysfs file.  This allows for easy
testing of behavior that relies on the client ID likes locks or
delegations with having to resort to separate VMs or containers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/nfs/client.c           | 12 ++++++++++++
 fs/nfs/fs_context.c       | 12 ++++++++++++
 fs/nfs/internal.h         |  2 ++
 fs/nfs/nfs4client.c       |  1 +
 fs/nfs/nfs4proc.c         |  7 ++++++-
 include/linux/nfs_fs_sb.h |  1 +
 6 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 47258dc3af70..1a55debab6e5 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -181,6 +181,12 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
 	clp->cl_nconnect = cl_init->nconnect;
 	clp->cl_max_connect = cl_init->max_connect ? cl_init->max_connect : 1;
 	clp->cl_net = get_net_track(cl_init->net, &clp->cl_ns_tracker, GFP_KERNEL);
+	if (cl_init->clientid) {
+		err = -ENOMEM;
+		clp->clientid = kstrdup(cl_init->clientid, GFP_KERNEL);
+		if (!clp->clientid)
+			goto error_free_host;
+	}
 
 #if IS_ENABLED(CONFIG_NFS_LOCALIO)
 	seqlock_init(&clp->cl_boot_lock);
@@ -193,6 +199,8 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
 	clp->cl_xprtsec = cl_init->xprtsec;
 	return clp;
 
+error_free_host:
+	kfree(clp->cl_hostname);
 error_cleanup:
 	put_nfs_version(clp->cl_nfs_mod);
 error_dealloc:
@@ -254,6 +262,7 @@ void nfs_free_client(struct nfs_client *clp)
 	put_nfs_version(clp->cl_nfs_mod);
 	kfree(clp->cl_hostname);
 	kfree(clp->cl_acceptor);
+	kfree(clp->clientid);
 	kfree_rcu(clp, rcu);
 }
 EXPORT_SYMBOL_GPL(nfs_free_client);
@@ -339,6 +348,9 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
 		if (clp->cl_xprtsec.policy != data->xprtsec.policy)
 			continue;
 
+		if (data->clientid && data->clientid != clp->clientid)
+			continue;
+
 		refcount_inc(&clp->cl_count);
 		return clp;
 	}
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 9e94d18448ff..fe9ecdc8db3c 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -98,6 +98,7 @@ enum nfs_param {
 	Opt_xprtsec,
 	Opt_cert_serial,
 	Opt_privkey_serial,
+	Opt_clientid,
 };
 
 enum {
@@ -225,6 +226,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
 	fsparam_string("xprtsec",	Opt_xprtsec),
 	fsparam_s32("cert_serial",	Opt_cert_serial),
 	fsparam_s32("privkey_serial",	Opt_privkey_serial),
+	fsparam_string("clientid",	Opt_clientid),
 	{}
 };
 
@@ -1031,6 +1033,14 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
 			goto out_invalid_value;
 		}
 		break;
+	case Opt_clientid:
+		if (!param->string || strlen(param->string) == 0 ||
+		    strlen(param->string) > NFS4_CLIENT_ID_UNIQ_LEN - 1)
+			goto out_of_bounds;
+		kfree(ctx->clientid);
+		ctx->clientid = param->string;
+		param->string = NULL;
+		break;
 
 		/*
 		 * Special options
@@ -1650,6 +1660,7 @@ static int nfs_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc)
 	ctx->nfs_server.hostname	= NULL;
 	ctx->fscache_uniq		= NULL;
 	ctx->clone_data.fattr		= NULL;
+	ctx->clientid			= NULL;
 	fc->fs_private = ctx;
 	return 0;
 }
@@ -1670,6 +1681,7 @@ static void nfs_fs_context_free(struct fs_context *fc)
 		kfree(ctx->fscache_uniq);
 		nfs_free_fhandle(ctx->mntfh);
 		nfs_free_fattr(ctx->clone_data.fattr);
+		kfree(ctx->clientid);
 		kfree(ctx);
 	}
 }
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 69c2c10ee658..1a392676d27c 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -86,6 +86,7 @@ struct nfs_client_initdata {
 	struct xprtsec_parms xprtsec;
 	unsigned long connect_timeout;
 	unsigned long reconnect_timeout;
+	const char *clientid;
 };
 
 /*
@@ -115,6 +116,7 @@ struct nfs_fs_context {
 	unsigned short		mountfamily;
 	bool			has_sec_mnt_opts;
 	int			lock_status;
+	char			*clientid;
 
 	struct {
 		union {
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 2e623da1a787..3ab5cc985224 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -1153,6 +1153,7 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
 		.xprtsec = ctx->xprtsec,
 		.nconnect = ctx->nfs_server.nconnect,
 		.max_connect = ctx->nfs_server.max_connect,
+		.clientid = ctx->clientid,
 	};
 	int error;
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ef2077e185b6..ad53bc4ef50c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6487,6 +6487,11 @@ nfs4_get_uniquifier(struct nfs_client *clp, char *buf, size_t buflen)
 
 	buf[0] = '\0';
 
+	if (clp->clientid) {
+		strscpy(buf, clp->clientid, buflen);
+		goto out;
+	}
+
 	if (nn_clp) {
 		rcu_read_lock();
 		id = rcu_dereference(nn_clp->identifier);
@@ -6497,7 +6502,7 @@ nfs4_get_uniquifier(struct nfs_client *clp, char *buf, size_t buflen)
 
 	if (nfs4_client_id_uniquifier[0] != '\0' && buf[0] == '\0')
 		strscpy(buf, nfs4_client_id_uniquifier, buflen);
-
+out:
 	return strlen(buf);
 }
 
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index d2d36711a119..73bed04529a7 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -128,6 +128,7 @@ struct nfs_client {
 	netns_tracker		cl_ns_tracker;
 	struct list_head	pending_cb_stateids;
 	struct rcu_head		rcu;
+	const char		*clientid;
 
 #if IS_ENABLED(CONFIG_NFS_LOCALIO)
 	struct timespec64	cl_nfssvc_boot;
-- 
2.47.2

Re: [PATCH 2/2] NFS: add a clientid mount option

[Chuck Lever]

On 7/14/25 2:30 AM, Christoph Hellwig wrote:
> Add a mount option to set a clientid, similarly to how it can be
> configured through the per-netfs sysfs file.  This allows for easy
> testing of behavior that relies on the client ID likes locks or
> delegations with having to resort to separate VMs or containers.

The problem with approaches like this is that it becomes difficult
to manage multiple mounts of the same server. Each of those mounts
really cannot have a different clientid.

For testing, why can't you use the per-container clientid setting?


> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfs/client.c           | 12 ++++++++++++
>  fs/nfs/fs_context.c       | 12 ++++++++++++
>  fs/nfs/internal.h         |  2 ++
>  fs/nfs/nfs4client.c       |  1 +
>  fs/nfs/nfs4proc.c         |  7 ++++++-
>  include/linux/nfs_fs_sb.h |  1 +
>  6 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 47258dc3af70..1a55debab6e5 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -181,6 +181,12 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
>  	clp->cl_nconnect = cl_init->nconnect;
>  	clp->cl_max_connect = cl_init->max_connect ? cl_init->max_connect : 1;
>  	clp->cl_net = get_net_track(cl_init->net, &clp->cl_ns_tracker, GFP_KERNEL);
> +	if (cl_init->clientid) {
> +		err = -ENOMEM;
> +		clp->clientid = kstrdup(cl_init->clientid, GFP_KERNEL);
> +		if (!clp->clientid)
> +			goto error_free_host;
> +	}
>  
>  #if IS_ENABLED(CONFIG_NFS_LOCALIO)
>  	seqlock_init(&clp->cl_boot_lock);
> @@ -193,6 +199,8 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
>  	clp->cl_xprtsec = cl_init->xprtsec;
>  	return clp;
>  
> +error_free_host:
> +	kfree(clp->cl_hostname);
>  error_cleanup:
>  	put_nfs_version(clp->cl_nfs_mod);
>  error_dealloc:
> @@ -254,6 +262,7 @@ void nfs_free_client(struct nfs_client *clp)
>  	put_nfs_version(clp->cl_nfs_mod);
>  	kfree(clp->cl_hostname);
>  	kfree(clp->cl_acceptor);
> +	kfree(clp->clientid);
>  	kfree_rcu(clp, rcu);
>  }
>  EXPORT_SYMBOL_GPL(nfs_free_client);
> @@ -339,6 +348,9 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
>  		if (clp->cl_xprtsec.policy != data->xprtsec.policy)
>  			continue;
>  
> +		if (data->clientid && data->clientid != clp->clientid)
> +			continue;
> +
>  		refcount_inc(&clp->cl_count);
>  		return clp;
>  	}
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 9e94d18448ff..fe9ecdc8db3c 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -98,6 +98,7 @@ enum nfs_param {
>  	Opt_xprtsec,
>  	Opt_cert_serial,
>  	Opt_privkey_serial,
> +	Opt_clientid,
>  };
>  
>  enum {
> @@ -225,6 +226,7 @@ static const struct fs_parameter_spec nfs_fs_parameters[] = {
>  	fsparam_string("xprtsec",	Opt_xprtsec),
>  	fsparam_s32("cert_serial",	Opt_cert_serial),
>  	fsparam_s32("privkey_serial",	Opt_privkey_serial),
> +	fsparam_string("clientid",	Opt_clientid),
>  	{}
>  };
>  
> @@ -1031,6 +1033,14 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
>  			goto out_invalid_value;
>  		}
>  		break;
> +	case Opt_clientid:
> +		if (!param->string || strlen(param->string) == 0 ||
> +		    strlen(param->string) > NFS4_CLIENT_ID_UNIQ_LEN - 1)
> +			goto out_of_bounds;
> +		kfree(ctx->clientid);
> +		ctx->clientid = param->string;
> +		param->string = NULL;
> +		break;
>  
>  		/*
>  		 * Special options
> @@ -1650,6 +1660,7 @@ static int nfs_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc)
>  	ctx->nfs_server.hostname	= NULL;
>  	ctx->fscache_uniq		= NULL;
>  	ctx->clone_data.fattr		= NULL;
> +	ctx->clientid			= NULL;
>  	fc->fs_private = ctx;
>  	return 0;
>  }
> @@ -1670,6 +1681,7 @@ static void nfs_fs_context_free(struct fs_context *fc)
>  		kfree(ctx->fscache_uniq);
>  		nfs_free_fhandle(ctx->mntfh);
>  		nfs_free_fattr(ctx->clone_data.fattr);
> +		kfree(ctx->clientid);
>  		kfree(ctx);
>  	}
>  }
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 69c2c10ee658..1a392676d27c 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -86,6 +86,7 @@ struct nfs_client_initdata {
>  	struct xprtsec_parms xprtsec;
>  	unsigned long connect_timeout;
>  	unsigned long reconnect_timeout;
> +	const char *clientid;
>  };
>  
>  /*
> @@ -115,6 +116,7 @@ struct nfs_fs_context {
>  	unsigned short		mountfamily;
>  	bool			has_sec_mnt_opts;
>  	int			lock_status;
> +	char			*clientid;
>  
>  	struct {
>  		union {
> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
> index 2e623da1a787..3ab5cc985224 100644
> --- a/fs/nfs/nfs4client.c
> +++ b/fs/nfs/nfs4client.c
> @@ -1153,6 +1153,7 @@ static int nfs4_init_server(struct nfs_server *server, struct fs_context *fc)
>  		.xprtsec = ctx->xprtsec,
>  		.nconnect = ctx->nfs_server.nconnect,
>  		.max_connect = ctx->nfs_server.max_connect,
> +		.clientid = ctx->clientid,
>  	};
>  	int error;
>  
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index ef2077e185b6..ad53bc4ef50c 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6487,6 +6487,11 @@ nfs4_get_uniquifier(struct nfs_client *clp, char *buf, size_t buflen)
>  
>  	buf[0] = '\0';
>  
> +	if (clp->clientid) {
> +		strscpy(buf, clp->clientid, buflen);
> +		goto out;
> +	}
> +
>  	if (nn_clp) {
>  		rcu_read_lock();
>  		id = rcu_dereference(nn_clp->identifier);
> @@ -6497,7 +6502,7 @@ nfs4_get_uniquifier(struct nfs_client *clp, char *buf, size_t buflen)
>  
>  	if (nfs4_client_id_uniquifier[0] != '\0' && buf[0] == '\0')
>  		strscpy(buf, nfs4_client_id_uniquifier, buflen);
> -
> +out:
>  	return strlen(buf);
>  }
>  
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index d2d36711a119..73bed04529a7 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -128,6 +128,7 @@ struct nfs_client {
>  	netns_tracker		cl_ns_tracker;
>  	struct list_head	pending_cb_stateids;
>  	struct rcu_head		rcu;
> +	const char		*clientid;
>  
>  #if IS_ENABLED(CONFIG_NFS_LOCALIO)
>  	struct timespec64	cl_nfssvc_boot;


-- 
Chuck Lever

Re: [PATCH 2/2] NFS: add a clientid mount option

[Christoph Hellwig]

On Mon, Jul 14, 2025 at 09:24:20AM -0400, Chuck Lever wrote:
> On 7/14/25 2:30 AM, Christoph Hellwig wrote:
> > Add a mount option to set a clientid, similarly to how it can be
> > configured through the per-netfs sysfs file.  This allows for easy
> > testing of behavior that relies on the client ID likes locks or
> > delegations with having to resort to separate VMs or containers.
> 
> The problem with approaches like this is that it becomes difficult
> to manage multiple mounts of the same server. Each of those mounts
> really cannot have a different clientid.

Having different clientids for multiple mounts from the same server
is the purpose and only reason for this option.

> For testing, why can't you use the per-container clientid setting?

Because having to create a container is a lot of effort when all
that is needed is just a mount with a different clientid.

Re: [PATCH 2/2] NFS: add a clientid mount option

[Chuck Lever]

On 7/14/25 9:31 AM, Christoph Hellwig wrote:
> On Mon, Jul 14, 2025 at 09:24:20AM -0400, Chuck Lever wrote:
>> On 7/14/25 2:30 AM, Christoph Hellwig wrote:
>>> Add a mount option to set a clientid, similarly to how it can be
>>> configured through the per-netfs sysfs file.  This allows for easy
>>> testing of behavior that relies on the client ID likes locks or
>>> delegations with having to resort to separate VMs or containers.
>>
>> The problem with approaches like this is that it becomes difficult
>> to manage multiple mounts of the same server. Each of those mounts
>> really cannot have a different clientid.
> 
> Having different clientids for multiple mounts from the same server
> is the purpose and only reason for this option.

It would be helpful to explain exactly what test you are trying to do or
what bug you are trying to explore. I can't think of a way that the
current client code base would ever need to behave this way. So I assume
you are trying to test some kind of server behavior. If that's the case,
why not craft one or more pynfs test cases? (Or, maybe pynfs already
handles this case).


>> For testing, why can't you use the per-container clientid setting?
> 
> Because having to create a container is a lot of effort when all
> that is needed is just a mount with a different clientid.

Since this is for development testing (?) I am hesitant to endorse
adding it as part of the everyday administrative interface. Especially
since this will break things (on purpose, of course). I don't relish
having to support administrators coming to us complaining that some
unimagined future use case is not working with the clientid= mount
option.

If clientid= does get merged, though, what is your plan for an nfs(5)
update?


-- 
Chuck Lever

Re: [PATCH 2/2] NFS: add a clientid mount option

[Trond Myklebust]

On Mon, 2025-07-14 at 10:47 -0400, Chuck Lever wrote:
> On 7/14/25 9:31 AM, Christoph Hellwig wrote:
> > On Mon, Jul 14, 2025 at 09:24:20AM -0400, Chuck Lever wrote:
> > > On 7/14/25 2:30 AM, Christoph Hellwig wrote:
> > > > Add a mount option to set a clientid, similarly to how it can
> > > > be
> > > > configured through the per-netfs sysfs file.  This allows for
> > > > easy
> > > > testing of behavior that relies on the client ID likes locks or
> > > > delegations with having to resort to separate VMs or
> > > > containers.
> > > 
> > > The problem with approaches like this is that it becomes
> > > difficult
> > > to manage multiple mounts of the same server. Each of those
> > > mounts
> > > really cannot have a different clientid.
> > 
> > Having different clientids for multiple mounts from the same server
> > is the purpose and only reason for this option.
> 
> It would be helpful to explain exactly what test you are trying to do
> or
> what bug you are trying to explore. I can't think of a way that the
> current client code base would ever need to behave this way. So I
> assume
> you are trying to test some kind of server behavior. If that's the
> case,
> why not craft one or more pynfs test cases? (Or, maybe pynfs already
> handles this case).
> 
> 
> > > For testing, why can't you use the per-container clientid
> > > setting?
> > 
> > Because having to create a container is a lot of effort when all
> > that is needed is just a mount with a different clientid.
> 
> Since this is for development testing (?) I am hesitant to endorse
> adding it as part of the everyday administrative interface.
> Especially
> since this will break things (on purpose, of course). I don't relish
> having to support administrators coming to us complaining that some
> unimagined future use case is not working with the clientid= mount
> option.
> 
> If clientid= does get merged, though, what is your plan for an nfs(5)
> update?
> 

There is a lot of potential for tripping over your own shoelaces with
this mount option.

I can't think of any circumstances where an ordinary user should need
to set a different client identifier depending on the server. I too am
therefore sceptical that anyone will need this functionality other than
for kernel development purposes. It requires very deep knowledge of the
NFSv4 protocol both to understand what it does, and to stay out of
trouble when using it.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH 1/2] NFS: pass struct nfs_client_initdata to nfs4_set_client

[Trond Myklebust]

On Mon, 2025-07-14 at 08:30 +0200, Christoph Hellwig wrote:
> Passed the partially filled out structure to nfs4_set_client instead
> of
> 11 arguments that then get stashed into the structure.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 

This looks like a decent cleanup.



-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com

Re: [PATCH 2/2] NFS: add a clientid mount option

[Christoph Hellwig]

On Mon, Jul 14, 2025 at 10:47:54AM -0400, Chuck Lever wrote:
> It would be helpful to explain exactly what test you are trying to do or
> what bug you are trying to explore. I can't think of a way that the
> current client code base would ever need to behave this way. So I assume
> you are trying to test some kind of server behavior. If that's the case,
> why not craft one or more pynfs test cases? (Or, maybe pynfs already
> handles this case).

In this case I test the performance of delegation recalls on the client,
for which I need another client to trigger the recall.  See the
"use a hash for looking up delegation" series for the result.

> Since this is for development testing (?) I am hesitant to endorse
> adding it as part of the everyday administrative interface. Especially
> since this will break things (on purpose, of course). I don't relish
> having to support administrators coming to us complaining that some
> unimagined future use case is not working with the clientid= mount
> option.

What use case would this break?  It basically means that if you mount
the same export multiple times, where at least one of the mounts has
this option specified you don't share the connection and sb, but get
a new one with a different client id.  I don't see any good use case
for that outside of testing, but I also don't see how it could create
problems.

> If clientid= does get merged, though, what is your plan for an nfs(5)
> update?

Add it and discourage the use.  Which reminds me that the man page also
needs an update for the tls key changes.

Re: [PATCH 2/2] NFS: add a clientid mount option

[Christoph Hellwig]

On Mon, Jul 14, 2025 at 08:49:51AM -0700, Trond Myklebust wrote:
> There is a lot of potential for tripping over your own shoelaces with
> this mount option.
> 
> I can't think of any circumstances where an ordinary user should need
> to set a different client identifier depending on the server. I too am
> therefore sceptical that anyone will need this functionality other than
> for kernel development purposes. It requires very deep knowledge of the
> NFSv4 protocol both to understand what it does,

I agree so far.

> and to stay out of
> trouble when using it.

But I don't really see the trouble when using it, except for the fact
that no one really should use it.

But hey, I can live with this not getting merged if the use case is
too narrow, at least it can be found now if someone needs it :)