Subject: Re: bin/10986: mount_nfs doesn't fallback to IPv4
To: Andrew Brown <atatat@atatdot.net>
From: Benedikt Meurer <bmeurer@fwdn.de>
List: netbsd-bugs
Date: 10/02/2002 19:04:12
On Tue, 01 Oct 2002, Andrew Brown wrote:

> >> > I prefer the "inet" names.
> >> 
> >> The correct solution is to use a "proto=" option, like Solaris has.
> >> Last time I looked into that, there was something that getmntopt(3)
> >> didn't like about that, but I don't quite remember.
> >> 
> >> Please don't implement anything else.
> >
> >Ok, I don't have access to any solaris system for now, but somebody
> >sent me the mount_nfs(1M) man page :-). We would then use proto=
> >{tcp,tcp6,udp,udp6} options and use getnetconfigent("...") to get
> >the struct netconfig, right? That would also obsolete ALTF_TCP, since
> >we could specify proto={tcp,tcp6}, or do I missed something about that?

Ok. I changed mount_nfs to support proto=<netid> option as mentioned in
the mount_nfs(1M) Solaris man page. In addition, I added mntproto=<netid>
to explicitly set the mount protocol to use (if differs from nfs
proto), that will obsolete -U and mntudp, since we now have a more generic
way to specify the mount protocol. I also added a vers=<nfs version number>
option as mentioned in the solaris mount_nfs manual to allow a smooth
integration of NFSv4 (NFSv5, ...), so we do not need to add -<NFS version> and
nfsv<NFS version> options, whenever new NFS protocol versions come up. This
will obsolete -2/-3 and nfsv2/nfsv3 (it would be a pain to handle int force2,
force3, force4, .. and so on). So e.g. if you used something like
	mount_nfs -3 -T -U host:/dirpath /localdir
before, you would now say
	mount_nfs -o vers=3,proto=tcp,mntproto=udp host:/dirpath /localdir
(or even udp6/tcp6). So it'll be easy to add support for new NFS version
and even support for new protocols, I think. We could also get rid of the
sluggish ISO code with this approach.
Maybe we should also replace things like -w with wsize= and -t with timeo=,
or are there any objections to the use of Solaris-like options?

It would also be nice, if people could test my patch. I've tested it with
IPv4 and IPv6 nfsd/mountd, and it seems to work fine.

HTH, Benedikt

Index: mount_nfs.c
===================================================================
RCS file: /cvsroot/basesrc/sbin/mount_nfs/mount_nfs.c,v
retrieving revision 1.35
diff -u -r1.35 mount_nfs.c
--- mount_nfs.c	2002/10/01 03:08:56	1.35
+++ mount_nfs.c	2002/10/02 16:40:25
@@ -98,15 +98,18 @@
 #define ALTF_DUMBTIMR	0x4
 #define ALTF_INTR	0x8
 #define ALTF_KERB	0x10
-#define ALTF_NFSV3	0x20
+#define ALTF_NFSV3	0x20		/* obsoleted by vers=3 */
 #define ALTF_RDIRPLUS	0x40
-#define	ALTF_MNTUDP	0x80
+#define ALTF_MNTUDP	0x80		/* obsoleted by mntproto=udp */
 #define ALTF_NORESPORT	0x100
 #define ALTF_SEQPACKET	0x200
 #define ALTF_NQNFS	0x400
 #define ALTF_SOFT	0x800
-#define ALTF_TCP	0x1000
-#define ALTF_NFSV2	0x2000
+#define ALTF_TCP	0x1000		/* obsoleted by proto=tcp */
+#define ALTF_NFSV2	0x2000		/* obsoleted by vers=2 */
+#define ALTF_PROTO	0x4000
+#define ALTF_VERS	0x8000
+#define ALTF_MNTPROTO	0x10000
 
 static const struct mntopt mopts[] = {
 	MOPT_STDOPTS,
@@ -131,6 +134,9 @@
 	{ "soft", 0, ALTF_SOFT, 1 },
 	{ "tcp", 0, ALTF_TCP, 1 },
 	{ "nfsv2", 0, ALTF_NFSV2, 1 },
+	{ "proto", 0, ALTF_PROTO, 1 },
+	{ "vers", 0, ALTF_VERS, 1 },
+	{ "mntproto", 0, ALTF_MNTPROTO, 1 },
 	{ NULL }
 };
 
@@ -167,10 +173,9 @@
 #define	ISBGRND	2
 int retrycnt;
 int opflags = 0;
-int nfsproto = IPPROTO_UDP;
-int force2 = 0;
-int force3 = 0;
-int mnttcp_ok = 1;
+int forceversion = 0;	/* this is better than having force2,force3,force4..*/
+int forcefamily = 0;
+char mntproto[7] = { 0, };
 
 #ifdef NFSKERB
 static char inst[INST_SZ];
@@ -198,6 +203,7 @@
 static void	usage __P((void));
 static int	xdr_dir __P((XDR *, char *));
 static int	xdr_fh __P((XDR *, struct nfhret *));
+static char *	getoptval __P((const char *, const char *, char *, size_t));
 
 #ifndef MOUNT_NOMAIN
 int
@@ -219,6 +225,7 @@
 	struct nfs_args nfsargs;
 	struct nfsd_cargs ncd;
 	int mntflags, altflags, i, nfssvc_flag, num;
+	char buffer[32];
 	char *name, *p, *spec, *ospec;
 #ifdef NFSKERB
 	uid_t last_ruid;
@@ -242,14 +249,16 @@
 	    "23a:bcCdD:g:I:iKL:lm:o:PpqR:r:sTt:w:x:UX")) != -1)
 		switch (c) {
 		case '3':
-			if (force2)
-				errx(1, "-2 and -3 are mutually exclusive");
-			force3 = 1;
+			warnx("-3 is obsolete, use vers=3 instead");
+			if (forceversion && forceversion != 3)
+				errx(1, "conflicting version options");
+			forceversion = 3;
 			break;
 		case '2':
-			if (force3)
-				errx(1, "-2 and -3 are mutually exclusive");
-			force2 = 1;
+			warnx("-2 is obsolete, use vers=2 instead");
+			if (forceversion && forceversion != 2)
+				errx(1, "conflicting version options");
+			forceversion = 2;
 			nfsargsp->flags &= ~NFSMNT_NFSV3;
 			break;
 		case 'a':
@@ -334,20 +343,27 @@
 				nfsargsp->flags |= NFSMNT_KERB;
 #endif
 			if (altflags & ALTF_NFSV3) {
-				if (force2)
+				warnx("nfsv3 is obsolete, use vers=3 instead");
+				if (forceversion && forceversion != 3)
 					errx(1, "conflicting version options");
-				force3 = 1;
+				forceversion = 3;
 			}
 			if (altflags & ALTF_NFSV2) {
-				if (force3)
+				warnx("nfsv2 is obsolete, use vers=2 instead");
+				if (forceversion && forceversion != 2)
 					errx(1, "conflicting version options");
-				force2 = 1;
+				forceversion = 2;
 				nfsargsp->flags &= ~NFSMNT_NFSV3;
 			}
 			if (altflags & ALTF_RDIRPLUS)
 				nfsargsp->flags |= NFSMNT_RDIRPLUS;
-			if (altflags & ALTF_MNTUDP)
-				mnttcp_ok = 0;
+			if (altflags & ALTF_MNTUDP) {
+				warnx("mntudp is obsolete, use mntproto=udp "
+					"instead");
+				if (*mntproto && strcmp(mntproto, "udp"))
+					errx(1, "conflicting mntproto options");
+				strlcpy(mntproto,"udp",sizeof(mntproto));
+			}
 			if (altflags & ALTF_NORESPORT)
 				nfsargsp->flags &= ~NFSMNT_RESVPORT;
 #ifdef ISO
@@ -355,16 +371,98 @@
 				nfsargsp->sotype = SOCK_SEQPACKET;
 #endif
 			if (altflags & ALTF_NQNFS) {
-				if (force2)
+				if (forceversion && forceversion != 3)
 					errx(1, "nqnfs only available with v3");
-				force3 = 1;
+				forceversion = 3;
 				nfsargsp->flags |= NFSMNT_NQNFS;
 			}
 			if (altflags & ALTF_SOFT)
 				nfsargsp->flags |= NFSMNT_SOFT;
 			if (altflags & ALTF_TCP) {
+				warnx("tcp is obsolete, use proto=tcp instead");
 				nfsargsp->sotype = SOCK_STREAM;
-				nfsproto = IPPROTO_TCP;
+			}
+			/* proto=<netid> should be the suggested way of
+			 * selecting a specific mount/nfs protocol, since its
+			 * a more generic way, which allows easy addition
+			 * of new protocols, without having to add any
+			 * further arguments/options to mount_nfs. ALTF_TCP
+			 * should therefore be considered obsolete (maybe
+			 * ALTF_MNTUDP too, but i'm not sure).
+			 * - Benedikt Meurer
+			 */
+			if (altflags & ALTF_PROTO) {
+				struct netconfig *nc;
+
+				if (getoptval(optarg, "proto", buffer,
+				    sizeof(buffer)) == NULL)
+					errx(1, "invalid proto option");
+
+				if ((nc = getnetconfigent (buffer)) == NULL)
+					errx(1, "%s: %s", buffer, nc_sperror());
+
+				/* XXX Is there any generic way? */ 
+				if (!strcmp(nc->nc_protofmly, NC_INET))
+					forcefamily = PF_INET;
+				else if (!strcmp(nc->nc_protofmly, NC_INET6))
+					forcefamily = PF_INET6;
+				else
+					errx(1, "%s: protocol family not "
+						"supported", nc->nc_protofmly);
+
+				/* XXX Is there any generic way? */
+				if (!strcmp(nc->nc_proto, NC_TCP))
+					nfsargsp->sotype = SOCK_STREAM;
+				else if (!strcmp(nc->nc_proto, NC_UDP))
+					nfsargsp->sotype = SOCK_DGRAM;
+				else
+					errx(1, "%s: unsupported protocol",
+						nc->nc_proto);
+
+				freenetconfigent(nc);
+			}
+			/* vers=<NFS version number> should be the suggested
+			 * way of specifing NFS protocol version to use, else
+			 * we'll probably end up with nfsv2,nfsv3,nfsv4,...
+			 * options. I think the Solaris like vers option is
+			 * easier to handle.
+			 * - Benedikt Meurer
+			 */
+			if (altflags & ALTF_VERS) {
+				if (getoptval(optarg, "vers", buffer,
+				    sizeof(buffer)) == NULL)
+					errx(1, "invalid vers option");
+
+				num = strtol(buffer, &p, 10);
+				if (*p || num <= 0)
+					errx(1, "illegal version -- %s",buffer);
+
+				warnx("vers: %d, forceversion: %d", num, forceversion);
+				if (forceversion && forceversion != num)
+					errx(1,"conflicting version options");
+
+				switch (num) {
+				case 3:
+					forceversion = 3;
+					nfsargsp->flags |= NFSMNT_NFSV3;
+					break;
+				case 2:
+					forceversion = 2;
+					nfsargsp->flags &= ~NFSMNT_NFSV3;
+					break;
+				default:
+					errx(1,"unsupported version -- %s",
+						buffer);
+				}
+			}
+			if (altflags & ALTF_MNTPROTO) {
+				if (getoptval(optarg,"mntproto",buffer,
+				    sizeof(buffer)) == NULL) {
+					errx(1,"invalid mntproto option");
+				}
+				if (*mntproto && strcmp(mntproto, buffer))
+					errx(1, "conflicting mntproto options");
+				strlcpy(mntproto, buffer, sizeof(mntproto));
 			}
 			altflags = 0;
 			break;
@@ -375,9 +473,9 @@
 			nfsargsp->flags &= ~NFSMNT_RESVPORT;
 			break;
 		case 'q':
-			if (force2)
+			if (forceversion && forceversion != 3)
 				errx(1, "nqnfs only available with v3");
-			force3 = 1;
+			forceversion = 3;
 			nfsargsp->flags |= NFSMNT_NQNFS;
 			break;
 		case 'R':
@@ -403,7 +501,6 @@
 			break;
 		case 'T':
 			nfsargsp->sotype = SOCK_STREAM;
-			nfsproto = IPPROTO_TCP;
 			break;
 		case 't':
 			num = strtol(optarg, &p, 10);
@@ -430,7 +527,9 @@
 			nfsargsp->flags |= NFSMNT_XLATECOOKIE;
 			break;
 		case 'U':
-			mnttcp_ok = 0;
+			if (*mntproto && strcmp(mntproto, "udp"))
+				errx(1, "conflicting mntproto options");
+			strlcpy(mntproto, "udp", sizeof(mntproto));
 			break;
 		default:
 			usage();
@@ -455,7 +554,7 @@
 	if ((retval = mount(MOUNT_NFS, name, mntflags, nfsargsp))) {
 		/* Did we just default to v3 on a v2-only kernel?
 		 * If so, default to v2 & try again */
-		if ((errno == EPROGMISMATCH) && !force3) {
+		if ((errno == EPROGMISMATCH) && forceversion < 3) {
 			nfsargsp->flags &= ~NFSMNT_NFSV3;
 			retval = mount(MOUNT_NFS, name, mntflags, nfsargsp);
 		}
@@ -575,7 +674,7 @@
 	char fbuf[2048];
 
 	(void)snprintb(fbuf, sizeof(fbuf), NFSMNT_BITS, nfsargsp->flags);
-	printf("version=%d, addrlen=%d, sotype=%d, proto=%d, fhsize=%d, "
+	(void)printf("version=%d, addrlen=%d, sotype=%d, proto=%d, fhsize=%d, "
 	    "flags=%s, wsize=%d, rsize=%d, readdirsize=%d, timeo=%d, "
 	    "retrans=%d, maxgrouplist=%d, readahead=%d, leaseterm=%d, "
 	    "deadthresh=%d\n",
@@ -673,6 +772,7 @@
 	memset(&hints, 0, sizeof hints);
 	hints.ai_flags = AI_NUMERICHOST;
 	hints.ai_socktype = nfsargsp->sotype;
+	hints.ai_family = forcefamily;
 	if (getaddrinfo(hostp, "nfs", &hints, &ai_nfs) == 0) {
 		if ((nfsargsp->flags & NFSMNT_KERB)) {
 			hints.ai_flags = 0;
@@ -702,12 +802,17 @@
 	}
 #endif /* NFSKERB */
 
-	if (force2) {
+	switch (forceversion) {
+	case 2:
 		nfsvers = NFS_VER2;
 		mntvers = RPCMNT_VER1;
-	} else {
+		break;
+	case 0: case 3:	/* defaults to NFSv3 */
 		nfsvers = NFS_VER3;
 		mntvers = RPCMNT_VER3;
+		break;
+	default:
+		errx(1, "unsupported nfs version \'%d\'", forceversion);
 	}
 	orgcnt = retrycnt;
 	nfhret.stat = EACCES;	/* Mark not yet successful */
@@ -729,6 +834,10 @@
 			netid = "udp";
 	}
 
+	/* mount protocol defaults to connection protocol */
+	if (*mntproto == '\0')
+		strlcpy(mntproto, netid, sizeof(mntproto));
+
 	nconf = getnetconfigent(netid);
 
 tryagain:
@@ -757,7 +866,7 @@
 			 * socket.
 			 */
 			clp = clnt_tp_create(hostp, RPCPROG_MNT, mntvers,
-			     mnttcp_ok ? nconf : getnetconfigent("udp"));
+				getnetconfigent(mntproto));
 			if (clp == NULL) {
 				if ((opflags & ISBGRND) == 0) {
 					clnt_pcreateerror(
@@ -778,7 +887,8 @@
 				    xdr_dir, spec, xdr_fh, &nfhret, try);
 				switch (clnt_stat) {
 				case RPC_PROGVERSMISMATCH:
-					if (nfsvers == NFS_VER3 && !force3) {
+					if (nfsvers == NFS_VER3 &&
+						forceversion < 3) {
 						nfsvers = NFS_VER2;
 						mntvers = RPCMNT_VER1;
 						nfsargsp->flags &=
@@ -898,6 +1008,41 @@
 		return (1);
 	};
 	return (0);
+}
+
+static char *
+getoptval(options, option, bufp, len)
+	const char *options;
+	const char *option;
+	char *bufp;
+	size_t len;
+{
+	/* XXX shouldn't we merge this code with getmntopts? Maybe using a
+	 * callback function? Or we could add something like m_wantval and
+	 * m_valuep to the mntopts struct.
+	 */
+	char *optbuf, *opt, *p;
+
+	if ((optbuf = strdup(options)) == NULL)
+		err(1, NULL);
+
+	for (opt = optbuf; (opt = strtok(opt, ",")) != NULL; opt = NULL) {
+		if ((p = strchr(opt, '=')) == NULL) {
+			free(optbuf);
+			return NULL;
+		}
+
+		*p++ = '\0';
+
+		if (!strcasecmp(opt, option)) {
+			strlcpy(bufp, p, len);
+			free(optbuf);
+			return bufp;
+		}
+	}
+
+	free(optbuf);
+	return NULL;
 }
 
 static void