Re: sys/vio: align receive buffers

Kirill A. Korinsky <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
On Wed, 10 Jun 2026 19:47:48 +0200,
Kirill A. Korinsky <[email protected]> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.