On 8 Feb 2022, at 15:27, Trond Myklebust wrote:
On Tue, 2022-02-08 at 15:03 -0500, Benjamin Coddington wrote:
In order to differentiate client state, assign a random uuid to the
uniquifing portion of the client identifier when a network namespace
is
created. Containers may still override this value if they wish to
maintain
stable client identifiers by writing to
/sys/fs/nfs/net/client/identifier,
either by udev rules or other means.
Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
---
fs/nfs/sysfs.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
index 8cb70755e3c9..9b811323fd7f 100644
--- a/fs/nfs/sysfs.c
+++ b/fs/nfs/sysfs.c
@@ -167,6 +167,18 @@ static struct nfs_netns_client
*nfs_netns_client_alloc(struct kobject *parent,
return NULL;
}
+static void assign_unique_clientid(struct nfs_netns_client *clp)
+{
+ unsigned char client_uuid[16];
+ char *uuid_str = kmalloc(UUID_STRING_LEN + 1,
GFP_KERNEL);
+
+ if (uuid_str) {
+ generate_random_uuid(client_uuid);
+ sprintf(uuid_str, "%pU", client_uuid);
+ rcu_assign_pointer(clp->identifier,
uuid_str);
+ }
+}
+
void nfs_netns_sysfs_setup(struct nfs_net *netns, struct net *net)
{
struct nfs_netns_client *clp;
@@ -174,6 +186,8 @@ void nfs_netns_sysfs_setup(struct nfs_net *netns,
struct net *net)
clp = nfs_netns_client_alloc(nfs_client_kobj, net);
if (clp) {
netns->nfs_client = clp;
+ if (net != &init_net)
+ assign_unique_clientid(clp);
Why shouldn't this go in nfs_netns_client_alloc? At this point you've
already published the pointer in netns, so you're prone to races.
No reason it shouldn't, I'll put it there if that's what you want.
I thought that's why it was RCU-ed, and figured we'd just do it before
the
uevent.
Also, why the exclusion of init_net?
Because we're unique enough if we're not in a network namespace, and any
additional uniqueness we might need (due to NAT, or cloned VMs) /should/
be
getting from udev, as you envisioned. That way, if we are getting
uniquified, its from a source that's likely persistent/deterministic.
If we
just generate a random uniquifier, its going to be different next boot
if
there's no udev rule and userspace helpers. That's going to make things
a
lot worse for simple setups.
Ben
