On Wed, Nov 08, 2017 at 06:49:30PM -0800, Carlos Cardenas wrote:
> Remove configuration items on switches for:
> * adding static interfaces
>
> Adding static interfaces are to be set in hostname.if.
>
> Changed rule on rdomain:
> * vm->interface->rdomain takes precedence over sw->rdomain
>
> Update examples/vm.conf and vm.conf manpage to reflect changes.
>
> Comments? Ok?
>
> Ok by reyk@
>
committed, thanks!
-ml
> diff --git etc/examples/vm.conf etc/examples/vm.conf
> index f35235e3fba..d61ce8c4977 100644
> --- etc/examples/vm.conf
> +++ etc/examples/vm.conf
> @@ -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 usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
> index a0e96545923..18543a74911 100644
> --- usr.sbin/vmd/parse.y
> +++ usr.sbin/vmd/parse.y
> @@ -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 usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
> index d585bf75a99..c1d0c0e13be 100644
> --- usr.sbin/vmd/priv.c
> +++ usr.sbin/vmd/priv.c
> @@ -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 usr.sbin/vmd/vm.conf.5 usr.sbin/vmd/vm.conf.5
> index 6ce21896657..fd87f999d9f 100644
> --- usr.sbin/vmd/vm.conf.5
> +++ usr.sbin/vmd/vm.conf.5
> @@ -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 usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
> index 9e90c63cc99..9dbea8479f7 100644
> --- usr.sbin/vmd/vmd.c
> +++ usr.sbin/vmd/vmd.c
> @@ -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);
> }
> @@ -1342,20 +1338,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 usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> index a82aa8e8107..1b0789be756 100644
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -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;
> };
> --
> 2.14.3
>
