In order to support ongoing work to implement variable-size mbufs, and

more generally make it easier to extend 'struct mbuf in the future', make
a number of changes to the data structure:

- As we anticipate embedding mbufs headers within variable-size regions of
  memory in the future, change the definitions of byte arrays embedded in
  mbufs to be of size [0] rather than [MLEN] and [MHLEN].  In fact, the
  cxgbe driver already uses 'struct mbuf' on the front of other storage
  sizes, but we would like the global mbuf allocator do be able to do this
  as well.

- Fold 'struct m_hdr' into 'struct mbuf' itself, eliminating a set of
  macros that aliased 'mh_foo' field names to 'm_foo' names such as
  'm_next'.  These present a particular problem as we would like to add
  new mbuf-header fields -- e.g., 'm_size' -- that, if similarly named via
  macros, would introduce collisions with many other variable names in the
  kernel.

- Rename 'struct m_ext' to 'struct struct_m_ext' so that we can add
  compile-time assertions without bumping into the still-extant 'm_ext'
  macro.

- Remove the MSIZE compile-time assertion for 'struct mbuf', but add new
  assertions for alignment of embedded data arrays (64-bit alignment even
  on 32-bit platforms), and for the sizes the mbuf header, packet header,
  and m_ext structure.

- Document that these assertions exist in comments in mbuf.h.

This change is not intended to cause (non-trivial) behavioural
differences, but is a precursor to further mbuf-allocator work.

Differential Revision:	https://reviews.freebsd.org/D1483
Reviewed by:	bz, gnn, np, glebius ("go ahead, I trust you")
Sponsored by:	EMC / Isilon Storage Division

2 files changed, 68 insertions, 31 deletions

diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index 0c583d7..7a5f624 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -88,11 +88,38 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, m_defragrandomfailures, CTLFLAG_RW,
  * Ensure the correct size of various mbuf parameters.  It could be off due
  * to compiler-induced padding and alignment artifacts.
  */
-CTASSERT(sizeof(struct mbuf) == MSIZE);
 CTASSERT(MSIZE - offsetof(struct mbuf, m_dat) == MLEN);
 CTASSERT(MSIZE - offsetof(struct mbuf, m_pktdat) == MHLEN);
 
 /*
+ * mbuf data storage should be 64-bit aligned regardless of architectural
+ * pointer size; check this is the case with and without a packet header.
+ */
+CTASSERT(offsetof(struct mbuf, m_dat) % 8 == 0);
+CTASSERT(offsetof(struct mbuf, m_pktdat) % 8 == 0);
+
+/*
+ * While the specific values here don't matter too much (i.e., +/- a few
+ * words), we do want to ensure that changes to these values are carefully
+ * reasoned about and properly documented.  This is especially the case as
+ * network-protocol and device-driver modules encode these layouts, and must
+ * be recompiled if the structures change.  Check these values at compile time
+ * against the ones documented in comments in mbuf.h.
+ *
+ * NB: Possibly they should be documented there via #define's and not just
+ * comments.
+ */
+#if defined(__LP64__)
+CTASSERT(offsetof(struct mbuf, m_dat) == 32);
+CTASSERT(sizeof(struct pkthdr) == 56);
+CTASSERT(sizeof(struct struct_m_ext) == 48);
+#else
+CTASSERT(offsetof(struct mbuf, m_dat) == 24);
+CTASSERT(sizeof(struct pkthdr) == 48);
+CTASSERT(sizeof(struct struct_m_ext) == 28);
+#endif
+
+/*
  * m_get2() allocates minimum mbuf that would fit "size" argument.
  */
 struct mbuf *
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index 91fffdc..2e94d2a 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -60,9 +60,15 @@
  * MLEN is data length in a normal mbuf.
  * MHLEN is data length in an mbuf with pktheader.
  * MINCLSIZE is a smallest amount of data that should be put into cluster.
+ *
+ * Compile-time assertions in uipc_mbuf.c test these values to ensure that
+ * they are sensible.
  */
-#define	MLEN		((int)(MSIZE - sizeof(struct m_hdr)))
-#define	MHLEN		((int)(MLEN - sizeof(struct pkthdr)))
+struct mbuf;
+#define	MHSIZE		offsetof(struct mbuf, M_dat.M_databuf)
+#define	MPKTHSIZE	offsetof(struct mbuf, M_dat.MH.MH_dat.MH_databuf)
+#define	MLEN		((int)(MSIZE - MHSIZE))
+#define	MHLEN		((int)(MSIZE - MPKTHSIZE))
 #define	MINCLSIZE	(MHLEN + 1)
 
 #ifdef _KERNEL
@@ -87,23 +93,6 @@ struct mb_args {
 #endif /* _KERNEL */
 
 /*
- * Header present at the beginning of every mbuf.
- * Size ILP32: 24
- *	 LP64: 32
- */
-struct m_hdr {
-	struct mbuf	*mh_next;	/* next buffer in chain */
-	struct mbuf	*mh_nextpkt;	/* next chain in queue/record */
-	caddr_t		 mh_data;	/* location of data */
-	int32_t		 mh_len;	/* amount of data in this mbuf */
-	uint32_t	 mh_type:8,	/* type of data in this mbuf */
-			 mh_flags:24;	/* flags; see below */
-#if !defined(__LP64__)
-	uint32_t	 mh_pad;	/* pad for 64bit alignment */
-#endif
-};
-
-/*
  * Packet tag structure (see below for details).
  */
 struct m_tag {
@@ -118,6 +107,8 @@ struct m_tag {
  * Record/packet header in first mbuf of chain; valid only if M_PKTHDR is set.
  * Size ILP32: 48
  *	 LP64: 56
+ * Compile-time assertions in uipc_mbuf.c test these values to ensure that
+ * they are correct.
  */
 struct pkthdr {
 	struct ifnet	*rcvif;		/* rcv interface */
@@ -166,8 +157,10 @@ struct pkthdr {
  * set.
  * Size ILP32: 28
  *	 LP64: 48
+ * Compile-time assertions in uipc_mbuf.c test these values to ensure that
+ * they are correct.
  */
-struct m_ext {
+struct struct_m_ext {
 	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 */
@@ -184,24 +177,41 @@ struct m_ext {
  * purposes.
  */
 struct mbuf {
-	struct m_hdr	m_hdr;
+	/*
+	 * Header present at the beginning of every mbuf.
+	 * Size ILP32: 24
+	 *      LP64: 32
+	 * Compile-time assertions in uipc_mbuf.c test these values to ensure
+	 * that they are correct.
+	 */
+	struct mbuf	*m_next;	/* next buffer in chain */
+	struct mbuf	*m_nextpkt;	/* next chain in queue/record */
+	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;	/* flags; see below */
+#if !defined(__LP64__)
+	uint32_t	 m_pad;		/* pad for 64bit alignment */
+#endif
+
+	/*
+	 * A set of optional headers (packet header, external storage header)
+	 * and internal data storage.  Historically, these arrays were sized
+	 * to MHLEN (space left after a packet header) and MLEN (space left
+	 * after only a regular mbuf header); they are now variable size in
+	 * order to support future work on variable-size mbufs.
+	 */
 	union {
 		struct {
 			struct pkthdr	MH_pkthdr;	/* M_PKTHDR set */
 			union {
-				struct m_ext	MH_ext;	/* M_EXT set */
-				char		MH_databuf[MHLEN];
+				struct struct_m_ext	MH_ext;	/* M_EXT set */
+				char		MH_databuf[0];
 			} MH_dat;
 		} MH;
-		char	M_databuf[MLEN];		/* !M_PKTHDR, !M_EXT */
+		char	M_databuf[0];			/* !M_PKTHDR, !M_EXT */
 	} M_dat;
 };
-#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_pkthdr	M_dat.MH.MH_pkthdr
 #define	m_ext		M_dat.MH.MH_dat.MH_ext
 #define	m_pktdat	M_dat.MH.MH_dat.MH_databuf