RFC: nlmclnt_grant() address check vs. callback address binding

RFC: nlmclnt_grant() address check vs. callback address binding

[Frank van Maarseveen]

Occasionally an NFS server needs to create an RPC transport for client
callbacks, e.g. when granting a blocked lock request. When a server
has multiple IP addresses (multihomed/multiple NICs or on a single
interface) then the client may be called back from a different address
because the server is free to select one: xs_udp_connect_worker() and
xs_tcp_connect_worker() call xs_bindresvport() only when a reserved port
is requested and xs_bindresvport() implicitly uses INADDR_ANY.

Currently this breaks locking when the server has more than one IP address
reachable from the client because nlmclnt_grant() on the client rejects
GRANT_MSG when the source IP address doesn't match the destination
address in the original request. This is happening in a real-world
situation where NFS servers have been virtualized using different IP
addresses on a single machine.

Basically there are two ways to fix this:
1.	server callbacks should bind to a local address, not INADDR_ANY.
2.	clients should accept callbacks from a different server address.

At first I thought fixing the server is the right thing to do because
the server behavior is arguably incorrect. Not easy but certainly doable
(I'd do it if it has a chance of being accepted). It might also help
NATting NFS traffic in some cases. However:

-	I'm not sure if an address check such as in nlmclnt_grant()
	is actually buying us anything.
-	clients requiring a correct callback src address might not
	work with other NFS server implementations and/or (future?)
	concepts for clustering/load balancing.
-	Multiple IP addresses may be a workaround for reserved port
	shortage.

As it currently stands, mounting from a secondary server IP address is
broken w.r.t. locking when both client and server run linux.

Any opinion about (1) and (2)? maybe we should do both?

Currently I use (2) as a workaround:

--- a/fs/lockd/clntlock.c	2007-02-27 22:55:27.000000000 +0100
+++ b/fs/lockd/clntlock.c	2007-07-02 17:36:29.000000000 +0200
@@ -124,8 +124,15 @@ __be32 nlmclnt_grant(const struct sockad
 		 */
 		if (fl_blocked->fl_u.nfs_fl.owner->pid != lock->svid)
 			continue;
-		if (!nlm_cmp_addr(&block->b_host->h_addr, addr))
-			continue;
+		/*
+		 * The granted message may come from a different IP address
+		 * when the server has more than one. Warn when debug is on.
+		 */
+		if (!nlm_cmp_addr(&block->b_host->h_addr, addr)) {
+			dprintk("nlmclnt_grant: "NIPQUAD_FMT" != "NIPQUAD_FMT"\n",
+				NIPQUAD(block->b_host->h_addr.sin_addr.s_addr),
+				NIPQUAD(addr->sin_addr.s_addr));
+		}
 		if (nfs_compare_fh(NFS_FH(fl_blocked->fl_file->f_path.dentry->d_inode) ,fh) != 0)
 			continue;
 		/* Alright, we found a lock. Set the return status

-- 
Frank

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: RFC: nlmclnt_grant() address check vs. callback address binding

[Trond Myklebust]

On Mon, 2007-07-02 at 17:49 +0200, Frank van Maarseveen wrote:

> At first I thought fixing the server is the right thing to do because
> the server behavior is arguably incorrect. Not easy but certainly doable
> (I'd do it if it has a chance of being accepted). It might also help
> NATting NFS traffic in some cases. However:
> 
> -	I'm not sure if an address check such as in nlmclnt_grant()
> 	is actually buying us anything.

Exactly how else is the client going to figure out which lock this
corresponds to?

> -	clients requiring a correct callback src address might not
> 	work with other NFS server implementations and/or (future?)
> 	concepts for clustering/load balancing.

NFSv3 development is dead. NLM locking development is doubly so...

> -	Multiple IP addresses may be a workaround for reserved port
> 	shortage.

There is _NO_ way for the client to figure out what lock the granted
request refers to if the server starts transmitting from random IP
addresses.
      * There is no universal namespace for filehandles that can be used
        to identify which file the granted request is meant for
      * The cookie is not guaranteed to be the same as that sent by the
        client for the LOCK request
      * There is nothing else in the nlm_lock that can be used to
        identify which file we're talking about.

Trond


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: RFC: nlmclnt_grant() address check vs. callback address binding

[Frank van Maarseveen]

On Mon, Jul 02, 2007 at 09:04:30PM -0400, Trond Myklebust wrote:
> On Mon, 2007-07-02 at 17:49 +0200, Frank van Maarseveen wrote:
> 
> > At first I thought fixing the server is the right thing to do because
> > the server behavior is arguably incorrect. Not easy but certainly doable
> > (I'd do it if it has a chance of being accepted). It might also help
> > NATting NFS traffic in some cases. However:
> > 
> > -	I'm not sure if an address check such as in nlmclnt_grant()
> > 	is actually buying us anything.
> 
> Exactly how else is the client going to figure out which lock this
> corresponds to?

The filehandle + pid/svid + lock start + lock end as it is doing right
now. Filehandles offer a pretty strong check already because they are
designed to handle inode reuse too.

The 32 bit pid/svid/lockowner number is conclusive.


> 
> > -	clients requiring a correct callback src address might not
> > 	work with other NFS server implementations and/or (future?)
> > 	concepts for clustering/load balancing.
> 
> NFSv3 development is dead. NLM locking development is doubly so...

Not everyone is using (or can use) NFSv4. This is not about developing
NFSv3 but about optimizing/fixing it in its current deployment.


> 
> > -	Multiple IP addresses may be a workaround for reserved port
> > 	shortage.
> 
> There is _NO_ way for the client to figure out what lock the granted
> request refers to if the server starts transmitting from random IP
> addresses.
>       * There is no universal namespace for filehandles that can be used
>         to identify which file the granted request is meant for
>       * The cookie is not guaranteed to be the same as that sent by the
>         client for the LOCK request
>       * There is nothing else in the nlm_lock that can be used to
>         identify which file we're talking about.

The svid/pid. See nlm_find_lockowner().

-- 
Frank

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: RFC: nlmclnt_grant() address check vs. callback address binding

[Trond Myklebust]

On Tue, 2007-07-03 at 09:01 +0200, Frank van Maarseveen wrote:

> > There is _NO_ way for the client to figure out what lock the granted
> > request refers to if the server starts transmitting from random IP
> > addresses.
> >       * There is no universal namespace for filehandles that can be used
> >         to identify which file the granted request is meant for
> >       * The cookie is not guaranteed to be the same as that sent by the
> >         client for the LOCK request
> >       * There is nothing else in the nlm_lock that can be used to
> >         identify which file we're talking about.
> 
> The svid/pid. See nlm_find_lockowner().

nlm_find_lockowner() allocates a svid/pid that is unique only on a
per-server basis.

Trond


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: RFC: nlmclnt_grant() address check vs. callback address binding

[Frank van Maarseveen]

On Tue, Jul 03, 2007 at 08:03:26AM -0400, Trond Myklebust wrote:
> On Tue, 2007-07-03 at 09:01 +0200, Frank van Maarseveen wrote:
> 
> > > There is _NO_ way for the client to figure out what lock the granted
> > > request refers to if the server starts transmitting from random IP
> > > addresses.
> > >       * There is no universal namespace for filehandles that can be used
> > >         to identify which file the granted request is meant for
> > >       * The cookie is not guaranteed to be the same as that sent by the
> > >         client for the LOCK request
> > >       * There is nothing else in the nlm_lock that can be used to
> > >         identify which file we're talking about.
> > 
> > The svid/pid. See nlm_find_lockowner().
> 
> nlm_find_lockowner() allocates a svid/pid that is unique only on a
> per-server basis.

Reading code... hmm, yes. One global 32 bit pid instead of one per server
would remove that problem. Note that this is still a very theoretical
problem because of all the other checks (filehandle check being the most
important one probably).

-- 
Frank

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: RFC: nlmclnt_grant() address check vs. callback address binding

[Trond Myklebust]

On Tue, 2007-07-03 at 22:47 +0200, Frank van Maarseveen wrote
> Reading code... hmm, yes. One global 32 bit pid instead of one per server
> would remove that problem. Note that this is still a very theoretical
> problem because of all the other checks (filehandle check being the most
> important one probably).

Fix the server instead... There is absolutely no reason to be accepting
GRANTED calls from random IP addresses.

Trond


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: RFC: nlmclnt_grant() address check vs. callback address binding

[J. Bruce Fields]

On Mon, Jul 02, 2007 at 05:49:18PM +0200, Frank van Maarseveen wrote:
> At first I thought fixing the server is the right thing to do because
> the server behavior is arguably incorrect. Not easy but certainly doable
> (I'd do it if it has a chance of being accepted).

Makes sense to me.

--b.

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs