From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: sys/vio: align receive buffers
To: tech@openbsd.org
Date: Wed, 17 Jun 2026 11:30:56 +0200

On Wed, 10 Jun 2026 19:47:48 +0200,
Kirill A. Korinsky <kirill@korins.ky> wrote:
> 
> tech@,
> 
> I'd like to move my diff from bugs@ to tech@.
> 
> Context: OpenBSD/armv7 in Qemu crashes with vio NIC at alignment fault,
> See: https://marc.info/?l=openbsd-bugs&m=178084629495768&w=2
> 
> Why? vio removes the virtio net header before handing packets to
> the Ethernet input path. With the 12 byte modern virtio net header this
> leaves the Ethernet frame at an address where the following IP header is not
> 32 bit aligned; armv7 traps on the resulting unaligned access while
> processing DHCP traffic.
> 
> Ok?
>

Anyone?

Index: sys/dev/pv/if_vio.c
===================================================================
RCS file: /home/cvs/src/sys/dev/pv/if_vio.c,v
diff -u -p -r1.79 if_vio.c
--- sys/dev/pv/if_vio.c	16 Apr 2026 21:00:00 -0000	1.79
+++ sys/dev/pv/if_vio.c	7 Jun 2026 22:28:23 -0000
@@ -297,6 +297,7 @@ struct vio_softc {
 	uint16_t		sc_rxr_lwm;
 	int			sc_tx_slots_per_req;
 	int			sc_rx_mbuf_size;
+	int			sc_rx_mbuf_offset;
 
 	enum vio_ctrl_state	sc_ctrl_inuse;
 
@@ -747,6 +748,8 @@ negotiate:
 	} else {
 		sc->sc_hdr_size = offsetof(struct virtio_net_hdr, num_buffers);
 	}
+	sc->sc_rx_mbuf_offset = (ETHER_ALIGN + sizeof(uint32_t) -
+	    sc->sc_hdr_size % sizeof(uint32_t)) % sizeof(uint32_t);
 
 	ifp->if_capabilities = 0;
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
@@ -780,9 +783,11 @@ negotiate:
 	if (!virtio_has_feature(vsc, VIRTIO_NET_F_MRG_RXBUF)) {
 		if (device_mtu)
 			sc->sc_rx_mbuf_size = MAX(sc->sc_rx_mbuf_size,
-			    device_mtu + sc->sc_hdr_size + ETHER_HDR_LEN);
+			    sc->sc_rx_mbuf_offset + device_mtu +
+			    sc->sc_hdr_size + ETHER_HDR_LEN);
 		ifp->if_hardmtu = MIN(ifp->if_hardmtu,
-		    sc->sc_rx_mbuf_size - sc->sc_hdr_size - ETHER_HDR_LEN);
+		    sc->sc_rx_mbuf_size - sc->sc_rx_mbuf_offset -
+		    sc->sc_hdr_size - ETHER_HDR_LEN);
 	}
 
 	txsize = ifp->if_hardmtu + sc->sc_hdr_size + ETHER_HDR_LEN;
@@ -790,13 +795,16 @@ negotiate:
 	    virtio_has_feature(vsc, VIRTIO_NET_F_HOST_TSO6))
 		txsize = MAXMCLBYTES + sc->sc_hdr_size;
 
-	rxsize = ifp->if_hardmtu + sc->sc_hdr_size + ETHER_HDR_LEN;
+	rxsize = ifp->if_hardmtu + sc->sc_rx_mbuf_offset +
+	    sc->sc_hdr_size + ETHER_HDR_LEN;
 	if (virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO4) ||
 	    virtio_has_feature(vsc, VIRTIO_NET_F_GUEST_TSO6))
-		rxsize = MAXMCLBYTES + sc->sc_hdr_size;
+		rxsize = MAXMCLBYTES + sc->sc_rx_mbuf_offset +
+		    sc->sc_hdr_size;
 
 	printf("\n");
-	sc->sc_rxr_lwm = 2 * howmany(rxsize, sc->sc_rx_mbuf_size);
+	sc->sc_rxr_lwm = 2 * howmany(rxsize,
+	    sc->sc_rx_mbuf_size - sc->sc_rx_mbuf_offset);
 
 	/* defrag for longer mbuf chains */
 	tx_max_segments = 16;
@@ -1438,7 +1446,7 @@ vio_add_rx_mbuf(struct vio_softc *sc, st
 		return ENOBUFS;
 	vioq->viq_rxmbufs[i] = m;
 	m->m_len = m->m_pkthdr.len = m->m_ext.ext_size;
-	/* XXX m_adj ETHER_ALIGN ? */
+	m_adj(m, sc->sc_rx_mbuf_offset);
 	r = bus_dmamap_load_mbuf(sc->sc_virtio->sc_dmat,
 	    vioq->viq_rxdmamaps[i], m, BUS_DMA_READ|BUS_DMA_NOWAIT);
 	if (r) {
@@ -1509,7 +1517,8 @@ vio_populate_rx_mbufs(struct vio_softc *
 			    0, sc->sc_hdr_size, 0);
 			virtio_enqueue_p(vq, slot, vioq->viq_rxdmamaps[slot],
 			    sc->sc_hdr_size,
-			    sc->sc_rx_mbuf_size - sc->sc_hdr_size, 0);
+			    sc->sc_rx_mbuf_size - sc->sc_rx_mbuf_offset -
+			    sc->sc_hdr_size, 0);
 		}
 		virtio_enqueue_commit(vsc, vq, slot, 0);
 		done = 1;

-- 
wbr, Kirill