[PATCH 00/16] NLM clean-ups for IPv6 support

[PATCH 00/16] NLM clean-ups for IPv6 support

[Chuck Lever]

Hi Bruce, Trond -

This patch series prepares the kernel NLM implementation to support IPv6
network addresses.  These should not cause any disruption for legacy IPv4
support, and shouldn't depend on the previous 15 patches I just posted.

Please consider these for 2.6.27.  I think it would be reasonable for this
work to get some exercise with IPv4 before we finish the IPv6
implementation.

I have about 10 more patches that add NSM support for IPv6 and expose NLM's
IPv6 capability.  However, we haven't been able to test these yet because
we don't have an IPv6-capable user space rpc.statd yet.  I think they can wait
for 2.6.28.

---

Chuck Lever (16):
      lockd: change nlmclnt_grant() to take a "struct sockaddr *"
      lockd: Adjust nlmsvc_lookup_host() to accomodate non-AF_INET
      lockd: Adjust nlmclnt_lookup_host() signature to accomodate non-AF_INET
      lockd: Support non-AF_INET addresses in nlm_lookup_host()
      lockd: Update nsm_find() to support non-AF_INET addresses
      lockd: Combine __nsm_find() and nsm_find().
      lockd: Support AF_INET6 when hashing addresses in nlm_lookup_host
      lockd: Teach nlm_cmp_addr() to support AF_INET6 addresses
      NSM: Use sockaddr_storage for sm_addr field
      lockd: Use sockaddr_storage for h_saddr field
      lockd: Use sockaddr_storage + length for h_addr field
      SUNRPC: Make svc_addr's argument a constant
      lockd: Add address family-agnostic helper for zeroing the port number
      lockd: Specify address family for source address
      lockd: address-family independent printable addresses
      lockd: Pass "struct sockaddr *" to new failover-by-IP function


 fs/lockd/clntlock.c         |    9 +
 fs/lockd/host.c             |  341 +++++++++++++++++++++++++++++++------------
 fs/lockd/mon.c              |    2 
 fs/lockd/svc4proc.c         |    2 
 fs/lockd/svcproc.c          |    2 
 fs/lockd/svcsubs.c          |   39 +++--
 fs/nfsd/nfsctl.c            |   15 +-
 include/linux/lockd/lockd.h |   99 +++++++++++-
 include/linux/sunrpc/svc.h  |    6 -
 9 files changed, 379 insertions(+), 136 deletions(-)

-- 
Chuck Lever
chu ckl eve rat ora cle dot com

[PATCH 01/16] lockd: Pass "struct sockaddr *" to new failover-by-IP function

[Chuck Lever]

Pass a more generic socket address type to nlmsvc_unlock_all_by_ip() to
allow for future support of IPv6.  Also provide additional sanity
checking in failover_unlock_ip() when constructing the server's IP
address.

As an added bonus, provide clean kerneldoc comments on related NLM
interfaces which were recently added.

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

 fs/lockd/svcsubs.c          |   39 +++++++++++++++++++++++++--------------
 fs/nfsd/nfsctl.c            |   15 ++++++++++-----
 include/linux/lockd/lockd.h |    2 +-
 3 files changed, 36 insertions(+), 20 deletions(-)


diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index d1c48b5..723b6d5 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -373,18 +373,18 @@ nlmsvc_free_host_resources(struct nlm_host *host)
 	}
 }
 
-/*
- * Remove all locks held for clients
+/**
+ * nlmsvc_invalidate_all - remove all locks held for clients
+ *
+ * Release all locks held by NFS clients.  Previously, the code
+ * would call nlmsvc_free_host_resources for each client in turn,
+ * which is about as inefficient as it gets.
+ *
+ * Now we just do it once in nlm_traverse_files.
  */
 void
 nlmsvc_invalidate_all(void)
 {
-	/* Release all locks held by NFS clients.
-	 * Previously, the code would call
-	 * nlmsvc_free_host_resources for each client in
-	 * turn, which is about as inefficient as it gets.
-	 * Now we just do it once in nlm_traverse_files.
-	 */
 	nlm_traverse_files(NULL, nlmsvc_is_client, NULL);
 }
 
@@ -396,6 +396,12 @@ nlmsvc_match_sb(void *datap, struct nlm_file *file)
 	return sb == file->f_file->f_path.mnt->mnt_sb;
 }
 
+/**
+ * nlmsvc_unlock_all_by_sb - release locks held on this file system
+ * @sb: super block
+ *
+ * Release all locks held by clients accessing this file system.
+ */
 int
 nlmsvc_unlock_all_by_sb(struct super_block *sb)
 {
@@ -409,17 +415,22 @@ EXPORT_SYMBOL_GPL(nlmsvc_unlock_all_by_sb);
 static int
 nlmsvc_match_ip(void *datap, struct nlm_host *host)
 {
-	__be32 *server_addr = datap;
-
-	return host->h_saddr.sin_addr.s_addr == *server_addr;
+	return nlm_cmp_addr(&host->h_saddr, datap);
 }
 
+/**
+ * nlmsvc_unlock_all_by_ip - release local locks by IP address
+ * @server_addr: server's IP address as seen by clients
+ *
+ * Release all locks held by clients accessing this host
+ * via the passed in IP address.
+ */
 int
-nlmsvc_unlock_all_by_ip(__be32 server_addr)
+nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr)
 {
 	int ret;
-	ret = nlm_traverse_files(&server_addr, nlmsvc_match_ip, NULL);
-	return ret ? -EIO : 0;
 
+	ret = nlm_traverse_files(server_addr, nlmsvc_match_ip, NULL);
+	return ret ? -EIO : 0;
 }
 EXPORT_SYMBOL_GPL(nlmsvc_unlock_all_by_ip);
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 5ac00c4..5b4a412 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -310,9 +310,12 @@ static ssize_t write_getfd(struct file *file, char *buf, size_t size)
 
 static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size)
 {
-	__be32 server_ip;
-	char *fo_path, c;
+	struct sockaddr_in sin = {
+		.sin_family	= AF_INET,
+	};
 	int b1, b2, b3, b4;
+	char c;
+	char *fo_path;
 
 	/* sanity check */
 	if (size == 0)
@@ -326,11 +329,13 @@ static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size)
 		return -EINVAL;
 
 	/* get ipv4 address */
-	if (sscanf(fo_path, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) != 4)
+	if (sscanf(fo_path, NIPQUAD_FMT "%c", &b1, &b2, &b3, &b4, &c) != 4)
+		return -EINVAL;
+	if (b1 > 255 || b2 > 255 || b3 > 255 || b4 > 255)
 		return -EINVAL;
-	server_ip = htonl((((((b1<<8)|b2)<<8)|b3)<<8)|b4);
+	sin.sin_addr.s_addr = htonl((b1 << 24) | (b2 << 16) | (b3 << 8) | b4);
 
-	return nlmsvc_unlock_all_by_ip(server_ip);
+	return nlmsvc_unlock_all_by_ip((struct sockaddr *)&sin);
 }
 
 static ssize_t failover_unlock_fs(struct file *file, char *buf, size_t size)
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 102d928..a6d1215 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -224,7 +224,7 @@ void		  nlmsvc_invalidate_all(void);
  * Cluster failover support
  */
 int           nlmsvc_unlock_all_by_sb(struct super_block *sb);
-int           nlmsvc_unlock_all_by_ip(__be32 server_addr);
+int           nlmsvc_unlock_all_by_ip(struct sockaddr *server_addr);
 
 static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
 {

[PATCH 02/16] lockd: address-family independent printable addresses

[Chuck Lever]

Knowing which source address is used for communicating with remote NLM
services can be helpful for debugging configuration problems on hosts
with multiple addresses.

Keep the dprintk debugging here, but adapt it so it displays AF_INET6
addresses properly.  There are also a couple of dprintk clean-ups as
well.

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

 fs/lockd/host.c             |   79 +++++++++++++++++++++++++++++++------------
 include/linux/lockd/lockd.h |    3 ++
 2 files changed, 60 insertions(+), 22 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index a17664c..350d757 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -11,6 +11,7 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/in.h>
+#include <linux/in6.h>
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/svc.h>
 #include <linux/lockd/lockd.h>
@@ -38,6 +39,28 @@ static struct nsm_handle *	nsm_find(const struct sockaddr_in *sin,
 					 const char *hostname,
 					 unsigned int hostname_len);
 
+static void nlm_display_address(const struct sockaddr *sap,
+				char *buf, const size_t len)
+{
+	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+	const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+
+	switch (sap->sa_family) {
+	case AF_UNSPEC:
+		snprintf(buf, len, "unspecified");
+		break;
+	case AF_INET:
+		snprintf(buf, len, NIPQUAD_FMT, NIPQUAD(sin->sin_addr.s_addr));
+		break;
+	case AF_INET6:
+		snprintf(buf, len, NIP6_FMT, NIP6(sin6->sin6_addr));
+		break;
+	default:
+		snprintf(buf, len, "unsupported address family");
+		break;
+	}
+}
+
 /*
  * Common host lookup routine for server & client
  */
@@ -54,14 +77,10 @@ static struct nlm_host *nlm_lookup_host(int server,
 	struct nsm_handle *nsm = NULL;
 	int		hash;
 
-	dprintk("lockd: nlm_lookup_host("NIPQUAD_FMT"->"NIPQUAD_FMT
-			", p=%d, v=%u, my role=%s, name=%.*s)\n",
-			NIPQUAD(ssin->sin_addr.s_addr),
-			NIPQUAD(sin->sin_addr.s_addr), proto, version,
-			server? "server" : "client",
-			hostname_len,
-			hostname? hostname : "<none>");
-
+	dprintk("lockd: nlm_lookup_host(proto=%d, vers=%u,"
+			" my role is %s, hostname=%.*s)\n",
+			proto, version, server? "server" : "client",
+			hostname_len, hostname? hostname : "<none>");
 
 	hash = NLM_ADDRHASH(sin->sin_addr.s_addr);
 
@@ -101,22 +120,31 @@ static struct nlm_host *nlm_lookup_host(int server,
 		hlist_add_head(&host->h_hash, chain);
 
 		nlm_get_host(host);
+		dprintk("lockd: nlm_lookup_host found host %s (%s)\n",
+				host->h_name, host->h_addrbuf);
 		goto out;
 	}
-	if (nsm)
-		atomic_inc(&nsm->sm_count);
 
-	host = NULL;
-
-	/* Sadly, the host isn't in our hash table yet. See if
-	 * we have an NSM handle for it. If not, create one.
+	/*
+	 * The host wasn't in our hash table.  If we don't
+	 * have an NSM handle for it yet, create one.
 	 */
-	if (!nsm && !(nsm = nsm_find(sin, hostname, hostname_len)))
-		goto out;
+	if (nsm)
+		atomic_inc(&nsm->sm_count);
+	else {
+		host = NULL;
+		nsm = nsm_find(sin, hostname, hostname_len);
+		if (!nsm) {
+			dprintk("lockd: nlm_lookup_host failed; "
+				"no nsm handle\n");
+			goto out;
+		}
+	}
 
 	host = kzalloc(sizeof(*host), GFP_KERNEL);
 	if (!host) {
 		nsm_release(nsm);
+		dprintk("lockd: nlm_lookup_host failed; no memory\n");
 		goto out;
 	}
 	host->h_name	   = nsm->sm_name;
@@ -143,6 +171,15 @@ static struct nlm_host *nlm_lookup_host(int server,
 	INIT_LIST_HEAD(&host->h_reclaim);
 
 	nrhosts++;
+
+	nlm_display_address((struct sockaddr *)&host->h_addr,
+				host->h_addrbuf, sizeof(host->h_addrbuf));
+	nlm_display_address((struct sockaddr *)&host->h_saddr,
+				host->h_saddrbuf, sizeof(host->h_saddrbuf));
+
+	dprintk("lockd: nlm_lookup_host created host %s\n",
+			host->h_name);
+
 out:
 	mutex_unlock(&nlm_host_mutex);
 	return host;
@@ -207,9 +244,8 @@ nlm_bind_host(struct nlm_host *host)
 {
 	struct rpc_clnt	*clnt;
 
-	dprintk("lockd: nlm_bind_host("NIPQUAD_FMT"->"NIPQUAD_FMT")\n",
-			NIPQUAD(host->h_saddr.sin_addr),
-			NIPQUAD(host->h_addr.sin_addr));
+	dprintk("lockd: nlm_bind_host %s (%s), my addr=%s\n",
+			host->h_name, host->h_addrbuf, host->h_saddrbuf);
 
 	/* Lock host handle */
 	mutex_lock(&host->h_mutex);
@@ -221,7 +257,7 @@ nlm_bind_host(struct nlm_host *host)
 		if (time_after_eq(jiffies, host->h_nextrebind)) {
 			rpc_force_rebind(clnt);
 			host->h_nextrebind = jiffies + NLM_HOST_REBIND;
-			dprintk("lockd: next rebind in %ld jiffies\n",
+			dprintk("lockd: next rebind in %lu jiffies\n",
 					host->h_nextrebind - jiffies);
 		}
 	} else {
@@ -324,8 +360,7 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
 	struct nsm_handle *nsm;
 	struct nlm_host	*host;
 
-	dprintk("lockd: nlm_host_rebooted(%s, %u.%u.%u.%u)\n",
-			hostname, NIPQUAD(sin->sin_addr));
+	dprintk("lockd: nlm_host_rebooted(%s)\n", hostname);
 
 	/* Find the NSM handle for this peer */
 	if (!(nsm = __nsm_find(sin, hostname, hostname_len, 0)))
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index a6d1215..5774d6e 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -61,6 +61,9 @@ struct nlm_host {
 	struct list_head	h_granted;	/* Locks in GRANTED state */
 	struct list_head	h_reclaim;	/* Locks in RECLAIM state */
 	struct nsm_handle *	h_nsmhandle;	/* NSM status handle */
+
+	char			h_addrbuf[48],	/* address eyecatchers */
+				h_saddrbuf[48];
 };
 
 struct nsm_handle {

[PATCH 03/16] lockd: Specify address family for source address

[Chuck Lever]

Make sure an address family is specified for source addresses passed to
nlm_lookup_host().  nlm_lookup_host() will need this when it becomes
capable of dealing with AF_INET6 addresses.

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

 fs/lockd/host.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 350d757..618c631 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -215,10 +215,12 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
 				     const char *hostname,
 				     unsigned int hostname_len)
 {
-	struct sockaddr_in ssin = {0};
+	const struct sockaddr_in source = {
+		.sin_family	= AF_UNSPEC,
+	};
 
 	return nlm_lookup_host(0, sin, proto, version,
-			       hostname, hostname_len, &ssin);
+			       hostname, hostname_len, &source);
 }
 
 /*
@@ -228,12 +230,14 @@ struct nlm_host *
 nlmsvc_lookup_host(struct svc_rqst *rqstp,
 			const char *hostname, unsigned int hostname_len)
 {
-	struct sockaddr_in ssin = {0};
+	const struct sockaddr_in source = {
+		.sin_family	= AF_INET,
+		.sin_addr	= rqstp->rq_daddr.addr,
+	};
 
-	ssin.sin_addr = rqstp->rq_daddr.addr;
 	return nlm_lookup_host(1, svc_addr_in(rqstp),
 			       rqstp->rq_prot, rqstp->rq_vers,
-			       hostname, hostname_len, &ssin);
+			       hostname, hostname_len, &source);
 }
 
 /*

[PATCH 04/16] lockd: Add address family-agnostic helper for zeroing the port number

[Chuck Lever]

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

 fs/lockd/host.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 618c631..f87066d 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -39,6 +39,21 @@ static struct nsm_handle *	nsm_find(const struct sockaddr_in *sin,
 					 const char *hostname,
 					 unsigned int hostname_len);
 
+static void nlm_clear_port(struct sockaddr *sap)
+{
+	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		sin->sin_port = 0;
+		break;
+	case AF_INET6:
+		sin6->sin6_port = 0;
+		break;
+	}
+}
+
 static void nlm_display_address(const struct sockaddr *sap,
 				char *buf, const size_t len)
 {
@@ -149,7 +164,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 	}
 	host->h_name	   = nsm->sm_name;
 	host->h_addr       = *sin;
-	host->h_addr.sin_port = 0;	/* ouch! */
+	nlm_clear_port((struct sockaddr *)&host->h_addr);
 	host->h_saddr	   = *ssin;
 	host->h_version    = version;
 	host->h_proto      = proto;

[PATCH 05/16] SUNRPC: Make svc_addr's argument a constant

[Chuck Lever]

Clean up: Add extra type safety and squash a few compiler complaints (in
upcoming patches).

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

 include/linux/sunrpc/svc.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)


diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 05312e7..8798342 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -267,17 +267,17 @@ struct svc_rqst {
 /*
  * Rigorous type checking on sockaddr type conversions
  */
-static inline struct sockaddr_in *svc_addr_in(struct svc_rqst *rqst)
+static inline struct sockaddr_in *svc_addr_in(const struct svc_rqst *rqst)
 {
 	return (struct sockaddr_in *) &rqst->rq_addr;
 }
 
-static inline struct sockaddr_in6 *svc_addr_in6(struct svc_rqst *rqst)
+static inline struct sockaddr_in6 *svc_addr_in6(const struct svc_rqst *rqst)
 {
 	return (struct sockaddr_in6 *) &rqst->rq_addr;
 }
 
-static inline struct sockaddr *svc_addr(struct svc_rqst *rqst)
+static inline struct sockaddr *svc_addr(const struct svc_rqst *rqst)
 {
 	return (struct sockaddr *) &rqst->rq_addr;
 }

[PATCH 06/16] lockd: Use sockaddr_storage + length for h_addr field

[Chuck Lever]

To store larger addresses in the nlm_host structure, make h_addr a
sockaddr_storage, and add an address length field.

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

 fs/lockd/clntlock.c         |    2 +-
 fs/lockd/host.c             |   11 ++++++-----
 include/linux/lockd/lockd.h |   16 +++++++++++++++-
 3 files changed, 22 insertions(+), 7 deletions(-)


diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index 0b45fd3..0df5587 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -166,7 +166,7 @@ __be32 nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *lock
 		 */
 		if (fl_blocked->fl_u.nfs_fl.owner->pid != lock->svid)
 			continue;
-		if (!nlm_cmp_addr(&block->b_host->h_addr, addr))
+		if (!nlm_cmp_addr(nlm_addr_in(block->b_host), addr))
 			continue;
 		if (nfs_compare_fh(NFS_FH(fl_blocked->fl_file->f_path.dentry->d_inode) ,fh) != 0)
 			continue;
diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index f87066d..e2f6212 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -114,7 +114,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 	 */
 	chain = &nlm_hosts[hash];
 	hlist_for_each_entry(host, pos, chain, h_hash) {
-		if (!nlm_cmp_addr(&host->h_addr, sin))
+		if (!nlm_cmp_addr(nlm_addr_in(host), sin))
 			continue;
 
 		/* See if we have an NSM handle for this client */
@@ -163,8 +163,9 @@ static struct nlm_host *nlm_lookup_host(int server,
 		goto out;
 	}
 	host->h_name	   = nsm->sm_name;
-	host->h_addr       = *sin;
-	nlm_clear_port((struct sockaddr *)&host->h_addr);
+	memcpy(nlm_addr(host), sin, sizeof(*sin));
+	host->h_addrlen = sizeof(*sin);
+	nlm_clear_port(nlm_addr(host));
 	host->h_saddr	   = *ssin;
 	host->h_version    = version;
 	host->h_proto      = proto;
@@ -289,8 +290,8 @@ nlm_bind_host(struct nlm_host *host)
 		};
 		struct rpc_create_args args = {
 			.protocol	= host->h_proto,
-			.address	= (struct sockaddr *)&host->h_addr,
-			.addrsize	= sizeof(host->h_addr),
+			.address	= nlm_addr(host),
+			.addrsize	= host->h_addrlen,
 			.saddress	= (struct sockaddr *)&host->h_saddr,
 			.timeout	= &timeparms,
 			.servername	= host->h_name,
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 5774d6e..217b32a 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -38,7 +38,8 @@
  */
 struct nlm_host {
 	struct hlist_node	h_hash;		/* doubly linked list */
-	struct sockaddr_in	h_addr;		/* peer address */
+	struct sockaddr_storage	h_addr;		/* peer address */
+	size_t			h_addrlen;
 	struct sockaddr_in	h_saddr;	/* our address (optional) */
 	struct rpc_clnt	*	h_rpcclnt;	/* RPC client to talk to peer */
 	char *			h_name;		/* remote hostname */
@@ -76,6 +77,19 @@ struct nsm_handle {
 };
 
 /*
+ * Rigorous type checking on sockaddr type conversions
+ */
+static inline struct sockaddr_in *nlm_addr_in(const struct nlm_host *host)
+{
+	return (struct sockaddr_in *)&host->h_addr;
+}
+
+static inline struct sockaddr *nlm_addr(const struct nlm_host *host)
+{
+	return (struct sockaddr *)&host->h_addr;
+}
+
+/*
  * Map an fl_owner_t into a unique 32-bit "pid"
  */
 struct nlm_lockowner {

[PATCH 07/16] lockd: Use sockaddr_storage for h_saddr field

[Chuck Lever]

To store larger addresses in the nlm_host structure, make h_saddr a
sockaddr_storage.  And let's call it something more self-explanatory:
"saddr" could easily be mistaken for "server address".

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

 fs/lockd/host.c             |   12 ++++++------
 fs/lockd/svcsubs.c          |    2 +-
 include/linux/lockd/lockd.h |   14 ++++++++++++--
 3 files changed, 19 insertions(+), 9 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index e2f6212..290358b 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -127,7 +127,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 			continue;
 		if (host->h_server != server)
 			continue;
-		if (!nlm_cmp_addr(&host->h_saddr, ssin))
+		if (!nlm_cmp_addr(nlm_srcaddr_in(host), ssin))
 			continue;
 
 		/* Move to head of hash chain. */
@@ -166,7 +166,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 	memcpy(nlm_addr(host), sin, sizeof(*sin));
 	host->h_addrlen = sizeof(*sin);
 	nlm_clear_port(nlm_addr(host));
-	host->h_saddr	   = *ssin;
+	memcpy(nlm_srcaddr(host), ssin, sizeof(*ssin));
 	host->h_version    = version;
 	host->h_proto      = proto;
 	host->h_rpcclnt    = NULL;
@@ -190,8 +190,8 @@ static struct nlm_host *nlm_lookup_host(int server,
 
 	nlm_display_address((struct sockaddr *)&host->h_addr,
 				host->h_addrbuf, sizeof(host->h_addrbuf));
-	nlm_display_address((struct sockaddr *)&host->h_saddr,
-				host->h_saddrbuf, sizeof(host->h_saddrbuf));
+	nlm_display_address((struct sockaddr *)&host->h_srcaddr,
+				host->h_srcaddrbuf, sizeof(host->h_srcaddrbuf));
 
 	dprintk("lockd: nlm_lookup_host created host %s\n",
 			host->h_name);
@@ -265,7 +265,7 @@ nlm_bind_host(struct nlm_host *host)
 	struct rpc_clnt	*clnt;
 
 	dprintk("lockd: nlm_bind_host %s (%s), my addr=%s\n",
-			host->h_name, host->h_addrbuf, host->h_saddrbuf);
+			host->h_name, host->h_addrbuf, host->h_srcaddrbuf);
 
 	/* Lock host handle */
 	mutex_lock(&host->h_mutex);
@@ -292,7 +292,7 @@ nlm_bind_host(struct nlm_host *host)
 			.protocol	= host->h_proto,
 			.address	= nlm_addr(host),
 			.addrsize	= host->h_addrlen,
-			.saddress	= (struct sockaddr *)&host->h_saddr,
+			.saddress	= nlm_srcaddr(host),
 			.timeout	= &timeparms,
 			.servername	= host->h_name,
 			.program	= &nlm_program,
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index 723b6d5..64ba7df 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -415,7 +415,7 @@ EXPORT_SYMBOL_GPL(nlmsvc_unlock_all_by_sb);
 static int
 nlmsvc_match_ip(void *datap, struct nlm_host *host)
 {
-	return nlm_cmp_addr(&host->h_saddr, datap);
+	return nlm_cmp_addr(nlm_srcaddr_in(host), datap);
 }
 
 /**
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 217b32a..8834d31 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -40,7 +40,7 @@ struct nlm_host {
 	struct hlist_node	h_hash;		/* doubly linked list */
 	struct sockaddr_storage	h_addr;		/* peer address */
 	size_t			h_addrlen;
-	struct sockaddr_in	h_saddr;	/* our address (optional) */
+	struct sockaddr_storage	h_srcaddr;	/* our address (optional) */
 	struct rpc_clnt	*	h_rpcclnt;	/* RPC client to talk to peer */
 	char *			h_name;		/* remote hostname */
 	u32			h_version;	/* interface version */
@@ -64,7 +64,7 @@ struct nlm_host {
 	struct nsm_handle *	h_nsmhandle;	/* NSM status handle */
 
 	char			h_addrbuf[48],	/* address eyecatchers */
-				h_saddrbuf[48];
+				h_srcaddrbuf[48];
 };
 
 struct nsm_handle {
@@ -89,6 +89,16 @@ static inline struct sockaddr *nlm_addr(const struct nlm_host *host)
 	return (struct sockaddr *)&host->h_addr;
 }
 
+static inline struct sockaddr_in *nlm_srcaddr_in(const struct nlm_host *host)
+{
+	return (struct sockaddr_in *)&host->h_srcaddr;
+}
+
+static inline struct sockaddr *nlm_srcaddr(const struct nlm_host *host)
+{
+	return (struct sockaddr *)&host->h_srcaddr;
+}
+
 /*
  * Map an fl_owner_t into a unique 32-bit "pid"
  */

[PATCH 08/16] NSM: Use sockaddr_storage for sm_addr field

[Chuck Lever]

To store larger addresses in the nsm_handle structure, make sm_addr a
sockaddr_storage.

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

 fs/lockd/host.c             |    5 +++--
 fs/lockd/mon.c              |    2 +-
 include/linux/lockd/lockd.h |   13 ++++++++++++-
 3 files changed, 16 insertions(+), 4 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 290358b..c2efbfb 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -544,7 +544,7 @@ retry:
 			if (strlen(pos->sm_name) != hostname_len
 			 || memcmp(pos->sm_name, hostname, hostname_len))
 				continue;
-		} else if (!nlm_cmp_addr(&pos->sm_addr, sin))
+		} else if (!nlm_cmp_addr(nsm_addr_in(pos), sin))
 			continue;
 		atomic_inc(&pos->sm_count);
 		kfree(nsm);
@@ -564,7 +564,8 @@ retry:
 	if (nsm == NULL)
 		return NULL;
 
-	nsm->sm_addr = *sin;
+	memcpy(nsm_addr(nsm), sin, sizeof(*sin));
+	nsm->sm_addrlen = sizeof(*sin);
 	nsm->sm_name = (char *) (nsm + 1);
 	memcpy(nsm->sm_name, hostname, hostname_len);
 	nsm->sm_name[hostname_len] = '\0';
diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index e4d5635..4e7e958 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -51,7 +51,7 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res)
 
 	memset(&args, 0, sizeof(args));
 	args.mon_name = nsm->sm_name;
-	args.addr = nsm->sm_addr.sin_addr.s_addr;
+	args.addr = nsm_addr_in(nsm)->sin_addr.s_addr;
 	args.prog = NLM_PROGRAM;
 	args.vers = 3;
 	args.proc = NLMPROC_NSM_NOTIFY;
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 8834d31..a1eecf5 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -71,7 +71,8 @@ struct nsm_handle {
 	struct list_head	sm_link;
 	atomic_t		sm_count;
 	char *			sm_name;
-	struct sockaddr_in	sm_addr;
+	struct sockaddr_storage	sm_addr;
+	size_t			sm_addrlen;
 	unsigned int		sm_monitored : 1,
 				sm_sticky : 1;	/* don't unmonitor */
 };
@@ -99,6 +100,16 @@ static inline struct sockaddr *nlm_srcaddr(const struct nlm_host *host)
 	return (struct sockaddr *)&host->h_srcaddr;
 }
 
+static inline struct sockaddr_in *nsm_addr_in(const struct nsm_handle *handle)
+{
+	return (struct sockaddr_in *)&handle->sm_addr;
+}
+
+static inline struct sockaddr *nsm_addr(const struct nsm_handle *handle)
+{
+	return (struct sockaddr *)&handle->sm_addr;
+}
+
 /*
  * Map an fl_owner_t into a unique 32-bit "pid"
  */

[PATCH 09/16] lockd: Teach nlm_cmp_addr() to support AF_INET6 addresses

[Chuck Lever]

Update the nlm_cmp_addr() helper to support AF_INET6 as well as AF_INET
addresses.  New version takes two "struct sockaddr *" arguments instead of
"struct sockaddr_in *" arguments.

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

 fs/lockd/clntlock.c         |    3 ++-
 fs/lockd/host.c             |    6 +++---
 fs/lockd/svcsubs.c          |    2 +-
 include/linux/lockd/lockd.h |   38 ++++++++++++++++++++++++++++++++++----
 4 files changed, 40 insertions(+), 9 deletions(-)


diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index 0df5587..237224a 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -166,7 +166,8 @@ __be32 nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *lock
 		 */
 		if (fl_blocked->fl_u.nfs_fl.owner->pid != lock->svid)
 			continue;
-		if (!nlm_cmp_addr(nlm_addr_in(block->b_host), addr))
+		if (!nlm_cmp_addr(nlm_addr(block->b_host),
+					(struct sockaddr *)addr))
 			continue;
 		if (nfs_compare_fh(NFS_FH(fl_blocked->fl_file->f_path.dentry->d_inode) ,fh) != 0)
 			continue;
diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index c2efbfb..cd3f663 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -114,7 +114,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 	 */
 	chain = &nlm_hosts[hash];
 	hlist_for_each_entry(host, pos, chain, h_hash) {
-		if (!nlm_cmp_addr(nlm_addr_in(host), sin))
+		if (!nlm_cmp_addr(nlm_addr(host), (struct sockaddr *)sin))
 			continue;
 
 		/* See if we have an NSM handle for this client */
@@ -127,7 +127,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 			continue;
 		if (host->h_server != server)
 			continue;
-		if (!nlm_cmp_addr(nlm_srcaddr_in(host), ssin))
+		if (!nlm_cmp_addr(nlm_srcaddr(host), (struct sockaddr *)ssin))
 			continue;
 
 		/* Move to head of hash chain. */
@@ -544,7 +544,7 @@ retry:
 			if (strlen(pos->sm_name) != hostname_len
 			 || memcmp(pos->sm_name, hostname, hostname_len))
 				continue;
-		} else if (!nlm_cmp_addr(nsm_addr_in(pos), sin))
+		} else if (!nlm_cmp_addr(nsm_addr(pos), (struct sockaddr *)sin))
 			continue;
 		atomic_inc(&pos->sm_count);
 		kfree(nsm);
diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
index 64ba7df..373ec0c 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -415,7 +415,7 @@ EXPORT_SYMBOL_GPL(nlmsvc_unlock_all_by_sb);
 static int
 nlmsvc_match_ip(void *datap, struct nlm_host *host)
 {
-	return nlm_cmp_addr(nlm_srcaddr_in(host), datap);
+	return nlm_cmp_addr(nlm_srcaddr(host), datap);
 }
 
 /**
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index a1eecf5..6af3b05 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -12,6 +12,8 @@
 #ifdef __KERNEL__
 
 #include <linux/in.h>
+#include <linux/in6.h>
+#include <net/ipv6.h>
 #include <linux/fs.h>
 #include <linux/kref.h>
 #include <linux/utsname.h>
@@ -269,13 +271,41 @@ static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
 	return file->f_file->f_path.dentry->d_inode;
 }
 
+static inline int __nlm_cmp_addr4(const struct sockaddr *sap1,
+				  const struct sockaddr *sap2)
+{
+	const struct sockaddr_in *sin1 = (const struct sockaddr_in *)sap1;
+	const struct sockaddr_in *sin2 = (const struct sockaddr_in *)sap2;
+	return sin1->sin_addr.s_addr == sin2->sin_addr.s_addr;
+}
+
+static inline int __nlm_cmp_addr6(const struct sockaddr *sap1,
+				  const struct sockaddr *sap2)
+{
+	const struct sockaddr_in6 *sin1 = (const struct sockaddr_in6 *)sap1;
+	const struct sockaddr_in6 *sin2 = (const struct sockaddr_in6 *)sap2;
+	return ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr);
+}
+
 /*
- * Compare two host addresses (needs modifying for ipv6)
+ * Compare two host addresses
+ *
+ * Return TRUE if the addresses are the same; otherwise FALSE.
  */
-static inline int nlm_cmp_addr(const struct sockaddr_in *sin1,
-			       const struct sockaddr_in *sin2)
+static inline int nlm_cmp_addr(const struct sockaddr *sap1,
+			       const struct sockaddr *sap2)
 {
-	return sin1->sin_addr.s_addr == sin2->sin_addr.s_addr;
+	if (sap1->sa_family != sap2->sa_family)
+		return 0;
+
+	switch (sap1->sa_family) {
+	case AF_INET:
+		return __nlm_cmp_addr4(sap1, sap2);
+	case AF_INET6:
+		return __nlm_cmp_addr6(sap1, sap2);
+	default:
+		return 0;
+	}
 }
 
 /*

[PATCH 10/16] lockd: Support AF_INET6 when hashing addresses in nlm_lookup_host

[Chuck Lever]

Adopt an approach similar to the RPC server's auth cache (from Aurelien
Charbon and Brian Haley).

Note the NLM's existing IPv4 address hash function has the same issue
with correctness on little-endian systems as the original IPv4 auth cache
hash function, so I've replaced it with a hash function similar to the new
auth cache hash function.

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

 fs/lockd/host.c |   49 +++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 43 insertions(+), 6 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index cd3f663..b10a6fe 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -21,7 +21,6 @@
 
 #define NLMDBG_FACILITY		NLMDBG_HOSTCACHE
 #define NLM_HOST_NRHASH		32
-#define NLM_ADDRHASH(addr)	(ntohl(addr) & (NLM_HOST_NRHASH-1))
 #define NLM_HOST_REBIND		(60 * HZ)
 #define NLM_HOST_EXPIRE		(300 * HZ)
 #define NLM_HOST_COLLECT	(120 * HZ)
@@ -39,6 +38,48 @@ static struct nsm_handle *	nsm_find(const struct sockaddr_in *sin,
 					 const char *hostname,
 					 unsigned int hostname_len);
 
+/*
+ * Hash function must work well on big- and little-endian platforms
+ */
+static unsigned int __nlm_hash32(const __be32 n)
+{
+	unsigned int hash = (__force u32)n ^ ((__force u32)n >> 16);
+	return hash ^ (hash >> 8);
+}
+
+static unsigned int __nlm_hash_addr4(const struct sockaddr *sap)
+{
+	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+	return __nlm_hash32(sin->sin_addr.s_addr);
+}
+
+static unsigned int __nlm_hash_addr6(const struct sockaddr *sap)
+{
+	const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+	const struct in6_addr addr = sin6->sin6_addr;
+	return __nlm_hash32(addr.s6_addr32[0]) ^
+	       __nlm_hash32(addr.s6_addr32[1]) ^
+	       __nlm_hash32(addr.s6_addr32[2]) ^
+	       __nlm_hash32(addr.s6_addr32[3]);
+}
+
+static unsigned int nlm_hash_address(const struct sockaddr *sap)
+{
+	unsigned int hash;
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		hash = __nlm_hash_addr4(sap);
+		break;
+	case AF_INET6:
+		hash = __nlm_hash_addr6(sap);
+		break;
+	default:
+		hash = 0;
+	}
+	return hash & (NLM_HOST_NRHASH - 1);
+}
+
 static void nlm_clear_port(struct sockaddr *sap)
 {
 	struct sockaddr_in *sin = (struct sockaddr_in *)sap;
@@ -90,16 +131,12 @@ static struct nlm_host *nlm_lookup_host(int server,
 	struct hlist_node *pos;
 	struct nlm_host	*host;
 	struct nsm_handle *nsm = NULL;
-	int		hash;
 
 	dprintk("lockd: nlm_lookup_host(proto=%d, vers=%u,"
 			" my role is %s, hostname=%.*s)\n",
 			proto, version, server? "server" : "client",
 			hostname_len, hostname? hostname : "<none>");
 
-	hash = NLM_ADDRHASH(sin->sin_addr.s_addr);
-
-	/* Lock hash table */
 	mutex_lock(&nlm_host_mutex);
 
 	if (time_after_eq(jiffies, next_gc))
@@ -112,7 +149,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 	 * different NLM rpc_clients into one single nlm_host object.
 	 * This would allow us to have one nlm_host per address.
 	 */
-	chain = &nlm_hosts[hash];
+	chain = &nlm_hosts[nlm_hash_address((struct sockaddr *)sin)];
 	hlist_for_each_entry(host, pos, chain, h_hash) {
 		if (!nlm_cmp_addr(nlm_addr(host), (struct sockaddr *)sin))
 			continue;

[PATCH 11/16] lockd: Combine __nsm_find() and nsm_find().

[Chuck Lever]

Clean up: Having two separate functions doesn't add clarity, so
eliminate one of them.  Use contemporary kernel coding conventions
where appropriate.

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

 fs/lockd/host.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index b10a6fe..808b67c 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -30,13 +30,11 @@ static unsigned long		next_gc;
 static int			nrhosts;
 static DEFINE_MUTEX(nlm_host_mutex);
 
-
 static void			nlm_gc_hosts(void);
-static struct nsm_handle *	__nsm_find(const struct sockaddr_in *,
-					const char *, unsigned int, int);
-static struct nsm_handle *	nsm_find(const struct sockaddr_in *sin,
-					 const char *hostname,
-					 unsigned int hostname_len);
+static struct nsm_handle	*nsm_find(const struct sockaddr_in *sin,
+						const char *hostname,
+						const unsigned int hostname_len,
+						const int create);
 
 /*
  * Hash function must work well on big- and little-endian platforms
@@ -185,7 +183,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 		atomic_inc(&nsm->sm_count);
 	else {
 		host = NULL;
-		nsm = nsm_find(sin, hostname, hostname_len);
+		nsm = nsm_find(sin, hostname, hostname_len, 1);
 		if (!nsm) {
 			dprintk("lockd: nlm_lookup_host failed; "
 				"no nsm handle\n");
@@ -419,8 +417,8 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
 
 	dprintk("lockd: nlm_host_rebooted(%s)\n", hostname);
 
-	/* Find the NSM handle for this peer */
-	if (!(nsm = __nsm_find(sin, hostname, hostname_len, 0)))
+	nsm = nsm_find(sin, hostname, hostname_len, 0);
+	if (!nsm)
 		return;
 
 	/* When reclaiming locks on this peer, make sure that
@@ -553,10 +551,10 @@ nlm_gc_hosts(void)
 static LIST_HEAD(nsm_handles);
 static DEFINE_SPINLOCK(nsm_lock);
 
-static struct nsm_handle *
-__nsm_find(const struct sockaddr_in *sin,
-		const char *hostname, unsigned int hostname_len,
-		int create)
+static struct nsm_handle *nsm_find(const struct sockaddr_in *sin,
+				   const char *hostname,
+				   const unsigned int hostname_len,
+				   const int create)
 {
 	struct nsm_handle *nsm = NULL;
 	struct nsm_handle *pos;
@@ -614,13 +612,6 @@ found:
 	return nsm;
 }
 
-static struct nsm_handle *
-nsm_find(const struct sockaddr_in *sin, const char *hostname,
-	 unsigned int hostname_len)
-{
-	return __nsm_find(sin, hostname, hostname_len, 1);
-}
-
 /*
  * Release an NSM handle
  */

[PATCH 12/16] lockd: Update nsm_find() to support non-AF_INET addresses

[Chuck Lever]

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

 fs/lockd/host.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 808b67c..0e749e6 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -31,9 +31,10 @@ static int			nrhosts;
 static DEFINE_MUTEX(nlm_host_mutex);
 
 static void			nlm_gc_hosts(void);
-static struct nsm_handle	*nsm_find(const struct sockaddr_in *sin,
+static struct nsm_handle	*nsm_find(const struct sockaddr *sap,
+						const size_t salen,
 						const char *hostname,
-						const unsigned int hostname_len,
+						const size_t hostname_len,
 						const int create);
 
 /*
@@ -183,7 +184,8 @@ static struct nlm_host *nlm_lookup_host(int server,
 		atomic_inc(&nsm->sm_count);
 	else {
 		host = NULL;
-		nsm = nsm_find(sin, hostname, hostname_len, 1);
+		nsm = nsm_find((struct sockaddr *)sin, sizeof(*sin),
+				hostname, hostname_len, 1);
 		if (!nsm) {
 			dprintk("lockd: nlm_lookup_host failed; "
 				"no nsm handle\n");
@@ -417,7 +419,8 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
 
 	dprintk("lockd: nlm_host_rebooted(%s)\n", hostname);
 
-	nsm = nsm_find(sin, hostname, hostname_len, 0);
+	nsm = nsm_find((struct sockaddr *)sin, sizeof(*sin),
+			hostname, hostname_len, 0);
 	if (!nsm)
 		return;
 
@@ -551,22 +554,23 @@ nlm_gc_hosts(void)
 static LIST_HEAD(nsm_handles);
 static DEFINE_SPINLOCK(nsm_lock);
 
-static struct nsm_handle *nsm_find(const struct sockaddr_in *sin,
+static struct nsm_handle *nsm_find(const struct sockaddr *sap,
+				   const size_t salen,
 				   const char *hostname,
-				   const unsigned int hostname_len,
+				   const size_t hostname_len,
 				   const int create)
 {
 	struct nsm_handle *nsm = NULL;
 	struct nsm_handle *pos;
 
-	if (!sin)
+	if (!sap)
 		return NULL;
 
 	if (hostname && memchr(hostname, '/', hostname_len) != NULL) {
 		if (printk_ratelimit()) {
 			printk(KERN_WARNING "Invalid hostname \"%.*s\" "
 					    "in NFS lock request\n",
-				hostname_len, hostname);
+						(int)hostname_len, hostname);
 		}
 		return NULL;
 	}
@@ -579,7 +583,7 @@ retry:
 			if (strlen(pos->sm_name) != hostname_len
 			 || memcmp(pos->sm_name, hostname, hostname_len))
 				continue;
-		} else if (!nlm_cmp_addr(nsm_addr(pos), (struct sockaddr *)sin))
+		} else if (!nlm_cmp_addr(nsm_addr(pos), sap))
 			continue;
 		atomic_inc(&pos->sm_count);
 		kfree(nsm);
@@ -599,8 +603,8 @@ retry:
 	if (nsm == NULL)
 		return NULL;
 
-	memcpy(nsm_addr(nsm), sin, sizeof(*sin));
-	nsm->sm_addrlen = sizeof(*sin);
+	memcpy(nsm_addr(nsm), sap, salen);
+	nsm->sm_addrlen = salen;
 	nsm->sm_name = (char *) (nsm + 1);
 	memcpy(nsm->sm_name, hostname, hostname_len);
 	nsm->sm_name[hostname_len] = '\0';

[PATCH 13/16] lockd: Support non-AF_INET addresses in nlm_lookup_host()

[Chuck Lever]

Make nlm_lookup_host() agnostic to address families.

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

 fs/lockd/host.c |   88 +++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 56 insertions(+), 32 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 0e749e6..5a8114d 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -37,6 +37,21 @@ static struct nsm_handle	*nsm_find(const struct sockaddr *sap,
 						const size_t hostname_len,
 						const int create);
 
+#define NLM_SERVER	(0)
+#define NLM_CLIENT	(1)
+
+struct nlm_lookup_host_info {
+	const int		server;		/* search for server|client */
+	const struct sockaddr	*sap;		/* address to search for */
+	const size_t		salen;		/* it's length */
+	const unsigned short	protocol;	/* transport to search for*/
+	const u32		version;	/* NLM version to search for */
+	const char		*hostname;	/* remote's hostname */
+	const size_t		hostname_len;	/* it's length */
+	const struct sockaddr	*src_sap;	/* our address (optional) */
+	const size_t		src_len;	/* it's length */
+};
+
 /*
  * Hash function must work well on big- and little-endian platforms
  */
@@ -119,23 +134,13 @@ static void nlm_display_address(const struct sockaddr *sap,
 /*
  * Common host lookup routine for server & client
  */
-static struct nlm_host *nlm_lookup_host(int server,
-					const struct sockaddr_in *sin,
-					int proto, u32 version,
-					const char *hostname,
-					unsigned int hostname_len,
-					const struct sockaddr_in *ssin)
+static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 {
 	struct hlist_head *chain;
 	struct hlist_node *pos;
 	struct nlm_host	*host;
 	struct nsm_handle *nsm = NULL;
 
-	dprintk("lockd: nlm_lookup_host(proto=%d, vers=%u,"
-			" my role is %s, hostname=%.*s)\n",
-			proto, version, server? "server" : "client",
-			hostname_len, hostname? hostname : "<none>");
-
 	mutex_lock(&nlm_host_mutex);
 
 	if (time_after_eq(jiffies, next_gc))
@@ -148,22 +153,22 @@ static struct nlm_host *nlm_lookup_host(int server,
 	 * different NLM rpc_clients into one single nlm_host object.
 	 * This would allow us to have one nlm_host per address.
 	 */
-	chain = &nlm_hosts[nlm_hash_address((struct sockaddr *)sin)];
+	chain = &nlm_hosts[nlm_hash_address(ni->sap)];
 	hlist_for_each_entry(host, pos, chain, h_hash) {
-		if (!nlm_cmp_addr(nlm_addr(host), (struct sockaddr *)sin))
+		if (!nlm_cmp_addr(nlm_addr(host), ni->sap))
 			continue;
 
 		/* See if we have an NSM handle for this client */
 		if (!nsm)
 			nsm = host->h_nsmhandle;
 
-		if (host->h_proto != proto)
+		if (host->h_proto != ni->protocol)
 			continue;
-		if (host->h_version != version)
+		if (host->h_version != ni->version)
 			continue;
-		if (host->h_server != server)
+		if (host->h_server != ni->server)
 			continue;
-		if (!nlm_cmp_addr(nlm_srcaddr(host), (struct sockaddr *)ssin))
+		if (!nlm_cmp_addr(nlm_srcaddr(host), ni->src_sap))
 			continue;
 
 		/* Move to head of hash chain. */
@@ -184,8 +189,8 @@ static struct nlm_host *nlm_lookup_host(int server,
 		atomic_inc(&nsm->sm_count);
 	else {
 		host = NULL;
-		nsm = nsm_find((struct sockaddr *)sin, sizeof(*sin),
-				hostname, hostname_len, 1);
+		nsm = nsm_find(ni->sap, ni->salen,
+					ni->hostname, ni->hostname_len, 1);
 		if (!nsm) {
 			dprintk("lockd: nlm_lookup_host failed; "
 				"no nsm handle\n");
@@ -200,12 +205,12 @@ static struct nlm_host *nlm_lookup_host(int server,
 		goto out;
 	}
 	host->h_name	   = nsm->sm_name;
-	memcpy(nlm_addr(host), sin, sizeof(*sin));
-	host->h_addrlen = sizeof(*sin);
+	memcpy(nlm_addr(host), ni->sap, ni->salen);
+	host->h_addrlen = ni->salen;
 	nlm_clear_port(nlm_addr(host));
-	memcpy(nlm_srcaddr(host), ssin, sizeof(*ssin));
-	host->h_version    = version;
-	host->h_proto      = proto;
+	memcpy(nlm_srcaddr(host), ni->src_sap, ni->src_len);
+	host->h_version    = ni->version;
+	host->h_proto      = ni->protocol;
 	host->h_rpcclnt    = NULL;
 	mutex_init(&host->h_mutex);
 	host->h_nextrebind = jiffies + NLM_HOST_REBIND;
@@ -216,7 +221,7 @@ static struct nlm_host *nlm_lookup_host(int server,
 	host->h_state      = 0;			/* pseudo NSM state */
 	host->h_nsmstate   = 0;			/* real NSM state */
 	host->h_nsmhandle  = nsm;
-	host->h_server	   = server;
+	host->h_server	   = ni->server;
 	hlist_add_head(&host->h_hash, chain);
 	INIT_LIST_HEAD(&host->h_lockowners);
 	spin_lock_init(&host->h_lock);
@@ -268,12 +273,22 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
 				     const char *hostname,
 				     unsigned int hostname_len)
 {
-	const struct sockaddr_in source = {
-		.sin_family	= AF_UNSPEC,
+	const struct sockaddr source = {
+		.sa_family	= AF_UNSPEC,
+	};
+	struct nlm_lookup_host_info ni = {
+		.server		= NLM_SERVER,
+		.sap		= (struct sockaddr *)sin,
+		.salen		= sizeof(*sin),
+		.protocol	= proto,
+		.version	= version,
+		.hostname	= hostname,
+		.hostname_len	= hostname_len,
+		.src_sap	= &source,
+		.src_len	= sizeof(source),
 	};
 
-	return nlm_lookup_host(0, sin, proto, version,
-			       hostname, hostname_len, &source);
+	return nlm_lookup_host(&ni);
 }
 
 /*
@@ -287,10 +302,19 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
 		.sin_family	= AF_INET,
 		.sin_addr	= rqstp->rq_daddr.addr,
 	};
+	struct nlm_lookup_host_info ni = {
+		.server		= NLM_CLIENT,
+		.sap		= svc_addr(rqstp),
+		.salen		= rqstp->rq_addrlen,
+		.protocol	= rqstp->rq_prot,
+		.version	= rqstp->rq_vers,
+		.hostname	= hostname,
+		.hostname_len	= hostname_len,
+		.src_sap	= (struct sockaddr *)&source,
+		.src_len	= sizeof(source),
+	};
 
-	return nlm_lookup_host(1, svc_addr_in(rqstp),
-			       rqstp->rq_prot, rqstp->rq_vers,
-			       hostname, hostname_len, &source);
+	return nlm_lookup_host(&ni);
 }
 
 /*

[PATCH 14/16] lockd: Adjust nlmclnt_lookup_host() signature to accomodate non-AF_INET

[Chuck Lever]

Pass a struct sockaddr * and a length to nlmclnt_lookup_host() to
accomodate non-AF_INET family addresses.

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

 fs/lockd/clntlock.c         |    5 ++---
 fs/lockd/host.c             |   34 ++++++++++++++++++++++++----------
 include/linux/lockd/lockd.h |    9 +++++----
 3 files changed, 31 insertions(+), 17 deletions(-)


diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index 237224a..9eaf306 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -58,10 +58,9 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
 	if (status < 0)
 		return ERR_PTR(status);
 
-	host = nlmclnt_lookup_host((struct sockaddr_in *)nlm_init->address,
+	host = nlmclnt_lookup_host(nlm_init->address, nlm_init->addrlen,
 				   nlm_init->protocol, nlm_version,
-				   nlm_init->hostname,
-				   strlen(nlm_init->hostname));
+				   nlm_init->hostname);
 	if (host == NULL) {
 		lockd_down();
 		return ERR_PTR(-ENOLCK);
diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 5a8114d..3c7916d 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -265,29 +265,43 @@ nlm_destroy_host(struct nlm_host *host)
 	kfree(host);
 }
 
-/*
- * Find an NLM server handle in the cache. If there is none, create it.
+/**
+ * nlmclnt_lookup_host - Find an NLM host handle matching a remote server
+ * @sap: network address of server
+ * @salen: length of server address
+ * @protocol: transport protocol to use
+ * @version: NLM protocol version
+ * @hostname: '\0'-terminated hostname of server
+ *
+ * Returns an nlm_host structure that matches the passed-in
+ * [server address, transport protocol, NLM version, server hostname].
+ * If one doesn't already exist in the host cache, a new handle is
+ * created and returned.
  */
-struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
-				     int proto, u32 version,
-				     const char *hostname,
-				     unsigned int hostname_len)
+struct nlm_host *nlmclnt_lookup_host(const struct sockaddr *sap,
+				     const size_t salen,
+				     const unsigned short protocol,
+				     const u32 version, const char *hostname)
 {
 	const struct sockaddr source = {
 		.sa_family	= AF_UNSPEC,
 	};
 	struct nlm_lookup_host_info ni = {
 		.server		= NLM_SERVER,
-		.sap		= (struct sockaddr *)sin,
-		.salen		= sizeof(*sin),
-		.protocol	= proto,
+		.sap		= sap,
+		.salen		= salen,
+		.protocol	= protocol,
 		.version	= version,
 		.hostname	= hostname,
-		.hostname_len	= hostname_len,
+		.hostname_len	= strlen(hostname),
 		.src_sap	= &source,
 		.src_len	= sizeof(source),
 	};
 
+	dprintk("lockd: nlmclnt_lookup_host(host='%s', vers=%u, proto=%s)\n",
+			(hostname ? hostname : "<none>"), version,
+			(protocol == IPPROTO_UDP ? "udp" : "tcp"));
+
 	return nlm_lookup_host(&ni);
 }
 
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 6af3b05..6e8909d 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -214,10 +214,11 @@ void		  nlmclnt_next_cookie(struct nlm_cookie *);
 /*
  * Host cache
  */
-struct nlm_host  *nlmclnt_lookup_host(const struct sockaddr_in *sin,
-					int proto, u32 version,
-					const char *hostname,
-					unsigned int hostname_len);
+struct nlm_host  *nlmclnt_lookup_host(const struct sockaddr *sap,
+					const size_t salen,
+					const unsigned short protocol,
+					const u32 version,
+					const char *hostname);
 struct nlm_host  *nlmsvc_lookup_host(struct svc_rqst *, const char *,
 					unsigned int);
 struct rpc_clnt * nlm_bind_host(struct nlm_host *);

[PATCH 15/16] lockd: Adjust nlmsvc_lookup_host() to accomodate non-AF_INET

[Chuck Lever]

Fix up nlmsvc_lookup_host() to pass non-AF_INET source addresses to
nlm_lookup_host().

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

 fs/lockd/host.c             |   51 +++++++++++++++++++++++++++++++++++--------
 include/linux/lockd/lockd.h |    5 +++-
 2 files changed, 44 insertions(+), 12 deletions(-)


diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 3c7916d..73e234a 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -305,29 +305,60 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr *sap,
 	return nlm_lookup_host(&ni);
 }
 
-/*
- * Find an NLM client handle in the cache. If there is none, create it.
+/**
+ * nlmsvc_lookup_host - Find an NLM host handle matching a remote client
+ * @rqstp: incoming NLM request
+ * @hostname: name of client host
+ * @hostname_len: length of client hostname
+ *
+ * Returns an nlm_host structure that matches the [client address,
+ * transport protocol, NLM version, client hostname] of the passed-in
+ * NLM request.  If one doesn't already exist in the host cache, a
+ * new handle is created and returned.
+ *
+ * Manufacture a specific source address in case the local system
+ * services clients from multiple IP addresses.  The family of the
+ * address in rq_daddr is guaranteed to be the same as the family of
+ * the address in rq_addr, so it's safe to use the same family and
+ * length for our manufactured source address.
  */
-struct nlm_host *
-nlmsvc_lookup_host(struct svc_rqst *rqstp,
-			const char *hostname, unsigned int hostname_len)
+struct nlm_host *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
+				    const char *hostname,
+				    const size_t hostname_len)
 {
-	const struct sockaddr_in source = {
-		.sin_family	= AF_INET,
-		.sin_addr	= rqstp->rq_daddr.addr,
+	const struct sockaddr *sap = svc_addr(rqstp);
+	struct sockaddr_storage source = {
+		.ss_family	= sap->sa_family,
 	};
+	struct sockaddr_in *sin = (struct sockaddr_in *)&source;
+	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)&source;
 	struct nlm_lookup_host_info ni = {
 		.server		= NLM_CLIENT,
-		.sap		= svc_addr(rqstp),
+		.sap		= sap,
 		.salen		= rqstp->rq_addrlen,
 		.protocol	= rqstp->rq_prot,
 		.version	= rqstp->rq_vers,
 		.hostname	= hostname,
 		.hostname_len	= hostname_len,
 		.src_sap	= (struct sockaddr *)&source,
-		.src_len	= sizeof(source),
+		.src_len	= rqstp->rq_addrlen,
 	};
 
+	dprintk("lockd: %s(host='%.*s', vers=%u, proto=%s)\n", __func__,
+			hostname_len, hostname, rqstp->rq_vers,
+			(rqstp->rq_prot == IPPROTO_UDP ? "udp" : "tcp"));
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		sin->sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
+		break;
+	case AF_INET6:
+		ipv6_addr_copy(&sin6->sin6_addr, &rqstp->rq_daddr.addr6);
+		break;
+	default:
+		return NULL;
+	}
+
 	return nlm_lookup_host(&ni);
 }
 
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 6e8909d..e327914 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -219,8 +219,9 @@ struct nlm_host  *nlmclnt_lookup_host(const struct sockaddr *sap,
 					const unsigned short protocol,
 					const u32 version,
 					const char *hostname);
-struct nlm_host  *nlmsvc_lookup_host(struct svc_rqst *, const char *,
-					unsigned int);
+struct nlm_host  *nlmsvc_lookup_host(const struct svc_rqst *rqstp,
+					const char *hostname,
+					const size_t hostname_len);
 struct rpc_clnt * nlm_bind_host(struct nlm_host *);
 void		  nlm_rebind_host(struct nlm_host *);
 struct nlm_host * nlm_get_host(struct nlm_host *);

[PATCH 16/16] lockd: change nlmclnt_grant() to take a "struct sockaddr *"

[Chuck Lever]

Adjust the signature and callers of nlmclnt_grant() to pass a "struct
sockaddr *" instead of a "struct sockaddr_in *" in order to support IPv6
addresses.

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

 fs/lockd/clntlock.c         |    5 ++---
 fs/lockd/svc4proc.c         |    2 +-
 fs/lockd/svcproc.c          |    2 +-
 include/linux/lockd/lockd.h |    3 ++-
 4 files changed, 6 insertions(+), 6 deletions(-)


diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
index 9eaf306..2976bf0 100644
--- a/fs/lockd/clntlock.c
+++ b/fs/lockd/clntlock.c
@@ -141,7 +141,7 @@ int nlmclnt_block(struct nlm_wait *block, struct nlm_rqst *req, long timeout)
 /*
  * The server lockd has called us back to tell us the lock was granted
  */
-__be32 nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *lock)
+__be32 nlmclnt_grant(const struct sockaddr *addr, const struct nlm_lock *lock)
 {
 	const struct file_lock *fl = &lock->fl;
 	const struct nfs_fh *fh = &lock->fh;
@@ -165,8 +165,7 @@ __be32 nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *lock
 		 */
 		if (fl_blocked->fl_u.nfs_fl.owner->pid != lock->svid)
 			continue;
-		if (!nlm_cmp_addr(nlm_addr(block->b_host),
-					(struct sockaddr *)addr))
+		if (!nlm_cmp_addr(nlm_addr(block->b_host), addr))
 			continue;
 		if (nfs_compare_fh(NFS_FH(fl_blocked->fl_file->f_path.dentry->d_inode) ,fh) != 0)
 			continue;
diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 385437e..5cc8999 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -232,7 +232,7 @@ nlm4svc_proc_granted(struct svc_rqst *rqstp, struct nlm_args *argp,
 	resp->cookie = argp->cookie;
 
 	dprintk("lockd: GRANTED       called\n");
-	resp->status = nlmclnt_grant(svc_addr_in(rqstp), &argp->lock);
+	resp->status = nlmclnt_grant(svc_addr(rqstp), &argp->lock);
 	dprintk("lockd: GRANTED       status %d\n", ntohl(resp->status));
 	return rpc_success;
 }
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index 88379cc..c7b5889 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -262,7 +262,7 @@ nlmsvc_proc_granted(struct svc_rqst *rqstp, struct nlm_args *argp,
 	resp->cookie = argp->cookie;
 
 	dprintk("lockd: GRANTED       called\n");
-	resp->status = nlmclnt_grant(svc_addr_in(rqstp), &argp->lock);
+	resp->status = nlmclnt_grant(svc_addr(rqstp), &argp->lock);
 	dprintk("lockd: GRANTED       status %d\n", ntohl(resp->status));
 	return rpc_success;
 }
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index e327914..a48d87a 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -206,7 +206,8 @@ int		  nlm_async_reply(struct nlm_rqst *, u32, const struct rpc_call_ops *);
 struct nlm_wait * nlmclnt_prepare_block(struct nlm_host *host, struct file_lock *fl);
 void		  nlmclnt_finish_block(struct nlm_wait *block);
 int		  nlmclnt_block(struct nlm_wait *block, struct nlm_rqst *req, long timeout);
-__be32		  nlmclnt_grant(const struct sockaddr_in *addr, const struct nlm_lock *);
+__be32		  nlmclnt_grant(const struct sockaddr *addr,
+				const struct nlm_lock *lock);
 void		  nlmclnt_recovery(struct nlm_host *);
 int		  nlmclnt_reclaim(struct nlm_host *, struct file_lock *);
 void		  nlmclnt_next_cookie(struct nlm_cookie *);

Re: [PATCH 01/16] lockd: Pass "struct sockaddr *" to new failover-by-IP function

[J. Bruce Fields]

On Mon, Jun 30, 2008 at 06:58:14PM -0400, Chuck Lever wrote:
> Pass a more generic socket address type to nlmsvc_unlock_all_by_ip() to
> allow for future support of IPv6.  Also provide additional sanity
> checking in failover_unlock_ip() when constructing the server's IP
> address.
> 
> As an added bonus, provide clean kerneldoc comments on related NLM
> interfaces which were recently added.

Looks good to me, thanks--applied, with one minor change:

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/lockd/svcsubs.c          |   39 +++++++++++++++++++++++++--------------
>  fs/nfsd/nfsctl.c            |   15 ++++++++++-----
>  include/linux/lockd/lockd.h |    2 +-
>  3 files changed, 36 insertions(+), 20 deletions(-)
> 
> 
> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
> index d1c48b5..723b6d5 100644
> --- a/fs/lockd/svcsubs.c
> +++ b/fs/lockd/svcsubs.c
> @@ -373,18 +373,18 @@ nlmsvc_free_host_resources(struct nlm_host *host)
>  	}
>  }
>  
> -/*
> - * Remove all locks held for clients
> +/**
> + * nlmsvc_invalidate_all - remove all locks held for clients
> + *
> + * Release all locks held by NFS clients.  Previously, the code
> + * would call nlmsvc_free_host_resources for each client in turn,
> + * which is about as inefficient as it gets.
> + *
> + * Now we just do it once in nlm_traverse_files.
>   */
>  void
>  nlmsvc_invalidate_all(void)
>  {
> -	/* Release all locks held by NFS clients.
> -	 * Previously, the code would call
> -	 * nlmsvc_free_host_resources for each client in
> -	 * turn, which is about as inefficient as it gets.
> -	 * Now we just do it once in nlm_traverse_files.
> -	 */

Mind if I keep the "Previously..." part in the body of the function?
I'd rather the kerneldoc comments be about the interface and leave
implementation details to the body of the function.

Also, one more minor question--

> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 5ac00c4..5b4a412 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -310,9 +310,12 @@ static ssize_t write_getfd(struct file *file, char *buf, size_t size)
>  
>  static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size)
>  {
> -	__be32 server_ip;
> -	char *fo_path, c;
> +	struct sockaddr_in sin = {
> +		.sin_family	= AF_INET,
> +	};
>  	int b1, b2, b3, b4;
> +	char c;
> +	char *fo_path;
>  
>  	/* sanity check */
>  	if (size == 0)
> @@ -326,11 +329,13 @@ static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size)
>  		return -EINVAL;
>  
>  	/* get ipv4 address */
> -	if (sscanf(fo_path, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) != 4)
> +	if (sscanf(fo_path, NIPQUAD_FMT "%c", &b1, &b2, &b3, &b4, &c) != 4)
> +		return -EINVAL;
> +	if (b1 > 255 || b2 > 255 || b3 > 255 || b4 > 255)

Should b1, b2, b3, b4 be unsigned?

--b.

>  		return -EINVAL;
> -	server_ip = htonl((((((b1<<8)|b2)<<8)|b3)<<8)|b4);
> +	sin.sin_addr.s_addr = htonl((b1 << 24) | (b2 << 16) | (b3 << 8) | b4);
>  
> -	return nlmsvc_unlock_all_by_ip(server_ip);
> +	return nlmsvc_unlock_all_by_ip((struct sockaddr *)&sin);
>  }

Re: [PATCH 02/16] lockd: address-family independent printable addresses

[J. Bruce Fields]

On Mon, Jun 30, 2008 at 06:58:22PM -0400, Chuck Lever wrote:
> Knowing which source address is used for communicating with remote NLM
> services can be helpful for debugging configuration problems on hosts
> with multiple addresses.
> 
> Keep the dprintk debugging here, but adapt it so it displays AF_INET6
> addresses properly.  There are also a couple of dprintk clean-ups as
> well.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/lockd/host.c             |   79 +++++++++++++++++++++++++++++++------------
>  include/linux/lockd/lockd.h |    3 ++
>  2 files changed, 60 insertions(+), 22 deletions(-)
> 
> 
> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
> index a17664c..350d757 100644
> --- a/fs/lockd/host.c
> +++ b/fs/lockd/host.c
> @@ -11,6 +11,7 @@
>  #include <linux/types.h>
>  #include <linux/slab.h>
>  #include <linux/in.h>
> +#include <linux/in6.h>
>  #include <linux/sunrpc/clnt.h>
>  #include <linux/sunrpc/svc.h>
>  #include <linux/lockd/lockd.h>
> @@ -38,6 +39,28 @@ static struct nsm_handle *	nsm_find(const struct sockaddr_in *sin,
>  					 const char *hostname,
>  					 unsigned int hostname_len);
>  
> +static void nlm_display_address(const struct sockaddr *sap,
> +				char *buf, const size_t len)
> +{
> +	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
> +	const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
> +
> +	switch (sap->sa_family) {
> +	case AF_UNSPEC:
> +		snprintf(buf, len, "unspecified");
> +		break;
> +	case AF_INET:
> +		snprintf(buf, len, NIPQUAD_FMT, NIPQUAD(sin->sin_addr.s_addr));
> +		break;
> +	case AF_INET6:
> +		snprintf(buf, len, NIP6_FMT, NIP6(sin6->sin6_addr));
> +		break;
> +	default:
> +		snprintf(buf, len, "unsupported address family");
> +		break;
> +	}
> +}

Is there code we could reuse from someplace else?

> +
>  /*
>   * Common host lookup routine for server & client
>   */
> @@ -54,14 +77,10 @@ static struct nlm_host *nlm_lookup_host(int server,
>  	struct nsm_handle *nsm = NULL;
>  	int		hash;
>  
> -	dprintk("lockd: nlm_lookup_host("NIPQUAD_FMT"->"NIPQUAD_FMT
> -			", p=%d, v=%u, my role=%s, name=%.*s)\n",
> -			NIPQUAD(ssin->sin_addr.s_addr),
> -			NIPQUAD(sin->sin_addr.s_addr), proto, version,
> -			server? "server" : "client",
> -			hostname_len,
> -			hostname? hostname : "<none>");
> -
> +	dprintk("lockd: nlm_lookup_host(proto=%d, vers=%u,"
> +			" my role is %s, hostname=%.*s)\n",
> +			proto, version, server? "server" : "client",
> +			hostname_len, hostname? hostname : "<none>");
>  
>  	hash = NLM_ADDRHASH(sin->sin_addr.s_addr);
>  
> @@ -101,22 +120,31 @@ static struct nlm_host *nlm_lookup_host(int server,
>  		hlist_add_head(&host->h_hash, chain);
>  
>  		nlm_get_host(host);
> +		dprintk("lockd: nlm_lookup_host found host %s (%s)\n",
> +				host->h_name, host->h_addrbuf);
>  		goto out;
>  	}
> -	if (nsm)
> -		atomic_inc(&nsm->sm_count);
>  
> -	host = NULL;
> -
> -	/* Sadly, the host isn't in our hash table yet. See if
> -	 * we have an NSM handle for it. If not, create one.
> +	/*
> +	 * The host wasn't in our hash table.  If we don't
> +	 * have an NSM handle for it yet, create one.
>  	 */
> -	if (!nsm && !(nsm = nsm_find(sin, hostname, hostname_len)))
> -		goto out;
> +	if (nsm)
> +		atomic_inc(&nsm->sm_count);
> +	else {
> +		host = NULL;
> +		nsm = nsm_find(sin, hostname, hostname_len);
> +		if (!nsm) {
> +			dprintk("lockd: nlm_lookup_host failed; "
> +				"no nsm handle\n");
> +			goto out;
> +		}
> +	}

This seems like an odd mixture of unrelated changes; I've lost track of
the rationale.  Maybe split out another patch?

--b.

>  
>  	host = kzalloc(sizeof(*host), GFP_KERNEL);
>  	if (!host) {
>  		nsm_release(nsm);
> +		dprintk("lockd: nlm_lookup_host failed; no memory\n");
>  		goto out;
>  	}
>  	host->h_name	   = nsm->sm_name;
> @@ -143,6 +171,15 @@ static struct nlm_host *nlm_lookup_host(int server,
>  	INIT_LIST_HEAD(&host->h_reclaim);
>  
>  	nrhosts++;
> +
> +	nlm_display_address((struct sockaddr *)&host->h_addr,
> +				host->h_addrbuf, sizeof(host->h_addrbuf));
> +	nlm_display_address((struct sockaddr *)&host->h_saddr,
> +				host->h_saddrbuf, sizeof(host->h_saddrbuf));
> +
> +	dprintk("lockd: nlm_lookup_host created host %s\n",
> +			host->h_name);
> +
>  out:
>  	mutex_unlock(&nlm_host_mutex);
>  	return host;
> @@ -207,9 +244,8 @@ nlm_bind_host(struct nlm_host *host)
>  {
>  	struct rpc_clnt	*clnt;
>  
> -	dprintk("lockd: nlm_bind_host("NIPQUAD_FMT"->"NIPQUAD_FMT")\n",
> -			NIPQUAD(host->h_saddr.sin_addr),
> -			NIPQUAD(host->h_addr.sin_addr));
> +	dprintk("lockd: nlm_bind_host %s (%s), my addr=%s\n",
> +			host->h_name, host->h_addrbuf, host->h_saddrbuf);
>  
>  	/* Lock host handle */
>  	mutex_lock(&host->h_mutex);
> @@ -221,7 +257,7 @@ nlm_bind_host(struct nlm_host *host)
>  		if (time_after_eq(jiffies, host->h_nextrebind)) {
>  			rpc_force_rebind(clnt);
>  			host->h_nextrebind = jiffies + NLM_HOST_REBIND;
> -			dprintk("lockd: next rebind in %ld jiffies\n",
> +			dprintk("lockd: next rebind in %lu jiffies\n",
>  					host->h_nextrebind - jiffies);
>  		}
>  	} else {
> @@ -324,8 +360,7 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
>  	struct nsm_handle *nsm;
>  	struct nlm_host	*host;
>  
> -	dprintk("lockd: nlm_host_rebooted(%s, %u.%u.%u.%u)\n",
> -			hostname, NIPQUAD(sin->sin_addr));
> +	dprintk("lockd: nlm_host_rebooted(%s)\n", hostname);
>  
>  	/* Find the NSM handle for this peer */
>  	if (!(nsm = __nsm_find(sin, hostname, hostname_len, 0)))
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index a6d1215..5774d6e 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -61,6 +61,9 @@ struct nlm_host {
>  	struct list_head	h_granted;	/* Locks in GRANTED state */
>  	struct list_head	h_reclaim;	/* Locks in RECLAIM state */
>  	struct nsm_handle *	h_nsmhandle;	/* NSM status handle */
> +
> +	char			h_addrbuf[48],	/* address eyecatchers */
> +				h_saddrbuf[48];
>  };
>  
>  struct nsm_handle {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/16] lockd: Pass "struct sockaddr *" to new failover-by-IP function

[Chuck Lever]

On Tue, Jul 15, 2008 at 4:12 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Mon, Jun 30, 2008 at 06:58:14PM -0400, Chuck Lever wrote:
>> Pass a more generic socket address type to nlmsvc_unlock_all_by_ip() to
>> allow for future support of IPv6.  Also provide additional sanity
>> checking in failover_unlock_ip() when constructing the server's IP
>> address.
>>
>> As an added bonus, provide clean kerneldoc comments on related NLM
>> interfaces which were recently added.
>
> Looks good to me, thanks--applied, with one minor change:
>
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>>  fs/lockd/svcsubs.c          |   39 +++++++++++++++++++++++++--------------
>>  fs/nfsd/nfsctl.c            |   15 ++++++++++-----
>>  include/linux/lockd/lockd.h |    2 +-
>>  3 files changed, 36 insertions(+), 20 deletions(-)
>>
>>
>> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
>> index d1c48b5..723b6d5 100644
>> --- a/fs/lockd/svcsubs.c
>> +++ b/fs/lockd/svcsubs.c
>> @@ -373,18 +373,18 @@ nlmsvc_free_host_resources(struct nlm_host *host)
>>       }
>>  }
>>
>> -/*
>> - * Remove all locks held for clients
>> +/**
>> + * nlmsvc_invalidate_all - remove all locks held for clients
>> + *
>> + * Release all locks held by NFS clients.  Previously, the code
>> + * would call nlmsvc_free_host_resources for each client in turn,
>> + * which is about as inefficient as it gets.
>> + *
>> + * Now we just do it once in nlm_traverse_files.
>>   */
>>  void
>>  nlmsvc_invalidate_all(void)
>>  {
>> -     /* Release all locks held by NFS clients.
>> -      * Previously, the code would call
>> -      * nlmsvc_free_host_resources for each client in
>> -      * turn, which is about as inefficient as it gets.
>> -      * Now we just do it once in nlm_traverse_files.
>> -      */
>
> Mind if I keep the "Previously..." part in the body of the function?
> I'd rather the kerneldoc comments be about the interface and leave
> implementation details to the body of the function.

It's a personal style choice.  Keeping generic implementation comments
out of the body of the function makes cleaner code, I think, and makes
it more likely the author will write self-documenting code.  In this
case, the comment is more historical than explanatory.

But I don't have any objection.

> Also, one more minor question--
>
>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>> index 5ac00c4..5b4a412 100644
>> --- a/fs/nfsd/nfsctl.c
>> +++ b/fs/nfsd/nfsctl.c
>> @@ -310,9 +310,12 @@ static ssize_t write_getfd(struct file *file, char *buf, size_t size)
>>
>>  static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size)
>>  {
>> -     __be32 server_ip;
>> -     char *fo_path, c;
>> +     struct sockaddr_in sin = {
>> +             .sin_family     = AF_INET,
>> +     };
>>       int b1, b2, b3, b4;
>> +     char c;
>> +     char *fo_path;
>>
>>       /* sanity check */
>>       if (size == 0)
>> @@ -326,11 +329,13 @@ static ssize_t failover_unlock_ip(struct file *file, char *buf, size_t size)
>>               return -EINVAL;
>>
>>       /* get ipv4 address */
>> -     if (sscanf(fo_path, "%u.%u.%u.%u%c", &b1, &b2, &b3, &b4, &c) != 4)
>> +     if (sscanf(fo_path, NIPQUAD_FMT "%c", &b1, &b2, &b3, &b4, &c) != 4)
>> +             return -EINVAL;
>> +     if (b1 > 255 || b2 > 255 || b3 > 255 || b4 > 255)
>
> Should b1, b2, b3, b4 be unsigned?

That would be more consistent with the %u formatting, wouldn't it.

>>               return -EINVAL;
>> -     server_ip = htonl((((((b1<<8)|b2)<<8)|b3)<<8)|b4);
>> +     sin.sin_addr.s_addr = htonl((b1 << 24) | (b2 << 16) | (b3 << 8) | b4);
>>
>> -     return nlmsvc_unlock_all_by_ip(server_ip);
>> +     return nlmsvc_unlock_all_by_ip((struct sockaddr *)&sin);
>>  }

-- 
Chuck Lever

Re: [PATCH 02/16] lockd: address-family independent printable addresses

[Chuck Lever]

On Tue, Jul 15, 2008 at 4:25 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Mon, Jun 30, 2008 at 06:58:22PM -0400, Chuck Lever wrote:
>> Knowing which source address is used for communicating with remote NLM
>> services can be helpful for debugging configuration problems on hosts
>> with multiple addresses.
>>
>> Keep the dprintk debugging here, but adapt it so it displays AF_INET6
>> addresses properly.  There are also a couple of dprintk clean-ups as
>> well.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>>  fs/lockd/host.c             |   79 +++++++++++++++++++++++++++++++------------
>>  include/linux/lockd/lockd.h |    3 ++
>>  2 files changed, 60 insertions(+), 22 deletions(-)
>>
>>
>> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
>> index a17664c..350d757 100644
>> --- a/fs/lockd/host.c
>> +++ b/fs/lockd/host.c
>> @@ -11,6 +11,7 @@
>>  #include <linux/types.h>
>>  #include <linux/slab.h>
>>  #include <linux/in.h>
>> +#include <linux/in6.h>
>>  #include <linux/sunrpc/clnt.h>
>>  #include <linux/sunrpc/svc.h>
>>  #include <linux/lockd/lockd.h>
>> @@ -38,6 +39,28 @@ static struct nsm_handle * nsm_find(const struct sockaddr_in *sin,
>>                                        const char *hostname,
>>                                        unsigned int hostname_len);
>>
>> +static void nlm_display_address(const struct sockaddr *sap,
>> +                             char *buf, const size_t len)
>> +{
>> +     const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
>> +     const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
>> +
>> +     switch (sap->sa_family) {
>> +     case AF_UNSPEC:
>> +             snprintf(buf, len, "unspecified");
>> +             break;
>> +     case AF_INET:
>> +             snprintf(buf, len, NIPQUAD_FMT, NIPQUAD(sin->sin_addr.s_addr));
>> +             break;
>> +     case AF_INET6:
>> +             snprintf(buf, len, NIP6_FMT, NIP6(sin6->sin6_addr));
>> +             break;
>> +     default:
>> +             snprintf(buf, len, "unsupported address family");
>> +             break;
>> +     }
>> +}
>
> Is there code we could reuse from someplace else?

In this case, we need to differentiate between "unspecified" and
"unsupported" addresses (see nlmclnt_ vs. nlmsvc_lookup).  The other
places don't need this distinction.  Each one of these has slightly
different details and requirements.

We have the same issue with setting and retrieving port numbers.

I think once the IPv6 support is integrated and the dust has settled
we will find plenty of places where refactoring would result in
smaller and more shared code.  The problem is picking a common place
to house all these little bits and pieces.

>
>> +
>>  /*
>>   * Common host lookup routine for server & client
>>   */
>> @@ -54,14 +77,10 @@ static struct nlm_host *nlm_lookup_host(int server,
>>       struct nsm_handle *nsm = NULL;
>>       int             hash;
>>
>> -     dprintk("lockd: nlm_lookup_host("NIPQUAD_FMT"->"NIPQUAD_FMT
>> -                     ", p=%d, v=%u, my role=%s, name=%.*s)\n",
>> -                     NIPQUAD(ssin->sin_addr.s_addr),
>> -                     NIPQUAD(sin->sin_addr.s_addr), proto, version,
>> -                     server? "server" : "client",
>> -                     hostname_len,
>> -                     hostname? hostname : "<none>");
>> -
>> +     dprintk("lockd: nlm_lookup_host(proto=%d, vers=%u,"
>> +                     " my role is %s, hostname=%.*s)\n",
>> +                     proto, version, server? "server" : "client",
>> +                     hostname_len, hostname? hostname : "<none>");
>>
>>       hash = NLM_ADDRHASH(sin->sin_addr.s_addr);
>>
>> @@ -101,22 +120,31 @@ static struct nlm_host *nlm_lookup_host(int server,
>>               hlist_add_head(&host->h_hash, chain);
>>
>>               nlm_get_host(host);
>> +             dprintk("lockd: nlm_lookup_host found host %s (%s)\n",
>> +                             host->h_name, host->h_addrbuf);
>>               goto out;
>>       }
>> -     if (nsm)
>> -             atomic_inc(&nsm->sm_count);
>>
>> -     host = NULL;
>> -
>> -     /* Sadly, the host isn't in our hash table yet. See if
>> -      * we have an NSM handle for it. If not, create one.
>> +     /*
>> +      * The host wasn't in our hash table.  If we don't
>> +      * have an NSM handle for it yet, create one.
>>        */
>> -     if (!nsm && !(nsm = nsm_find(sin, hostname, hostname_len)))
>> -             goto out;
>> +     if (nsm)
>> +             atomic_inc(&nsm->sm_count);
>> +     else {
>> +             host = NULL;
>> +             nsm = nsm_find(sin, hostname, hostname_len);
>> +             if (!nsm) {
>> +                     dprintk("lockd: nlm_lookup_host failed; "
>> +                             "no nsm handle\n");
>> +                     goto out;
>> +             }
>> +     }
>
> This seems like an odd mixture of unrelated changes; I've lost track of
> the rationale.  Maybe split out another patch?

I noticed this the other day, and I can't remember why I had to move
this hunk into this patch.  Normally I would immediately agree to this
request, but I am supposed to be on vacation this week :-)

More below...

>>
>>       host = kzalloc(sizeof(*host), GFP_KERNEL);
>>       if (!host) {
>>               nsm_release(nsm);
>> +             dprintk("lockd: nlm_lookup_host failed; no memory\n");
>>               goto out;
>>       }
>>       host->h_name       = nsm->sm_name;
>> @@ -143,6 +171,15 @@ static struct nlm_host *nlm_lookup_host(int server,
>>       INIT_LIST_HEAD(&host->h_reclaim);
>>
>>       nrhosts++;
>> +
>> +     nlm_display_address((struct sockaddr *)&host->h_addr,
>> +                             host->h_addrbuf, sizeof(host->h_addrbuf));
>> +     nlm_display_address((struct sockaddr *)&host->h_saddr,
>> +                             host->h_saddrbuf, sizeof(host->h_saddrbuf));
>> +
>> +     dprintk("lockd: nlm_lookup_host created host %s\n",
>> +                     host->h_name);
>> +
>>  out:
>>       mutex_unlock(&nlm_host_mutex);
>>       return host;
>> @@ -207,9 +244,8 @@ nlm_bind_host(struct nlm_host *host)
>>  {
>>       struct rpc_clnt *clnt;
>>
>> -     dprintk("lockd: nlm_bind_host("NIPQUAD_FMT"->"NIPQUAD_FMT")\n",
>> -                     NIPQUAD(host->h_saddr.sin_addr),
>> -                     NIPQUAD(host->h_addr.sin_addr));
>> +     dprintk("lockd: nlm_bind_host %s (%s), my addr=%s\n",
>> +                     host->h_name, host->h_addrbuf, host->h_saddrbuf);
>>
>>       /* Lock host handle */
>>       mutex_lock(&host->h_mutex);
>> @@ -221,7 +257,7 @@ nlm_bind_host(struct nlm_host *host)
>>               if (time_after_eq(jiffies, host->h_nextrebind)) {
>>                       rpc_force_rebind(clnt);
>>                       host->h_nextrebind = jiffies + NLM_HOST_REBIND;
>> -                     dprintk("lockd: next rebind in %ld jiffies\n",
>> +                     dprintk("lockd: next rebind in %lu jiffies\n",
>>                                       host->h_nextrebind - jiffies);
>>               }
>>       } else {
>> @@ -324,8 +360,7 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
>>       struct nsm_handle *nsm;
>>       struct nlm_host *host;
>>
>> -     dprintk("lockd: nlm_host_rebooted(%s, %u.%u.%u.%u)\n",
>> -                     hostname, NIPQUAD(sin->sin_addr));
>> +     dprintk("lockd: nlm_host_rebooted(%s)\n", hostname);

I've re-written this piece to keep the display of the address.  I
think that would continue to be a valuable piece of debugging
information.

If you're about to include this patch, I'd like to send you the new
version of this patch and the later ones that change as a result.

>>
>>       /* Find the NSM handle for this peer */
>>       if (!(nsm = __nsm_find(sin, hostname, hostname_len, 0)))
>> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
>> index a6d1215..5774d6e 100644
>> --- a/include/linux/lockd/lockd.h
>> +++ b/include/linux/lockd/lockd.h
>> @@ -61,6 +61,9 @@ struct nlm_host {
>>       struct list_head        h_granted;      /* Locks in GRANTED state */
>>       struct list_head        h_reclaim;      /* Locks in RECLAIM state */
>>       struct nsm_handle *     h_nsmhandle;    /* NSM status handle */
>> +
>> +     char                    h_addrbuf[48],  /* address eyecatchers */
>> +                             h_saddrbuf[48];
>>  };
>>
>>  struct nsm_handle {

-- 
Chuck Lever

Re: [PATCH 02/16] lockd: address-family independent printable addresses

[J. Bruce Fields]

On Tue, Jul 15, 2008 at 05:41:15PM -0400, Chuck Lever wrote:
> I've re-written this piece to keep the display of the address.  I
> think that would continue to be a valuable piece of debugging
> information.
> 
> If you're about to include this patch, I'd like to send you the new
> version of this patch and the later ones that change as a result.

No, I knew you were still doing a little work on these.

--b.