[PATCH 0/7] More IPv6 support for lockd

[PATCH 0/7] More IPv6 support for lockd

[Chuck Lever]

Hi Bruce-

Here's the next set of IPv6-related patches for lockd.  Please consider these
for 2.6.28.

---

Chuck Lever (7):
      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


 fs/lockd/clntlock.c         |    3 +
 fs/lockd/host.c             |  115 ++++++++++++++++++++++++++++---------------
 fs/lockd/mon.c              |    2 -
 fs/lockd/svcsubs.c          |    2 -
 include/linux/lockd/lockd.h |   79 +++++++++++++++++++++++++++---
 5 files changed, 149 insertions(+), 52 deletions(-)

-- 
Chuck Lever

[PATCH 1/7] 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 008e402..8c7022e 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -116,7 +116,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 */
@@ -165,8 +165,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;
@@ -291,8 +292,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 0691efb..41d7a8e 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 */
@@ -77,6 +78,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 2/7] 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 8c7022e..3ce2702 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -129,7 +129,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. */
@@ -168,7 +168,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;
@@ -192,8 +192,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);
@@ -267,7 +267,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);
@@ -294,7 +294,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 198b4e5..d3d1330 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -418,7 +418,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 41d7a8e..964e6c9 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 {
@@ -90,6 +90,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 3/7] 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 3ce2702..510ebcf 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -551,7 +551,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);
@@ -571,7 +571,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 964e6c9..b1dfa0b 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 */
 	char			sm_addrbuf[48];	/* address eyecatcher */
@@ -100,6 +101,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 4/7] 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 |   36 ++++++++++++++++++++++++++++++++----
 4 files changed, 38 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 510ebcf..dbf3fe6 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -116,7 +116,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 */
@@ -129,7 +129,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. */
@@ -551,7 +551,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 d3d1330..34c2766 100644
--- a/fs/lockd/svcsubs.c
+++ b/fs/lockd/svcsubs.c
@@ -418,7 +418,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 b1dfa0b..ec8af11 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>
@@ -272,13 +274,39 @@ 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) {
+		switch (sap1->sa_family) {
+		case AF_INET:
+			return __nlm_cmp_addr4(sap1, sap2);
+		case AF_INET6:
+			return __nlm_cmp_addr6(sap1, sap2);
+		}
+	}
+	return 0;
 }
 
 /*

[PATCH 5/7] 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 nlm_lookup_host()'s existing IP address hash function has the same
issue with correctness on little-endian systems as the original IPv4 auth
cache hash function, so I've also updated 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 dbf3fe6..1f9d72a 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -22,7 +22,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)
@@ -40,6 +39,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)
 {
 	switch (sap->sa_family) {
@@ -92,16 +133,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))
@@ -114,7 +151,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 6/7] 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 |   32 +++++++++++---------------------
 1 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 1f9d72a..b9eeafe 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -31,13 +31,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 size_t hostname_len,
+						const int create);
 
 /*
  * Hash function must work well on big- and little-endian platforms
@@ -187,7 +185,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,7 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
 	struct nsm_handle *nsm;
 	struct nlm_host	*host;
 
-	/* Find the NSM handle for this peer */
-	nsm = __nsm_find(sin, hostname, hostname_len, 0);
+	nsm = nsm_find(sin, hostname, hostname_len, 0);
 	if (nsm == NULL) {
 		dprintk("lockd: never saw rebooted peer '%.*s' before\n",
 				hostname_len, hostname);
@@ -560,10 +557,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 size_t hostname_len,
+				   const int create)
 {
 	struct nsm_handle *nsm = NULL;
 	struct nsm_handle *pos;
@@ -575,7 +572,7 @@ __nsm_find(const struct sockaddr_in *sin,
 		if (printk_ratelimit()) {
 			printk(KERN_WARNING "Invalid hostname \"%.*s\" "
 					    "in NFS lock request\n",
-				hostname_len, hostname);
+				(int)hostname_len, hostname);
 		}
 		return NULL;
 	}
@@ -623,13 +620,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 7/7] 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 |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index b9eeafe..be8f19d 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -32,7 +32,8 @@ 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 size_t hostname_len,
 						const int create);
@@ -185,7 +186,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,
 	struct nsm_handle *nsm;
 	struct nlm_host	*host;
 
-	nsm = nsm_find(sin, hostname, hostname_len, 0);
+	nsm = nsm_find((struct sockaddr *)sin, sizeof(*sin),
+			hostname, hostname_len, 0);
 	if (nsm == NULL) {
 		dprintk("lockd: never saw rebooted peer '%.*s' before\n",
 				hostname_len, hostname);
@@ -557,7 +560,8 @@ 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 size_t hostname_len,
 				   const int create)
@@ -565,7 +569,7 @@ static struct nsm_handle *nsm_find(const struct sockaddr_in *sin,
 	struct nsm_handle *nsm = NULL;
 	struct nsm_handle *pos;
 
-	if (!sin)
+	if (!sap)
 		return NULL;
 
 	if (hostname && memchr(hostname, '/', hostname_len) != NULL) {
@@ -585,7 +589,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);
@@ -605,8 +609,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';

Re: [PATCH 1/7] lockd: Use sockaddr_storage + length for h_addr field

[J. Bruce Fields]

On Wed, Sep 03, 2008 at 02:35:39PM -0400, Chuck Lever wrote:
> To store larger addresses in the nlm_host structure, make h_addr a
> sockaddr_storage, and add an address length field.

Erps, I just noticed that mail to bfields@citi.umich.edu started getting
shunted off into a mailbox I don't check starting a week or two ago.  If
anyone sent me private mail to that address and hasn't gotten a
response, you might want to resend....  Anyway, thanks, I'll try to look
at this soon.

--b.

> 
> 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 008e402..8c7022e 100644
> --- a/fs/lockd/host.c
> +++ b/fs/lockd/host.c
> @@ -116,7 +116,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 */
> @@ -165,8 +165,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;
> @@ -291,8 +292,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 0691efb..41d7a8e 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 */
> @@ -77,6 +78,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 {
> 

Re: [PATCH 2/7] lockd: Use sockaddr_storage for h_saddr field

[J. Bruce Fields]

On Wed, Sep 03, 2008 at 02:35:46PM -0400, Chuck Lever wrote:
> @@ -294,7 +294,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),

Just out of curiosity--in the patch 1 and 3, you also add a length
field.  Here you don't.  Why the difference?  None of the new length
fields seem to be used yet.

--b.

>  			.timeout	= &timeparms,
>  			.servername	= host->h_name,
>  			.program	= &nlm_program,
> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
> index 198b4e5..d3d1330 100644
> --- a/fs/lockd/svcsubs.c
> +++ b/fs/lockd/svcsubs.c
> @@ -418,7 +418,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 41d7a8e..964e6c9 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 {
> @@ -90,6 +90,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"
>   */
> 

Re: [PATCH 2/7] lockd: Use sockaddr_storage for h_saddr field

[Chuck Lever]

On Thu, Sep 11, 2008 at 9:41 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Wed, Sep 03, 2008 at 02:35:46PM -0400, Chuck Lever wrote:
>> @@ -294,7 +294,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),
>
> Just out of curiosity--in the patch 1 and 3, you also add a length
> field.  Here you don't.  Why the difference?  None of the new length
> fields seem to be used yet.

h_addrlen should be used in nlm_bind_host already in patch 1.  When we
get up to the NSM changes, sm_addrlen will be used in nsm_mon_unmon.

The source address is provided to rpc_create() only, which doesn't
take an address length.

>>                       .timeout        = &timeparms,
>>                       .servername     = host->h_name,
>>                       .program        = &nlm_program,
>> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
>> index 198b4e5..d3d1330 100644
>> --- a/fs/lockd/svcsubs.c
>> +++ b/fs/lockd/svcsubs.c
>> @@ -418,7 +418,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 41d7a8e..964e6c9 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 {
>> @@ -90,6 +90,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"
>>   */
>>
> --
> 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
>



-- 
"If you simplify your English, you are freed from the worst follies of
orthodoxy."
 -- George Orwell

Re: [PATCH 6/7] lockd: Combine __nsm_find() and nsm_find().

[J. Bruce Fields]

On Wed, Sep 03, 2008 at 02:36:16PM -0400, Chuck Lever wrote:
> Clean up: Having two separate functions doesn't add clarity, so
> eliminate one of them.

Yeah, agreed, there's no way to guess what the difference between
__nsm_find(...,0) and nsm_find(...) is without looking up the prototype,
so why bother?--there's no loss here in just using nsm_find(...,0) and
nsm_find(...,1) instead.

That said, with a couple wrappers, say nsm_lookup() and nsm_create(),
you might be able to save the reader the trouble of having to go look at
the implementation.

Up to you.  I'll apply this and you can send an incremental patch if you
want.

--b.

> Use contemporary kernel coding conventions
> where appropriate.

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/lockd/host.c |   32 +++++++++++---------------------
>  1 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
> index 1f9d72a..b9eeafe 100644
> --- a/fs/lockd/host.c
> +++ b/fs/lockd/host.c
> @@ -31,13 +31,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 size_t hostname_len,
> +						const int create);
>  
>  /*
>   * Hash function must work well on big- and little-endian platforms
> @@ -187,7 +185,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,7 @@ void nlm_host_rebooted(const struct sockaddr_in *sin,
>  	struct nsm_handle *nsm;
>  	struct nlm_host	*host;
>  
> -	/* Find the NSM handle for this peer */
> -	nsm = __nsm_find(sin, hostname, hostname_len, 0);
> +	nsm = nsm_find(sin, hostname, hostname_len, 0);
>  	if (nsm == NULL) {
>  		dprintk("lockd: never saw rebooted peer '%.*s' before\n",
>  				hostname_len, hostname);
> @@ -560,10 +557,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 size_t hostname_len,
> +				   const int create)
>  {
>  	struct nsm_handle *nsm = NULL;
>  	struct nsm_handle *pos;
> @@ -575,7 +572,7 @@ __nsm_find(const struct sockaddr_in *sin,
>  		if (printk_ratelimit()) {
>  			printk(KERN_WARNING "Invalid hostname \"%.*s\" "
>  					    "in NFS lock request\n",
> -				hostname_len, hostname);
> +				(int)hostname_len, hostname);
>  		}
>  		return NULL;
>  	}
> @@ -623,13 +620,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
>   */
> 

Re: [PATCH 5/7] lockd: Support AF_INET6 when hashing addresses in nlm_lookup_host

[J. Bruce Fields]

On Wed, Sep 03, 2008 at 02:36:08PM -0400, Chuck Lever wrote:
> Adopt an approach similar to the RPC server's auth cache (from Aurelien
> Charbon and Brian Haley).
> 
> Note nlm_lookup_host()'s existing IP address hash function has the same
> issue with correctness

(Wasn't it more perforance than correctness?  Arguably a hash is
*correct* as long as the output is uniquely determined by the input, but
the issue in this case was just that it was likely to be constant in
common cases, right?)

Anyway, looks fine, applied.

--b.

> on little-endian systems as the original IPv4 auth
> cache hash function, so I've also updated 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 dbf3fe6..1f9d72a 100644
> --- a/fs/lockd/host.c
> +++ b/fs/lockd/host.c
> @@ -22,7 +22,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)
> @@ -40,6 +39,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)
>  {
>  	switch (sap->sa_family) {
> @@ -92,16 +133,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))
> @@ -114,7 +151,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;
> 

Re: [PATCH 2/7] lockd: Use sockaddr_storage for h_saddr field

[J. Bruce Fields]

On Thu, Sep 11, 2008 at 11:14:39PM -0400, Chuck Lever wrote:
> On Thu, Sep 11, 2008 at 9:41 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Wed, Sep 03, 2008 at 02:35:46PM -0400, Chuck Lever wrote:
> >> @@ -294,7 +294,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),
> >
> > Just out of curiosity--in the patch 1 and 3, you also add a length
> > field.  Here you don't.  Why the difference?  None of the new length
> > fields seem to be used yet.
> 
> h_addrlen should be used in nlm_bind_host already in patch 1.

Whoops, yes, I missed the rpc_create_args assignment, thanks.

--b.

> When we
> get up to the NSM changes, sm_addrlen will be used in nsm_mon_unmon.
> 
> The source address is provided to rpc_create() only, which doesn't
> take an address length.
> 
> >>                       .timeout        = &timeparms,
> >>                       .servername     = host->h_name,
> >>                       .program        = &nlm_program,
> >> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
> >> index 198b4e5..d3d1330 100644
> >> --- a/fs/lockd/svcsubs.c
> >> +++ b/fs/lockd/svcsubs.c
> >> @@ -418,7 +418,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 41d7a8e..964e6c9 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 {
> >> @@ -90,6 +90,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"
> >>   */
> >>
> > --
> > 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
> >
> 
> 
> 
> -- 
> "If you simplify your English, you are freed from the worst follies of
> orthodoxy."
>  -- George Orwell

Re: [PATCH 0/7] More IPv6 support for lockd

[J. Bruce Fields]

On Wed, Sep 03, 2008 at 02:35:31PM -0400, Chuck Lever wrote:
> Hi Bruce-
> 
> Here's the next set of IPv6-related patches for lockd.  Please consider these
> for 2.6.28.

Thanks; applied to

	git://linux-nfs.org/~bfields/linux.git for-2.6.28

--b.

> 
> ---
> 
> Chuck Lever (7):
>       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
> 
> 
>  fs/lockd/clntlock.c         |    3 +
>  fs/lockd/host.c             |  115 ++++++++++++++++++++++++++++---------------
>  fs/lockd/mon.c              |    2 -
>  fs/lockd/svcsubs.c          |    2 -
>  include/linux/lockd/lockd.h |   79 +++++++++++++++++++++++++++---
>  5 files changed, 149 insertions(+), 52 deletions(-)
> 
> -- 
> Chuck Lever