Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Benjamin Coddington <bcodding@redhat.com>
To:     trond.myklebust@hammerspace.com, anna.schumaker@netapp.com
Cc:     linux-nfs@vger.kernel.org
Subject: [PATCH] NFSv4: Tune the race to set and use the client id uniquifier
Date:   Wed, 16 Feb 2022 12:42:00 -0500
Message-Id: <61a5993a1f9bbed2ba1227bd3376e92232e0530a.1645033262.git.bcodding@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

In order to set a unique but persistent value for the nfs client's ID, the
client exposes a per-network-namespace sysfs file that can be used to
configure a "uniquifier" for this value.

However any userspace mechanism used to configure this value must do so in
the potentially small window between either (1) the nfs module getting
loaded or (2) the creation of a new network namespace, and the client
sending SETCLIENTID or EXCHANGE_ID.

In (1) the time between these events can be very small if the kernel loads
the nfs module as triggered by the first mount request.  That leaves little
time for another process such as a userspace helper to lookup or generate a
uniquifier and write it to the kernel before the kernel attempts to create
and use the identifier.

In (2) the network namespace may be created but network configuration and
processes within that namespace that may trigger on the creation of the
sysfs file are not ready, or the setup of filesystems and tools for that
namespace may happen in parallel.

Fix this by creating a new nfs module parameter "nfs4_unique_id_timeout"
that will allow userspace a tunable window of time to uniquify the client.
When set, the client waits for a uniquifier to be set before sending
SETCLIENTID or EXCHANGE_ID.  The parameter defaults to 500ms. Setting the
parameter to zero reverts any waiting for a uniquifier.

A tunable delay can accommodate situations where the size of the race
window needs to be modified due to hardware differences or various
approaches to container initialization with respect to the use of NFS
within those containers.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/nfs4_fs.h  | 1 +
 fs/nfs/nfs4proc.c | 4 ++++
 fs/nfs/super.c    | 4 ++++
 fs/nfs/sysfs.c    | 3 +++
 fs/nfs/sysfs.h    | 2 ++
 5 files changed, 14 insertions(+)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 3e344bec3647..052805c3cfc0 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -544,6 +544,7 @@ extern bool recover_lost_locks;
 
 #define NFS4_CLIENT_ID_UNIQ_LEN		(64)
 extern char nfs4_client_id_uniquifier[NFS4_CLIENT_ID_UNIQ_LEN];
+extern unsigned int nfs4_client_id_uniquifier_timeout;
 
 extern int nfs4_try_get_tree(struct fs_context *);
 extern int nfs4_get_referral_tree(struct fs_context *);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3106bd28b113..2ddffd799c7f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6135,6 +6135,10 @@ nfs4_get_uniquifier(struct nfs_client *clp, char *buf, size_t buflen)
 	buf[0] = '\0';
 
 	if (nn_clp) {
+		if (!nn_clp->user_uniquified && nfs4_client_id_uniquifier_timeout)
+			wait_for_completion_interruptible_timeout(&nn_clp->uniquified,
+				msecs_to_jiffies(nfs4_client_id_uniquifier_timeout));
+
 		rcu_read_lock();
 		id = rcu_dereference(nn_clp->identifier);
 		if (id)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 4034102010f0..cad5acd1f79d 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -1329,6 +1329,7 @@ unsigned short max_session_slots = NFS4_DEF_SLOT_TABLE_SIZE;
 unsigned short max_session_cb_slots = NFS4_DEF_CB_SLOT_TABLE_SIZE;
 unsigned short send_implementation_id = 1;
 char nfs4_client_id_uniquifier[NFS4_CLIENT_ID_UNIQ_LEN] = "";
+unsigned int nfs4_client_id_uniquifier_timeout = 500;
 bool recover_lost_locks = false;
 
 EXPORT_SYMBOL_GPL(nfs_callback_nr_threads);
@@ -1339,6 +1340,7 @@ EXPORT_SYMBOL_GPL(max_session_slots);
 EXPORT_SYMBOL_GPL(max_session_cb_slots);
 EXPORT_SYMBOL_GPL(send_implementation_id);
 EXPORT_SYMBOL_GPL(nfs4_client_id_uniquifier);
+EXPORT_SYMBOL_GPL(nfs4_client_id_uniquifier_timeout);
 EXPORT_SYMBOL_GPL(recover_lost_locks);
 
 #define NFS_CALLBACK_MAXPORTNR (65535U)
@@ -1370,6 +1372,7 @@ module_param(nfs_idmap_cache_timeout, int, 0644);
 module_param(nfs4_disable_idmapping, bool, 0644);
 module_param_string(nfs4_unique_id, nfs4_client_id_uniquifier,
 			NFS4_CLIENT_ID_UNIQ_LEN, 0600);
+module_param_named(nfs4_unique_id_timeout, nfs4_client_id_uniquifier_timeout, int, 0644);
 MODULE_PARM_DESC(nfs4_disable_idmapping,
 		"Turn off NFSv4 idmapping when using 'sec=sys'");
 module_param(max_session_slots, ushort, 0644);
@@ -1382,6 +1385,7 @@ module_param(send_implementation_id, ushort, 0644);
 MODULE_PARM_DESC(send_implementation_id,
 		"Send implementation ID with NFSv4.1 exchange_id");
 MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4 uniquifier string");
+MODULE_PARM_DESC(nfs4_unique_id_timeout, "msecs to wait for nfs_client_id4 uniquifier string");
 
 module_param(recover_lost_locks, bool, 0644);
 MODULE_PARM_DESC(recover_lost_locks,
diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
index be05522a2c8b..a54d342bc381 100644
--- a/fs/nfs/sysfs.c
+++ b/fs/nfs/sysfs.c
@@ -117,6 +117,8 @@ static ssize_t nfs_netns_identifier_store(struct kobject *kobj,
 		synchronize_rcu();
 		kfree(old);
 	}
+	c->user_uniquified = true;
+	complete(&c->uniquified);
 	return count;
 }
 
@@ -171,6 +173,7 @@ static struct nfs_netns_client *nfs_netns_client_alloc(struct kobject *parent,
 	if (p) {
 		if (net != &init_net)
 			assign_unique_clientid(p);
+		init_completion(&p->uniquified);
 		p->net = net;
 		p->kobject.kset = nfs_client_kset;
 		if (kobject_init_and_add(&p->kobject, &nfs_netns_client_type,
diff --git a/fs/nfs/sysfs.h b/fs/nfs/sysfs.h
index 5501ef573c32..f733439a1084 100644
--- a/fs/nfs/sysfs.h
+++ b/fs/nfs/sysfs.h
@@ -12,6 +12,8 @@ struct nfs_netns_client {
 	struct kobject kobject;
 	struct net *net;
 	const char __rcu *identifier;
+	bool user_uniquified;
+	struct completion uniquified;
 };
 
 extern struct kobject *nfs_client_kobj;
-- 
2.31.1