On Tue, Oct 10, 2017 at 03:57:25PM -0700, Carlos Cardenas wrote:
> Destroy switch on `vmctl reload` to allow SIOCBRDGADD to succeed
> when creating new bridge and attaching interfaces to it.
>
> Comments? Ok?
I don't think it is a good idea to destroy and recreate bridge interfaces
on every reload. This changes the underlying network and may result in
broken connections and network hickups. I would suggest that vmctl is
instead checking which interfaces are on the bridge and calls SIOCBRDDEL
if needed and skips the SIOCBRDGADD if not needed.
> diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
> index ef42549d105..0190c049837 100644
> --- usr.sbin/vmd/priv.c
> +++ usr.sbin/vmd/priv.c
> @@ -88,6 +88,7 @@ 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_IFDESTROY:
> case IMSG_VMDOP_PRIV_IFRDOMAIN:
> case IMSG_VMDOP_PRIV_IFADD:
> case IMSG_VMDOP_PRIV_IFUP:
> @@ -125,6 +126,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p,
> struct imsg *imsg)
> errno != EEXIST)
> log_warn("SIOCIFCREATE");
> break;
> + case IMSG_VMDOP_PRIV_IFDESTROY:
> + /* Destroy the bridge */
> + strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> + if (ioctl(env->vmd_fd, SIOCIFDESTROY, &ifr) < 0 &&
> + errno != EEXIST)
> + log_warn("SIOCIFDESTROY");
> + break;
> case IMSG_VMDOP_PRIV_IFRDOMAIN:
> strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name));
> ifr.ifr_rdomainid = vfr.vfr_id;
> @@ -444,6 +452,23 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch
> *vsw)
> return (0);
> }
>
> +int
> +vm_priv_brdestroy(struct privsep *ps, struct vmd_switch *vsw)
> +{
> + struct vmop_ifreq vfr;
> +
> + memset(&vfr, 0, sizeof(vfr));
> +
> + if (strlcpy(vfr.vfr_name, vsw->sw_ifname,
> + sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
> + return (-1);
> +
> + proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESTROY,
> + &vfr, sizeof(vfr));
> +
> + return (0);
> +}
> +
> uint32_t
> vm_priv_addr(struct address *h, uint32_t vmid, int idx, int isvm)
> {
> diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
> index f1abc54d9a3..834654a76de 100644
> --- usr.sbin/vmd/vmd.c
> +++ usr.sbin/vmd/vmd.c
> @@ -852,7 +852,7 @@ int
> vmd_reload(unsigned int reset, const char *filename)
> {
> struct vmd_vm *vm, *next_vm;
> - struct vmd_switch *vsw;
> + struct vmd_switch *vsw, *next_vsw;
> int reload = 0;
>
> /* Switch back to the default config file */
> @@ -885,6 +885,19 @@ vmd_reload(unsigned int reset, const char *filename)
> }
> }
>
> + TAILQ_FOREACH_SAFE(vsw, env->vmd_switches,
> + sw_entry, next_vsw) {
> + log_debug("%s: removing %s",
> + __func__, vsw->sw_ifname);
> + if (vm_priv_brdestroy(&env->vmd_ps,
> + vsw) == -1 ) {
> + log_warn("%s: failed to destroy switch",
> + __func__);
> + }
> + TAILQ_REMOVE(env->vmd_switches, vsw, sw_entry);
> + free(vsw);
> + }
> +
> /* Update shared global configuration in all children */
> if (config_setconfig(env) == -1)
> return (-1);
> diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> index 4b7b5f70495..fce5fcaaa60 100644
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -100,6 +100,7 @@ enum imsg_type {
> IMSG_VMDOP_PRIV_IFGROUP,
> IMSG_VMDOP_PRIV_IFADDR,
> IMSG_VMDOP_PRIV_IFRDOMAIN,
> + IMSG_VMDOP_PRIV_IFDESTROY,
> IMSG_VMDOP_VM_SHUTDOWN,
> IMSG_VMDOP_VM_REBOOT,
> IMSG_VMDOP_CONFIG
> @@ -323,6 +324,7 @@ int priv_findname(const char *, const char **);
> int priv_validgroup(const char *);
> int vm_priv_ifconfig(struct privsep *, struct vmd_vm *);
> int vm_priv_brconfig(struct privsep *, struct vmd_switch *);
> +int vm_priv_brdestroy(struct privsep *, struct vmd_switch *);
> uint32_t vm_priv_addr(struct address *, uint32_t, int, int);
>
> /* vmm.c */
> --
> 2.14.2
>
--
:wq Claudio
