Index | Thread | Search

From:
Tobias Heider <tobias.heider@stusta.de>
Subject:
vionet: fix feature negotiation with Linux 6.17
To:
tech@openbsd.org
Cc:
dv@openbsd.org, mlarkin@openbsd.org
Date:
Sun, 19 Oct 2025 15:28:17 +0200

Download raw body.

Thread
After upgrading my Linux kernel vionet stopped working.
It looks like a recent kernel change exposes a bug in our feature negotiation
code.

The responsible Linux change seems to be:
https://github.com/torvalds/linux/commit/69b9461512246599ed80cf13358e7e6aff7285f9

This extends the feature bits to 128 meaning Linux will query
device_feature_select 0-3 instead of 0-1 as it did previously.

In vionet_cfg_read we only handle device_feature_seelect 0 and 1 in the
VIO1_PCI_DEVICE_FEATURE case and (falsely) return -1 on higher values.
This translates to fffffffff which Linux interprets as all features are
supported.

I hacked some prints into the Linux kernel to show what happens on the
driver side:
Oct 19 12:38:44 ubuntu kernel: vp_modern_get_extended_features: 0:0020
Oct 19 12:38:44 ubuntu kernel: vp_modern_get_extended_features: 1:0001
Oct 19 12:38:44 ubuntu kernel: vp_modern_get_extended_features: 2:ffffffff
Oct 19 12:38:44 ubuntu kernel: vp_modern_get_extended_features: 3:ffffffff
Oct 19 12:38:44 ubuntu kernel: vp_modern_get_extended_features: 00000020ffffffff
Oct 19 12:38:44 ubuntu kernel: vp_modern_set_extended_features: 000000200000001e
Oct 19 12:38:44 ubuntu kernel: vp_modern_set_extended_features: 0:0020
Oct 19 12:38:44 ubuntu kernel: vp_modern_set_extended_features: 1:0001
Oct 19 12:38:44 ubuntu kernel: vp_modern_set_extended_features: 2:001e
Oct 19 12:38:44 ubuntu kernel: vp_modern_set_extended_features: 3:0000

Linux as a consequence enables GSO on the driver side causing it to send a
larger than expected vionet header which vmd can't handle and causes a bunch
of extra null bytes in front of each sent network frame.

The is changing the default return value to 0 in vionet_cfg_read as is the
case in virtio.c:virtio_io_cfg(). As far as I can tell all the error cases
are essentially meant to be no-ops, so I don't see when we would ever want
to return -1.

diff /usr/src
path + /usr/src
commit - 0f168beaa75c01d98837c6bdc72dedfa4aa42cd4
blob - 6eb013e48bf23565b1e57abf5b2d039ec073783f
file + usr.sbin/vmd/vionet.c
--- usr.sbin/vmd/vionet.c
+++ usr.sbin/vmd/vionet.c
@@ -980,7 +980,7 @@ static uint32_t
 vionet_cfg_read(struct virtio_dev *dev, struct viodev_msg *msg)
 {
 	struct virtio_pci_common_cfg *pci_cfg = &dev->pci_cfg;
-	uint32_t data = (uint32_t)(-1);
+	uint32_t data = 0;
 	uint16_t reg = msg->reg & 0x00FF;
 
 	pthread_rwlock_rdlock(&lock);