2014-02-20 06:38:37

by NeilBrown

[permalink] [raw]
Subject: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc

There are a number of NFS-related setting that currently must be set
by writing to various files under /proc.
This is a bit clumsy, particularly for systemd unit files.

So this series adds options to a number of commands where relevant.

The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
The third (nlm grace time) I think is probably right but if someone can argue
an alternate approach I'm unlikely to resist.
The fourth is .... uhm. You better look yourself.

Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
and /etc/modprobe.d should have something like

install lockd sysctl -p /etc/sysctl.d/lockd

but last time I tried that it broke "modprobe --show-depends".
Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd

Thoughts?

Thanks,
NeilBrown


---

Neil Brown (4):
nfsd: add -r and --rdma options to request rdma service.
nfsd: alloc nfsv4leasetime and nfsv4gracetime to be set.
nfsd: set nlm grace time to make NFSv4 grace time
statd: add options to set port number of lockd.


utils/nfsd/nfsd.c | 52 +++++++++++++++++++++++++++++++++++++++----
utils/nfsd/nfsd.man | 21 +++++++++++++++++
utils/nfsd/nfssvc.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
utils/nfsd/nfssvc.h | 2 ++
utils/statd/statd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
utils/statd/statd.man | 24 ++++++++++++++++++--
6 files changed, 204 insertions(+), 11 deletions(-)

--
Signature



2014-02-20 06:39:10

by NeilBrown

[permalink] [raw]
Subject: [PATCH 4/4] statd: add options to set port number of lockd.

Even though lockd is a totally separate process to statd, they
depended on each other: statd much be running for lockd to be useful.
So an easy way to set the port numbers used by lockd is to get statd
to set them.
This patch add --nlm-port and --nlm-tcp-port to that end.

Signed-off-by: NeilBrown <[email protected]>
---
utils/statd/statd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
utils/statd/statd.man | 24 +++++++++++++++++++--
2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/utils/statd/statd.c b/utils/statd/statd.c
index 8f3111173887..51a016e935e1 100644
--- a/utils/statd/statd.c
+++ b/utils/statd/statd.c
@@ -60,6 +60,8 @@ static struct option longopts[] =
{ "notify-mode", 0, 0, 'N' },
{ "ha-callout", 1, 0, 'H' },
{ "no-notify", 0, 0, 'L' },
+ { "nlm-port", 1, 0, 'T'},
+ { "nlm-udp-port", 1, 0, 'U'},
{ NULL, 0, 0, 0 }
};

@@ -209,7 +211,32 @@ static void run_sm_notify(int outport)
exit(2);

}
-/*
+
+static void set_nlm_port(char *type, int port)
+{
+ char nbuf[20];
+ char pathbuf[40];
+ int fd;
+ if (!port)
+ return;
+ snprintf(nbuf, sizeof(nbuf), "%d", port);
+ snprintf(pathbuf, sizeof(pathbuf), "/proc/sys/fs/nfs/nlm_%sport", type);
+ fd = open(pathbuf, O_WRONLY);
+ if (fd < 0 && errno == ENOENT) {
+ /* probably module not loaded */
+ system("modprobe lockd");
+ fd = open(pathbuf, O_WRONLY);
+ }
+ if (fd >= 0) {
+ if (write(fd, nbuf, strlen(nbuf)) != (ssize_t)strlen(nbuf))
+ fprintf(stderr, "%s: fail to set NLM %s port: %m\n",
+ name_p, type);
+ close(fd);
+ } else
+ fprintf(stderr, "%s: failed to open %s: %m\n", name_p, pathbuf);
+}
+
+/*
* Entry routine/main loop.
*/
int main (int argc, char **argv)
@@ -218,6 +245,7 @@ int main (int argc, char **argv)
int pid;
int arg;
int port = 0, out_port = 0;
+ int nlm_udp = 0, nlm_tcp = 0;
struct rlimit rlim;

int pipefds[2] = { -1, -1};
@@ -239,7 +267,7 @@ int main (int argc, char **argv)
MY_NAME = NULL;

/* Process command line switches */
- while ((arg = getopt_long(argc, argv, "h?vVFNH:dn:p:o:P:L", longopts, NULL)) != EOF) {
+ while ((arg = getopt_long(argc, argv, "h?vVFNH:dn:p:o:P:LT:U:", longopts, NULL)) != EOF) {
switch (arg) {
case 'V': /* Version */
case 'v':
@@ -275,6 +303,26 @@ int main (int argc, char **argv)
exit(1);
}
break;
+ case 'T': /* NLM TCP and UDP port */
+ nlm_tcp = atoi(optarg);
+ if (nlm_tcp < 1 || nlm_tcp > 65535) {
+ fprintf(stderr, "%s: bad nlm port number: %s\n",
+ argv[0], optarg);
+ usage();
+ exit(1);
+ }
+ if (nlm_udp == 0)
+ nlm_udp = nlm_tcp;
+ break;
+ case 'U': /* NLM UDP port */
+ nlm_udp = atoi(optarg);
+ if (nlm_udp < 1 || nlm_udp > 65535) {
+ fprintf(stderr, "%s: bad nlm UDP port number: %s\n",
+ argv[0], optarg);
+ usage();
+ exit(1);
+ }
+ break;
case 'n': /* Specify local hostname */
run_mode |= STATIC_HOSTNAME;
MY_NAME = xstrdup(optarg);
@@ -337,12 +385,15 @@ int main (int argc, char **argv)
}
}

+ set_nlm_port("tcp", nlm_tcp);
+ set_nlm_port("udp", nlm_udp);
+
#ifdef SIMULATIONS
if (argc > 1)
/* LH - I _really_ need to update simulator... */
simulator (--argc, ++argv); /* simulator() does exit() */
#endif
-
+
if (!(run_mode & MODE_NODAEMON)) {
int tempfd;

diff --git a/utils/statd/statd.man b/utils/statd/statd.man
index c3c53548f35d..896c2f8a98ce 100644
--- a/utils/statd/statd.man
+++ b/utils/statd/statd.man
@@ -12,7 +12,11 @@
.SH NAME
rpc.statd \- NSM service daemon
.SH SYNOPSIS
-.BI "rpc.statd [-dh?FLNvV] [-H " prog "] [-n " my-name "] [-o " outgoing-port "] [-p " listener-port "] [-P " path " ]
+.BI "rpc.statd [-dh?FLNvV] [-H " prog "] [-n " my-name "] [-o " outgoing-port ]
+.ti +10
+.BI "[-p " listener-port "] [-P " path ]
+.ti +10
+.BI "[--nlm-port " port "] [--nlm-udp-port " port ]
.SH DESCRIPTION
File locks are not part of persistent file system state.
Lock state is thus lost when a host reboots.
@@ -225,7 +229,23 @@ if gets port succeed, set the same port for all listener socket,
otherwise chooses a random ephemeral port for each listener socket.
.IP
This option can be used to fix the port value of its listeners when
-SM_NOTIFY requests must traverse a firewall between clients and servers.
+SM_NOTIFY requests must traverse a firewall between clients and
+servers.
+.TP
+.BI "\-T," "" " \-\-nlm\-port " port
+Specifies the port number that
+.I lockd
+should listen on for
+.B NLM
+requests. This sets both the TCP and UDP ports unless the UDP port is
+set separately.
+.TP
+.BI "\-U," "" " \-\-nlm\-udp\-port " port
+Specifies the UDP port number that
+.I lockd
+should listen on for
+.B NLM
+requests.
.TP
.BI "\-P, " "" \-\-state\-directory\-path " pathname
Specifies the pathname of the parent directory



2014-02-20 16:40:30

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 3/4] nfsd: set nlm grace time to make NFSv4 grace time

On Thu, Feb 20, 2014 at 05:36:48PM +1100, Neil Brown wrote:
> These two values are conceptually very similar, so it probably makes
> sense to set them to the same value at the same time.

Agreed. It was a mistake to add nfsv4gracetime in the first place, but
I guess we can't get rid of it now.

--b.

>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> utils/nfsd/nfsd.man | 3 ++-
> utils/nfsd/nfssvc.c | 8 ++++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index 58b53cbff009..c6d3ffbd3675 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -89,7 +89,8 @@ clients need to confirm their state with the server. Valid range is
> from 10 to 3600 seconds.
> .TP
> .B \-G " or " \-\-grace-time seconds
> -Set the grace-time used for NFSv4. New file open requests will not be
> +Set the grace-time used for NFSv4 and NLM (for NFSv2 and NFSv3).
> +New file open requests (NFSv4) and new file locks (NLM) will not be
> allowed until after this time has passed to allow clients to recover state.
> .TP
> .I nproc
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 337ab169c194..5e14cce58053 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -317,6 +317,14 @@ nfssvc_set_time(const char *type, const int seconds)
> xlog(L_ERROR, "Unable to set nfsv4%stime: %m", type);
> close(fd);
> }
> + if (strcmp(type, "grace") == 0) {
> + /* set same value for lockd */
> + fd = open("/proc/sys/fs/nfs/nlm_grace_period", O_WRONLY);
> + if (fd >= 0) {
> + write(fd, nbuf, strlen(nbuf));
> + close(fd);
> + }
> + }
> }
>
> void
>
>
> --
> 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

2014-02-25 01:47:39

by Trond Myklebust

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc


On Feb 24, 2014, at 17:44, Trond Myklebust <[email protected]> wrote:

>
> On Feb 24, 2014, at 17:37, NeilBrown <[email protected]> wrote:
>
>> On Thu, 20 Feb 2014 08:11:47 -0500 Trond Myklebust
>> <[email protected]> wrote:
>>
>>>
>>> On Feb 20, 2014, at 1:36, Neil Brown <[email protected]> wrote:
>>>
>>>> There are a number of NFS-related setting that currently must be set
>>>> by writing to various files under /proc.
>>>> This is a bit clumsy, particularly for systemd unit files.
>>>>
>>>> So this series adds options to a number of commands where relevant.
>>>>
>>>> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
>>>> The third (nlm grace time) I think is probably right but if someone can argue
>>>> an alternate approach I'm unlikely to resist.
>>>> The fourth is .... uhm. You better look yourself.
>>>>
>>>> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
>>>> and /etc/modprobe.d should have something like
>>>>
>>>> install lockd sysctl -p /etc/sysctl.d/lockd
>>>>
>>>> but last time I tried that it broke "modprobe --show-depends".
>>>> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>>>>
>>>> Thoughts?
>>>
>>> Why not just do most of this at module load time with something like "modprobe lockd lockd.nlm_grace_period=<nsecs> lockd.nlm_tcpport=<portnr> ???
>>> Better yet, add/edit appropriate entries in /etc/modprobe.conf.d at system setup time.
>>>
>>
>> Adding entries to /etc/modprobe.conf.d doesn't help if nfs is compiled in to
>> the base kernel.
>> Conversely, adding entries to /etc/sysctl.d doesn't help if nfs is a module.
>> You could conceivable do both (for those few values that are available both
>> as module parameters and sysctl settings) but that is clumsy and error prone.
>>
>
> All the NFS sysctl settings now have module parameter equivalents.
> Note that systemd can also override using the /sys/module interface, but the problem with both sysctl and /sys/module is that they get set _after_ the module has been loaded, and hence there is plenty of potential for races with mount requests.

BTW: if you don?t want to compile your kernel with module support, then those same parameters can be used on the kernel command line. That provides the same guarantees w.r.t. races...

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]


2014-02-20 14:32:17

by Chuck Lever III

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc


On Feb 20, 2014, at 5:11 AM, Trond Myklebust <[email protected]> wrote:

>
> On Feb 20, 2014, at 1:36, Neil Brown <[email protected]> wrote:
>
>> There are a number of NFS-related setting that currently must be set
>> by writing to various files under /proc.
>> This is a bit clumsy, particularly for systemd unit files.
>>
>> So this series adds options to a number of commands where relevant.
>>
>> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
>> The third (nlm grace time) I think is probably right but if someone can argue
>> an alternate approach I'm unlikely to resist.
>> The fourth is .... uhm. You better look yourself.
>>
>> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
>> and /etc/modprobe.d should have something like
>>
>> install lockd sysctl -p /etc/sysctl.d/lockd
>>
>> but last time I tried that it broke "modprobe --show-depends".
>> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>>
>> Thoughts?
>
> Why not just do most of this at module load time with something like "modprobe lockd lockd.nlm_grace_period=<nsecs> lockd.nlm_tcpport=<portnr> ???
> Better yet, add/edit appropriate entries in /etc/modprobe.conf.d at system setup time.

If we?re setting configuration options in a file in /etc, maybe we?d be better off if these daemons each had their own .conf files.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




2014-02-20 06:38:45

by NeilBrown

[permalink] [raw]
Subject: [PATCH 1/4] nfsd: add -r and --rdma options to request rdma service.

As nfsd establishes UDP and TCP ports, it should establish RDMA too.

Signed-off-by: NeilBrown <[email protected]>
---
utils/nfsd/nfsd.c | 22 +++++++++++++++++++---
utils/nfsd/nfsd.man | 9 +++++++++
utils/nfsd/nfssvc.c | 34 ++++++++++++++++++++++++++++++++++
utils/nfsd/nfssvc.h | 1 +
4 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index a9d77ab5c586..d67d1c3ee050 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -45,6 +45,7 @@ static struct option longopts[] =
{ "port", 1, 0, 'p' },
{ "debug", 0, 0, 'd' },
{ "syslog", 0, 0, 's' },
+ { "rdma", 2, 0, 'R' },
{ NULL, 0, 0, 0 }
};

@@ -96,7 +97,7 @@ int
main(int argc, char **argv)
{
int count = NFSD_NPROC, c, error = 0, portnum = 0, fd, found_one;
- char *p, *progname, *port;
+ char *p, *progname, *port, *rdma_port = NULL;
char *haddr = NULL;
int socket_up = 0;
int minorvers[NFS4_MAXMINOR + 1] = {0};
@@ -120,7 +121,7 @@ main(int argc, char **argv)
xlog_syslog(0);
xlog_stderr(1);

- while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTU", longopts, NULL)) != EOF) {
+ while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUr", longopts, NULL)) != EOF) {
switch(c) {
case 'd':
xlog_config(D_ALL, 1);
@@ -155,6 +156,16 @@ main(int argc, char **argv)
exit(1);
}
break;
+ case 'r':
+ rdma_port = "nfsrdma";
+ break;
+ case 'R': /* --rdma */
+ if (optarg)
+ rdma_port = optarg;
+ else
+ rdma_port = "nfsrdma";
+ break;
+
case 'N':
switch((c = strtol(optarg, &p, 0))) {
case 4:
@@ -294,6 +305,11 @@ main(int argc, char **argv)
socket_up = 1;
#endif /* IPV6_SUPPORTED */

+ if (rdma_port) {
+ error = nfssvc_set_rdmaport(rdma_port);
+ if (!error)
+ socket_up = 1;
+ }
set_threads:
/* don't start any threads if unable to hand off any sockets */
if (!socket_up) {
@@ -334,7 +350,7 @@ static void
usage(const char *prog)
{
fprintf(stderr, "Usage:\n"
- "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version] [-V|--nfs-version version] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
+ "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version] [-V|--nfs-version version] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] [-r|--rdma=] nrservs\n",
prog);
exit(2);
}
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 7de0867e39e8..aedf1409dc53 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -42,6 +42,15 @@ specify a different port to listen on for NFS requests. By default,
.B rpc.nfsd
will listen on port 2049.
.TP
+.B \-r " or " \-\-rdma
+specify that NFS requests on the standard RDMA port ("nfsrdma", port
+20049) should be honored.
+.TP
+.BI \-\-rdma= port
+Listen for RDMA requests on an alternate port - may be a number or a
+name listed in
+.BR /etc/services .
+.TP
.B \-N " or " \-\-no-nfs-version vers
This option can be used to request that
.B rpc.nfsd
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 1b50abaf9ca8..7eaeef3fb476 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -268,6 +268,40 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
return nfssvc_setfds(&hints, host, port);
}

+int
+nfssvc_set_rdmaport(const char *port)
+{
+ struct servent *sv = getservbyname(port, "tcp");
+ int nport;
+ char buf[20];
+ int ret;
+ int fd;
+
+ if (sv)
+ nport = sv->s_port;
+ else {
+ char *ep;
+ nport = strtol(port, &ep, 10);
+ if (!*port || *ep) {
+ xlog(L_ERROR, "unable to interpret port name %s",
+ port);
+ return 1;
+ }
+ }
+
+ fd = open(NFSD_PORTS_FILE, O_WRONLY);
+ if (fd < 0)
+ return 1;
+ snprintf(buf, sizeof(buf), "rdma %d", nport);
+ ret = 0;
+ if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf)) {
+ ret= errno;
+ xlog(L_ERROR, "Unable to request RDMA services: %m");
+ }
+ close(fd);
+ return ret;
+}
+
void
nfssvc_setvers(unsigned int ctlbits, int minorvers[])
{
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
index 2bbd3d30f92a..bb7fccee7c19 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -24,5 +24,6 @@ void nfssvc_mount_nfsdfs(char *progname);
int nfssvc_inuse(void);
int nfssvc_set_sockets(const int family, const unsigned int protobits,
const char *host, const char *port);
+int nfssvc_set_rdmaport(const char *port);
void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
int nfssvc_threads(unsigned short port, int nrservs);



2014-02-20 06:38:54

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/4] nfsd: alloc nfsv4leasetime and nfsv4gracetime to be set.

New arguments --gracetime (-G) and --leasetime (-L)

Signed-off-by: NeilBrown <[email protected]>
---
utils/nfsd/nfsd.c | 34 ++++++++++++++++++++++++++++++----
utils/nfsd/nfsd.man | 11 ++++++++++-
utils/nfsd/nfssvc.c | 17 +++++++++++++++++
utils/nfsd/nfssvc.h | 1 +
4 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index d67d1c3ee050..dbd0d98a8e68 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -46,6 +46,8 @@ static struct option longopts[] =
{ "debug", 0, 0, 'd' },
{ "syslog", 0, 0, 's' },
{ "rdma", 2, 0, 'R' },
+ { "grace-time", 1, 0, 'G'},
+ { "lease-time", 1, 0, 'L'},
{ NULL, 0, 0, 0 }
};

@@ -105,6 +107,8 @@ main(int argc, char **argv)
unsigned int protobits = NFSCTL_ALLBITS;
unsigned int proto4 = 0;
unsigned int proto6 = 0;
+ int grace = -1;
+ int lease = -1;

progname = strdup(basename(argv[0]));
if (!progname) {
@@ -121,7 +125,7 @@ main(int argc, char **argv)
xlog_syslog(0);
xlog_stderr(1);

- while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUr", longopts, NULL)) != EOF) {
+ while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUrG:L:", longopts, NULL)) != EOF) {
switch(c) {
case 'd':
xlog_config(D_ALL, 1);
@@ -218,6 +222,20 @@ main(int argc, char **argv)
case 'U':
NFSCTL_UDPUNSET(protobits);
break;
+ case 'G':
+ grace = strtol(optarg, &p, 0);
+ if (*p || grace <= 0) {
+ fprintf(stderr, "%s: Unrecognized grace time.\n", optarg);
+ exit(1);
+ }
+ break;
+ case 'L':
+ lease = strtol(optarg, &p, 0);
+ if (*p || grace <= 0) {
+ fprintf(stderr, "%s: Unrecognized lease time.\n", optarg);
+ exit(1);
+ }
+ break;
default:
fprintf(stderr, "Invalid argument: '%c'\n", c);
case 'h':
@@ -265,7 +283,7 @@ main(int argc, char **argv)
if (!found_one) {
xlog(L_ERROR, "no version specified");
exit(1);
- }
+ }

if (NFSCTL_VERISSET(versbits, 4) &&
!NFSCTL_TCPISSET(proto4) &&
@@ -294,7 +312,7 @@ main(int argc, char **argv)
* interfaces, these are a no-op.
*/
nfssvc_setvers(versbits, minorvers);
-
+
error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
if (!error)
socket_up = 1;
@@ -310,6 +328,11 @@ main(int argc, char **argv)
if (!error)
socket_up = 1;
}
+ if (grace > 0)
+ nfssvc_set_time("grace", grace);
+ if (lease > 0)
+ nfssvc_set_time("lease", lease);
+
set_threads:
/* don't start any threads if unable to hand off any sockets */
if (!socket_up) {
@@ -350,7 +373,10 @@ static void
usage(const char *prog)
{
fprintf(stderr, "Usage:\n"
- "%s [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version] [-V|--nfs-version version] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] [-r|--rdma=] nrservs\n",
+ "%s [-d|--debug] [-H hostname] [-p|-P|--port port]\n"
+ " [-N|--no-nfs-version version] [-V|--nfs-version version]\n"
+ " [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] [-r|--rdma=]\n"
+ " [-G|--gracetime secs] [-L|--leasetime secs] nrservs\n",
prog);
exit(2);
}
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index aedf1409dc53..58b53cbff009 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -2,7 +2,7 @@
.\" nfsd(8)
.\"
.\" Copyright (C) 1999 Olaf Kirch <[email protected]>
-.TH rpc.nfsd 8 "7 Aug 2006"
+.TH rpc.nfsd 8 "20 Feb 2014"
.SH NAME
rpc.nfsd \- NFS server process
.SH SYNOPSIS
@@ -83,6 +83,15 @@ offer certain versions of NFS. The current version of
.B rpc.nfsd
can support NFS versions 2,3,4 and the newer version 4.1.
.TP
+.B \-L " or " \-\-lease-time seconds
+Set the lease-time used for NFSv4. This corresponds to how often
+clients need to confirm their state with the server. Valid range is
+from 10 to 3600 seconds.
+.TP
+.B \-G " or " \-\-grace-time seconds
+Set the grace-time used for NFSv4. New file open requests will not be
+allowed until after this time has passed to allow clients to recover state.
+.TP
.I nproc
specify the number of NFS server threads. By default, just one
thread is started. However, for optimum performance several threads
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 7eaeef3fb476..337ab169c194 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -303,6 +303,23 @@ nfssvc_set_rdmaport(const char *port)
}

void
+nfssvc_set_time(const char *type, const int seconds)
+{
+ char pathbuf[40];
+ char nbuf[10];
+ int fd;
+
+ snprintf(pathbuf, sizeof(pathbuf), NFSD_FS_DIR "/nfsv4%stime", type);
+ snprintf(nbuf, sizeof(nbuf), "%d", seconds);
+ fd = open(pathbuf, O_WRONLY);
+ if (fd >= 0) {
+ if (write(fd, nbuf, strlen(nbuf)) != (ssize_t)strlen(nbuf))
+ xlog(L_ERROR, "Unable to set nfsv4%stime: %m", type);
+ close(fd);
+ }
+}
+
+void
nfssvc_setvers(unsigned int ctlbits, int minorvers[])
{
int fd, n, off;
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
index bb7fccee7c19..aa946a48a71e 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -24,6 +24,7 @@ void nfssvc_mount_nfsdfs(char *progname);
int nfssvc_inuse(void);
int nfssvc_set_sockets(const int family, const unsigned int protobits,
const char *host, const char *port);
+void nfssvc_set_time(const char *type, const int seconds);
int nfssvc_set_rdmaport(const char *port);
void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
int nfssvc_threads(unsigned short port, int nrservs);



2014-02-20 13:11:50

by Trond Myklebust

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc


On Feb 20, 2014, at 1:36, Neil Brown <[email protected]> wrote:

> There are a number of NFS-related setting that currently must be set
> by writing to various files under /proc.
> This is a bit clumsy, particularly for systemd unit files.
>
> So this series adds options to a number of commands where relevant.
>
> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
> The third (nlm grace time) I think is probably right but if someone can argue
> an alternate approach I'm unlikely to resist.
> The fourth is .... uhm. You better look yourself.
>
> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
> and /etc/modprobe.d should have something like
>
> install lockd sysctl -p /etc/sysctl.d/lockd
>
> but last time I tried that it broke "modprobe --show-depends".
> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>
> Thoughts?

Why not just do most of this at module load time with something like "modprobe lockd lockd.nlm_grace_period=<nsecs> lockd.nlm_tcpport=<portnr> ???
Better yet, add/edit appropriate entries in /etc/modprobe.conf.d at system setup time.

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]


2014-02-20 06:39:02

by NeilBrown

[permalink] [raw]
Subject: [PATCH 3/4] nfsd: set nlm grace time to make NFSv4 grace time

These two values are conceptually very similar, so it probably makes
sense to set them to the same value at the same time.

Signed-off-by: NeilBrown <[email protected]>
---
utils/nfsd/nfsd.man | 3 ++-
utils/nfsd/nfssvc.c | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 58b53cbff009..c6d3ffbd3675 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -89,7 +89,8 @@ clients need to confirm their state with the server. Valid range is
from 10 to 3600 seconds.
.TP
.B \-G " or " \-\-grace-time seconds
-Set the grace-time used for NFSv4. New file open requests will not be
+Set the grace-time used for NFSv4 and NLM (for NFSv2 and NFSv3).
+New file open requests (NFSv4) and new file locks (NLM) will not be
allowed until after this time has passed to allow clients to recover state.
.TP
.I nproc
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 337ab169c194..5e14cce58053 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -317,6 +317,14 @@ nfssvc_set_time(const char *type, const int seconds)
xlog(L_ERROR, "Unable to set nfsv4%stime: %m", type);
close(fd);
}
+ if (strcmp(type, "grace") == 0) {
+ /* set same value for lockd */
+ fd = open("/proc/sys/fs/nfs/nlm_grace_period", O_WRONLY);
+ if (fd >= 0) {
+ write(fd, nbuf, strlen(nbuf));
+ close(fd);
+ }
+ }
}

void



2014-02-25 01:44:36

by Trond Myklebust

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc


On Feb 24, 2014, at 17:37, NeilBrown <[email protected]> wrote:

> On Thu, 20 Feb 2014 08:11:47 -0500 Trond Myklebust
> <[email protected]> wrote:
>
>>
>> On Feb 20, 2014, at 1:36, Neil Brown <[email protected]> wrote:
>>
>>> There are a number of NFS-related setting that currently must be set
>>> by writing to various files under /proc.
>>> This is a bit clumsy, particularly for systemd unit files.
>>>
>>> So this series adds options to a number of commands where relevant.
>>>
>>> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
>>> The third (nlm grace time) I think is probably right but if someone can argue
>>> an alternate approach I'm unlikely to resist.
>>> The fourth is .... uhm. You better look yourself.
>>>
>>> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
>>> and /etc/modprobe.d should have something like
>>>
>>> install lockd sysctl -p /etc/sysctl.d/lockd
>>>
>>> but last time I tried that it broke "modprobe --show-depends".
>>> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>>>
>>> Thoughts?
>>
>> Why not just do most of this at module load time with something like "modprobe lockd lockd.nlm_grace_period=<nsecs> lockd.nlm_tcpport=<portnr> ???
>> Better yet, add/edit appropriate entries in /etc/modprobe.conf.d at system setup time.
>>
>
> Adding entries to /etc/modprobe.conf.d doesn't help if nfs is compiled in to
> the base kernel.
> Conversely, adding entries to /etc/sysctl.d doesn't help if nfs is a module.
> You could conceivable do both (for those few values that are available both
> as module parameters and sysctl settings) but that is clumsy and error prone.
>

All the NFS sysctl settings now have module parameter equivalents.
Note that systemd can also override using the /sys/module interface, but the problem with both sysctl and /sys/module is that they get set _after_ the module has been loaded, and hence there is plenty of potential for races with mount requests.

> Your argument could equally well apply to setting the NFS versions that nfsd
> supports, but we have explicit command-line arguments for that.
>
> Due to the highly ad-hoc collection of configuration settings and different
> ways to set them and rules for when they are set, I think it is best to have
> the settings imposed by code we control rather than requiring a similarly
> ad-hoc collection of additions to various configuration files in various
> directories.

See above. The races are the reason why I switched from sysctl to module params.

> Ultimately I would like all nfs-utils daemons to take settings out
> of /etc/sysconfig/nfs (either by reading it directly, or having systemd/bash
> load it into the environment, then the utils using getenv()), so that one
> file (which multiple distros already work with) can configure most of
> nfs-utils. This is a step in that direction with (I think) immediate rewards.
>
> Thanks,
>
> NeilBrown

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]


2014-02-25 01:37:14

by NeilBrown

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc

On Thu, 20 Feb 2014 08:11:47 -0500 Trond Myklebust
<[email protected]> wrote:

>
> On Feb 20, 2014, at 1:36, Neil Brown <[email protected]> wrote:
>
> > There are a number of NFS-related setting that currently must be set
> > by writing to various files under /proc.
> > This is a bit clumsy, particularly for systemd unit files.
> >
> > So this series adds options to a number of commands where relevant.
> >
> > The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
> > The third (nlm grace time) I think is probably right but if someone can argue
> > an alternate approach I'm unlikely to resist.
> > The fourth is .... uhm. You better look yourself.
> >
> > Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
> > and /etc/modprobe.d should have something like
> >
> > install lockd sysctl -p /etc/sysctl.d/lockd
> >
> > but last time I tried that it broke "modprobe --show-depends".
> > Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
> >
> > Thoughts?
>
> Why not just do most of this at module load time with something like "modprobe lockd lockd.nlm_grace_period=<nsecs> lockd.nlm_tcpport=<portnr> …”?
> Better yet, add/edit appropriate entries in /etc/modprobe.conf.d at system setup time.
>

Adding entries to /etc/modprobe.conf.d doesn't help if nfs is compiled in to
the base kernel.
Conversely, adding entries to /etc/sysctl.d doesn't help if nfs is a module.
You could conceivable do both (for those few values that are available both
as module parameters and sysctl settings) but that is clumsy and error prone.

Your argument could equally well apply to setting the NFS versions that nfsd
supports, but we have explicit command-line arguments for that.

Due to the highly ad-hoc collection of configuration settings and different
ways to set them and rules for when they are set, I think it is best to have
the settings imposed by code we control rather than requiring a similarly
ad-hoc collection of additions to various configuration files in various
directories.

Ultimately I would like all nfs-utils daemons to take settings out
of /etc/sysconfig/nfs (either by reading it directly, or having systemd/bash
load it into the environment, then the utils using getenv()), so that one
file (which multiple distros already work with) can configure most of
nfs-utils. This is a step in that direction with (I think) immediate rewards.

Thanks,

NeilBrown


Attachments:
signature.asc (828.00 B)

2014-03-10 00:47:25

by NeilBrown

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc

On Sat, 08 Mar 2014 11:56:23 -0500 Steve Dickson <[email protected]> wrote:

>
>
> On 02/20/2014 01:36 AM, Neil Brown wrote:
> > There are a number of NFS-related setting that currently must be set
> > by writing to various files under /proc.
> > This is a bit clumsy, particularly for systemd unit files.
> >
> > So this series adds options to a number of commands where relevant.
> >
> > The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
> > The third (nlm grace time) I think is probably right but if someone can argue
> > an alternate approach I'm unlikely to resist.
> > The fourth is .... uhm. You better look yourself.
> >
> > Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
> > and /etc/modprobe.d should have something like
> >
> > install lockd sysctl -p /etc/sysctl.d/lockd
> >
> > but last time I tried that it broke "modprobe --show-depends".
> > Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
> >
> > Thoughts?
> I finally got the cycles to take a look at these... My apologies for
> taking so long...

Thanks for getting to it!

>
> So I went ahead took a look... Clean them up a bit. There were a couple
> typos and they did not apply cleanly to my tree... While I was
> doing this I got this gnawing feeling that we probably should have
> some type of global configuration file where all these command
> line variables can be set.
>
> It would have to be distro friendly meaning the same place in all
> distros... Maybe something like /etc/nfsclient.conf patterned off
> the /etc/nfsmount.conf config file??
>
> So I'm thinking it does make sense to have a way to set
> all these but I'm just not keen on how they are being set.
> IDK... Maybe I'm over thinking it.. :-)

I have a couple of (complimentary) thoughts on this.

My early experience with md/raid raid taught me to be very wary of requiring
a config file. The old "raidtools" requires you to make a config file just
to create an array - or even to stop an array I think. The new mdadm allows
you do do everything with command line switches and that makes certain things
so much easier.

When I was experimenting with fallback from v4 to v3 when TCP is not
supported it was really nice to be able to, e.g.
rpc.nfsd 0
rpc.nfsd -T 8
to change nfsd to stop accepting TCP connections. Had I needed to edit a
config file every time I did that it would have been a pain.

This doesn't mean that config file are bad - not at all. Just that I really
like that ability to over-ride config files on the command line.

Secondly, we already have a config file for NFS. It is
call /etc/sysconfig/nfs, though Debian spells it "/etc/default/nfs".

I believe that the best was forward is to make this more standard.
I think the best way to do this is to teach various nfs utilities to use e.g.
getenv("NFS_LISTEN_TCP")
to get defaults for various settings before parsing command line options.
Then whatever is used to run these utilities can
source /etc/sysconfig/nfs
or
EnvironmentFile=/etc/sysconfig/nfs
first.
Thus we have a ready-made configfile name, a ready-made configfile syntax,
and just need to agree on values can be set.

A particular value of this approach is that /etc/sysconfig file are easy for
admin tools to manipulate. SUSE's 'yast' allows markup in the file so that
informative prompts and basic syntax checks can be applied. e.g.:

## Path: Network/File systems/NFS server
## Description: number of threads for kernel nfs server
## Type: integer
## Default: 4
## ServiceRestart: nfsserver
#
# the kernel nfs-server supports multiple server threads
#
USE_KERNEL_NFSD_NUMBER="8"

We would need to transition from the current setting names to the new agreed
setting names over a couple of released, but that isn't rocket science and is
our problem.



>
> Finally, during my testing the only flags that seem to work
> was the statd ones:
>
> # rpc.nfsd --rdma 8
> rpc.nfsd: Unable to request RDMA services: Protocol not supported

If you don't have configured RDMA hardware on your test machine I would
expect this. My testing largely involved running the tool under 'strace'
and making sure the correct string was written.

>
> # rpc.nfsd --grace-time 66
> rpc.nfsd: Unable to set /proc/fs/nfsd/nfsv4gracetime: Device or resource busy
>
> # rpc.nfsd --lease-time 66
> rpc.nfsd: Unable to set /proc/fs/nfsd/nfsv4leasetime: Device or resource busy
>
> Is this expected?

No.. that was a bit careless.
The grace-time and leave-time need to be set before the ports are opened.

i.e. the following incremental patch. Do you want to merge it with what you
have, or will I resend that patch (or the whole series)?

Thanks,
NeilBrown

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index dbd0d98a8e68..32d22d8ab63b 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -307,11 +307,17 @@ main(int argc, char **argv)
}

/*
- * must set versions before the fd's so that the right versions get
+ * Must set versions before the fd's so that the right versions get
* registered with rpcbind. Note that on older kernels w/o the right
* interfaces, these are a no-op.
+ * Timeouts must also be set before ports are created else we get
+ * EBUSY.
*/
nfssvc_setvers(versbits, minorvers);
+ if (grace > 0)
+ nfssvc_set_time("grace", grace);
+ if (lease > 0)
+ nfssvc_set_time("lease", lease);

error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
if (!error)
@@ -328,10 +334,6 @@ main(int argc, char **argv)
if (!error)
socket_up = 1;
}
- if (grace > 0)
- nfssvc_set_time("grace", grace);
- if (lease > 0)
- nfssvc_set_time("lease", lease);

set_threads:
/* don't start any threads if unable to hand off any sockets */


Attachments:
signature.asc (828.00 B)

2014-03-12 05:43:18

by NeilBrown

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc

On Mon, 10 Mar 2014 12:58:34 -0400 Steve Dickson <[email protected]> wrote:


> >
> > I believe that the best was forward is to make this more standard.
> > I think the best way to do this is to teach various nfs utilities to use e.g.
> > getenv("NFS_LISTEN_TCP")
> > to get defaults for various settings before parsing command line options.
> > Then whatever is used to run these utilities can
> > source /etc/sysconfig/nfs
> > or
> > EnvironmentFile=/etc/sysconfig/nfs
> > first.
> > Thus we have a ready-made configfile name, a ready-made configfile syntax,
> > and just need to agree on values can be set.
> I think this is a good idea... Which would override which? The command
> line override the environments? What should happen if neither are set?
>

Commandline should definitely over-ride environment or config file.
If neither is set we get some sensible default, just as you currently do if
you do nothing.

I noticed that rpc.nfsd always explicitly sets the version. So if I echo
something to /proc/fs/nfsd/versions and then run rpc.nfsd, the versions will
be over-written. I suspect that is reasonable behaviour.

Should e.g. nfs4gracetime be treated the same - always explicitly set
something. Or should we just leave it unchanged.
The former is more consistent. The later is backward compatible.
I lean toward the later which is what the current code does, but I thought it
was worth mentioning in case anyone disagrees.


Thanks,
NeilBrown


Attachments:
signature.asc (828.00 B)

2014-03-08 16:56:26

by Steve Dickson

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc



On 02/20/2014 01:36 AM, Neil Brown wrote:
> There are a number of NFS-related setting that currently must be set
> by writing to various files under /proc.
> This is a bit clumsy, particularly for systemd unit files.
>
> So this series adds options to a number of commands where relevant.
>
> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
> The third (nlm grace time) I think is probably right but if someone can argue
> an alternate approach I'm unlikely to resist.
> The fourth is .... uhm. You better look yourself.
>
> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
> and /etc/modprobe.d should have something like
>
> install lockd sysctl -p /etc/sysctl.d/lockd
>
> but last time I tried that it broke "modprobe --show-depends".
> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>
> Thoughts?
I finally got the cycles to take a look at these... My apologies for
taking so long...

So I went ahead took a look... Clean them up a bit. There were a couple
typos and they did not apply cleanly to my tree... While I was
doing this I got this gnawing feeling that we probably should have
some type of global configuration file where all these command
line variables can be set.

It would have to be distro friendly meaning the same place in all
distros... Maybe something like /etc/nfsclient.conf patterned off
the /etc/nfsmount.conf config file??

So I'm thinking it does make sense to have a way to set
all these but I'm just not keen on how they are being set.
IDK... Maybe I'm over thinking it.. :-)

Finally, during my testing the only flags that seem to work
was the statd ones:

# rpc.nfsd --rdma 8
rpc.nfsd: Unable to request RDMA services: Protocol not supported

# rpc.nfsd --grace-time 66
rpc.nfsd: Unable to set /proc/fs/nfsd/nfsv4gracetime: Device or resource busy

# rpc.nfsd --lease-time 66
rpc.nfsd: Unable to set /proc/fs/nfsd/nfsv4leasetime: Device or resource busy

Is this expected?

steved.

>
> Thanks,
> NeilBrown
>
>
> ---
>
> Neil Brown (4):
> nfsd: add -r and --rdma options to request rdma service.
> nfsd: alloc nfsv4leasetime and nfsv4gracetime to be set.
> nfsd: set nlm grace time to make NFSv4 grace time
> statd: add options to set port number of lockd.
>
>
> utils/nfsd/nfsd.c | 52 +++++++++++++++++++++++++++++++++++++++----
> utils/nfsd/nfsd.man | 21 +++++++++++++++++
> utils/nfsd/nfssvc.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
> utils/nfsd/nfssvc.h | 2 ++
> utils/statd/statd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
> utils/statd/statd.man | 24 ++++++++++++++++++--
> 6 files changed, 204 insertions(+), 11 deletions(-)
>

2014-03-11 16:06:04

by Steve Dickson

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc



On 02/20/2014 01:36 AM, Neil Brown wrote:
> There are a number of NFS-related setting that currently must be set
> by writing to various files under /proc.
> This is a bit clumsy, particularly for systemd unit files.
>
> So this series adds options to a number of commands where relevant.
>
> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
> The third (nlm grace time) I think is probably right but if someone can argue
> an alternate approach I'm unlikely to resist.
> The fourth is .... uhm. You better look yourself.
>
> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
> and /etc/modprobe.d should have something like
>
> install lockd sysctl -p /etc/sysctl.d/lockd
>
> but last time I tried that it broke "modprobe --show-depends".
> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>
> Thoughts?
All 4 patches tested and committed...

steved.

>
> Thanks,
> NeilBrown
>
>
> ---
>
> Neil Brown (4):
> nfsd: add -r and --rdma options to request rdma service.
> nfsd: alloc nfsv4leasetime and nfsv4gracetime to be set.
> nfsd: set nlm grace time to make NFSv4 grace time
> statd: add options to set port number of lockd.
>
>
> utils/nfsd/nfsd.c | 52 +++++++++++++++++++++++++++++++++++++++----
> utils/nfsd/nfsd.man | 21 +++++++++++++++++
> utils/nfsd/nfssvc.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
> utils/nfsd/nfssvc.h | 2 ++
> utils/statd/statd.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--
> utils/statd/statd.man | 24 ++++++++++++++++++--
> 6 files changed, 204 insertions(+), 11 deletions(-)
>

2014-03-08 15:20:20

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/4] nfsd: add -r and --rdma options to request rdma service.

Hey Neil,

This part did not apply cleanly

On 02/20/2014 01:36 AM, Neil Brown wrote:
> void
> nfssvc_setvers(unsigned int ctlbits, int minorvers[])
> {
> diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
> index 2bbd3d30f92a..bb7fccee7c19 100644
> --- a/utils/nfsd/nfssvc.h
> +++ b/utils/nfsd/nfssvc.h
> @@ -24,5 +24,6 @@ void nfssvc_mount_nfsdfs(char *progname);
> int nfssvc_inuse(void);
> int nfssvc_set_sockets(const int family, const unsigned int protobits,
> const char *host, const char *port);
> +int nfssvc_set_rdmaport(const char *port);
> void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
^^^ this interface is different that the one I have in my tree

The one I have is
void nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers4, unsigned int minorvers4set);

No biggie but I'm just wandering if I'm missing a patch...

steved.
> int nfssvc_threads(unsigned short port, int nrservs);

2014-03-10 00:10:35

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 1/4] nfsd: add -r and --rdma options to request rdma service.

On Sat, 08 Mar 2014 10:20:16 -0500 Steve Dickson <[email protected]> wrote:

> Hey Neil,
>
> This part did not apply cleanly
>
> On 02/20/2014 01:36 AM, Neil Brown wrote:
> > void
> > nfssvc_setvers(unsigned int ctlbits, int minorvers[])
> > {
> > diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
> > index 2bbd3d30f92a..bb7fccee7c19 100644
> > --- a/utils/nfsd/nfssvc.h
> > +++ b/utils/nfsd/nfssvc.h
> > @@ -24,5 +24,6 @@ void nfssvc_mount_nfsdfs(char *progname);
> > int nfssvc_inuse(void);
> > int nfssvc_set_sockets(const int family, const unsigned int protobits,
> > const char *host, const char *port);
> > +int nfssvc_set_rdmaport(const char *port);
> > void nfssvc_setvers(unsigned int ctlbits, int minorvers4[]);
> ^^^ this interface is different that the one I have in my tree
>
> The one I have is
> void nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers4, unsigned int minorvers4set);

The patch which changed that interface to the new one predates my patch by 7
days, so maybe I didn't pull before I did the development.
(I was doing that in my systemd-devel branch which I hadn't rebased recently).

Thanks,
NeilBrown


>
> No biggie but I'm just wandering if I'm missing a patch...
>
> steved.
> > int nfssvc_threads(unsigned short port, int nrservs);


Attachments:
signature.asc (828.00 B)

2014-03-10 16:58:40

by Steve Dickson

[permalink] [raw]
Subject: Re: [nfs-utils RPC-PATCH 0/4] Add options to nfsd etc to avoid needing to write to /proc



On 03/09/2014 08:47 PM, NeilBrown wrote:
> On Sat, 08 Mar 2014 11:56:23 -0500 Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 02/20/2014 01:36 AM, Neil Brown wrote:
>>> There are a number of NFS-related setting that currently must be set
>>> by writing to various files under /proc.
>>> This is a bit clumsy, particularly for systemd unit files.
>>>
>>> So this series adds options to a number of commands where relevant.
>>>
>>> The first two (rdma, and nfsv4{grace,lease}time) I am quite comfortable with.
>>> The third (nlm grace time) I think is probably right but if someone can argue
>>> an alternate approach I'm unlikely to resist.
>>> The fourth is .... uhm. You better look yourself.
>>>
>>> Part of me thinks that nlm port numbers should be set in /etc/sysctl.conf (or sysctl.d)
>>> and /etc/modprobe.d should have something like
>>>
>>> install lockd sysctl -p /etc/sysctl.d/lockd
>>>
>>> but last time I tried that it broke "modprobe --show-depends".
>>> Also it is awkward to get setting from /etc/sysconfig/nfs into /etc/sysctl.d/lockd
>>>
>>> Thoughts?
>> I finally got the cycles to take a look at these... My apologies for
>> taking so long...
>
> Thanks for getting to it!
>
>>
>> So I went ahead took a look... Clean them up a bit. There were a couple
>> typos and they did not apply cleanly to my tree... While I was
>> doing this I got this gnawing feeling that we probably should have
>> some type of global configuration file where all these command
>> line variables can be set.
>>
>> It would have to be distro friendly meaning the same place in all
>> distros... Maybe something like /etc/nfsclient.conf patterned off
>> the /etc/nfsmount.conf config file??
>>
>> So I'm thinking it does make sense to have a way to set
>> all these but I'm just not keen on how they are being set.
>> IDK... Maybe I'm over thinking it.. :-)
>
> I have a couple of (complimentary) thoughts on this.
>
> My early experience with md/raid raid taught me to be very wary of requiring
> a config file. The old "raidtools" requires you to make a config file just
> to create an array - or even to stop an array I think. The new mdadm allows
> you do do everything with command line switches and that makes certain things
> so much easier.
Fair enough... This sounds like wisdom to me... ;-) which is good enough for me!

>
> When I was experimenting with fallback from v4 to v3 when TCP is not
> supported it was really nice to be able to, e.g.
> rpc.nfsd 0
> rpc.nfsd -T 8
> to change nfsd to stop accepting TCP connections. Had I needed to edit a
> config file every time I did that it would have been a pain.
>
> This doesn't mean that config file are bad - not at all. Just that I really
> like that ability to over-ride config files on the command line.
Point taken...

>
> Secondly, we already have a config file for NFS. It is
> call /etc/sysconfig/nfs, though Debian spells it "/etc/default/nfs".
Which is unfortunate, IMHO... To bad we can't meld those together...

>
> I believe that the best was forward is to make this more standard.
> I think the best way to do this is to teach various nfs utilities to use e.g.
> getenv("NFS_LISTEN_TCP")
> to get defaults for various settings before parsing command line options.
> Then whatever is used to run these utilities can
> source /etc/sysconfig/nfs
> or
> EnvironmentFile=/etc/sysconfig/nfs
> first.
> Thus we have a ready-made configfile name, a ready-made configfile syntax,
> and just need to agree on values can be set.
I think this is a good idea... Which would override which? The command
line override the environments? What should happen if neither are set?

>
> A particular value of this approach is that /etc/sysconfig file are easy for
> admin tools to manipulate. SUSE's 'yast' allows markup in the file so that
> informative prompts and basic syntax checks can be applied. e.g.:
>
> ## Path: Network/File systems/NFS server
> ## Description: number of threads for kernel nfs server
> ## Type: integer
> ## Default: 4
> ## ServiceRestart: nfsserver
> #
> # the kernel nfs-server supports multiple server threads
> #
> USE_KERNEL_NFSD_NUMBER="8"
>
> We would need to transition from the current setting names to the new agreed
> setting names over a couple of released, but that isn't rocket science and is
> our problem.
I guess this would be that melding I was talking about.. ;-)

>
>
>
>>
>> Finally, during my testing the only flags that seem to work
>> was the statd ones:
>>
>> # rpc.nfsd --rdma 8
>> rpc.nfsd: Unable to request RDMA services: Protocol not supported
>
> If you don't have configured RDMA hardware on your test machine I would
> expect this. My testing largely involved running the tool under 'strace'
> and making sure the correct string was written.
>
>>
>> # rpc.nfsd --grace-time 66
>> rpc.nfsd: Unable to set /proc/fs/nfsd/nfsv4gracetime: Device or resource busy
>>
>> # rpc.nfsd --lease-time 66
>> rpc.nfsd: Unable to set /proc/fs/nfsd/nfsv4leasetime: Device or resource busy
>>
>> Is this expected?
>
> No.. that was a bit careless.
> The grace-time and leave-time need to be set before the ports are opened.
>
> i.e. the following incremental patch. Do you want to merge it with what you
> have, or will I resend that patch (or the whole series)?
Just resend the patch... that will be good enough...

steved.
>
> Thanks,
> NeilBrown
>
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index dbd0d98a8e68..32d22d8ab63b 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -307,11 +307,17 @@ main(int argc, char **argv)
> }
>
> /*
> - * must set versions before the fd's so that the right versions get
> + * Must set versions before the fd's so that the right versions get
> * registered with rpcbind. Note that on older kernels w/o the right
> * interfaces, these are a no-op.
> + * Timeouts must also be set before ports are created else we get
> + * EBUSY.
> */
> nfssvc_setvers(versbits, minorvers);
> + if (grace > 0)
> + nfssvc_set_time("grace", grace);
> + if (lease > 0)
> + nfssvc_set_time("lease", lease);
>
> error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
> if (!error)
> @@ -328,10 +334,6 @@ main(int argc, char **argv)
> if (!error)
> socket_up = 1;
> }
> - if (grace > 0)
> - nfssvc_set_time("grace", grace);
> - if (lease > 0)
> - nfssvc_set_time("lease", lease);
>
> set_threads:
> /* don't start any threads if unable to hand off any sockets */
>