2015-01-03 21:10:01

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 1/4] NFSv4.1: Fix client id trunking on Linux

Currently, our trunking code will check for session trunking, but will
fail to detect client id trunking. This is a problem, because it means
that the client will fail to recognise that the two connections represent
shared state, even if they do not permit a shared session.
By removing the check for the server minor id, and only checking the
major id, we will end up doing the right thing in both cases: we close
down the new nfs_client and fall back to using the existing one.

Fixes: 05f4c350ee02e ("NFS: Discover NFSv4 server trunking when mounting")
Cc: Chuck Lever <[email protected]>
Cc: [email protected] # 3.7.x
Tested-by: Chuck Lever <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4client.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 03311259b0c4..d949d0f378ec 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -566,20 +566,14 @@ static bool nfs4_match_clientids(struct nfs_client *a, struct nfs_client *b)
}

/*
- * Returns true if the server owners match
+ * Returns true if the server major ids match
*/
static bool
-nfs4_match_serverowners(struct nfs_client *a, struct nfs_client *b)
+nfs4_check_clientid_trunking(struct nfs_client *a, struct nfs_client *b)
{
struct nfs41_server_owner *o1 = a->cl_serverowner;
struct nfs41_server_owner *o2 = b->cl_serverowner;

- if (o1->minor_id != o2->minor_id) {
- dprintk("NFS: --> %s server owner minor IDs do not match\n",
- __func__);
- return false;
- }
-
if (o1->major_id_sz != o2->major_id_sz)
goto out_major_mismatch;
if (memcmp(o1->major_id, o2->major_id, o1->major_id_sz) != 0)
@@ -654,7 +648,12 @@ int nfs41_walk_client_list(struct nfs_client *new,
if (!nfs4_match_clientids(pos, new))
continue;

- if (!nfs4_match_serverowners(pos, new))
+ /*
+ * Note that session trunking is just a special subcase of
+ * client id trunking. In either case, we want to fall back
+ * to using the existing nfs_client.
+ */
+ if (!nfs4_check_clientid_trunking(pos, new))
continue;

atomic_inc(&pos->cl_count);
--
2.1.0



2015-01-03 21:10:01

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 2/4] NFSv4: Cache the NFSv4/v4.1 client owner_id in the struct nfs_client

Ensure that we cache the NFSv4/v4.1 client owner_id so that we can
verify it when we're doing trunking detection.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4client.c | 1 +
fs/nfs/nfs4proc.c | 19 +++++++++++++++----
include/linux/nfs_fs_sb.h | 3 +++
3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index d949d0f378ec..6ee9bf69a7a6 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -228,6 +228,7 @@ static void nfs4_shutdown_client(struct nfs_client *clp)
kfree(clp->cl_serverowner);
kfree(clp->cl_serverscope);
kfree(clp->cl_implid);
+ kfree(clp->cl_owner_id);
}

void nfs4_free_client(struct nfs_client *clp)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 002c7dfedb08..8d06217b95ce 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4917,11 +4917,14 @@ static void nfs4_init_boot_verifier(const struct nfs_client *clp,
}

static unsigned int
-nfs4_init_nonuniform_client_string(const struct nfs_client *clp,
+nfs4_init_nonuniform_client_string(struct nfs_client *clp,
char *buf, size_t len)
{
unsigned int result;

+ if (clp->cl_owner_id != NULL)
+ return strlcpy(buf, clp->cl_owner_id, len);
+
rcu_read_lock();
result = scnprintf(buf, len, "Linux NFSv4.0 %s/%s %s",
clp->cl_ipaddr,
@@ -4930,24 +4933,32 @@ nfs4_init_nonuniform_client_string(const struct nfs_client *clp,
rpc_peeraddr2str(clp->cl_rpcclient,
RPC_DISPLAY_PROTO));
rcu_read_unlock();
+ clp->cl_owner_id = kstrdup(buf, GFP_KERNEL);
return result;
}

static unsigned int
-nfs4_init_uniform_client_string(const struct nfs_client *clp,
+nfs4_init_uniform_client_string(struct nfs_client *clp,
char *buf, size_t len)
{
const char *nodename = clp->cl_rpcclient->cl_nodename;
+ unsigned int result;
+
+ if (clp->cl_owner_id != NULL)
+ return strlcpy(buf, clp->cl_owner_id, len);

if (nfs4_client_id_uniquifier[0] != '\0')
- return scnprintf(buf, len, "Linux NFSv%u.%u %s/%s",
+ result = scnprintf(buf, len, "Linux NFSv%u.%u %s/%s",
clp->rpc_ops->version,
clp->cl_minorversion,
nfs4_client_id_uniquifier,
nodename);
- return scnprintf(buf, len, "Linux NFSv%u.%u %s",
+ else
+ result = scnprintf(buf, len, "Linux NFSv%u.%u %s",
clp->rpc_ops->version, clp->cl_minorversion,
nodename);
+ clp->cl_owner_id = kstrdup(buf, GFP_KERNEL);
+ return result;
}

/*
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 1e37fbb78f7a..ddea982355f3 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -74,6 +74,9 @@ struct nfs_client {
/* idmapper */
struct idmap * cl_idmap;

+ /* Client owner identifier */
+ const char * cl_owner_id;
+
/* Our own IP address, as a null-terminated string.
* This is used to generate the mv0 callback address.
*/
--
2.1.0


2015-01-03 21:10:03

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 3/4] NFSv4/v4.1: Verify the client owner id during trunking detection

While we normally expect the NFSv4 client to always send the same client
owner to all servers, there are a couple of situations where that is not
the case:
1) In NFSv4.0, switching between use of '-omigration' and not will cause
the kernel to switch between using the non-uniform and uniform client
strings.
2) In NFSv4.1, or NFSv4.0 when using uniform client strings, if the
uniquifier string is suddenly changed.

This patch will catch those situations by checking the client owner id
in the trunking detection code, and will do the right thing if it notices
that the strings differ.

Cc: Chuck Lever <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4client.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 6ee9bf69a7a6..b1024bcc65c8 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -453,6 +453,14 @@ static void nfs4_swap_callback_idents(struct nfs_client *keep,
spin_unlock(&nn->nfs_client_lock);
}

+static bool nfs4_match_client_owner_id(const struct nfs_client *clp1,
+ const struct nfs_client *clp2)
+{
+ if (clp1->cl_owner_id == NULL || clp2->cl_owner_id == NULL)
+ return true;
+ return strcmp(clp1->cl_owner_id, clp2->cl_owner_id) == 0;
+}
+
/**
* nfs40_walk_client_list - Find server that recognizes a client ID
*
@@ -511,6 +519,9 @@ int nfs40_walk_client_list(struct nfs_client *new,
if (pos->cl_clientid != new->cl_clientid)
continue;

+ if (!nfs4_match_client_owner_id(pos, new))
+ continue;
+
atomic_inc(&pos->cl_count);
spin_unlock(&nn->nfs_client_lock);

@@ -657,6 +668,13 @@ int nfs41_walk_client_list(struct nfs_client *new,
if (!nfs4_check_clientid_trunking(pos, new))
continue;

+ /* Unlike NFSv4.0, we know that NFSv4.1 always uses the
+ * uniform string, however someone might switch the
+ * uniquifier string on us.
+ */
+ if (!nfs4_match_client_owner_id(pos, new))
+ continue;
+
atomic_inc(&pos->cl_count);
*result = pos;
status = 0;
--
2.1.0


2015-01-03 21:10:03

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 4/4] NFSv4: Ignore netid in trunking detection code

The only thing a server cares about when deciding whether or not it is
talking to the same client in a setclientid or exchange_id, is the value
of the client owner id field. In particular, it does not distinguish
between setclientid/exchange_id calls on different types of transport.
When doing trunking detection, the client must therefore also ensure
it does not discriminate between connections on different transports.

Reported-by: Chuck Lever <[email protected]>
Cc: Chuck Lever <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4client.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index b1024bcc65c8..953daa44a282 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -492,9 +492,6 @@ int nfs40_walk_client_list(struct nfs_client *new,
if (pos->rpc_ops != new->rpc_ops)
continue;

- if (pos->cl_proto != new->cl_proto)
- continue;
-
if (pos->cl_minorversion != new->cl_minorversion)
continue;

@@ -627,9 +624,6 @@ int nfs41_walk_client_list(struct nfs_client *new,
if (pos->rpc_ops != new->rpc_ops)
continue;

- if (pos->cl_proto != new->cl_proto)
- continue;
-
if (pos->cl_minorversion != new->cl_minorversion)
continue;

--
2.1.0