Index | Thread | Search

From:
Alexander Bluhm <alexander.bluhm@gmx.net>
Subject:
revert m_defrag header align
To:
tech@openbsd.org
Date:
Fri, 23 Feb 2024 11:55:43 +0100

Download raw body.

Thread
Hi,

I think we should revert the diff that aligns the mbuf to IP header
in m_defrag().  The cause of the change was that ether_extract_headers()
crashed in em(4) when it was reading TCP header for offloading on
sparc64.  This has been fixed by using memcpy().

Now viommu_dvmamap_load_raw() on sparc64 calls iommu_iomap_insert_page()
twice as often with overlapping pages.  m_defrag() is intended as
last resort to make DMA transfers to the hardware.  Therefore page
alingment is more important than IP header alignment.

ok?

bluhm

Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
diff -u -p -r1.289 uipc_mbuf.c
--- kern/uipc_mbuf.c	21 Feb 2024 09:28:29 -0000	1.289
+++ kern/uipc_mbuf.c	23 Feb 2024 10:45:15 -0000
@@ -545,29 +545,27 @@ m_purge(struct mbuf *m)
  * mbuf chain defragmenter. This function uses some evil tricks to defragment
  * an mbuf chain into a single buffer without changing the mbuf pointer.
  * This needs to know a lot of the mbuf internals to make this work.
+ * The resulting mbuf is not aligned to IP header to assist DMA transfers.
  */
 int
 m_defrag(struct mbuf *m, int how)
 {
 	struct mbuf *m0;
-	unsigned int adj;
 
 	if (m->m_next == NULL)
 		return (0);
 
 	KASSERT(m->m_flags & M_PKTHDR);
 
-	adj = mtod(m, unsigned long) & (sizeof(long) - 1);
 	if ((m0 = m_gethdr(how, m->m_type)) == NULL)
 		return (ENOBUFS);
-	if (m->m_pkthdr.len + adj > MHLEN) {
-		MCLGETL(m0, how, m->m_pkthdr.len + adj);
+	if (m->m_pkthdr.len > MHLEN) {
+		MCLGETL(m0, how, m->m_pkthdr.len);
 		if (!(m0->m_flags & M_EXT)) {
 			m_free(m0);
 			return (ENOBUFS);
 		}
 	}
-	m0->m_data += adj;
 	m_copydata(m, 0, m->m_pkthdr.len, mtod(m0, caddr_t));
 	m0->m_pkthdr.len = m0->m_len = m->m_pkthdr.len;
 
@@ -586,9 +584,9 @@ m_defrag(struct mbuf *m, int how)
 		memcpy(&m->m_ext, &m0->m_ext, sizeof(struct mbuf_ext));
 		MCLINITREFERENCE(m);
 		m->m_flags |= m0->m_flags & (M_EXT|M_EXTWR);
-		m->m_data = m->m_ext.ext_buf + adj;
+		m->m_data = m->m_ext.ext_buf;
 	} else {
-		m->m_data = m->m_pktdat + adj;
+		m->m_data = m->m_pktdat;
 		memcpy(m->m_data, m0->m_data, m0->m_len);
 	}
 	m->m_pkthdr.len = m->m_len = m0->m_len;