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
>
