NLM lock failover - lock release

The patch piggy-backs the logic into "rq_daddr" field of struct svc_rqst where NFS server ip address is stored. Upon writing IPv4 address in standard dot notation into /proc/fs/nfsd/nlm_unlock, the logic will examine NLM's global nlm_files list and subsequently unlock the associated file if server ip address matches.

Due to the size of rq_daddr (u32), we would not be able to support IPV6 for this round of changes. Another to-do item is to enable client:server ip pairs to allow NFS V4 failover.

Quoted 11 lines · Wendy Cheng

The patch piggy-backs the logic into "rq_daddr" field of struct svc_rqst where NFS server ip address is stored. Upon writing IPv4 address in standard dot notation into /proc/fs/nfsd/nlm_unlock, the logic will examine NLM's global nlm_files list and subsequently unlock the associated file if server ip address matches.

Due to the size of rq_daddr (u32), we would not be able to support IPV6 for this round of changes. Another to-do item is to enable client:server ip pairs to allow NFS V4 failover.

FYI: I have a patch in my IPv6 patchset that increases the size of this field specifically in order to hold an IPv6 address. See:

http://oss.oracle.com/~cel/linux-2.6/2.6.17/patches/52-svc-rq_daddr.diff

for the individual patch, and surrounding patches for more context.

The patch piggy-backs the logic into "rq_daddr" field of struct svc_rqst where NFS server ip address is stored. Upon writing IPv4 address in standard dot notation into /proc/fs/nfsd/nlm_unlock, the logic will examine NLM's global nlm_files list and subsequently unlock the associated file if server ip address matches.

Due to the size of rq_daddr (u32), we would not be able to support IPV6 for this round of changes. Another to-do item is to enable client:server ip pairs to allow NFS V4 failover.

plain text document attachment (gfs_nlm_unlock.patch)

 fs/lockd/svcsubs.c          |   58 ++++++++++++++++++++++++++++++++++++++++----
 fs/nfsd/nfsctl.c            |   54 ++++++++++++++++++++++++++++++++++++++++
 include/linux/lockd/bind.h  |    5 +++
 include/linux/lockd/lockd.h |    3 ++
 net/sunrpc/svcsock.c        |    5 +++
 5 files changed, 120 insertions(+), 5 deletions(-)

include/linux/lockd/lockd.h +3 -0

@@ -105,6 +105,7 @@ struct nlm_file {
 	unsigned int		f_count;	/* reference count */
 	struct semaphore	f_sema;		/* avoid concurrent access */
 	int		       	f_hash;		/* hash of f_handle */
+	__u32			f_iaddr;	/* server ip for failover */
 };
 
 /*

@@ -133,6 +134,7 @@ struct nlm_block {
 #define NLM_ACT_CHECK		0		/* check for locks */
 #define NLM_ACT_MARK		1		/* mark & sweep */
 #define NLM_ACT_UNLOCK		2		/* release all locks */
+#define NLM_ACT_FO_UNLOCK	3		/* failover release locks */
 
 /*
  * Global variables

@@ -196,6 +198,7 @@ void		  nlm_release_file(struct nlm_file
 void		  nlmsvc_mark_resources(void);
 void		  nlmsvc_free_host_resources(struct nlm_host *);
 void		  nlmsvc_invalidate_all(void);
+int 		  nlmsvc_fo_unlock(struct in_addr *);
 
 static __inline__ struct inode *
 nlmsvc_file_inode(struct nlm_file *file)

net/sunrpc/svcsock.c +1 -0

@@ -454,6 +454,7 @@ svc_recvfrom(struct svc_rqst *rqstp, str
 	struct msghdr	msg;
 	struct socket	*sock;
 	int		len, alen;
+	struct sockaddr_in   daddr;
 
 	rqstp->rq_addrlen = sizeof(rqstp->rq_addr);
 	sock = rqstp->rq_sock->sk_sock;

Review

Quoted 5 lines

(quoted diff) +4 -0

+	/* add server ip for nlm lock failover */
+	sock->ops->getname(sock, (struct sockaddr *)&daddr, &alen, 0);
+	rqstp->rq_daddr = daddr.sin_addr.s_addr;
+

Hmm.... Why would you want to do this on every receive when you could just store the ip address in the struct svc_sock once and for all?

That said, how do you envisage this working in the cases where the socket is bound to INADDR_ANY?

Quoted 8 lines · Trond Myklebust

Review

Quoted 5 lines

(quoted diff) +4 -0

+	/* add server ip for nlm lock failover */
+	sock->ops->getname(sock, (struct sockaddr *)&daddr, &alen, 0);
+	rqstp->rq_daddr = daddr.sin_addr.s_addr;
+

Hmm.... Why would you want to do this on every receive when you could just store the ip address in the struct svc_sock once and for all?

ok, will do that - save latency. Thanks.

Quoted 1 line

Quoted 2 lines

That said, how do you envisage this working in the cases where the socket is bound to INADDR_ANY?

This is "our" (server's) address, not peer address - for this request to arrive "here", it can't be INADDR_ANY. Can it ? Remember "rq_daddr" will only be used during failover in a clustered NFS servers environment.

The socket can only be bound to INADDR_ANY for UDP, and in that case we already set rq_daddr correctly.

Quoted 2 lines · Neil Brown

The socket can only be bound to INADDR_ANY for UDP, and in that case we already set rq_daddr correctly.

As I understand it, Wendy is considering the case of a multi-homed server. She wants to record the IP address on which we received the datagram so that she knows which locks to invalidate in the case of a migration of that particular IP address onto another server.

My point is that she won't get that information if the socket is bound to INADDR_ANY, as would be the case for a UDP socket.

This is "our" (server's) address, not peer address - for this request to arrive "here", it can't be INADDR_ANY. Can it ?