Re: [PATCH] exportfs: modify can_test() to use LONG_MAX when appropriate

[Harshula Jayasuriya <harshula@redhat.com>]

On Mon, 2013-11-11 at 22:20 +1100, Harshula Jayasuriya wrote:
> On Mon, 2013-11-11 at 18:53 +1100, NeilBrown wrote:
> > On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <harshula@redhat.com>
> > wrote:
> > 
> > > This patch is the nfs-utils patch corresponding to the kernel patch 
> > > commit 2f74f972d4cc7d83408ea0c32d424edcb44887bf "sunrpc: prepare NFS 
> > > for 2038": "The kernel sunrpc code needs to handle seconds since epoch 
> > > greater than 2147483647. This means functions that parse time as an 
> > > int need to handle it as time_t."
> > > 
> > > When appropriate exportfs should use LONG_MAX in can_test() instead of
> > > INT_MAX.
> > > 
> > > kernel INT_MAX + exportfs INT_MAX: "Tue Jan 19 03:14:08 UTC 2038"
> > > =================================================================
> > > 
> > > exportfs: /mnt/export does not support NFS export:
> > > ------------------------------------------------------------
> > >  # cat /proc/net/rpc/auth.unix.ip/content
> > >  #class IP domain
> > > ------------------------------------------------------------
> > > 
> > > + mount fail:
> > > ------------------------------------------------------------
> > >  # cat /proc/net/rpc/auth.unix.ip/content
> > >  #class IP domain
> > >  # expiry=2147483768 refcnt=2 flags=4
> > >  # nfsd 192.168.1.6 -no-domain-
> > > ------------------------------------------------------------
> > > 
> > > kernel LONG_MAX + exportfs INT_MAX: "Tue Jan 19 03:14:08 UTC 2038"
> > > ==================================================================
> > > 
> > > "exportfs: /mnt/export does not support NFS export":
> > > ------------------------------------------------------------
> > >  # cat /proc/net/rpc/auth.unix.ip/content
> > >  #class IP domain
> > > ------------------------------------------------------------
> > > 
> > > + mount success:
> > > ------------------------------------------------------------
> > >  # cat /proc/net/rpc/auth.unix.ip/content
> > >  #class IP domain
> > >  # expiry=2147485448 refcnt=2 flags=1
> > >  nfsd 192.168.1.6 *
> > > ------------------------------------------------------------
> > > 
> > > kernel LONG_MAX + exportfs LONG_MAX: "Tue Jan 19 03:14:08 UTC 2038"
> > > ===================================================================
> > > 
> > > exportfs:
> > > ------------------------------------------------------------
> > >  # cat /proc/net/rpc/auth.unix.ip/content
> > >  #class IP domain
> > >  # expiry=9223372036854775807 refcnt=1 flags=1
> > >  nfsd 0.0.0.0 -test-client-
> > > ------------------------------------------------------------
> > > 
> > > + mount success:
> > > ------------------------------------------------------------
> > >  # cat /proc/net/rpc/auth.unix.ip/content
> > >  #class IP domain
> > >  # expiry=2147485448 refcnt=2 flags=1
> > >  nfsd 192.168.1.6 *
> > >  # expiry=9223372036854775807 refcnt=1 flags=1
> > >  nfsd 0.0.0.0 -test-client-
> > > ------------------------------------------------------------
> > > 
> > > Signed-off-by: Harshula Jayasuriya <harshula@redhat.com>
> > > ---
> > >  utils/exportfs/exportfs.c | 20 ++++++++++++++++----
> > >  1 file changed, 16 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> > > index da5fe21..bccbed5 100644
> > > --- a/utils/exportfs/exportfs.c
> > > +++ b/utils/exportfs/exportfs.c
> > > @@ -27,6 +27,8 @@
> > >  #include <netdb.h>
> > >  #include <errno.h>
> > >  #include <dirent.h>
> > > +#include <limits.h>
> > > +#include <time.h>
> > >  
> > >  #include "sockaddr.h"
> > >  #include "misc.h"
> > > @@ -406,17 +408,27 @@ unexportfs(char *arg, int verbose)
> > >  
> > >  static int can_test(void)
> > >  {
> > > +	char buf[1024];
> > >  	int fd;
> > >  	int n;
> > > -	char *setup = "nfsd 0.0.0.0 2147483647 -test-client-\n";
> > > +
> > >  	fd = open("/proc/net/rpc/auth.unix.ip/channel", O_WRONLY);
> > > -	if ( fd < 0) return 0;
> > > -	n = write(fd, setup, strlen(setup));
> > > +	if (fd < 0)
> > > +		return 0;
> > > +
> > > +	if (time(NULL) > INT_MAX)
> > > +		sprintf(buf, "nfsd 0.0.0.0 %ld -test-client-\n", LONG_MAX);
> > > +	else
> > > +		sprintf(buf, "nfsd 0.0.0.0 %d -test-client-\n", INT_MAX);
> > 
> > This generally makes sense, but I think the cut-off is a bit too fine.
> > If time(NULL) is exactly INT_MAX, then this might not do what is required.
> > How about
> >    if (time(NULL) > INT_MAX - (24*3600))
> > ??
> 
> Good point. Mount attempts seem to fail not because of when exportfs is
> run, but when the first mount request is made.

Clarification, what I said above is only true when the kernel does *not*
have commit 2f74f972d4cc7d83408ea0c32d424edcb44887bf.

I did a few more tests with kernel commit
2f74f972d4cc7d83408ea0c32d424edcb44887bf and found:

1) When the mount attempt arrives before the expiry time, set in
can_test(), is reached, /proc/net/rpc/auth.unix.ip/content will contain
the entry for the real mount attempt and test_export()'s "-test-client-"
entry. If the mount attempt arrives after the expiry time, only the
entry for the real mount attempt will appear. In both cases the mount
attempt succeeds.

2) If the expiry time, set in can_test(), is older than the time when
exportfs is run, then test_export() will fail (write
to /proc/net/rpc/nfsd.export/channel fails) and validate_export() will
print the following error "exportfs: /mnt/export does not support NFS
export". However, the mount attempt still succeeds.

Neil, have I missed something here? It looks like we don't need to add
the tolerance to the time check.

cya,
#

> Is 1 day the right amount of tolerance? Or should we just go for a much
> larger tolerance, on the assumption that the running kernel will have
> commit 2f74f972d4cc7d83408ea0c32d424edcb44887bf by then? :-)
> 
> cya,
> #
> 
> 
> > > +
> > > +	n = write(fd, buf, strlen(buf));
> > >  	close(fd);
> > >  	if (n < 0)
> > >  		return 0;
> > > +
> > >  	fd = open("/proc/net/rpc/nfsd.export/channel", O_WRONLY);
> > > -	if ( fd < 0) return 0;
> > > +	if (fd < 0)
> > > +		return 0;
> > >  	close(fd);
> > >  	return 1;
> > >  }
> 
> --
> 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