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

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

[Jeff Layton]

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

1) removal of the -4 and -6 options. The preferred way to disable
address families for now will be to configure /etc/netconfig. It's
probably better not to add new command line options until there is a
clear need for them. Since I'm not adding new options, I've dropped the
manpage patch too. We may still want a manpage update eventually to
discuss how one should disable address families for nfsd.

2) Fixed the error handling when ipv6.ko is blacklisted. The earlier
patchset would cause the program to error out when it couldn't create a
socket for an address family. Now, as long as the program succeeds in
creating sockets for at least one address family it will declare
success. It will however, log an error that it was unable to create the
socket for the other.

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've also tested this with
a rpc.nfsd built for IPv6 on a host that has ipv6.ko 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 (5):
  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

 support/include/nfslib.h |    8 ++-
 support/nfs/nfssvc.c     |  201 +++++++++++++++++++++++++++++++++++----------
 utils/nfsd/Makefile.am   |    4 +-
 utils/nfsd/nfsd.c        |  180 ++++++++++++++++++++++++++++++++++-------
 4 files changed, 316 insertions(+), 77 deletions(-)

[PATCH 1/5] 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/5] 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     |  107 +++++++++++++++++++++++++++------------------
 utils/nfsd/nfsd.c        |   20 ++++++++-
 3 files changed, 88 insertions(+), 45 deletions(-)

diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index ae98650..4cb1dc0 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);
+int			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..7634562 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -25,78 +25,101 @@
 #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;
+}
+
+int
+nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
+{
+	int fd, on = 1;
+	char buf[BUFSIZ];
+	int udpfd = -1, tcpfd = -1, rc = 0;
+
+	/*
+	 * if file can't be opened, then assume that it's not available and
+	 * that the caller should just fall back to the old nfsctl interface
 	 */
 	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);
+		return 0;
 
 	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);
+			rc = -errno;
+			goto error;
 		}
-		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);
+			rc = -errno;
+			goto error;
 		}
 	}
 
 	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);
+			rc = -errno;
+			goto error;
 		}
 		if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) < 0) {
 			syslog(LOG_ERR, "nfssvc: unable to set SO_REUSEADDR: "
 				"errno %d (%s)\n", errno, strerror(errno));
-			exit(1);
+			rc = -errno;
+			goto error;
 		}
-		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);
+			rc = -errno;
+			goto error;
 		}
 		if (listen(tcpfd, 64) < 0){
 			syslog(LOG_ERR, "nfssvc: unable to create listening socket: "
 				"errno %d (%s)\n", errno, strerror(errno));
-			exit(1);
+			rc = -errno;
+			goto error;
 		}
 	}
 	if (udpfd >= 0) {
 		snprintf(buf, BUFSIZ,"%d\n", udpfd); 
 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
 			syslog(LOG_ERR, 
-			       "nfssvc: writing fds to kernel failed: errno %d (%s)", 
+			       "nfssvc: writing udp fd to kernel failed: errno %d (%s)", 
 			       errno, strerror(errno));
+			rc = -errno;
+			goto error;
 		}
 		close(fd);
 		fd = -1;
@@ -107,16 +130,21 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
 		snprintf(buf, BUFSIZ,"%d\n", tcpfd); 
 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
 			syslog(LOG_ERR, 
-			       "nfssvc: writing fds to kernel failed: errno %d (%s)", 
+			       "nfssvc: writing tcp fd to kernel failed: errno %d (%s)", 
 			       errno, strerror(errno));
+			rc = -errno;
 		}
 	}
-	close(fd);
 
-	return;
+error:
+	if (fd >= 0)
+		close(fd);
+
+	return rc;
 }
-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 +175,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..2a3cd0a 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,13 +164,30 @@ 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);
+		error = nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin));
+		if (error)
+			goto out;
+	}
+
 	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();
 	}
 
+out:
 	return (error != 0);
 }
 
-- 
1.6.0.6

[PATCH 3/5] 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 |   31 ++++++++++++++++++++++++++-----
 1 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 7634562..2aa5281 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -60,6 +60,7 @@ nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 	int fd, on = 1;
 	char buf[BUFSIZ];
 	int udpfd = -1, tcpfd = -1, rc = 0;
+	unsigned short family = sa->sa_family;
 
 	/*
 	 * if file can't be opened, then assume that it's not available and
@@ -70,9 +71,19 @@ nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 		return 0;
 
 	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: "
+			syslog(LOG_ERR, "nfssvc: unable to create %s UDP "
+				"socket: errno %d (%s)\n",
+				family == AF_INET6 ? "ipv6" : "ipv4", errno,
+				strerror(errno));
+			rc = -errno;
+			goto error;
+		}
+		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));
 			rc = -errno;
 			goto error;
@@ -86,10 +97,12 @@ 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));
+			syslog(LOG_ERR, "nfssvc: unable to create %s TCP "
+				"socket: errno %d (%s)\n",
+				family == AF_INET6 ? "ipv6" : "ipv4", errno,
+				strerror(errno));
 			rc = -errno;
 			goto error;
 		}
@@ -99,6 +112,14 @@ nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 			rc = -errno;
 			goto error;
 		}
+		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/5] nfs-utils: add IPv6 support to nfsd

[Jeff Layton]

Add support for handing off IPv6 sockets to the kernel for nfsd. One of
the main goals here is to not change the behavior of options and not to
add any new ones, so this patch attempts to do that.

We also don't want to break anything in the event that someone has an
rpc.nfsd program built with IPv6 capability, but the knfsd doesn't
support IPv6. Ditto for the cases where IPv6 is either not compiled in
or is compiled in and blacklisted.

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

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 2a3cd0a..77c7e1b 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -41,17 +41,24 @@ 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)
 {
-	int	count = 1, c, error, port, fd, found_one;
+	int	count = 1, c, error = 0, port, fd, found_one;
 	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)
@@ -62,15 +69,9 @@ main(int argc, char **argv)
 	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", 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,79 @@ 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;
 		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 +182,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 +201,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,24 +222,55 @@ 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);
-		error = nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin));
-		if (error)
-			goto out;
+	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;
+
+		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin)))
+			ipv4 = 0;
 	}
 
-	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;
+
+		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6, sizeof(sin6)))
+			ipv6 = 0;
+	}
+#endif /* IPV6_SUPPORTED */
+
+	/*
+	 * if ipv4 and ipv6 are both 0 here, then we were unable to pass off sockets
+	 * to the kernel. Don't try to bring up any threads.
+	 */
+	if (!ipv4 && !ipv6 && count > 0) {
+		syslog(LOG_ERR, "nfssvc: failed to hand off sockets to kernel\n");
+		error = -1;
+		goto out;
+	}
+
+set_threads:
 	if ((error = nfssvc_threads(port, count)) < 0) {
 		int e = errno;
 		syslog(LOG_ERR, "nfssvc: %s", strerror(e));
@@ -188,6 +278,8 @@ main(int argc, char **argv)
 	}
 
 out:
+	free(haddr);
+	closelog();
 	return (error != 0);
 }
 
-- 
1.6.0.6

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

[Chuck Lever]

On May 27, 2009, at 7:54 AM, Jeff Layton wrote:

> Add support for handing off IPv6 sockets to the kernel for nfsd. One  
> of
> the main goals here is to not change the behavior of options and not  
> to
> add any new ones, so this patch attempts to do that.
>
> We also don't want to break anything in the event that someone has an
> rpc.nfsd program built with IPv6 capability, but the knfsd doesn't
> support IPv6. Ditto for the cases where IPv6 is either not compiled in
> or is compiled in and blacklisted.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> utils/nfsd/nfsd.c |  158 +++++++++++++++++++++++++++++++++++++++++ 
> +-----------
> 1 files changed, 125 insertions(+), 33 deletions(-)
>
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 2a3cd0a..77c7e1b 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -41,17 +41,24 @@ 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)
> {
> -	int	count = 1, c, error, port, fd, found_one;
> +	int	count = 1, c, error = 0, port, fd, found_one;
> 	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)
> @@ -62,15 +69,9 @@ main(int argc, char **argv)
> 	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", 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,79 @@ 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;

Clean up: separate patch?

> 		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;
> +		}

Using getaddrinfo(3) with AI_NUMERICHOST might allow a lot of  
simplification here.  See utils/new-statd/hostname.c in my repo for  
some possibilities.

You could also easily pass the text version of the -p option into  
getaddrinfo() to have it plant the port number into the bind address.

This logic is complex enough that you may want to extract it into a  
helper anyway.

> +
> +		/* 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);
> +	}

Is this needed if you don't have -4 and -6?  I'm still familiarizing  
myself with these patches, but I have a vague feeling that this  
checking could be made much simpler, especially if the netconfig- 
related logic was introduced at the same time rather than in a  
separate patch.

> +
> +	/* 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 +182,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 +201,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,24 +222,55 @@ main(int argc, char **argv)
> 		(void) dup2(fd, 2);
> 	}
> 	closeall(3);
> +	openlog("nfsd", LOG_PID, LOG_DAEMON);

Might be nice if rpc.nfsd used xlog() instead.  Maybe for another day.

> -	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);
> -		error = nfssvc_setfds(protobits, (struct sockaddr *) &sin,  
> sizeof(sin));
> -		if (error)
> -			goto out;
> +	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;
> +
> +		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin,  
> sizeof(sin)))
> +			ipv4 = 0;
> 	}
>
> -	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;
> +
> +		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6,  
> sizeof(sin6)))
> +			ipv6 = 0;
> +	}
> +#endif /* IPV6_SUPPORTED */
> +
> +	/*
> +	 * if ipv4 and ipv6 are both 0 here, then we were unable to pass  
> off sockets
> +	 * to the kernel. Don't try to bring up any threads.
> +	 */
> +	if (!ipv4 && !ipv6 && count > 0) {
> +		syslog(LOG_ERR, "nfssvc: failed to hand off sockets to kernel\n");
> +		error = -1;
> +		goto out;
> +	}
> +
> +set_threads:
> 	if ((error = nfssvc_threads(port, count)) < 0) {
> 		int e = errno;
> 		syslog(LOG_ERR, "nfssvc: %s", strerror(e));
> @@ -188,6 +278,8 @@ main(int argc, char **argv)
> 	}
>
> out:
> +	free(haddr);
> +	closelog();
> 	return (error != 0);
> }
>
> -- 
> 1.6.0.6
>

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

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

[Jeff Layton]

On Wed, 27 May 2009 12:22:57 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On May 27, 2009, at 7:54 AM, Jeff Layton wrote:
> 
> > Add support for handing off IPv6 sockets to the kernel for nfsd. One  
> > of
> > the main goals here is to not change the behavior of options and not  
> > to
> > add any new ones, so this patch attempts to do that.
> >
> > We also don't want to break anything in the event that someone has an
> > rpc.nfsd program built with IPv6 capability, but the knfsd doesn't
> > support IPv6. Ditto for the cases where IPv6 is either not compiled in
> > or is compiled in and blacklisted.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > utils/nfsd/nfsd.c |  158 +++++++++++++++++++++++++++++++++++++++++ 
> > +-----------
> > 1 files changed, 125 insertions(+), 33 deletions(-)
> >
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index 2a3cd0a..77c7e1b 100644
> > --- a/utils/nfsd/nfsd.c
> > +++ b/utils/nfsd/nfsd.c
> > @@ -41,17 +41,24 @@ 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)
> > {
> > -	int	count = 1, c, error, port, fd, found_one;
> > +	int	count = 1, c, error = 0, port, fd, found_one;
> > 	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)
> > @@ -62,15 +69,9 @@ main(int argc, char **argv)
> > 	while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", 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,79 @@ 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;
> 
> Clean up: separate patch?
> 

Yep. Leftover from when I had the -4/-6 options in there. I'll take
that out on the next respin (or do a cleanup patch).

> > 		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;
> > +		}
> 
> Using getaddrinfo(3) with AI_NUMERICHOST might allow a lot of  
> simplification here.  See utils/new-statd/hostname.c in my repo for  
> some possibilities.
> 
> You could also easily pass the text version of the -p option into  
> getaddrinfo() to have it plant the port number into the bind address.
> 
> This logic is complex enough that you may want to extract it into a  
> helper anyway.
> 

Ok, I'll look at that. I don't think getaddrinfo really buys us much
here though so I may keep the logic like this. You are right though
that this should be in a helper function. I'll plan to move it into one
for the next iteration.

> > +
> > +		/* 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);
> > +	}
> 
> Is this needed if you don't have -4 and -6?  I'm still familiarizing  
> myself with these patches, but I have a vague feeling that this  
> checking could be made much simpler, especially if the netconfig- 
> related logic was introduced at the same time rather than in a  
> separate patch.
> 

I could probably eliminate that particular check. I have a similar one
lower in the code already that should cover that case. The problem
there of course is that at some point nfsd closes its stdout/stderr,
and the only reporting mechanism we have after that is return code and
syslog messages. If I eliminate that check then errors on startup
aren't as evident.

I'd like to better understand the problem that leads to the "KLUDGE
ALERT" in the code. Maybe we can eliminate syslog-based error reporting
from nfsd for modern kernels? If so, what criteria should we use to
determine that?

On the other issue, the existing code is "subtractive". It enables
everything and then disables protocols based on command line options. I
decided to follow the same scheme to keep the changes to a minimum, but
maybe it's better to just overhaul it completely. Moving this code to a
new regime while preserving the existing behavior is a little tricky,
though...

> > +
> > +	/* 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 +182,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 +201,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,24 +222,55 @@ main(int argc, char **argv)
> > 		(void) dup2(fd, 2);
> > 	}
> > 	closeall(3);
> > +	openlog("nfsd", LOG_PID, LOG_DAEMON);
> 
> Might be nice if rpc.nfsd used xlog() instead.  Maybe for another day.
> 

That's probably doable, but I'd much prefer that we just make nfsd
report errors to stderr when run on modern kernels. Not sure whether we
can reasonably do that however.

> > -	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);
> > -		error = nfssvc_setfds(protobits, (struct sockaddr *) &sin,  
> > sizeof(sin));
> > -		if (error)
> > -			goto out;
> > +	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;
> > +
> > +		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin,  
> > sizeof(sin)))
> > +			ipv4 = 0;
> > 	}
> >
> > -	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;
> > +
> > +		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6,  
> > sizeof(sin6)))
> > +			ipv6 = 0;
> > +	}
> > +#endif /* IPV6_SUPPORTED */
> > +
> > +	/*
> > +	 * if ipv4 and ipv6 are both 0 here, then we were unable to pass  
> > off sockets
> > +	 * to the kernel. Don't try to bring up any threads.
> > +	 */
> > +	if (!ipv4 && !ipv6 && count > 0) {
> > +		syslog(LOG_ERR, "nfssvc: failed to hand off sockets to kernel\n");
> > +		error = -1;
> > +		goto out;
> > +	}
> > +
> > +set_threads:
> > 	if ((error = nfssvc_threads(port, count)) < 0) {
> > 		int e = errno;
> > 		syslog(LOG_ERR, "nfssvc: %s", strerror(e));
> > @@ -188,6 +278,8 @@ main(int argc, char **argv)
> > 	}
> >
> > out:
> > +	free(haddr);
> > +	closelog();
> > 	return (error != 0);
> > }
> >
> > -- 
> > 1.6.0.6
> >
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com


-- 
Jeff Layton <jlayton@redhat.com>

[PATCH 5/5] 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        |   32 +++++++++++++++------
 3 files changed, 94 insertions(+), 9 deletions(-)

diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index 4cb1dc0..bae0902 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -133,6 +133,8 @@ int			nfsctl(int, struct nfsctl_arg *, union nfsctl_res *);
 int			nfssvc_inuse(void);
 int			nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa,
 					socklen_t addrlen);
+unsigned int		nfssvc_set_family_proto(const sa_family_t family,
+						unsigned int ctlbits);
 void			nfssvc_setvers(unsigned int ctlbits, int minorvers4);
 int			nfssvc_threads(unsigned short port, int nrservs);
 int			nfsaddclient(struct nfsctl_client *clp);
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 2aa5281..a91c892 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;
+}
+
 int
 nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t addrlen)
 {
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 77c7e1b..45bede9 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 */
@@ -159,15 +161,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);
 	}
 
@@ -183,7 +195,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);
 	}
@@ -244,7 +258,7 @@ family_check:
 		if (!haddr)
 			sin.sin_addr.s_addr = INADDR_ANY;
 
-		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin, sizeof(sin)))
+		if (nfssvc_setfds(proto4, (struct sockaddr *) &sin, sizeof(sin)))
 			ipv4 = 0;
 	}
 
@@ -255,7 +269,7 @@ family_check:
 		if (!haddr)
 			sin6.sin6_addr = in6addr_any;
 
-		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6, sizeof(sin6)))
+		if (nfssvc_setfds(proto6, (struct sockaddr *) &sin6, sizeof(sin6)))
 			ipv6 = 0;
 	}
 #endif /* IPV6_SUPPORTED */
-- 
1.6.0.6

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

[Chuck Lever]

On May 27, 2009, at 7:54 AM, Jeff Layton wrote:

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

I kind of favor the converse approach -- enable the ones that _are_  
marked visible -- which appears to be the more common usage of  
netconfig.

> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> support/include/nfslib.h |    2 +
> support/nfs/nfssvc.c     |   69 +++++++++++++++++++++++++++++++++++++ 
> +++++++++
> utils/nfsd/nfsd.c        |   32 +++++++++++++++------
> 3 files changed, 94 insertions(+), 9 deletions(-)
>
> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> index 4cb1dc0..bae0902 100644
> --- a/support/include/nfslib.h
> +++ b/support/include/nfslib.h
> @@ -133,6 +133,8 @@ int			nfsctl(int, struct nfsctl_arg *, union  
> nfsctl_res *);
> int			nfssvc_inuse(void);
> int			nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa,
> 					socklen_t addrlen);
> +unsigned int		nfssvc_set_family_proto(const sa_family_t family,
> +						unsigned int ctlbits);
> void			nfssvc_setvers(unsigned int ctlbits, int minorvers4);
> int			nfssvc_threads(unsigned short port, int nrservs);
> int			nfsaddclient(struct nfsctl_client *clp);
> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> index 2aa5281..a91c892 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;

Oddly I don't see any usage in libtirpc of getprotobynumber(3).  It  
seems to stick with defined constants such as NC_TCP.  I suspect there  
is really no guaranteed relationship between getprotobynumber and  
getnetconfig.

I guess this is exactly the friction point between the world of TI-RPC  
and netids and the kernel's world which doesn't have that concept.

> +
> +	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;
> +}
> +
> int
> nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t  
> addrlen)
> {
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 77c7e1b..45bede9 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 */
> @@ -159,15 +161,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);
> 	}
>
> @@ -183,7 +195,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);
> 	}
> @@ -244,7 +258,7 @@ family_check:
> 		if (!haddr)
> 			sin.sin_addr.s_addr = INADDR_ANY;
>
> -		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin,  
> sizeof(sin)))
> +		if (nfssvc_setfds(proto4, (struct sockaddr *) &sin, sizeof(sin)))
> 			ipv4 = 0;
> 	}
>
> @@ -255,7 +269,7 @@ family_check:
> 		if (!haddr)
> 			sin6.sin6_addr = in6addr_any;
>
> -		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6,  
> sizeof(sin6)))
> +		if (nfssvc_setfds(proto6, (struct sockaddr *) &sin6, sizeof(sin6)))
> 			ipv6 = 0;
> 	}
> #endif /* IPV6_SUPPORTED */
> -- 
> 1.6.0.6
>

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

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

[Jeff Layton]

On Wed, 27 May 2009 12:30:21 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On May 27, 2009, at 7:54 AM, Jeff Layton wrote:
> 
> > ...disable any that aren't listed or aren't marked as "visible".
> 
> I kind of favor the converse approach -- enable the ones that _are_  
> marked visible -- which appears to be the more common usage of  
> netconfig.
> 

Fair enough. That may be cleaner. I'll probably change it to do that.

> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > support/include/nfslib.h |    2 +
> > support/nfs/nfssvc.c     |   69 +++++++++++++++++++++++++++++++++++++ 
> > +++++++++
> > utils/nfsd/nfsd.c        |   32 +++++++++++++++------
> > 3 files changed, 94 insertions(+), 9 deletions(-)
> >
> > diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> > index 4cb1dc0..bae0902 100644
> > --- a/support/include/nfslib.h
> > +++ b/support/include/nfslib.h
> > @@ -133,6 +133,8 @@ int			nfsctl(int, struct nfsctl_arg *, union  
> > nfsctl_res *);
> > int			nfssvc_inuse(void);
> > int			nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa,
> > 					socklen_t addrlen);
> > +unsigned int		nfssvc_set_family_proto(const sa_family_t family,
> > +						unsigned int ctlbits);
> > void			nfssvc_setvers(unsigned int ctlbits, int minorvers4);
> > int			nfssvc_threads(unsigned short port, int nrservs);
> > int			nfsaddclient(struct nfsctl_client *clp);
> > diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> > index 2aa5281..a91c892 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;
> 
> Oddly I don't see any usage in libtirpc of getprotobynumber(3).  It  
> seems to stick with defined constants such as NC_TCP.  I suspect there  
> is really no guaranteed relationship between getprotobynumber and  
> getnetconfig.
> 
> I guess this is exactly the friction point between the world of TI-RPC  
> and netids and the kernel's world which doesn't have that concept.
> 

Hmm good point...I'll plan to switch the next iteration of this set not
to use getprotobynumber.

FWIW, you may want to have a look at nfs_gp_get_netid since it does the
same thing.

> > +
> > +	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;
> > +}
> > +
> > int
> > nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t  
> > addrlen)
> > {
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index 77c7e1b..45bede9 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 */
> > @@ -159,15 +161,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);
> > 	}
> >
> > @@ -183,7 +195,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);
> > 	}
> > @@ -244,7 +258,7 @@ family_check:
> > 		if (!haddr)
> > 			sin.sin_addr.s_addr = INADDR_ANY;
> >
> > -		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin,  
> > sizeof(sin)))
> > +		if (nfssvc_setfds(proto4, (struct sockaddr *) &sin, sizeof(sin)))
> > 			ipv4 = 0;
> > 	}
> >
> > @@ -255,7 +269,7 @@ family_check:
> > 		if (!haddr)
> > 			sin6.sin6_addr = in6addr_any;
> >
> > -		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6,  
> > sizeof(sin6)))
> > +		if (nfssvc_setfds(proto6, (struct sockaddr *) &sin6, sizeof(sin6)))
> > 			ipv6 = 0;
> > 	}
> > #endif /* IPV6_SUPPORTED */
> > -- 
> > 1.6.0.6
> >
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com


-- 
Jeff Layton <jlayton@redhat.com>

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

[Chuck Lever]

On May 29, 2009, at 7:20 AM, Jeff Layton wrote:

> On Wed, 27 May 2009 12:30:21 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
>
>>
>> On May 27, 2009, at 7:54 AM, Jeff Layton wrote:
>>
>>> ...disable any that aren't listed or aren't marked as "visible".
>>
>> I kind of favor the converse approach -- enable the ones that _are_
>> marked visible -- which appears to be the more common usage of
>> netconfig.
>>
>
> Fair enough. That may be cleaner. I'll probably change it to do that.
>
>>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>>> ---
>>> support/include/nfslib.h |    2 +
>>> support/nfs/nfssvc.c     |   69 +++++++++++++++++++++++++++++++++++ 
>>> ++
>>> +++++++++
>>> utils/nfsd/nfsd.c        |   32 +++++++++++++++------
>>> 3 files changed, 94 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/support/include/nfslib.h b/support/include/nfslib.h
>>> index 4cb1dc0..bae0902 100644
>>> --- a/support/include/nfslib.h
>>> +++ b/support/include/nfslib.h
>>> @@ -133,6 +133,8 @@ int			nfsctl(int, struct nfsctl_arg *, union
>>> nfsctl_res *);
>>> int			nfssvc_inuse(void);
>>> int			nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa,
>>> 					socklen_t addrlen);
>>> +unsigned int		nfssvc_set_family_proto(const sa_family_t family,
>>> +						unsigned int ctlbits);
>>> void			nfssvc_setvers(unsigned int ctlbits, int minorvers4);
>>> int			nfssvc_threads(unsigned short port, int nrservs);
>>> int			nfsaddclient(struct nfsctl_client *clp);
>>> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
>>> index 2aa5281..a91c892 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;
>>
>> Oddly I don't see any usage in libtirpc of getprotobynumber(3).  It
>> seems to stick with defined constants such as NC_TCP.  I suspect  
>> there
>> is really no guaranteed relationship between getprotobynumber and
>> getnetconfig.
>>
>> I guess this is exactly the friction point between the world of TI- 
>> RPC
>> and netids and the kernel's world which doesn't have that concept.
>>
>
> Hmm good point...I'll plan to switch the next iteration of this set  
> not
> to use getprotobynumber.
>
> FWIW, you may want to have a look at nfs_gp_get_netid since it does  
> the
> same thing.

All of that code will get a fair scrubbing after we finish the first  
pass of IPv6 support in nfs-utils.

>>> +
>>> +	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;
>>> +}
>>> +
>>> int
>>> nfssvc_setfds(unsigned int ctlbits, struct sockaddr *sa, socklen_t
>>> addrlen)
>>> {
>>> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
>>> index 77c7e1b..45bede9 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 */
>>> @@ -159,15 +161,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);
>>> 	}
>>>
>>> @@ -183,7 +195,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);
>>> 	}
>>> @@ -244,7 +258,7 @@ family_check:
>>> 		if (!haddr)
>>> 			sin.sin_addr.s_addr = INADDR_ANY;
>>>
>>> -		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin,
>>> sizeof(sin)))
>>> +		if (nfssvc_setfds(proto4, (struct sockaddr *) &sin, sizeof(sin)))
>>> 			ipv4 = 0;
>>> 	}
>>>
>>> @@ -255,7 +269,7 @@ family_check:
>>> 		if (!haddr)
>>> 			sin6.sin6_addr = in6addr_any;
>>>
>>> -		if (nfssvc_setfds(protobits, (struct sockaddr *) &sin6,
>>> sizeof(sin6)))
>>> +		if (nfssvc_setfds(proto6, (struct sockaddr *) &sin6,  
>>> sizeof(sin6)))
>>> 			ipv6 = 0;
>>> 	}
>>> #endif /* IPV6_SUPPORTED */
>>> -- 
>>> 1.6.0.6
>>>
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>
>
> -- 
> Jeff Layton <jlayton@redhat.com>
> --
> 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

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