Subject: Re: SoC ideas - mbuf API
To: None <pavel@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-net
Date: 06/26/2006 19:04:52
--NextPart-20060626190011-0124300
Content-Type: Text/Plain; charset=us-ascii

> The 1st problem: if the code does not use m_pullup/pulldown when it should, it
> works in most cases, but when it encounters a packet fragmented in a right
> way, it malfunctions.
> 
> Example: kern/29014.

i've played with some debug code in mtod() while ago.  (a patch attached)

YAMAMOTO Takashi

--NextPart-20060626190011-0124300
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="mtod_check.diff"

Index: kern/uipc_mbuf.c
===================================================================
--- kern/uipc_mbuf.c	(revision 947)
+++ kern/uipc_mbuf.c	(working copy)
@@ -1367,3 +1367,20 @@ m_getptr(struct mbuf *m, int loc, int *o
 
 	return (NULL);
 }
+
+#if 1
+void *
+_mtod_check(const struct mbuf *m, size_t sz)
+{
+
+	/*
+	 * strictly this isn't a requiremnt.
+	 * but it's likely a bug.
+	 */
+
+	if (m->m_len < sz)
+		panic("mtod: too short mbuf %d < %zu", m->m_len, sz);
+
+	return m->m_data;
+}
+#endif
Index: dev/ic/rtl81x9.c
===================================================================
--- dev/ic/rtl81x9.c	(revision 947)
+++ dev/ic/rtl81x9.c	(working copy)
@@ -1295,7 +1295,7 @@ STATIC void rtk_start(ifp)
 		 * if the packet is too small, copy it too, so we're sure
 		 * so have enouth room for the pad buffer.
 		 */
-		if ((mtod(m_head, uintptr_t) & 3) != 0 ||
+		if ((((uintptr_t)mtod(m_head, const char *)) & 3) != 0 ||
 		    m_head->m_pkthdr.len < ETHER_PAD_LEN ||
 		    bus_dmamap_load_mbuf(sc->sc_dmat, txd->txd_dmamap,
 			m_head, BUS_DMA_WRITE|BUS_DMA_NOWAIT) != 0) {
Index: dev/ic/tulip.c
===================================================================
--- dev/ic/tulip.c	(revision 990)
+++ dev/ic/tulip.c	(working copy)
@@ -753,7 +753,8 @@ tlp_start(ifp)
 		 * likely we'll trip the alignment test than the
 		 * more-than-one-segment test.
 		 */
-		if ((sc->sc_ntxsegs == 1 && (mtod(m0, uintptr_t) & 3) != 0) ||
+		if ((sc->sc_ntxsegs == 1 &&
+		    (((uintptr_t)mtod(m0, const char *)) & 3) != 0) ||
 		    bus_dmamap_load_mbuf(sc->sc_dmat, dmamap, m0,
 		      BUS_DMA_WRITE|BUS_DMA_NOWAIT) != 0) {
 			MGETHDR(m, M_DONTWAIT, MT_DATA);
Index: net/bpf.c
===================================================================
--- net/bpf.c	(revision 976)
+++ net/bpf.c	(working copy)
@@ -263,11 +263,11 @@ bpf_movein(uio, linktype, mtu, mp, sockp
 		m->m_len -= align;
 	}
 
-	error = uiomove(mtod(m, void *), len, uio);
+	error = uiomove(mtod(m, char *), len, uio);
 	if (error)
 		goto bad;
 	if (hlen != 0) {
-		memcpy(sockp->sa_data, mtod(m, void *), hlen);
+		memcpy(sockp->sa_data, mtod(m, const char *), hlen);
 		m->m_data += hlen; /* XXX */
 		len -= hlen;
 	}
@@ -1199,7 +1199,7 @@ bpf_mcpy(void *dst_arg, const void *src_
 		if (m == 0)
 			panic("bpf_mcpy");
 		count = min(m->m_len, len);
-		memcpy(dst, mtod(m, void *), count);
+		memcpy(dst, mtod(m, const char *), count);
 		m = m->m_next;
 		dst += count;
 		len -= count;
@@ -1274,7 +1274,7 @@ bpf_mtap(void *arg, struct mbuf *m)
 
 	if (pktlen == m->m_len) {
 		cpfn = memcpy;
-		marg = mtod(m, void *);
+		marg = mtod(m, char *);
 		buflen = pktlen;
 	} else {
 		cpfn = bpf_mcpy;
Index: netkey/key.c
===================================================================
--- netkey/key.c	(revision 976)
+++ netkey/key.c	(working copy)
@@ -7851,7 +7851,7 @@ sysctl_net_key_dumpsa(SYSCTLFN_ARGS)
 		for (n = m; n; n = n->m_next) {
 			len =  (ep - p < n->m_len) ?
 				ep - p : n->m_len;
-			error = copyout(mtod(n, const void *), p, len);
+			error = copyout(mtod(n, const char *), p, len);
 			p += len;
 			if (error)
 				break;
@@ -7897,7 +7897,7 @@ sysctl_net_key_dumpsp(SYSCTLFN_ARGS)
 		for (n = m; n; n = n->m_next) {
 			len =  (ep - p < n->m_len) ?
 				ep - p : n->m_len;
-			error = copyout(mtod(n, const void *), p, len);
+			error = copyout(mtod(n, const char *), p, len);
 			p += len;
 			if (error)
 				break;
Index: sys/mbuf.h
===================================================================
--- sys/mbuf.h	(revision 900)
+++ sys/mbuf.h	(working copy)
@@ -122,7 +122,14 @@ struct mowner {
  * Macros for type conversion
  * mtod(m,t) -	convert mbuf pointer to data pointer of correct type
  */
+#if 0
 #define	mtod(m,t)	((t)((m)->m_data))
+#else
+struct mbuf;
+void *_mtod_check(const struct mbuf *, size_t);
+#define	_sz(t) (sizeof(*((t)0)))
+#define	mtod(m, t)	((t)(_mtod_check((m), _sz(t))))
+#endif
 
 /* header at beginning of each mbuf: */
 struct m_hdr {

--NextPart-20060626190011-0124300--