On Wed, Oct 11, 2017 at 06:46:22AM +0200, Claudio Jeker wrote: > 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. >
you mean vmd, right? vmctl has no special privilege to do this. > > > 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 >