Hi folks,
Dave Voutila <d...@sisu.io> writes:
> Lucas <lu...@sexy.is> writes:
>
> >>Fix:
> > Dunno. I have the theory that this "broke" (idk if it worked
> > before or not) after "vmd(8): introduce multi-process model for
> > virtio devices." Haven't found out yet a way to notify vionet
> > process about the config: calls into vionet_main (triggered from
> > vm_main) happen before configuration parsing but after init, so
> > it gets the default value of 100.64.0.0/10. Haven't checked it
> > neither, but I also predict that "vmctl load" changes won't be
> > reflected.
>
> Thanks, I'll look to reproduce. There's definitely a message type for
> mac address setting, but I honestly don't know how we were handling this
> prior to the split out into a device process. This might need to be a
> new imsg for any runtime changes via "vmctl load".
I've been able to reproduce this issue as well on my own server, and
was able to get things working properly with the patch inlined below.
Some details: as alluded to above, the vm processes (and thus the
device processes they spawn, including vionet) don't have an up to
date copy of the config (including the 'local prefix[6]'). I was able
to solve this without adding a new message type - by stashing the
currently configured interface prefix into the vmop_create_params
struct when config_setvm() is called. From here, the prefix flows
down from vmm -> vm -> vionet, where tweaked address calculation
functions (vm_priv_addr and vm_priv_addr8) extract this information
rather than falling back on the (incorrect) global config.
I briefly tested this patch and confirmed things now work as they
should - more thorough testing is forthcoming. A downside of
this approach vs. the messaging approach is that a vmctl load will
only change the prefix setting for VMs created in the future. I'm
not sure whether this is a problem / what the utility of changing
the prefix for existing VMs on the fly would be.
I'm new to contributing to vmd and OpenBSD in general, so of course
I'm open to any critiques - thanks!
Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/config.c,v
retrieving revision 1.71
diff -u -p -u -p -r1.71 config.c
--- config.c 28 Apr 2023 19:46:42 -0000 1.71
+++ config.c 14 May 2023 17:04:42 -0000
@@ -210,9 +210,10 @@ config_getreset(struct vmd *env, struct
* Returns 0 on success, error code on failure.
*/
int
-config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid,
uid_t uid)
+config_setvm(struct vmd *env, struct vmd_vm *vm, uint32_t peerid, uid_t uid)
{
int diskfds[VM_MAX_DISKS_PER_VM][VM_MAX_BASE_PER_DISK];
+ struct privsep *ps = &env->vmd_ps;
struct vmd_if *vif;
struct vmop_create_params *vmc = &vm->vm_params;
struct vm_create_params *vcp = &vmc->vmc_params;
@@ -279,6 +280,13 @@ config_setvm(struct privsep *ps, struct
vm->vm_peerid = peerid;
vm->vm_uid = uid;
+
+ /*
+ * Plumb configured localprefix through to vmc so it
+ * makes it to child processes
+ */
+ vmc->vmc_localprefix = env->vmd_cfg.cfg_localprefix;
+ vmc->vmc_localprefix6 = env->vmd_cfg.cfg_localprefix6;
/*
* From here onward, all failures need cleanup and use goto fail
Index: dhcp.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 dhcp.c
--- dhcp.c 27 Apr 2023 22:47:27 -0000 1.12
+++ dhcp.c 14 May 2023 17:04:42 -0000
@@ -41,6 +41,7 @@
static const uint8_t broadcast[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
extern struct vmd *env;
+extern struct vmd_vm *current_vm;
ssize_t
dhcp_request(struct virtio_dev *dev, char *buf, size_t buflen, char **obuf)
@@ -147,8 +148,7 @@ dhcp_request(struct virtio_dev *dev, cha
}
if ((client_addr.s_addr =
- vm_priv_addr(&env->vmd_cfg,
- dev->vm_vmid, vionet->idx, 1)) == 0)
+ vm_priv_addr(current_vm, vionet->idx, 1)) == 0)
return (-1);
memcpy(&resp.yiaddr, &client_addr,
sizeof(client_addr));
@@ -156,8 +156,8 @@ dhcp_request(struct virtio_dev *dev, cha
sizeof(client_addr));
ss2sin(&pc.pc_dst)->sin_port = htons(CLIENT_PORT);
- if ((server_addr.s_addr = vm_priv_addr(&env->vmd_cfg, dev->vm_vmid,
- vionet->idx, 0)) == 0)
+ if ((server_addr.s_addr =
+ vm_priv_addr(current_vm, vionet->idx, 0)) == 0)
return (-1);
memcpy(&resp.siaddr, &server_addr, sizeof(server_addr));
memcpy(&ss2sin(&pc.pc_src)->sin_addr, &server_addr,
Index: priv.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/priv.c,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 priv.c
--- priv.c 28 Jan 2023 14:40:53 -0000 1.22
+++ priv.c 14 May 2023 17:04:42 -0000
@@ -466,8 +466,7 @@ vm_priv_ifconfig(struct privsep *ps, str
sin4->sin_family = AF_INET;
sin4->sin_len = sizeof(*sin4);
if ((sin4->sin_addr.s_addr =
- vm_priv_addr(&env->vmd_cfg,
- vm->vm_vmid, i, 0)) == 0)
+ vm_priv_addr(vm, i, 0)) == 0)
return (-1);
inet_ntop(AF_INET, &sin4->sin_addr,
@@ -493,8 +492,7 @@ vm_priv_ifconfig(struct privsep *ps, str
sin6 = ss2sin6(&vfr.vfr_addr);
sin6->sin6_family = AF_INET6;
sin6->sin6_len = sizeof(*sin6);
- if (vm_priv_addr6(&env->vmd_cfg,
- vm->vm_vmid, i, 0, &sin6->sin6_addr) == -1)
+ if (vm_priv_addr6(vm, i, 0, &sin6->sin6_addr) == -1)
return (-1);
inet_ntop(AF_INET6, &sin6->sin6_addr,
@@ -565,9 +563,9 @@ vm_priv_brconfig(struct privsep *ps, str
}
uint32_t
-vm_priv_addr(struct vmd_config *cfg, uint32_t vmid, int idx, int isvm)
+vm_priv_addr(struct vmd_vm *vm, int idx, int isvm)
{
- struct address *h = &cfg->cfg_localprefix;
+ struct address *h = &vm->vm_params.vmc_localprefix;
in_addr_t prefix, mask, addr;
/*
@@ -580,7 +578,7 @@ vm_priv_addr(struct vmd_config *cfg, uin
mask = prefixlen2mask(h->prefixlen);
/* 2. Encode the VM ID as a per-VM subnet range N, 100.64.N.0/24. */
- addr = vmid << 8;
+ addr = vm->vm_vmid << 8;
/*
* 3. Assign a /31 subnet M per VM interface, 100.64.N.M/31.
@@ -603,7 +601,7 @@ vm_priv_addr(struct vmd_config *cfg, uin
*/
if (prefix != (addr & mask) || idx >= 0x7f) {
log_warnx("%s: dhcp address range exceeded,"
- " vm id %u interface %d", __func__, vmid, idx);
+ " vm id %u interface %d", __func__, vm->vm_vmid, idx);
return (0);
}
@@ -611,10 +609,9 @@ vm_priv_addr(struct vmd_config *cfg, uin
}
int
-vm_priv_addr6(struct vmd_config *cfg, uint32_t vmid,
- int idx, int isvm, struct in6_addr *in6_addr)
+vm_priv_addr6(struct vmd_vm *vm, int idx, int isvm, struct in6_addr
*in6_addr)
{
- struct address *h = &cfg->cfg_localprefix6;
+ struct address *h = &vm->vm_params.vmc_localprefix6;
struct in6_addr addr, mask;
uint32_t addr4;
@@ -626,7 +623,7 @@ vm_priv_addr6(struct vmd_config *cfg, ui
prefixlen2mask6(h->prefixlen, &mask);
/* 2. Encode the VM IPv4 address as subnet, fd00::NN:NN:0:0/96. */
- if ((addr4 = vm_priv_addr(cfg, vmid, idx, 1)) == 0)
+ if ((addr4 = vm_priv_addr(vm, idx, 1)) == 0)
return (0);
memcpy(&addr.s6_addr[8], &addr4, sizeof(addr4));
Index: vmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.149
diff -u -p -u -p -r1.149 vmd.c
--- vmd.c 13 May 2023 23:15:28 -0000 1.149
+++ vmd.c 14 May 2023 17:04:42 -0000
@@ -142,7 +142,7 @@ vmd_dispatch_control(int fd, struct priv
}
/* Try to start the launch of the VM. */
- res = config_setvm(ps, vm, imsg->hdr.peerid,
+ res = config_setvm(env, vm, imsg->hdr.peerid,
vm->vm_params.vmc_owner.uid);
if (res)
cmd = IMSG_VMDOP_START_VM_RESPONSE;
@@ -318,7 +318,7 @@ vmd_dispatch_control(int fd, struct priv
close(imsg->fd);
} else {
vm->vm_state |= VM_STATE_RECEIVED;
- config_setvm(ps, vm, imsg->hdr.peerid,
+ config_setvm(env, vm, imsg->hdr.peerid,
vmc.vmc_owner.uid);
log_debug("%s: sending fd to vmm", __func__);
proc_compose_imsg(ps, PROC_VMM, -1,
@@ -494,7 +494,7 @@ vmd_dispatch_vmm(int fd, struct privsep_
} else {
/* Stop VM instance but keep the tty open */
vm_stop(vm, 1, __func__);
- config_setvm(ps, vm, (uint32_t)-1, vm->vm_uid);
+ config_setvm(env, vm, (uint32_t)-1, vm->vm_uid);
}
/* The error is meaningless for deferred responses */
@@ -995,7 +995,7 @@ start_vm_batch(int fd, short type, void
break;
}
vm->vm_state &= ~VM_STATE_WAITING;
- config_setvm(&env->vmd_ps, vm, -1, vm->vm_params.vmc_owner.uid);
+ config_setvm(env, vm, -1, vm->vm_params.vmc_owner.uid);
}
log_debug("%s: done starting vms", __func__);
}
Index: vmd.h
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
retrieving revision 1.122
diff -u -p -u -p -r1.122 vmd.h
--- vmd.h 13 May 2023 23:15:28 -0000 1.122
+++ vmd.h 14 May 2023 17:04:42 -0000
@@ -195,6 +195,13 @@ struct vmop_owner {
int64_t gid;
};
+struct address {
+ struct sockaddr_storage ss;
+ int prefixlen;
+ TAILQ_ENTRY(address) entry;
+};
+TAILQ_HEAD(addresslist, address);
+
struct vmop_create_params {
struct vm_create_params vmc_params;
unsigned int vmc_flags;
@@ -238,6 +245,8 @@ struct vmop_create_params {
char vmc_ifgroup[VM_MAX_NICS_PER_VM][IF_NAMESIZE];
unsigned int vmc_ifrdomain[VM_MAX_NICS_PER_VM];
uint8_t vmc_macs[VM_MAX_NICS_PER_VM][6];
+ struct address vmc_localprefix;
+ struct address vmc_localprefix6;
struct vmop_owner vmc_owner;
@@ -341,13 +350,6 @@ struct name2id {
};
TAILQ_HEAD(name2idlist, name2id);
-struct address {
- struct sockaddr_storage ss;
- int prefixlen;
- TAILQ_ENTRY(address) entry;
-};
-TAILQ_HEAD(addresslist, address);
-
#define SUN_PATH_LEN (sizeof(((struct sockaddr_un *)NULL)->sun_path))
struct vmd_agentx {
int ax_enabled;
@@ -473,9 +475,8 @@ int priv_findname(const char *, const c
int priv_validgroup(const char *);
int vm_priv_ifconfig(struct privsep *, struct vmd_vm *);
int vm_priv_brconfig(struct privsep *, struct vmd_switch *);
-uint32_t vm_priv_addr(struct vmd_config *, uint32_t, int, int);
-int vm_priv_addr6(struct vmd_config *, uint32_t, int, int,
- struct in6_addr *);
+uint32_t vm_priv_addr(struct vmd_vm *, int, int);
+int vm_priv_addr6(struct vmd_vm *, int, int, struct in6_addr *);
/* vmm.c */
void vmm(struct privsep *, struct privsep_proc *);
@@ -505,7 +506,7 @@ int config_setconfig(struct vmd *);
int config_getconfig(struct vmd *, struct imsg *);
int config_setreset(struct vmd *, unsigned int);
int config_getreset(struct vmd *, struct imsg *);
-int config_setvm(struct privsep *, struct vmd_vm *, uint32_t, uid_t);
+int config_setvm(struct vmd *, struct vmd_vm *, uint32_t, uid_t);
int config_getvm(struct privsep *, struct imsg *);
int config_getdisk(struct privsep *, struct imsg *);
int config_getif(struct privsep *, struct imsg *);
