Linux NFS development
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: trond.myklebust@netapp.com
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 10/14] NFS: Always use the same SETCLIENTID boot verifier
Date: Fri, 18 May 2012 18:06:41 -0400	[thread overview]
Message-ID: <20120518220641.774.31404.stgit@degas.1015granger.net> (raw)
In-Reply-To: <20120518220145.774.53741.stgit@degas.1015granger.net>

Currently our NFS client assigns a unique SETCLIENTID boot verifier
for each server IP address it knows about.  It's set to CURRENT_TIME
when the struct nfs_client for that server IP is created.

During the SETCLIENTID operation, our client also presents an
nfs_client_id4 string to servers, as an identifier on which the server
can hang all of this client's NFSv4 state.  Our client's
nfs_client_id4 string is unique for each server IP address.

An NFSv4 server is obligated to wipe all NFSv4 state associated with
an nfs_client_id4 string when the client presents this string with a
changed SETCLIENTID boot verifier.

When our client unmounts the last of a server's shares, it destroys
that server's struct nfs_client.  The next time the client mounts that
NFS server, it creates a fresh struct nfs_client with a fresh boot
verifier.  On seeing the fresh verifer, the server wipes any previous
NFSv4 state associated with that nfs_client_id4.

However, NFSv4.1 clients are supposed to present the same
nfs_client_id4 string to all servers.  And, to support Transparent
State Migration, the same nfs_client_id4 string should be presented
to all NFSv4.0 servers so they recognize that migrated state for this
client belongs with state a server may already have for this client.

If the nfs_client_id4 string is the same but the boot verifier changes
for each server IP address, SETCLIENTIDs from such a client could
unintentionally result in a server wiping the client's previously
obtained lease.

Thus, if our NFS client is going to use a fixed nfs_client_id4 string,
either for NFSv4.0 or NFSv4.1 mounts, our NFS client should use a
SETCLIENTID boot verifier that does not change depending on server IP
address.

Replace our current per-nfs_client boot verifier with a per-nfs_net
boot verifier.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfs/client.c           |    2 +-
 fs/nfs/netns.h            |    5 +++++
 fs/nfs/nfs4proc.c         |   14 ++++++++------
 fs/nfs/nfs4xdr.c          |    5 ++++-
 include/linux/nfs_fs_sb.h |    3 ---
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index c430acd..84bca37 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -182,7 +182,6 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
 	spin_lock_init(&clp->cl_lock);
 	INIT_DELAYED_WORK(&clp->cl_renewd, nfs4_renew_state);
 	rpc_init_wait_queue(&clp->cl_rpcwaitq, "NFS client");
-	clp->cl_boot_time = CURRENT_TIME;
 	clp->cl_state = 1 << NFS4CLNT_LEASE_EXPIRED;
 	clp->cl_minorversion = cl_init->minorversion;
 	clp->cl_mvops = nfs_v4_minor_ops[cl_init->minorversion];
@@ -1805,6 +1804,7 @@ void nfs_clients_init(struct net *net)
 	idr_init(&nn->cb_ident_idr);
 #endif
 	spin_lock_init(&nn->nfs_client_lock);
+	nn->boot_time = CURRENT_TIME;
 }
 
 #ifdef CONFIG_PROC_FS
diff --git a/fs/nfs/netns.h b/fs/nfs/netns.h
index aa14ec3..8a6394e 100644
--- a/fs/nfs/netns.h
+++ b/fs/nfs/netns.h
@@ -1,3 +1,7 @@
+/*
+ * NFS-private data for each "struct net".  Accessed with net_generic().
+ */
+
 #ifndef __NFS_NETNS_H__
 #define __NFS_NETNS_H__
 
@@ -20,6 +24,7 @@ struct nfs_net {
 	struct idr cb_ident_idr; /* Protected by nfs_client_lock */
 #endif
 	spinlock_t nfs_client_lock;
+	struct timespec boot_time;
 };
 
 extern int nfs_net_id;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c56d547..aaf6b28 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -64,6 +64,7 @@
 #include "iostat.h"
 #include "callback.h"
 #include "pnfs.h"
+#include "netns.h"
 
 #define NFSDBG_FACILITY		NFSDBG_PROC
 
@@ -3932,8 +3933,8 @@ wait_on_recovery:
 	return -EAGAIN;
 }
 
-static void nfs4_construct_boot_verifier(struct nfs_client *clp,
-					 nfs4_verifier *bootverf)
+static void nfs4_init_boot_verifier(const struct nfs_client *clp,
+				    nfs4_verifier *bootverf)
 {
 	__be32 verf[2];
 
@@ -3941,8 +3942,9 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
 		verf[0] = (__be32)CURRENT_TIME.tv_sec;
 		verf[1] = (__be32)CURRENT_TIME.tv_nsec;
 	} else {
-		verf[0] = (__be32)clp->cl_boot_time.tv_sec;
-		verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
+		struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
+		verf[0] = (__be32)nn->boot_time.tv_sec;
+		verf[1] = (__be32)nn->boot_time.tv_nsec;
 	}
 	memcpy(bootverf->data, verf, sizeof(bootverf->data));
 }
@@ -3966,7 +3968,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 	int loop = 0;
 	int status;
 
-	nfs4_construct_boot_verifier(clp, &sc_verifier);
+	nfs4_init_boot_verifier(clp, &sc_verifier);
 
 	for(;;) {
 		rcu_read_lock();
@@ -5125,7 +5127,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 	dprintk("--> %s\n", __func__);
 	BUG_ON(clp == NULL);
 
-	nfs4_construct_boot_verifier(clp, &verifier);
+	nfs4_init_boot_verifier(clp, &verifier);
 
 	args.id_len = scnprintf(args.id, sizeof(args.id),
 				"%s/%s/%u",
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index c54aae3..4538df1 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -53,9 +53,11 @@
 #include <linux/nfs4.h>
 #include <linux/nfs_fs.h>
 #include <linux/nfs_idmap.h>
+
 #include "nfs4_fs.h"
 #include "internal.h"
 #include "pnfs.h"
+#include "netns.h"
 
 #define NFSDBG_FACILITY		NFSDBG_XDR
 
@@ -1726,6 +1728,7 @@ static void encode_create_session(struct xdr_stream *xdr,
 	char machine_name[NFS4_MAX_MACHINE_NAME_LEN];
 	uint32_t len;
 	struct nfs_client *clp = args->client;
+	struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
 	u32 max_resp_sz_cached;
 
 	/*
@@ -1767,7 +1770,7 @@ static void encode_create_session(struct xdr_stream *xdr,
 	*p++ = cpu_to_be32(RPC_AUTH_UNIX);			/* auth_sys */
 
 	/* authsys_parms rfc1831 */
-	*p++ = cpu_to_be32((u32)clp->cl_boot_time.tv_nsec);	/* stamp */
+	*p++ = (__be32)nn->boot_time.tv_nsec;		/* stamp */
 	p = xdr_encode_opaque(p, machine_name, len);
 	*p++ = cpu_to_be32(0);				/* UID */
 	*p++ = cpu_to_be32(0);				/* GID */
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 59410b3..fbec57d 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -61,9 +61,6 @@ struct nfs_client {
 
 	struct rpc_wait_queue	cl_rpcwaitq;
 
-	/* used for the setclientid verifier */
-	struct timespec		cl_boot_time;
-
 	/* idmapper */
 	struct idmap *		cl_idmap;
 


  parent reply	other threads:[~2012-05-18 22:06 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-18 22:05 [PATCH 00/14] UCS pre-requisites for 3.5 Chuck Lever
2012-05-18 22:05 ` [PATCH 01/14] NFS: Fix comment misspelling in struct nfs_client definition Chuck Lever
2012-05-18 22:05 ` [PATCH 02/14] NFS: Use proper naming conventions for NFSv4.1 server scope fields Chuck Lever
2012-05-19 21:08   ` Myklebust, Trond
2012-05-18 22:05 ` [PATCH 03/14] NFS: Use proper naming conventions for nfs_client.impl_id field Chuck Lever
2012-05-21 15:06   ` Adamson, Dros
2012-05-18 22:05 ` [PATCH 04/14] NFS: Use proper naming conventions for the nfs_client.net field Chuck Lever
2012-05-18 22:05 ` [PATCH 05/14] NFS: Clean up return code checking in nfs4_proc_exchange_id() Chuck Lever
2012-05-21 15:10   ` Adamson, Dros
2012-05-21 15:19     ` Chuck Lever
2012-05-18 22:06 ` [PATCH 06/14] NFS: Remove nfs_unique_id Chuck Lever
2012-05-18 22:06 ` [PATCH 07/14] NFS: Don't swap bytes in nfs4_construct_boot_verifier() Chuck Lever
2012-05-21 15:40   ` J. Bruce Fields
2012-05-21 15:47     ` Chuck Lever
2012-05-21 15:51       ` J. Bruce Fields
2012-05-18 22:06 ` [PATCH 08/14] NFS: Add NFSDBG_STATE Chuck Lever
2012-05-18 22:06 ` [PATCH 09/14] NFS: Force server to drop NFSv4 state Chuck Lever
2012-05-21 16:08   ` J. Bruce Fields
2012-05-21 16:11     ` J. Bruce Fields
2012-05-21 16:11     ` Chuck Lever
2012-05-21 16:16       ` J. Bruce Fields
2012-05-21 16:24         ` Chuck Lever
2012-05-21 16:49           ` Myklebust, Trond
2012-05-18 22:06 ` Chuck Lever [this message]
2012-05-18 22:06 ` [PATCH 11/14] NFS: Refactor nfs_get_client(): add nfs_found_client() Chuck Lever
2012-05-18 22:06 ` [PATCH 12/14] NFS: Refactor nfs_get_client(): initialize nfs_client Chuck Lever
2012-05-18 22:07 ` [PATCH 13/14] NFS: Add nfs_client behavior flags Chuck Lever
2012-05-18 22:07 ` [PATCH 14/14] NFS: EXCHANGE_ID should save the server major and minor ID Chuck Lever
  -- strict thread matches above, loose matches on Subject: below --
2012-05-22  2:44 [PATCH 00/14 v2] UCS pre-requisites for 3.5 Chuck Lever
2012-05-22  2:45 ` [PATCH 10/14] NFS: Always use the same SETCLIENTID boot verifier Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120518220641.774.31404.stgit@degas.1015granger.net \
    --to=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@netapp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox