2013-10-22 08:18:51

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 0/3] mountd: disabling turning off TCP listeners

Recently it was pointed out to me that the [-n | --no-tcp] flags
were broken in mountd. Sure enough they are and they broke
when nfs-utils moved to using libtirpc, which was years ago.

Obviously nobody is using these flags since has not been
notice until now, but it seemed to me it no longer makes
any sense to have flags. We really want people to use TCP
so why should there be a way to turn it off? It should be
the opposite... They should be able to turn off UDP listeners
not TCP...

So this patch set does just that. It deprecate the ability to
disable TCP listeners and addd the ability to disable UDP listeners.

Steve Dickson (3):
mountd: Use protocol bit fields to turn protocols off.
mountd: Deprecate the ability to disable TCP listeners.
mountd: Add the ability to disable UDP listeners.

support/include/rpcmisc.h | 2 +-
support/nfs/rpcmisc.c | 19 ++++++++++++++-----
support/nfs/svc_create.c | 5 +++++
utils/mountd/mountd.c | 15 +++++++++++----
utils/mountd/mountd.man | 6 +++---
5 files changed, 34 insertions(+), 13 deletions(-)

--
1.8.3.1



2013-10-23 13:58:28

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 2/3] mountd: Deprecate the ability to disable TCP listeners.



On 22/10/13 07:39, Jim Rees wrote:
> Steve Dickson wrote:
>
> -" [-N version|--no-nfs-version version] [-n|--no-tcp]\n"
> +" [-N version|--no-nfs-version version] [-n|--no-tcp(deprecate)]\n"
>
> I would take this out.
>
Take out the word "deprecate" or the "[-n|--no-tcp(deprecate)]" flag?

steved.

2013-10-22 08:18:51

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 1/3] mountd: Use protocol bit fields to turn protocols off.

Convert the current code to used the NFSCTL_XXX macros
to turn off the TCP listener.

Signed-off-by: Steve Dickson <[email protected]>
---
support/include/rpcmisc.h | 2 +-
support/nfs/rpcmisc.c | 19 ++++++++++++++-----
support/nfs/svc_create.c | 5 +++++
utils/mountd/mountd.c | 2 +-
4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/support/include/rpcmisc.h b/support/include/rpcmisc.h
index b806227..31c8e5d 100644
--- a/support/include/rpcmisc.h
+++ b/support/include/rpcmisc.h
@@ -56,7 +56,7 @@ void rpc_dispatch(struct svc_req *rq, SVCXPRT *xprt,
int getservport(u_long number, const char *proto);

extern int _rpcpmstart;
-extern int _rpcfdtype;
+extern unsigned int _rpcprotobits;
extern int _rpcsvcdirty;

static inline struct sockaddr_in *nfs_getrpccaller_in(SVCXPRT *xprt)
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index b73187a..64c98ff 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -39,7 +39,7 @@

#define _RPCSVC_CLOSEDOWN 120
int _rpcpmstart = 0;
-int _rpcfdtype = 0;
+unsigned int _rpcprotobits = (NFSCTL_UDPBIT|NFSCTL_TCPBIT);
int _rpcsvcdirty = 0;

static void
@@ -51,7 +51,7 @@ closedown(int sig)
static int size;
int i, openfd;

- if (_rpcfdtype == SOCK_DGRAM)
+ if (NFSCTL_TCPISSET(_rpcprotobits) == 0)
exit(0);

if (size == 0)
@@ -130,7 +130,16 @@ rpc_init(char *name, int prog, int vers,
* listen will fail on a connected TCP socket(passed by rsh).
*/
if (!(fdtype == SOCK_STREAM && listen(0,5) == -1)) {
- _rpcfdtype = fdtype;
+ switch(fdtype) {
+ case SOCK_DGRAM:
+ NFSCTL_UDPSET(_rpcprotobits);
+ break;
+ case SOCK_STREAM:
+ NFSCTL_TCPSET(_rpcprotobits);
+ break;
+ default:
+ xlog(L_FATAL, "getsockopt returns bad socket type: %d", fdtype);
+ }
_rpcpmstart = 1;
}
}
@@ -139,7 +148,7 @@ rpc_init(char *name, int prog, int vers,
sock = RPC_ANYSOCK;
}

- if ((_rpcfdtype == 0) || (_rpcfdtype == SOCK_DGRAM)) {
+ if (NFSCTL_UDPISSET(_rpcprotobits)) {
static SVCXPRT *last_transp = NULL;

if (_rpcpmstart == 0) {
@@ -167,7 +176,7 @@ rpc_init(char *name, int prog, int vers,
last_transp = transp;
}

- if ((_rpcfdtype == 0) || (_rpcfdtype == SOCK_STREAM)) {
+ if (NFSCTL_TCPISSET(_rpcprotobits)) {
static SVCXPRT *last_transp = NULL;

if (_rpcpmstart == 0) {
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index c159fc8..9ae2965 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -29,6 +29,7 @@
#include <unistd.h>
#include <errno.h>
#include <netdb.h>
+#include "nfslib.h"

#include <netinet/in.h>

@@ -417,6 +418,10 @@ nfs_svc_create(char *name, const rpcprog_t program, const rpcvers_t version,
if (!(nconf->nc_flag & NC_VISIBLE))
continue;
visible++;
+
+ if (!strcmp(nconf->nc_proto, NC_TCP) && !NFSCTL_TCPISSET(_rpcprotobits))
+ continue;
+
if (port == 0)
servport = getservport(program, nconf->nc_proto);
else
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 993b6e6..f918472 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -755,7 +755,7 @@ main(int argc, char **argv)
nfs_version &= ~NFSVERSBIT(vers);
break;
case 'n':
- _rpcfdtype = SOCK_DGRAM;
+ NFSCTL_TCPUNSET(_rpcprotobits);
break;
case 'r':
reverse_resolve = 1;
--
1.8.3.1


2013-10-23 14:59:41

by Jim Rees

[permalink] [raw]
Subject: Re: [PATCH 2/3] mountd: Deprecate the ability to disable TCP listeners.

Steve Dickson wrote:

On 22/10/13 07:39, Jim Rees wrote:
> Steve Dickson wrote:
>
> -" [-N version|--no-nfs-version version] [-n|--no-tcp]\n"
> +" [-N version|--no-nfs-version version] [-n|--no-tcp(deprecate)]\n"
>
> I would take this out.
>
Take out the word "deprecate" or the "[-n|--no-tcp(deprecate)]" flag?

Like this:

" [-N version|--no-nfs-version version]\n"

2013-10-22 11:39:46

by Jim Rees

[permalink] [raw]
Subject: Re: [PATCH 2/3] mountd: Deprecate the ability to disable TCP listeners.

Steve Dickson wrote:

-" [-N version|--no-nfs-version version] [-n|--no-tcp]\n"
+" [-N version|--no-nfs-version version] [-n|--no-tcp(deprecate)]\n"

I would take this out.

2013-10-22 08:18:52

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 3/3] mountd: Add the ability to disable UDP listeners.

Add the ability to turn off UDP listeners with the
new "-u | --no-udp" flag.

Signed-off-by: Steve Dickson <[email protected]>
---
support/nfs/svc_create.c | 2 +-
utils/mountd/mountd.c | 8 ++++++--
utils/mountd/mountd.man | 3 +++
3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index 9ae2965..dac17d9 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -419,7 +419,7 @@ nfs_svc_create(char *name, const rpcprog_t program, const rpcvers_t version,
continue;
visible++;

- if (!strcmp(nconf->nc_proto, NC_TCP) && !NFSCTL_TCPISSET(_rpcprotobits))
+ if (!strcmp(nconf->nc_proto, NC_UDP) && !NFSCTL_UDPISSET(_rpcprotobits))
continue;

if (port == 0)
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 46ff05f..3d7f7f3 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -68,6 +68,7 @@ static struct option longopts[] =
{ "num-threads", 1, 0, 't' },
{ "reverse-lookup", 0, 0, 'r' },
{ "manage-gids", 0, 0, 'g' },
+ { "no-udp", 0, 0, 'u' },
{ NULL, 0, 0, 0 }
};

@@ -708,7 +709,7 @@ main(int argc, char **argv)

/* Parse the command line options and arguments. */
opterr = 0;
- while ((c = getopt_long(argc, argv, "o:nFd:f:p:P:hH:N:V:vrs:t:g", longopts, NULL)) != EOF)
+ while ((c = getopt_long(argc, argv, "o:nFd:f:p:P:hH:N:V:vurs:t:g", longopts, NULL)) != EOF)
switch (c) {
case 'g':
manage_gids = 1;
@@ -785,6 +786,9 @@ main(int argc, char **argv)
case 'v':
printf("%s version " VERSION "\n", progname);
exit(0);
+ case 'u':
+ NFSCTL_UDPUNSET(_rpcprotobits);
+ break;
case 0:
break;
case '?':
@@ -910,6 +914,6 @@ usage(const char *prog, int n)
" [-p|--port port] [-V version|--nfs-version version]\n"
" [-N version|--no-nfs-version version] [-n|--no-tcp(deprecate)]\n"
" [-H ha-callout-prog] [-s|--state-directory-path path]\n"
-" [-g|--manage-gids] [-t num|--num-threads=num]\n", prog);
+" [-g|--manage-gids] [-t num|--num-threads=num] [-u|--no-udp]\n", prog);
exit(n);
}
diff --git a/utils/mountd/mountd.man b/utils/mountd/mountd.man
index c049e35..5d99659 100644
--- a/utils/mountd/mountd.man
+++ b/utils/mountd/mountd.man
@@ -178,6 +178,9 @@ threads are usually only needed for NFS servers which need to handle
mount storms of hundreds of NFS mounts in a few seconds, or when
your DNS server is slow or unreliable.
.TP
+.B \-u " or " \-\-no-udp
+Don't advertise UDP for mounting
+.TP
.B \-V " or " \-\-nfs-version
This option can be used to request that
.B rpc.mountd
--
1.8.3.1


2013-10-22 08:18:51

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 2/3] mountd: Deprecate the ability to disable TCP listeners.

Disable the ability to turn off TCP listeners since that
is the protocol now required and best suited for
NFS traffic.

Signed-off-by: Steve Dickson <[email protected]>
---
utils/mountd/mountd.c | 7 +++++--
utils/mountd/mountd.man | 3 ---
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index f918472..46ff05f 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -755,7 +755,10 @@ main(int argc, char **argv)
nfs_version &= ~NFSVERSBIT(vers);
break;
case 'n':
- NFSCTL_TCPUNSET(_rpcprotobits);
+ fprintf(stderr,
+ "Deprecated: The -n flag (the ability to disable TCP listeners)" \
+ " is no longer supported\n");
+ usage(progname, 1);
break;
case 'r':
reverse_resolve = 1;
@@ -905,7 +908,7 @@ usage(const char *prog, int n)
"Usage: %s [-F|--foreground] [-h|--help] [-v|--version] [-d kind|--debug kind]\n"
" [-o num|--descriptors num] [-f exports-file|--exports-file=file]\n"
" [-p|--port port] [-V version|--nfs-version version]\n"
-" [-N version|--no-nfs-version version] [-n|--no-tcp]\n"
+" [-N version|--no-nfs-version version] [-n|--no-tcp(deprecate)]\n"
" [-H ha-callout-prog] [-s|--state-directory-path path]\n"
" [-g|--manage-gids] [-t num|--num-threads=num]\n", prog);
exit(n);
diff --git a/utils/mountd/mountd.man b/utils/mountd/mountd.man
index b60dc90..c049e35 100644
--- a/utils/mountd/mountd.man
+++ b/utils/mountd/mountd.man
@@ -112,9 +112,6 @@ either one of these version should not be offered,
must be invoked with the option
.B "\-\-no-nfs-version <vers>" .
.TP
-.B \-n " or " \-\-no-tcp
-Don't advertise TCP for mount.
-.TP
.B \-P
Ignored (compatibility with unfsd??).
.TP
--
1.8.3.1