o replace special handling of clone operations by a clone callback

  mechanism
o change vlan cloning to use callback and pass all vlan parameters
  on create using the new SIOCREATE2 ioctl
o update vlan set logic to match existing practice

4 files changed, 127 insertions, 100 deletions

diff --git a/sbin/ifconfig/ifclone.c b/sbin/ifconfig/ifclone.c
index 8b613ad..0dafac4 100644
--- a/sbin/ifconfig/ifclone.c
+++ b/sbin/ifconfig/ifclone.c
@@ -88,48 +88,62 @@ list_cloners(void)
 	free(buf);
 }
 
+static clone_callback_func *clone_cb = NULL;
+
 void
-clone_create(void)
+clone_setcallback(clone_callback_func *p)
 {
-	int s;
+	if (clone_cb != NULL && clone_cb != p)
+		errx(1, "conflicting device create parameters");
+	clone_cb = p;
+}
 
-	s = socket(AF_INET, SOCK_DGRAM, 0);
-	if (s == -1)
-		err(1, "socket(AF_INET,SOCK_DGRAM)");
+/*
+ * Do the actual clone operation.  Any parameters must have been
+ * setup by now.  If a callback has been setup to do the work
+ * then defer to it; otherwise do a simple create operation with
+ * no parameters.
+ */
+static void
+ifclonecreate(int s, void *arg)
+{
+	struct ifreq ifr;
 
 	memset(&ifr, 0, sizeof(ifr));
 	(void) strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
-	if (ioctl(s, SIOCIFCREATE, &ifr) < 0)
-		err(1, "SIOCIFCREATE");
+	if (clone_cb == NULL) {
+		/* NB: no parameters */
+		if (ioctl(s, SIOCIFCREATE2, &ifr) < 0)
+			err(1, "SIOCIFCREATE2");
+	} else {
+		clone_cb(s, &ifr);
+	}
 
 	/*
-	 * If we get a different name back then we put in, we probably
-	 * want to print it out, but we might change our mind later so
-	 * we just signal our intrest and leave the printout for later.
+	 * If we get a different name back than we put in, print it.
 	 */
-	if (strcmp(name, ifr.ifr_name) != 0) {
-		printname = 1;
+	if (strncmp(name, ifr.ifr_name, sizeof(name)) != 0) {
 		strlcpy(name, ifr.ifr_name, sizeof(name));
+		printf("%s\n", name);
 	}
-
-	close(s);
 }
 
-static void
-clone_destroy(const char *val, int d, int s, const struct afswtch *rafp)
+static
+DECL_CMD_FUNC(clone_create, arg, d)
 {
+	callback_register(ifclonecreate, NULL);
+}
 
+static
+DECL_CMD_FUNC(clone_destroy, arg, d)
+{
 	(void) strncpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
 	if (ioctl(s, SIOCIFDESTROY, &ifr) < 0)
 		err(1, "SIOCIFDESTROY");
-	/*
-	 * If we create and destroy an interface in the same command,
-	 * there isn't any reason to print it's name.
-	 */
-	printname = 0;
 }
 
 static struct cmd clone_cmds[] = {
+	DEF_CMD("create",	0,	clone_create),
 	DEF_CMD("destroy",	0,	clone_destroy),
 	DEF_CMD("unplumb",	0,	clone_destroy),
 };
diff --git a/sbin/ifconfig/ifconfig.c b/sbin/ifconfig/ifconfig.c
index a97b089..3f0172f 100644
--- a/sbin/ifconfig/ifconfig.c
+++ b/sbin/ifconfig/ifconfig.c
@@ -92,7 +92,6 @@ int	verbose;
 
 int	supmedia = 0;
 int	printkeys = 0;		/* Print keying material for interfaces. */
-int	printname = 0;		/* Print the name of the created interface. */
 
 static	int ifconfig(int argc, char *const *argv, const struct afswtch *afp);
 static	void status(const struct afswtch *afp, int addrcount,
@@ -234,21 +233,20 @@ main(int argc, char *argv[])
 		/* check and maybe load support for this interface */
 		ifmaybeload(name);
 
-		/*
-		 * NOTE:  We must special-case the `create' command right
-		 * here as we would otherwise fail when trying to find
-		 * the interface.
-		 */
-		if (argc > 0 && (strcmp(argv[0], "create") == 0 ||
-		    strcmp(argv[0], "plumb") == 0)) {
-			clone_create();
-			argc--, argv++;
-			if (argc == 0)
-				goto end;
-		}
 		ifindex = if_nametoindex(name);
-		if (ifindex == 0)
+		if (ifindex == 0) {
+			/*
+			 * NOTE:  We must special-case the `create' command
+			 * right here as we would otherwise fail when trying
+			 * to find the interface.
+			 */
+			if (argc > 0 && (strcmp(argv[0], "create") == 0 ||
+			    strcmp(argv[0], "plumb") == 0)) {
+				ifconfig(argc, argv, NULL);
+				exit(0);
+			}
 			errx(1, "interface %s does not exist", name);
+		}
 	}
 
 	/* Check for address family */
@@ -356,9 +354,6 @@ retry:
 
 	if (namesonly && need_nl > 0)
 		putchar('\n');
-end:
-	if (printname)
-		printf("%s\n", name);
 
 	exit (0);
 }
@@ -782,12 +777,6 @@ setifname(const char *val, int dummy __unused, int s,
 	}
 	strlcpy(name, newname, sizeof(name));
 	free(newname);
-
-	/*
-	 * Even if we just created the interface, we don't need to print
-	 * its name because we just nailed it down separately.
-	 */
-	printname = 0;
 }
 
 /*
diff --git a/sbin/ifconfig/ifconfig.h b/sbin/ifconfig/ifconfig.h
index dc68a81..79f1fcb 100644
--- a/sbin/ifconfig/ifconfig.h
+++ b/sbin/ifconfig/ifconfig.h
@@ -127,7 +127,6 @@ extern	char name[IFNAMSIZ];	/* name of interface */
 extern	int allmedia;
 extern	int supmedia;
 extern	int printkeys;
-extern	int printname;
 extern	int flags;
 extern	int newaddr;
 extern	int verbose;
@@ -140,4 +139,5 @@ void	printb(const char *s, unsigned value, const char *bits);
 
 void	ifmaybeload(char *name);
 
-void	clone_create(void);
+typedef void clone_callback_func(int, struct ifreq *);
+void	clone_setcallback(clone_callback_func *);
diff --git a/sbin/ifconfig/ifvlan.c b/sbin/ifconfig/ifvlan.c
index f2dfba4..205ed21 100644
--- a/sbin/ifconfig/ifvlan.c
+++ b/sbin/ifconfig/ifvlan.c
@@ -58,93 +58,117 @@
 static const char rcsid[] =
   "$FreeBSD$";
 #endif
-static struct vlanreq		__vreq;
-static int			__have_dev = 0;
-static int			__have_tag = 0;
 
-static void			vlan_set(int);
+#define	NOTAG	((u_short) -1)
+
+static 	struct vlanreq params = {
+	.vlr_tag	= NOTAG,
+};
+
+static int
+getvlan(int s, struct ifreq *ifr, struct vlanreq *vreq)
+{
+	bzero((char *)vreq, sizeof(*vreq));
+	ifr->ifr_data = (caddr_t)vreq;
+
+	return ioctl(s, SIOCGETVLAN, (caddr_t)ifr);
+}
 
 static void
 vlan_status(int s)
 {
 	struct vlanreq		vreq;
 
-	bzero((char *)&vreq, sizeof(vreq));
-	ifr.ifr_data = (caddr_t)&vreq;
+	if (getvlan(s, &ifr, &vreq) != -1)
+		printf("\tvlan: %d parent interface: %s\n",
+		    vreq.vlr_tag, vreq.vlr_parent[0] == '\0' ?
+		    "<none>" : vreq.vlr_parent);
+}
 
-	if (ioctl(s, SIOCGETVLAN, (caddr_t)&ifr) == -1)
-		return;
+static void
+vlan_create(int s, struct ifreq *ifr)
+{
+	if (params.vlr_tag != NOTAG || params.vlr_parent[0] != '\0') {
+		/*
+		 * One or both parameters were specified, make sure both.
+		 */
+		if (params.vlr_tag == NOTAG)
+			errx(1, "must specify a tag for vlan create");
+		if (params.vlr_parent[0] == '\0')
+			errx(1, "must specify a parent device for vlan create");
+		ifr->ifr_data = (caddr_t) &params;
+	}
+	if (ioctl(s, SIOCIFCREATE2, ifr) < 0)
+		err(1, "SIOCIFCREATE2");
+}
 
-	printf("\tvlan: %d parent interface: %s\n",
-	    vreq.vlr_tag, vreq.vlr_parent[0] == '\0' ?
-	    "<none>" : vreq.vlr_parent);
+static void
+vlan_cb(int s, void *arg)
+{
+	if ((params.vlr_tag != NOTAG) ^ (params.vlr_parent[0] != '\0'))
+		errx(1, "both vlan and vlandev must be specified");
 }
 
 static void
-setvlantag(const char *val, int d, int s, const struct afswtch	*afp)
+vlan_set(int s, struct ifreq *ifr)
+{
+	if (params.vlr_tag != NOTAG && params.vlr_parent[0] != '\0') {
+		ifr->ifr_data = (caddr_t) &params;
+		if (ioctl(s, SIOCSETVLAN, (caddr_t)ifr) == -1)
+			err(1, "SIOCSETVLAN");
+	}
+}
+
+static
+DECL_CMD_FUNC(setvlantag, val, d)
 {
-	char			*endp;
-	u_long			ul;
+	struct vlanreq vreq;
+	u_long ul;
+	char *endp;
 
 	ul = strtoul(val, &endp, 0);
 	if (*endp != '\0')
 		errx(1, "invalid value for vlan");
-	__vreq.vlr_tag = ul;
+	params.vlr_tag = ul;
 	/* check if the value can be represented in vlr_tag */
-	if (__vreq.vlr_tag != ul)
+	if (params.vlr_tag != ul)
 		errx(1, "value for vlan out of range");
-	/* the kernel will do more specific checks on vlr_tag */
-	__have_tag = 1;
-	vlan_set(s);	/* try setting vlan params in kernel */
+
+	if (getvlan(s, &ifr, &vreq) != -1)
+		vlan_set(s, &ifr);
+	else
+		clone_setcallback(vlan_create);
 }
 
-static void
-setvlandev(const char *val, int d, int s, const struct afswtch	*afp)
+static
+DECL_CMD_FUNC(setvlandev, val, d)
 {
+	struct vlanreq vreq;
+
+	strlcpy(params.vlr_parent, val, sizeof(params.vlr_parent));
 
-	strncpy(__vreq.vlr_parent, val, sizeof(__vreq.vlr_parent));
-	__have_dev = 1;
-	vlan_set(s);	/* try setting vlan params in kernel */
+	if (getvlan(s, &ifr, &vreq) != -1)
+		vlan_set(s, &ifr);
+	else
+		clone_setcallback(vlan_create);
 }
 
-static void
-unsetvlandev(const char *val, int d, int s, const struct afswtch *afp)
+static
+DECL_CMD_FUNC(unsetvlandev, val, d)
 {
+	struct vlanreq		vreq;
 
-	if (val != NULL)
-		warnx("argument to -vlandev is useless and hence deprecated");
+	bzero((char *)&vreq, sizeof(struct vlanreq));
+	ifr.ifr_data = (caddr_t)&vreq;
 
-	bzero((char *)&__vreq, sizeof(__vreq));
-	ifr.ifr_data = (caddr_t)&__vreq;
-#if 0	/* this code will be of use when we can alter vlan or vlandev only */
 	if (ioctl(s, SIOCGETVLAN, (caddr_t)&ifr) == -1)
 		err(1, "SIOCGETVLAN");
 
-	bzero((char *)&__vreq.vlr_parent, sizeof(__vreq.vlr_parent));
-	__vreq.vlr_tag = 0; /* XXX clear parent only (no kernel support now) */
-#endif
+	bzero((char *)&vreq.vlr_parent, sizeof(vreq.vlr_parent));
+	vreq.vlr_tag = 0;
+
 	if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
 		err(1, "SIOCSETVLAN");
-	__have_dev = __have_tag = 0;
-}
-
-static void
-vlan_cb(int s, void *arg)
-{
-
-	if (__have_tag ^ __have_dev)
-		errx(1, "both vlan and vlandev must be specified");
-}
-
-static void
-vlan_set(int s)
-{
-
-	if (__have_tag && __have_dev) {
-		ifr.ifr_data = (caddr_t)&__vreq;
-		if (ioctl(s, SIOCSETVLAN, (caddr_t)&ifr) == -1)
-			err(1, "SIOCSETVLAN");
-	}
 }
 
 static struct cmd vlan_cmds[] = {