Index | Thread | Search

From:
Stefan Fritsch <sf@sfritsch.de>
Subject:
vio: Improve feature negotiation for LRO/TSO
To:
tech@openbsd.org
Cc:
Jan Klemkow <j.klemkow@wemelug.de>
Date:
Mon, 12 Jan 2026 16:18:22 +0100

Download raw body.

Thread
Hi,

OpenBSD requires that LRO can be switched on and off for things like 
bridged vlan(4), vxlan(4), bpe(4). We currently only support switching LRO 
on/off if the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature was negotiated. But 
this means if the hypervisor only offers VIRTIO_NET_F_GUEST_TSO4/6 but not 
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, things will break. In this case we must 
redo feature negotation without the GUEST_TSO4/6 features.

Also, if the hypervisor offers GUEST_TSO4/6 but not the 
VIRTIO_NET_F_MRG_RXBUF feature, we currently put rx buffers with a single 
4k mbuf into the rx queue while the standard says we SHOULD insert buffers 
of at least 65562 bytes. Apple Virtualization refuses to work with this 
configuration. As 65562 is larger than MAXMCLBYTES, we would need to 
rework how we allocate our rx buffers to make this work. For now, we would 
to like to simply disable GUEST_TSO4/6 if MRG_RXBUF is missing.  
Unfortunately, Apple Virtualization still refuses to work unless 
HOST_TSO4/6 is also disabled. Therefore, we disable all TSO if MRG_RXBUF 
is missing.

With lots of input from and tested by helg@

ok?

Cheers,
Stefan

---
 sys/dev/pv/if_vio.c | 48 +++++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c
index 85849e1eb14..6eb2377009c 100644
--- a/sys/dev/pv/if_vio.c
+++ b/sys/dev/pv/if_vio.c
@@ -624,7 +624,7 @@ vio_attach(struct device *parent, struct device *self, void *aux)
 	struct vio_softc *sc = (struct vio_softc *)self;
 	struct virtio_softc *vsc = (struct virtio_softc *)parent;
 	struct virtio_attach_args *va = aux;
-	int i, r, tx_max_segments;
+	int i, r, tx_max_segments, want_tso = 1;
 	struct ifnet *ifp = &sc->sc_ac.ac_if;
 
 	if (vsc->sc_child != NULL) {
@@ -637,24 +637,48 @@ vio_attach(struct device *parent, struct device *self, void *aux)
 
 	vsc->sc_child = self;
 	vsc->sc_ipl = IPL_NET | IPL_MPSAFE;
-	vsc->sc_driver_features = VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS |
-	    VIRTIO_NET_F_CTRL_VQ | VIRTIO_NET_F_CTRL_RX |
-	    VIRTIO_NET_F_MRG_RXBUF | VIRTIO_NET_F_CSUM |
-	    VIRTIO_F_RING_EVENT_IDX | VIRTIO_NET_F_GUEST_CSUM;
 
+negotiate:
+	vsc->sc_driver_features = VIRTIO_F_RING_EVENT_IDX;
+	vsc->sc_driver_features |= VIRTIO_NET_F_MAC;
+	vsc->sc_driver_features |= VIRTIO_NET_F_STATUS;
+	vsc->sc_driver_features |= VIRTIO_NET_F_CTRL_VQ;
+	vsc->sc_driver_features |= VIRTIO_NET_F_CTRL_RX;
+	vsc->sc_driver_features |= VIRTIO_NET_F_MRG_RXBUF;
+	vsc->sc_driver_features |= VIRTIO_NET_F_CSUM;
+	vsc->sc_driver_features |= VIRTIO_NET_F_GUEST_CSUM;
+	if (want_tso) {
+		vsc->sc_driver_features |= VIRTIO_NET_F_CTRL_GUEST_OFFLOADS;
+		vsc->sc_driver_features |= VIRTIO_NET_F_GUEST_TSO4;
+		vsc->sc_driver_features |= VIRTIO_NET_F_GUEST_TSO6;
+		vsc->sc_driver_features |= VIRTIO_NET_F_HOST_TSO4;
+		vsc->sc_driver_features |= VIRTIO_NET_F_HOST_TSO6;
+	}
 	if (va->va_nintr > 3 && ncpus > 1)
 		vsc->sc_driver_features |= VIRTIO_NET_F_MQ;
 
-	vsc->sc_driver_features |= VIRTIO_NET_F_HOST_TSO4;
-	vsc->sc_driver_features |= VIRTIO_NET_F_HOST_TSO6;
-
-	vsc->sc_driver_features |= VIRTIO_NET_F_CTRL_GUEST_OFFLOADS;
-	vsc->sc_driver_features |= VIRTIO_NET_F_GUEST_TSO4;
-	vsc->sc_driver_features |= VIRTIO_NET_F_GUEST_TSO6;
-
 	if (virtio_negotiate_features(vsc, virtio_net_feature_names) != 0)
 		goto err;
 
+	if ((virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO4) ||
+	     virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO6)) &&
+	    (!virtio_has_feature(vsc, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) ||
+	     !virtio_has_feature(vsc, VIRTIO_NET_F_MRG_RXBUF))) {
+		/*
+		 * We only support VIRTIO_NET_F_GUEST_TSO4/6 if we also have
+		 * both VIRTIO_NET_F_CTRL_GUEST_OFFLOADS and
+		 * VIRTIO_NET_F_MRG_RXBUF. In order to make this known to the
+		 * hypervisor, we need to redo feature negotiation.
+		 *
+		 * Unfortunately, Apple Virtualisation does not work with
+		 * only VIRTIO_NET_F_HOST_TSO4/6. Therefore we disable all TSO
+		 * in the fallback case.
+		 */
+		want_tso = 0;
+		virtio_reset(vsc);
+		goto negotiate;
+	}
+
 	if (virtio_has_feature(vsc, VIRTIO_NET_F_MQ)) {
 		i = virtio_read_device_config_2(vsc,
 		    VIRTIO_NET_CONFIG_MAX_QUEUES);
-- 
2.49.0