All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default
@ 2017-02-25 13:13 Jeff Layton
  2017-02-27 21:53 ` J. Bruce Fields
  2017-04-05 17:30 ` Steve Dickson
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Layton @ 2017-02-25 13:13 UTC (permalink / raw)
  To: linux-nfs

Most major NFS clients have supported TCP for at least a decade now,
and v4-only shops are becoming more prevalent. It seems reasonable that
serving over UDP should be something that is "opt-in".

I've always hesitated to do this in the past, but now that we have
nfs.conf, it seems like the time may be right to disable UDP in default
configurations. In particular, it would be good to try this in the more
bleeding edge distros (Fedora, Ubuntu, SuSE, etc...) and see how
problematic it is.

Change the default in rpc.nfsd to just open TCP ports by default. Add
new -u and -t options that allow users to explicitly override what's
in the config file, and update the usage message and manpage.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 nfs.conf                  |  2 +-
 support/include/nfs/nfs.h |  2 +-
 utils/nfsd/nfsd.c         | 18 +++++++++++++-----
 utils/nfsd/nfsd.man       | 14 ++++++++------
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/nfs.conf b/nfs.conf
index 81ece0602dba..5d5a23463cff 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -42,7 +42,7 @@
 # port=0
 # grace-time=90
 # lease-time=90
-# udp=y
+# udp=n
 # tcp=y
 # vers2=n
 # vers3=y
diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 15ecc6bfc485..5b1ef0ce3883 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -27,6 +27,7 @@ struct nfs_fh_len {
 
 #define NFSCTL_UDPBIT		      (1 << (17 - 1))
 #define NFSCTL_TCPBIT		      (1 << (18 - 1))
+#define NFSCTL_PROTODEFAULT	      (NFSCTL_TCPBIT)
 
 #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1))) 
 #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_UDPBIT) 
@@ -42,6 +43,5 @@ struct nfs_fh_len {
 #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |= NFSCTL_TCPBIT)
 
 #define NFSCTL_ANYPROTO(_cltbits)     ((_cltbits) & (NFSCTL_UDPBIT | NFSCTL_TCPBIT))
-#define NFSCTL_ALLBITS (~0)
 
 #endif /* _NFS_NFS_H */
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 20f4b7952203..5d101f0e98c3 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -44,7 +44,9 @@ static struct option longopts[] =
 	{ "help", 0, 0, 'h' },
 	{ "no-nfs-version", 1, 0, 'N' },
 	{ "nfs-version", 1, 0, 'V' },
+	{ "tcp", 0, 0, 't' },
 	{ "no-tcp", 0, 0, 'T' },
+	{ "udp", 0, 0, 'u' },
 	{ "no-udp", 0, 0, 'U' },
 	{ "port", 1, 0, 'P' },
 	{ "port", 1, 0, 'p' },
@@ -68,7 +70,7 @@ main(int argc, char **argv)
 	unsigned int minorvers = 0;
 	unsigned int minorversset = 0;
 	unsigned int versbits = NFSCTL_VERDEFAULT;
-	unsigned int protobits = NFSCTL_ALLBITS;
+	unsigned int protobits = NFSCTL_PROTODEFAULT;
 	int grace = -1;
 	int lease = -1;
 
@@ -138,7 +140,7 @@ main(int argc, char **argv)
 		}
 	}
 
-	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUrG:L:", longopts, NULL)) != EOF) {
+	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:stTitUrG:L:", longopts, NULL)) != EOF) {
 		switch(c) {
 		case 'd':
 			xlog_config(D_ALL, 1);
@@ -222,9 +224,15 @@ main(int argc, char **argv)
 			xlog_syslog(1);
 			xlog_stderr(0);
 			break;
+		case 't':
+			NFSCTL_TCPSET(protobits);
+			break;
 		case 'T':
 			NFSCTL_TCPUNSET(protobits);
 			break;
+		case 'u':
+			NFSCTL_UDPSET(protobits);
+			break;
 		case 'U':
 			NFSCTL_UDPUNSET(protobits);
 			break;
@@ -372,9 +380,9 @@ usage(const char *prog)
 {
 	fprintf(stderr, "Usage:\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|--grace-time secs] [-L|--leasetime secs] nrservs\n",
+		"   [-N|--no-nfs-version version] [-V|--nfs-version version]\n"
+		"   [-s|--syslog] [-t|--tcp] [-T|--no-tcp] [-u|--udp] [-U|--no-udp]\n"
+		"   [-r|--rdma=] [-G|--grace-time secs] [-L|--leasetime secs] nrservs\n",
 		prog);
 	exit(2);
 }
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 8901fb6c6872..e0c640a64094 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -67,15 +67,17 @@ logs error messages (and debug messages, if enabled) to stderr. This option make
 log these messages to syslog instead. Note that errors encountered during
 option processing will still be logged to stderr regardless of this option.
 .TP
+.B \-t " or " \-\-tcp
+Instruct the kernel nfs server to open and listen on a TCP socket. This is the default.
+.TP
 .B \-T " or " \-\-no-tcp
-Disable 
-.B rpc.nfsd 
-from accepting TCP connections from clients.
+Instruct the kernel nfs server not to open and listen on a TCP socket.
+.TP
+.B \-u " or " \-\-udp
+Instruct the kernel nfs server to open and listen on a UDP socket.
 .TP
 .B \-U " or " \-\-no-udp
-Disable
-.B rpc.nfsd
-from accepting UDP connections from clients.
+Instruct the kernel nfs server not to open and listen on a UDP socket. This is the default.
 .TP
 .B \-V " or " \-\-nfs-version vers
 This option can be used to request that 
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default
  2017-02-25 13:13 [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default Jeff Layton
@ 2017-02-27 21:53 ` J. Bruce Fields
  2017-02-27 22:47   ` Jeff Layton
  2017-04-05 17:30 ` Steve Dickson
  1 sibling, 1 reply; 4+ messages in thread
From: J. Bruce Fields @ 2017-02-27 21:53 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-nfs

On Sat, Feb 25, 2017 at 08:13:11AM -0500, Jeff Layton wrote:
> Most major NFS clients have supported TCP for at least a decade now,
> and v4-only shops are becoming more prevalent. It seems reasonable that
> serving over UDP should be something that is "opt-in".
> 
> I've always hesitated to do this in the past, but now that we have
> nfs.conf, it seems like the time may be right to disable UDP in default
> configurations. In particular, it would be good to try this in the more
> bleeding edge distros (Fedora, Ubuntu, SuSE, etc...) and see how
> problematic it is.
>
> Change the default in rpc.nfsd to just open TCP ports by default. Add
> new -u and -t options that allow users to explicitly override what's
> in the config file, and update the usage message and manpage.

I guess I'm OK with trying it.

What does the failure look like (from the Linux client at least) in the
case of a preexisting UDP mount, or a new UDP mount?  (Do they get a
useful error, or at least something in sylog if there's a hang?)

--b.

> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  nfs.conf                  |  2 +-
>  support/include/nfs/nfs.h |  2 +-
>  utils/nfsd/nfsd.c         | 18 +++++++++++++-----
>  utils/nfsd/nfsd.man       | 14 ++++++++------
>  4 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/nfs.conf b/nfs.conf
> index 81ece0602dba..5d5a23463cff 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -42,7 +42,7 @@
>  # port=0
>  # grace-time=90
>  # lease-time=90
> -# udp=y
> +# udp=n
>  # tcp=y
>  # vers2=n
>  # vers3=y
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 15ecc6bfc485..5b1ef0ce3883 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -27,6 +27,7 @@ struct nfs_fh_len {
>  
>  #define NFSCTL_UDPBIT		      (1 << (17 - 1))
>  #define NFSCTL_TCPBIT		      (1 << (18 - 1))
> +#define NFSCTL_PROTODEFAULT	      (NFSCTL_TCPBIT)
>  
>  #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1))) 
>  #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_UDPBIT) 
> @@ -42,6 +43,5 @@ struct nfs_fh_len {
>  #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |= NFSCTL_TCPBIT)
>  
>  #define NFSCTL_ANYPROTO(_cltbits)     ((_cltbits) & (NFSCTL_UDPBIT | NFSCTL_TCPBIT))
> -#define NFSCTL_ALLBITS (~0)
>  
>  #endif /* _NFS_NFS_H */
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 20f4b7952203..5d101f0e98c3 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -44,7 +44,9 @@ static struct option longopts[] =
>  	{ "help", 0, 0, 'h' },
>  	{ "no-nfs-version", 1, 0, 'N' },
>  	{ "nfs-version", 1, 0, 'V' },
> +	{ "tcp", 0, 0, 't' },
>  	{ "no-tcp", 0, 0, 'T' },
> +	{ "udp", 0, 0, 'u' },
>  	{ "no-udp", 0, 0, 'U' },
>  	{ "port", 1, 0, 'P' },
>  	{ "port", 1, 0, 'p' },
> @@ -68,7 +70,7 @@ main(int argc, char **argv)
>  	unsigned int minorvers = 0;
>  	unsigned int minorversset = 0;
>  	unsigned int versbits = NFSCTL_VERDEFAULT;
> -	unsigned int protobits = NFSCTL_ALLBITS;
> +	unsigned int protobits = NFSCTL_PROTODEFAULT;
>  	int grace = -1;
>  	int lease = -1;
>  
> @@ -138,7 +140,7 @@ main(int argc, char **argv)
>  		}
>  	}
>  
> -	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUrG:L:", longopts, NULL)) != EOF) {
> +	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:stTitUrG:L:", longopts, NULL)) != EOF) {
>  		switch(c) {
>  		case 'd':
>  			xlog_config(D_ALL, 1);
> @@ -222,9 +224,15 @@ main(int argc, char **argv)
>  			xlog_syslog(1);
>  			xlog_stderr(0);
>  			break;
> +		case 't':
> +			NFSCTL_TCPSET(protobits);
> +			break;
>  		case 'T':
>  			NFSCTL_TCPUNSET(protobits);
>  			break;
> +		case 'u':
> +			NFSCTL_UDPSET(protobits);
> +			break;
>  		case 'U':
>  			NFSCTL_UDPUNSET(protobits);
>  			break;
> @@ -372,9 +380,9 @@ usage(const char *prog)
>  {
>  	fprintf(stderr, "Usage:\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|--grace-time secs] [-L|--leasetime secs] nrservs\n",
> +		"   [-N|--no-nfs-version version] [-V|--nfs-version version]\n"
> +		"   [-s|--syslog] [-t|--tcp] [-T|--no-tcp] [-u|--udp] [-U|--no-udp]\n"
> +		"   [-r|--rdma=] [-G|--grace-time secs] [-L|--leasetime secs] nrservs\n",
>  		prog);
>  	exit(2);
>  }
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index 8901fb6c6872..e0c640a64094 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -67,15 +67,17 @@ logs error messages (and debug messages, if enabled) to stderr. This option make
>  log these messages to syslog instead. Note that errors encountered during
>  option processing will still be logged to stderr regardless of this option.
>  .TP
> +.B \-t " or " \-\-tcp
> +Instruct the kernel nfs server to open and listen on a TCP socket. This is the default.
> +.TP
>  .B \-T " or " \-\-no-tcp
> -Disable 
> -.B rpc.nfsd 
> -from accepting TCP connections from clients.
> +Instruct the kernel nfs server not to open and listen on a TCP socket.
> +.TP
> +.B \-u " or " \-\-udp
> +Instruct the kernel nfs server to open and listen on a UDP socket.
>  .TP
>  .B \-U " or " \-\-no-udp
> -Disable
> -.B rpc.nfsd
> -from accepting UDP connections from clients.
> +Instruct the kernel nfs server not to open and listen on a UDP socket. This is the default.
>  .TP
>  .B \-V " or " \-\-nfs-version vers
>  This option can be used to request that 
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default
  2017-02-27 21:53 ` J. Bruce Fields
@ 2017-02-27 22:47   ` Jeff Layton
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2017-02-27 22:47 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: linux-nfs

On Mon, 2017-02-27 at 16:53 -0500, J. Bruce Fields wrote:
> On Sat, Feb 25, 2017 at 08:13:11AM -0500, Jeff Layton wrote:
> > Most major NFS clients have supported TCP for at least a decade now,
> > and v4-only shops are becoming more prevalent. It seems reasonable that
> > serving over UDP should be something that is "opt-in".
> > 
> > I've always hesitated to do this in the past, but now that we have
> > nfs.conf, it seems like the time may be right to disable UDP in default
> > configurations. In particular, it would be good to try this in the more
> > bleeding edge distros (Fedora, Ubuntu, SuSE, etc...) and see how
> > problematic it is.
> > 
> > Change the default in rpc.nfsd to just open TCP ports by default. Add
> > new -u and -t options that allow users to explicitly override what's
> > in the config file, and update the usage message and manpage.
> 
> I guess I'm OK with trying it.
> 
> What does the failure look like (from the Linux client at least) in the
> case of a preexisting UDP mount, or a new UDP mount?  (Do they get a
> useful error, or at least something in sylog if there's a hang?)
> 
> --b.
> 

Just this:

     [  792.075135] nfs: server knfsdsrv not responding, still trying

...basically the kernel stops listening on the socket, so the RPCs just
get an ICMP Destination unreachable back. I'm not sure what we could do
to make it more obvious to users, really.

> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  nfs.conf                  |  2 +-
> >  support/include/nfs/nfs.h |  2 +-
> >  utils/nfsd/nfsd.c         | 18 +++++++++++++-----
> >  utils/nfsd/nfsd.man       | 14 ++++++++------
> >  4 files changed, 23 insertions(+), 13 deletions(-)
> > 
> > diff --git a/nfs.conf b/nfs.conf
> > index 81ece0602dba..5d5a23463cff 100644
> > --- a/nfs.conf
> > +++ b/nfs.conf
> > @@ -42,7 +42,7 @@
> >  # port=0
> >  # grace-time=90
> >  # lease-time=90
> > -# udp=y
> > +# udp=n
> >  # tcp=y
> >  # vers2=n
> >  # vers3=y
> > diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> > index 15ecc6bfc485..5b1ef0ce3883 100644
> > --- a/support/include/nfs/nfs.h
> > +++ b/support/include/nfs/nfs.h
> > @@ -27,6 +27,7 @@ struct nfs_fh_len {
> >  
> >  #define NFSCTL_UDPBIT		      (1 << (17 - 1))
> >  #define NFSCTL_TCPBIT		      (1 << (18 - 1))
> > +#define NFSCTL_PROTODEFAULT	      (NFSCTL_TCPBIT)
> >  
> >  #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1))) 
> >  #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_UDPBIT) 
> > @@ -42,6 +43,5 @@ struct nfs_fh_len {
> >  #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |= NFSCTL_TCPBIT)
> >  
> >  #define NFSCTL_ANYPROTO(_cltbits)     ((_cltbits) & (NFSCTL_UDPBIT | NFSCTL_TCPBIT))
> > -#define NFSCTL_ALLBITS (~0)
> >  
> >  #endif /* _NFS_NFS_H */
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index 20f4b7952203..5d101f0e98c3 100644
> > --- a/utils/nfsd/nfsd.c
> > +++ b/utils/nfsd/nfsd.c
> > @@ -44,7 +44,9 @@ static struct option longopts[] =
> >  	{ "help", 0, 0, 'h' },
> >  	{ "no-nfs-version", 1, 0, 'N' },
> >  	{ "nfs-version", 1, 0, 'V' },
> > +	{ "tcp", 0, 0, 't' },
> >  	{ "no-tcp", 0, 0, 'T' },
> > +	{ "udp", 0, 0, 'u' },
> >  	{ "no-udp", 0, 0, 'U' },
> >  	{ "port", 1, 0, 'P' },
> >  	{ "port", 1, 0, 'p' },
> > @@ -68,7 +70,7 @@ main(int argc, char **argv)
> >  	unsigned int minorvers = 0;
> >  	unsigned int minorversset = 0;
> >  	unsigned int versbits = NFSCTL_VERDEFAULT;
> > -	unsigned int protobits = NFSCTL_ALLBITS;
> > +	unsigned int protobits = NFSCTL_PROTODEFAULT;
> >  	int grace = -1;
> >  	int lease = -1;
> >  
> > @@ -138,7 +140,7 @@ main(int argc, char **argv)
> >  		}
> >  	}
> >  
> > -	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUrG:L:", longopts, NULL)) != EOF) {
> > +	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:stTitUrG:L:", longopts, NULL)) != EOF) {
> >  		switch(c) {
> >  		case 'd':
> >  			xlog_config(D_ALL, 1);
> > @@ -222,9 +224,15 @@ main(int argc, char **argv)
> >  			xlog_syslog(1);
> >  			xlog_stderr(0);
> >  			break;
> > +		case 't':
> > +			NFSCTL_TCPSET(protobits);
> > +			break;
> >  		case 'T':
> >  			NFSCTL_TCPUNSET(protobits);
> >  			break;
> > +		case 'u':
> > +			NFSCTL_UDPSET(protobits);
> > +			break;
> >  		case 'U':
> >  			NFSCTL_UDPUNSET(protobits);
> >  			break;
> > @@ -372,9 +380,9 @@ usage(const char *prog)
> >  {
> >  	fprintf(stderr, "Usage:\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|--grace-time secs] [-L|--leasetime secs] nrservs\n",
> > +		"   [-N|--no-nfs-version version] [-V|--nfs-version version]\n"
> > +		"   [-s|--syslog] [-t|--tcp] [-T|--no-tcp] [-u|--udp] [-U|--no-udp]\n"
> > +		"   [-r|--rdma=] [-G|--grace-time secs] [-L|--leasetime secs] nrservs\n",
> >  		prog);
> >  	exit(2);
> >  }
> > diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> > index 8901fb6c6872..e0c640a64094 100644
> > --- a/utils/nfsd/nfsd.man
> > +++ b/utils/nfsd/nfsd.man
> > @@ -67,15 +67,17 @@ logs error messages (and debug messages, if enabled) to stderr. This option make
> >  log these messages to syslog instead. Note that errors encountered during
> >  option processing will still be logged to stderr regardless of this option.
> >  .TP
> > +.B \-t " or " \-\-tcp
> > +Instruct the kernel nfs server to open and listen on a TCP socket. This is the default.
> > +.TP
> >  .B \-T " or " \-\-no-tcp
> > -Disable 
> > -.B rpc.nfsd 
> > -from accepting TCP connections from clients.
> > +Instruct the kernel nfs server not to open and listen on a TCP socket.
> > +.TP
> > +.B \-u " or " \-\-udp
> > +Instruct the kernel nfs server to open and listen on a UDP socket.
> >  .TP
> >  .B \-U " or " \-\-no-udp
> > -Disable
> > -.B rpc.nfsd
> > -from accepting UDP connections from clients.
> > +Instruct the kernel nfs server not to open and listen on a UDP socket. This is the default.
> >  .TP
> >  .B \-V " or " \-\-nfs-version vers
> >  This option can be used to request that 
> > -- 
> > 2.9.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default
  2017-02-25 13:13 [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default Jeff Layton
  2017-02-27 21:53 ` J. Bruce Fields
@ 2017-04-05 17:30 ` Steve Dickson
  1 sibling, 0 replies; 4+ messages in thread
From: Steve Dickson @ 2017-04-05 17:30 UTC (permalink / raw)
  To: Jeff Layton, linux-nfs



On 02/25/2017 08:13 AM, Jeff Layton wrote:
> Most major NFS clients have supported TCP for at least a decade now,
> and v4-only shops are becoming more prevalent. It seems reasonable that
> serving over UDP should be something that is "opt-in".
> 
> I've always hesitated to do this in the past, but now that we have
> nfs.conf, it seems like the time may be right to disable UDP in default
> configurations. In particular, it would be good to try this in the more
> bleeding edge distros (Fedora, Ubuntu, SuSE, etc...) and see how
> problematic it is.
> 
> Change the default in rpc.nfsd to just open TCP ports by default. Add
> new -u and -t options that allow users to explicitly override what's
> in the config file, and update the usage message and manpage.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
Committed... Let's see what breaks! ;-)

steved.
> ---
>  nfs.conf                  |  2 +-
>  support/include/nfs/nfs.h |  2 +-
>  utils/nfsd/nfsd.c         | 18 +++++++++++++-----
>  utils/nfsd/nfsd.man       | 14 ++++++++------
>  4 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/nfs.conf b/nfs.conf
> index 81ece0602dba..5d5a23463cff 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -42,7 +42,7 @@
>  # port=0
>  # grace-time=90
>  # lease-time=90
> -# udp=y
> +# udp=n
>  # tcp=y
>  # vers2=n
>  # vers3=y
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 15ecc6bfc485..5b1ef0ce3883 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -27,6 +27,7 @@ struct nfs_fh_len {
>  
>  #define NFSCTL_UDPBIT		      (1 << (17 - 1))
>  #define NFSCTL_TCPBIT		      (1 << (18 - 1))
> +#define NFSCTL_PROTODEFAULT	      (NFSCTL_TCPBIT)
>  
>  #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1))) 
>  #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_UDPBIT) 
> @@ -42,6 +43,5 @@ struct nfs_fh_len {
>  #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |= NFSCTL_TCPBIT)
>  
>  #define NFSCTL_ANYPROTO(_cltbits)     ((_cltbits) & (NFSCTL_UDPBIT | NFSCTL_TCPBIT))
> -#define NFSCTL_ALLBITS (~0)
>  
>  #endif /* _NFS_NFS_H */
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 20f4b7952203..5d101f0e98c3 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -44,7 +44,9 @@ static struct option longopts[] =
>  	{ "help", 0, 0, 'h' },
>  	{ "no-nfs-version", 1, 0, 'N' },
>  	{ "nfs-version", 1, 0, 'V' },
> +	{ "tcp", 0, 0, 't' },
>  	{ "no-tcp", 0, 0, 'T' },
> +	{ "udp", 0, 0, 'u' },
>  	{ "no-udp", 0, 0, 'U' },
>  	{ "port", 1, 0, 'P' },
>  	{ "port", 1, 0, 'p' },
> @@ -68,7 +70,7 @@ main(int argc, char **argv)
>  	unsigned int minorvers = 0;
>  	unsigned int minorversset = 0;
>  	unsigned int versbits = NFSCTL_VERDEFAULT;
> -	unsigned int protobits = NFSCTL_ALLBITS;
> +	unsigned int protobits = NFSCTL_PROTODEFAULT;
>  	int grace = -1;
>  	int lease = -1;
>  
> @@ -138,7 +140,7 @@ main(int argc, char **argv)
>  		}
>  	}
>  
> -	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:sTUrG:L:", longopts, NULL)) != EOF) {
> +	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:stTitUrG:L:", longopts, NULL)) != EOF) {
>  		switch(c) {
>  		case 'd':
>  			xlog_config(D_ALL, 1);
> @@ -222,9 +224,15 @@ main(int argc, char **argv)
>  			xlog_syslog(1);
>  			xlog_stderr(0);
>  			break;
> +		case 't':
> +			NFSCTL_TCPSET(protobits);
> +			break;
>  		case 'T':
>  			NFSCTL_TCPUNSET(protobits);
>  			break;
> +		case 'u':
> +			NFSCTL_UDPSET(protobits);
> +			break;
>  		case 'U':
>  			NFSCTL_UDPUNSET(protobits);
>  			break;
> @@ -372,9 +380,9 @@ usage(const char *prog)
>  {
>  	fprintf(stderr, "Usage:\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|--grace-time secs] [-L|--leasetime secs] nrservs\n",
> +		"   [-N|--no-nfs-version version] [-V|--nfs-version version]\n"
> +		"   [-s|--syslog] [-t|--tcp] [-T|--no-tcp] [-u|--udp] [-U|--no-udp]\n"
> +		"   [-r|--rdma=] [-G|--grace-time secs] [-L|--leasetime secs] nrservs\n",
>  		prog);
>  	exit(2);
>  }
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index 8901fb6c6872..e0c640a64094 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -67,15 +67,17 @@ logs error messages (and debug messages, if enabled) to stderr. This option make
>  log these messages to syslog instead. Note that errors encountered during
>  option processing will still be logged to stderr regardless of this option.
>  .TP
> +.B \-t " or " \-\-tcp
> +Instruct the kernel nfs server to open and listen on a TCP socket. This is the default.
> +.TP
>  .B \-T " or " \-\-no-tcp
> -Disable 
> -.B rpc.nfsd 
> -from accepting TCP connections from clients.
> +Instruct the kernel nfs server not to open and listen on a TCP socket.
> +.TP
> +.B \-u " or " \-\-udp
> +Instruct the kernel nfs server to open and listen on a UDP socket.
>  .TP
>  .B \-U " or " \-\-no-udp
> -Disable
> -.B rpc.nfsd
> -from accepting UDP connections from clients.
> +Instruct the kernel nfs server not to open and listen on a UDP socket. This is the default.
>  .TP
>  .B \-V " or " \-\-nfs-version vers
>  This option can be used to request that 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-04-05 17:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-25 13:13 [RFC nfs-utils PATCH] nfsd: don't enable a UDP socket by default Jeff Layton
2017-02-27 21:53 ` J. Bruce Fields
2017-02-27 22:47   ` Jeff Layton
2017-04-05 17:30 ` Steve Dickson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.