On 26/10/17(Thu) 16:23, Carlos Cardenas wrote:
> * Require interface name to be defined for switches in vm.conf
> ** Requires user to create bridge(4) beforehand
> * Remove code to create bridges on the fly
> * Add code to ensure bridge really exists
> * Update manpage switch and example sections
What's the motivation to ask people to create a bridge(4) beforehand?
> diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
> index 55a9b0c7acc..a0b21fa8ef1 100644
> --- usr.sbin/vmd/parse.y
> +++ usr.sbin/vmd/parse.y
> @@ -90,7 +90,6 @@ static struct vm_create_params *vcp;
> static struct vmd_switch *vsw;
> static struct vmd_if *vif;
> static struct vmd_vm *vm;
> -static unsigned int vsw_unit;
> static char vsw_type[IF_NAMESIZE];
> static int vcp_disable;
> static size_t vcp_nnics;
> @@ -194,12 +193,17 @@ switch : SWITCH string {
> vsw->sw_id = env->vmd_nswitches + 1;
> vsw->sw_name = $2;
> vsw->sw_flags = VMIFF_UP;
> - snprintf(vsw->sw_ifname, sizeof(vsw->sw_ifname),
> - "%s%u", vsw_type, vsw_unit++);
> TAILQ_INIT(&vsw->sw_ifs);
>
> vcp_disable = 0;
> } '{' optnl switch_opts_l '}' {
> + if (strnlen(vsw->sw_ifname,
> + sizeof(vsw->sw_ifname)) == 0) {
> + yyerror("switch \"%s\" is missing interface
> name",
> + vsw->sw_name);
> + YYERROR;
> + }
> +
> if (vcp_disable) {
> log_debug("%s:%d: switch \"%s\""
> " skipped (disabled)",
> @@ -244,13 +248,12 @@ switch_opts : disable {
> vsw->sw_group = $2;
> }
> | INTERFACE string {
> - if (priv_getiftype($2, vsw_type, &vsw_unit) == -1 ||
> + if (priv_getiftype($2, vsw_type, NULL) == -1 ||
> priv_findname(vsw_type, vmd_descsw) == -1) {
> yyerror("invalid switch interface: %s", $2);
> free($2);
> YYERROR;
> }
> - vsw_unit++;
>
> if (strlcpy(vsw->sw_ifname, $2,
> sizeof(vsw->sw_ifname)) >= sizeof(vsw->sw_ifname)) {
> diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
> index ef42549d105..d66bdcc9b4f 100644
> --- usr.sbin/vmd/priv.c
> +++ usr.sbin/vmd/priv.c
> @@ -87,8 +87,8 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct
> imsg *imsg)
>
> switch (imsg->hdr.type) {
> case IMSG_VMDOP_PRIV_IFDESCR:
> - case IMSG_VMDOP_PRIV_IFCREATE:
> case IMSG_VMDOP_PRIV_IFRDOMAIN:
> + case IMSG_VMDOP_PRIV_IFEXISTS:
> case IMSG_VMDOP_PRIV_IFADD:
> case IMSG_VMDOP_PRIV_IFUP:
> case IMSG_VMDOP_PRIV_IFDOWN:
> @@ -118,13 +118,6 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
> struct imsg *imsg)
> if (ioctl(env->vmd_fd, SIOCSIFDESCR, &ifr) < 0)
> log_warn("SIOCSIFDESCR");
> break;
> - case IMSG_VMDOP_PRIV_IFCREATE:
> - /* Create the bridge if it doesn't exist */
> - strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> - if (ioctl(env->vmd_fd, SIOCIFCREATE, &ifr) < 0 &&
> - errno != EEXIST)
> - log_warn("SIOCIFCREATE");
> - break;
> case IMSG_VMDOP_PRIV_IFRDOMAIN:
> strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> ifr.ifr_rdomainid = vfr.vfr_id;
> @@ -145,6 +138,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
> struct imsg *imsg)
> errno != EEXIST)
> log_warn("SIOCBRDGADD");
> break;
> + case IMSG_VMDOP_PRIV_IFEXISTS:
> + /* Determine if bridge/switch exists */
> + strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> + if (ioctl(env->vmd_fd, SIOCGIFFLAGS, &ifr) < 0)
> + fatalx("%s: bridge \"%s\" does not exist",
> + __func__, vfr.vfr_name);
> + break;
> case IMSG_VMDOP_PRIV_IFUP:
> case IMSG_VMDOP_PRIV_IFDOWN:
> /* Set the interface status */
> @@ -319,10 +319,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
> log_debug("%s: interface %s add %s", __func__,
> vfbr.vfr_name, vfbr.vfr_value);
>
> - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFCREATE,
> - &vfbr, sizeof(vfbr));
> - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
> - &vfbr, sizeof(vfbr));
> proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFADD,
> &vfbr, sizeof(vfbr));
> } else if (vif->vif_switch != NULL)
> @@ -398,7 +394,8 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch
> *vsw)
> sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
> return (-1);
>
> - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFCREATE,
> + // ensure bridge/switch exists
> + proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFEXISTS,
> &vfr, sizeof(vfr));
>
> /* Use the configured rdomain or get it from the process */
> diff --git usr.sbin/vmd/vm.conf.5 usr.sbin/vmd/vm.conf.5
> index 0927fbfecf9..f4bfc2f215b 100644
> --- usr.sbin/vmd/vm.conf.5
> +++ usr.sbin/vmd/vm.conf.5
> @@ -279,11 +279,6 @@ Set the
> or
> .Xr bridge 4
> network interface of this switch.
> -If not specified,
> -.Ar bridge0
> -will be used where the interface unit will be incremented for each switch,
> -e.g.\&
> -.Ar bridge0 , bridge1 , ...
> If the type is changed to
> .Ar switch0 ,
> it will be used for each following switch.
> @@ -318,6 +313,7 @@ vm "vm2.example.com" {
> Create the switch "uplink" with an additional physical network interface:
> .Bd -literal -offset indent
> switch "uplink" {
> + interface bridge0
> add em0
> }
> .Ed
> diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> index 4b7b5f70495..e02c2ea7882 100644
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -94,7 +94,7 @@ enum imsg_type {
> IMSG_VMDOP_RELOAD,
> IMSG_VMDOP_PRIV_IFDESCR,
> IMSG_VMDOP_PRIV_IFADD,
> - IMSG_VMDOP_PRIV_IFCREATE,
> + IMSG_VMDOP_PRIV_IFEXISTS,
> IMSG_VMDOP_PRIV_IFUP,
> IMSG_VMDOP_PRIV_IFDOWN,
> IMSG_VMDOP_PRIV_IFGROUP,
> --
> 2.14.2
>
