Index | Thread | Search

From:
Mike Larkin <mlarkin@nested.page>
Subject:
Re: vio: Improve feature negotiation for LRO/TSO
To:
Stefan Fritsch <sf@sfritsch.de>
Cc:
tech@openbsd.org, Jan Klemkow <j.klemkow@wemelug.de>
Date:
Mon, 12 Jan 2026 14:04:15 -0800

Download raw body.

Thread
On Mon, Jan 12, 2026 at 04:18:22PM +0100, Stefan Fritsch wrote:
> 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?
>

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
>