freebsd11_network: Implement m_ext reference-counting and M_NOFREE.

Some FreeBSD developers report that this is required for HT mode, which
might explain why it's so broken on Haiku. I was also told the iwm driver
requires it for multi-frame RX, but as per the previous commit it still
KDLs even with it.

This commit also includes a refactor of the mbuf header implementation,
which now more closely mirrors FreeBSD's.

src/libs/compat/freebsd11_network/compat/sys/cdefs.h

@@ -172,6 +172,22 @@
 #define	__rangeof(type, start, end) \
 	(__offsetof(type, end) - __offsetof(type, start))
 
+/*
+ * Given the pointer x to the member m of the struct s, return
+ * a pointer to the containing structure.  When using GCC, we first
+ * assign pointer x to a local variable, to check that its type is
+ * compatible with member m.
+ */
+#if __GNUC_PREREQ__(3, 1)
+#define	__containerof(x, s, m) ({					\
+	const volatile __typeof(((s *)0)->m) *__x = (x);		\
+	__DEQUALIFY(s *, (const volatile char *)__x - __offsetof(s, m));\
+})
+#else
+#define	__containerof(x, s, m)						\
+	__DEQUALIFY(s *, (const volatile char *)(x) - __offsetof(s, m))
+#endif
+
 /*
  * Compiler-dependent macros to help declare dead (non-returning) and
  * pure (no side effects) functions, and unused variables.  They are
@@ -312,7 +328,11 @@
 #endif
 
 #ifndef	__DEVOLATILE
-#define	__DEVOLATILE(type, var)	((type)(__uintptr_t)(volatile void *)(var))
+#define	__DEVOLATILE(type, var)	((type)(uintptr_t)(volatile void *)(var))
+#endif
+
+#ifndef	__DEQUALIFY
+#define	__DEQUALIFY(type, var)	((type)(uintptr_t)(const volatile void *)(var))
 #endif
 
 #if __GNUC_PREREQ__(4,6) && !defined(__cplusplus)

src/libs/compat/freebsd11_network/compat/sys/mbuf-fbsd.h

@@ -25,71 +25,92 @@
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
- *
- *	@(#)mbuf.h	8.5 (Berkeley) 2/19/95
- * $FreeBSD: src/sys/sys/mbuf.h,v 1.170.2.6 2006/03/23 23:24:32 sam Exp $
  */
 #ifndef _FBSD_COMPAT_SYS_MBUF_FBSD_H_
 #define _FBSD_COMPAT_SYS_MBUF_FBSD_H_
 
 /*
- * Set the m_data pointer of a newly-allocated mbuf (m_get/MGET) to place
+ * Return the address of the start of the buffer associated with an mbuf,
- * an object of the specified size at the end of the mbuf, longword aligned.
+ * handling external storage, packet-header mbufs, and regular data mbufs.
  */
-#define	M_ALIGN(m, len) do {						\
+#define	M_START(m)							\
-	(m)->m_data += (MLEN - (len)) & ~(sizeof(long) - 1);		\
+	(((m)->m_flags & M_EXT) ? (m)->m_ext.ext_buf :			\
-} while (0)
+	 ((m)->m_flags & M_PKTHDR) ? &(m)->m_pktdat[0] :		\
+	 &(m)->m_dat[0])
 
 /*
- * As above, for mbufs allocated with m_gethdr/MGETHDR
+ * Return the size of the buffer associated with an mbuf, handling external
- * or initialized by M_COPY_PKTHDR.
+ * storage, packet-header mbufs, and regular data mbufs.
  */
-#define	MH_ALIGN(m, len) do {						\
+#define	M_SIZE(m)							\
-	(m)->m_data += (MHLEN - (len)) & ~(sizeof(long) - 1);		\
+	(((m)->m_flags & M_EXT) ? (m)->m_ext.ext_size :			\
-} while (0)
+	 ((m)->m_flags & M_PKTHDR) ? MHLEN :				\
+	 MLEN)
 
 /*
-#define	MEXT_IS_REF(m)	(((m)->m_ext.ref_cnt != NULL)			\
+ * Set the m_data pointer of a newly allocated mbuf to place an object of the
-    && (*((m)->m_ext.ref_cnt) > 1))
+ * specified size at the end of the mbuf, longword aligned.
+ *
+ * NB: Historically, we had M_ALIGN(), MH_ALIGN(), and MEXT_ALIGN() as
+ * separate macros, each asserting that it was called at the proper moment.
+ * This required callers to themselves test the storage type and call the
+ * right one.  Rather than require callers to be aware of those layout
+ * decisions, we centralize here.
  */
-#define MEXT_IS_REF(m)	0
+static __inline void
+m_align(struct mbuf *m, int len)
+{
+#ifdef INVARIANTS
+	const char *msg = "%s: not a virgin mbuf";
+#endif
+	int adjust;
+
+	KASSERT(m->m_data == M_START(m), (msg, __func__));
+
+	adjust = M_SIZE(m) - len;
+	m->m_data += adjust &~ (sizeof(long)-1);
+}
+
+#define	M_ALIGN(m, len)		m_align(m, len)
+#define	MH_ALIGN(m, len)	m_align(m, len)
+#define	MEXT_ALIGN(m, len)	m_align(m, len)
 
 /*
- * Evaluate TRUE if it's safe to write to the mbuf m's data region (this
+ * Evaluate TRUE if it's safe to write to the mbuf m's data region (this can
- * can be both the local data payload, or an external buffer area,
+ * be both the local data payload, or an external buffer area, depending on
- * depending on whether M_EXT is set).
+ * whether M_EXT is set).
  */
+#define	M_WRITABLE(m)	(!((m)->m_flags & M_RDONLY) &&			\
+			 (!(((m)->m_flags & M_EXT)) ||			\
+			 (m_extrefcnt(m) == 1)))
 
 /*
-#define	M_WRITABLE(m)	(!((m)->m_flags & M_RDONLY) && (!((m)->m_flags  \
+ * Compute the amount of space available before the current start of data in
-			    & M_EXT) || !MEXT_IS_REF(m)))
+ * an mbuf.
- */
-#define M_WRITABLE(m)	(!((m)->m_flags & M_EXT) || !MEXT_IS_REF(m))
-
-/*
- * Compute the amount of space available
- * before the current start of data in an mbuf.
  *
  * The M_WRITABLE() is a temporary, conservative safety measure: the burden
  * of checking writability of the mbuf data area rests solely with the caller.
+ *
+ * NB: In previous versions, M_LEADINGSPACE() would only check M_WRITABLE()
+ * for mbufs with external storage.  We now allow mbuf-embedded data to be
+ * read-only as well.
  */
 #define	M_LEADINGSPACE(m)						\
-	((m)->m_flags & M_EXT ?						\
+	(M_WRITABLE(m) ? ((m)->m_data - M_START(m)) : 0)
-	    (M_WRITABLE(m) ? (m)->m_data - (m)->m_ext.ext_buf : 0):	\
-	    (m)->m_flags & M_PKTHDR ? (m)->m_data - (m)->m_pktdat :	\
-	    (m)->m_data - (m)->m_dat)
 
 /*
  * Compute the amount of space available after the end of data in an mbuf.
  *
  * The M_WRITABLE() is a temporary, conservative safety measure: the burden
  * of checking writability of the mbuf data area rests solely with the caller.
+ *
+ * NB: In previous versions, M_TRAILINGSPACE() would only check M_WRITABLE()
+ * for mbufs with external storage.  We now allow mbuf-embedded data to be
+ * read-only as well.
  */
 #define	M_TRAILINGSPACE(m)						\
-	((m)->m_flags & M_EXT ?						\
+	(M_WRITABLE(m) ?						\
-	    (M_WRITABLE(m) ? (m)->m_ext.ext_buf + (m)->m_ext.ext_size	\
+		((M_START(m) + M_SIZE(m)) - ((m)->m_data + (m)->m_len)) : 0)
-		- ((m)->m_data + (m)->m_len) : 0) :			\
-	    &(m)->m_dat[MLEN] - ((m)->m_data + (m)->m_len))
 
 /*
  * Arrange to prepend space of size plen to mbuf m.
@@ -123,6 +144,15 @@ m_clrprotoflags(struct mbuf *m)
 	}
 }
 
+static inline u_int
+m_extrefcnt(struct mbuf *m)
+{
+	KASSERT(m->m_flags & M_EXT, ("%s: M_EXT missing", __func__));
+
+	return ((m->m_ext.ext_flags & EXT_FLAG_EMBREF) ? m->m_ext.ext_count :
+		*m->m_ext.ext_cnt);
+}
+
 /* mbufq */
 
 struct mbufq {

src/libs/compat/freebsd11_network/compat/sys/mbuf.h

@@ -89,6 +89,15 @@
 #define EXT_JUMBO9		5		// 9 * 1024 bytes
 #define EXT_NET_DRV		100		// custom ext_buf provided by net driver
 
+#define EXT_EXTREF		255		// has externally maintained ext_cnt ptr
+
+/*
+ * Flags for external mbuf buffer types.
+ * NB: limited to the lower 24 bits.
+ */
+#define EXT_FLAG_EMBREF		0x000001	/* embedded ext_count */
+#define EXT_FLAG_EXTREF		0x000002	/* external ext_cnt, notyet */
+
 #define CSUM_IP			0x0001
 #define CSUM_TCP		0x0002
 #define CSUM_UDP		0x0004
@@ -109,15 +118,6 @@ extern int max_hdr;
 extern int max_datalen;		// MHLEN - max_hdr
 
 
-struct m_hdr {
-	struct mbuf*	mh_next;
-	struct mbuf*	mh_nextpkt;
-	caddr_t			mh_data;
-	int				mh_len;
-	int				mh_flags;
-	short			mh_type;
-};
-
 struct pkthdr {
 	struct ifnet*					rcvif;
 	int								len;
@@ -137,9 +137,14 @@ struct m_tag {
 };
 
 struct m_ext {
-	caddr_t			ext_buf;
+	union {
-	unsigned int	ext_size;
+		volatile u_int	 ext_count;	/* value of ref count info */
-	int				ext_type;
+		volatile u_int	*ext_cnt;	/* pointer to ref count info */
+	};
+	caddr_t		 ext_buf;	 /* start of buffer */
+	uint32_t	 ext_size;	 /* size of buffer, for ext_free */
+	uint32_t	 ext_type:8, /* type of external storage */
+			 ext_flags:24;	 /* external storage mbuf flags */
 };
 
 struct mbuf {
@@ -153,8 +158,14 @@ struct mbuf {
 		SLIST_ENTRY(mbuf)	m_slistpkt;
 		STAILQ_ENTRY(mbuf)	m_stailqpkt;
 	};
+	caddr_t		 m_data;	/* location of data */
+	int32_t		 m_len;		/* amount of data in this mbuf */
+	uint32_t	 m_type:8,	/* type of data in this mbuf */
+			 m_flags:24;
+#if !defined(__LP64__)
+	uint32_t	 m_pad;		/* pad for 64bit alignment */
+#endif
 
-	struct m_hdr m_hdr;
 	union {
 		struct {
 			struct pkthdr	MH_pkthdr;
@@ -168,12 +179,6 @@ struct mbuf {
 };
 
 
-#define m_next		m_hdr.mh_next
-#define m_len		m_hdr.mh_len
-#define m_data		m_hdr.mh_data
-#define m_type		m_hdr.mh_type
-#define m_flags		m_hdr.mh_flags
-#define m_nextpkt	m_hdr.mh_nextpkt
 #define m_act		m_nextpkt
 #define m_pkthdr	M_dat.MH.MH_pkthdr
 #define m_ext		M_dat.MH.MH_dat.MH_ext
@@ -183,7 +188,6 @@ struct mbuf {
 
 void			m_catpkt(struct mbuf *m, struct mbuf *n);
 void			m_adj(struct mbuf*, int);
-void			m_align(struct mbuf*, int);
 int				m_append(struct mbuf*, int, c_caddr_t);
 void			m_cat(struct mbuf*, struct mbuf*);
 int				m_clget(struct mbuf*, int);

src/libs/compat/freebsd11_network/fbsd_mbuf.c

@@ -653,13 +653,29 @@ bad:
 static void
 mb_dupcl(struct mbuf *n, struct mbuf *m)
 {
-	KASSERT((m->m_flags & M_EXT) == M_EXT, ("%s: M_EXT not set", __func__));
+	volatile u_int *refcnt;
-	KASSERT((n->m_flags & M_EXT) == 0, ("%s: M_EXT set", __func__));
 
-	n->m_ext.ext_buf = m->m_ext.ext_buf;
+	KASSERT(m->m_flags & M_EXT, ("%s: M_EXT not set on %p", __func__, m));
-	n->m_ext.ext_size = m->m_ext.ext_size;
+	KASSERT(!(n->m_flags & M_EXT), ("%s: M_EXT set on %p", __func__, n));
-	n->m_ext.ext_type = m->m_ext.ext_type;
+
+	n->m_ext = m->m_ext;
 	n->m_flags |= M_EXT;
+	n->m_flags |= m->m_flags & M_RDONLY;
+
+	/* See if this is the mbuf that holds the embedded refcount. */
+	if (m->m_ext.ext_flags & EXT_FLAG_EMBREF) {
+		refcnt = n->m_ext.ext_cnt = &m->m_ext.ext_count;
+		n->m_ext.ext_flags &= ~EXT_FLAG_EMBREF;
+	} else {
+		KASSERT(m->m_ext.ext_cnt != NULL,
+			("%s: no refcounting pointer on %p", __func__, m));
+		refcnt = m->m_ext.ext_cnt;
+	}
+
+	if (*refcnt == 1)
+		*refcnt += 1;
+	else
+		atomic_add_int(refcnt, 1);
 }
 
 /*
@@ -1057,25 +1073,6 @@ m_unshare(struct mbuf *m0, int how)
 	return (m0);
 }
 
-/*
- * Set the m_data pointer of a newly-allocated mbuf
- * to place an object of the specified size at the
- * end of the mbuf, longword aligned.
- */
-void
-m_align(struct mbuf *m, int len)
-{
-	int adjust;
-
-	if (m->m_flags & M_EXT)
-		adjust = m->m_ext.ext_size - len;
-	else if (m->m_flags & M_PKTHDR)
-		adjust = MHLEN - len;
-	else
-		adjust = MLEN - len;
-	m->m_data += adjust &~ (sizeof(long)-1);
-}
-
 /*
  * Copy an entire packet, including header (which must be present).
  * An optimization of the common case `m_copym(m, 0, M_COPYALL, how)'.

src/libs/compat/freebsd11_network/mbuf.c

@@ -88,11 +88,10 @@ construct_ext_sized_mbuf(struct mbuf *memoryBuffer, int how, int size)
 
 	memoryBuffer->m_data = memoryBuffer->m_ext.ext_buf;
 	memoryBuffer->m_flags |= M_EXT;
-	/* mb->m_ext.ext_free = NULL; */
-	/* mb->m_ext.ext_args = NULL; */
 	memoryBuffer->m_ext.ext_size = size;
 	memoryBuffer->m_ext.ext_type = extType;
-	/* mb->m_ext.ref_cnt = NULL; */
+	memoryBuffer->m_ext.ext_flags = EXT_FLAG_EMBREF;
+	memoryBuffer->m_ext.ext_count = 1;
 
 	return 0;
 }
@@ -227,12 +226,44 @@ m_freem(struct mbuf *memoryBuffer)
 static void
 mb_free_ext(struct mbuf *memoryBuffer)
 {
-	/*
-	if (m->m_ext.ref_count != NULL)
-		panic("unsupported");
-	*/
-
 	object_cache *cache = NULL;
+	volatile u_int *refcnt;
+	struct mbuf *mref;
+	int freembuf;
+
+	KASSERT(memoryBuffer->m_flags & M_EXT, ("%s: M_EXT not set on %p",
+		__func__, memoryBuffer));
+
+	/* See if this is the mbuf that holds the embedded refcount. */
+	if (memoryBuffer->m_ext.ext_flags & EXT_FLAG_EMBREF) {
+		refcnt = &memoryBuffer->m_ext.ext_count;
+		mref = memoryBuffer;
+	} else {
+		KASSERT(memoryBuffer->m_ext.ext_cnt != NULL,
+			("%s: no refcounting pointer on %p", __func__, memoryBuffer));
+		refcnt = memoryBuffer->m_ext.ext_cnt;
+		mref = __containerof(refcnt, struct mbuf, m_ext.ext_count);
+	}
+
+	/*
+	 * Check if the header is embedded in the cluster.  It is
+	 * important that we can't touch any of the mbuf fields
+	 * after we have freed the external storage, since mbuf
+	 * could have been embedded in it.  For now, the mbufs
+	 * embedded into the cluster are always of type EXT_EXTREF,
+	 * and for this type we won't free the mref.
+	 */
+	if (memoryBuffer->m_flags & M_NOFREE) {
+		freembuf = 0;
+		KASSERT(memoryBuffer->m_ext.ext_type == EXT_EXTREF,
+			("%s: no-free mbuf %p has wrong type", __func__, memoryBuffer));
+	} else
+		freembuf = 1;
+
+	/* Free attached storage only if this mbuf is the only reference to it. */
+	if (!(*refcnt == 1 || atomic_add(refcnt, -1) == 1)
+			&& !(freembuf && memoryBuffer != mref))
+		return;
 
 	if (memoryBuffer->m_ext.ext_type == EXT_CLUSTER)
 		cache = sChunkCache;