update switch handling in vmd(8). vmd now gets switch information (rdomain,

etc) from underlying switch interface instead of handling this on its
own.

Diff from carlos cardenas, Thanks!

ok reyk@

6 files changed, 71 insertions, 95 deletions

diff --git a/etc/examples/vm.conf b/etc/examples/vm.conf
index f35235e3fba..12f1610399a 100644
--- a/etc/examples/vm.conf
+++ b/etc/examples/vm.conf
@@ -1,4 +1,4 @@
-# $OpenBSD: vm.conf,v 1.7 2017/03/25 16:28:25 reyk Exp $
+# $OpenBSD: vm.conf,v 1.8 2017/11/11 02:50:07 mlarkin Exp $
 
 #
 # Macros
@@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/"
 #
 
 switch "uplink" {
-	# This interface will default to bridge0, but switch(4) is supported
+	# This switch will use bridge0, defined by /etc/hostname.bridge0, as
+	# the underlying interface.  switch(4) is also supported
 	#interface switch0
-
-	# Add additional members
-	add em0
+	interface bridge0
 }
 
 switch "local" {
-	add vether0
+	interface bridge1
 	down
 }
 
diff --git a/usr.sbin/vmd/parse.y b/usr.sbin/vmd/parse.y
index a0e96545923..b909660829e 100644
--- a/usr.sbin/vmd/parse.y
+++ b/usr.sbin/vmd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.30 2017/10/30 03:37:33 mlarkin Exp $	*/
+/*	$OpenBSD: parse.y,v 1.31 2017/11/11 02:50:07 mlarkin Exp $	*/
 
 /*
  * Copyright (c) 2007-2016 Reyk Floeter <reyk@openbsd.org>
@@ -88,7 +88,6 @@ int		 parse_disk(char *);
 static struct vmop_create_params vmc;
 static struct vm_create_params	*vcp;
 static struct vmd_switch	*vsw;
-static struct vmd_if		*vif;
 static struct vmd_vm		*vm;
 static char			 vsw_type[IF_NAMESIZE];
 static int			 vcp_disable;
@@ -193,7 +192,6 @@ switch		: SWITCH string			{
 			vsw->sw_id = env->vmd_nswitches + 1;
 			vsw->sw_name = $2;
 			vsw->sw_flags = VMIFF_UP;
-			TAILQ_INIT(&vsw->sw_ifs);
 
 			vcp_disable = 0;
 		} '{' optnl switch_opts_l '}'	{
@@ -224,21 +222,6 @@ switch_opts_l	: switch_opts_l switch_opts nl
 switch_opts	: disable			{
 			vcp_disable = $1;
 		}
-		| ADD string			{
-			char		type[IF_NAMESIZE];
-
-			if ((vif = calloc(1, sizeof(*vif))) == NULL)
-				fatal("could not allocate interface");
-
-			if (priv_getiftype($2, type, NULL) == -1) {
-				yyerror("invalid interface: %s", $2);
-				free($2);
-				YYERROR;
-			}
-			vif->vif_name = $2;
-
-			TAILQ_INSERT_TAIL(&vsw->sw_ifs, vif, vif_entry);
-		}
 		| GROUP string			{
 			if (priv_validgroup($2) == -1) {
 				yyerror("invalid group name: %s", $2);
diff --git a/usr.sbin/vmd/priv.c b/usr.sbin/vmd/priv.c
index d585bf75a99..0ccf09a7164 100644
--- a/usr.sbin/vmd/priv.c
+++ b/usr.sbin/vmd/priv.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: priv.c,v 1.12 2017/10/30 03:37:33 mlarkin Exp $	*/
+/*	$OpenBSD: priv.c,v 1.13 2017/11/11 02:50:07 mlarkin Exp $	*/
 
 /*
  * Copyright (c) 2016 Reyk Floeter <reyk@openbsd.org>
@@ -255,9 +255,18 @@ priv_validgroup(const char *name)
 }
 
 /*
- * Called from the process peer
+ * Called from the Parent process to setup vm interface(s)
+ * - ensure the interface has the description set (tracking purposes)
+ * - if interface is to be attached to a switch, attach it
+ * - check if rdomain is set on interface and switch
+ *   - if interface only or both, use interface rdomain
+ *   - if switch only, use switch rdomain
+ * - check if group is set on interface and switch
+ *   - if interface, add it
+ *   - if switch, add it
+ * - ensure the interface is up/down
+ * - if local interface, set address
  */
-
 int
 vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 {
@@ -279,18 +288,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 		    sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
 			return (-1);
 
-		/* Use the configured rdomain or get it from the process */
-		if (vif->vif_flags & VMIFF_RDOMAIN)
-			vfr.vfr_id = vif->vif_rdomain;
-		else
-			vfr.vfr_id = getrtable();
-		if (vfr.vfr_id != 0)
-			log_debug("%s: interface %s rdomain %u", __func__,
-			    vfr.vfr_name, vfr.vfr_id);
-
-		proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
-		    &vfr, sizeof(vfr));
-
 		/* Description can be truncated */
 		(void)snprintf(vfr.vfr_value, sizeof(vfr.vfr_value),
 		    "vm%u-if%u-%s", vm->vm_vmid, i, vcp->vcp_name);
@@ -301,8 +298,17 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 		proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR,
 		    &vfr, sizeof(vfr));
 
-		/* Add interface to bridge/switch */
-		if ((vsw = switch_getbyname(vif->vif_switch)) != NULL) {
+		/* set default rdomain */
+		vfr.vfr_id = getrtable();
+
+		vsw = switch_getbyname(vif->vif_switch);
+
+		/* Check if switch should exist */
+		if (vsw == NULL && vif->vif_switch != NULL)
+			log_warnx("switch \"%s\" not found", vif->vif_switch);
+
+		/* Add interface to switch and set proper rdomain */
+		if (vsw != NULL) {
 			memset(&vfbr, 0, sizeof(vfbr));
 
 			if (strlcpy(vfbr.vfr_name, vsw->sw_ifname,
@@ -311,18 +317,32 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 			if (strlcpy(vfbr.vfr_value, vif->vif_name,
 			    sizeof(vfbr.vfr_value)) >= sizeof(vfbr.vfr_value))
 				return (-1);
-			if (vsw->sw_flags & VMIFF_RDOMAIN)
-				vfbr.vfr_id = vsw->sw_rdomain;
-			else
-				vfbr.vfr_id = getrtable();
 
-			log_debug("%s: interface %s add %s", __func__,
-			    vfbr.vfr_name, vfbr.vfr_value);
+			log_debug("%s: switch \"%s\" interface %s add %s",
+			    __func__, vsw->sw_name, vfbr.vfr_name,
+			    vfbr.vfr_value);
 
 			proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFADD,
 			    &vfbr, sizeof(vfbr));
-		} else if (vif->vif_switch != NULL)
-			log_warnx("switch %s not found", vif->vif_switch);
+
+			/* Check rdomain properties */
+			if (vif->vif_flags & VMIFF_RDOMAIN)
+				vfr.vfr_id = vif->vif_rdomain;
+			else if (vsw->sw_flags & VMIFF_RDOMAIN)
+				vfr.vfr_id = vsw->sw_rdomain;
+		} else {
+			/* No switch to attach case */
+			if (vif->vif_flags & VMIFF_RDOMAIN)
+				vfr.vfr_id = vif->vif_rdomain;
+		}
+
+		/* Set rdomain on interface */
+		if (vfr.vfr_id != 0)
+			log_debug("%s: interface %s rdomain %u", __func__,
+			    vfr.vfr_name, vfr.vfr_id);
+
+		proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
+		    &vfr, sizeof(vfr));
 
 		/* First group is defined per-interface */
 		if (vif->vif_group) {
@@ -343,7 +363,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 			    sizeof(vfr.vfr_value)) >= sizeof(vfr.vfr_value))
 				return (-1);
 
-			log_debug("%s: interface %s group %s switch %s",
+			log_debug("%s: interface %s group %s switch \"%s\"",
 			    __func__, vfr.vfr_name, vfr.vfr_value,
 			    vsw->sw_name);
 
@@ -356,6 +376,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 		    IMSG_VMDOP_PRIV_IFUP : IMSG_VMDOP_PRIV_IFDOWN,
 		    &vfr, sizeof(vfr));
 
+		/* Set interface address if it is a local interface */
 		if (vm->vm_params.vmc_ifflags[i] & VMIFF_LOCAL) {
 			sin4 = (struct sockaddr_in *)&vfr.vfr_ifra.ifra_mask;
 			sin4->sin_family = AF_INET;
@@ -382,10 +403,16 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 	return (0);
 }
 
+/*
+ * Called from the Parent process to setup underlying switch interface
+ * - ensure the interface exists
+ * - ensure the interface has the correct rdomain set
+ * - ensure the interface has the description set (tracking purposes)
+ * - ensure the interface is up/down
+ */
 int
 vm_priv_brconfig(struct privsep *ps, struct vmd_switch *vsw)
 {
-	struct vmd_if		*vif;
 	struct vmop_ifreq	 vfr;
 
 	memset(&vfr, 0, sizeof(vfr));
@@ -407,6 +434,7 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch *vsw)
 		log_debug("%s: interface %s rdomain %u", __func__,
 		    vfr.vfr_name, vfr.vfr_id);
 
+	/* ensure switch has the correct rodmain */
 	proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
 	    &vfr, sizeof(vfr));
 
@@ -420,18 +448,6 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch *vsw)
 	proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR,
 	    &vfr, sizeof(vfr));
 
-	TAILQ_FOREACH(vif, &vsw->sw_ifs, vif_entry) {
-		if (strlcpy(vfr.vfr_value, vif->vif_name,
-		    sizeof(vfr.vfr_value)) >= sizeof(vfr.vfr_value))
-			return (-1);
-
-		log_debug("%s: interface %s add %s", __func__,
-		    vfr.vfr_name, vfr.vfr_value);
-
-		proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFADD,
-		    &vfr, sizeof(vfr));
-	}
-
 	/* Set the new interface status to up or down */
 	proc_compose(ps, PROC_PRIV, (vsw->sw_flags & VMIFF_UP) ?
 	    IMSG_VMDOP_PRIV_IFUP : IMSG_VMDOP_PRIV_IFDOWN,
diff --git a/usr.sbin/vmd/vm.conf.5 b/usr.sbin/vmd/vm.conf.5
index 89d25b3faf5..ee8e1d72bb0 100644
--- a/usr.sbin/vmd/vm.conf.5
+++ b/usr.sbin/vmd/vm.conf.5
@@ -1,4 +1,4 @@
-.\" $OpenBSD: vm.conf.5,v 1.24 2017/11/05 20:01:09 reyk Exp $
+.\" $OpenBSD: vm.conf.5,v 1.25 2017/11/11 02:50:07 mlarkin Exp $
 .\"
 .\" Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
 .\" Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: November 5 2017 $
+.Dd $Mdocdate: November 11 2017 $
 .Dt VM.CONF 5
 .Os
 .Sh NAME
@@ -166,6 +166,11 @@ will drop packets from the VM with altered source addresses.
 .It Cm rdomain Ar rdomainid
 Attach the interface to the routing domain with the specified
 .Ar rdomainid .
+If attaching to a switch that also has a
+.Ar rdomainid
+set, the
+.Ar rdomainid
+configured for the interface takes precedence.
 .It Cm switch Ar name
 Set the virtual switch
 by
@@ -211,9 +216,7 @@ or
 .Xr switch 4 .
 The network interface for each virtual switch defined in
 .Nm
-is automatically created by
-.Xr vmd 8 ,
-but it is also possible to pre-configure switch interfaces using
+is pre-configured using
 .Xr hostname.if 5
 or
 .Xr ifconfig 8
@@ -244,12 +247,6 @@ This name can be any string, and is typically a network name.
 .Pp
 Followed by a block of parameters that is enclosed in curly brackets:
 .Bl -tag -width Ds
-.It Cm add Ar interface
-Add
-.Ar interface
-as a member of the switch.
-Any network interface can be added, typically as an uplink interface,
-but it can be a member of at most one switch.
 .It Cm enable
 Automatically configure the switch.
 This is the default if neither
@@ -285,9 +282,6 @@ it will be used for each following switch.
 .It Cm rdomain Ar rdomainid
 Set the routing domain of the switch and all of its VM interfaces to
 .Ar rdomainid .
-This overwrites the
-.Cm rdomain
-option of VM interfaces.
 .It Cm up
 Start the switch forwarding packets.
 This is the default.
@@ -314,7 +308,6 @@ Create the switch "uplink" with an additional physical network interface:
 .Bd -literal -offset indent
 switch "uplink" {
 	interface bridge0
-	add em0
 }
 .Ed
 .Sh SEE ALSO
diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c
index c8377fd3eef..c6438dcf091 100644
--- a/usr.sbin/vmd/vmd.c
+++ b/usr.sbin/vmd/vmd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.c,v 1.73 2017/11/07 07:38:30 mlarkin Exp $	*/
+/*	$OpenBSD: vmd.c,v 1.74 2017/11/11 02:50:07 mlarkin Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -1172,10 +1172,6 @@ vm_register(struct privsep *ps, struct vmop_create_params *vmc,
 		vm->vm_ifs[i].vif_fd = -1;
 
 		if ((sw = switch_getbyname(vmc->vmc_ifswitch[i])) != NULL) {
-			/* overwrite the rdomain, if configured on the switch */
-			if (sw->sw_flags & VMIFF_RDOMAIN)
-				vmc->vmc_ifrdomain[i] = sw->sw_rdomain;
-
 			/* inherit per-interface flags from the switch */
 			vmc->vmc_ifflags[i] |= (sw->sw_flags & VMIFF_OPTMASK);
 		}
@@ -1357,20 +1353,11 @@ vm_closetty(struct vmd_vm *vm)
 void
 switch_remove(struct vmd_switch *vsw)
 {
-	struct vmd_if	*vif;
-
 	if (vsw == NULL)
 		return;
 
 	TAILQ_REMOVE(env->vmd_switches, vsw, sw_entry);
 
-	while ((vif = TAILQ_FIRST(&vsw->sw_ifs)) != NULL) {
-		free(vif->vif_name);
-		free(vif->vif_switch);
-		TAILQ_REMOVE(&vsw->sw_ifs, vif, vif_entry);
-		free(vif);
-	}
-
 	free(vsw->sw_group);
 	free(vsw->sw_name);
 	free(vsw);
diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h
index a82aa8e8107..0263cb44a4e 100644
--- a/usr.sbin/vmd/vmd.h
+++ b/usr.sbin/vmd/vmd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.h,v 1.65 2017/10/30 03:37:33 mlarkin Exp $	*/
+/*	$OpenBSD: vmd.h,v 1.66 2017/11/11 02:50:08 mlarkin Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -191,7 +191,6 @@ struct vmd_if {
 	unsigned int		 vif_flags;
 	TAILQ_ENTRY(vmd_if)	 vif_entry;
 };
-TAILQ_HEAD(viflist, vmd_if);
 
 struct vmd_switch {
 	uint32_t		 sw_id;
@@ -200,7 +199,6 @@ struct vmd_switch {
 	char			*sw_group;
 	unsigned int		 sw_rdomain;
 	unsigned int		 sw_flags;
-	struct viflist		 sw_ifs;
 	int			 sw_running;
 	TAILQ_ENTRY(vmd_switch)	 sw_entry;
 };