sunrpc: prepare NFS for 2038

PATCH In review Harshula Jayasuriya 4 participants 10 messages Aug 15, 2013 – Nov 20, 2013

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Harshula Jayasuriya

author

2013-08-15 17:46 UTC #0 lore raw

PATCH 1/1 sunrpc: prepare NFS for 2038

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(-)

include/linux/sunrpc/cache.h +20 -2

index 6ce690d..437ddb6 100644
@@ -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;

Signature · 1 line

1.8.3.1

J.Bruce Fields

2013-08-15 17:52 UTC #1 lore raw

Quoted 3 lines · Harshula Jayasuriya

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.

Quoted 2 lines

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.

Quoted 46 lines

Signed-off-by Harshula Jayasuriya <[email protected]>

---
 include/linux/sunrpc/cache.h | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

include/linux/sunrpc/cache.h +20 -2

index 6ce690d..437ddb6 100644
@@ -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;

Signature · 1 line

1.8.3.1

Harshula Jayasuriya

author

2013-11-11 07:20 UTC #2 lore raw

PATCH 1/1 exportfs: modify can_test() to use LONG_MAX when appropriate

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(-)

utils/exportfs/exportfs.c +16 -4

index da5fe21..bccbed5 100644
@@ -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;
 }

Signature · 1 line

1.8.3.1

NeilBrown

2013-11-11 07:54 UTC #3 lore raw

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

Quoted 91 lines · On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>…

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(-)
utils/exportfs/exportfs.c +4 -1
index da5fe21..bccbed5 100644
@@ -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";
+

utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 422

Quoted 10 lines

utils/exportfs/exportfs.c +7 -2

 	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

Quoted 13 lines

utils/exportfs/exportfs.c +5 -1

+
+	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;
 }

Harshula Jayasuriya

author

2013-11-11 11:20 UTC #4 lore raw

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

Quoted 102 lines · NeilBrown

Quoted 99 lines · On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>…
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(-)
utils/exportfs/exportfs.c +9 -3
index da5fe21..bccbed5 100644
@@ -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);

Quoted 8 lines

utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 422
Quoted 2 lines
utils/exportfs/exportfs.c +2 -0
+	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,
#

Quoted 13 lines

utils/exportfs/exportfs.c +5 -1

+
+	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;
 }

Harshula Jayasuriya

author

2013-11-15 06:15 UTC #5 lore raw

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

Quoted 112 lines · Harshula Jayasuriya

Quoted 102 lines · NeilBrown
Quoted 99 lines · On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>…
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(-)
utils/exportfs/exportfs.c +9 -3
index da5fe21..bccbed5 100644
@@ -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);
Quoted 8 lines
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 422
Quoted 2 lines
utils/exportfs/exportfs.c +2 -0
+	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))
??

Quoted 2 lines

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,
#

Quoted 26 lines

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,
#
Quoted 5 lines
Quoted 5 lines
utils/exportfs/exportfs.c +2 -0
+
+	n = write(fd, buf, strlen(buf));
 	close(fd);
 	if (n < 0)
 		return 0;
Quoted 8 lines
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 434
Quoted 8 lines
utils/exportfs/exportfs.c +3 -1
+
 	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

NeilBrown

2013-11-15 06:39 UTC #6 lore raw

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

Quoted 129 lines · On Fri, 15 Nov 2013 17:15:33 +1100 Harshula Jayasuriya <[email protected]>…

Quoted 112 lines · Harshula Jayasuriya
Quoted 102 lines · NeilBrown
Quoted 99 lines · On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>…
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(-)
utils/exportfs/exportfs.c +9 -3
index da5fe21..bccbed5 100644
@@ -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);
Quoted 8 lines
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 422
Quoted 2 lines
utils/exportfs/exportfs.c +2 -0
+	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))
??
Quoted 2 lines
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.

Quoted 8 lines

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:

Quoted 1 line

Quoted 1 line
Quoted 1 line
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 419
Quoted 1 line
utils/exportfs/exportfs.c +1 -0
+	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

Quoted 30 lines

cya,
#
Quoted 26 lines
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,
#
Quoted 5 lines
Quoted 5 lines
utils/exportfs/exportfs.c +2 -0
+
+	n = write(fd, buf, strlen(buf));
 	close(fd);
 	if (n < 0)
 		return 0;
Quoted 8 lines
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 434
Quoted 8 lines
utils/exportfs/exportfs.c +3 -1
+
 	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

Harshula Jayasuriya

author

2013-11-15 10:39 UTC #7 lore raw

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

Quoted 148 lines · NeilBrown

Quoted 129 lines · On Fri, 15 Nov 2013 17:15:33 +1100 Harshula Jayasuriya <[email protected]>…
Quoted 112 lines · Harshula Jayasuriya
Quoted 102 lines · NeilBrown
Quoted 99 lines · On Mon, 11 Nov 2013 18:20:34 +1100 Harshula Jayasuriya <[email protected]>…
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(-)
utils/exportfs/exportfs.c +9 -3
index da5fe21..bccbed5 100644
@@ -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);
Quoted 8 lines
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 422
Quoted 2 lines
utils/exportfs/exportfs.c +2 -0
+	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))
??
Quoted 2 lines
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.
Quoted 8 lines
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:
Quoted 1 line
Quoted 1 line
Quoted 1 line
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 419
Quoted 1 line
utils/exportfs/exportfs.c +1 -0
+	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'.

Quoted 5 lines

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.

Quoted 6 lines

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,
#

Quoted 35 lines

NeilBrown
Quoted 30 lines
cya,
#
Quoted 26 lines
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,
#
Quoted 5 lines
Quoted 5 lines
utils/exportfs/exportfs.c +2 -0
+
+	n = write(fd, buf, strlen(buf));
 	close(fd);
 	if (n < 0)
 		return 0;
Quoted 8 lines
utils/exportfs/exportfs.c unexportfs(char *arg, int verbose) · line 434
Quoted 8 lines
utils/exportfs/exportfs.c +3 -1
+
 	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

Harshula Jayasuriya

author

2013-11-17 23:45 UTC #8 lore raw

PATCH v2 1/1 exportfs: modify can_test() to use LONG_MAX when appropriate

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(-)

utils/exportfs/exportfs.c +23 -4

index da5fe21..9f13858 100644
@@ -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;
 }

Signature · 1 line

1.8.3.1

Steve Dickson

2013-11-20 21:19 UTC #9 lore raw

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

Quoted 63 lines · Harshula Jayasuriya

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-
------------------------------------------------------------

Quoted 1 line

Signed-off-by Harshula Jayasuriya <[email protected]>

Committed (tag: nfs-utils-1-2-10-rc1)

steved.

Quoted 57 lines

---
 utils/exportfs/exportfs.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

utils/exportfs/exportfs.c +23 -4

index da5fe21..9f13858 100644
@@ -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;
 }