[PATCH 0/7] Replacements for last week's patches

[PATCH 0/7] Replacements for last week's patches

[Chuck Lever]

Hi Bruce-

Here's a patch series to replace the patches you reviewed earlier this
week.  I've addressed the issues you mentioned in your review comments,
and dropped the NSM-related patches that treat the 16-byte opaque as an
address, as discussed.

The last patches I intend to submit for 2.6.28 will address the lack of
a client-side UDP lockd listener.  I will propose a fix later today in
a subsequent patch series.

---

Chuck Lever (7):
      lockd: Remove unused fields in the nlm_reboot structure
      lockd: Add helper to sanity check incoming NOTIFY requests
      lockd: change nlmclnt_grant() to take a "struct sockaddr *"
      lockd: Adjust nlmsvc_lookup_host() to accomodate AF_INET6 addresses
      lockd: Adjust nlmclnt_lookup_host() signature to accomodate non-AF_INET
      lockd: Support non-AF_INET addresses in nlm_lookup_host()
      NLM: Convert nlm_lookup_host() to use a single argument


 fs/lockd/clntlock.c         |   10 +--
 fs/lockd/host.c             |  161 +++++++++++++++++++++++++++++++------------
 fs/lockd/svc4proc.c         |    8 +-
 fs/lockd/svcproc.c          |    8 +-
 fs/lockd/xdr.c              |    2 -
 fs/lockd/xdr4.c             |    2 -
 include/linux/lockd/lockd.h |   56 +++++++++++++--
 include/linux/lockd/xdr.h   |    2 -
 8 files changed, 176 insertions(+), 73 deletions(-)

-- 
Chuck Lever

[PATCH 1/7] NLM: Convert nlm_lookup_host() to use a single argument

[Chuck Lever]

The nlm_lookup_host() function already has a large number of arguments,
and I'm about to add a few more.  As a clean up, convert the function
to use a single data structure argument.

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

 fs/lockd/host.c |   89 ++++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 59 insertions(+), 30 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index be8f19d..1630588 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -38,6 +38,20 @@ 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		peer;		/* search for server|client */
+	const struct sockaddr_in *sin;		/* address to search for */
+	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_in *src_sin;	/* our address (optional) */
+	const size_t		src_len;	/* it's length */
+};
+
 /*
  * Hash function must work well on big- and little-endian platforms
  */
@@ -121,23 +135,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))
@@ -150,22 +154,23 @@ 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((struct sockaddr *)ni->sin)];
 	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), (struct sockaddr *)ni->sin))
 			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->peer)
 			continue;
-		if (!nlm_cmp_addr(nlm_srcaddr(host), (struct sockaddr *)ssin))
+		if (!nlm_cmp_addr(nlm_srcaddr(host),
+					(struct sockaddr *)ni->src_sin))
 			continue;
 
 		/* Move to head of hash chain. */
@@ -186,8 +191,9 @@ 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((struct sockaddr *)ni->sin,
+				sizeof(struct sockaddr_in),
+				ni->hostname, ni->hostname_len, 1);
 		if (!nsm) {
 			dprintk("lockd: nlm_lookup_host failed; "
 				"no nsm handle\n");
@@ -202,12 +208,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->sin, sizeof(struct sockaddr_in));
+	host->h_addrlen = sizeof(struct sockaddr_in);
 	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_sin, sizeof(struct sockaddr_in));
+	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;
@@ -218,7 +224,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->peer;
 	hlist_add_head(&host->h_hash, chain);
 	INIT_LIST_HEAD(&host->h_lockowners);
 	spin_lock_init(&host->h_lock);
@@ -273,9 +279,21 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
 	const struct sockaddr_in source = {
 		.sin_family	= AF_UNSPEC,
 	};
+	struct nlm_lookup_host_info ni = {
+		.peer		= NLM_SERVER,
+		.sin		= sin,
+		.protocol	= proto,
+		.version	= version,
+		.hostname	= hostname,
+		.hostname_len	= hostname_len,
+		.src_sin	= &source,
+	};
 
-	return nlm_lookup_host(0, sin, proto, version,
-			       hostname, hostname_len, &source);
+	dprintk("lockd: %s(host='%s', vers=%u, proto=%s)\n", __func__,
+			(hostname ? hostname : "<none>"), version,
+			(proto == IPPROTO_UDP ? "udp" : "tcp"));
+
+	return nlm_lookup_host(&ni);
 }
 
 /*
@@ -289,10 +307,21 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
 		.sin_family	= AF_INET,
 		.sin_addr	= rqstp->rq_daddr.addr,
 	};
+	struct nlm_lookup_host_info ni = {
+		.peer		= NLM_CLIENT,
+		.sin		= svc_addr_in(rqstp),
+		.protocol	= rqstp->rq_prot,
+		.version	= rqstp->rq_vers,
+		.hostname	= hostname,
+		.hostname_len	= hostname_len,
+		.src_sin	= &source,
+	};
+
+	dprintk("lockd: %s(host='%*s', vers=%u, proto=%s)\n", __func__,
+			(int)hostname_len, hostname, rqstp->rq_vers,
+			(rqstp->rq_prot == IPPROTO_UDP ? "udp" : "tcp"));
 
-	return nlm_lookup_host(1, svc_addr_in(rqstp),
-			       rqstp->rq_prot, rqstp->rq_vers,
-			       hostname, hostname_len, &source);
+	return nlm_lookup_host(&ni);
 }
 
 /*

Re: [PATCH 1/7] NLM: Convert nlm_lookup_host() to use a single argument

[Trond Myklebust]

On Fri, 2008-10-03 at 12:50 -0400, Chuck Lever wrote:
> The nlm_lookup_host() function already has a large number of arguments,
> and I'm about to add a few more.  As a clean up, convert the function
> to use a single data structure argument.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/lockd/host.c |   89 ++++++++++++++++++++++++++++++++++++-------------------
>  1 files changed, 59 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
> index be8f19d..1630588 100644
> --- a/fs/lockd/host.c
> +++ b/fs/lockd/host.c
> @@ -38,6 +38,20 @@ 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)
> +

Could we please get RID of this crap instead of 'documenting' it? There
is absolutely _no_ excuse for keeping a single list for both server and
client struct hosts...

Strongly NACKED....

> +struct nlm_lookup_host_info {
> +	const int		peer;		/* search for server|client */
> +	const struct sockaddr_in *sin;		/* address to search for */
> +	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_in *src_sin;	/* our address (optional) */
> +	const size_t		src_len;	/* it's length */
> +};
> +
>  /*
>   * Hash function must work well on big- and little-endian platforms
>   */
> @@ -121,23 +135,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))
> @@ -150,22 +154,23 @@ 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((struct sockaddr *)ni->sin)];
>  	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), (struct sockaddr *)ni->sin))
>  			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->peer)
>  			continue;
> -		if (!nlm_cmp_addr(nlm_srcaddr(host), (struct sockaddr *)ssin))
> +		if (!nlm_cmp_addr(nlm_srcaddr(host),
> +					(struct sockaddr *)ni->src_sin))
>  			continue;
>  
>  		/* Move to head of hash chain. */
> @@ -186,8 +191,9 @@ 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((struct sockaddr *)ni->sin,
> +				sizeof(struct sockaddr_in),
> +				ni->hostname, ni->hostname_len, 1);
>  		if (!nsm) {
>  			dprintk("lockd: nlm_lookup_host failed; "
>  				"no nsm handle\n");
> @@ -202,12 +208,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->sin, sizeof(struct sockaddr_in));
> +	host->h_addrlen = sizeof(struct sockaddr_in);
>  	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_sin, sizeof(struct sockaddr_in));
> +	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;
> @@ -218,7 +224,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->peer;
>  	hlist_add_head(&host->h_hash, chain);
>  	INIT_LIST_HEAD(&host->h_lockowners);
>  	spin_lock_init(&host->h_lock);
> @@ -273,9 +279,21 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
>  	const struct sockaddr_in source = {
>  		.sin_family	= AF_UNSPEC,
>  	};
> +	struct nlm_lookup_host_info ni = {
> +		.peer		= NLM_SERVER,
> +		.sin		= sin,
> +		.protocol	= proto,
> +		.version	= version,
> +		.hostname	= hostname,
> +		.hostname_len	= hostname_len,
> +		.src_sin	= &source,
> +	};
>  
> -	return nlm_lookup_host(0, sin, proto, version,
> -			       hostname, hostname_len, &source);
> +	dprintk("lockd: %s(host='%s', vers=%u, proto=%s)\n", __func__,
> +			(hostname ? hostname : "<none>"), version,
> +			(proto == IPPROTO_UDP ? "udp" : "tcp"));
> +
> +	return nlm_lookup_host(&ni);
>  }
>  
>  /*
> @@ -289,10 +307,21 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
>  		.sin_family	= AF_INET,
>  		.sin_addr	= rqstp->rq_daddr.addr,
>  	};
> +	struct nlm_lookup_host_info ni = {
> +		.peer		= NLM_CLIENT,
> +		.sin		= svc_addr_in(rqstp),
> +		.protocol	= rqstp->rq_prot,
> +		.version	= rqstp->rq_vers,
> +		.hostname	= hostname,
> +		.hostname_len	= hostname_len,
> +		.src_sin	= &source,
> +	};
> +
> +	dprintk("lockd: %s(host='%*s', vers=%u, proto=%s)\n", __func__,
> +			(int)hostname_len, hostname, rqstp->rq_vers,
> +			(rqstp->rq_prot == IPPROTO_UDP ? "udp" : "tcp"));
>  
> -	return nlm_lookup_host(1, svc_addr_in(rqstp),
> -			       rqstp->rq_prot, rqstp->rq_vers,
> -			       hostname, hostname_len, &source);
> +	return nlm_lookup_host(&ni);
>  }
>  
>  /*
> 
> --
> 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 1/7] NLM: Convert nlm_lookup_host() to use a single argument

[Chuck Lever]

On Oct 3, 2008, at Oct 3, 2008, 1:16 PM, Trond Myklebust wrote:
> On Fri, 2008-10-03 at 12:50 -0400, Chuck Lever wrote:
>> The nlm_lookup_host() function already has a large number of  
>> arguments,
>> and I'm about to add a few more.  As a clean up, convert the function
>> to use a single data structure argument.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>> fs/lockd/host.c |   89 +++++++++++++++++++++++++++++++++++ 
>> +-------------------
>> 1 files changed, 59 insertions(+), 30 deletions(-)
>>
>> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
>> index be8f19d..1630588 100644
>> --- a/fs/lockd/host.c
>> +++ b/fs/lockd/host.c
>> @@ -38,6 +38,20 @@ 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)
>> +
>
> Could we please get RID of this crap instead of 'documenting' it?  
> There
> is absolutely _no_ excuse for keeping a single list for both server  
> and
> client struct hosts...
>
> Strongly NACKED....

We haven't dropped that work item, but Bruce and I agreed months ago  
that he would handle the client/server list split *after* I had  
finished the IPv6 changes.  I think the IPv6 work is a much higher  
priority here.


>
>
>> +struct nlm_lookup_host_info {
>> +	const int		peer;		/* search for server|client */
>> +	const struct sockaddr_in *sin;		/* address to search for */
>> +	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_in *src_sin;	/* our address (optional) */
>> +	const size_t		src_len;	/* it's length */
>> +};
>> +
>> /*
>>  * Hash function must work well on big- and little-endian platforms
>>  */
>> @@ -121,23 +135,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))
>> @@ -150,22 +154,23 @@ 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((struct sockaddr *)ni->sin)];
>> 	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), (struct sockaddr *)ni->sin))
>> 			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->peer)
>> 			continue;
>> -		if (!nlm_cmp_addr(nlm_srcaddr(host), (struct sockaddr *)ssin))
>> +		if (!nlm_cmp_addr(nlm_srcaddr(host),
>> +					(struct sockaddr *)ni->src_sin))
>> 			continue;
>>
>> 		/* Move to head of hash chain. */
>> @@ -186,8 +191,9 @@ 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((struct sockaddr *)ni->sin,
>> +				sizeof(struct sockaddr_in),
>> +				ni->hostname, ni->hostname_len, 1);
>> 		if (!nsm) {
>> 			dprintk("lockd: nlm_lookup_host failed; "
>> 				"no nsm handle\n");
>> @@ -202,12 +208,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->sin, sizeof(struct sockaddr_in));
>> +	host->h_addrlen = sizeof(struct sockaddr_in);
>> 	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_sin, sizeof(struct sockaddr_in));
>> +	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;
>> @@ -218,7 +224,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->peer;
>> 	hlist_add_head(&host->h_hash, chain);
>> 	INIT_LIST_HEAD(&host->h_lockowners);
>> 	spin_lock_init(&host->h_lock);
>> @@ -273,9 +279,21 @@ struct nlm_host *nlmclnt_lookup_host(const  
>> struct sockaddr_in *sin,
>> 	const struct sockaddr_in source = {
>> 		.sin_family	= AF_UNSPEC,
>> 	};
>> +	struct nlm_lookup_host_info ni = {
>> +		.peer		= NLM_SERVER,
>> +		.sin		= sin,
>> +		.protocol	= proto,
>> +		.version	= version,
>> +		.hostname	= hostname,
>> +		.hostname_len	= hostname_len,
>> +		.src_sin	= &source,
>> +	};
>>
>> -	return nlm_lookup_host(0, sin, proto, version,
>> -			       hostname, hostname_len, &source);
>> +	dprintk("lockd: %s(host='%s', vers=%u, proto=%s)\n", __func__,
>> +			(hostname ? hostname : "<none>"), version,
>> +			(proto == IPPROTO_UDP ? "udp" : "tcp"));
>> +
>> +	return nlm_lookup_host(&ni);
>> }
>>
>> /*
>> @@ -289,10 +307,21 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
>> 		.sin_family	= AF_INET,
>> 		.sin_addr	= rqstp->rq_daddr.addr,
>> 	};
>> +	struct nlm_lookup_host_info ni = {
>> +		.peer		= NLM_CLIENT,
>> +		.sin		= svc_addr_in(rqstp),
>> +		.protocol	= rqstp->rq_prot,
>> +		.version	= rqstp->rq_vers,
>> +		.hostname	= hostname,
>> +		.hostname_len	= hostname_len,
>> +		.src_sin	= &source,
>> +	};
>> +
>> +	dprintk("lockd: %s(host='%*s', vers=%u, proto=%s)\n", __func__,
>> +			(int)hostname_len, hostname, rqstp->rq_vers,
>> +			(rqstp->rq_prot == IPPROTO_UDP ? "udp" : "tcp"));
>>
>> -	return nlm_lookup_host(1, svc_addr_in(rqstp),
>> -			       rqstp->rq_prot, rqstp->rq_vers,
>> -			       hostname, hostname_len, &source);
>> +	return nlm_lookup_host(&ni);
>> }
>>
>> /*
>>
>> --
>> 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
>

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 1/7] NLM: Convert nlm_lookup_host() to use a single argument

[Trond Myklebust]

On Fri, 2008-10-03 at 13:33 -0400, Chuck Lever wrote:
> >> +#define NLM_SERVER	(0)
> >> +#define NLM_CLIENT	(1)
> >> +
> >
> > Could we please get RID of this crap instead of 'documenting' it?  
> > There
> > is absolutely _no_ excuse for keeping a single list for both server  
> > and
> > client struct hosts...
> >
> > Strongly NACKED....
> 
> We haven't dropped that work item, but Bruce and I agreed months ago  
> that he would handle the client/server list split *after* I had  
> finished the IPv6 changes.  I think the IPv6 work is a much higher  
> priority here.

Then please don't throw in unnecessary trivia that can confuse people.

[PATCH 2/7] lockd: Support non-AF_INET addresses in nlm_lookup_host()

[Chuck Lever]

Use struct sockaddr * and length in nlm_lookup_host_info to all callers
to pass in either AF_INET or AF_INET6 addresses.

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

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

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 1630588..0a0ceb8 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -43,12 +43,13 @@ static struct nsm_handle	*nsm_find(const struct sockaddr *sap,
 
 struct nlm_lookup_host_info {
 	const int		peer;		/* search for server|client */
-	const struct sockaddr_in *sin;		/* address to search for */
+	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_in *src_sin;	/* our address (optional) */
+	const struct sockaddr	*src_sap;	/* our address (optional) */
 	const size_t		src_len;	/* it's length */
 };
 
@@ -154,9 +155,9 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 	 * 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 *)ni->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 *)ni->sin))
+		if (!nlm_cmp_addr(nlm_addr(host), ni->sap))
 			continue;
 
 		/* See if we have an NSM handle for this client */
@@ -169,8 +170,7 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 			continue;
 		if (host->h_server != ni->peer)
 			continue;
-		if (!nlm_cmp_addr(nlm_srcaddr(host),
-					(struct sockaddr *)ni->src_sin))
+		if (!nlm_cmp_addr(nlm_srcaddr(host), ni->src_sap))
 			continue;
 
 		/* Move to head of hash chain. */
@@ -191,8 +191,7 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 		atomic_inc(&nsm->sm_count);
 	else {
 		host = NULL;
-		nsm = nsm_find((struct sockaddr *)ni->sin,
-				sizeof(struct sockaddr_in),
+		nsm = nsm_find(ni->sap, ni->salen,
 				ni->hostname, ni->hostname_len, 1);
 		if (!nsm) {
 			dprintk("lockd: nlm_lookup_host failed; "
@@ -208,10 +207,10 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 		goto out;
 	}
 	host->h_name	   = nsm->sm_name;
-	memcpy(nlm_addr(host), ni->sin, sizeof(struct sockaddr_in));
-	host->h_addrlen = sizeof(struct sockaddr_in);
+	memcpy(nlm_addr(host), ni->sap, ni->salen);
+	host->h_addrlen = ni->salen;
 	nlm_clear_port(nlm_addr(host));
-	memcpy(nlm_srcaddr(host), ni->src_sin, sizeof(struct sockaddr_in));
+	memcpy(nlm_srcaddr(host), ni->src_sap, ni->src_len);
 	host->h_version    = ni->version;
 	host->h_proto      = ni->protocol;
 	host->h_rpcclnt    = NULL;
@@ -276,17 +275,19 @@ 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 = {
 		.peer		= NLM_SERVER,
-		.sin		= sin,
+		.sap		= (struct sockaddr *)sin,
+		.salen		= sizeof(*sin),
 		.protocol	= proto,
 		.version	= version,
 		.hostname	= hostname,
 		.hostname_len	= hostname_len,
-		.src_sin	= &source,
+		.src_sap	= &source,
+		.src_len	= sizeof(source),
 	};
 
 	dprintk("lockd: %s(host='%s', vers=%u, proto=%s)\n", __func__,
@@ -309,12 +310,14 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
 	};
 	struct nlm_lookup_host_info ni = {
 		.peer		= NLM_CLIENT,
-		.sin		= svc_addr_in(rqstp),
+		.sap		= svc_addr(rqstp),
+		.salen		= rqstp->rq_addrlen,
 		.protocol	= rqstp->rq_prot,
 		.version	= rqstp->rq_vers,
 		.hostname	= hostname,
 		.hostname_len	= hostname_len,
-		.src_sin	= &source,
+		.src_sap	= (struct sockaddr *)&source,
+		.src_len	= sizeof(source),
 	};
 
 	dprintk("lockd: %s(host='%*s', vers=%u, proto=%s)\n", __func__,

[PATCH 3/7] 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.

As a side benefit, eliminate the hostname_len argument, as the hostname
is always NUL-terminated.

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

 fs/lockd/clntlock.c         |    5 ++---
 fs/lockd/host.c             |   32 +++++++++++++++++++++-----------
 include/linux/lockd/lockd.h |    9 +++++----
 3 files changed, 28 insertions(+), 18 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 0a0ceb8..95a91e8 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -267,32 +267,42 @@ 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 = {
 		.peer		= 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: %s(host='%s', vers=%u, proto=%s)\n", __func__,
 			(hostname ? hostname : "<none>"), version,
-			(proto == IPPROTO_UDP ? "udp" : "tcp"));
+			(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 ec8af11..693d57b 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -215,10 +215,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 4/7] lockd: Adjust nlmsvc_lookup_host() to accomodate AF_INET6 addresses

[Chuck Lever]

Fix up nlmsvc_lookup_host() to pass AF_INET6 source addresses to
nlm_lookup_host().

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

 fs/lockd/host.c             |   47 +++++++++++++++++++++++++++++++++++--------
 include/linux/lockd/lockd.h |    5 +++--
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 95a91e8..541e157 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -307,16 +307,33 @@ 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.
+ *
+ * Before possibly creating a new nlm_host, construct a sockaddr
+ * for a specific source address in case the local system has
+ * multiple network 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 for
+ * the 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 = {
+	struct sockaddr_in sin = {
 		.sin_family	= AF_INET,
-		.sin_addr	= rqstp->rq_daddr.addr,
+	};
+	struct sockaddr_in6 sin6 = {
+		.sin6_family	= AF_INET6,
 	};
 	struct nlm_lookup_host_info ni = {
 		.peer		= NLM_CLIENT,
@@ -326,14 +343,26 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
 		.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__,
 			(int)hostname_len, hostname, rqstp->rq_vers,
 			(rqstp->rq_prot == IPPROTO_UDP ? "udp" : "tcp"));
 
+	switch (ni.sap->sa_family) {
+	case AF_INET:
+		sin.sin_addr.s_addr = rqstp->rq_daddr.addr.s_addr;
+		ni.src_sap = (struct sockaddr *)&sin;
+		break;
+	case AF_INET6:
+		ipv6_addr_copy(&sin6.sin6_addr, &rqstp->rq_daddr.addr6);
+		ni.src_sap = (struct sockaddr *)&sin6;
+		break;
+	default:
+		return NULL;
+	}
+
 	return nlm_lookup_host(&ni);
 }
 
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 693d57b..2e13c0b 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -220,8 +220,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 5/7] 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 4a714f6..89eb6f9 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -231,7 +231,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 76262c1..361aac2 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -261,7 +261,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 2e13c0b..165ef7a 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -207,7 +207,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 *);

[PATCH 6/7] lockd: Add helper to sanity check incoming NOTIFY requests

[Chuck Lever]

lockd accepts SM_NOTIFY calls only from a privileged process on the
local system.  If lockd uses an AF_INET6 listener, the sender's address
(ie the local rpc.statd) will be the IPv6 loopback address, not the
IPv4 loopback address.

Make sure the privilege test in nlmsvc_proc_sm_notify() and
nlm4svc_proc_sm_notify() works for both AF_INET and AF_INET6 family
addresses by refactoring the test into a helper and adding support for
IPv6 addresses.

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

 fs/lockd/svc4proc.c         |    6 ++----
 fs/lockd/svcproc.c          |    6 ++----
 include/linux/lockd/lockd.h |   41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/fs/lockd/svc4proc.c b/fs/lockd/svc4proc.c
index 89eb6f9..1ca972d 100644
--- a/fs/lockd/svc4proc.c
+++ b/fs/lockd/svc4proc.c
@@ -432,11 +432,9 @@ nlm4svc_proc_sm_notify(struct svc_rqst *rqstp, struct nlm_reboot *argp,
 {
 	struct sockaddr_in	saddr;
 
-	memcpy(&saddr, svc_addr_in(rqstp), sizeof(saddr));
-
 	dprintk("lockd: SM_NOTIFY     called\n");
-	if (saddr.sin_addr.s_addr != htonl(INADDR_LOOPBACK)
-	 || ntohs(saddr.sin_port) >= 1024) {
+
+	if (!nlm_privileged_requester(rqstp)) {
 		char buf[RPC_MAX_ADDRBUFLEN];
 		printk(KERN_WARNING "lockd: rejected NSM callback from %s\n",
 				svc_print_addr(rqstp, buf, sizeof(buf)));
diff --git a/fs/lockd/svcproc.c b/fs/lockd/svcproc.c
index 361aac2..212e4a3 100644
--- a/fs/lockd/svcproc.c
+++ b/fs/lockd/svcproc.c
@@ -464,11 +464,9 @@ nlmsvc_proc_sm_notify(struct svc_rqst *rqstp, struct nlm_reboot *argp,
 {
 	struct sockaddr_in	saddr;
 
-	memcpy(&saddr, svc_addr_in(rqstp), sizeof(saddr));
-
 	dprintk("lockd: SM_NOTIFY     called\n");
-	if (saddr.sin_addr.s_addr != htonl(INADDR_LOOPBACK)
-	 || ntohs(saddr.sin_port) >= 1024) {
+
+	if (!nlm_privileged_requester(rqstp)) {
 		char buf[RPC_MAX_ADDRBUFLEN];
 		printk(KERN_WARNING "lockd: rejected NSM callback from %s\n",
 				svc_print_addr(rqstp, buf, sizeof(buf)));
diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
index 165ef7a..888b92c 100644
--- a/include/linux/lockd/lockd.h
+++ b/include/linux/lockd/lockd.h
@@ -277,6 +277,47 @@ static inline struct inode *nlmsvc_file_inode(struct nlm_file *file)
 	return file->f_file->f_path.dentry->d_inode;
 }
 
+static inline int __nlm_privileged_request4(const struct sockaddr *sap)
+{
+	const struct sockaddr_in *sin = (struct sockaddr_in *)sap;
+	return (sin->sin_addr.s_addr == htonl(INADDR_LOOPBACK)) &&
+			(ntohs(sin->sin_port) < 1024);
+}
+
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+static inline int __nlm_privileged_request6(const struct sockaddr *sap)
+{
+	const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap;
+	return (ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LOOPBACK) &&
+			(ntohs(sin6->sin6_port) < 1024);
+}
+#else	/* defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) */
+static inline int __nlm_privileged_request6(const struct sockaddr *sap)
+{
+	return 0;
+}
+#endif	/* defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) */
+
+/*
+ * Ensure incoming requests are from local privileged callers.
+ *
+ * Return TRUE if sender is local and is connecting via a privileged port;
+ * otherwise return FALSE.
+ */
+static inline int nlm_privileged_requester(const struct svc_rqst *rqstp)
+{
+	const struct sockaddr *sap = svc_addr(rqstp);
+
+	switch (sap->sa_family) {
+	case AF_INET:
+		return __nlm_privileged_request4(sap);
+	case AF_INET6:
+		return __nlm_privileged_request6(sap);
+	default:
+		return 0;
+	}
+}
+
 static inline int __nlm_cmp_addr4(const struct sockaddr *sap1,
 				  const struct sockaddr *sap2)
 {

[PATCH 7/7] lockd: Remove unused fields in the nlm_reboot structure

[Chuck Lever]

The nlm_reboot structure is used to store information provided by the
NSM_NOTIFY procedure.  This procedure is not specified by the NLM or NSM
protocols, other than to say that the procedure can be used to transmit
information private to a particular NLM/NSM implementation.

For Linux, the callback arguments include the name of the monitored host,
the new NSM state of the host, and a 16-byte private opaque.

As a clean up, remove the unused fields and the server-side XDR logic that
decodes them.

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

 fs/lockd/xdr.c            |    2 --
 fs/lockd/xdr4.c           |    2 --
 include/linux/lockd/xdr.h |    2 --
 3 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/fs/lockd/xdr.c b/fs/lockd/xdr.c
index 3e459e1..1f22629 100644
--- a/fs/lockd/xdr.c
+++ b/fs/lockd/xdr.c
@@ -351,8 +351,6 @@ nlmsvc_decode_reboot(struct svc_rqst *rqstp, __be32 *p, struct nlm_reboot *argp)
 	argp->state = ntohl(*p++);
 	/* Preserve the address in network byte order */
 	argp->addr = *p++;
-	argp->vers = *p++;
-	argp->proto = *p++;
 	return xdr_argsize_check(rqstp, p);
 }
 
diff --git a/fs/lockd/xdr4.c b/fs/lockd/xdr4.c
index 43ff939..50c493a 100644
--- a/fs/lockd/xdr4.c
+++ b/fs/lockd/xdr4.c
@@ -358,8 +358,6 @@ nlm4svc_decode_reboot(struct svc_rqst *rqstp, __be32 *p, struct nlm_reboot *argp
 	argp->state = ntohl(*p++);
 	/* Preserve the address in network byte order */
 	argp->addr  = *p++;
-	argp->vers  = *p++;
-	argp->proto = *p++;
 	return xdr_argsize_check(rqstp, p);
 }
 
diff --git a/include/linux/lockd/xdr.h b/include/linux/lockd/xdr.h
index df18fa0..d6b3a80 100644
--- a/include/linux/lockd/xdr.h
+++ b/include/linux/lockd/xdr.h
@@ -81,8 +81,6 @@ struct nlm_reboot {
 	unsigned int	len;
 	u32		state;
 	__be32		addr;
-	__be32		vers;
-	__be32		proto;
 };
 
 /*

Re: [PATCH 0/7] Replacements for last week's patches

[J. Bruce Fields]

On Fri, Oct 03, 2008 at 12:49:58PM -0400, Chuck Lever wrote:
> Hi Bruce-
> 
> Here's a patch series to replace the patches you reviewed earlier this
> week.  I've addressed the issues you mentioned in your review comments,
> and dropped the NSM-related patches that treat the 16-byte opaque as an
> address, as discussed.
> 
> The last patches I intend to submit for 2.6.28 will address the lack of
> a client-side UDP lockd listener.  I will propose a fix later today in
> a subsequent patch series.

Thanks, applied to for-2.6.28, with the first patch adjusted as follows.

--b.

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index 1630588..3c4dc33 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -38,11 +38,8 @@ 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		peer;		/* search for server|client */
+	const int		server;		/* search for server|client */
 	const struct sockaddr_in *sin;		/* address to search for */
 	const unsigned short	protocol;	/* transport to search for*/
 	const u32		version;	/* NLM version to search for */
@@ -167,7 +164,7 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 			continue;
 		if (host->h_version != ni->version)
 			continue;
-		if (host->h_server != ni->peer)
+		if (host->h_server != ni->server)
 			continue;
 		if (!nlm_cmp_addr(nlm_srcaddr(host),
 					(struct sockaddr *)ni->src_sin))
@@ -224,7 +221,7 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
 	host->h_state      = 0;			/* pseudo NSM state */
 	host->h_nsmstate   = 0;			/* real NSM state */
 	host->h_nsmhandle  = nsm;
-	host->h_server	   = ni->peer;
+	host->h_server	   = ni->server;
 	hlist_add_head(&host->h_hash, chain);
 	INIT_LIST_HEAD(&host->h_lockowners);
 	spin_lock_init(&host->h_lock);
@@ -280,7 +277,7 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
 		.sin_family	= AF_UNSPEC,
 	};
 	struct nlm_lookup_host_info ni = {
-		.peer		= NLM_SERVER,
+		.server		= 0,
 		.sin		= sin,
 		.protocol	= proto,
 		.version	= version,
@@ -308,7 +305,7 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
 		.sin_addr	= rqstp->rq_daddr.addr,
 	};
 	struct nlm_lookup_host_info ni = {
-		.peer		= NLM_CLIENT,
+		.server		= 1,
 		.sin		= svc_addr_in(rqstp),
 		.protocol	= rqstp->rq_prot,
 		.version	= rqstp->rq_vers,

Re: [PATCH 0/7] Replacements for last week's patches

[J. Bruce Fields]

On Fri, Oct 03, 2008 at 05:45:58PM -0400, bfields wrote:
> On Fri, Oct 03, 2008 at 12:49:58PM -0400, Chuck Lever wrote:
> > Hi Bruce-
> > 
> > Here's a patch series to replace the patches you reviewed earlier this
> > week.  I've addressed the issues you mentioned in your review comments,
> > and dropped the NSM-related patches that treat the 16-byte opaque as an
> > address, as discussed.
> > 
> > The last patches I intend to submit for 2.6.28 will address the lack of
> > a client-side UDP lockd listener.  I will propose a fix later today in
> > a subsequent patch series.
> 
> Thanks, applied to for-2.6.28, with the first patch adjusted as follows.

(So the first patch now looks like this.)

--b.

commit 7f1ed18bd3aa1e8008cf5cc768a141787633da18
Author: Chuck Lever <chuck.lever@oracle.com>
Date:   Fri Oct 3 12:50:07 2008 -0400

    NLM: Convert nlm_lookup_host() to use a single argument
    
    The nlm_lookup_host() function already has a large number of arguments,
    and I'm about to add a few more.  As a clean up, convert the function
    to use a single data structure argument.
    
    Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
    Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index be8f19d..3c4dc33 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -38,6 +38,17 @@ static struct nsm_handle	*nsm_find(const struct sockaddr *sap,
 						const size_t hostname_len,
 						const int create);
 
+struct nlm_lookup_host_info {
+	const int		server;		/* search for server|client */
+	const struct sockaddr_in *sin;		/* address to search for */
+	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_in *src_sin;	/* our address (optional) */
+	const size_t		src_len;	/* it's length */
+};
+
 /*
  * Hash function must work well on big- and little-endian platforms
  */
@@ -121,23 +132,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))
@@ -150,22 +151,23 @@ 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((struct sockaddr *)ni->sin)];
 	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), (struct sockaddr *)ni->sin))
 			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),
+					(struct sockaddr *)ni->src_sin))
 			continue;
 
 		/* Move to head of hash chain. */
@@ -186,8 +188,9 @@ 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((struct sockaddr *)ni->sin,
+				sizeof(struct sockaddr_in),
+				ni->hostname, ni->hostname_len, 1);
 		if (!nsm) {
 			dprintk("lockd: nlm_lookup_host failed; "
 				"no nsm handle\n");
@@ -202,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->sin, sizeof(struct sockaddr_in));
+	host->h_addrlen = sizeof(struct sockaddr_in);
 	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_sin, sizeof(struct sockaddr_in));
+	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;
@@ -218,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);
@@ -273,9 +276,21 @@ struct nlm_host *nlmclnt_lookup_host(const struct sockaddr_in *sin,
 	const struct sockaddr_in source = {
 		.sin_family	= AF_UNSPEC,
 	};
+	struct nlm_lookup_host_info ni = {
+		.server		= 0,
+		.sin		= sin,
+		.protocol	= proto,
+		.version	= version,
+		.hostname	= hostname,
+		.hostname_len	= hostname_len,
+		.src_sin	= &source,
+	};
 
-	return nlm_lookup_host(0, sin, proto, version,
-			       hostname, hostname_len, &source);
+	dprintk("lockd: %s(host='%s', vers=%u, proto=%s)\n", __func__,
+			(hostname ? hostname : "<none>"), version,
+			(proto == IPPROTO_UDP ? "udp" : "tcp"));
+
+	return nlm_lookup_host(&ni);
 }
 
 /*
@@ -289,10 +304,21 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
 		.sin_family	= AF_INET,
 		.sin_addr	= rqstp->rq_daddr.addr,
 	};
+	struct nlm_lookup_host_info ni = {
+		.server		= 1,
+		.sin		= svc_addr_in(rqstp),
+		.protocol	= rqstp->rq_prot,
+		.version	= rqstp->rq_vers,
+		.hostname	= hostname,
+		.hostname_len	= hostname_len,
+		.src_sin	= &source,
+	};
+
+	dprintk("lockd: %s(host='%*s', vers=%u, proto=%s)\n", __func__,
+			(int)hostname_len, hostname, rqstp->rq_vers,
+			(rqstp->rq_prot == IPPROTO_UDP ? "udp" : "tcp"));
 
-	return nlm_lookup_host(1, svc_addr_in(rqstp),
-			       rqstp->rq_prot, rqstp->rq_vers,
-			       hostname, hostname_len, &source);
+	return nlm_lookup_host(&ni);
 }
 
 /*

Re: [PATCH 0/7] Replacements for last week's patches

[Chuck Lever]

On Oct 3, 2008, at Oct 3, 2008, 5:45 PM, J. Bruce Fields wrote:
> On Fri, Oct 03, 2008 at 12:49:58PM -0400, Chuck Lever wrote:
>> Hi Bruce-
>>
>> Here's a patch series to replace the patches you reviewed earlier  
>> this
>> week.  I've addressed the issues you mentioned in your review  
>> comments,
>> and dropped the NSM-related patches that treat the 16-byte opaque  
>> as an
>> address, as discussed.
>>
>> The last patches I intend to submit for 2.6.28 will address the  
>> lack of
>> a client-side UDP lockd listener.  I will propose a fix later today  
>> in
>> a subsequent patch series.
>
> Thanks, applied to for-2.6.28, with the first patch adjusted as  
> follows.

OK, thanks.

>
>
> --b.
>
> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
> index 1630588..3c4dc33 100644
> --- a/fs/lockd/host.c
> +++ b/fs/lockd/host.c
> @@ -38,11 +38,8 @@ 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		peer;		/* search for server|client */
> +	const int		server;		/* search for server|client */
> 	const struct sockaddr_in *sin;		/* address to search for */
> 	const unsigned short	protocol;	/* transport to search for*/
> 	const u32		version;	/* NLM version to search for */
> @@ -167,7 +164,7 @@ static struct nlm_host *nlm_lookup_host(struct  
> nlm_lookup_host_info *ni)
> 			continue;
> 		if (host->h_version != ni->version)
> 			continue;
> -		if (host->h_server != ni->peer)
> +		if (host->h_server != ni->server)
> 			continue;
> 		if (!nlm_cmp_addr(nlm_srcaddr(host),
> 					(struct sockaddr *)ni->src_sin))
> @@ -224,7 +221,7 @@ static struct nlm_host *nlm_lookup_host(struct  
> nlm_lookup_host_info *ni)
> 	host->h_state      = 0;			/* pseudo NSM state */
> 	host->h_nsmstate   = 0;			/* real NSM state */
> 	host->h_nsmhandle  = nsm;
> -	host->h_server	   = ni->peer;
> +	host->h_server	   = ni->server;
> 	hlist_add_head(&host->h_hash, chain);
> 	INIT_LIST_HEAD(&host->h_lockowners);
> 	spin_lock_init(&host->h_lock);
> @@ -280,7 +277,7 @@ struct nlm_host *nlmclnt_lookup_host(const  
> struct sockaddr_in *sin,
> 		.sin_family	= AF_UNSPEC,
> 	};
> 	struct nlm_lookup_host_info ni = {
> -		.peer		= NLM_SERVER,
> +		.server		= 0,
> 		.sin		= sin,
> 		.protocol	= proto,
> 		.version	= version,
> @@ -308,7 +305,7 @@ nlmsvc_lookup_host(struct svc_rqst *rqstp,
> 		.sin_addr	= rqstp->rq_daddr.addr,
> 	};
> 	struct nlm_lookup_host_info ni = {
> -		.peer		= NLM_CLIENT,
> +		.server		= 1,
> 		.sin		= svc_addr_in(rqstp),
> 		.protocol	= rqstp->rq_prot,
> 		.version	= rqstp->rq_vers,

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com