1) 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.
2) The kernel changes must be accompanied by userspace changes
in nfs-utils.
Signed-off-by: Harshula Jayasuriya <[email protected]>
---
include/linux/sunrpc/cache.h | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 6ce690d..437ddb6 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -264,12 +264,30 @@ static inline int get_uint(char **bpp, unsigned int *anint)
return 0;
}
+static inline int get_time(char **bpp, time_t *time)
+{
+ char buf[50];
+ long long ll;
+ int len = qword_get(bpp, buf, sizeof(buf));
+
+ if (len < 0)
+ return -EINVAL;
+ if (len == 0)
+ return -ENOENT;
+
+ if (kstrtoll(buf, 0, &ll))
+ return -EINVAL;
+
+ *time = (time_t)ll;
+ return 0;
+}
+
static inline time_t get_expiry(char **bpp)
{
- int rv;
+ time_t rv;
struct timespec boot;
- if (get_int(bpp, &rv))
+ if (get_time(bpp, &rv))
return 0;
if (rv < 0)
return 0;
--
1.8.3.1
On Fri, Aug 16, 2013 at 03:46:40AM +1000, Harshula Jayasuriya wrote:
> 1) 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.
Anyone expecting not to have to upgrade their nfs server once between
now and 2038 is interesting, but... yes, may as well get it fixed now,
OK.
> 2) The kernel changes must be accompanied by userspace changes
> in nfs-utils.
This all looks backwards-compatible, so I assume that "must" is only if
you want this particular bug fixed.
Applying for 3.12 absent any objections.
--b.
>
> Signed-off-by: Harshula Jayasuriya <[email protected]>
> ---
> include/linux/sunrpc/cache.h | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
> index 6ce690d..437ddb6 100644
> --- a/include/linux/sunrpc/cache.h
> +++ b/include/linux/sunrpc/cache.h
> @@ -264,12 +264,30 @@ static inline int get_uint(char **bpp, unsigned int *anint)
> return 0;
> }
>
> +static inline int get_time(char **bpp, time_t *time)
> +{
> + char buf[50];
> + long long ll;
> + int len = qword_get(bpp, buf, sizeof(buf));
> +
> + if (len < 0)
> + return -EINVAL;
> + if (len == 0)
> + return -ENOENT;
> +
> + if (kstrtoll(buf, 0, &ll))
> + return -EINVAL;
> +
> + *time = (time_t)ll;
> + return 0;
> +}
> +
> static inline time_t get_expiry(char **bpp)
> {
> - int rv;
> + time_t rv;
> struct timespec boot;
>
> - if (get_int(bpp, &rv))
> + if (get_time(bpp, &rv))
> return 0;
> if (rv < 0)
> return 0;
> --
> 1.8.3.1
>
On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>
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 <[email protected]>
> ---
> 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))
??
NeilBrown
> +
> + 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;
> }
On Mon, 2013-11-11 at 18:53 +1100, NeilBrown wrote:
> On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>
> 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 <[email protected]>
> > ---
> > 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.
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;
> > }
On 17/11/13 18:45, Harshula Jayasuriya wrote:
> This patch is the nfs-utils patch corresponding to the kernel patch
> "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 <[email protected]>
Committed (tag: nfs-utils-1-2-10-rc1)
steved.
> ---
> utils/exportfs/exportfs.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index da5fe21..9f13858 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,34 @@ 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;
> +
> + /* We introduce tolerance of 1 day to ensure that we use a
> + * LONG_MAX for the expiry timestamp before it is actually
> + * needed. To use LONG_MAX, the kernel code must have commit
> + * 2f74f972d4cc7d83408ea0c32d424edcb44887bf
> + * (sunrpc: prepare NFS for 2038).
> + */
> +#define INT_TO_LONG_THRESHOLD_SECS (INT_MAX - (60 * 60 * 24))
> + if (time(NULL) > INT_TO_LONG_THRESHOLD_SECS)
> + 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);
> +
> + 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;
> }
>
On Fri, 15 Nov 2013 17:15:33 +1100 Harshula Jayasuriya <[email protected]>
wrote:
> 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 <[email protected]>
> > > 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 <[email protected]>
> > > > ---
> > > > 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.
The problem scenario that I imagine is:
> > > > + if (time(NULL) > INT_MAX)
happens towards the end ofthe second when time(NULL) == INT_MAX. The test
fails and INT_MAX is stored in 'buf'.
By the time buf gets written to the kernel, time has passed and so the kernel
sees that the entry written has passed.
So an already-expired entry gets stored in the cache, which is clearly
pointless and could conceivably result in the in the subsequent
'test_export()' call failing incorrectly.
It is admittedly a tiny window and it may not cause a problem.
It just seems to me that waiting until the very last moment to switch over to
used the new max-time value is pointless brinkmanship.
It is obviously correct to use LONG_MAX a day (or even a month) before the
change over. So just do it and don't worry about trying to prove whether it
is actually necessary or not.
NeilBrown
> 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 [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Fri, 2013-11-15 at 17:38 +1100, NeilBrown wrote:
> On Fri, 15 Nov 2013 17:15:33 +1100 Harshula Jayasuriya <[email protected]>
> wrote:
>
> > 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 <[email protected]>
> > > > 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 <[email protected]>
> > > > > ---
> > > > > 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.
>
> The problem scenario that I imagine is:
>
> > > > > + if (time(NULL) > INT_MAX)
>
> happens towards the end ofthe second when time(NULL) == INT_MAX. The test
> fails and INT_MAX is stored in 'buf'.
>
> By the time buf gets written to the kernel, time has passed and so the kernel
> sees that the entry written has passed.
> So an already-expired entry gets stored in the cache, which is clearly
> pointless and could conceivably result in the in the subsequent
> 'test_export()' call failing incorrectly.
True.
> It is admittedly a tiny window and it may not cause a problem.
> It just seems to me that waiting until the very last moment to switch over to
> used the new max-time value is pointless brinkmanship.
> It is obviously correct to use LONG_MAX a day (or even a month) before the
> change over. So just do it and don't worry about trying to prove whether it
> is actually necessary or not.
Sure :-)
cya,
#
> NeilBrown
>
>
>
> > 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 [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
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 <[email protected]>
---
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);
+
+ 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;
}
--
1.8.3.1
This patch is the nfs-utils patch corresponding to the kernel patch
"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 <[email protected]>
---
utils/exportfs/exportfs.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index da5fe21..9f13858 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,34 @@ 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;
+
+ /* We introduce tolerance of 1 day to ensure that we use a
+ * LONG_MAX for the expiry timestamp before it is actually
+ * needed. To use LONG_MAX, the kernel code must have commit
+ * 2f74f972d4cc7d83408ea0c32d424edcb44887bf
+ * (sunrpc: prepare NFS for 2038).
+ */
+#define INT_TO_LONG_THRESHOLD_SECS (INT_MAX - (60 * 60 * 24))
+ if (time(NULL) > INT_TO_LONG_THRESHOLD_SECS)
+ 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);
+
+ 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;
}
--
1.8.3.1
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 <[email protected]>
> > 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 <[email protected]>
> > > ---
> > > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html