From: Dave Voutila <dv@sisu.io>
Subject: Stop vmd's vionet from spinning when device isn't up
To: "tech@openbsd.org" <tech@openbsd.org>
Cc: Mike Larkin <mlarkin@openbsd.org>
Date: Thu, 01 Feb 2024 16:44:52 -0500

My latest refactor of the vmd(8) vionet device introduced an issue where
if you boot a guest with a network device attached, the second the
driver signals it's "ok" (before it's even set "up") vmd enables the
EV_READ on the tap fd.

The problem is the guest may not have added buffers to the rx virtqueue,
so when these EV_READ events fire we notice there's nowhere to put the
data and don't attempt the read(2) of the tap(4) fd.

As such, the vionet process spins aggressively until the guest brings
the vio(4) up.

In practice, people probably haven't noticed. It's easy to reproduce if
you boot a ramdisk with a network device and don't `ifconfig vio0 up`
for a moment:

    # vmctl start -Lc -b /bsd.rd test
    ( note the vionet process cpu climbing on the host)
    # ifconfig vio0 up
    ( note the vionet process cpu drop towards zero)

The diff below moves the event_add(3) calls to in the virtqueue
notification handler, so when the guest kicks the device to say "hey my
virtqueue is ready" we enable the event.

Note: it's possible we event_add(3) multiple times without a
corresponding event_del(3), but that's fine as we're not using timeouts.

ok?
-dv

diff refs/heads/master refs/heads/vmd-vionet-spin
commit - 78de35773665c96cc0dbb081f7ba59064bb221c2
commit + c1166d5db821533504562012e467e76173e77835
blob - b973577fad9c8facd68c91fe8d780acde1da1682
blob + 751c617ae30067f75d1c5786cdb8ba5b18871144
--- usr.sbin/vmd/vionet.c
+++ usr.sbin/vmd/vionet.c
@@ -622,7 +622,12 @@ vionet_notifyq(struct virtio_dev *dev)
 	struct vionet_dev *vionet = &dev->vionet;

 	switch (vionet->cfg.queue_notify) {
-	case TXQ: return vionet_notify_tx(dev);
+	case RXQ:
+		event_add(&ev_tap, NULL);
+		event_add(&ev_inject, NULL);
+		break;
+	case TXQ:
+		return vionet_notify_tx(dev);
 	default:
 		/*
 		 * Catch the unimplemented queue ID 2 (control queue) as
@@ -1027,17 +1032,11 @@ handle_io_write(struct viodev_msg *msg, struct virtio_
 			vionet->vq[TXQ].last_avail = 0;
 			vionet->vq[TXQ].notified_avail = 0;
 			virtio_deassert_pic_irq(dev, msg->vcpu);
+
+			event_del(&ev_tap);
+			event_del(&ev_inject);
 		}
-		event_del(&ev_tap);
-		event_del(&ev_inject);
-		if (vionet->cfg.device_status
-		    & VIRTIO_CONFIG_DEVICE_STATUS_DRIVER_OK) {
-			event_add(&ev_tap, NULL);
-			event_add(&ev_inject, NULL);
-		}
 		break;
-	default:
-		break;
 	}
 	return (intr);
 }