svn commit: r277203 - in head/sys: kern sys
Robert Watson
rwatson at FreeBSD.org
Wed Jan 14 23:44:01 UTC 2015
Author: rwatson
Date: Wed Jan 14 23:44:00 2015
New Revision: 277203
URL: https://svnweb.freebsd.org/changeset/base/277203
Log:
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
Modified:
head/sys/kern/uipc_mbuf.c
head/sys/sys/mbuf.h
Modified: head/sys/kern/uipc_mbuf.c
==============================================================================
--- head/sys/kern/uipc_mbuf.c Wed Jan 14 23:34:00 2015 (r277202)
+++ head/sys/kern/uipc_mbuf.c Wed Jan 14 23:44:00 2015 (r277203)
@@ -88,11 +88,38 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, m_defrag
* 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 *
Modified: head/sys/sys/mbuf.h
==============================================================================
--- head/sys/sys/mbuf.h Wed Jan 14 23:34:00 2015 (r277202)
+++ head/sys/sys/mbuf.h Wed Jan 14 23:44:00 2015 (r277203)
@@ -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
More information about the svn-src-head
mailing list