Add /proc/sys/fs/nfs sysctls to nfsd module

From: [email protected]
Subject: Add /proc/sys/fs/nfs sysctls to nfsd module

This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs, and makes the max size of a readdirplus reply tunable.

The reason for this is that we found that some clients do not seem to grok readdirplus replies larger than 4K. Rather than reducing the overall number this sysctl allows admins to work around these clients.

Signed-off-by Olaf Kirch <[email protected]>

 fs/nfsd/nfs3xdr.c         |    5 ++++-
 fs/nfsd/nfsctl.c          |   28 ++++++++++++++++++++++++++++
 include/linux/nfsd/xdr3.h |    4 ++++
 3 files changed, 36 insertions(+), 1 deletion(-)

Index: linux-2.6.18/fs/nfsd/nfsctl.c
===================================================================

fs/nfsd/nfsctl.c +28 -0

@@ -31,6 +31,7 @@
 #include <linux/nfsd/nfsd.h>
 #include <linux/nfsd/cache.h>
 #include <linux/nfsd/xdr.h>
+#include <linux/nfsd/xdr3.h>
 #include <linux/nfsd/syscall.h>
 #include <linux/nfsd/interface.h>
 

@@ -507,8 +508,28 @@ static struct file_system_type nfsd_fs_t
        .kill_sb        = kill_litter_super,
 };
 
+ /*
+ * NFS sysctls
+ */
+static struct ctl_table_header *nfsd_sysctl_table;
+
+static ctl_table nfsd_sysctls[] = {
+       {
+               .ctl_name       = -2,
+               .procname       = "nfsd_readdirplus_max",
+               .data           = &nfsd_readdirplus_max,
+               .maxlen         = sizeof(nfsd_readdirplus_max),
+               .mode           = 0644,
+               .proc_handler   = &proc_dointvec_minmax,
+               .extra1         = &nfsd_readdirplus_max_lb,
+               .extra2         = &nfsd_readdirplus_max_ub,
+       },
+       { .ctl_name = 0 }
+};
+
 static int __init init_nfsd(void)
 {
+       struct ctl_path ctl_path[] = { { CTL_FS, "fs", 0555 }, { -2, "nfs", 0555 }, { 0 } };
        int retval;
        printk(KERN_INFO "Installing knfsd (copyright (C) 1996 [email protected]).\n");
 

@@ -518,6 +539,7 @@ static int __init init_nfsd(void)
        nfsd_lockd_init();      /* lockd->nfsd callbacks */
        nfs4_state_init();      /* NFSv4 locking state */
        nfsd_idmap_init();      /* Name to ID mapping */
+       nfsd_sysctl_table = register_sysctl_table_path(nfsd_sysctls, ctl_path);
        if (proc_mkdir("fs/nfs", NULL)) {
                struct proc_dir_entry *entry;
                entry = create_proc_entry("fs/nfs/exports", 0, NULL);

@@ -532,6 +554,9 @@ static int __init init_nfsd(void)
                remove_proc_entry("fs/nfs", NULL);
                nfsd_stat_shutdown();
                nfsd_lockd_shutdown();
+               if (nfsd_sysctl_table)
+                       unregister_sysctl_table(nfsd_sysctl_table);
+               nfsd_sysctl_table = NULL;
        }
        return retval;
 }

@@ -546,6 +571,9 @@ static void __exit exit_nfsd(void)
        nfsd_lockd_shutdown();
        nfsd_idmap_shutdown();
        unregister_filesystem(&nfsd_fs_type);
+       if (nfsd_sysctl_table)
+               unregister_sysctl_table(nfsd_sysctl_table);
+       nfsd_sysctl_table = NULL;
 }
 
 MODULE_AUTHOR("Olaf Kirch <[email protected]>");

Index: linux-2.6.18/fs/nfsd/nfs3xdr.c
===================================================================

fs/nfsd/nfs3xdr.c +4 -1

@@ -28,6 +28,9 @@
 # define inline
 #endif
 
+unsigned int   nfsd_readdirplus_max = NFSSVC_MAXBLKSIZE;
+unsigned int   nfsd_readdirplus_max_lb = 512;
+unsigned int   nfsd_readdirplus_max_ub = NFSSVC_MAXBLKSIZE;
 
 /*
  * Mapping of S_IF* types to NFS file types

@@ -573,7 +576,7 @@ nfs3svc_decode_readdirplusargs(struct sv
        args->dircount = ntohl(*p++);
        args->count    = ntohl(*p++);
 
-       len = (args->count > NFSSVC_MAXBLKSIZE) ? NFSSVC_MAXBLKSIZE :
+       len = (args->count > nfsd_readdirplus_max) ? nfsd_readdirplus_max :
                                                  args->count;
        args->count = len;
 

Index: linux-2.6.18/include/linux/nfsd/xdr3.h
===================================================================

I don't recall the history of /proc/sys/fs -- is there also a /proc/sys/fs/nfsd directory, and wouldn't it be better to add this particular sysctl in there?

Note, however, that the entire sysctl should be unnecessary. Clients are supposed to tell the server how large a reply to return, by means of the 'maxcount' argument (see RFC1813). If the clients aren't doing that, then they really need to be fixed.

From a distributor's point of view, waiting for the client vendor to

Quoted 9 lines · Olaf Kirch

From: [email protected]
Subject: Add /proc/sys/fs/nfs sysctls to nfsd module

This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs, and makes the max size of a readdirplus reply tunable.

The reason for this is that we found that some clients do not seem to grok readdirplus replies larger than 4K. Rather than reducing the overall number this sysctl allows admins to work around these clients.

fs/nfsd/nfs3xdr.c nfs3svc_decode_readdirplusargs(struct sv · line 582

Quoted 11 lines

fs/nfsd/nfs3xdr.c +1 -1

@@ -573,7 +576,7 @@ nfs3svc_decode_readdirplusargs(struct sv
        args->dircount = ntohl(*p++);
        args->count    = ntohl(*p++);
 
-       len = (args->count > NFSSVC_MAXBLKSIZE) ? NFSSVC_MAXBLKSIZE :
+       len = (args->count > nfsd_readdirplus_max) ? nfsd_readdirplus_max :
                                                  args->count;
        args->count = len;
 

I don't recall the history of /proc/sys/fs -- is there also a /proc/sys/fs/nfsd directory,

and wouldn't it be better to add this
particular sysctl in there?

Quoted 4 lines · Chuck Lever

Quoted 3 lines · Olaf Kirch

From: [email protected]
Subject: Add /proc/sys/fs/nfs sysctls to nfsd module

Quoted 6 lines

Quoted 6 lines

This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs, and makes the max size of a readdirplus reply tunable.

The reason for this is that we found that some clients do not seem to grok readdirplus replies larger than 4K. Rather than reducing the overall number this sysctl allows admins to work around these clients.

Neat.

Not really. It would be better to identify and fix the buggy clients that aren't setting the READDIRPLUS maxcount parameter correctly.

Quoted 2 lines · Trond Myklebust

Not really. It would be better to identify and fix the buggy clients that aren't setting the READDIRPLUS maxcount parameter correctly.

One of them was Solaris 9, IIRC. Don't recall what the other one was. I contacted at least Sun, but that didn't go anywhere.

Quoted 8 lines · Olaf Kirch

Quoted 3 lines · Trond Myklebust

Not really. It would be better to identify and fix the buggy clients that aren't setting the READDIRPLUS maxcount parameter correctly.

One of them was Solaris 9, IIRC. Don't recall what the other one was. I contacted at least Sun, but that didn't go anywhere.

Yeah.... Solaris appears to have a warped design in their READDIR code. They want to specify one size for the actual READDIR reply, and one size for all the extra READDIRPLUS gunk (see pages 209 and 210 in Brent's 'NFS Illustrated').
In their case, you want to look at the 'dircount' parameter, and truncate the readdirplus reply when you hit 'dircount' bytes of filename +fileid information. After that, you have to look at the total XDR information, and truncate again when you hit 'maxcount' bytes.

Sucks, sucks, sucks....

The two fields were intended to be used to reduce the overhead on the server by passing a hint for the amount of data that the client was actually looking for and then a count for the maximum sized response that it was prepared to handle.

Solaris probably uses something like 1048/32768 or 8192/32768, doesn't it?

Quoted 1 line

Sucks, sucks, sucks....

Actually, the Solaris implementation matches the intended semantics as described in RFC1813. The number of bytes of requested information via readdir(3) is passed via dircount and then the maximum size of the response is passed through maxcount.

The two fields were intended to be used to reduce the overhead on the server by passing a hint for the amount of data that the client was actually looking for and then a count for the maximum sized response that it was prepared to handle.

Neil, a headsup: this hunk clashes with one in the server large wsize patch, and will require minor and obvious manual adjustment.

Quoted 2 lines

I don't recall the history of /proc/sys/fs -- is there also a /proc/sys/fs/nfsd directory,

No.

Quoted 2 lines

and wouldn't it be better to add this
particular sysctl in there?

I'd be happy to see such a thing. One of my pending patches adds serverside sysctls; it feels wrong putting them in /proc/sys/fs/nfs/. OTOH that directory already has four serverside NLM entries that people do use.

So of the three options - the one that I find it hardest to justify if /proc/sys, which is what the original patch uses - sad.