[PATCH 0/6] nfs-utils: add IPv6 support for rpc.nfsd (try #2)

[PATCH 0/6] nfs-utils: add IPv6 support for rpc.nfsd (try #2)

[Jeff Layton]

This is the second attempt to add support IPv6 to rpc.nfsd. The main
changes since the last patchset are:

1) the addition of a patch that makes nfsd look at /etc/netconfig when
tirpc is enabled, and to disable any family/protocol combinations that
don't have visible netid's.

2) stop linking in some unneeded .a libs.

3) some minor bugfixes

4) update to the nfsd manpage for new options

I think the set is bisectable, but have only really tested the final
result. I've also tested the final result when built with tirpc enabled
and disabled, and ipv6 enabled and disabled.

I haven't seen any regressions when testing on recent mainline kernels
and The only real difference that anyone may notice is that when IPv6
support is built in, and the kernel doesn't support IPv6, nfsd logs this
message a couple of times when starting nfsd:

nfssvc: writing fds to kernel failed: errno 97 (Address family not supported by protocol)

Most of the testing I've done has been by watching the program under
strace.  Since most of the kernel work for IPv6 support hasn't made it
to mainline, it's a little difficult to do much testing of this beyond
that.

Jeff Layton (6):
  nfs-utils: don't link libexport.a and libmisc.a to nfsd
  nfs-utils: break up nfssvc.c into more individually callable
    functions
  nfs-utils: set IPV6_V6ONLY on nfssvc IPv6 sockets
  nfs-utils: add IPv6 support to nfsd
  nfs-utils: limit protocols and families used by nfsd to those listed
    in /etc/netconfig
  nfs-utils: add -4 and -6 options to nfsd manpage

 support/include/nfslib.h |    8 ++-
 support/nfs/nfssvc.c     |  164 +++++++++++++++++++++++++++++++++----------
 utils/nfsd/Makefile.am   |    4 +-
 utils/nfsd/nfsd.c        |  175 ++++++++++++++++++++++++++++++++++++++--------
 utils/nfsd/nfsd.man      |   10 +++
 5 files changed, 292 insertions(+), 69 deletions(-)

[PATCH 1/6] nfs-utils: don't link libexport.a and libmisc.a to nfsd

[Jeff Layton]

...they aren't needed.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsd/Makefile.am |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/utils/nfsd/Makefile.am b/utils/nfsd/Makefile.am
index 445e3fd..ea85cd5 100644
--- a/utils/nfsd/Makefile.am
+++ b/utils/nfsd/Makefile.am
@@ -8,9 +8,7 @@ KPREFIX		= @kprefix@
 sbin_PROGRAMS	= nfsd
 
 nfsd_SOURCES = nfsd.c
-nfsd_LDADD = ../../support/export/libexport.a \
-	     ../../support/nfs/libnfs.a \
-	     ../../support/misc/libmisc.a
+nfsd_LDADD = ../../support/nfs/libnfs.a
 
 MAINTAINERCLEANFILES = Makefile.in
 
-- 
1.6.0.6

[PATCH 2/6] nfs-utils: break up nfssvc.c into more individually callable functions

[Jeff Layton]

nfssvc.c contains functions for starting up knfsd. Currently, the only
non-static function in that file is nfssvc(). In order to add IPv6
support, we'll need to be able to call some of these functions in a
more granular fashion.

Reorganize these functions and add prototypes to the header so that
they can be called individually, and change the nfsd program to call
those routines individually.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 support/include/nfslib.h |    6 +++-
 support/nfs/nfssvc.c     |   75 +++++++++++++++++++++++++---------------------
 utils/nfsd/nfsd.c        |   17 ++++++++++-
 3 files changed, 62 insertions(+), 36 deletions(-)

diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index ae98650..0bfb2ee 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -130,7 +130,11 @@ int			wildmat(char *text, char *pattern);
  * nfsd library functions.
  */
 int			nfsctl(int, struct nfsctl_arg *, union nfsctl_res *);
-int			nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4, unsigned int portbits, char *haddr);
+int			nfssvc_inuse(void);
+void			nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa,
+					socklen_t addrlen);
+void			nfssvc_setvers(unsigned int ctlbits, int minorvers4);
+int			nfssvc_threads(unsigned short port, int nrservs);
 int			nfsaddclient(struct nfsctl_client *clp);
 int			nfsdelclient(struct nfsctl_client *clp);
 int			nfsexport(struct nfsctl_export *exp);
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 33c15a7..c2c5480 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -25,44 +25,55 @@
 #define NFSD_VERS_FILE    "/proc/fs/nfsd/versions"
 #define NFSD_THREAD_FILE  "/proc/fs/nfsd/threads"
 
-static void
-nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
+/*
+ * Are there already sockets configured? If not, then it is safe to try to
+ * open some and pass them through.
+ *
+ * Note: If the user explicitly asked for 'udp', then we should probably check
+ * if that is open, and should open it if not.  However we don't yet. All
+ * sockets * have to be opened when the first daemon is started.
+ */
+int
+nfssvc_inuse(void)
 {
-	int fd, n, on=1;
+	int fd, n;
 	char buf[BUFSIZ];
-	int udpfd = -1, tcpfd = -1;
-	struct sockaddr_in sin;
 
 	fd = open(NFSD_PORTS_FILE, O_RDONLY);
+
+	/* problem opening file, assume that nothing is configured */
 	if (fd < 0)
-		return;
+		return 0;
+
 	n = read(fd, buf, BUFSIZ);
 	close(fd);
+
 	if (n != 0)
-		return;
-	/* there are no ports currently open, so it is safe to
-	 * try to open some and pass them through.
-	 * Note: If the user explicitly asked for 'udp', then
-	 * we should probably check if that is open, and should
-	 * open it if not.  However we don't yet.  All sockets
-	 * have to be opened when the first daemon is started.
-	 */
+		return 1;
+
+	return 0;
+}
+
+void
+nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
+{
+	int fd, on = 1;
+	char buf[BUFSIZ];
+	int udpfd = -1, tcpfd = -1;
+
 	fd = open(NFSD_PORTS_FILE, O_WRONLY);
 	if (fd < 0)
 		return;
-	sin.sin_family = AF_INET;
-	sin.sin_port   = htons(port);
-	sin.sin_addr.s_addr =  inet_addr(haddr);
 
 	if (NFSCTL_UDPISSET(ctlbits)) {
 		udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
 		if (udpfd < 0) {
-			syslog(LOG_ERR, "nfssvc: unable to create UPD socket: "
+			syslog(LOG_ERR, "nfssvc: unable to create UDP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
 		}
-		if (bind(udpfd, (struct  sockaddr  *)&sin, sizeof(sin)) < 0){
-			syslog(LOG_ERR, "nfssvc: unable to bind UPD socket: "
+		if (bind(udpfd, sa, addrlen) < 0) {
+			syslog(LOG_ERR, "nfssvc: unable to bind UDP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
 		}
@@ -71,7 +82,7 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
 	if (NFSCTL_TCPISSET(ctlbits)) {
 		tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
 		if (tcpfd < 0) {
-			syslog(LOG_ERR, "nfssvc: unable to createt tcp socket: "
+			syslog(LOG_ERR, "nfssvc: unable to create TCP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
 		}
@@ -80,7 +91,7 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
 		}
-		if (bind(tcpfd, (struct  sockaddr  *)&sin, sizeof(sin)) < 0){
+		if (bind(tcpfd, sa, addrlen) < 0) {
 			syslog(LOG_ERR, "nfssvc: unable to bind TCP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
@@ -111,12 +122,15 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
 			       errno, strerror(errno));
 		}
 	}
-	close(fd);
+
+	if (fd >= 0)
+		close(fd);
 
 	return;
 }
-static void
-nfssvc_versbits(unsigned int ctlbits, int minorvers4)
+
+void
+nfssvc_setvers(unsigned int ctlbits, int minorvers4)
 {
 	int fd, n, off;
 	char buf[BUFSIZ], *ptr;
@@ -147,20 +161,13 @@ nfssvc_versbits(unsigned int ctlbits, int minorvers4)
 
 	return;
 }
+
 int
-nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
-	unsigned protobits, char *haddr)
+nfssvc_threads(unsigned short port, int nrservs)
 {
 	struct nfsctl_arg	arg;
 	int fd;
 
-	/* Note: must set versions before fds so that
-	 * the ports get registered with portmap against correct
-	 * versions
-	 */
-	nfssvc_versbits(versbits, minorvers4);
-	nfssvc_setfds(port, protobits, haddr);
-
 	fd = open(NFSD_THREAD_FILE, O_WRONLY);
 	if (fd < 0)
 		fd = open("/proc/fs/nfs/threads", O_WRONLY);
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index e3c0094..ec6c49a 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -51,6 +51,7 @@ main(int argc, char **argv)
 	struct servent *ent;
 	struct hostent *hp;
 	char *p;
+	struct sockaddr_in sin;
 
 	ent = getservbyname ("nfs", "udp");
 	if (ent != NULL)
@@ -163,8 +164,22 @@ main(int argc, char **argv)
 	}
 	closeall(3);
 
+	sin.sin_family = AF_INET;
+	sin.sin_port = htons(port);
+	sin.sin_addr.s_addr = inet_addr(haddr);
+
+	/*
+	 * 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.
+	 */
+	if (!nfssvc_inuse()) {
+		nfssvc_setvers(versbits, minorvers4);
+		nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin));
+	}
+
 	openlog("nfsd", LOG_PID, LOG_DAEMON);
-	if ((error = nfssvc(port, count, versbits, minorvers4, protobits, haddr)) < 0) {
+	if ((error = nfssvc_threads(port, count)) < 0) {
 		int e = errno;
 		syslog(LOG_ERR, "nfssvc: %s", strerror(e));
 		closelog();
-- 
1.6.0.6

[PATCH 3/6] nfs-utils: set IPV6_V6ONLY on nfssvc IPv6 sockets

[Jeff Layton]

IPv6 sockets for knfsd can't be allowed to accept IPv4 packets. Set the
correct option to prevent it.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 support/nfs/nfssvc.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index c2c5480..b14d37d 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -60,18 +60,26 @@ nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 	int fd, on = 1;
 	char buf[BUFSIZ];
 	int udpfd = -1, tcpfd = -1;
+	unsigned short family = sa->sa_family;
 
 	fd = open(NFSD_PORTS_FILE, O_WRONLY);
 	if (fd < 0)
 		return;
 
 	if (NFSCTL_UDPISSET(ctlbits)) {
-		udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
+		udpfd = socket(family, SOCK_DGRAM, IPPROTO_UDP);
 		if (udpfd < 0) {
 			syslog(LOG_ERR, "nfssvc: unable to create UDP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
 		}
+		if (family == AF_INET6 && setsockopt(udpfd, IPPROTO_IPV6,
+						     IPV6_V6ONLY, &on,
+						     sizeof(on))) {
+			syslog(LOG_ERR, "nfssvc: unable to set IPV6_V6ONLY: "
+				"errno %d (%s)\n", errno, strerror(errno));
+			exit(1);
+		}
 		if (bind(udpfd, sa, addrlen) < 0) {
 			syslog(LOG_ERR, "nfssvc: unable to bind UDP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
@@ -80,7 +88,7 @@ nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 	}
 
 	if (NFSCTL_TCPISSET(ctlbits)) {
-		tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+		tcpfd = socket(family, SOCK_STREAM, IPPROTO_TCP);
 		if (tcpfd < 0) {
 			syslog(LOG_ERR, "nfssvc: unable to create TCP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
@@ -91,6 +99,14 @@ nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 				"errno %d (%s)\n", errno, strerror(errno));
 			exit(1);
 		}
+		if (family == AF_INET6 && setsockopt(tcpfd, IPPROTO_IPV6,
+						     IPV6_V6ONLY, &on,
+						     sizeof(on))) {
+			syslog(LOG_ERR, "nfssvc: unable to set IPV6_V6ONLY: "
+				"errno %d (%s)\n", errno, strerror(errno));
+			exit(1);
+		}
+
 		if (bind(tcpfd, sa, addrlen) < 0) {
 			syslog(LOG_ERR, "nfssvc: unable to bind TCP socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
-- 
1.6.0.6

[PATCH 4/6] nfs-utils: add IPv6 support to nfsd

[Jeff Layton]

Add 2 new options to rpc.nfsd -- -4 and -6. -4 makes it an IPv4-only
server, and -6 makes it IPv6-only. Restructure the -H option so that
if the address appears to be or resolves to an IPv4 address, that
IPv6 is disabled. Ditto if it resolves to an IPv6 address.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsd/nfsd.c |  151 ++++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 120 insertions(+), 31 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index ec6c49a..29611d4 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -41,8 +41,6 @@ static struct option longopts[] =
 };
 unsigned int protobits = NFSCTL_ALLBITS;
 unsigned int versbits = NFSCTL_ALLBITS;
-int minorvers4 = NFSD_MAXMINORVERS4;		/* nfsv4 minor version */
-char *haddr = NULL;
 
 int
 main(int argc, char **argv)
@@ -51,7 +49,16 @@ main(int argc, char **argv)
 	struct servent *ent;
 	struct hostent *hp;
 	char *p;
-	struct sockaddr_in sin;
+	struct sockaddr_in sin = { };
+	struct sockaddr_in6 sin6 = { };
+	int minorvers4 = NFSD_MAXMINORVERS4;	/* nfsv4 minor version */
+	char	*haddr = NULL;
+	int	ipv4 = 1;
+#ifdef IPV6_SUPPORTED
+	int	ipv6 = 1;
+#else  /* IPV6_SUPPORTED */
+	int	ipv6 = 0;
+#endif /* IPV6_SUPPORTED */
 
 	ent = getservbyname ("nfs", "udp");
 	if (ent != NULL)
@@ -59,18 +66,12 @@ main(int argc, char **argv)
 	else
 		port = 2049;
 
-	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", longopts, NULL)) != EOF) {
+	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU46", longopts, NULL)) != EOF) {
 		switch(c) {
 		case 'H':
-			if (inet_addr(optarg) != INADDR_NONE) {
-				haddr = strdup(optarg);
-			} else if ((hp = gethostbyname(optarg)) != NULL) {
-				haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
-			} else {
-				fprintf(stderr, "%s: Unknown hostname: %s\n",
-					argv[0], optarg);
-				usage(argv [0]);
-			}
+			/* we only deal with one host addr at the moment */
+			free(haddr);
+			haddr = strdup(optarg);
 			break;
 		case 'P':	/* XXX for nfs-server compatibility */
 		case 'p':
@@ -98,24 +99,85 @@ main(int argc, char **argv)
 			}
 			break;
 		case 'T':
-				NFSCTL_TCPUNSET(protobits);
-				break;
+			NFSCTL_TCPUNSET(protobits);
+			break;
 		case 'U':
-				NFSCTL_UDPUNSET(protobits);
-				break;
+			NFSCTL_UDPUNSET(protobits);
+			break;
+		case '4':
+			ipv6 = 0;
+			break;
+		case '6':
+			ipv4 = 0;
+			break;
 		default:
 			fprintf(stderr, "Invalid argument: '%c'\n", c);
 		case 'h':
 			usage(argv[0]);
 		}
 	}
-	/*
-	 * Do some sanity checking, if the ctlbits are set
-	 */
+
+	/* sanity checks */
+
+	/* if an address was specified, check it first */
+	if (haddr) {
+		/* does it look like an addr of some sort? */
+		if (inet_pton(AF_INET, haddr, &sin.sin_addr)) {
+			ipv6 = 0;
+			goto family_check;
+		} else if (inet_pton(AF_INET6, haddr, &sin6.sin6_addr)) {
+			ipv4 = 0;
+			goto family_check;
+		}
+
+		/* see if it's a hostname */
+		hp = gethostbyname(haddr);
+		if (!hp) {
+			fprintf(stderr, "%s: gethostbyname on %s failed: %d\n",
+					argv[0], haddr, h_errno);
+			error = h_errno;
+			usage(argv[0]);
+		}
+
+		switch (hp->h_addrtype) {
+		case AF_INET:
+			ipv6 = 0;
+			memcpy(&sin.sin_addr, hp->h_addr_list[0],
+			       hp->h_length);
+			if (sin.sin_addr.s_addr == INADDR_NONE) {
+				fprintf(stderr, "%s: Bad hostaddr %s\n",
+						argv[0], haddr);
+				usage(argv[0]);
+			}
+			break;
+#ifdef IPV6_SUPPORTED
+		case AF_INET6:
+			ipv4 = 0;
+			memcpy(&sin6.sin6_addr, hp->h_addr_list[0],
+			       hp->h_length);
+			break;
+#endif /* IPV6_SUPPORTED */
+		default:
+			fprintf(stderr, "%s: unsupported address family %d\n",
+					argv[0], hp->h_addrtype);
+			exit(0);
+		}
+	}
+
+family_check:
+	/* make sure at least one address family is enabled */
+	if (!ipv4 && !ipv6) {
+		fprintf(stderr, "no address families enabled\n");
+		exit(1);
+	}
+
+	/* make sure at least one protocol type is enabled */
 	if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
 		fprintf(stderr, "invalid protocol specified\n");
 		exit(1);
 	}
+
+	/* make sure that at least one version is enabled */
 	found_one = 0;
 	for (c = NFSD_MINVERS; c <= NFSD_MAXVERS; c++) {
 		if (NFSCTL_VERISSET(versbits, c))
@@ -126,14 +188,11 @@ main(int argc, char **argv)
 		exit(1);
 	}			
 
+	/* must have TCP for NFSv4 */
 	if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
 		fprintf(stderr, "version 4 requires the TCP protocol\n");
 		exit(1);
 	}
-	if (haddr == NULL) {
-		struct in_addr in = {INADDR_ANY}; 
-		haddr = strdup(inet_ntoa(in));
-	}
 
 	if (chdir(NFS_STATEDIR)) {
 		fprintf(stderr, "%s: chdir(%s) failed: %s\n",
@@ -148,6 +207,12 @@ main(int argc, char **argv)
 				"%s: invalid server count (%d), using 1\n",
 				argv[0], count);
 			count = 1;
+		} else if (count == 0) {
+			/*
+			 * don't bother setting anything else if the threads
+			 * are coming down anyway.
+			 */
+			goto set_threads;
 		}
 	}
 	/* KLUDGE ALERT:
@@ -163,28 +228,52 @@ main(int argc, char **argv)
 		(void) dup2(fd, 2);
 	}
 	closeall(3);
+	openlog("nfsd", LOG_PID, LOG_DAEMON);
 
-	sin.sin_family = AF_INET;
-	sin.sin_port = htons(port);
-	sin.sin_addr.s_addr = inet_addr(haddr);
+	/*
+	 * skip everything but setting of number of threads if sockets are
+	 * already open and in use.
+	 */
+	if (nfssvc_inuse())
+		goto set_threads;
 
 	/*
 	 * 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.
 	 */
-	if (!nfssvc_inuse()) {
-		nfssvc_setvers(versbits, minorvers4);
+	nfssvc_setvers(versbits, minorvers4);
+
+	if (ipv4) {
+		sin.sin_family = AF_INET;
+		sin.sin_port = htons(port);
+		if (!haddr)
+			sin.sin_addr.s_addr = INADDR_ANY;
+
 		nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin));
 	}
 
-	openlog("nfsd", LOG_PID, LOG_DAEMON);
+#ifdef IPV6_SUPPORTED
+	if (ipv6) {
+		sin6.sin6_family = AF_INET6;
+		sin6.sin6_port = htons(port);
+		if (!haddr)
+			sin6.sin6_addr = in6addr_any;
+
+		nfssvc_setfds(protobits, (struct sockaddr *) &sin6,
+				sizeof(sin6));
+	}
+#endif /* IPV6_SUPPORTED */
+
+set_threads:
 	if ((error = nfssvc_threads(port, count)) < 0) {
 		int e = errno;
 		syslog(LOG_ERR, "nfssvc: %s", strerror(e));
 		closelog();
 	}
 
+	free(haddr);
+	closelog();
 	return (error != 0);
 }
 
@@ -192,7 +281,7 @@ static void
 usage(const char *prog)
 {
 	fprintf(stderr, "Usage:\n"
-		"%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-T|--no-tcp] [-U|--no-udp] nrservs\n", 
+		"%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-T|--no-tcp] [-U|--no-udp] [-4] [-6] nrservs\n", 
 		prog);
 	exit(2);
 }
-- 
1.6.0.6

[PATCH 5/6] nfs-utils: limit protocols and families used by nfsd to those listed in /etc/netconfig

[Jeff Layton]

...disable any that aren't listed or aren't marked as "visible".

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 support/include/nfslib.h |    2 +
 support/nfs/nfssvc.c     |   69 ++++++++++++++++++++++++++++++++++++++++++++++
 utils/nfsd/nfsd.c        |   33 +++++++++++++++------
 3 files changed, 94 insertions(+), 10 deletions(-)

diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index 0bfb2ee..7871ac1 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -131,6 +131,8 @@ int			wildmat(char *text, char *pattern);
  */
 int			nfsctl(int, struct nfsctl_arg *, union nfsctl_res *);
 int			nfssvc_inuse(void);
+unsigned int		nfssvc_set_family_proto(const sa_family_t family,
+						unsigned int ctlbits);
 void			nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa,
 					socklen_t addrlen);
 void			nfssvc_setvers(unsigned int ctlbits, int minorvers4);
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index b14d37d..e5b52f2 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -18,8 +18,13 @@
 #include <errno.h>
 #include <syslog.h>
 
+#ifdef HAVE_LIBTIRPC
+#include <netdb.h>
+#include <netconfig.h>
+#endif
 
 #include "nfslib.h"
+#include "nfsrpc.h"
 
 #define NFSD_PORTS_FILE     "/proc/fs/nfsd/portlist"
 #define NFSD_VERS_FILE    "/proc/fs/nfsd/versions"
@@ -54,6 +59,70 @@ nfssvc_inuse(void)
 	return 0;
 }
 
+#ifdef HAVE_LIBTIRPC
+static unsigned int
+nfssvc_netid_visible(const sa_family_t family, const unsigned short protocol)
+{
+	char *nc_protofmly, *nc_proto;
+	struct netconfig *nconf;
+	struct protoent *proto;
+	void *handle;
+
+	switch (family) {
+	case AF_LOCAL:
+	case AF_INET:
+		nc_protofmly = NC_INET;
+		break;
+	case AF_INET6:
+		nc_protofmly = NC_INET6;
+		break;
+	default:
+		return 0;
+	}
+
+	proto = getprotobynumber(protocol);
+	if (proto == NULL)
+		return 0;
+	nc_proto = proto->p_name;
+
+	handle = setnetconfig();
+	while((nconf = getnetconfig(handle))) {
+		if (!(nconf->nc_flag & NC_VISIBLE))
+			continue;
+		if (nconf->nc_protofmly &&
+		    strcmp(nconf->nc_protofmly, nc_protofmly))
+			continue;
+		if (nconf->nc_proto && strcmp(nconf->nc_proto, nc_proto))
+			continue;
+		endnetconfig(handle);
+		return 1;
+	}
+	endnetconfig(handle);
+	return 0;
+}
+#else
+static unsigned int
+nfssvc_netid_visible(const sa_family_t family, const unsigned short protocol)
+{
+	return 1;
+}
+#endif
+
+/* given a family and ctlbits, disable any that aren't listed in netconfig */
+unsigned int
+nfssvc_set_family_proto(const sa_family_t family, unsigned int ctlbits)
+{
+	if (NFSCTL_UDPISSET(ctlbits) &&
+	    !nfssvc_netid_visible(family, IPPROTO_UDP))
+		NFSCTL_UDPUNSET(ctlbits);
+
+	if (NFSCTL_TCPISSET(ctlbits) &&
+	    !nfssvc_netid_visible(family, IPPROTO_TCP))
+		NFSCTL_TCPUNSET(ctlbits);
+
+	return ctlbits;
+}
+
 void
 nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 {
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 29611d4..578fcbe 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -54,6 +54,8 @@ main(int argc, char **argv)
 	int minorvers4 = NFSD_MAXMINORVERS4;	/* nfsv4 minor version */
 	char	*haddr = NULL;
 	int	ipv4 = 1;
+	unsigned int proto4;
+	unsigned int proto6;
 #ifdef IPV6_SUPPORTED
 	int	ipv6 = 1;
 #else  /* IPV6_SUPPORTED */
@@ -165,15 +167,25 @@ main(int argc, char **argv)
 	}
 
 family_check:
-	/* make sure at least one address family is enabled */
-	if (!ipv4 && !ipv6) {
-		fprintf(stderr, "no address families enabled\n");
-		exit(1);
+	/* limit protocols to use based on /etc/netconfig */
+	proto4 = nfssvc_set_family_proto(AF_INET, protobits);
+	proto6 = nfssvc_set_family_proto(AF_INET6, protobits);
+
+	/* make sure at least one protocol type is enabled */
+	if (ipv4 && !NFSCTL_UDPISSET(proto4) && !NFSCTL_TCPISSET(proto4)) {
+		fprintf(stderr, "WARNING: no protocols enabled for IPv4\n");
+		ipv4 = 0;
 	}
 
 	/* make sure at least one protocol type is enabled */
-	if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
-		fprintf(stderr, "invalid protocol specified\n");
+	if (ipv6 && !NFSCTL_UDPISSET(proto6) && !NFSCTL_TCPISSET(proto6)) {
+		fprintf(stderr, "WARNING: no protocols enabled for IPv6\n");
+		ipv6 = 0;
+	}
+
+	/* make sure at least one address family is enabled */
+	if (!ipv4 && !ipv6) {
+		fprintf(stderr, "no address families enabled\n");
 		exit(1);
 	}
 
@@ -189,7 +201,9 @@ family_check:
 	}			
 
 	/* must have TCP for NFSv4 */
-	if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
+	if (NFSCTL_VERISSET(versbits, 4) &&
+	    !NFSCTL_TCPISSET(proto4) &&
+	    !NFSCTL_TCPISSET(proto6)) {
 		fprintf(stderr, "version 4 requires the TCP protocol\n");
 		exit(1);
 	}
@@ -250,7 +264,7 @@ family_check:
 		if (!haddr)
 			sin.sin_addr.s_addr = INADDR_ANY;
 
-		nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin));
+		nfssvc_setfds(proto4, (struct sockaddr *) &sin, sizeof(sin));
 	}
 
 #ifdef IPV6_SUPPORTED
@@ -260,8 +274,7 @@ family_check:
 		if (!haddr)
 			sin6.sin6_addr = in6addr_any;
 
-		nfssvc_setfds(protobits, (struct sockaddr *) &sin6,
-				sizeof(sin6));
+		nfssvc_setfds(proto6, (struct sockaddr *) &sin6, sizeof(sin6));
 	}
 #endif /* IPV6_SUPPORTED */
 
-- 
1.6.0.6

[PATCH 6/6] nfs-utils: add -4 and -6 options to nfsd manpage

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsd/nfsd.man |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 4eb3e21..f4c0475 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -22,6 +22,16 @@ server provides an ancillary service needed to satisfy mount requests
 by NFS clients.
 .SH OPTIONS
 .TP
+.B \-4
+Disable
+.B rpc.nfsd
+listening on IPv4 sockets.
+.TP
+.B \-6
+Disable
+.B rpc.nfsd
+listening on IPv6 sockets.
+.TP
 .B \-H " or " \-\-host  hostname
 specify a particular hostname (or address) that NFS requests will
 be accepted on. By default,
-- 
1.6.0.6

Re: [PATCH 4/6] nfs-utils: add IPv6 support to nfsd

[Chuck Lever]

Hi Jeff-

On May 26, 2009, at 11:15 AM, Jeff Layton wrote:

> Add 2 new options to rpc.nfsd -- -4 and -6. -4 makes it an IPv4-only
> server, and -6 makes it IPv6-only. Restructure the -H option so that
> if the address appears to be or resolves to an IPv4 address, that
> IPv6 is disabled. Ditto if it resolves to an IPv6 address.

The new rpc.statd doesn't have a "-4" or "-6" option.  It looks in / 
etc/netconfig to start only the visible transports.  I was suggesting  
that rpc.nfsd use /etc/netconfig _instead_ of having a -4/-6 command- 
line option.

Just a thought.

> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> utils/nfsd/nfsd.c |  151 +++++++++++++++++++++++++++++++++++++++++ 
> +-----------
> 1 files changed, 120 insertions(+), 31 deletions(-)
>
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index ec6c49a..29611d4 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -41,8 +41,6 @@ static struct option longopts[] =
> };
> unsigned int protobits = NFSCTL_ALLBITS;
> unsigned int versbits = NFSCTL_ALLBITS;
> -int minorvers4 = NFSD_MAXMINORVERS4;		/* nfsv4 minor version */
> -char *haddr = NULL;
>
> int
> main(int argc, char **argv)
> @@ -51,7 +49,16 @@ main(int argc, char **argv)
> 	struct servent *ent;
> 	struct hostent *hp;
> 	char *p;
> -	struct sockaddr_in sin;
> +	struct sockaddr_in sin = { };
> +	struct sockaddr_in6 sin6 = { };
> +	int minorvers4 = NFSD_MAXMINORVERS4;	/* nfsv4 minor version */
> +	char	*haddr = NULL;
> +	int	ipv4 = 1;
> +#ifdef IPV6_SUPPORTED
> +	int	ipv6 = 1;
> +#else  /* IPV6_SUPPORTED */
> +	int	ipv6 = 0;
> +#endif /* IPV6_SUPPORTED */
>
> 	ent = getservbyname ("nfs", "udp");
> 	if (ent != NULL)
> @@ -59,18 +66,12 @@ main(int argc, char **argv)
> 	else
> 		port = 2049;
>
> -	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", longopts,  
> NULL)) != EOF) {
> +	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU46", longopts,  
> NULL)) != EOF) {
> 		switch(c) {
> 		case 'H':
> -			if (inet_addr(optarg) != INADDR_NONE) {
> -				haddr = strdup(optarg);
> -			} else if ((hp = gethostbyname(optarg)) != NULL) {
> -				haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
> -			} else {
> -				fprintf(stderr, "%s: Unknown hostname: %s\n",
> -					argv[0], optarg);
> -				usage(argv [0]);
> -			}
> +			/* we only deal with one host addr at the moment */
> +			free(haddr);
> +			haddr = strdup(optarg);
> 			break;
> 		case 'P':	/* XXX for nfs-server compatibility */
> 		case 'p':
> @@ -98,24 +99,85 @@ main(int argc, char **argv)
> 			}
> 			break;
> 		case 'T':
> -				NFSCTL_TCPUNSET(protobits);
> -				break;
> +			NFSCTL_TCPUNSET(protobits);
> +			break;
> 		case 'U':
> -				NFSCTL_UDPUNSET(protobits);
> -				break;
> +			NFSCTL_UDPUNSET(protobits);
> +			break;
> +		case '4':
> +			ipv6 = 0;
> +			break;
> +		case '6':
> +			ipv4 = 0;
> +			break;
> 		default:
> 			fprintf(stderr, "Invalid argument: '%c'\n", c);
> 		case 'h':
> 			usage(argv[0]);
> 		}
> 	}
> -	/*
> -	 * Do some sanity checking, if the ctlbits are set
> -	 */
> +
> +	/* sanity checks */
> +
> +	/* if an address was specified, check it first */
> +	if (haddr) {
> +		/* does it look like an addr of some sort? */
> +		if (inet_pton(AF_INET, haddr, &sin.sin_addr)) {
> +			ipv6 = 0;
> +			goto family_check;
> +		} else if (inet_pton(AF_INET6, haddr, &sin6.sin6_addr)) {
> +			ipv4 = 0;
> +			goto family_check;
> +		}
> +
> +		/* see if it's a hostname */
> +		hp = gethostbyname(haddr);
> +		if (!hp) {
> +			fprintf(stderr, "%s: gethostbyname on %s failed: %d\n",
> +					argv[0], haddr, h_errno);
> +			error = h_errno;
> +			usage(argv[0]);
> +		}
> +
> +		switch (hp->h_addrtype) {
> +		case AF_INET:
> +			ipv6 = 0;
> +			memcpy(&sin.sin_addr, hp->h_addr_list[0],
> +			       hp->h_length);
> +			if (sin.sin_addr.s_addr == INADDR_NONE) {
> +				fprintf(stderr, "%s: Bad hostaddr %s\n",
> +						argv[0], haddr);
> +				usage(argv[0]);
> +			}
> +			break;
> +#ifdef IPV6_SUPPORTED
> +		case AF_INET6:
> +			ipv4 = 0;
> +			memcpy(&sin6.sin6_addr, hp->h_addr_list[0],
> +			       hp->h_length);
> +			break;
> +#endif /* IPV6_SUPPORTED */
> +		default:
> +			fprintf(stderr, "%s: unsupported address family %d\n",
> +					argv[0], hp->h_addrtype);
> +			exit(0);
> +		}
> +	}
> +
> +family_check:
> +	/* make sure at least one address family is enabled */
> +	if (!ipv4 && !ipv6) {
> +		fprintf(stderr, "no address families enabled\n");
> +		exit(1);
> +	}
> +
> +	/* make sure at least one protocol type is enabled */
> 	if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
> 		fprintf(stderr, "invalid protocol specified\n");
> 		exit(1);
> 	}
> +
> +	/* make sure that at least one version is enabled */
> 	found_one = 0;
> 	for (c = NFSD_MINVERS; c <= NFSD_MAXVERS; c++) {
> 		if (NFSCTL_VERISSET(versbits, c))
> @@ -126,14 +188,11 @@ main(int argc, char **argv)
> 		exit(1);
> 	}			
>
> +	/* must have TCP for NFSv4 */
> 	if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
> 		fprintf(stderr, "version 4 requires the TCP protocol\n");
> 		exit(1);
> 	}
> -	if (haddr == NULL) {
> -		struct in_addr in = {INADDR_ANY};
> -		haddr = strdup(inet_ntoa(in));
> -	}
>
> 	if (chdir(NFS_STATEDIR)) {
> 		fprintf(stderr, "%s: chdir(%s) failed: %s\n",
> @@ -148,6 +207,12 @@ main(int argc, char **argv)
> 				"%s: invalid server count (%d), using 1\n",
> 				argv[0], count);
> 			count = 1;
> +		} else if (count == 0) {
> +			/*
> +			 * don't bother setting anything else if the threads
> +			 * are coming down anyway.
> +			 */
> +			goto set_threads;
> 		}
> 	}
> 	/* KLUDGE ALERT:
> @@ -163,28 +228,52 @@ main(int argc, char **argv)
> 		(void) dup2(fd, 2);
> 	}
> 	closeall(3);
> +	openlog("nfsd", LOG_PID, LOG_DAEMON);
>
> -	sin.sin_family = AF_INET;
> -	sin.sin_port = htons(port);
> -	sin.sin_addr.s_addr = inet_addr(haddr);
> +	/*
> +	 * skip everything but setting of number of threads if sockets are
> +	 * already open and in use.
> +	 */
> +	if (nfssvc_inuse())
> +		goto set_threads;
>
> 	/*
> 	 * 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.
> 	 */
> -	if (!nfssvc_inuse()) {
> -		nfssvc_setvers(versbits, minorvers4);
> +	nfssvc_setvers(versbits, minorvers4);
> +
> +	if (ipv4) {
> +		sin.sin_family = AF_INET;
> +		sin.sin_port = htons(port);
> +		if (!haddr)
> +			sin.sin_addr.s_addr = INADDR_ANY;
> +
> 		nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin));
> 	}
>
> -	openlog("nfsd", LOG_PID, LOG_DAEMON);
> +#ifdef IPV6_SUPPORTED
> +	if (ipv6) {
> +		sin6.sin6_family = AF_INET6;
> +		sin6.sin6_port = htons(port);
> +		if (!haddr)
> +			sin6.sin6_addr = in6addr_any;
> +
> +		nfssvc_setfds(protobits, (struct sockaddr *) &sin6,
> +				sizeof(sin6));
> +	}
> +#endif /* IPV6_SUPPORTED */
> +
> +set_threads:
> 	if ((error = nfssvc_threads(port, count)) < 0) {
> 		int e = errno;
> 		syslog(LOG_ERR, "nfssvc: %s", strerror(e));
> 		closelog();
> 	}
>
> +	free(haddr);
> +	closelog();
> 	return (error != 0);
> }
>
> @@ -192,7 +281,7 @@ static void
> usage(const char *prog)
> {
> 	fprintf(stderr, "Usage:\n"
> -		"%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version  
> version ] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> +		"%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version  
> version ] [-T|--no-tcp] [-U|--no-udp] [-4] [-6] nrservs\n",
> 		prog);
> 	exit(2);
> }
> -- 
> 1.6.0.6
>

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

Re: [PATCH 0/6] nfs-utils: add IPv6 support for rpc.nfsd (try #2)

[Chuck Lever]

On May 26, 2009, at 11:15 AM, Jeff Layton wrote:
> This is the second attempt to add support IPv6 to rpc.nfsd. The main
> changes since the last patchset are:
>
> 1) the addition of a patch that makes nfsd look at /etc/netconfig when
> tirpc is enabled, and to disable any family/protocol combinations that
> don't have visible netid's.
>
> 2) stop linking in some unneeded .a libs.
>
> 3) some minor bugfixes
>
> 4) update to the nfsd manpage for new options
>
> I think the set is bisectable, but have only really tested the final
> result. I've also tested the final result when built with tirpc  
> enabled
> and disabled, and ipv6 enabled and disabled.
>
> I haven't seen any regressions when testing on recent mainline kernels
> and The only real difference that anyone may notice is that when IPv6
> support is built in, and the kernel doesn't support IPv6, nfsd logs  
> this
> message a couple of times when starting nfsd:
>
> nfssvc: writing fds to kernel failed: errno 97 (Address family not  
> supported by protocol)

We should also ensure this behaves itself if IPv6 support is built  
into the kernel, but ipv6.ko is blacklisted.

> Most of the testing I've done has been by watching the program under
> strace.  Since most of the kernel work for IPv6 support hasn't made it
> to mainline, it's a little difficult to do much testing of this beyond
> that.
>
> Jeff Layton (6):
>  nfs-utils: don't link libexport.a and libmisc.a to nfsd
>  nfs-utils: break up nfssvc.c into more individually callable
>    functions
>  nfs-utils: set IPV6_V6ONLY on nfssvc IPv6 sockets
>  nfs-utils: add IPv6 support to nfsd
>  nfs-utils: limit protocols and families used by nfsd to those listed
>    in /etc/netconfig
>  nfs-utils: add -4 and -6 options to nfsd manpage
>
> support/include/nfslib.h |    8 ++-
> support/nfs/nfssvc.c     |  164 ++++++++++++++++++++++++++++++++ 
> +----------
> utils/nfsd/Makefile.am   |    4 +-
> utils/nfsd/nfsd.c        |  175 +++++++++++++++++++++++++++++++++++++ 
> +--------
> utils/nfsd/nfsd.man      |   10 +++
> 5 files changed, 292 insertions(+), 69 deletions(-)
>

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

Re: [PATCH 4/6] nfs-utils: add IPv6 support to nfsd

[Jeff Layton]

On Tue, 26 May 2009 11:24:05 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> Hi Jeff-
> 
> On May 26, 2009, at 11:15 AM, Jeff Layton wrote:
> 
> > Add 2 new options to rpc.nfsd -- -4 and -6. -4 makes it an IPv4-only
> > server, and -6 makes it IPv6-only. Restructure the -H option so that
> > if the address appears to be or resolves to an IPv4 address, that
> > IPv6 is disabled. Ditto if it resolves to an IPv6 address.
> 
> The new rpc.statd doesn't have a "-4" or "-6" option.  It looks in / 
> etc/netconfig to start only the visible transports.  I was suggesting  
> that rpc.nfsd use /etc/netconfig _instead_ of having a -4/-6 command- 
> line option.
> 
> Just a thought.
> 

I don't feel strongly about it either way. It seems reasonable that
someone might want to disable ipv4 or ipv6 without changing
/etc/netconfig. This also has some parity with the -T and -U options
too...

It would be good however to have nfsd and statd share options and
behavior here, so I think we should implement the same convention for
both. Do you think it would be better to remove -4/-6 from nfsd, or add
those options to the new statd?

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 0/6] nfs-utils: add IPv6 support for rpc.nfsd (try #2)

[Jeff Layton]

On Tue, 26 May 2009 11:24:57 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> We should also ensure this behaves itself if IPv6 support is built  
> into the kernel, but ipv6.ko is blacklisted.
> 

Good call. I tested that and rpc.nfsd errored out when creating an
IPv6 socket. It should be simple enough to fix. Let me do that and
I'll resend in another day or two.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 4/6] nfs-utils: add IPv6 support to nfsd

[Chuck Lever]

On May 26, 2009, at 12:49 PM, Jeff Layton wrote:

> On Tue, 26 May 2009 11:24:05 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
>
>> Hi Jeff-
>>
>> On May 26, 2009, at 11:15 AM, Jeff Layton wrote:
>>
>>> Add 2 new options to rpc.nfsd -- -4 and -6. -4 makes it an IPv4-only
>>> server, and -6 makes it IPv6-only. Restructure the -H option so that
>>> if the address appears to be or resolves to an IPv4 address, that
>>> IPv6 is disabled. Ditto if it resolves to an IPv6 address.
>>
>> The new rpc.statd doesn't have a "-4" or "-6" option.  It looks in /
>> etc/netconfig to start only the visible transports.  I was suggesting
>> that rpc.nfsd use /etc/netconfig _instead_ of having a -4/-6 command-
>> line option.
>>
>> Just a thought.
>>
>
> I don't feel strongly about it either way. It seems reasonable that
> someone might want to disable ipv4 or ipv6 without changing
> /etc/netconfig. This also has some parity with the -T and -U options
> too...
>
> It would be good however to have nfsd and statd share options and
> behavior here, so I think we should implement the same convention for
> both. Do you think it would be better to remove -4/-6 from nfsd, or  
> add
> those options to the new statd?

My sense is statd is a kind of set-it-and-forget-it kind of thing.  It  
doesn't have a lockd_up() kind of API that, for example, mount.nfs or  
rpc.nfsd can invoke, to ensure that it's listening for a particular  
protocol family or protocol (eventually mount.nfs could ping it via  
IPv6 when mounting an IPv6 server, but it doesn't do that today).  So  
it starts everything to cover all the bases.  That can change if  
someone has a cogent argument!

rpc.nfsd is another kind of beast, as it's starting kernel services,  
not a user space listener.  Even given rpc.nfsd's past of allowing -T  
and -U, it's hard to say whether users will want the flexibility of  
enabling IPv6 or IPv4, or whether they will want whatever families are  
supported with the option of supporting UDP or TCP on both families.   
If there's a way to make it all automatic, that might be the best  
choice.

But I'd hesitate to introduce -4 and -6 if there's a realistic  
possibility that we may not want or need that kind of flexibility in  
the end.  Maybe we can go without for now, and add those options later  
if needed?

Plus, do we want a "don't ever use IPv4" or "IPv6" kind of setting, or  
just a "please start IPv4" or "IPv6" setting?  Does -6 exclude IPv4  
and vice versa?  What should it mean when neither are present?

Once we have everything done, we might look back and say we definitely  
need these options on all the user space components, and at that point  
we'd have a clear idea how to make the options behave consistently for  
all of the pieces.  I guess this is an argument for leaving these  
options off until we have a better sense of what is needed.

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

Re: [PATCH 4/6] nfs-utils: add IPv6 support to nfsd

[Jeff Layton]

On Tue, 26 May 2009 13:31:47 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On May 26, 2009, at 12:49 PM, Jeff Layton wrote:
> 
> > On Tue, 26 May 2009 11:24:05 -0400
> > Chuck Lever <chuck.lever@oracle.com> wrote:
> >
> >> Hi Jeff-
> >>
> >> On May 26, 2009, at 11:15 AM, Jeff Layton wrote:
> >>
> >>> Add 2 new options to rpc.nfsd -- -4 and -6. -4 makes it an IPv4-only
> >>> server, and -6 makes it IPv6-only. Restructure the -H option so that
> >>> if the address appears to be or resolves to an IPv4 address, that
> >>> IPv6 is disabled. Ditto if it resolves to an IPv6 address.
> >>
> >> The new rpc.statd doesn't have a "-4" or "-6" option.  It looks in /
> >> etc/netconfig to start only the visible transports.  I was suggesting
> >> that rpc.nfsd use /etc/netconfig _instead_ of having a -4/-6 command-
> >> line option.
> >>
> >> Just a thought.
> >>
> >
> > I don't feel strongly about it either way. It seems reasonable that
> > someone might want to disable ipv4 or ipv6 without changing
> > /etc/netconfig. This also has some parity with the -T and -U options
> > too...
> >
> > It would be good however to have nfsd and statd share options and
> > behavior here, so I think we should implement the same convention for
> > both. Do you think it would be better to remove -4/-6 from nfsd, or  
> > add
> > those options to the new statd?
> 
> My sense is statd is a kind of set-it-and-forget-it kind of thing.  It  
> doesn't have a lockd_up() kind of API that, for example, mount.nfs or  
> rpc.nfsd can invoke, to ensure that it's listening for a particular  
> protocol family or protocol (eventually mount.nfs could ping it via  
> IPv6 when mounting an IPv6 server, but it doesn't do that today).  So  
> it starts everything to cover all the bases.  That can change if  
> someone has a cogent argument!
> 
> rpc.nfsd is another kind of beast, as it's starting kernel services,  
> not a user space listener.  Even given rpc.nfsd's past of allowing -T  
> and -U, it's hard to say whether users will want the flexibility of  
> enabling IPv6 or IPv4, or whether they will want whatever families are  
> supported with the option of supporting UDP or TCP on both families.   
> If there's a way to make it all automatic, that might be the best  
> choice.
> 
> But I'd hesitate to introduce -4 and -6 if there's a realistic  
> possibility that we may not want or need that kind of flexibility in  
> the end.  Maybe we can go without for now, and add those options later  
> if needed?
> 
> Plus, do we want a "don't ever use IPv4" or "IPv6" kind of setting, or  
> just a "please start IPv4" or "IPv6" setting?  Does -6 exclude IPv4  
> and vice versa?  What should it mean when neither are present?
> 

The current options that I've implemented are "subtractive" -- they
disable the address families specified. If neither are present, then
nfsd just errors out.

> Once we have everything done, we might look back and say we definitely  
> need these options on all the user space components, and at that point  
> we'd have a clear idea how to make the options behave consistently for  
> all of the pieces.  I guess this is an argument for leaving these  
> options off until we have a better sense of what is needed.
> 

That sounds reasonable. I'll go ahead and rip out support for those
options. It's probably better to not introduce new knobs unless there's
a clear need for them. It's something we can easily add later.

Cheers,
-- 
Jeff Layton <jlayton@redhat.com>